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

tor-proto: only parse allowed ChanMsg types during handshake.

This commit is contained in:
Nick Mathewson 2023-02-08 09:15:55 -05:00
parent 1c1dec0948
commit b49bd3b121
3 changed files with 100 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/tor-proto/src/channel.rs
View File

@ -75,6 +75,7 @@ use safelog::sensitive as sv;
use std::pin::Pin;
use std::sync::{Mutex, MutexGuard};
use std::time::Duration;
use tor_cell::chancell::msg::AnyChanMsg;
use tor_cell::chancell::ChanMsg;
use tor_cell::chancell::{msg, msg::PaddingNegotiate, AnyChanCell, CircId};
use tor_error::internal;
@ -116,7 +117,8 @@ pub use handshake::{OutboundClientHandshake, UnverifiedChannel, VerifiedChannel}
/// Type alias: A Sink and Stream that transforms a TLS connection into
/// a cell-based communication mechanism.
type CellFrame<T> = futures_codec::Framed<T, crate::channel::codec::ChannelCodec>;
type CellFrame<T> =
futures_codec::Framed<T, crate::channel::codec::ChannelCodec<AnyChanMsg, AnyChanMsg>>;
/// An open client channel, ready to send and receive Tor cells.
///

85
crates/tor-proto/src/channel/codec.rs
View File

