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

Quiche drops the packet with OOB stream id instead of sending CONNECTION_CLOSE #565

Closed
goelvidhi opened this issue Jun 16, 2020 · 3 comments · Fixed by #567
Closed

Quiche drops the packet with OOB stream id instead of sending CONNECTION_CLOSE #565

goelvidhi opened this issue Jun 16, 2020 · 3 comments · Fixed by #567

Comments

@goelvidhi
Copy link

goelvidhi commented Jun 16, 2020

When we inject a MAX_STREAMS frame with a value greater than 2^60 . The QUIC library must close the connection immediately with a connection error STREAM_LIMIT_ERROR.

Example,
MAX_STREAMS_BIDI[id=1152921504606846977]

Similar test for injecting OOB STREAMS_BLOCKED has same result, i.e. the packet is dropped without connection close.

ghedo added a commit that referenced this issue Jun 18, 2020
Also slightly refactor existing check to avoid repeating the same
constant over and over.

Fixes #565.
@LPardue
Copy link
Contributor

LPardue commented Jun 18, 2020

https://tools.ietf.org/html/draft-ietf-quic-transport-29#section-19.11-5 says

Maximum Streams: A count of the cumulative number of streams of the
corresponding type that can be opened over the lifetime of the
connection. This value cannot exceed 2^60, as it is not possible
to encode stream IDs larger than 2^62-1. Receipt of a frame that
permits opening of a stream larger than this limit MUST be treated
as a FRAME_ENCODING_ERROR.

So I think this needs to be FRAME_ENCODING_ERROR, not STREAM_LIMIT_ERROR.

ghedo added a commit that referenced this issue Jun 18, 2020
Also slightly refactor existing check to avoid repeating the same
constant over and over.

Fixes #565.
ghedo added a commit that referenced this issue Jun 18, 2020
This also changes the error used when the stream limit is exceeded as
per the latest draft.

Also slightly refactor existing check to avoid repeating the same
constant over and over.

Fixes #565.
ghedo added a commit that referenced this issue Jun 18, 2020
This also changes the error used when the stream limit is exceeded as
per the latest draft.

Also slightly refactor existing check to avoid repeating the same
constant over and over.

Fixes #565.
@goelvidhi
Copy link
Author

The draft specifies both the error codes for this scenario

For example, below is from section 4.5
https://tools.ietf.org/html/draft-ietf-quic-transport-29#section-4.5

If a max_streams transport parameter or MAX_STREAMS frame is received
with a value greater than 2^60, this would allow a maximum stream ID
that cannot be expressed as a variable-length integer; see
Section 16. If either is received, the connection MUST be closed
immediately with a connection error of type STREAM_LIMIT_ERROR

Either should be fine, I think.

@LPardue
Copy link
Contributor

LPardue commented Jun 21, 2020

Good point, I tend to agree either seems fine. I opened an issue on the QUIC specs to suggest it clarifies the text on this matter but I don't think the outcome affects the quiche PR; I'd be happy with either.

ghedo added a commit that referenced this issue Jun 23, 2020
This also changes the error used when the stream limit is exceeded as
per the latest draft.

Also slightly refactor existing check to avoid repeating the same
constant over and over.

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

Successfully merging a pull request may close this issue.

2 participants