Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased

* Split local DTLS invalid-state errors from peer-input errors #126
* Reject malformed DTLS 1.3 ClientHello extension vectors #133
* Reject malformed DTLS 1.3 KeyUpdate bodies #131
* Reject malformed DTLS 1.3 ACK record-number vectors #130
Expand Down
92 changes: 79 additions & 13 deletions src/dtls12/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ impl State {
};

if certificate.certificate_list.is_empty() {
return Err(Error::UnexpectedMessage(
return Err(Error::CertificateError(
"No server certificate received".to_string(),
));
}
Expand Down Expand Up @@ -585,7 +585,7 @@ impl State {
let cipher_suite = client
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

if cipher_suite.is_psk() {
self.await_server_key_exchange_psk(client)
Expand Down Expand Up @@ -669,7 +669,7 @@ impl State {
let cipher_suite = client
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

// Ensure the server's (hash, signature) pair was offered by the client
let offered = SignatureAndHashAlgorithm::supported().contains(&signature_algorithm);
Expand Down Expand Up @@ -843,7 +843,7 @@ impl State {
let cipher_suite = client
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

if cipher_suite.is_psk() {
// PSK: no certificates involved
Expand Down Expand Up @@ -905,7 +905,7 @@ impl State {
let cipher_suite = client
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

let suite_hash = cipher_suite.hash_algorithm();
let mut buf = Buf::new();
Expand Down Expand Up @@ -949,15 +949,13 @@ impl State {
fn derive_keys(client: &mut Client) -> Result<(), Error> {
trace!("Deriving keys");
let Some(cipher_suite) = client.engine.cipher_suite() else {
return Err(Error::UnexpectedMessage(
"No cipher suite selected".to_string(),
));
return Err(Error::InvalidState("No cipher suite selected".to_string()));
};

trace!("Using cipher suite for key derivation: {:?}", cipher_suite);

let Some(server_random) = &client.server_random else {
return Err(Error::UnexpectedMessage(
return Err(Error::InvalidState(
"No server random available".to_string(),
));
};
Expand All @@ -978,7 +976,7 @@ impl State {

// Use the captured session hash from when ServerHelloDone was received
let session_hash = client.captured_session_hash.as_ref().ok_or_else(|| {
Error::CryptoError(
Error::InvalidState(
"Extended Master Secret negotiated but session hash not captured".to_string(),
)
})?;
Expand Down Expand Up @@ -1256,9 +1254,7 @@ fn handshake_create_certificate(body: &mut Buf, engine: &mut Engine) -> Result<(
fn handshake_create_client_key_exchange(body: &mut Buf, engine: &mut Engine) -> Result<(), Error> {
// Just check that a cipher suite exists without binding to unused variable
let Some(cipher_suite) = engine.cipher_suite() else {
return Err(Error::UnexpectedMessage(
"No cipher suite selected".to_string(),
));
return Err(Error::InvalidState("No cipher suite selected".to_string()));
};
let key_exchange_algorithm = cipher_suite.as_key_exchange_algorithm();

Expand Down Expand Up @@ -1384,3 +1380,73 @@ impl LocalEvent {
}
}
}

#[cfg(test)]
mod tests {
use std::sync::Arc;

use super::*;

fn psk_client() -> Client {
let engine = Engine::new(Arc::new(Config::default()), AuthMode::Psk);
Client::new_with_engine(engine, Instant::now())
}

fn epoch0_handshake_packet(msg_type: MessageType, message_seq: u16, body: &[u8]) -> Vec<u8> {
let handshake_len = 12 + body.len();
let mut packet = Vec::new();
packet.push(ContentType::Handshake.as_u8());
packet.extend_from_slice(&[0xfe, 0xfd]);
packet.extend_from_slice(&0u16.to_be_bytes());
packet.extend_from_slice(&0u64.to_be_bytes()[2..]);
packet.extend_from_slice(&(handshake_len as u16).to_be_bytes());
packet.push(msg_type.as_u8());
packet.extend_from_slice(&(body.len() as u32).to_be_bytes()[1..]);
packet.extend_from_slice(&message_seq.to_be_bytes());
packet.extend_from_slice(&0u32.to_be_bytes()[1..]);
packet.extend_from_slice(&(body.len() as u32).to_be_bytes()[1..]);
packet.extend_from_slice(body);
packet
}

#[test]
fn empty_server_certificate_is_certificate_error() {
let mut client = psk_client();
client
.engine
.parse_packet(&epoch0_handshake_packet(
MessageType::Certificate,
0,
&[0, 0, 0],
))
.expect("queue empty Certificate");

let err = State::AwaitCertificate
.await_certificate(&mut client)
.expect_err("empty server Certificate should fail");

assert!(matches!(err, Error::CertificateError(_)));
}

#[test]
fn derive_keys_without_cipher_suite_is_invalid_state() {
let mut client = psk_client();

let err = State::derive_keys(&mut client)
.expect_err("derive_keys requires negotiated cipher suite");

assert!(matches!(err, Error::InvalidState(_)));
}

#[test]
fn derive_keys_without_server_random_is_invalid_state() {
let mut client = psk_client();
client
.engine
.set_cipher_suite(Dtls12CipherSuite::PSK_AES128_CCM_8);

let err = State::derive_keys(&mut client).expect_err("derive_keys requires server random");

assert!(matches!(err, Error::InvalidState(_)));
}
}
22 changes: 10 additions & 12 deletions src/dtls12/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,14 +728,14 @@ impl Engine {
where
F: FnOnce(&mut Buf),
{
let maybe_suite =
if epoch >= 1 {
Some(self.cipher_suite().ok_or_else(|| {
Error::UnexpectedMessage("No cipher suite selected".to_string())
})?)
} else {
None
};
let maybe_suite = if epoch >= 1 {
Some(
self.cipher_suite()
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?,
)
} else {
None
};

// Prepare the plaintext fragment
let mut fragment = self.buffers_free.pop();
Expand Down Expand Up @@ -923,7 +923,7 @@ impl Engine {
// explicit_nonce + tag). Used to size fragments to fit the MTU.
let protection_overhead = if epoch >= 1 {
self.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;
self.min_protected_fragment_len()
} else {
0
Expand Down Expand Up @@ -1190,9 +1190,7 @@ impl Engine {

pub fn generate_verify_data(&mut self, is_client: bool) -> Result<[u8; 12], Error> {
let Some(suite) = self.cipher_suite() else {
return Err(Error::UnexpectedMessage(
"No cipher suite selected".to_string(),
));
return Err(Error::InvalidState("No cipher suite selected".to_string()));
};
let algorithm = suite.hash_algorithm();
let mut handshake_hash = self.buffers_free.pop();
Expand Down
18 changes: 8 additions & 10 deletions src/dtls12/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ impl State {
let cs = server
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

// PSK suites skip Certificate
if cs.is_psk() {
Expand All @@ -532,15 +532,15 @@ impl State {
let cs = server
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

if cs.is_psk() {
return self.send_server_key_exchange_psk(server);
}

let client_random = server
.client_random
.ok_or_else(|| Error::UnexpectedMessage("No client random".to_string()))?;
.ok_or_else(|| Error::InvalidState("No client random".to_string()))?;
// unwrap: is ok because we set the random in handle_timeout
let server_random = server.random.unwrap();

Expand Down Expand Up @@ -668,7 +668,7 @@ impl State {
let cs = server
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

// PSK: no client certificates
if cs.is_psk() {
Expand Down Expand Up @@ -747,7 +747,7 @@ impl State {
let suite = server
.engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite selected".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite selected".to_string()))?;

if suite.is_psk() {
// Extract PSK identity range before dropping handshake
Expand Down Expand Up @@ -839,7 +839,7 @@ impl State {
};

let session_hash = server.captured_session_hash.as_ref().ok_or_else(|| {
Error::CryptoError(
Error::InvalidState(
"Extended Master Secret negotiated but session hash not captured".to_string(),
)
})?;
Expand Down Expand Up @@ -1189,7 +1189,7 @@ fn handshake_create_server_hello(

let cs = engine
.cipher_suite()
.ok_or_else(|| Error::UnexpectedMessage("No cipher suite".to_string()))?;
.ok_or_else(|| Error::InvalidState("No cipher suite".to_string()))?;

let srtp_pid = negotiated_srtp_profile.map(|p| match p {
SrtpProfile::AEAD_AES_256_GCM => SrtpProfileId::SRTP_AEAD_AES_256_GCM,
Expand Down Expand Up @@ -1220,9 +1220,7 @@ fn handshake_create_server_key_exchange(
algorithm: SignatureAndHashAlgorithm,
) -> Result<(), Error> {
let Some(cipher_suite) = engine.cipher_suite() else {
return Err(Error::UnexpectedMessage(
"No cipher suite selected".to_string(),
));
return Err(Error::InvalidState("No cipher suite selected".to_string()));
};

let key_exchange_algorithm = cipher_suite.as_key_exchange_algorithm();
Expand Down
64 changes: 59 additions & 5 deletions src/dtls13/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ impl State {
let key_exchange = client
.active_key_exchange
.take()
.ok_or_else(|| Error::CryptoError("No active key exchange".to_string()))?;
.ok_or_else(|| Error::InvalidState("No active key exchange".to_string()))?;

if key_exchange.group() != server_group {
return Err(Error::SecurityError(format!(
Expand Down Expand Up @@ -751,7 +751,7 @@ impl State {
}

if certificate.certificate_list.is_empty() {
return Err(Error::UnexpectedMessage(
return Err(Error::CertificateError(
"No server certificate received".to_string(),
));
}
Expand Down Expand Up @@ -864,7 +864,7 @@ impl State {
let server_hs_secret = client
.server_hs_traffic_secret
.as_ref()
.ok_or_else(|| Error::CryptoError("No server handshake traffic secret".to_string()))?;
.ok_or_else(|| Error::InvalidState("No server handshake traffic secret".to_string()))?;
let expected_verify_data = client.engine.compute_verify_data(server_hs_secret)?;

let maybe = client
Expand Down Expand Up @@ -903,7 +903,7 @@ impl State {

// Derive application secrets from handshake secret + transcript through server Finished
let handshake_secret = client.handshake_secret.as_ref().ok_or_else(|| {
Error::CryptoError("No handshake secret for application key derivation".to_string())
Error::InvalidState("No handshake secret for application key derivation".to_string())
})?;

let (c_ap_traffic, s_ap_traffic) =
Expand Down Expand Up @@ -997,7 +997,7 @@ impl State {
}

let client_hs_secret = client.client_hs_traffic_secret.as_ref().ok_or_else(|| {
Error::CryptoError("No client handshake traffic secret for Finished".to_string())
Error::InvalidState("No client handshake traffic secret for Finished".to_string())
})?;
let mut client_hs_secret_copy = Buf::new();
client_hs_secret_copy.extend_from_slice(client_hs_secret);
Expand Down Expand Up @@ -1526,3 +1526,57 @@ impl LocalEvent {
}
}
}

#[cfg(all(test, feature = "rcgen"))]
mod tests {
use std::sync::Arc;

use super::*;
use crate::Config;
use crate::certificate::generate_self_signed_certificate;

fn client() -> Client {
let cert = generate_self_signed_certificate().expect("generate cert");
let engine = Engine::new(Arc::new(Config::default()), cert);
Client::new_with_engine(engine, Instant::now())
}

fn epoch0_handshake_packet(msg_type: MessageType, message_seq: u16, body: &[u8]) -> Vec<u8> {
let handshake_len = 12 + body.len();
let mut packet = Vec::new();
packet.push(ContentType::Handshake.as_u8());
packet.extend_from_slice(&[0xfe, 0xfd]);
packet.extend_from_slice(&0u16.to_be_bytes());
packet.extend_from_slice(&0u64.to_be_bytes()[2..]);
packet.extend_from_slice(&(handshake_len as u16).to_be_bytes());
packet.push(msg_type.as_u8());
packet.extend_from_slice(&(body.len() as u32).to_be_bytes()[1..]);
packet.extend_from_slice(&message_seq.to_be_bytes());
packet.extend_from_slice(&0u32.to_be_bytes()[1..]);
packet.extend_from_slice(&(body.len() as u32).to_be_bytes()[1..]);
packet.extend_from_slice(body);
packet
}

#[test]
fn empty_server_certificate_is_certificate_error() {
let mut client = client();
client.engine.advance_peer_handshake_seq(); // ServerHello
client.engine.advance_peer_handshake_seq(); // EncryptedExtensions

client
.engine
.parse_packet(&epoch0_handshake_packet(
MessageType::Certificate,
2,
&[0, 0, 0, 0],
))
.expect("queue empty Certificate");

let err = State::AwaitCertificate
.await_certificate(&mut client)
.expect_err("empty server Certificate should fail");

assert!(matches!(err, Error::CertificateError(_)));
}
}
Loading
Loading