Clean up checking of hs joiner state

- Also check at the end of a handshake flight (bogo now has tests for this).
- Unduplicate the code for CCS checking.
- Send a more accurate alert type.

rustls/src/client/hs.rs

@@ -86,7 +86,8 @@ pub fn illegal_param(sess: &mut ClientSessionImpl, why: &str) -> TLSError {
 
 pub fn check_aligned_handshake(sess: &mut ClientSessionImpl) -> Result<(), TLSError> {
     if !sess.common.handshake_joiner.is_empty() {
-        Err(illegal_param(sess, "keys changed with pending hs fragment"))
+        sess.common.send_fatal_alert(AlertDescription::UnexpectedMessage);
+        Err(TLSError::PeerMisbehavedError("key epoch or handshake flight with pending fragment".to_string()))
     } else {
         Ok(())
     }
@@ -648,6 +649,8 @@ impl ExpectServerHelloOrHelloRetryRequest {
         let hrr = extract_handshake!(m, HandshakePayload::HelloRetryRequest).unwrap();
         trace!("Got HRR {:?}", hrr);
 
+        check_aligned_handshake(sess)?;
+
         let has_cookie = hrr.get_cookie().is_some();
         let req_group = hrr.get_requested_key_share_group();
 

rustls/src/client/tls12.rs

@@ -449,6 +449,8 @@ impl hs::State for ExpectServerDone {
         let mut st = *self;
         st.handshake.transcript.add_message(&m);
 
+        hs::check_aligned_handshake(sess)?;
+
         debug!("Server cert is {:?}", st.server_cert.cert_chain);
         debug!("Server DNS name is {:?}", st.handshake.dns_name);
 
@@ -604,13 +606,7 @@ impl hs::State for ExpectCCS {
     fn handle(self: Box<Self>, sess: &mut ClientSessionImpl, _m: Message) -> hs::NextStateOrError {
         // CCS should not be received interleaved with fragmented handshake-level
         // message.
-        if !sess.common.handshake_joiner.is_empty() {
-            warn!("CCS received interleaved with fragmented handshake");
-            return Err(TLSError::InappropriateMessage {
-                expect_types: vec![ ContentType::Handshake ],
-                got_type: ContentType::ChangeCipherSpec,
-            });
-        }
+        hs::check_aligned_handshake(sess)?;
 
         // nb. msgs layer validates trivial contents of CCS
         sess.common
@@ -729,6 +725,8 @@ impl hs::State for ExpectFinished {
         let mut st = *self;
         let finished = extract_handshake!(m, HandshakePayload::Finished).unwrap();
 
+        hs::check_aligned_handshake(sess)?;
+
         // Work out what verify_data we expect.
         let vh = st.handshake.transcript.get_current_hash();
         let expect_verify_data = st.secrets

rustls/src/server/hs.rs

@@ -124,7 +124,8 @@ fn same_dns_name_or_both_none(a: Option<&webpki::DNSName>,
 // which is illegal.  Not mentioned in RFC.
 pub fn check_aligned_handshake(sess: &mut ServerSessionImpl) -> Result<(), TLSError> {
     if !sess.common.handshake_joiner.is_empty() {
-        Err(illegal_param(sess, "keys changed with pending hs fragment"))
+        sess.common.send_fatal_alert(AlertDescription::UnexpectedMessage);
+        Err(TLSError::PeerMisbehavedError("key epoch or handshake flight with pending fragment".to_string()))
     } else {
         Ok(())
     }
@@ -577,6 +578,9 @@ impl State for ExpectClientHello {
             return Err(decode_error(sess, "client sent duplicate extensions"));
         }
 
+        // No handshake messages should follow this one in this flight.
+        check_aligned_handshake(sess)?;
+
         // Are we doing TLS1.3?
         let maybe_versions_ext = client_hello.get_versions_extension();
         if let Some(versions) = maybe_versions_ext {

rustls/src/server/tls12.rs

@@ -12,7 +12,7 @@ use crate::session::SessionSecrets;
 use crate::server::ServerSessionImpl;
 use crate::verify;
 #[cfg(feature = "logging")]
-use crate::log::{warn, trace, debug};
+use crate::log::{trace, debug};
 use crate::error::TLSError;
 use crate::handshake::{check_handshake_message, check_message};
 
@@ -228,13 +228,7 @@ impl hs::State for ExpectCCS {
     fn handle(self: Box<Self>, sess: &mut ServerSessionImpl, _m: Message) -> hs::NextStateOrError {
         // CCS should not be received interleaved with fragmented handshake-level
         // message.
-        if !sess.common.handshake_joiner.is_empty() {
-            warn!("CCS received interleaved with fragmented handshake");
-            return Err(TLSError::InappropriateMessage {
-                expect_types: vec![ ContentType::Handshake ],
-                got_type: ContentType::ChangeCipherSpec,
-            });
-        }
+        hs::check_aligned_handshake(sess)?;
 
         sess.common
             .record_layer
@@ -348,6 +342,8 @@ impl hs::State for ExpectFinished {
     fn handle(mut self: Box<Self>, sess: &mut ServerSessionImpl, m: Message) -> hs::NextStateOrError {
         let finished = extract_handshake!(m, HandshakePayload::Finished).unwrap();
 
+        hs::check_aligned_handshake(sess)?;
+
         let vh = self.handshake.transcript.get_current_hash();
         let expect_verify_data = self.secrets.client_verify_data(&vh);