-
-
Notifications
You must be signed in to change notification settings - Fork 482
feat: add event and function to wait for resumption tickets #2257
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,11 @@ pub trait Session: Send + Sync + 'static { | |
label: &[u8], | ||
context: &[u8], | ||
) -> Result<(), ExportKeyingMaterialError>; | ||
|
||
/// Returns the number of TLS1.3 session resumption tickets that were received | ||
/// | ||
/// Returns `None` on the server side. | ||
fn resumption_tickets_received(&self) -> Option<u32>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Providing a default impl returning |
||
} | ||
|
||
/// A pair of keys for bidirectional communication | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -651,6 +651,37 @@ impl Connection { | |
// May need to send MAX_STREAMS to make progress | ||
conn.wake(); | ||
} | ||
|
||
/// Waits until the connection received TLS resumption tickets | ||
/// | ||
/// Yields `true` once resumption tickets were received. Resolves immediately | ||
/// if tickets were already received, otherwise it resolves once tickets arrive. | ||
/// If the server does not send any tickets, the returned future will remain pending forever. | ||
/// | ||
/// This should only be used on the client side. On the server side, it will | ||
/// always resolve immediately and yield `false`. | ||
pub fn resumption_tickets_received(&self) -> impl Future<Output = bool> + Send + 'static { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is anyone going to ever inspect this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is the user meant to actually get the ticket? If the expectation is that they'll still be going through rustls's |
||
let conn = self.0.clone(); | ||
async move { | ||
let notify; | ||
let (mut notified, out) = { | ||
let conn = conn.state.lock("resumption_tickets_received"); | ||
let (notified, out) = match conn.resumption_tickets.as_ref() { | ||
Some(ResumptionTicketState::Received) => (None, true), | ||
Some(ResumptionTicketState::Pending(n)) => { | ||
notify = n.clone(); | ||
(Some(notify.notified()), true) | ||
} | ||
None => (None, false), | ||
}; | ||
(notified, out) | ||
}; | ||
if let Some(notified) = notified.take() { | ||
notified.await; | ||
} | ||
out | ||
} | ||
} | ||
} | ||
|
||
pin_project! { | ||
|
@@ -885,6 +916,10 @@ impl ConnectionRef { | |
socket: Arc<dyn AsyncUdpSocket>, | ||
runtime: Arc<dyn Runtime>, | ||
) -> Self { | ||
let resumption_tickets = match conn.side() { | ||
Side::Client => Some(ResumptionTicketState::Pending(Default::default())), | ||
Side::Server => None, | ||
}; | ||
Self(Arc::new(ConnectionInner { | ||
state: Mutex::new(State { | ||
inner: conn, | ||
|
@@ -907,6 +942,7 @@ impl ConnectionRef { | |
runtime, | ||
send_buffer: Vec::new(), | ||
buffered_transmit: None, | ||
resumption_tickets, | ||
}), | ||
shared: Shared::default(), | ||
})) | ||
|
@@ -989,6 +1025,8 @@ pub(crate) struct State { | |
send_buffer: Vec<u8>, | ||
/// We buffer a transmit when the underlying I/O would block | ||
buffered_transmit: Option<proto::Transmit>, | ||
/// Whether we received resumption tickets. None on the server side. | ||
resumption_tickets: Option<ResumptionTicketState>, | ||
} | ||
|
||
impl State { | ||
|
@@ -1123,6 +1161,14 @@ impl State { | |
wake_all_notify(&mut self.stopped); | ||
} | ||
} | ||
ResumptionEnabled => { | ||
if let Some(ResumptionTicketState::Pending(notify)) = | ||
self.resumption_tickets.as_mut() | ||
{ | ||
notify.notify_waiters(); | ||
self.resumption_tickets = Some(ResumptionTicketState::Received); | ||
} | ||
} | ||
ConnectionLost { reason } => { | ||
self.terminate(reason, shared); | ||
} | ||
|
@@ -1293,6 +1339,12 @@ fn wake_all_notify(wakers: &mut FxHashMap<StreamId, Arc<Notify>>) { | |
.for_each(|(_, notify)| notify.notify_waiters()) | ||
} | ||
|
||
#[derive(Debug)] | ||
enum ResumptionTicketState { | ||
Received, | ||
Pending(Arc<Notify>), | ||
} | ||
|
||
/// Errors that can arise when sending a datagram | ||
#[derive(Debug, Error, Clone, Eq, PartialEq)] | ||
pub enum SendDatagramError { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -384,6 +384,71 @@ async fn zero_rtt() { | |
endpoint.wait_idle().await; | ||
} | ||
|
||
#[tokio::test] | ||
async fn zero_rtt_resumption() { | ||
let _guard = subscribe(); | ||
let endpoint = endpoint(); | ||
|
||
let endpoint2 = endpoint.clone(); | ||
tokio::spawn(async move { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to join this at the end of the test to ensure any panics are surfaced? |
||
for _ in 0..12 { | ||
let incoming = endpoint2.accept().await.unwrap().accept().unwrap(); | ||
let (connection, _established) = | ||
incoming.into_0rtt().unwrap_or_else(|_| unreachable!()); | ||
connection.closed().await; | ||
} | ||
}); | ||
|
||
let connect_0rtt = || { | ||
endpoint | ||
.connect(endpoint.local_addr().unwrap(), "localhost") | ||
.unwrap() | ||
.into_0rtt() | ||
.unwrap_or_else(|_| panic!("missing 0-RTT keys")) | ||
}; | ||
|
||
let connect_0rtt_fail = || { | ||
endpoint | ||
.connect(endpoint.local_addr().unwrap(), "localhost") | ||
.unwrap() | ||
.into_0rtt() | ||
.err() | ||
.expect("0-RTT succeeded without keys") | ||
}; | ||
|
||
let connect_full = || async { | ||
endpoint | ||
.connect(endpoint.local_addr().unwrap(), "localhost") | ||
.unwrap() | ||
.into_0rtt() | ||
.err() | ||
.expect("0-RTT succeeded without keys") | ||
.await | ||
.expect("connect") | ||
}; | ||
|
||
// 0rtt without full connection should fail | ||
connect_0rtt_fail(); | ||
// now do a full connection | ||
connect_full().await; | ||
// we received two tickets, so we should be able to resume two times, and then fail | ||
connect_0rtt(); | ||
connect_0rtt(); | ||
connect_0rtt_fail(); | ||
|
||
// now do another full connection | ||
connect_full().await; | ||
connect_0rtt(); | ||
// this time we wait to receive resumption tickets on the zero-rtt connection | ||
let (conn, _0rtt_accepted) = connect_0rtt(); | ||
conn.resumption_tickets_received().await; | ||
// and we can do two more 0rtt conns | ||
connect_0rtt(); | ||
connect_0rtt(); | ||
// and then fail again | ||
connect_0rtt_fail(); | ||
} | ||
|
||
#[test] | ||
#[cfg_attr( | ||
any(target_os = "solaris", target_os = "illumos"), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the number of tickets received guaranteed to change at most once in the lifetime of a connection? If not, should we raise an event every time it changes?