Emond Papegaaij created WICKET-7197:
---------------------------------------
Summary: Validate the origin of WebSocket handshakes
Key: WICKET-7197
URL: https://issues.apache.org/jira/browse/WICKET-7197
Project: Wicket
Issue Type: New Feature
Components: wicket-core, wicket-native-websocket
Reporter: Emond Papegaaij
Assignee: Martin Tzvetanov Grigorov
Wicket does not validate the origin of a WebSocket handshake by default. Three
opt-in hooks exist and all three are inert out of the box:
* {{WebSocketSettings#connectionFilter}} has no initialiser, so it is null.
* {{WebSocketConnectionOriginFilter}} accepts everything while its
allowed-domains
list is empty, which is its initial state.
* {{AbstractUpgradeFilter#verifyOrigin}} returns true unconditionally, and is
never
called (see the notes below).
For HTTP, {{ResourceIsolationRequestCycleListener}} already implements exactly
this
decision, with a pluggable {{IResourceIsolationPolicy}} and a Fetch Metadata
implementation. WebSocket traffic does not use it:
{{WebSocketAwareResourceIsolationRequestCycleListener}} exempts the WebSocket
handler
from the check.
*That exemption is correct and should stay.* At frame time the available request
object reports its scheme as {{ws}}/{{wss}}, so an Origin-based policy could
never
match a browser's {{Origin}} value; its header map is case-sensitive, so the
lowercase Fetch Metadata lookups miss; and its header values are single-use
enumerations. Removing the exemption would abort every WebSocket frame,
including
same-origin ones.
*The check belongs at the handshake, not at the frame.* Origin is a property of
the
connection: once a socket is accepted, every frame on it comes from whoever
opened
it, so a per-frame check is both redundant and the wrong shape. At handshake
time
there is a real {{HttpServletRequest}} with real headers.
h3. Proposal
# Gate the WebSocket upgrade on {{IResourceIsolationPolicy}}, evaluated once
against
the handshake request, so there is a single place where Wicket decides whether
another origin may talk to the application.
# Teach {{FetchMetadataResourceIsolationPolicy}} about WebSockets. Browsers send
{{Sec-Fetch-Dest: websocket}}, {{Sec-Fetch-Mode: websocket}} and the usual
{{Sec-Fetch-Site: same-origin | same-site | cross-site}} on the upgrade
request. The
policy currently knows {{document}}, {{script}}, {{image}}, {{object}} and
{{embed}};
{{websocket}} needs adding. The header values should be confirmed against
current
browsers before implementing, since this is external behaviour.
# Consider retiring the header-based {{OriginResourceIsolationPolicy}} in
favour of
Fetch Metadata only. Fetch Metadata has shipped in Chrome since 2019, Firefox
since
2021 and Safari since early 2023. {{OriginResourceIsolationPolicy}} implements
only
the two-argument {{isRequestAllowed}}, so it ignores {{RequestType}} and cannot
distinguish a top-level navigation from a subresource load, which makes it
awkward to
keep correct. It is public API, so this needs a deprecation cycle rather than
removal
in a patch release.
# Decide explicitly what a request carrying *no* Fetch Metadata headers means:
allow
(compatible, fail-open) or deny (strict). Today that choice is masked by the
Origin
policy picking up the slack; removing the fallback forces it into the open.
h3. Notes
* {{AbstractUpgradeFilter#verifyOrigin}} is documented as the extension point
for
validating the origin of a handshake ("intended to be overridden by sub-classes
that
wish to verify the origin of a WebSocket request before processing it"), but the
override can never take effect. The only concrete subclass,
{{JavaxWebSocketFilter}},
wraps its own {{FilterConfig}} in {{init()}} specifically to add the WebSocket
path to
the filter's ignored-paths list. The endpoint is registered with the container
by
class scan, so the filter code containing the {{verifyOrigin}} call does not
run for a
handshake in a normal deployment. An application that follows the javadoc and
implements a real origin check believes it is protected and is not, with nothing
logged. Once the handshake is gated on {{IResourceIsolationPolicy}}, removing
{{verifyOrigin}} and its call site in favour of the single enforcement point is
the
cleaner outcome, with the usual deprecation cycle since it is public API.
* {{FetchMetadataResourceIsolationPolicy}} looks its headers up in lowercase.
{{HttpServletRequest#getHeader}} is case-insensitive by specification, so this
is fine
for a real request, but any code path that reads headers from a *copy* of a
request
needs to preserve case-insensitive lookup, or the policy will silently see
nothing and
return its no-header answer.
* Fetch Metadata is sent by the browser whether or not a cookie accompanies the
request, so a handshake-time check does not depend on the container's cookie
configuration. That is the main practical argument for this approach over
anything
cookie-based.
* Neither {{WebSocketSettings#setConnectionFilter}} nor
{{WebSocketConnectionOriginFilter}} appears in the user guide, so an
application that
wants to restrict which origins may open a socket has no documented route to
doing so.
Whatever mechanism comes out of this issue needs to be written down.
* This is hardening, not a vulnerability. A cross-site handshake only reaches an
authenticated session if the browser attaches the container's session cookie to
a
cross-site subresource request, which requires {{SameSite=None}}; modern
browsers
apply lax-by-default, and that cookie is issued by the servlet container, not by
Wicket. Cross-origin WebSocket protection has been opt-in and off by default
since
7.0, so no default was lost and there is no regression. This is worth doing
because
the current situation is confusing and one documented hook does not work.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
