Skip to content

Commit 1031234

Browse files
jaredwolffclaude
andcommitted
Prevent PSK cipher suite downgrade in certificate mode
Certificate-mode contexts (with a private key) could negotiate PSK suites, skipping Certificate/CertificateVerify and bypassing certificate authentication. Fix is_cipher_suite_compatible() to reject PSK suites when a private key is present, and filter PSK suites from dtls12_cipher_suites() when no PskResolver is configured. Also fixes pre-existing clippy lints (items_after_test_module in server.rs, needless_borrows_for_generic_args in kx_group.rs, redundant_pattern_matching in cross_matrix.rs) and runs cargo fmt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8c1ad4b commit 1031234

11 files changed

Lines changed: 205 additions & 87 deletions

File tree

src/config.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ impl fmt::Debug for Config {
5656
.field("mtu", &self.mtu)
5757
.field("max_queue_rx", &self.max_queue_rx)
5858
.field("max_queue_tx", &self.max_queue_tx)
59-
.field("require_client_certificate", &self.require_client_certificate)
59+
.field(
60+
"require_client_certificate",
61+
&self.require_client_certificate,
62+
)
6063
.field("use_server_cookie", &self.use_server_cookie)
6164
.field("flight_start_rto", &self.flight_start_rto)
6265
.field("flight_retries", &self.flight_retries)
@@ -212,16 +215,22 @@ impl Config {
212215
/// Returns all provider-supported DTLS 1.2 cipher suites when no filter
213216
/// is set. When a filter is set via the builder's `dtls12_cipher_suites`
214217
/// method, only suites in both the provider and the filter are returned.
218+
///
219+
/// PSK cipher suites are excluded when no [`PskResolver`] is configured,
220+
/// preventing a certificate-mode endpoint from negotiating a PSK suite
221+
/// and inadvertently skipping certificate authentication.
215222
pub fn dtls12_cipher_suites(
216223
&self,
217224
) -> impl Iterator<Item = &'static dyn SupportedDtls12CipherSuite> + '_ {
218225
let filter = self.dtls12_cipher_suites.as_ref();
226+
let has_psk = self.psk_resolver.is_some();
219227
self.crypto_provider
220228
.supported_cipher_suites()
221229
.filter(move |cs| match filter {
222230
Some(list) => list.contains(&cs.suite()),
223231
None => true,
224232
})
233+
.filter(move |cs| has_psk || !cs.suite().is_psk())
225234
}
226235

227236
/// Allowed DTLS 1.3 cipher suites, filtered by the config's allow-list.
@@ -286,7 +295,10 @@ impl fmt::Debug for ConfigBuilder {
286295
.field("mtu", &self.mtu)
287296
.field("max_queue_rx", &self.max_queue_rx)
288297
.field("max_queue_tx", &self.max_queue_tx)
289-
.field("require_client_certificate", &self.require_client_certificate)
298+
.field(
299+
"require_client_certificate",
300+
&self.require_client_certificate,
301+
)
290302
.field("use_server_cookie", &self.use_server_cookie)
291303
.field("flight_start_rto", &self.flight_start_rto)
292304
.field("flight_retries", &self.flight_retries)
@@ -786,11 +798,40 @@ mod tests {
786798
fn no_filter_returns_all() {
787799
let config = Config::default();
788800
// Default provider should have at least 2 DTLS 1.2 and 2 DTLS 1.3 suites
801+
// (PSK suites are excluded without a resolver, so only non-PSK count)
789802
assert!(config.dtls12_cipher_suites().count() >= 2);
790803
assert!(config.dtls13_cipher_suites().count() >= 2);
791804
assert!(config.kx_groups().count() >= 2);
792805
}
793806

807+
#[test]
808+
fn psk_suites_excluded_without_resolver() {
809+
let config = Config::default();
810+
assert!(
811+
config.dtls12_cipher_suites().all(|cs| !cs.suite().is_psk()),
812+
"PSK suites should be excluded when no PskResolver is configured"
813+
);
814+
}
815+
816+
#[test]
817+
fn psk_suites_included_with_resolver() {
818+
struct DummyResolver;
819+
impl PskResolver for DummyResolver {
820+
fn resolve(&self, _identity: &[u8]) -> Option<Vec<u8>> {
821+
None
822+
}
823+
}
824+
825+
let config = Config::builder()
826+
.with_psk_resolver(Arc::new(DummyResolver))
827+
.build()
828+
.expect("config with PSK resolver should build");
829+
assert!(
830+
config.dtls12_cipher_suites().any(|cs| cs.suite().is_psk()),
831+
"PSK suites should be included when a PskResolver is configured"
832+
);
833+
}
834+
794835
#[test]
795836
fn filter_with_explicit_provider() {
796837
#[cfg(feature = "aws-lc-rs")]

src/crypto/aws_lc_rs/cipher_suite.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,9 @@ impl SupportedDtls12CipherSuite for PskAes128Ccm8 {
258258
}
259259

260260
fn create_cipher(&self, key: &[u8]) -> Result<Box<dyn Cipher>, String> {
261-
Ok(Box::new(crate::crypto::ccm_cipher::AesCcm8Cipher::new(key)?))
261+
Ok(Box::new(crate::crypto::ccm_cipher::AesCcm8Cipher::new(
262+
key,
263+
)?))
262264
}
263265
}
264266

src/crypto/rust_crypto/cipher_suite.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,9 @@ impl SupportedDtls12CipherSuite for PskAes128Ccm8 {
308308
}
309309

310310
fn create_cipher(&self, key: &[u8]) -> Result<Box<dyn Cipher>, String> {
311-
Ok(Box::new(crate::crypto::ccm_cipher::AesCcm8Cipher::new(key)?))
311+
Ok(Box::new(crate::crypto::ccm_cipher::AesCcm8Cipher::new(
312+
key,
313+
)?))
312314
}
313315
}
314316

src/crypto/rust_crypto/kx_group.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl EcdhKeyExchange {
4747
match group {
4848
NamedGroup::X25519 => {
4949
use rand_core::OsRng;
50-
let secret = x25519_dalek::EphemeralSecret::random_from_rng(&mut OsRng);
50+
let secret = x25519_dalek::EphemeralSecret::random_from_rng(OsRng);
5151
let public_key_obj = x25519_dalek::PublicKey::from(&secret);
5252
buf.clear();
5353
buf.extend_from_slice(public_key_obj.as_bytes());

src/dtls12/client.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,7 @@ impl State {
714714
};
715715

716716
let hint_range = match &ske.params {
717-
crate::dtls12::message::ServerKeyExchangeParams::Psk(psk) => {
718-
psk.hint_range.clone()
719-
}
717+
crate::dtls12::message::ServerKeyExchangeParams::Psk(psk) => psk.hint_range.clone(),
720718
_ => {
721719
return Err(Error::UnexpectedMessage(
722720
"ECDHE ServerKeyExchange in PSK path".to_string(),
@@ -1238,9 +1236,7 @@ fn handshake_create_client_key_exchange(body: &mut Buf, engine: &mut Engine) ->
12381236
.psk_resolver()
12391237
.ok_or_else(|| Error::SecurityError("No PSK resolver configured".to_string()))?
12401238
.resolve(&identity)
1241-
.ok_or_else(|| {
1242-
Error::SecurityError("PSK resolver returned no key".to_string())
1243-
})?;
1239+
.ok_or_else(|| Error::SecurityError("PSK resolver returned no key".to_string()))?;
12441240

12451241
// Set the PSK and compute pre-master secret
12461242
let crypto = engine.crypto_context_mut();

src/dtls12/context.rs

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -582,8 +582,8 @@ impl CryptoContext {
582582
.private_key
583583
.as_ref()
584584
.is_some_and(|pk| sig_alg == pk.algorithm()),
585-
// PSK suite: no certificate needed
586-
None => true,
585+
// PSK suite: only compatible in PSK mode (no private key)
586+
None => self.private_key.is_none(),
587587
}
588588
}
589589

@@ -614,3 +614,72 @@ impl CryptoContext {
614614
)
615615
}
616616
}
617+
618+
#[cfg(test)]
619+
mod tests {
620+
use super::*;
621+
use crate::Config;
622+
623+
#[test]
624+
fn certificate_mode_rejects_psk_suites() {
625+
let cert = crate::certificate::generate_self_signed_certificate().expect("generate cert");
626+
let config = Arc::new(Config::default());
627+
let ctx = CryptoContext::new(cert.certificate, cert.private_key, config);
628+
629+
for suite in Dtls12CipherSuite::supported() {
630+
if suite.is_psk() {
631+
assert!(
632+
!ctx.is_cipher_suite_compatible(*suite),
633+
"Certificate-mode context must reject PSK suite {:?}",
634+
suite
635+
);
636+
}
637+
}
638+
}
639+
640+
#[test]
641+
fn certificate_mode_accepts_ecdhe_suites() {
642+
let cert = crate::certificate::generate_self_signed_certificate().expect("generate cert");
643+
let config = Arc::new(Config::default());
644+
let ctx = CryptoContext::new(cert.certificate, cert.private_key, config);
645+
646+
// At least one ECDHE_ECDSA suite should be compatible
647+
assert!(
648+
Dtls12CipherSuite::supported()
649+
.iter()
650+
.filter(|s| !s.is_psk())
651+
.any(|s| ctx.is_cipher_suite_compatible(*s)),
652+
"Certificate-mode context must accept at least one ECDHE suite"
653+
);
654+
}
655+
656+
#[test]
657+
fn psk_mode_rejects_certificate_suites() {
658+
let config = Arc::new(Config::default());
659+
let ctx = CryptoContext::new_psk(config);
660+
661+
for suite in Dtls12CipherSuite::supported() {
662+
if !suite.is_psk() {
663+
assert!(
664+
!ctx.is_cipher_suite_compatible(*suite),
665+
"PSK-mode context must reject certificate suite {:?}",
666+
suite
667+
);
668+
}
669+
}
670+
}
671+
672+
#[test]
673+
fn psk_mode_accepts_psk_suites() {
674+
let config = Arc::new(Config::default());
675+
let ctx = CryptoContext::new_psk(config);
676+
677+
assert!(
678+
Dtls12CipherSuite::supported()
679+
.iter()
680+
.filter(|s| s.is_psk())
681+
.any(|s| ctx.is_cipher_suite_compatible(*s)),
682+
"PSK-mode context must accept at least one PSK suite"
683+
);
684+
}
685+
}

src/dtls12/message/client_key_exchange.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,16 @@ impl ClientPskKeys {
115115
let (input, identity_len) = nom::number::complete::be_u16(input)?;
116116
let (input, identity_slice) = take(identity_len as usize)(input)?;
117117

118-
let relative_offset =
119-
identity_slice.as_ptr() as usize - original_input.as_ptr() as usize;
118+
let relative_offset = identity_slice.as_ptr() as usize - original_input.as_ptr() as usize;
120119
let start = base_offset + relative_offset;
121120
let end = start + identity_slice.len();
122121

123-
Ok((input, ClientPskKeys { identity_range: start..end }))
122+
Ok((
123+
input,
124+
ClientPskKeys {
125+
identity_range: start..end,
126+
},
127+
))
124128
}
125129

126130
pub fn serialize(&self, buf: &[u8], output: &mut Buf) {

src/dtls12/message/server_key_exchange.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,7 @@ impl ServerKeyExchange {
4343
ServerKeyExchangeParams::Ecdh(ecdh_params) => {
4444
ecdh_params.serialize(buf, output, with_signature)
4545
}
46-
ServerKeyExchangeParams::Psk(psk_params) => {
47-
psk_params.serialize(buf, output)
48-
}
46+
ServerKeyExchangeParams::Psk(psk_params) => psk_params.serialize(buf, output),
4947
}
5048
}
5149

@@ -140,12 +138,16 @@ impl PskParams {
140138
let (input, hint_len) = nom::number::complete::be_u16(input)?;
141139
let (input, hint_slice) = take(hint_len as usize)(input)?;
142140

143-
let relative_offset =
144-
hint_slice.as_ptr() as usize - original_input.as_ptr() as usize;
141+
let relative_offset = hint_slice.as_ptr() as usize - original_input.as_ptr() as usize;
145142
let start = base_offset + relative_offset;
146143
let end = start + hint_slice.len();
147144

148-
Ok((input, PskParams { hint_range: start..end }))
145+
Ok((
146+
input,
147+
PskParams {
148+
hint_range: start..end,
149+
},
150+
))
149151
}
150152

151153
pub fn serialize(&self, buf: &[u8], output: &mut Buf) {

0 commit comments

Comments
 (0)