Skip to content

Commit

Permalink
Merge pull request from GHSA-rfhg-rjfp-9q8q
Browse files Browse the repository at this point in the history
* fix(s2n-quic-transport): close connection when no available connection ids

* allow all connection migrations when testing

* add generator to stateless reset token

* fix partial eq impl for connection IDs

* add new connection ID to fuzz test

* implement hash and fix merge issues

* Impl PartialOrd and Ord

* PR feedback

* clippy
  • Loading branch information
WesleyRosenblum authored Jul 24, 2023
1 parent 6ea5851 commit 73a8142
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 210 deletions.
26 changes: 25 additions & 1 deletion quic/s2n-quic-core/src/connection/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub const MAX_LIFETIME: Duration = Duration::from_secs(24 * 60 * 60); // one day
macro_rules! id {
($type:ident, $min_len:expr) => {
/// Uniquely identifies a QUIC connection between 2 peers
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Copy, Clone, Eq)]
#[cfg_attr(any(feature = "generator", test), derive(TypeGenerator))]
pub struct $type {
bytes: [u8; MAX_LEN],
Expand All @@ -49,6 +49,30 @@ macro_rules! id {
}
}

impl PartialEq for $type {
fn eq(&self, other: &Self) -> bool {
self.as_bytes() == other.as_bytes()
}
}

impl core::hash::Hash for $type {
fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
self.as_bytes().hash(state);
}
}

impl PartialOrd for $type {
fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
Some(self.cmp(other))
}
}

impl Ord for $type {
fn cmp(&self, other: &Self) -> core::cmp::Ordering {
self.as_bytes().cmp(&other.as_bytes())
}
}

impl $type {
/// The minimum length for this connection ID type
pub const MIN_LEN: usize = $min_len;
Expand Down
15 changes: 15 additions & 0 deletions quic/s2n-quic-core/src/path/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,18 @@ pub mod disabled {
}
}
}

#[cfg(any(test, feature = "testing"))]
pub mod allow_all {
use super::*;

#[derive(Debug, Default)]
pub struct Validator;

impl super::Validator for Validator {
fn on_migration_attempt(&mut self, _attempt: &Attempt) -> Outcome {
// allow all migration attempts
Outcome::Allow
}
}
}
4 changes: 4 additions & 0 deletions quic/s2n-quic-core/src/stateless_reset/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ pub const LEN: usize = 128 / 8;
// Hash can still be derived.
#[allow(clippy::derived_hash_with_manual_eq)]
#[derive(Copy, Clone, Debug, Eq, Hash)]
#[cfg_attr(
any(test, feature = "generator"),
derive(bolero_generator::TypeGenerator)
)]
pub struct Token([u8; LEN]);

impl Token {
Expand Down
4 changes: 2 additions & 2 deletions quic/s2n-quic-transport/src/endpoint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ pub mod testing {
type StreamManager = crate::stream::DefaultStreamManager;
type ConnectionCloseFormatter = s2n_quic_core::connection::close::Development;
type EventSubscriber = Subscriber;
type PathMigrationValidator = path::migration::default::Validator;
type PathMigrationValidator = path::migration::allow_all::Validator;
type PacketInterceptor = s2n_quic_core::packet::interceptor::Disabled;
type DatagramEndpoint = s2n_quic_core::datagram::Disabled;

Expand Down Expand Up @@ -1202,7 +1202,7 @@ pub mod testing {
type StreamManager = crate::stream::DefaultStreamManager;
type ConnectionCloseFormatter = s2n_quic_core::connection::close::Development;
type EventSubscriber = Subscriber;
type PathMigrationValidator = path::migration::default::Validator;
type PathMigrationValidator = path::migration::allow_all::Validator;
type PacketInterceptor = s2n_quic_core::packet::interceptor::Disabled;
type DatagramEndpoint = s2n_quic_core::datagram::Disabled;

Expand Down
9 changes: 4 additions & 5 deletions quic/s2n-quic-transport/src/path/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,11 +705,10 @@ impl<Config: endpoint::Config> Manager<Config> {
active_path_connection_id,
publisher,
)
.expect(
"since we are only checking the active path and new id was delivered \
via the new_connection_id frames, there will always be a new id available \
to consume if necessary",
);
.ok_or(transport::Error::PROTOCOL_VIOLATION.with_reason(
"A NEW_CONNECTION_ID frame was received that retired the active path's \
connection ID and no unused connection IDs remain to replace it",
))?
}

Ok(())
Expand Down
Loading

0 comments on commit 73a8142

Please sign in to comment.