client: detect HRR with incorrect session_id

See comment for justification from RFC.
This commit is contained in:
Joseph Birr-Pixton 2023-07-26 15:16:11 +01:00 committed by Daniel McCarney
parent 43fb7be823
commit aae5f19c85
2 changed files with 32 additions and 6 deletions

View File

@ -709,6 +709,27 @@ impl ExpectServerHelloOrHelloRetryRequest {
.illegal_param("server requested hrr with no changes"));
}
// Or does not echo the session_id from our ClientHello:
//
// > the HelloRetryRequest has the same format as a ServerHello message,
// > and the legacy_version, legacy_session_id_echo, cipher_suite, and
// > legacy_compression_method fields have the same meaning
// <https://www.rfc-editor.org/rfc/rfc8446#section-4.1.4>
//
// and
//
// > A client which receives a legacy_session_id_echo field that does not
// > match what it sent in the ClientHello MUST abort the handshake with an
// > "illegal_parameter" alert.
// <https://www.rfc-editor.org/rfc/rfc8446#section-4.1.3>
if hrr.session_id != self.next.session_id {
cx.common
.send_fatal_alert(AlertDescription::IllegalParameter);
return Err(Error::PeerMisbehavedError(
"server did not echo the session_id from client hello".to_string(),
));
}
// Or asks us to talk a protocol we didn't offer, or doesn't support HRR at all.
match hrr.get_supported_versions() {
Some(ProtocolVersion::TLSv1_3) => {

View File

@ -3713,9 +3713,8 @@ fn test_client_sends_helloretryrequest() {
}
#[test]
fn test_server_requests_retry_with_echoed_session_id() {
use rustls::internal::msgs::handshake::SessionID;
let expected_session_id = SessionID::random().unwrap();
fn test_client_rejects_hrr_with_varied_session_id() {
let different_session_id = SessionID::random().unwrap();
let assert_client_sends_hello_with_secp384 = |msg: &mut Message| -> Altered {
if let MessagePayload::Handshake { parsed, encoded } = &mut msg.payload {
@ -3726,7 +3725,7 @@ fn test_server_requests_retry_with_echoed_session_id() {
assert_eq!(keyshares.len(), 1);
assert_eq!(keyshares[0].group, rustls::NamedGroup::secp384r1);
ch.session_id = expected_session_id;
ch.session_id = different_session_id;
*encoded = Payload::new(parsed.get_encoding());
}
}
@ -3739,7 +3738,7 @@ fn test_server_requests_retry_with_echoed_session_id() {
let group = hrr.get_requested_key_share_group();
assert_eq!(group, Some(rustls::NamedGroup::X25519));
assert_eq!(hrr.session_id, expected_session_id);
assert_eq!(hrr.session_id, different_session_id);
}
}
Altered::InPlace
@ -3767,7 +3766,12 @@ fn test_server_requests_retry_with_echoed_session_id() {
assert_server_requests_retry_and_echoes_session_id,
&mut client,
);
client.process_new_packets().unwrap();
assert_eq!(
client.process_new_packets(),
Err(Error::PeerMisbehavedError(
"server did not echo the session_id from client hello".to_string()
))
);
}
#[test]
@ -3928,6 +3932,7 @@ fn connection_types_are_not_huge() {
assert_lt(mem::size_of::<ClientConnection>(), 1600);
}
use rustls::internal::msgs::handshake::SessionID;
use rustls::internal::msgs::{
handshake::ClientExtension, handshake::HandshakePayload, message::Message,
message::MessagePayload,