refactor(s2n-quic-core): break CryptoError up into tls::Error and packet_protection::Error (#2113)

* refactor(s2n-quic-core): break CryptoError up into tls::Error and packet_protection::Error

* remove `with_reason`

Author: WesleyRosenblum

quic/s2n-quic-core/src/connection/close.rs

@@ -1,7 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::{application, crypto, transport};
+use crate::{application, crypto::tls, transport};
 pub use crate::{frame::ConnectionClose, inet::SocketAddress};
 
 /// Provides a hook for applications to rewrite CONNECTION_CLOSE frames
@@ -116,8 +116,8 @@ impl Formatter for Production {
         //# includes replacing any alert with a generic alert, such as
         //# handshake_failure (0x0128 in QUIC).  Endpoints MAY use a generic
         //# error code to avoid possibly exposing confidential information.
-        if error.try_into_crypto_error().is_some() {
-            return transport::Error::from(crypto::CryptoError::HANDSHAKE_FAILURE).into();
+        if error.try_into_tls_error().is_some() {
+            return transport::Error::from(tls::Error::HANDSHAKE_FAILURE).into();
         }
 
         // only preserve the error code

quic/s2n-quic-core/src/connection/error.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{
-    application, connection, crypto::CryptoError, endpoint, frame::ConnectionClose, transport,
+    application, connection, crypto::packet_protection, endpoint, frame::ConnectionClose, transport,
 };
 use core::{convert::TryInto, fmt, panic, time::Duration};
 
@@ -469,13 +469,6 @@ impl From<transport::Error> for Error {
     }
 }
 
-impl From<CryptoError> for Error {
-    #[track_caller]
-    fn from(error: CryptoError) -> Self {
-        transport::Error::from(error).into()
-    }
-}
-
 impl<'a> From<ConnectionClose<'a>> for Error {
     #[track_caller]
     fn from(error: ConnectionClose) -> Self {
@@ -530,7 +523,7 @@ impl From<Error> for std::io::ErrorKind {
     }
 }
 
-/// Some connection methods may need to indicate both `TransportError`s and `CryptoError`s. This
+/// Some connection methods may need to indicate both `ConnectionError`s and `DecryptError`s. This
 /// enum is used to allow for either error type to be returned as appropriate.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum ProcessingError {
@@ -548,21 +541,12 @@ impl From<Error> for ProcessingError {
 impl From<crate::transport::Error> for ProcessingError {
     #[track_caller]
     fn from(inner_error: crate::transport::Error) -> Self {
-        // Try extracting out the decrypt error from other transport errors
-        if let Some(error) = inner_error.try_into_crypto_error() {
-            error.into()
-        } else {
-            Self::ConnectionError(inner_error.into())
-        }
+        Self::ConnectionError(inner_error.into())
     }
 }
 
-impl From<CryptoError> for ProcessingError {
-    fn from(inner_error: CryptoError) -> Self {
-        if inner_error.code == CryptoError::DECRYPT_ERROR.code {
-            Self::DecryptError
-        } else {
-            Self::ConnectionError(inner_error.into())
-        }
+impl From<packet_protection::Error> for ProcessingError {
+    fn from(_: packet_protection::Error) -> Self {
+        Self::DecryptError
     }
 }

quic/s2n-quic-core/src/crypto/application/keyset.rs

@@ -203,7 +203,7 @@ impl<K: OneRttKey> KeySet<K> {
                     return Err(transport::Error::AEAD_LIMIT_REACHED.into());
                 }
 
-                Err(err.into())
+                Err(err)
             }
         }
     }

quic/s2n-quic-core/src/crypto/key.rs

@@ -1,7 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::crypto::CryptoError;
+use crate::crypto::packet_protection;
 use s2n_codec::encoder::scatter;
 
 /// A trait for crypto keys
@@ -12,15 +12,15 @@ pub trait Key: Send {
         packet_number: u64,
         header: &[u8],
         payload: &mut [u8],
-    ) -> Result<(), CryptoError>;
+    ) -> Result<(), packet_protection::Error>;
 
     /// Encrypt a payload
     fn encrypt(
         &self,
         packet_number: u64,
         header: &[u8],
         payload: &mut scatter::Buffer,
-    ) -> Result<(), CryptoError>;
+    ) -> Result<(), packet_protection::Error>;
 
     /// Length of the appended tag
     fn tag_len(&self) -> usize;
@@ -37,8 +37,9 @@ pub trait Key: Send {
 #[cfg(any(test, feature = "testing"))]
 pub mod testing {
     use crate::crypto::{
+        packet_protection,
         retry::{IntegrityTag, INTEGRITY_TAG_LEN},
-        scatter, CryptoError, HandshakeHeaderKey, HandshakeKey, HeaderKey as CryptoHeaderKey,
+        scatter, HandshakeHeaderKey, HandshakeKey, HeaderKey as CryptoHeaderKey,
         HeaderProtectionMask, InitialHeaderKey, InitialKey, OneRttHeaderKey, OneRttKey, RetryKey,
         ZeroRttHeaderKey, ZeroRttKey,
     };
@@ -77,9 +78,9 @@ pub mod testing {
             _packet_number: u64,
             _header: &[u8],
             _payload: &mut [u8],
-        ) -> Result<(), CryptoError> {
+        ) -> Result<(), packet_protection::Error> {
             if self.fail_on_decrypt {
-                return Err(CryptoError::DECRYPT_ERROR);
+                return Err(packet_protection::Error::DECRYPT_ERROR);
             }
 
             Ok(())
@@ -91,7 +92,7 @@ pub mod testing {
             _packet_number: u64,
             _header: &[u8],
             payload: &mut scatter::Buffer,
-        ) -> Result<(), CryptoError> {
+        ) -> Result<(), packet_protection::Error> {
             // copy any bytes into the final slice
             payload.flatten();
             Ok(())
@@ -145,7 +146,7 @@ pub mod testing {
         fn generate_tag(_payload: &[u8]) -> IntegrityTag {
             [0u8; INTEGRITY_TAG_LEN]
         }
-        fn validate(_payload: &[u8], _tag: IntegrityTag) -> Result<(), CryptoError> {
+        fn validate(_payload: &[u8], _tag: IntegrityTag) -> Result<(), packet_protection::Error> {
             Ok(())
         }
     }

quic/s2n-quic-core/src/crypto/mod.rs

@@ -132,7 +132,6 @@
 //!
 
 pub mod application;
-pub mod error;
 pub mod handshake;
 pub mod header_crypto;
 pub mod initial;
@@ -149,7 +148,6 @@ pub mod zero_rtt;
 mod tests;
 
 pub use application::*;
-pub use error::*;
 pub use handshake::*;
 pub use header_crypto::*;
 pub use initial::*;
@@ -213,7 +211,7 @@ pub fn encrypt<'a, K: Key>(
     packet_number_len: PacketNumberLen,
     header_len: usize,
     payload: scatter::Buffer<'a>,
-) -> Result<(EncryptedPayload<'a>, EncoderBuffer<'a>), CryptoError> {
+) -> Result<(EncryptedPayload<'a>, EncoderBuffer<'a>), packet_protection::Error> {
     let header_with_pn_len = packet_number_len.bytesize() + header_len;
 
     let (mut payload, extra) = payload.into_inner();
@@ -254,7 +252,7 @@ pub fn decrypt<'a, K: Key>(
     key: &K,
     packet_number: PacketNumber,
     payload: EncryptedPayload<'a>,
-) -> Result<(DecoderBufferMut<'a>, DecoderBufferMut<'a>), CryptoError> {
+) -> Result<(DecoderBufferMut<'a>, DecoderBufferMut<'a>), packet_protection::Error> {
     let (header, payload) = payload.split_mut();
     key.decrypt(packet_number.as_crypto_nonce(), header, payload)?;
 

quic/s2n-quic-core/src/crypto/packet_protection.rs

@@ -14,3 +14,55 @@ pub const QUIC_IV_LABEL: [u8; 7] = *b"quic iv";
 //= https://www.rfc-editor.org/rfc/rfc9001#section-5.1
 //# The header protection key uses the "quic hp" label; see Section 5.4.
 pub const QUIC_HP_LABEL: [u8; 7] = *b"quic hp";
+
+use core::fmt;
+use s2n_codec::DecoderError;
+
+/// Error type for errors during removal of packet protection
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "thiserror", derive(thiserror::Error))]
+pub struct Error {
+    pub reason: &'static str,
+}
+
+impl Error {
+    pub const DECODE_ERROR: Self = Self {
+        reason: "DECODE_ERROR",
+    };
+    pub const DECRYPT_ERROR: Self = Self {
+        reason: "DECRYPT_ERROR",
+    };
+    pub const INTERNAL_ERROR: Self = Self {
+        reason: "INTERNAL_ERROR",
+    };
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if !self.reason.is_empty() {
+            self.reason.fmt(f)
+        } else {
+            write!(f, "packet_protection::Error")
+        }
+    }
+}
+
+impl fmt::Debug for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let mut d = f.debug_struct("packet_protection::Error");
+
+        if !self.reason.is_empty() {
+            d.field("reason", &self.reason);
+        }
+
+        d.finish()
+    }
+}
+
+impl From<DecoderError> for Error {
+    fn from(decoder_error: DecoderError) -> Self {
+        Self {
+            reason: decoder_error.into(),
+        }
+    }
+}

quic/s2n-quic-core/src/crypto/retry.rs

@@ -1,15 +1,15 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::crypto::CryptoError;
+use crate::crypto::packet_protection;
 use hex_literal::hex;
 
 pub const INTEGRITY_TAG_LEN: usize = 16;
 pub type IntegrityTag = [u8; INTEGRITY_TAG_LEN];
 
 pub trait RetryKey {
     fn generate_tag(payload: &[u8]) -> IntegrityTag;
-    fn validate(payload: &[u8], tag: IntegrityTag) -> Result<(), CryptoError>;
+    fn validate(payload: &[u8], tag: IntegrityTag) -> Result<(), packet_protection::Error>;
 }
 
 //= https://www.rfc-editor.org/rfc/rfc9001#section-5.8

quic/s2n-quic-core/src/crypto/tests.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{
-    crypto::{scatter, CryptoError, HeaderKey, HeaderProtectionMask, Key, ProtectedPayload},
+    crypto::{packet_protection, scatter, HeaderKey, HeaderProtectionMask, Key, ProtectedPayload},
     packet::number::{PacketNumber, PacketNumberSpace},
     varint::VarInt,
 };
@@ -43,7 +43,7 @@ fn round_trip() {
 fn fuzz_unprotect(
     input: &mut [u8],
     largest_packet_number: PacketNumber,
-) -> Result<(PacketNumber, usize), CryptoError> {
+) -> Result<(PacketNumber, usize), packet_protection::Error> {
     let buffer = DecoderBufferMut::new(input);
     let header_len = {
         let peek = buffer.peek();
@@ -64,7 +64,7 @@ fn fuzz_unprotect(
     packet_number
         .truncate(largest_packet_number)
         .filter(|actual| truncated_packet_number.eq(actual))
-        .ok_or(CryptoError::DECODE_ERROR)?;
+        .ok_or(packet_protection::Error::DECODE_ERROR)?;
 
     let (_header, _payload) = crate::crypto::decrypt(&FuzzCrypto, packet_number, payload)?;
 
@@ -76,7 +76,7 @@ fn fuzz_protect(
     header_len: usize,
     largest_packet_number: PacketNumber,
     packet_number: PacketNumber,
-) -> Result<(), CryptoError> {
+) -> Result<(), packet_protection::Error> {
     let payload_len = input.len();
     let mut payload = EncoderBuffer::new(input);
     payload.set_position(payload_len);
@@ -108,7 +108,7 @@ impl Key for FuzzCrypto {
         packet_number: u64,
         _header: &[u8],
         payload: &mut [u8],
-    ) -> Result<(), CryptoError> {
+    ) -> Result<(), packet_protection::Error> {
         let mask = packet_number as u8;
         for byte in payload.iter_mut() {
             *byte ^= mask;
@@ -121,7 +121,7 @@ impl Key for FuzzCrypto {
         packet_number: u64,
         _header: &[u8],
         payload: &mut scatter::Buffer,
-    ) -> Result<(), CryptoError> {
+    ) -> Result<(), packet_protection::Error> {
         let payload = payload.flatten();
         let (payload, _) = payload.split_mut();
 

quic/s2n-quic-core/src/crypto/tls.rs

@@ -6,6 +6,9 @@ pub use bytes::{Bytes, BytesMut};
 use core::{convert::TryFrom, fmt::Debug};
 use zerocopy::{AsBytes, FromBytes, FromZeroes, Unaligned};
 
+mod error;
+pub use error::Error;
+
 #[cfg(any(test, feature = "testing"))]
 pub mod testing;