[Spec] Network revocation patches for WebSocket and WebTransport APIs #206
Conversation
|
LGTM % 1 comment |
spec.bs
Outdated
| @@ -2123,6 +2135,16 @@ Issue: This will require a RFC to add a test-only function to the WPT web driver | |||
| 1. [=set/Append=] |nonce| to the user agent's [=network revocation nonce set=]. | |||
|
|
|||
| 1. [=fetch group/terminated|Terminate=] |settings|'s [=fetch/fetch group=]. | |||
|
|
|||
| 1. [=list/For each=] {{WebSocket}} object |webSocket| whose [=relevant settings object=] is | |||
There was a problem hiding this comment.
This algorithm is called in parallel so I don't really think it's valid for us to be inspecting "each" WebIDL WebSocket object of a given settings object, and running algorithms on it that (a) HTML currently runs as a task on the Document's event loop, and (b) the WebSocket spec itself only runs as proper event-loop-bound networking tasks. So we should probably (a) figure out a better way to restructure this, and (b) add a step at the beginning of this algorithm to assert that this is running in parallel, to make it easier to catch this kind of stuff in the future.
There was a problem hiding this comment.
I'm going to move this to the disableUntrustedNetwork() algorithm itself, before the parallel steps. I think that should work.
For WebSockets, the document unloading steps you linked calls the make disappear algorithm, which directly calls fail the websocket connection without first moving into a different event loop.
For WebTransport, the cleanup algorithm is invoked by close(), and it doesn't drop into a different event loop either.
Based on that, I think it should be fine to put it directly in the non-parallel section of disableUntrustedNetwork().
There was a problem hiding this comment.
How does this compare to the implementation?
There was a problem hiding this comment.
The implementation checks for whether the nonce stored internally corresponds to one of the network revoked nonces. Nonces in the spec are not well defined at the moment. The assumption I'm making here is that there's a 1:1 ratio of nonces to fenced frame trees/windows. If we want to explicitly check the nonce value here, that will get more tricky.
I think we can check the nonce for WebTransport, since a WebTransport session references an underlying connection, and from there you can just grab its associated key's second tuple value.
WebSockets are more tricky. The underlying connection it uses is its own separate thing that's not shareable with the fetch connection object. They do have a nonce being generated during the connection process, but I'm wasn't sure if that's actually one that would be usable for what we're trying to do.
As an alternative, I opted to not use nonces in the spec. If you think that is too much of a deviation let me know and I can try to think of a better way to do this. If that is the case, this will probably be either blocked on the existing partition nonce work, or will require extra nonce work that will be blocked on that work.
There was a problem hiding this comment.
I think I'm more concerned about the timing of the revocation, and less about the nonces involved. I just want to make sure we have something in the implementation observable equivalent to the lines this PR is adding to disableUntrustedNetwork().
This ensures that the following things happen for both
WebTransportandWebSocketobjects:Preview | Diff