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

feat: In-place crypto #2385

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 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
12 changes: 10 additions & 2 deletions neqo-common/src/datagram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::{net::SocketAddr, ops::Deref};
use std::{
net::SocketAddr,
ops::{Deref, DerefMut},
};

use crate::{hex_with_len, IpTos};

Expand Down Expand Up @@ -47,7 +50,6 @@ impl<D: AsRef<[u8]>> Datagram<D> {
}
}

#[cfg(test)]
impl<D: AsMut<[u8]> + AsRef<[u8]>> AsMut<[u8]> for Datagram<D> {
fn as_mut(&mut self) -> &mut [u8] {
self.d.as_mut()
Expand All @@ -65,6 +67,12 @@ impl Datagram<Vec<u8>> {
}
}

impl<D: AsRef<[u8]> + AsMut<[u8]>> DerefMut for Datagram<D> {
fn deref_mut(&mut self) -> &mut Self::Target {
AsMut::<[u8]>::as_mut(self)
}
}

impl<D: AsRef<[u8]>> Deref for Datagram<D> {
type Target = [u8];
#[must_use]
Expand Down
70 changes: 69 additions & 1 deletion neqo-crypto/src/aead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use std::{
fmt,
ops::{Deref, DerefMut},
ops::{Deref, DerefMut, Range},
os::raw::{c_char, c_uint},
ptr::null_mut,
};
Expand Down Expand Up @@ -126,6 +126,39 @@ impl RealAead {
Ok(&output[0..(l.try_into()?)])
}

/// Encrypt `data` consisting of `aad` and plaintext `input` in place.
///
/// The space provided in `data` needs to allow `Aead::expansion` more bytes to be appended.
///
/// # Errors
larseggert marked this conversation as resolved.
Show resolved Hide resolved
///
/// If the input can't be protected or any input is too large for NSS.
pub fn encrypt_in_place(
&self,
count: u64,
aad: Range<usize>,
larseggert marked this conversation as resolved.
Show resolved Hide resolved
input: Range<usize>,
data: &mut [u8],
) -> Res<usize> {
let aad = &data[aad];
let input = &data[input];
larseggert marked this conversation as resolved.
Show resolved Hide resolved
let mut l: c_uint = 0;
unsafe {
SSL_AeadEncrypt(
*self.ctx,
count,
aad.as_ptr(),
c_uint::try_from(aad.len())?,
input.as_ptr(),
c_uint::try_from(input.len())?,
input.as_ptr(),
&mut l,
c_uint::try_from(input.len() + self.expansion())?,
)
}?;
Ok(l.try_into()?)
}

/// Decrypt a ciphertext.
///
/// Note that NSS insists upon having extra space available for decryption, so
Expand Down Expand Up @@ -158,6 +191,41 @@ impl RealAead {
}?;
Ok(&output[0..(l.try_into()?)])
}

/// Decrypt a ciphertext in place.
///
/// Note that NSS insists upon having extra space available for decryption, so
/// the buffer for `output` should be the same length as `input`, even though
/// the final result will be shorter.
larseggert marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Errors
///
/// If the input isn't authenticated or any input is too large for NSS.
pub fn decrypt_in_place(
&self,
count: u64,
aad: Range<usize>,
input: Range<usize>,
data: &mut [u8],
larseggert marked this conversation as resolved.
Show resolved Hide resolved
) -> Res<usize> {
let aad = &data[aad];
let input = &data[input];
let mut l: c_uint = 0;
unsafe {
SSL_AeadDecrypt(
*self.ctx,
count,
aad.as_ptr(),
c_uint::try_from(aad.len())?,
input.as_ptr(),
c_uint::try_from(input.len())?,
input.as_ptr(),
&mut l,
c_uint::try_from(input.len())?,
)
}?;
Ok(l.try_into()?)
}
}

