Skip to content

Commit

Permalink
Fix DoS vulnerability in TLS1.3 middlebox CCS handling
Browse files Browse the repository at this point in the history
This affects TLS1.3 client and server sessions.  It does not
affect TLS1.2 sessions.

Discussion
==========

RFC8446 says of the "Middlebox Compatibility Mode" feature:

  Either side can send change_cipher_spec at any time during
  the handshake, as they must be ignored by the peer

This unnecessary flexibility meant we can't weave an
optional receipt of a CCS into our state machine (like we did for TLS1.2),
so we just drop CCS messages received after negotiating TLS1.3.

That's a problem, though: CCS messages are 6 bytes long, and many
can be delivered in a single TCP segment.  Each one results in a
small but non-zero amount of processing.

However, this code path is fast: in benchmarks rustls can drop ~4 million
CCSs per second, per core.  In the PoC code graciously provided by the
reporter, ~168Mbps of traffic needs to pass over lo to saturate a single
CPU core (you'll note these measurements agree with each other, to an order
of magnitude).

It's really likely that a better overall DoS vector is *just sending ClientHellos*,
where each core can only process ~thousands per second, for the cost of ~200 bytes;
ie 250Kbps (as an order of magnitude) to saturate one core.  This is especially
powerful if TFO is supported by both hosts.  But it's also more noisy.

So while this vulnerability is not thought to be serious, we can fix it
at negligible cost: only allow a maximum of one CCS per TLS1.3 handshake.

Thanks to Lenny Wang of Tencent Security Xuanwu Lab for the report.
  • Loading branch information
ctz committed Aug 16, 2020
1 parent 881bdcb commit e51bf92
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
3 changes: 2 additions & 1 deletion bogo/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
"FallbackSCSV*": "fallback countermeasure not yet implemented",
"RequireAnyClientCertificate-TLS12": "we don't send an alert in this case",
"TooManyKeyUpdates": "no limit implemented",
"TooManyChangeCipherSpec-*": "",
"SendUserCanceledAlerts-TooMany-TLS13": "",
"ServerBogusVersion": "we ignore legacy_version if there's an extension",
"Renegotiate-Client-*": "no reneg",
Expand Down Expand Up @@ -359,6 +358,8 @@
"Downgrade-TLS10-Server": ":INCOMPATIBLE:",
"SecondServerHelloNoVersion-TLS13": ":PEER_MISBEHAVIOUR:",
"SecondServerHelloWrongVersion-TLS13": ":INCOMPATIBLE:",
"TooManyChangeCipherSpec-Client-TLS13": ":PEER_MISBEHAVIOUR:",
"TooManyChangeCipherSpec-Server-TLS13": ":PEER_MISBEHAVIOUR:",
"EarlyData-CipherMismatch-Client-TLS13": ":PEER_MISBEHAVIOUR:",
"EarlyDataVersionDowngrade-Client-TLS13": ":WRONG_VERSION:",
"EarlyDataWithoutResume-Client-TLS13": ":PEER_MISBEHAVIOUR:",
Expand Down
6 changes: 2 additions & 4 deletions rustls/src/client/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::msgs::enums::CipherSuite;
use crate::msgs::enums::{AlertDescription, HandshakeType};
use crate::session::{Session, SessionCommon};
use crate::session::{Session, SessionCommon, MiddleboxCCS};
use crate::keylog::{KeyLog, NoKeyLog};
use crate::suites::{SupportedCipherSuite, ALL_CIPHERSUITES};
use crate::msgs::handshake::CertificatePayload;
Expand Down Expand Up @@ -457,9 +457,7 @@ impl ClientSessionImpl {

pub fn process_msg(&mut self, mut msg: Message) -> Result<(), TLSError> {
// TLS1.3: drop CCS at any time during handshaking
if self.common.is_tls13()
&& msg.is_content_type(ContentType::ChangeCipherSpec)
&& self.is_handshaking() {
if let MiddleboxCCS::Drop = self.common.filter_tls13_ccs(&msg)? {
trace!("Dropping CCS");
return Ok(());
}
Expand Down
6 changes: 2 additions & 4 deletions rustls/src/server/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::session::{Session, SessionCommon};
use crate::session::{Session, SessionCommon, MiddleboxCCS};
use crate::keylog::{KeyLog, NoKeyLog};
use crate::suites::{SupportedCipherSuite, ALL_CIPHERSUITES};
use crate::msgs::enums::ContentType;
Expand Down Expand Up @@ -377,9 +377,7 @@ impl ServerSessionImpl {

pub fn process_msg(&mut self, mut msg: Message) -> Result<(), TLSError> {
// TLS1.3: drop CCS at any time during handshaking
if self.common.is_tls13()
&& msg.is_content_type(ContentType::ChangeCipherSpec)
&& self.is_handshaking() {
if let MiddleboxCCS::Drop = self.common.filter_tls13_ccs(&msg)? {
trace!("Dropping CCS");
return Ok(());
}
Expand Down
32 changes: 32 additions & 0 deletions rustls/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,16 @@ enum Limit {
No
}

/// For TLS1.3 middlebox compatibility mode, how to handle
/// a received ChangeCipherSpec message.
pub enum MiddleboxCCS {
/// process the message as normal
Process,

/// just ignore it
Drop,
}

pub struct SessionCommon {
pub negotiated_version: Option<ProtocolVersion>,
pub is_client: bool,
Expand All @@ -399,6 +409,7 @@ pub struct SessionCommon {
pub traffic: bool,
pub early_traffic: bool,
sent_fatal_alert: bool,
received_middlebox_ccs: bool,
pub message_deframer: MessageDeframer,
pub handshake_joiner: HandshakeJoiner,
pub message_fragmenter: MessageFragmenter,
Expand All @@ -422,6 +433,7 @@ impl SessionCommon {
traffic: false,
early_traffic: false,
sent_fatal_alert: false,
received_middlebox_ccs: false,
message_deframer: MessageDeframer::new(),
handshake_joiner: HandshakeJoiner::new(),
message_fragmenter: MessageFragmenter::new(mtu.unwrap_or(MAX_FRAGMENT_LEN)),
Expand Down Expand Up @@ -463,6 +475,26 @@ impl SessionCommon {
}
}

pub fn filter_tls13_ccs(&mut self, msg: &Message) -> Result<MiddleboxCCS, TLSError> {
// pass message to handshake state machine if any of these are true:
// - TLS1.2 (where it's part of the state machine),
// - prior to determining the version (it's illegal as a first message)
// - if it's not a CCS at all
// - if we've finished the handshake
if !self.is_tls13() ||
!msg.is_content_type(ContentType::ChangeCipherSpec) ||
self.traffic {
return Ok(MiddleboxCCS::Process);
}

if self.received_middlebox_ccs {
Err(TLSError::PeerMisbehavedError("illegal middlebox CCS received".into()))
} else {
self.received_middlebox_ccs = true;
Ok(MiddleboxCCS::Drop)
}
}

pub fn decrypt_incoming(&mut self, encr: Message) -> Result<Message, TLSError> {
if self.record_layer.wants_close_before_decrypt() {
self.send_close_notify();
Expand Down

0 comments on commit e51bf92

Please sign in to comment.