@ -1,8 +1,9 @@
//! Wrap tor_cell::...:::ChannelCodec for use with the futures_codec
//! crate.
use std::io::Error as IoError;
use std::{io::Error as IoError, marker::PhantomData};
use tor_cell::chancell::{codec, AnyChanCell};
use futures::{AsyncRead, AsyncWrite};
use tor_cell::chancell::{codec, ChanCell, ChanMsg};
use asynchronous_codec as futures_codec;
use bytes::BytesMut;
@ -31,36 +32,89 @@ pub(crate) enum CodecError {
/// for use with futures_codec.
///
/// This type lets us wrap a TLS channel (or some other secure
/// AsyncRead+AsyncWrite type) as a Sink and a Stream of ChanCell, so we
/// can forget about byte-oriented communication.
pub(crate) struct ChannelCodec(codec::ChannelCodec);
/// AsyncRead+AsyncWrite type) as a Sink and a Stream of ChanCell, so we can
/// forget about byte-oriented communication.
///
/// It's parameterized on two message types: one that we're allowed to receive
/// (`IN`), and one that we're allowed to send (`OUT`).
pub(crate) struct ChannelCodec<IN, OUT> {
/// The cell codec that we'll use to encode and decode our cells.
inner: codec::ChannelCodec,
/// Tells the compiler that we're using IN, and we might
/// consume values of type IN.
_phantom_in: PhantomData<fn(IN)>,
/// Tells the compiler that we're using OUT, and we might
/// produce values of type OUT.
_phantom_out: PhantomData<fn() -> OUT>,
}
impl ChannelCodec {
impl<IN, OUT> ChannelCodec<IN, OUT> {
/// Create a new ChannelCodec with a given link protocol.
pub(crate) fn new(link_proto: u16) -> Self {
ChannelCodec(codec::ChannelCodec::new(link_proto))
ChannelCodec {
inner: codec::ChannelCodec::new(link_proto),
_phantom_in: PhantomData,
_phantom_out: PhantomData,
}
}
/// Consume this codec, and return a new one that sends and receives
/// different message types.
pub(crate) fn change_message_types<IN2, OUT2>(self) -> ChannelCodec<IN2, OUT2> {
ChannelCodec {
inner: self.inner,
_phantom_in: PhantomData,
_phantom_out: PhantomData,
}
}
}
impl futures_codec::Encoder for ChannelCodec {
type Item = AnyChanCell;
impl<IN, OUT> futures_codec::Encoder for ChannelCodec<IN, OUT>
where
OUT: ChanMsg,
{
type Item = ChanCell<OUT>;
type Error = CodecError;
fn encode(&mut self, item: Self::Item, dst: &mut BytesMut) -> Result<(), Self::Error> {
self.0.write_cell(item, dst).map_err(CodecError::EncCell)?;
self.inner
.write_cell(item, dst)
.map_err(CodecError::EncCell)?;
Ok(())
}
}
impl futures_codec::Decoder for ChannelCodec {
type Item = AnyChanCell;
impl<IN, OUT> futures_codec::Decoder for ChannelCodec<IN, OUT>
where
IN: ChanMsg,
{
type Item = ChanCell<IN>;
type Error = CodecError;
fn decode(&mut self, src: &mut BytesMut) -> Result<Option<Self::Item>, Self::Error> {
self.0.decode_cell(src).map_err(CodecError::DecCell)
self.inner.decode_cell(src).map_err(CodecError::DecCell)
}
}
/// Consume a [`Framed`](futures_codec::Framed) codec user, and produce one that
/// sends and receives different message types.
pub(crate) fn change_message_types<T, IN, OUT, IN2, OUT2>(
framed: futures_codec::Framed<T, ChannelCodec<IN, OUT>>,
) -> futures_codec::Framed<T, ChannelCodec<IN2, OUT2>>
where
T: AsyncRead + AsyncWrite,
IN: ChanMsg,
OUT: ChanMsg,
IN2: ChanMsg,
OUT2: ChanMsg,
{
futures_codec::Framed::from_parts(
framed
.into_parts()
.map_codec(ChannelCodec::change_message_types),
)
}
#[cfg(test)]
pub(crate) mod test {
#![allow(clippy::unwrap_used)]
@ -70,6 +124,7 @@ pub(crate) mod test {
use futures::task::{Context, Poll};
use hex_literal::hex;
use std::pin::Pin;
use tor_cell::chancell::msg::AnyChanMsg;
use super::{futures_codec, ChannelCodec};
use tor_cell::chancell::{msg, AnyChanCell, ChanCmd, ChanMsg, CircId};
@ -128,7 +183,9 @@ pub(crate) mod test {
}
}
fn frame_buf(mbuf: MsgBuf) -> futures_codec::Framed<MsgBuf, ChannelCodec> {
fn frame_buf(
mbuf: MsgBuf,
) -> futures_codec::Framed<MsgBuf, ChannelCodec<AnyChanMsg, AnyChanMsg>> {
futures_codec::Framed::new(mbuf, ChannelCodec::new(4))
}

40
crates/tor-proto/src/channel/handshake.rs
View File

@ -5,9 +5,10 @@ use asynchronous_codec as futures_codec;
use futures::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use futures::sink::SinkExt;
use futures::stream::StreamExt;
use tor_cell::restricted_msg;
use tor_error::internal;
use crate::channel::codec::{ChannelCodec, CodecError};
use crate::channel::codec::{self, ChannelCodec, CodecError};
use crate::channel::UniqId;
use crate::util::skew::ClockSkew;
use crate::{Error, Result};
@ -106,6 +107,26 @@ pub struct VerifiedChannel<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S
clock_skew: ClockSkew,
}
restricted_msg! {
/// A restricted subset of ChanMsg that can arrive during a handshake.
///
/// (These are messages that come after the VERSIONS cell, up to and
/// including the NETINFO.)
///
/// Note that unrecognized message types (ones not yet implemented in Arti)
/// cause an error, rather than getting ignored. That's intentional: if we
/// start to allow them in the future, we should negotiate a new Channel
/// protocol for the VERSIONS cell.
#[derive(Clone,Debug)]
enum HandshakeMsg : ChanMsg {
Padding,
Vpadding,
AuthChallenge,
Certs,
Netinfo
}
}
/// Convert a CodecError to an Error, under the context that it occurs while
/// doing a channel handshake.
fn codec_err_to_handshake(err: CodecError) -> Error {
@ -210,7 +231,7 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider>
// Now we can switch to using a "Framed". We can ignore the
// AsyncRead/AsyncWrite aspects of the tls, and just treat it
// as a stream and a sink for cells.
let codec = ChannelCodec::new(link_protocol);
let codec = ChannelCodec::<HandshakeMsg, HandshakeMsg>::new(link_protocol);
let mut tls = futures_codec::Framed::new(self.tls, codec);
// Read until we have the netinfo cells.
@ -221,14 +242,12 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider>
// Loop: reject duplicate and unexpected cells
trace!("{}: waiting for rest of handshake.", self.unique_id);
while let Some(m) = tls.next().await {
use msg::AnyChanMsg::*;
use HandshakeMsg::*;
let (_, m) = m.map_err(codec_err_to_handshake)?.into_circid_and_msg();
trace!("{}: received a {} cell.", self.unique_id, m.cmd());
match m {
// Are these technically allowed?
Padding(_) | Vpadding(_) => (),
// Unrecognized cells get ignored.
Unrecognized(_) => (),
// Clients don't care about AuthChallenge
AuthChallenge(_) => {
if seen_authchallenge {
@ -253,13 +272,6 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider>
netinfo = Some((n, coarsetime::Instant::now()));
break;
}
// No other cell types are allowed.
m => {
return Err(Error::HandshakeProto(format!(
"Unexpected cell type {}",
m.cmd()
)))
}
}
}
@ -285,7 +297,7 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider>
};
Ok(UnverifiedChannel {
link_protocol,
tls,
tls: codec::change_message_types(tls),
certs_cell,
netinfo_cell,
clock_skew,
@ -806,7 +818,7 @@ pub(super) mod test {
assert!(matches!(err, Error::HandshakeProto(_)));
assert_eq!(
format!("{}", err),
"Handshake protocol violation: Unexpected cell type CREATE"
"Handshake protocol violation: Invalid cell on handshake: Error while parsing channel cell"
);
});
}