diff --git a/Cargo.lock b/Cargo.lock index 0cff20a23..28e19050b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -250,15 +250,18 @@ dependencies = [ "serde", "serde_json", "thiserror", + "tiny-keccak", "tor-async-utils", "tor-basic-utils", "tor-bytes", "tor-error", + "tor-llcrypto", "tor-rpcbase", "tor-rtcompat", "tracing", "typetag", "weak-table", + "zeroize", ] [[package]] @@ -844,6 +847,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crunchy" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" + [[package]] name = "crypto-common" version = "0.1.6" @@ -3572,6 +3581,15 @@ dependencies = [ "time-core", ] +[[package]] +name = "tiny-keccak" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c9d3793400a45f954c52e73d068316d76b6f4e36977e3fcebb13a2721e80237" +dependencies = [ + "crunchy", +] + [[package]] name = "tinystr" version = "0.7.1" diff --git a/crates/arti-rpcserver/Cargo.toml b/crates/arti-rpcserver/Cargo.toml index 1a37a2a7a..c21b7d76a 100644 --- a/crates/arti-rpcserver/Cargo.toml +++ b/crates/arti-rpcserver/Cargo.toml @@ -13,7 +13,14 @@ repository = "https://gitlab.torproject.org/tpo/core/arti.git/" [features] default = [] -full = ["arti-client/full", "tor-async-utils/full", "tor-error/full", "tor-rpcbase/full", "tor-rtcompat/full", "tor-bytes/full"] +full = [ + "arti-client/full", + "tor-async-utils/full", + "tor-error/full", + "tor-rpcbase/full", + "tor-rtcompat/full", + "tor-bytes/full", +] [dependencies] arti-client = { path = "../arti-client", version = "0.9.1", features = ["rpc"] } @@ -29,14 +36,17 @@ rand = "0.8" serde = { version = "1.0.103", features = ["derive"] } serde_json = "1.0.50" thiserror = "1" +tiny-keccak = { version = "2", features = ["k12"] } tor-async-utils = { path = "../tor-async-utils", version = "0.1.1" } tor-bytes = { path = "../tor-bytes", version = "0.7.1" } tor-error = { path = "../tor-error", version = "0.5.1" } +tor-llcrypto = { path = "../tor-llcrypto", version = "0.5.1" } tor-rpcbase = { path = "../tor-rpcbase", version = "0.1.1" } tor-rtcompat = { path = "../tor-rtcompat", version = "0.9.1" } tracing = "0.1.36" typetag = "0.2.7" weak-table = "0.3.0" +zeroize = "1" [dev-dependencies] futures-await-test = "0.3.0" diff --git a/crates/arti-rpcserver/src/connection.rs b/crates/arti-rpcserver/src/connection.rs index 8da2e7fb1..d4779f0ff 100644 --- a/crates/arti-rpcserver/src/connection.rs +++ b/crates/arti-rpcserver/src/connection.rs @@ -86,9 +86,27 @@ pub(crate) type BoxedResponseSink = Pin + Send>>; /// A random value used to identify an connection. -#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, derive_more::From, derive_more::Into)] +#[derive( + Copy, + Clone, + Debug, + Eq, + PartialEq, + Hash, + derive_more::From, + derive_more::Into, + derive_more::AsRef, +)] + +// TODO RPC: Document this, and make it participate in the Reader/Writer API +// enough that we can stop referring to its internals elsewhere. pub(crate) struct ConnectionId([u8; 16]); +impl ConnectionId { + /// The length of a ConnectionId. + pub(crate) const LEN: usize = 16; +} + impl Connection { /// Create a new connection. pub(crate) fn new( @@ -112,18 +130,23 @@ impl Connection { self: &Arc, id: &rpc::ObjectId, ) -> Result, rpc::LookupError> { - let inner = self.inner.lock().expect("lock poisoned"); - if id.as_ref() == "connection" { Ok(self.clone()) } else { - inner - .objects - .lookup(crate::objmap::GenIdx::try_decode(id)?) + self.lookup_by_idx(crate::objmap::GenIdx::try_decode(id)?) .ok_or(rpc::LookupError::NoObject(id.clone())) } } + /// As `lookup_object`, but expect a `GenIdx`. + pub(crate) fn lookup_by_idx( + self: &Arc, + idx: crate::objmap::GenIdx, + ) -> Option> { + let inner = self.inner.lock().expect("lock poisoned"); + inner.objects.lookup(idx) + } + /// Un-register the request `id` and stop tracking its information. fn remove_request(&self, id: &RequestId) { let mut inner = self.inner.lock().expect("lock poisoned"); diff --git a/crates/arti-rpcserver/src/globalid.rs b/crates/arti-rpcserver/src/globalid.rs new file mode 100644 index 000000000..95739cc43 --- /dev/null +++ b/crates/arti-rpcserver/src/globalid.rs @@ -0,0 +1,226 @@ +//! Manager-global identifiers, for things that need to be identified outside +//! the scope of a single RPC connection. +//! +//! We expect to use this code to identify `TorClient`s and similar objects that +//! can be passed as the target of a SOCKS request. Since the SOCKS request is +//! not part of the RPC session, we need a way for it to refer to these objects. + +#![allow(dead_code)] // XXXXXXX + +use tor_bytes::Reader; +use tor_llcrypto::util::ct::CtByteArray; +use tor_rpcbase::{LookupError, ObjectId}; +use zeroize::Zeroizing; + +use crate::{connection::ConnectionId, objmap::GenIdx}; + +/// A [RpcMgr](crate::RpcMgr)-scoped identifier for an RPC object. +/// +/// A `GlobalId` identifies an RPC object uniquely among all the objects visible +/// to any active session on an RpcMgr. +/// +/// Its encoding is unforgeable. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct GlobalId { + /// The RPC connection within whose object map `local_id` is visible. + pub(crate) connection: ConnectionId, + /// The identifier of the object within `connection`'s object map. + pub(crate) local_id: GenIdx, +} + +/// The number of bytes in our [`MacKey`]. +/// +/// (Our choice of algorithm allows any key length we want; 128 bits should be +/// secure enough.) +const MAC_KEY_LEN: usize = 16; +/// The number of bytes in a [`Mac`]. +/// +/// (Our choice of algorithm allows any MAC length we want; 128 bits should be +/// enough to make the results unforgeable.) +const MAC_LEN: usize = 16; + +/// An key that we use to compute message authentication codes (MACs) for our +/// [`GlobalId`]s +/// +/// We do not guarantee any particular MAC algorithm; we should be able to +/// change MAC algorithms without breaking any user code. Right now, we choose a +/// Kangaroo12-based construction in order to be reasonably fast. +#[derive(Clone)] +pub(crate) struct MacKey { + /// The key itself. + key: Zeroizing<[u8; MAC_KEY_LEN]>, +} + +/// A message authentication code produced by [`MacKey::mac`]. +type Mac = CtByteArray; + +impl MacKey { + /// Construct a new random `MacKey`. + pub(crate) fn new(rng: &mut Rng) -> Self { + Self { + key: Zeroizing::new(rng.gen()), + } + } + + /// Compute the AMC of a given input `inp`, and store the result into `out`. + /// + /// The current construction allows `out` to be any length. + fn mac(&self, inp: &[u8], out: &mut [u8]) { + use tiny_keccak::{Hasher as _, KangarooTwelve as K12}; + // This is the HopMAC construction from draft-irtf-cfrg-kangarootwelve-10: + // + // HopMAC(Key, M, C, L) = K12(Key, K12(M, C, 32), L) + // + // TODO RPC: Just use KMAC or something. + let mut inner = K12::new(b"artirpc globalid"); + let mut hash = [0_u8; 32]; + inner.update(inp); + inner.finalize(&mut hash); + + let mut outer = K12::new(hash); + outer.update(&self.key[..]); + outer.finalize(out); + } +} + +impl GlobalId { + /// The number of bytes used to encode a `GlobalId` in binary form. + const ENCODED_LEN: usize = MAC_LEN + ConnectionId::LEN + GenIdx::BYTE_LEN; + + /// Encode this ID in an unforgeable string that we can later use to + /// uniquely identify an RPC object. + /// + /// As with local IDs, this encoding is nondeterministic. + pub(crate) fn encode(&self, key: &MacKey) -> ObjectId { + use base64ct::{Base64Unpadded as B64, Encoding}; + let bytes = self.encode_as_bytes(key, &mut rand::thread_rng()); + B64::encode_string(&bytes[..]).into() + } + + /// As `encode`, but do not base64-encode the result. + fn encode_as_bytes(&self, key: &MacKey, rng: &mut R) -> Vec { + let mut bytes = Vec::with_capacity(Self::ENCODED_LEN); + bytes.resize(MAC_LEN, 0); + bytes.extend_from_slice(self.connection.as_ref()); + bytes.extend_from_slice(&self.local_id.to_bytes(rng)); + { + // TODO RPC: Maybe we should stick the MAC at the end to make everything simpler. + let (mac, text) = bytes.split_at_mut(MAC_LEN); + key.mac(text, mac); + } + bytes + } + + /// Try to decode and validate `s` as a [`GlobalId`]. + pub(crate) fn try_decode(key: &MacKey, s: &ObjectId) -> Result { + use base64ct::{Base64Unpadded as B64, Encoding}; + let mut bytes = [0_u8; Self::ENCODED_LEN]; + let byte_slice = B64::decode(s.as_ref(), &mut bytes[..]) + .map_err(|_| LookupError::NoObject(s.clone()))?; + Self::try_decode_from_bytes(key, byte_slice).ok_or_else(|| LookupError::NoObject(s.clone())) + } + + /// As `try_decode`, but expect a byte slice rather than a base64-encoded string. + fn try_decode_from_bytes(key: &MacKey, bytes: &[u8]) -> Option { + if bytes.len() != Self::ENCODED_LEN { + return None; + } + + // TODO RPC: Just use Reader here? + + let mut found_mac = [0; MAC_LEN]; + key.mac(&bytes[MAC_LEN..], &mut found_mac[..]); + let found_mac = Mac::from(found_mac); + + let mut r: Reader = Reader::from_slice(bytes); + let declared_mac: Mac = r.extract().ok()?; + if found_mac != declared_mac { + return None; + } + let connection = r.extract::<[u8; ConnectionId::LEN]>().ok()?.into(); + let rest = r.into_rest(); + let local_id = GenIdx::from_bytes(rest)?; + + Some(Self { + connection, + local_id, + }) + } +} + +#[cfg(test)] +mod test { + // @@ begin test lint list maintained by maint/add_warning @@ + #![allow(clippy::bool_assert_comparison)] + #![allow(clippy::clone_on_copy)] + #![allow(clippy::dbg_macro)] + #![allow(clippy::print_stderr)] + #![allow(clippy::print_stdout)] + #![allow(clippy::single_char_pattern)] + #![allow(clippy::unwrap_used)] + #![allow(clippy::unchecked_duration_subtraction)] + //! + + use super::*; + + #[test] + fn roundtrip() { + use generational_arena as ga; + let mut rng = tor_basic_utils::test_rng::testing_rng(); + + let conn1 = ConnectionId::from(*b"example1-------!"); + let conn2 = ConnectionId::from(*b"example2!!!!!!!!"); + let genidx_s1 = GenIdx::Strong(ga::Index::from_raw_parts(42, 42)); + let genidx_w2 = GenIdx::Weak(ga::Index::from_raw_parts(172, 171)); + + let gid1 = GlobalId { + connection: conn1, + local_id: genidx_s1, + }; + let gid2 = GlobalId { + connection: conn2, + local_id: genidx_w2, + }; + + let mac_key = MacKey::new(&mut rng); + let enc1 = gid1.encode(&mac_key); + let gid1_decoded = GlobalId::try_decode(&mac_key, &enc1).unwrap(); + assert_eq!(gid1, gid1_decoded); + + let enc2 = gid2.encode(&mac_key); + let gid2_decoded = GlobalId::try_decode(&mac_key, &enc2).unwrap(); + assert_eq!(gid2, gid2_decoded); + assert_ne!(gid1_decoded, gid2_decoded); + } + + #[test] + fn mac_works() { + use generational_arena as ga; + let mut rng = tor_basic_utils::test_rng::testing_rng(); + + let conn1 = ConnectionId::from(*b"example1-------!"); + let conn2 = ConnectionId::from(*b"example2!!!!!!!!"); + let genidx_s1 = GenIdx::Strong(ga::Index::from_raw_parts(42, 42)); + let genidx_w1 = GenIdx::Weak(ga::Index::from_raw_parts(172, 171)); + + let gid1 = GlobalId { + connection: conn1, + local_id: genidx_s1, + }; + let gid2 = GlobalId { + connection: conn2, + local_id: genidx_w1, + }; + let mac_key = MacKey::new(&mut rng); + let enc1 = gid1.encode_as_bytes(&mac_key, &mut rng); + let enc2 = gid2.encode_as_bytes(&mac_key, &mut rng); + + // Make a 'combined' encoded gid with the mac from one and the info from + // the other. + let mut combined = Vec::from(&enc1[0..MAC_LEN]); + combined.extend_from_slice(&enc2[MAC_LEN..]); + let outcome = GlobalId::try_decode_from_bytes(&mac_key, &combined[..]); + // Can't decode, because MAC was wrong. + assert!(outcome.is_none()); + } +} diff --git a/crates/arti-rpcserver/src/lib.rs b/crates/arti-rpcserver/src/lib.rs index 01c13ae8d..9c7966e0a 100644 --- a/crates/arti-rpcserver/src/lib.rs +++ b/crates/arti-rpcserver/src/lib.rs @@ -40,6 +40,7 @@ mod cancel; mod connection; mod err; +mod globalid; mod mgr; mod msgs; mod objmap; diff --git a/crates/arti-rpcserver/src/mgr.rs b/crates/arti-rpcserver/src/mgr.rs index 71e80c1f0..7b469b880 100644 --- a/crates/arti-rpcserver/src/mgr.rs +++ b/crates/arti-rpcserver/src/mgr.rs @@ -8,7 +8,10 @@ use tor_rpcbase as rpc; use tor_rtcompat::Runtime; use weak_table::WeakValueHashMap; -use crate::connection::{Connection, ConnectionId}; +use crate::{ + connection::{Connection, ConnectionId}, + globalid::{GlobalId, MacKey}, +}; /// Shared state, configuration, and data for all RPC sessions. /// @@ -16,6 +19,11 @@ use crate::connection::{Connection, ConnectionId}; /// /// TODO RPC: Actually not all of the above functionality is implemented yet. But it should be. pub struct RpcMgr { + /// A key that we use to ensure that identifiers are unforgeable. + /// + /// When giving out a global identifier. + mac_key: MacKey, + /// Lock-protected view of the manager's state. // // TODO RPC: We should probably move everything into Inner, and move an Arc @@ -48,6 +56,7 @@ impl RpcMgr { #[allow(clippy::new_without_default)] pub fn new() -> Self { RpcMgr { + mac_key: MacKey::new(&mut rand::thread_rng()), inner: Mutex::new(Inner { dispatch_table: Arc::new(rpc::DispatchTable::from_inventory()), connections: WeakValueHashMap::new(), @@ -81,4 +90,26 @@ impl RpcMgr { ); connection } + + /// Look up an object in the context of this `RpcMgr`. + /// + /// Some object identifiers exist in a manager-global context, so that they + /// can be used outside of a single RPC session. This function looks up an + /// object by such an identifier string. It returns an error if the + /// identifier is invalid or the object does not exist. + pub fn lookup_object( + &self, + id: &rpc::ObjectId, + ) -> Result, rpc::LookupError> { + let global_id = GlobalId::try_decode(&self.mac_key, id)?; + self.lookup_by_global_id(&global_id) + .ok_or_else(|| rpc::LookupError::NoObject(id.clone())) + } + + /// As `lookup_object`, but takes a parsed and validated [`GlobalId`]. + pub(crate) fn lookup_by_global_id(&self, id: &GlobalId) -> Option> { + let inner = self.inner.lock().expect("lock poisoned"); + let connection = inner.connections.get(&id.connection)?; + connection.lookup_by_idx(id.local_id) + } }