Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider whether we need sub-protocol negotiation #536

Open
vasilvv opened this issue Aug 15, 2023 · 8 comments · May be fixed by #598
Open

Reconsider whether we need sub-protocol negotiation #536

vasilvv opened this issue Aug 15, 2023 · 8 comments · May be fixed by #598
Assignees

Comments

@vasilvv
Copy link
Contributor

vasilvv commented Aug 15, 2023

Both WebSocket and QUIC have a subprotocol negotiation mechanism (QUIC mandates ALPN, WebSocket has Sec-WebSocket-Protocol). When we designed WebTransport originally, we decided that between the application streams and the URL path/query, those are redundant.

Reasons why we may want to reconsider this:

  • People design applications for QUIC, and they may rely on ALPN for things like version negotiation. Supporting a similar mechanism in WebTransport would make porting those protocols easier.
  • QUIC requires ALPN for protection against cross-protocol attacks, which may be a useful property here too.

Reasons why we may want to keep it as-is:

  • Mitigating cross-protocol attacks might not be particularly useful. While HTTP itself does support a similar mechanism (Content-Type/Accept), Web developers often assume that the URL implies the format it contains (i.e. if you have an endpoint called /api/update-entry, you know what can be POSTed onto it, and what format the response will be).
@jan-ivar
Copy link
Member

jan-ivar commented Aug 29, 2023

Meeting:

  • would make it easier to port raw QUIC apps to WebTransport that don't have an obvious URL
  • what about https://myurl/protocol
  • but e.g. a https://myorigin/stream?id=queryargs
  • can be added later
  • QUIC provides ALPN, so if it's not available in WebTransport then MOQ will need to do something else.
  • ALPN is like port numbers for TLS. Seems a bit dangerous to expose if we don't have to.

@martinthomson
Copy link
Member

@LPardue
Copy link

LPardue commented Aug 30, 2023

I noted the ALPN header RFC during the call, I think the discussion us summarized under the final point Jan-Iver captured

@jan-ivar
Copy link
Member

Meeting:

  • Do we strictly needed? No. Would it be useful? Yes.
  • Request is from MOQ transport.
  • Proposal is you offer a list of strings and the server has to pick one of those
  • Would require a new header in the IETF.
  • Or could put stuff in the URL, except there's no such channel server side
  • Do we want it?
  • Use case: versioning negotiating in MOQ we want it to negotiate protocol version, same way HTTP/3 used to negotiate them. Typically people organize their stacks to receive ALPN, look up some code to handle specific protocol. Handler determination logic. This would let you port from QUIC to WebTransport. In WebTransport we already require you to read headers before payloads.
  • There is utility of being able to receive this data before you have any application stream/datagram handling.
  • API would be constructor options to add a protocol strings.
  • Why is doing it in the app-layer insufficient? (if you're designing the application layer)
  • Use case 2: CDN wants to stand up generic services. Which handler should the server invoke when the connection is made? Parse it as MOQ? A CDN only controls one side of the application

@ekinnear
Copy link
Contributor

New header is available from the IETF side in this PR, so we should be fine to start offering a constructor that takes this list of protocol strings.

@wilaw wilaw added Discuss at next meeting Flags an issue to be discussed at the next WG working and removed IETF dependency labels Nov 29, 2023
@jan-ivar
Copy link
Member

jan-ivar commented Nov 29, 2023

So something like:

const wt = new WebTransport(url, {protocols: ["moqt", "loc"]});
try {
  await wt.ready;
  console.log(wt.protocol); // "moqt" or "loc"
} catch (e) {
  if (e.name != SomeUniqueError) throw;
  // client did not provide a suitable protocol
}

@jan-ivar
Copy link
Member

Meeting:

  • A PR would need to validate length limits for string inputs based on Add subprotocol negotiation ietf-wg-webtrans/draft-ietf-webtrans-http3#144 ...
  • ...which BTW also says "Servers MAY reject the request if the client did not include a suitable subprotocol."
  • If the server doesn't reject over unsupported protocol, should the API still reject?
  • E.g. either
    • It could succeed and wt.protocol is null, OR
    • wt.ready rejects anyway
  • Found ref "Parsers must support tokens of at least 512 characters."
  • Maybe look at WebSocket for mitigations?

@jan-ivar jan-ivar added Ready for PR and removed Discuss at next meeting Flags an issue to be discussed at the next WG working labels Dec 19, 2023
@wilaw wilaw added this to the Candidate Recommendation milestone Dec 20, 2023
@vasilvv
Copy link
Contributor Author

vasilvv commented Dec 23, 2023

If the server doesn't reject over unsupported protocol, should the API still reject?

So, the reason we changed protocol from non-nullable to nullable is that this provides a way for applications that have not used subprotocols previously to start using them; it would make sense for the API to follow that logic.

Found ref "Parsers must support tokens of at least 512 characters."

I think we should pick some number and make sure that we support it, and reject everything above (the server has to pick one of the client-offered tokens, so if we say "reject above N bytes", that will be the de-facto limit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants