[Jmap] Benjamin Kaduk's Discuss on draft-ietf-jmap-websocket-04: (with DISCUSS and COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-jmap-websocket-04: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-jmap-websocket/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

I support Alissa's Discuss and suggest an approach of making a
definitive statement about "wss://" usage, akin to RFC 8620's "All HTTP
requests MUST use the 'https://' scheme".

I don't understand how adding a "pushState" value to the StateChange
object (Section 4.2.4.1) that reflects the state of "ALL of the data
types in the account" can work, if the regular StateChange and
notification flows are not tied to a specific account (and in fact the
example in Section 7.1.1 of RFC 8620 includes state changes about
multiple accounts in a single StateChange) but rather the authentication
credentials.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

The discussion of compression bears some closer scrutiny, as if we allow
the entire websocket frame to be part of the compression state,
then there may be more avenues for an attacker to inject content into
the same compression state as data that we want to keep somewhat
confidential.  (This is not as bad as, say, the initial CRIME attack
impact, as the authentication credentials are not being repeatedly sent,
but the general principle of mixing attacker-controlled and confidential
data into the same compression domain is the same.)

We could say something in the security considerations about the
potential consequences for a client that sends multiple requests in
parallel without request IDs and gets back responses in a different
order.  (Or just require unique request IDs, I suppose.)

Section 1

   resources.  In such circumstances, authenticating every JMAP API
   request may harm performance.

I'd suggest phrasing this as "reauthenticating for every JMAP API
request" -- we still want to have the API requests be authenticated,
we're just batching them via a persistent connection.

Section 4.1

   The JMAP WebSocket client and JMAP WebSocket server negotiate the use
   of the WebSocket JMAP subprotocol during the WebSocket handshake,
   either via a HTTP/1.1 Upgrade request (see Section 1.3 of [RFC6455])
   or a HTTP/2 Extended CONNECT request (see Section 5 of [RFC8441]).

nit: "either" suggests that these are the only options, unnecessarily
ruling out (e.g.) HTTP/3 websockets.

   header field.  The reply from the server MUST also contain "jmap" in
   its corresponding "Sec-WebSocket-Protocol" header field in order for
   a JMAP subprotocol connection to be established.

   If a client receives a handshake response that does not include
   "jmap" in the "Sec-WebSocket-Protocol" header, then a JMAP
   subprotocol WebSocket connection was not established and the client
   MUST close the WebSocket connection.

[just to note that what the TSV-art reviewer pointed out is valid --
core websockets requires exactly one value in the response]

   The credentials used for authenticating the HTTP request to initiate
   the handshake remain in effect for the duration of the WebSocket
   connection.

I think we should say something about the server closing the websocket
if the underlying credentials expire.

Section 4.2

   Data frame messages in the JMAP subprotocol MUST be of the text type
   and contain UTF-8 encoded data.  The messages MUST be in the form of

nit: RFC 6455 seems to refer to these as being "text frames" or frames
with "text data type", but not "text type".

Also, just to double-check: we're requiring a 1:1 mapping between JMAP
message and websocket frame, with no fragmentation or anything like
that allowed?

Section 4.2.1

   o  id: "String" (optional)

      A client-specified identifier for the request.

I see that RFC 8620 defines an "id" type that is more-specific than just
"String", but also note that it claims to always be server-assigned and
to correspond to "real resources" (in some sense of the term) as opposed
to ephemeral requests, so I can understand if pulling in the additional
guidance about formatting and length is not desired.

   Additionally, the "maxConcurrentRequests" field in the "capabilities"
   object (see Section 2 of [RFC8620]) limits the number of inflight
   requests over the WebSocket.

nit(?): a strict reading of RFC 8620 would be that "maxConcurrent
Requests" applies only to the (traditional) JMAP API endpoint, and the
websocket endpoint is presumably a different end point, which would not
be covered by that strict reading.  So it might be worth phrasing this
more definitively, a la "the 'maxConcurrentRequests' JMAP capability
also applies to requests made on the websocket connection".
(Presumably, also specifying whether the cap is per-endpoint or shared
across traditional requests and websocket ones.)

Section 4.2.4.2

Are we implicitly saying that the WebSocketPushEnable object, unlike the
RFC 8620 PushSubscription object, does have an accountId property?
This does not seem to be reflected in the examples.

   o  pushState: "String" (optional)

      The last "pushState" token that the client received from the
      server.  Upon receipt of a "pushState" token, the server SHOULD
      immediately send all changes since that state token.

What does the server do if there is no provided pushState?

Section 4.2.4.3

What is the granularity at which the WebSocketPushDisable applies?
Per-authentication-credentials?

Section 4.3

Do we want to say anything about having prior "pushState" of "aaa" in
the WebSocketPushEnable message for the HTTP/1.1 example?
I'd also suggest saying something about what triggers the state change
for the push notification, as I don't see how the junk message ("The
quick brown fox jumps over the lazy dog") would affect the Email state.