vincenzopalazzo
/
arti
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tor-cell: Refactor relay cells to copy much less 

We now manipulate raw relay cell bodies as (an alias for)
`Box<[u8;509]>` rather than as (an alias for) `[u8;509]`.  This
enables us to do much less copying.  It will become more important
soon, as we defer parsing relay cell bodies even longer.

Related to #7.

We also use SliceWriter to avoid allocating a Vec<> for every relay
message we want to encode, and instead encode directly into the
cell.
This commit is contained in:
Nick Mathewson 2023-02-12 21:19:28 -05:00
parent 9e2b6f3aed
commit ca3b33a1af
11 changed files with 94 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@ -4136,6 +4136,7 @@ dependencies = [
"cipher",
"coarsetime",
"derive_builder_fork_arti",
"derive_more",
"digest 0.10.6",
"educe",
"futures",

9
crates/tor-bytes/src/slicewriter.rs
View File

@ -65,8 +65,15 @@ impl<T> SliceWriter<T> {
///
/// On failure (if we tried to write too much), return an error.
pub fn try_unwrap(self) -> Result<(T, usize), SliceWriterError> {
let offset = self.offset()?;
Ok((self.data, offset))
}
/// Return the number of bytes written into this `SliceWriter` so far,
/// or an error if it has overflowed.
pub fn offset(&self) -> Result<usize, SliceWriterError> {
if self.offset != usize::MAX {
Ok((self.data, self.offset))
Ok(self.offset)
} else {
Err(SliceWriterError::Truncated)
}

2
crates/tor-cell/semver.md
View File

@ -8,3 +8,5 @@ BREAKING: Renamed ChanCell->AnyChanCell, ChanMsg->AnyChanMsg.
BREAKING: Renamed RelayCell->AnyRelayCell, RelayMsg->AnyRelayMsg.
BREAKING: Make ChannelCodec::decode() parameterized.
BREAKING: RelayEarly is now a real type.
BREAKING: RelayCell encoding and decoding functions now expect a Box<Body.

5
crates/tor-cell/src/chancell.rs
View File

@ -22,6 +22,11 @@ pub const CELL_DATA_LEN: usize = 509;
/// A cell body considered as a raw array of bytes
pub type RawCellBody = [u8; CELL_DATA_LEN];
/// A [`RawCellBody`] stored on the heap.
///
/// We use this often to avoid copying cell bodies around.
pub type BoxedCellBody = Box<RawCellBody>;
/// Channel-local identifier for a circuit.
///
/// A circuit ID can be 2 or 4 bytes long; since version 4 of the Tor

19
crates/tor-cell/src/chancell/msg.rs
View File

@ -1,6 +1,6 @@
//! Different kinds of messages that can be encoded in channel cells.
use super::{ChanCmd, RawCellBody, CELL_DATA_LEN};
use super::{BoxedCellBody, ChanCmd, RawCellBody, CELL_DATA_LEN};
use std::net::{IpAddr, Ipv4Addr};
use tor_basic_utils::skip_fmt;
use tor_bytes::{self, EncodeError, EncodeResult, Error, Readable, Reader, Result, Writer};
@ -353,7 +353,7 @@ impl Readable for Created2 {
///
/// A different protocol is defined over the relay cells; it is implemented
/// in the [crate::relaycell] module.
#[derive(Clone, Educe)]
#[derive(Clone, Educe, derive_more::From)]
#[educe(Debug)]
pub struct Relay {
/// The contents of the relay cell as encoded for transfer.
@ -364,7 +364,7 @@ pub struct Relay {
/// necessary happen. We should refactor our data handling until we're mostly
/// moving around pointers rather than copying data; see ticket #7.
#[educe(Debug(method = "skip_fmt"))]
body: Box<RawCellBody>,
body: BoxedCellBody,
}
impl Relay {
/// Construct a Relay message from a slice containing its contents.
@ -385,11 +385,10 @@ impl Relay {
body: Box::new(body),
}
}
/// Consume this Relay message and return a RelayCellBody for
/// Consume this Relay message and return a BoxedCellBody for
/// encryption/decryption.
pub fn into_relay_body(self) -> RawCellBody {
*self.body
pub fn into_relay_body(self) -> BoxedCellBody {
self.body
}
/// Wrap this Relay message into a RelayMsg as a RELAY_EARLY cell.
pub fn into_early(self) -> AnyChanMsg {
@ -426,13 +425,13 @@ impl Body for RelayEarly {
}
}
impl RelayEarly {
/// Consume this RelayEarly message and return a RelayCellBody for
/// Consume this RelayEarly message and return a BoxedCellBody for
/// encryption/decryption.
//
// (Since this method takes `self` by value, we can't take advantage of
// Deref.)
pub fn into_relay_body(self) -> RawCellBody {
*self.0.body
pub fn into_relay_body(self) -> BoxedCellBody {
self.0.body
}
}

82
crates/tor-cell/src/relaycell.rs
View File

@ -1,7 +1,7 @@
//! Implementation for parsing and encoding relay cells
use crate::chancell::{RawCellBody, CELL_DATA_LEN};
use tor_bytes::{EncodeResult, Error, Result};
use crate::chancell::{BoxedCellBody, CELL_DATA_LEN};
use tor_bytes::{EncodeError, EncodeResult, Error, Result};
use tor_bytes::{Reader, Writer};
use tor_error::internal;
@ -231,54 +231,76 @@ impl<M: RelayMsg> RelayCell<M> {
}
/// Consume this relay message and encode it as a 509-byte padded cell
/// body.
pub fn encode<R: Rng + CryptoRng>(self, rng: &mut R) -> crate::Result<RawCellBody> {
pub fn encode<R: Rng + CryptoRng>(self, rng: &mut R) -> crate::Result<BoxedCellBody> {
/// We skip this much space before adding any random padding to the
/// end of the cell
const MIN_SPACE_BEFORE_PADDING: usize = 4;
// TODO: This implementation is inefficient; it copies too much.
let encoded = self.encode_to_vec()?;
let enc_len = encoded.len();
if enc_len > CELL_DATA_LEN {
return Err(crate::Error::Internal(internal!(
"too many bytes in relay cell"
)));
}
let mut raw = [0_u8; CELL_DATA_LEN];
raw[0..enc_len].copy_from_slice(&encoded);
let (mut body, enc_len) = self.encode_to_cell()?;
debug_assert!(enc_len <= CELL_DATA_LEN);
if enc_len < CELL_DATA_LEN - MIN_SPACE_BEFORE_PADDING {
rng.fill_bytes(&mut raw[enc_len + MIN_SPACE_BEFORE_PADDING..]);
rng.fill_bytes(&mut body[enc_len + MIN_SPACE_BEFORE_PADDING..]);
}
Ok(raw)
Ok(body)
}
/// Consume a relay cell and return its contents, encoded for use
/// in a RELAY or RELAY_EARLY cell
///
/// TODO: not the best interface, as this requires copying into a cell.
fn encode_to_vec(self) -> EncodeResult<Vec<u8>> {
let mut w = Vec::new();
/// in a RELAY or RELAY_EARLY cell.
fn encode_to_cell(self) -> EncodeResult<(BoxedCellBody, usize)> {
// NOTE: This implementation is a bit optimized, since it happens to
// literally every relay cell that we produce.
// TODO -NM: Add a specialized implementation for making a DATA cell from
// a body?
/// Wrap a BoxedCellBody and implement AsMut<[u8]>
struct BodyWrapper(BoxedCellBody);
impl AsMut<[u8]> for BodyWrapper {
fn as_mut(&mut self) -> &mut [u8] {
self.0.as_mut()
}
}
/// The position of the length field within a relay cell.
const LEN_POS: usize = 9;
/// The position of the body a relay cell.
const BODY_POS: usize = 11;
let body = BodyWrapper(Box::new([0_u8; 509]));
let mut w = tor_bytes::SliceWriter::new(body);
w.write_u8(self.msg.cmd().into());
w.write_u16(0); // "Recognized"
w.write_u16(self.streamid.0);
w.write_u32(0); // Digest
let len_pos = w.len();
// (It would be simpler to use NestedWriter at this point, but it uses an internal Vec that we are trying to avoid.)
debug_assert_eq!(
w.offset().expect("Overflowed a cell with just the header!"),
LEN_POS
);
w.write_u16(0); // Length.
let body_pos = w.len();
self.msg.encode_onto(&mut w)?;
assert!(w.len() >= body_pos); // nothing was removed
let payload_len = w.len() - body_pos;
assert!(payload_len <= std::u16::MAX as usize);
*(array_mut_ref![w, len_pos, 2]) = (payload_len as u16).to_be_bytes();
Ok(w)
debug_assert_eq!(
w.offset().expect("Overflowed a cell with just the header!"),
BODY_POS
);
self.msg.encode_onto(&mut w)?; // body
let (mut body, written) = w.try_unwrap().map_err(|_| {
EncodeError::Bug(internal!(
"Encoding of relay message was too long to fit into a cell!"
))
})?;
let payload_len = written - BODY_POS;
debug_assert!(payload_len < std::u16::MAX as usize);
*(array_mut_ref![body.0, LEN_POS, 2]) = (payload_len as u16).to_be_bytes();
Ok((body.0, written))
}
/// Parse a RELAY or RELAY_EARLY cell body into a RelayCell.
///
/// Requires that the cryptographic checks on the message have already been
/// performed
pub fn decode(body: RawCellBody) -> Result<Self> {
#[allow(clippy::needless_pass_by_value)] // TODO this will go away soon.
pub fn decode(body: BoxedCellBody) -> Result<Self> {
let mut reader = Reader::from_slice(body.as_ref());
Self::decode_from_reader(&mut reader)
}

4
crates/tor-cell/tests/test_relaycell.rs
View File

@ -34,7 +34,7 @@ impl rand::RngCore for BadRng {
// I won't tell if you don't.
impl rand::CryptoRng for BadRng {}
fn decode(body: &str) -> [u8; CELL_BODY_LEN] {
fn decode(body: &str) -> Box<[u8; CELL_BODY_LEN]> {
let mut body = body.to_string();
body.retain(|c| !c.is_whitespace());
let mut body = hex::decode(body).unwrap();
@ -42,7 +42,7 @@ fn decode(body: &str) -> [u8; CELL_BODY_LEN] {
let mut result = [0; CELL_BODY_LEN];
result.copy_from_slice(&body[..]);
result
Box::new(result)
}
fn cell(body: &str, id: StreamId, msg: AnyRelayMsg) {

1
crates/tor-proto/Cargo.toml
View File

@ -33,6 +33,7 @@ bytes = "1"
cipher = { version = "0.4.1", features = ["zeroize"] }
coarsetime = "0.1.20"
derive_builder = { version = "0.11.2", package = "derive_builder_fork_arti" }
derive_more = "0.99.3"
digest = "0.10.0"
educe = "0.4.6"
futures = "0.3.14"

9
crates/tor-proto/src/circuit.rs
View File

@ -815,7 +815,7 @@ mod test {
use hex_literal::hex;
use std::time::Duration;
use tor_basic_utils::test_rng::testing_rng;
use tor_cell::chancell::{msg as chanmsg, AnyChanCell};
use tor_cell::chancell::{msg as chanmsg, AnyChanCell, BoxedCellBody};
use tor_cell::relaycell::{msg as relaymsg, AnyRelayCell, StreamId};
use tor_linkspec::OwnedCircTarget;
use tor_rtcompat::{Runtime, SleepProvider};
@ -825,11 +825,10 @@ mod test {
where
ID: Into<StreamId>,
{
let body: RelayCellBody = AnyRelayCell::new(id.into(), msg)
let body: BoxedCellBody = AnyRelayCell::new(id.into(), msg)
.encode(&mut testing_rng())
.unwrap()
.into();
let chanmsg = chanmsg::Relay::from_raw(body.into());
.unwrap();
let chanmsg = chanmsg::Relay::from(body);
ClientCircChanMsg::Relay(chanmsg)
}

4
crates/tor-proto/src/circuit/reactor.rs
View File

@ -34,7 +34,7 @@ use crate::circuit::sendme::StreamSendWindow;
use crate::crypto::handshake::ntor::{NtorClient, NtorPublicKey};
use crate::crypto::handshake::{ClientHandshake, KeyGenerator};
use safelog::sensitive as sv;
use tor_cell::chancell::{self, ChanMsg};
use tor_cell::chancell::{self, BoxedCellBody, ChanMsg};
use tor_cell::chancell::{AnyChanCell, CircId};
use tor_linkspec::{LinkSpec, OwnedChanTarget, RelayIds};
use tor_llcrypto::pk;
@ -1005,7 +1005,7 @@ impl Reactor {
let tag = self.crypto_out.encrypt(&mut body, hop)?;
// NOTE(eta): Now that we've encrypted the cell, we *must* either send it or abort
// the whole circuit (e.g. by returning an error).
let msg = chancell::msg::Relay::from_raw(body.into());
let msg = chancell::msg::Relay::from(BoxedCellBody::from(body));
let msg = if early {
AnyChanMsg::RelayEarly(msg.into())
} else {

26
crates/tor-proto/src/crypto/cell.rs
View File

@ -8,25 +8,15 @@
//!
use crate::{Error, Result};
use tor_cell::chancell::RawCellBody;
use tor_cell::chancell::BoxedCellBody;
use tor_error::internal;
use generic_array::GenericArray;
/// Type for the body of a relay cell.
#[derive(Clone)]
pub(crate) struct RelayCellBody(RawCellBody);
#[derive(Clone, derive_more::From, derive_more::Into)]
pub(crate) struct RelayCellBody(BoxedCellBody);
impl From<RawCellBody> for RelayCellBody {
fn from(body: RawCellBody) -> Self {
RelayCellBody(body)
}
}
impl From<RelayCellBody> for RawCellBody {
fn from(cell: RelayCellBody) -> Self {
cell.0
}
}
impl AsRef<[u8]> for RelayCellBody {
fn as_ref(&self) -> &[u8] {
&self.0[..]
@ -447,7 +437,7 @@ mod test {
let mut rng = testing_rng();
for _ in 1..300 {
// outbound cell
let mut cell = [0_u8; 509];
let mut cell = Box::new([0_u8; 509]);
let mut cell_orig = [0_u8; 509];
rng.fill_bytes(&mut cell_orig);
cell.copy_from_slice(&cell_orig);
@ -461,7 +451,7 @@ mod test {
assert_eq!(&cell.as_ref()[9..], &cell_orig.as_ref()[9..]);
// inbound cell
let mut cell = [0_u8; 509];
let mut cell = Box::new([0_u8; 509]);
let mut cell_orig = [0_u8; 509];
rng.fill_bytes(&mut cell_orig);
cell.copy_from_slice(&cell_orig);
@ -480,14 +470,14 @@ mod test {
// Try a failure: sending a cell to a nonexistent hop.
{
let mut cell = [0_u8; 509].into();
let mut cell = Box::new([0_u8; 509]).into();
let err = cc_out.encrypt(&mut cell, 10.into());
assert!(matches!(err, Err(Error::NoSuchHop)));
}
// Try a failure: A junk cell with no correct auth from any layer.
{
let mut cell = [0_u8; 509].into();
let mut cell = Box::new([0_u8; 509]).into();
let err = cc_in.decrypt(&mut cell);
assert!(matches!(err, Err(Error::BadCellAuth)));
}
@ -527,7 +517,7 @@ mod test {
let mut j = 0;
for cellno in 0..51 {
let mut body = [0_u8; 509];
let mut body = Box::new([0_u8; 509]);
body[0] = 2; // command: data.
body[4] = 1; // streamid: 1.
body[9] = 1; // length: 498