RPC: Implement a "global identifier" for non-session-bound IDs

These identifiers are actually only "global" with respect to a given
`RpcMgr`, but they should not be forgeable or reusable across RpcMgr
objects.  We're going to use them so that we have a kind of identifier
for `TorClient`s that we can expose to SOCKS.

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"

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"

crates/arti-rpcserver/src/connection.rs

@@ -86,9 +86,27 @@ pub(crate) type BoxedResponseSink =
     Pin<Box<dyn Sink<BoxedResponse, Error = asynchronous_codec::JsonCodecError> + 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<Self>,
         id: &rpc::ObjectId,
     ) -> Result<Arc<dyn rpc::Object>, 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<Self>,
+        idx: crate::objmap::GenIdx,
+    ) -> Option<Arc<dyn rpc::Object>> {
+        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");

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<MAC_LEN>;
+
+impl MacKey {
+    /// Construct a new random `MacKey`.
+    pub(crate) fn new<Rng: rand::Rng + rand::CryptoRng>(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<R: rand::RngCore>(&self, key: &MacKey, rng: &mut R) -> Vec<u8> {
+        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<Self, LookupError> {
+        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<Self> {
+        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)]
+    //! <!-- @@ end test lint list maintained by maint/add_warning @@ -->
+
+    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());
+    }
+}

crates/arti-rpcserver/src/lib.rs

@@ -40,6 +40,7 @@
 mod cancel;
 mod connection;
 mod err;
+mod globalid;
 mod mgr;
 mod msgs;
 mod objmap;

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<Arc<dyn rpc::Object>, 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<Arc<dyn rpc::Object>> {
+        let inner = self.inner.lock().expect("lock poisoned");
+        let connection = inner.connections.get(&id.connection)?;
+        connection.lookup_by_idx(id.local_id)
+    }
 }