Skip to content

Commit c7a60fc

Browse files
authored
fix: return HandshakePending for auto-sense pending state
1 parent d92257a commit c7a60fc

6 files changed

Lines changed: 85 additions & 30 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Unreleased
22

3+
* Fix panic in auto DTLS version selection #65
4+
* Add `Error::HandshakePending` for auto-sense pending state (breaking) #65
35
* DTLS 1.2 ECDSA determine curve from certificate, not hash algorithm #57
46
* DTLS 1.3 enforce SignatureScheme curve matches certificate key #60
57

src/dtls12/client.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ impl Client {
124124
config: std::sync::Arc<crate::Config>,
125125
certificate: crate::DtlsCertificate,
126126
now: Instant,
127-
) -> Client {
127+
) -> Result<Client, Error> {
128128
let mut engine = Engine::new(config, certificate);
129129
engine.set_client(true);
130130
// The hybrid ClientHello was sent with message_seq=0 outside this
@@ -137,7 +137,7 @@ impl Client {
137137
// Advance epoch-0 record sequence past the hybrid CH record.
138138
engine.advance_epoch_0_sequence();
139139

140-
Client {
140+
let mut client = Client {
141141
state: State::AwaitHelloVerifyRequest,
142142
engine,
143143
random: Some(random),
@@ -153,7 +153,9 @@ impl Client {
153153
last_now: now,
154154
local_events: VecDeque::new(),
155155
queued_data: Vec::new(),
156-
}
156+
};
157+
client.handle_timeout(now)?;
158+
Ok(client)
157159
}
158160

159161
pub fn into_server(self) -> Server {

src/dtls12/engine.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,9 @@ impl Engine {
446446
}
447447
(Timeout::Armed(c), _) => c,
448448
(_, Timeout::Armed(f)) => f,
449-
_ => unreachable!(),
449+
// Both Unarmed or mixed Unarmed/Disabled: return current time
450+
// to trigger handle_timeout on the next cycle.
451+
_ => now,
450452
}
451453
}
452454

src/dtls13/client.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,15 @@ impl Client {
174174
config: std::sync::Arc<crate::Config>,
175175
certificate: crate::DtlsCertificate,
176176
now: Instant,
177-
) -> Client {
177+
) -> Result<Client, Error> {
178178
let mut engine = Engine::new(config, certificate);
179179
engine.set_client(true);
180180

181181
// Inject transcript + sequence state from the hybrid CH that was
182182
// already sent on the wire by ClientPending.
183183
engine.inject_hybrid_client_hello(&hybrid.transcript_bytes);
184184

185-
Client {
185+
let mut client = Client {
186186
state: State::AwaitServerHello,
187187
engine,
188188
random: Some(hybrid.random),
@@ -204,7 +204,9 @@ impl Client {
204204
handshake_secret: None,
205205
client_hs_traffic_secret: None,
206206
server_hs_traffic_secret: None,
207-
}
207+
};
208+
client.handle_timeout(now)?;
209+
Ok(client)
208210
}
209211

210212
pub fn into_server(self) -> Server {

src/error.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ pub enum Error {
2929
TooManyRecords,
3030
/// Peer attempted renegotiation (not supported)
3131
RenegotiationAttempt,
32+
/// Application data cannot be sent because the handshake is not yet complete.
33+
///
34+
/// For auto-sense instances this means the version has not yet been
35+
/// resolved. Callers should buffer the data and retry once the
36+
/// handshake advances.
37+
HandshakePending,
3238
}
3339

3440
impl<'a> From<nom::Err<nom::error::Error<&'a [u8]>>> for Error {
@@ -59,6 +65,9 @@ impl std::fmt::Display for Error {
5965
Error::ConfigError(msg) => write!(f, "config error: {}", msg),
6066
Error::TooManyRecords => write!(f, "too many records in packet"),
6167
Error::RenegotiationAttempt => write!(f, "peer attempted renegotiation"),
68+
Error::HandshakePending => {
69+
write!(f, "handshake pending: cannot send application data yet")
70+
}
6271
}
6372
}
6473
}

src/lib.rs

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -358,21 +358,30 @@ impl Dtls {
358358
detect::client_hello_version(packet),
359359
detect::DetectedVersion::Dtls13
360360
);
361-
let resolved = if is_13 {
362-
let mut server = Server13::new(config, certificate, now);
363-
server.handle_timeout(now)?;
364-
Inner::Server13(server)
361+
self.inner = if is_13 {
362+
Some(Inner::Server13(Server13::new(config, certificate, now)))
365363
} else {
366-
let mut server = Server12::new(config, certificate, now);
367-
server.handle_timeout(now)?;
368-
Inner::Server12(server)
364+
Some(Inner::Server12(Server12::new(config, certificate, now)))
369365
};
370-
self.inner = Some(resolved);
366+
367+
// Arm the server's random + retransmit timers.
368+
// inner is already set, so errors won't leave it as None.
369+
self.handle_timeout(now)?;
371370
}
372371

373372
// Auto-sense client: resolve version on first server response
374373
if matches!(self.inner, Some(Inner::ClientPending(_))) {
375374
let version = detect::server_hello_version(packet);
375+
376+
// Check version before taking inner — returning an error
377+
// while inner is None would leave us unable to poll/timeout.
378+
if matches!(version, detect::DetectedVersion::Unknown) {
379+
return Err(Error::UnexpectedMessage(
380+
"Unrecognized response from server".to_string(),
381+
));
382+
}
383+
384+
// unwrap: guarded by the matches! check above
376385
let inner = self.inner.take().unwrap();
377386
let Inner::ClientPending(cp) = inner else {
378387
unreachable!()
@@ -386,24 +395,26 @@ impl Dtls {
386395
config,
387396
certificate,
388397
now,
389-
);
398+
)?;
390399
// Feed the HVR to Client12 — it enters
391400
// AwaitHelloVerifyRequest and processes the cookie.
392-
client12.handle_packet(packet)?;
401+
if let Err(e) = client12.handle_packet(packet) {
402+
self.inner = Some(Inner::Client12(client12));
403+
return Err(e);
404+
}
393405
self.inner = Some(Inner::Client12(client12));
394406
return Ok(());
395407
}
396408
detect::DetectedVersion::Dtls13 => {
397-
let mut client13 = Client13::new_from_hybrid(hybrid, config, certificate, now);
398-
client13.handle_packet(packet)?;
409+
let mut client13 = Client13::new_from_hybrid(hybrid, config, certificate, now)?;
410+
if let Err(e) = client13.handle_packet(packet) {
411+
self.inner = Some(Inner::Client13(client13));
412+
return Err(e);
413+
}
399414
self.inner = Some(Inner::Client13(client13));
400415
return Ok(());
401416
}
402-
detect::DetectedVersion::Unknown => {
403-
return Err(Error::UnexpectedMessage(
404-
"Unrecognized response from server".to_string(),
405-
));
406-
}
417+
detect::DetectedVersion::Unknown => unreachable!(),
407418
}
408419
}
409420

