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

Should there be size limits for messages and strings? #473

Open
huitema opened this issue Jun 24, 2024 · 7 comments
Open

Should there be size limits for messages and strings? #473

huitema opened this issue Jun 24, 2024 · 7 comments
Assignees
Labels
Needs PR Security/DoS Issues with MoQ's security or ways it may be vulnerable to denial of service / resource exhaustion Wire Format Related to how messages are serialized and parsed

Comments

@huitema
Copy link
Contributor

huitema commented Jun 24, 2024

The messages defined in MoQ Transport include variable length components:

  • name space,
  • track name,
  • setup path,
  • go away URI,
  • authorization info,
  • number of supported versions,
  • number of setup or subscribe parameters.

These components have no specified maximum size, which opens an avenue for DOS attacks: start sending a message with components of arbitrary length, up to 2^62, and watch as the other endpoint allocates memory to hold the message. Application developers are very likely to mitigate the DOS attack by setting some limits, either to the global size of messages or to the size of individual components. We may expect different implementations to pick different limits, with a fairly good potential of causing interoperability issues.

Maybe we should specify some limits.

@TimEvens
Copy link

+1 to having some limits. I think we should have some defaults in the standard and then negotiate those during client/server setup.

@LPardue
Copy link
Contributor

LPardue commented Jun 24, 2024

Guidance on limits is good.

However, limit negotiation doesn't tend to work well when intermediaries enter the mix. We've seen this in HTTP/2 and HTTP/3 with the max field section size parameter. If a proxy is given something from the origin that exceeds the client's limit, its not really advisable to silently drop it.

@afrind afrind added Wire Format Related to how messages are serialized and parsed labels Jun 25, 2024
@fluffy
Copy link
Contributor

fluffy commented Jul 4, 2024

All implementations end up with limits and having them all be different in different implementations with no way to say they were exceeded is a nightmare.

I would argue on limits for everything and sending protocol error and closing the connection if limits are exceeded.

@huitema
Copy link
Contributor Author

huitema commented Jul 4, 2024

If limit negotiation is known to not work, then we should have the limits expressed in the base spec.

@TimEvens
Copy link

TimEvens commented Jul 5, 2024

Currently, I've mapped the termination reason (https://datatracker.ietf.org/doc/html/draft-ietf-moq-transport-04#section-3.5) as the application reason code (https://github.com/private-octopus/picoquic/blob/master/picoquic/picoquic.h#L780) when closing a connection.

It's starting to look like this is not right. If we terminate by protocol volition (e.g., 0x3) then the remote side is left unaware of the specific protocol violation. Was it malformed, was it caused by invalid state machine, was it a timeout, was it a size that exceed, ??? If it was a size limit, then what size was the problem and is there suggested value to use? How would the remote side know what to do, other than repeatedly reconnect that results in only the same violation over and over again?

@huitema
Copy link
Contributor Author

huitema commented Jul 5, 2024

The error code in the application-specific variant of CONNECTION_CLOSE" is defined as "codes defined by the application protocol". So it is completely legit to put the specific MoQ code there. You may also want to use a reason phrase if you want to facilitate debugging.

@afrind
Copy link
Collaborator

afrind commented Jul 8, 2024

You may also want to use a reason phrase

An endorsement for reason phrases!

See also #273

@ianswett ianswett added the Security/DoS Issues with MoQ's security or ways it may be vulnerable to denial of service / resource exhaustion label Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs PR Security/DoS Issues with MoQ's security or ways it may be vulnerable to denial of service / resource exhaustion Wire Format Related to how messages are serialized and parsed
Projects
None yet
Development

No branches or pull requests

6 participants