Skip to content

Commit

Permalink
refactor(s2n-quic-core): break CryptoError up into tls::Error and pac…
Browse files Browse the repository at this point in the history
…ket_protection::Error
  • Loading branch information
WesleyRosenblum committed Feb 8, 2024
1 parent 5f9f09c commit d3d6bc2
Show file tree
Hide file tree
Showing 33 changed files with 206 additions and 167 deletions.
6 changes: 3 additions & 3 deletions quic/s2n-quic-core/src/connection/close.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
28 changes: 6 additions & 22 deletions quic/s2n-quic-core/src/connection/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
2 changes: 1 addition & 1 deletion quic/s2n-quic-core/src/crypto/application/keyset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl<K: OneRttKey> KeySet<K> {
return Err(transport::Error::AEAD_LIMIT_REACHED.into());
}

Err(err.into())
Err(err)
}
}
}
Expand Down
17 changes: 9 additions & 8 deletions quic/s2n-quic-core/src/crypto/key.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -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,
};
Expand Down Expand Up @@ -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(())
Expand All @@ -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(())
Expand Down Expand Up @@ -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(())
}
}
Expand Down
6 changes: 2 additions & 4 deletions quic/s2n-quic-core/src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@
//!
pub mod application;
pub mod error;
pub mod handshake;
pub mod header_crypto;
pub mod initial;
Expand All @@ -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::*;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)?;

Expand Down
59 changes: 59 additions & 0 deletions quic/s2n-quic-core/src/crypto/packet_protection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,62 @@ 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 {
/// Sets the reason for the `packet_protection::Error`
#[must_use]
pub const fn with_reason(mut self, reason: &'static str) -> Self {
self.reason = reason;
self
}

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(),
}
}
}
4 changes: 2 additions & 2 deletions quic/s2n-quic-core/src/crypto/retry.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
12 changes: 6 additions & 6 deletions quic/s2n-quic-core/src/crypto/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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();
Expand All @@ -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)?;

Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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();

Expand Down
3 changes: 3 additions & 0 deletions quic/s2n-quic-core/src/crypto/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading

0 comments on commit d3d6bc2

Please sign in to comment.