Skip to content

Commit 29c22d8

Browse files
jaredwolffclaude
andcommitted
Fix PSK config validation and handshake edge cases
Review follow-ups to the initial PSK commit, squashed from three in-flight fixups plus additional validation tightening. Config validation: - Reject PSK configs where DTLS 1.2 has no PSK suite after filtering, regardless of DTLS 1.3 state. The only PSK suite this crate implements is DTLS 1.2 (0xC0A8), so a surviving DTLS 1.3 suite is not a fallback for Dtls::new_12_psk; building such a config produced a runtime-only failure instead of a clear build error. - Require kx groups whenever a cert-based DTLS 1.2 suite survives the filter, even when PSK is also configured. Previously a `with_psk_*().kx_groups(&[])` config that kept ECDHE suites in the DTLS 1.2 filter built successfully and then failed in send_server_key_exchange/process_ecdh_params. - Skip the kx-group check only when the surviving DTLS 1.2 suites are exclusively PSK, instead of whenever PSK is configured. - Reject builders whose constructor validation would otherwise silently accept a PSK-suite-free DTLS 1.2 filter. Constructor: - Dtls::new_12_psk asserts the config has a PSK configured so a missing resolver fails fast at construction rather than producing zero negotiable suites. Handshake: - Omit ServerKeyExchange entirely when the server has no PSK identity hint configured (RFC 4279 §2). Docs: - Clarify that require_client_certificate applies only to certificate-authenticated cipher suites and has no effect on a negotiated PSK handshake. Tests: - Add psk_with_dtls13_but_no_psk_dtls12_suite_rejected and psk_with_cert_dtls12_and_empty_kx_groups_rejected to cover the new validation paths. - Tighten psk_client_with_empty_kx_groups_builds to an explicitly PSK-only DTLS 1.2 filter, matching the stricter semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e1a1bf5 commit 29c22d8

3 files changed

Lines changed: 172 additions & 22 deletions

File tree

src/config.rs

