Skip to content

Commit

Permalink
Inline trivial alert helper functions
Browse files Browse the repository at this point in the history
  • Loading branch information
@djc
djc committed Apr 13, 2023
1 parent aecd933 commit 86719f8
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 120 deletions.
98 changes: 66 additions & 32 deletions rustls/src/client/hs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,10 @@ pub(super) fn process_alpn_protocol(
.alpn_protocols
.contains(alpn_protocol)
{
return Err(common.illegal_param(PeerMisbehaved::SelectedUnofferedApplicationProtocol));
return Err(common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::SelectedUnofferedApplicationProtocol,
));
}
}

Expand Down Expand Up @@ -513,9 +516,12 @@ impl State<ClientConnectionData> for ExpectServerHello {
.get_supported_versions()
.is_some()
{
return Err(cx
.common
.illegal_param(PeerMisbehaved::SelectedTls12UsingTls13VersionExtension));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::SelectedTls12UsingTls13VersionExtension,
)
});
}

TLSv1_2
Expand All @@ -532,9 +538,12 @@ impl State<ClientConnectionData> for ExpectServerHello {
};

if server_hello.compression_method != Compression::Null {
return Err(cx
.common
.illegal_param(PeerMisbehaved::SelectedUnofferedCompression));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::SelectedUnofferedCompression,
)
});
}

if server_hello.has_duplicate_extension() {
Expand Down Expand Up @@ -584,16 +593,22 @@ impl State<ClientConnectionData> for ExpectServerHello {
})?;

if version != suite.version().version {
return Err(cx
.common
.illegal_param(PeerMisbehaved::SelectedUnusableCipherSuiteForVersion));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::SelectedUnusableCipherSuiteForVersion,
)
});
}

match self.suite {
Some(prev_suite) if prev_suite != suite => {
return Err(cx
.common
.illegal_param(PeerMisbehaved::SelectedDifferentCipherSuiteAfterRetry));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::SelectedDifferentCipherSuiteAfterRetry,
)
});
}
_ => {
debug!("Using ciphersuite {:?}", suite);
Expand Down Expand Up @@ -691,17 +706,23 @@ impl ExpectServerHelloOrHelloRetryRequest {
// A retry request is illegal if it contains no cookie and asks for
// retry of a group we already sent.
if cookie.is_none() && req_group == Some(offered_key_share.group()) {
return Err(cx
.common
.illegal_param(PeerMisbehaved::IllegalHelloRetryRequestWithOfferedGroup));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::IllegalHelloRetryRequestWithOfferedGroup,
)
});
}

// Or has an empty cookie.
if let Some(cookie) = cookie {
if cookie.0.is_empty() {
return Err(cx
.common
.illegal_param(PeerMisbehaved::IllegalHelloRetryRequestWithEmptyCookie));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::IllegalHelloRetryRequestWithEmptyCookie,
)
});
}
}

Expand All @@ -715,16 +736,22 @@ impl ExpectServerHelloOrHelloRetryRequest {

// Or has the same extensions more than once
if hrr.has_duplicate_extension() {
return Err(cx
.common
.illegal_param(PeerMisbehaved::DuplicateHelloRetryRequestExtensions));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::DuplicateHelloRetryRequestExtensions,
)
});
}

// Or asks us to change nothing.
if cookie.is_none() && req_group.is_none() {
return Err(cx
.common
.illegal_param(PeerMisbehaved::IllegalHelloRetryRequestWithNoChanges));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::IllegalHelloRetryRequestWithNoChanges,
)
});
}

// Or asks us to talk a protocol we didn't offer, or doesn't support HRR at all.
Expand All @@ -733,9 +760,12 @@ impl ExpectServerHelloOrHelloRetryRequest {
cx.common.negotiated_version = Some(ProtocolVersion::TLSv1_3);
}
_ => {
return Err(cx.common.illegal_param(
PeerMisbehaved::IllegalHelloRetryRequestWithUnsupportedVersion,
));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::IllegalHelloRetryRequestWithUnsupportedVersion,
)
});
}
}

Expand All @@ -744,9 +774,12 @@ impl ExpectServerHelloOrHelloRetryRequest {
let cs = match config.find_cipher_suite(hrr.cipher_suite) {
Some(cs) => cs,
None => {
return Err(cx.common.illegal_param(
PeerMisbehaved::IllegalHelloRetryRequestWithUnofferedCipherSuite,
));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::IllegalHelloRetryRequestWithUnofferedCipherSuite,
)
});
}
};

Expand Down Expand Up @@ -775,7 +808,8 @@ impl ExpectServerHelloOrHelloRetryRequest {
let key_share = match req_group {
Some(group) if group != offered_key_share.group() => {
let group = kx::KeyExchange::choose(group, &config.kx_groups).ok_or_else(|| {
cx.common.illegal_param(
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::IllegalHelloRetryRequestWithUnofferedNamedGroup,
)
})?;
Expand Down
9 changes: 6 additions & 3 deletions rustls/src/client/tls12.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,12 @@ mod server_hello {
// public values and don't require constant time comparison
let has_downgrade_marker = self.randoms.server[24..] == tls12::DOWNGRADE_SENTINEL;
if tls13_supported && has_downgrade_marker {
return Err(cx
.common
.illegal_param(PeerMisbehaved::AttemptedDowngradeToTls12WhenTls13IsSupported));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::AttemptedDowngradeToTls12WhenTls13IsSupported,
)
});
}

// Doing EMS?
Expand Down
36 changes: 24 additions & 12 deletions rustls/src/client/tls13.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ pub(super) fn handle_server_hello(
})?;

if our_key_share.group() != their_key_share.group {
return Err(cx
.common
.illegal_param(PeerMisbehaved::WrongGroupForKeyShare));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::WrongGroupForKeyShare,
)
});
}

let key_schedule_pre_handshake = if let (Some(selected_psk), Some(early_key_schedule)) =
Expand All @@ -100,24 +103,33 @@ pub(super) fn handle_server_hello(
let resuming_suite = match suite.can_resume_from(resuming.suite()) {
Some(resuming) => resuming,
None => {
return Err(cx.common.illegal_param(
PeerMisbehaved::ResumptionOfferedWithIncompatibleCipherSuite,
));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::ResumptionOfferedWithIncompatibleCipherSuite,
)
});
}
};

// If the server varies the suite here, we will have encrypted early data with
// the wrong suite.
if cx.data.early_data.is_enabled() && resuming_suite != suite {
return Err(cx
.common
.illegal_param(PeerMisbehaved::EarlyDataOfferedWithVariedCipherSuite));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::EarlyDataOfferedWithVariedCipherSuite,
)
});
}

if selected_psk != 0 {
return Err(cx
.common
.illegal_param(PeerMisbehaved::SelectedInvalidPsk));
return Err({
cx.common.send_fatal_alert(
AlertDescription::IllegalParameter,
PeerMisbehaved::SelectedInvalidPsk,
)
});
}

debug!("Resuming using PSK");
Expand Down
4 changes: 0 additions & 4 deletions rustls/src/common_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,6 @@ impl CommonState {
}
}

pub(crate) fn illegal_param(&mut self, why: PeerMisbehaved) -> Error {
self.send_fatal_alert(AlertDescription::IllegalParameter, why)
}

/// Fragment `m`, encrypt the fragments, and then queue
/// the encrypted fragments for sending.
pub(crate) fn send_msg_encrypt(&mut self, m: PlainMessage) {
Expand Down

0 comments on commit 86719f8

Please sign in to comment.