Skip to content

Commit

Permalink
fix(s2n-quic-core): remove unwraps in null TLS provider (#1881)
Browse files Browse the repository at this point in the history
  • Loading branch information
@camshaft
camshaft authored Jul 25, 2023
1 parent efa07e0 commit 8460c72
Show file tree
Hide file tree
Showing 11 changed files with 186 additions and 82 deletions.
3 changes: 2 additions & 1 deletion common/s2n-codec/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ exclude = ["corpus.tar.gz"]

[features]
default = ["std", "bytes"]
std = []
alloc = []
std = ["alloc"]
testing = ["std", "generator"]
checked_range_unsafe = []
generator = ["bolero-generator"]
Expand Down
8 changes: 8 additions & 0 deletions common/s2n-codec/src/encoder/value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ pub trait EncoderValue: Sized {
len.encode(encoder);
self.encode(encoder);
}

#[cfg(feature = "alloc")]
fn encode_to_vec(&self) -> alloc::vec::Vec<u8> {
let len = self.encoding_size();
let mut buffer = alloc::vec![0u8; len];
self.encode(&mut crate::EncoderBuffer::new(&mut buffer));
buffer
}
}

macro_rules! encoder_value_byte {
Expand Down
3 changes: 3 additions & 0 deletions common/s2n-codec/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

#![cfg_attr(not(any(test, feature = "std")), no_std)]

#[cfg(feature = "alloc")]
extern crate alloc;

#[cfg(any(feature = "testing", test))]
#[macro_use]
pub mod testing;
Expand Down
2 changes: 1 addition & 1 deletion quic/s2n-quic-core/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ exclude = ["corpus.tar.gz"]

[features]
default = ["alloc", "std"]
alloc = ["atomic-waker", "bytes", "crossbeam-utils"]
alloc = ["atomic-waker", "bytes", "crossbeam-utils", "s2n-codec/alloc"]
std = ["alloc", "once_cell"]
testing = ["std", "generator", "s2n-codec/testing", "checked-counters", "insta", "futures-test"]
generator = ["bolero-generator"]
Expand Down
109 changes: 69 additions & 40 deletions quic/s2n-quic-core/src/crypto/tls/null.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl crypto::tls::Endpoint for Endpoint {
&mut self,
transport_parameters: &Params,
) -> Self::Session {
let params = encode_transport_parameters(transport_parameters);
let params = transport_parameters.encode_to_vec().into();
Session::Server(server::Session::Init {
transport_parameters: params,
})
Expand All @@ -99,7 +99,7 @@ impl crypto::tls::Endpoint for Endpoint {
) -> Self::Session {
assert_eq!(server_name, LOCALHOST);

let params = encode_transport_parameters(transport_parameters);
let params = transport_parameters.encode_to_vec().into();
Session::Client(client::Session::Init {
transport_parameters: params,
})
Expand Down Expand Up @@ -156,14 +156,6 @@ impl tls::Session for Session {
}
}

/// Encodes transport parameters into a byte vec
fn encode_transport_parameters<Params: s2n_codec::EncoderValue>(params: &Params) -> Bytes {
let len = params.encoding_size();
let mut buffer = vec![0; len];
params.encode(&mut s2n_codec::EncoderBuffer::new(&mut buffer));
buffer.into()
}

static FIN: Bytes = Bytes::from_static(b"FIN");
static NULL: Bytes = Bytes::from_static(b"NULL");

Expand Down Expand Up @@ -191,7 +183,7 @@ pub mod client {
} => {
context.send_initial(core::mem::take(transport_parameters));

context.on_server_name(LOCALHOST.clone()).unwrap();
context.on_server_name(LOCALHOST.clone())?;

*self = Self::WaitingInitial {};
}
Expand All @@ -201,9 +193,7 @@ pub mod client {
None => return Poll::Pending,
};

context
.on_handshake_keys(key::NoCrypto, key::NoCrypto)
.unwrap();
context.on_handshake_keys(key::NoCrypto, key::NoCrypto)?;

// notify the server we're done
context.send_handshake(FIN.clone());
Expand All @@ -215,19 +205,17 @@ pub mod client {
return Poll::Pending;
}

context.on_application_protocol(NULL.clone()).unwrap();
context.on_application_protocol(NULL.clone())?;

context
.on_one_rtt_keys(
key::NoCrypto,
key::NoCrypto,
tls::ApplicationParameters {
transport_parameters: params,
},
)
.unwrap();
context.on_one_rtt_keys(
key::NoCrypto,
key::NoCrypto,
tls::ApplicationParameters {
transport_parameters: params,
},
)?;

context.on_handshake_complete().unwrap();
context.on_handshake_complete()?;

*self = Self::Complete;

Expand Down Expand Up @@ -267,24 +255,20 @@ pub mod server {
};
context.send_initial(core::mem::take(transport_parameters));

context
.on_handshake_keys(key::NoCrypto, key::NoCrypto)
.unwrap();
context.on_handshake_keys(key::NoCrypto, key::NoCrypto)?;
context.send_handshake(FIN.clone());

context.on_application_protocol(NULL.clone()).unwrap();
context.on_application_protocol(NULL.clone())?;

context
.on_one_rtt_keys(
key::NoCrypto,
key::NoCrypto,
tls::ApplicationParameters {
transport_parameters: &client_params,
},
)
.unwrap();
context.on_one_rtt_keys(
key::NoCrypto,
key::NoCrypto,
tls::ApplicationParameters {
transport_parameters: &client_params,
},
)?;

context.on_server_name(LOCALHOST.clone()).unwrap();
context.on_server_name(LOCALHOST.clone())?;

*self = Self::WaitingComplete;
}
Expand All @@ -294,7 +278,7 @@ pub mod server {
}

*self = Self::Complete;
context.on_handshake_complete().unwrap();
context.on_handshake_complete()?;

return Ok(()).into();
}
Expand Down Expand Up @@ -446,6 +430,9 @@ mod key {
mod tests {
use super::*;
use crate::crypto::tls::testing::Pair;
use bolero::check;
use bytes::{BufMut, Bytes, BytesMut};
use std::collections::VecDeque;

#[test]
fn null_test() {
Expand All @@ -460,4 +447,46 @@ mod tests {

pair.finish();
}

#[test]
fn fuzz_test() {
let mut server = Endpoint::default();
let mut client = Endpoint::default();

check!().for_each(|mut bytes| {
// replaces a single buffer with fuzz bytes
let mut replace_bytes = |chunks: &mut VecDeque<Bytes>| {
for chunk in chunks {
let len = chunk.len().min(bytes.len());
let (data, remaining) = bytes.split_at(len);
bytes = remaining;
let mut replacement = BytesMut::with_capacity(chunk.len());
replacement.put_slice(data);
replacement.put_bytes(0, chunk.len() - data.len());
assert_eq!(chunk.len(), replacement.len());
*chunk = replacement.freeze();
}
};

let mut pair = Pair::new(&mut server, &mut client, LOCALHOST.clone());

while pair.is_handshaking() {
if pair.poll_start().is_err() {
break;
}

// replace all of the buffers with fuzz bytes
replace_bytes(&mut pair.server.context.initial.rx);
replace_bytes(&mut pair.server.context.initial.tx);
replace_bytes(&mut pair.server.context.handshake.rx);
replace_bytes(&mut pair.server.context.handshake.tx);
replace_bytes(&mut pair.server.context.application.rx);
replace_bytes(&mut pair.server.context.application.tx);

if pair.poll_finish(None).is_err() {
break;
}
}
});
}
}
Loading

0 comments on commit 8460c72

Please sign in to comment.