Lines changed: 152 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ impl Config {
118118
///
119119
/// This will cause the server to send a CertificateRequest message.
120120
/// Makes the server fail if the client does not send a certificate.
121+
///
122+
/// Applies only to certificate-authenticated cipher suites. For RFC 4279
123+
/// PSK suites the client never sends a certificate, so this flag has no
124+
/// effect on a negotiated PSK handshake.
121125
#[inline(always)]
122126
pub fn require_client_certificate(&self) -> bool {
123127
self.require_client_certificate
@@ -327,6 +331,11 @@ impl ConfigBuilder {
327331
/// This will cause the server to send a CertificateRequest message.
328332
/// Makes the server fail if the client does not send a certificate.
329333
/// Defaults to true.
334+
///
335+
/// Applies only to certificate-authenticated cipher suites. For RFC 4279
336+
/// PSK suites the client never sends a certificate, so this flag has no
337+
/// effect on a negotiated PSK handshake; no opt-out is required when
338+
/// combining this builder with [`with_psk_server`](Self::with_psk_server).
330339
pub fn require_client_certificate(mut self, require: bool) -> Self {
331340
self.require_client_certificate = require;
332341
self
@@ -505,14 +514,21 @@ impl ConfigBuilder {
505514
));
506515
}
507516

508-
// Validate cipher suite filters: at least one version must have suites
509-
let dtls12_count = {
517+
// Validate cipher suite filters: at least one version must have suites.
518+
// Mirror Config::dtls12_cipher_suites() by dropping PSK suites when no PSK
519+
// is configured, so a PSK-only filter without a PSK resolver fails fast.
520+
let has_psk = self.psk.is_some();
521+
let dtls12_suites: Vec<_> = {
510522
let all = crypto_provider.supported_cipher_suites();
511523
match &self.dtls12_cipher_suites {
512-
Some(list) => all.filter(|cs| list.contains(&cs.suite())).count(),
513-
None => all.count(),
524+
Some(list) => all
525+
.filter(|cs| list.contains(&cs.suite()))
526+
.filter(|cs| has_psk || !cs.suite().is_psk())
527+
.collect(),
528+
None => all.filter(|cs| has_psk || !cs.suite().is_psk()).collect(),
514529
}
515530
};
531+
let dtls12_count = dtls12_suites.len();
516532
let dtls13_count = {
517533
let all = crypto_provider.dtls13_cipher_suites.iter();
518534
match &self.dtls13_cipher_suites {
@@ -528,18 +544,25 @@ impl ConfigBuilder {
528544
));
529545
}
530546

531-
// Check if we have any non-PSK DTLS 1.2 suites that need key exchange groups
532-
let has_non_psk_dtls12 = {
533-
match &self.dtls12_cipher_suites {
534-
Some(list) => crypto_provider
535-
.supported_cipher_suites()
536-
.filter(|cs| list.contains(&cs.suite()))
537-
.any(|cs| !cs.suite().is_psk()),
538-
None => crypto_provider
539-
.supported_cipher_suites()
540-
.any(|cs| !cs.suite().is_psk()),
541-
}
542-
};
547+
// When PSK is configured, at least one negotiable DTLS 1.2 suite must be
548+
// a PSK suite. The only PSK suite we implement today is DTLS 1.2 (0xC0A8),
549+
// so a surviving DTLS 1.3 suite is not a fallback: Dtls::new_12_psk only
550+
// speaks DTLS 1.2, and under AuthMode::Psk every non-PSK suite is rejected
551+
// by CryptoContext::is_cipher_suite_compatible.
552+
if has_psk && !dtls12_suites.iter().any(|cs| cs.suite().is_psk()) {
553+
return Err(Error::ConfigError(
554+
"PSK is configured but no PSK cipher suite remains after filtering \
555+
DTLS 1.2 suites. Include at least one PSK suite in \
556+
dtls12_cipher_suites."
557+
.to_string(),
558+
));
559+
}
560+
561+
// Skip DTLS 1.2 kx-group validation only when the surviving DTLS 1.2
562+
// suites are exclusively PSK — those don't negotiate an ECDHE group.
563+
// Any cert-based DTLS 1.2 suite left in the filter still needs a
564+
// compatible key exchange group, even when PSK is also configured.
565+
let has_non_psk_dtls12 = dtls12_suites.iter().any(|cs| !cs.suite().is_psk());
543566

544567
// Validate kx_groups filter: each enabled version needs compatible groups
545568
// (PSK-only DTLS 1.2 configs don't need key exchange groups)
@@ -881,6 +904,119 @@ mod tests {
881904
);
882905
}
883906

907+
#[test]
908+
fn psk_config_with_only_non_psk_dtls12_filter_rejected() {
909+
struct DummyResolver;
910+
impl PskResolver for DummyResolver {
911+
fn resolve(&self, _identity: &[u8]) -> Option<Vec<u8>> {
912+
Some(b"key".to_vec())
913+
}
914+
}
915+
916+
// PSK config but the user filtered DTLS 1.2 down to a cert-only suite
917+
// and disabled DTLS 1.3. AuthMode::Psk would reject every surviving
918+
// suite at runtime, so build() should fail fast here.
919+
let result = Config::builder()
920+
.with_psk_client(b"identity".to_vec(), Arc::new(DummyResolver))
921+
.dtls12_cipher_suites(&[Dtls12CipherSuite::ECDHE_ECDSA_AES128_GCM_SHA256])
922+
.dtls13_cipher_suites(&[])
923+
.build();
924+
match result {
925+
Err(Error::ConfigError(msg)) => assert!(
926+
msg.contains("PSK"),
927+
"error should mention PSK: {msg}"
928+
),
929+
Err(other) => panic!("expected ConfigError, got: {other:?}"),
930+
Ok(_) => panic!("expected error for PSK config with only non-PSK suites"),
931+
}
932+
}
933+
934+
#[test]
935+
fn psk_with_dtls13_but_no_psk_dtls12_suite_rejected() {
936+
struct DummyResolver;
937+
impl PskResolver for DummyResolver {
938+
fn resolve(&self, _identity: &[u8]) -> Option<Vec<u8>> {
939+
Some(b"key".to_vec())
940+
}
941+
}
942+
943+
// PSK configured, DTLS 1.2 filtered to cert-only, DTLS 1.3 left enabled.
944+
// The surviving DTLS 1.3 suite is not a fallback for Dtls::new_12_psk,
945+
// so build() must reject this config instead of producing one that can
946+
// never complete a PSK handshake.
947+
let result = Config::builder()
948+
.with_psk_client(b"identity".to_vec(), Arc::new(DummyResolver))
949+
.dtls12_cipher_suites(&[Dtls12CipherSuite::ECDHE_ECDSA_AES128_GCM_SHA256])
950+
.build();
951+
match result {
952+
Err(Error::ConfigError(msg)) => assert!(
953+
msg.contains("PSK"),
954+
"error should mention PSK: {msg}"
955+
),
956+
Err(other) => panic!("expected ConfigError, got: {other:?}"),
957+
Ok(_) => panic!(
958+
"expected error for PSK config with only non-PSK DTLS 1.2 suites, \
959+
even when DTLS 1.3 is enabled"
960+
),
961+
}
962+
}
963+
964+
#[test]
965+
fn psk_with_cert_dtls12_and_empty_kx_groups_rejected() {
966+
struct DummyResolver;
967+
impl PskResolver for DummyResolver {
968+
fn resolve(&self, _identity: &[u8]) -> Option<Vec<u8>> {
969+
Some(b"key".to_vec())
970+
}
971+
}
972+
973+
// Mixed config: PSK is set, but a cert-based DTLS 1.2 suite is also in
974+
// the filter alongside a PSK suite. That cert suite still needs an
975+
// ECDHE group, so kx_groups(&[]) must fail build — the fact that PSK
976+
// is also configured does not excuse the missing groups.
977+
let result = Config::builder()
978+
.with_psk_server(None, Arc::new(DummyResolver))
979+
.dtls12_cipher_suites(&[
980+
Dtls12CipherSuite::ECDHE_ECDSA_AES128_GCM_SHA256,
981+
Dtls12CipherSuite::PSK_AES128_CCM_8,
982+
])
983+
.dtls13_cipher_suites(&[])
984+
.kx_groups(&[])
985+
.build();
986+
match result {
987+
Err(Error::ConfigError(msg)) => assert!(
988+
msg.contains("key exchange"),
989+
"error should mention key exchange groups: {msg}"
990+
),
991+
Err(other) => panic!("expected ConfigError, got: {other:?}"),
992+
Ok(_) => panic!(
993+
"expected error when a cert-based DTLS 1.2 suite is enabled \
994+
without any kx groups, even alongside PSK"
995+
),
996+
}
997+
}
998+
999+
#[test]
1000+
fn psk_client_with_empty_kx_groups_builds() {
1001+
struct DummyResolver;
1002+
impl PskResolver for DummyResolver {
1003+
fn resolve(&self, _identity: &[u8]) -> Option<Vec<u8>> {
1004+
Some(b"key".to_vec())
1005+
}
1006+
}
1007+
1008+
// PSK suites don't need ECDHE groups. A truly PSK-only endpoint (with
1009+
// the DTLS 1.2 filter narrowed to PSK suites and DTLS 1.3 disabled)
1010+
// should be able to opt out of kx_groups entirely.
1011+
Config::builder()
1012+
.with_psk_client(b"identity".to_vec(), Arc::new(DummyResolver))
1013+
.dtls12_cipher_suites(&[Dtls12CipherSuite::PSK_AES128_CCM_8])
1014+
.dtls13_cipher_suites(&[])
1015+
.kx_groups(&[])
1016+
.build()
1017+
.expect("PSK-only client with empty kx_groups should build");
1018+
}
1019+
8841020
#[test]
8851021
fn filter_with_explicit_provider() {
8861022
#[cfg(feature = "aws-lc-rs")]

src/dtls12/server.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -575,13 +575,11 @@ impl State {
575575
}
576576

577577
/// PSK ServerKeyExchange: send identity hint only (no ECDHE, no signature).
578+
/// Per RFC 4279 §2, the message is omitted entirely when no hint is configured.
578579
fn send_server_key_exchange_psk(self, server: &mut Server) -> Result<Self, Error> {
579-
let hint = server
580-
.engine
581-
.config()
582-
.psk_identity_hint()
583-
.unwrap_or(&[])
584-
.to_vec();
580+
let Some(hint) = server.engine.config().psk_identity_hint().map(<[u8]>::to_vec) else {
581+
return Ok(Self::SendServerHelloDone);
582+
};
585583

586584
server
587585
.engine

src/lib.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,16 @@ impl Dtls {
300300
/// before the handshake begins. The `config` must have a
301301
/// [`PskResolver`] configured, and for clients a PSK identity
302302
/// via [`ConfigBuilder::with_psk_client`](ConfigBuilder).
303+
///
304+
/// Panics if `config` has no PSK configured. Without PSK data the
305+
/// PSK suite filter would leave zero negotiable suites, so failing
306+
/// fast at construction is preferable to a late handshake error.
303307
pub fn new_12_psk(config: Arc<Config>, now: Instant) -> Self {
308+
assert!(
309+
config.psk().is_some(),
310+
"Dtls::new_12_psk requires a PSK configuration; \
311+
set one via ConfigBuilder::with_psk_client or with_psk_server"
312+
);
304313
let inner = Inner::Server12(Server12::new_psk(config, now));
305314
Dtls { inner: Some(inner) }
306315
}
@@ -774,6 +783,13 @@ mod test {
774783
assert_eq!(dtls.protocol_version(), None);
775784
}
776785

786+
#[test]
787+
#[should_panic(expected = "requires a PSK configuration")]
788+
fn new_12_psk_panics_without_psk_config() {
789+
let config = Arc::new(Config::default());
790+
let _ = Dtls::new_12_psk(config, Instant::now());
791+
}
792+
777793
#[test]
778794
fn test_auto_server_send_application_data_pending() {
779795
let mut dtls = new_instance_auto();

0 commit comments

Comments
 (0)