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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
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
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
///
/// 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>,
input: Range<usize>,
data: &mut [u8],
) -> Res<usize> {
let aad = &data[aad];
let input = &data[input];
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.
///
/// # 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],
) -> 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
57 changes: 36 additions & 21 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 @@ -1502,15 +1506,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 +1526,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 +1539,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,19 +1562,22 @@ 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) => {
Expand Down Expand Up @@ -1592,9 +1605,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 +1620,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 +1629,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 +1652,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 Down Expand Up @@ -1980,7 +1995,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 +2008,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
42 changes: 32 additions & 10 deletions neqo-transport/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use crate::{
Error, Res,
};

const MAX_AUTH_TAG: usize = 32;
/// The number of invocations remaining on a write cipher before we try
/// to update keys. This has to be much smaller than the number returned
/// by `CryptoDxState::limit` or updates will happen too often. As we don't
Expand Down Expand Up @@ -634,12 +633,18 @@ impl CryptoDxState {
self.used_pn.end
}

pub fn encrypt(&mut self, pn: PacketNumber, hdr: &[u8], body: &[u8]) -> Res<Vec<u8>> {
pub fn encrypt(
&mut self,
pn: PacketNumber,
hdr: Range<usize>,
body: Range<usize>,
data: &mut [u8],
) -> Res<usize> {
debug_assert_eq!(self.direction, CryptoDxDirection::Write);
qtrace!(
"[{self}] encrypt pn={pn} hdr={} body={}",
hex(hdr),
hex(body)
"[{self}] encrypt_in_place pn={pn} hdr={} body={}",
hex(data[hdr.clone()].as_ref()),
hex(data[body.clone()].as_ref())
);

// The numbers in `Self::limit` assume a maximum packet size of `LIMIT`.
Expand All @@ -653,14 +658,12 @@ impl CryptoDxState {
}
self.invoked()?;

let size = body.len() + MAX_AUTH_TAG;
let mut out = vec![0; size];
let res = self.aead.encrypt(pn, hdr, body, &mut out)?;
let len = self.aead.encrypt_in_place(pn, hdr, body, data)?;

qtrace!("[{self}] encrypt ct={}", hex(res));
qtrace!("[{self}] encrypt ct={}", hex(data));
debug_assert_eq!(pn, self.next_pn());
self.used(pn)?;
Ok(res.to_vec())
Ok(len)
}

#[must_use]
Expand All @@ -682,6 +685,25 @@ impl CryptoDxState {
Ok(res.to_vec())
}

pub fn decrypt_in_place(
&mut self,
pn: PacketNumber,
hdr: Range<usize>,
body: Range<usize>,
data: &mut [u8],
) -> Res<usize> {
debug_assert_eq!(self.direction, CryptoDxDirection::Read);
qtrace!(
"[{self}] decrypt pn={pn} hdr={} body={}",
hex(data[hdr.clone()].as_ref()),
hex(data[body.clone()].as_ref())
);
self.invoked()?;
let len = self.aead.decrypt_in_place(pn, hdr, body, data)?;
self.used(pn)?;
Ok(len)
}

#[cfg(all(test, not(feature = "disable-encryption")))]
pub(crate) fn test_default() -> Self {
// This matches the value in packet.rs
Expand Down
Loading
Loading