impl fmt::Debug for RealAead {
Expand Down
13 changes: 12 additions & 1 deletion neqo-crypto/src/aead_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::fmt;
use std::{fmt, ops::Range};

use crate::{
constants::{Cipher, Version},
Expand Down Expand Up @@ -46,6 +46,17 @@ impl AeadNull {
Ok(&output[..l + 16])
}

#[allow(clippy::missing_errors_doc)]
pub fn encrypt_in_place(
&self,
_count: u64,
_aad: Range<usize>,
input: Range<usize>,
_data: &mut [u8],
) -> Res<usize> {
Ok(input.len() + 16)
}

#[allow(clippy::missing_errors_doc)]
pub fn decrypt<'a>(
&self,
Expand Down
72 changes: 47 additions & 25 deletions neqo-transport/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use std::{

use neqo_common::{
event::Provider as EventProvider, hex, hex_snip_middle, hrtime, qdebug, qerror, qinfo,
qlog::NeqoQlog, qtrace, qwarn, Datagram, Decoder, Encoder, Role,
qlog::NeqoQlog, qtrace, qwarn, Datagram, Decoder, Encoder, IpTos, Role,
};
use neqo_crypto::{
agent::CertificateInfo, Agent, AntiReplay, AuthenticationStatus, Cipher, Client, Group,
Expand Down Expand Up @@ -1020,14 +1020,14 @@ impl Connection {
}

/// Process new input datagrams on the connection.
pub fn process_input(&mut self, d: Datagram<impl AsRef<[u8]>>, now: Instant) {
pub fn process_input(&mut self, d: Datagram<impl AsRef<[u8]> + AsMut<[u8]>>, now: Instant) {
self.process_multiple_input(iter::once(d), now);
}

/// Process new input datagrams on the connection.
pub fn process_multiple_input(
&mut self,
dgrams: impl IntoIterator<Item = Datagram<impl AsRef<[u8]>>>,
dgrams: impl IntoIterator<Item = Datagram<impl AsRef<[u8]> + AsMut<[u8]>>>,
now: Instant,
) {
let mut dgrams = dgrams.into_iter().peekable();
Expand Down Expand Up @@ -1160,7 +1160,11 @@ impl Connection {

/// Process input and generate output.
#[must_use = "Output of the process function must be handled"]
pub fn process(&mut self, dgram: Option<Datagram<impl AsRef<[u8]>>>, now: Instant) -> Output {
pub fn process(
&mut self,
dgram: Option<Datagram<impl AsRef<[u8]> + AsMut<[u8]>>>,
now: Instant,
) -> Output {
if let Some(d) = dgram {
self.input(d, now, now);
self.process_saved(now);
Expand Down Expand Up @@ -1238,7 +1242,12 @@ impl Connection {
}
}

fn is_stateless_reset(&self, path: &PathRef, d: &Datagram<impl AsRef<[u8]>>) -> bool {
fn is_stateless_reset(
&self,
path: &PathRef,
d: &[u8],
// d: &Datagram<impl AsRef<[u8]>>
) -> bool {
// If the datagram is too small, don't try.
// If the connection is connected, then the reset token will be invalid.
if d.len() < 16 || !self.state.connected() {
Expand All @@ -1251,7 +1260,8 @@ impl Connection {
fn check_stateless_reset(
&mut self,
path: &PathRef,
d: &Datagram<impl AsRef<[u8]>>,
d: &[u8],
// d: &Datagram<impl AsRef<[u8]>>,
first: bool,
now: Instant,
) -> Res<()> {
Expand Down Expand Up @@ -1502,15 +1512,16 @@ impl Connection {
fn postprocess_packet(
&mut self,
path: &PathRef,
d: &Datagram<impl AsRef<[u8]>>,
tos: IpTos,
remote: SocketAddr,
packet: &PublicPacket,
migrate: bool,
now: Instant,
) {
let space = PacketNumberSpace::from(packet.packet_type());
if let Some(space) = self.acks.get_mut(space) {
let space_ecn_marks = space.ecn_marks();
*space_ecn_marks += d.tos().into();
*space_ecn_marks += tos.into();
self.stats.borrow_mut().ecn_rx = *space_ecn_marks;
} else {
qtrace!("Not tracking ECN for dropped packet number space");
Expand All @@ -1521,7 +1532,7 @@ impl Connection {
}

if self.state.connected() {
self.handle_migration(path, d, migrate, now);
self.handle_migration(path, remote, migrate, now);
} else if self.role != Role::Client
&& (packet.packet_type() == PacketType::Handshake
|| (packet.dcid().len() >= 8 && packet.dcid() == self.local_initial_source_cid))
Expand All @@ -1534,7 +1545,12 @@ impl Connection {

/// Take a datagram as input. This reports an error if the packet was bad.
/// This takes two times: when the datagram was received, and the current time.
fn input(&mut self, d: Datagram<impl AsRef<[u8]>>, received: Instant, now: Instant) {
fn input(
&mut self,
d: Datagram<impl AsRef<[u8]> + AsMut<[u8]>>,
received: Instant,
now: Instant,
) {
// First determine the path.
let path = self.paths.find_path(
d.destination(),
Expand All @@ -1552,24 +1568,27 @@ impl Connection {
fn input_path(
&mut self,
path: &PathRef,
d: Datagram<impl AsRef<[u8]>>,
mut d: Datagram<impl AsRef<[u8]> + AsMut<[u8]>>,
now: Instant,
) -> Res<()> {
let mut slc = d.as_ref();
let mut dcid = None;

qtrace!("[{self}] {} input {}", path.borrow(), hex(&d));
let tos = d.tos();
let remote = d.source();
let len = d.len();
let mut slc = d.as_mut();
let mut dcid = None;
let pto = path.borrow().rtt().pto(self.confirmed());

// Handle each packet in the datagram.
while !slc.is_empty() {
self.stats.borrow_mut().packets_rx += 1;
let (packet, remainder) =
let slc_len = slc.len();
let (mut packet, remainder) =
match PublicPacket::decode(slc, self.cid_manager.decoder().as_ref()) {
Ok((packet, remainder)) => (packet, remainder),
Err(e) => {
qinfo!("[{self}] Garbage packet: {e}");
qtrace!("[{self}] Garbage packet contents: {}", hex(slc));
// qtrace!("[{self}] Garbage packet contents: {}", hex(slc));
self.stats.borrow_mut().pkt_dropped("Garbage packet");
break;
}
Expand All @@ -1592,9 +1611,9 @@ impl Connection {
"-> RX",
payload.packet_type(),
payload.pn(),
&payload[..],
d.tos(),
d.len(),
payload.as_ref(),
tos,
len,
);

#[cfg(feature = "build-fuzzing-corpus")]
Expand All @@ -1607,7 +1626,7 @@ impl Connection {
neqo_common::write_item_to_fuzzing_corpus(target, &payload[..]);
}

qlog::packet_received(&self.qlog, &packet, &payload, now);
// qlog::packet_received(&self.qlog, &packet, &payload, now);
let space = PacketNumberSpace::from(payload.packet_type());
if let Some(space) = self.acks.get_mut(space) {
if space.is_duplicate(payload.pn()) {
Expand All @@ -1616,7 +1635,9 @@ impl Connection {
} else {
match self.process_packet(path, &payload, now) {
Ok(migrate) => {
self.postprocess_packet(path, &d, &packet, migrate, now);
self.postprocess_packet(
path, tos, remote, &packet, migrate, now,
);
}
Err(e) => {
self.ensure_error_path(path, &packet, now);
Expand All @@ -1637,7 +1658,7 @@ impl Connection {
Error::KeysPending(cspace) => {
// This packet can't be decrypted because we don't have the keys yet.
// Don't check this packet for a stateless reset, just return.
let remaining = slc.len();
let remaining = slc_len;
self.save_datagram(cspace, d, remaining, now);
return Ok(());
}
Expand All @@ -1656,9 +1677,10 @@ impl Connection {
// Decryption failure, or not having keys is not fatal.
// If the state isn't available, or we can't decrypt the packet, drop
// the rest of the datagram on the floor, but don't generate an error.
self.check_stateless_reset(path, &d, dcid.is_none(), now)?;
self.check_stateless_reset(path, packet.data(), dcid.is_none(), now)?;
self.stats.borrow_mut().pkt_dropped("Decryption failure");
qlog::packet_dropped(&self.qlog, &packet, now);
break;
}
}
slc = remainder;
Expand Down Expand Up @@ -1980,7 +2002,7 @@ impl Connection {
fn handle_migration(
&mut self,
path: &PathRef,
d: &Datagram<impl AsRef<[u8]>>,
remote: SocketAddr,
migrate: bool,
now: Instant,
) {
Expand All @@ -1993,7 +2015,7 @@ impl Connection {

if self.ensure_permanent(path, now).is_ok() {
self.paths
.handle_migration(path, d.source(), now, &mut self.stats.borrow_mut());
.handle_migration(path, remote, now, &mut self.stats.borrow_mut());
} else {
qinfo!(
"[{self}] {} Peer migrated, but no connection ID available",
Expand Down
Loading
Loading