@@ -444,18 +455,17 @@ impl Dtls {
444455
}
445456

446457
/// Send application data over the established DTLS session.
458+
///
459+
/// Returns [`Error::HandshakePending`] if the DTLS version has not
460+
/// yet been resolved (auto-sense pending). Callers should buffer
461+
/// the data externally and retry after the handshake progresses.
447462
pub fn send_application_data(&mut self, data: &[u8]) -> Result<(), Error> {
448463
match self.inner.as_mut().unwrap() {
449464
Inner::Client12(client) => client.send_application_data(data),
450465
Inner::Server12(server) => server.send_application_data(data),
451466
Inner::Client13(client) => client.send_application_data(data),
452467
Inner::Server13(server) => server.send_application_data(data),
453-
Inner::ServerPending(_) => Err(Error::UnexpectedMessage(
454-
"cannot send application data: handshake not started".to_string(),
455-
)),
456-
Inner::ClientPending(_) => Err(Error::UnexpectedMessage(
457-
"cannot send application data: handshake not complete".to_string(),
458-
)),
468+
Inner::ServerPending(_) | Inner::ClientPending(_) => Err(Error::HandshakePending),
459469
}
460470
}
461471
}
@@ -583,6 +593,34 @@ mod test {
583593
assert!(matches!(result, Output::Timeout(_)));
584594
}
585595

596+
#[test]
597+
fn test_auto_client_unknown_version_no_panic() {
598+
// Regression: handle_packet returning UnexpectedMessage for an
599+
// unrecognized server response must not leave inner as None,
600+
// which would panic on the next poll_output/handle_timeout.
601+
let mut dtls = new_instance_auto();
602+
dtls.set_active(true);
603+
let now = Instant::now();
604+
dtls.handle_timeout(now).unwrap();
605+
606+
// Drain the hybrid ClientHello
607+
let mut buf = [0u8; 2048];
608+
loop {
609+
if matches!(dtls.poll_output(&mut buf), Output::Timeout(_)) {
610+
break;
611+
}
612+
}
613+
614+
// Feed a garbage packet that won't be recognized as DTLS 1.2 or 1.3
615+
let garbage = [0xFF; 64];
616+
let err = dtls.handle_packet(&garbage).unwrap_err();
617+
assert!(matches!(err, Error::UnexpectedMessage(_)));
618+
619+
// These must NOT panic — inner should still be intact
620+
dtls.handle_timeout(now).unwrap();
621+
let _ = dtls.poll_output(&mut buf);
622+
}
623+
586624
#[test]
587625
fn is_send() {
588626
fn is_send<T: Send>(_t: T) {}

0 commit comments

Comments
 (0)