feat: expose 0-RTT detection at stream level #323
Conversation
iadev09
force-pushed
the
feature/stream-level-0rtt
branch
from
95fc57a to
f97258d
Compare
iadev09
force-pushed
the
feature/stream-level-0rtt
branch
from
f077a14 to
b2939cf
Compare
Ruben2424
left a comment
Ruben2424
left a comment
There was a problem hiding this comment.
Thanks for the PR. I left some comments below.
h3/src/frame.rs
Outdated
|
|
||
| pub(crate) fn is_0rtt(&self) -> bool | ||
| where | ||
| S: crate::server::Is0rtt, |
There was a problem hiding this comment.
Is there a reason you did not make an extra impl block? Like for S: RecvStream and T: SendStream<B>
h3/src/stream.rs
Outdated
|
|
||
| pub(crate) fn is_0rtt(&self) -> bool | ||
| where | ||
| S: crate::server::Is0rtt, |
There was a problem hiding this comment.
Same question, why not extra impl block?
h3/src/server/stream.rs
Outdated
|
|
||
| /// Trait for QUIC streams that support 0-RTT detection. | ||
| pub trait Is0rtt { | ||
| /// Check if this stream was opened during 0-RTT. |
There was a problem hiding this comment.
All traits implemented by quic transport layer usually are in this file https://github.com/hyperium/h3/blob/master/h3/src/quic.rs
h3-quinn/src/lib.rs
Outdated
| self.stream | ||
| .as_ref() | ||
| .map(|s| s.is_0rtt()) | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
when self.stream is None this will always return false.
IIRC this can only happen if the user uses the poll API of the streams wrong. If so this function would return false even for 0rtt request.
We could just change this to expect.
The better way IMO would be to call is_0rtt in the new function and store the result in a new field for this function to return.
Also you could implement Is0rtt trait also for RecvStream
h3-quinn/src/lib.rs
Outdated
| /// | ||
| /// See [RFC 8470 Section 5.2](https://www.rfc-editor.org/rfc/rfc8470.html#section-5.2). | ||
| pub fn is_0rtt(&self) -> bool { | ||
| self.recv.is_0rtt() |
There was a problem hiding this comment.
Why not put self.recv.is_0rtt() in the trait impl of h3::server::Is0rtt? Then this function could be removed.
|
Thanks a lot for the detailed review! I initially introduced the That said, your points about consistency make sense — I’ll move the trait into |
|
I had a thought while looking at this. Does it have to be something known by What if you could peek at specific |
Could be useful generally. But in this case having a trait could allow libraries also generic over quic transport to use this information for example to automatically return 425 status code. Maybe something for hyper? |
Address review feedback by refactoring 0-RTT detection: - Move Is0rtt trait from h3::server to h3::quic module All QUIC transport traits belong in the quic abstraction layer, allowing libraries like hyper to use 0-RTT detection generically across different QUIC implementations. - Fix RecvStream::is_0rtt() to cache value at construction time Previously used unwrap_or(false) which could incorrectly return false for 0-RTT streams if poll API was misused. Now stores the 0-RTT flag in a dedicated field initialized in new(). - Implement Is0rtt trait for both BidiStream and RecvStream Ensures consistent 0-RTT detection across all stream types. - Simplify BidiStream implementation Remove redundant public is_0rtt() method, keep only trait impl that delegates to recv.is_0rtt(). - Keep RequestStream::is_0rtt() convenience method for ergonomic stream-level access in server applications. This maintains backward compatibility for users while fixing the potential security issue where 0-RTT status could be lost. Refs: PR hyperium#323
|
For reference, this topic was also discussed in quinn-rs/quinn#2440 |
3 participants
Summary
is_0rtt()method toRequestStreamto detect streams opened during 0-RTTIs0rtttrait for QUIC stream typesMotivation
Applications need to detect 0-RTT streams to properly handle replay-attack risks (RFC 8470 § 5.2).
For example, non-idempotent operations such as POST, PUT, or DELETE should be rejected or handled with extra care on 0-RTT streams.