Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
52 changes: 52 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,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(),
}
}
}
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
Loading