RPC: Give out and accept GlobalIds for appropriate objects.

crates/arti-rpcserver/src/connection.rs

@@ -22,8 +22,9 @@ use tor_async_utils::SinkExt as _;
 use crate::{
     cancel::{Cancel, CancelHandle},
     err::RequestParseError,
+    globalid::{GlobalId, MacKey},
     msgs::{BoxedResponse, FlexibleRequest, Request, RequestId, ResponseBody},
-    objmap::ObjMap,
+    objmap::{GenIdx, ObjMap},
 };
 
 use tor_rpcbase as rpc;
@@ -48,6 +49,10 @@ pub struct Connection {
     /// TODO RPC: Remove this if it turns out to be unneeded.
     #[allow(unused)]
     connection_id: ConnectionId,
+
+    /// A `MacKey` used to create `GlobalIds` for the objects whose identifiers
+    /// need to exist outside this connection.
+    global_id_mac_key: MacKey,
 }
 impl rpc::Object for Connection {}
 rpc::decl_object! {Connection}
@@ -112,6 +117,7 @@ impl Connection {
     pub(crate) fn new(
         connection_id: ConnectionId,
         dispatch_table: Arc<rpc::DispatchTable>,
+        global_id_mac_key: MacKey,
         client: Arc<dyn rpc::Object>,
     ) -> Self {
         Self {
@@ -122,6 +128,42 @@ impl Connection {
             }),
             dispatch_table,
             connection_id,
+            global_id_mac_key,
+        }
+    }
+
+    /// If possible, convert an `ObjectId` into a `GenIdx` that can be used in
+    /// this connection's ObjMap.
+    fn id_into_local_idx(&self, id: &rpc::ObjectId) -> Result<GenIdx, rpc::LookupError> {
+        // TODO RPC: Use a tag byte instead of a magic length.
+
+        if id.as_ref().len() == GlobalId::B64_ENCODED_LEN {
+            // This is the right length to be a GlobalId; let's see if it really
+            // is one.
+            //
+            // Design note: It's not really necessary from a security POV to
+            // check the MAC here; any possible GenIdx we return will either
+            // refer to some object we're allowed to name in this session, or to
+            // no object at all.  Still, we check anyway, since it shouldn't
+            // hurt to do so.
+            let global_id = GlobalId::try_decode(&self.global_id_mac_key, id)?;
+            // We have a GlobalId with a valid MAC. Let's make sure it applies
+            // to this connection's ObjMap.  (We do not support referring to
+            // anyone else's objects.)
+            //
+            // Design note: As above, this check is a protection against
+            // accidental misuse, not a security feature: even if we removed
+            // this check, we would still only allow objects that this session
+            // is allowed to name.
+            if global_id.connection == self.connection_id {
+                Ok(global_id.local_id)
+            } else {
+                Err(rpc::LookupError::NoObject(id.clone()))
+            }
+        } else {
+            // It's not a GlobalId; let's see if we can make sense of it as an
+            // ObjMap index.
+            Ok(GenIdx::try_decode(id)?)
         }
     }
 
@@ -133,7 +175,9 @@ impl Connection {
         if id.as_ref() == "connection" {
             Ok(self.clone())
         } else {
-            self.lookup_by_idx(crate::objmap::GenIdx::try_decode(id)?)
+            let local_id = self.id_into_local_idx(id)?;
+
+            self.lookup_by_idx(local_id)
                 .ok_or(rpc::LookupError::NoObject(id.clone()))
         }
     }
@@ -397,27 +441,45 @@ impl rpc::Context for RequestContext {
     }
 
     fn register_owned(&self, object: Arc<dyn rpc::Object>) -> rpc::ObjectId {
-        self.conn
+        let use_global_id = object.expose_outside_of_session();
+        let local_id = self
+            .conn
             .inner
             .lock()
             .expect("Lock poisoned")
             .objects
-            .insert_strong(object)
-            .encode()
+            .insert_strong(object);
+
+        // Design note: It is a deliberate decision to _always_ use GlobalId for
+        // objects whose IDs are _ever_ exported for use in SOCKS requests.  Some
+        // alternatives would be to use GlobalId conditionally, or to have a
+        // separate Method to create a new GlobalId given an existing LocalId.
+        if use_global_id {
+            GlobalId::new(self.conn.connection_id, local_id).encode(&self.conn.global_id_mac_key)
+        } else {
+            local_id.encode()
+        }
     }
 
     fn register_weak(&self, object: Arc<dyn rpc::Object>) -> rpc::ObjectId {
-        self.conn
+        let use_global_id = object.expose_outside_of_session();
+        let local_id = self
+            .conn
             .inner
             .lock()
             .expect("Lock poisoned")
             .objects
-            .insert_weak(object)
-            .encode()
+            .insert_weak(object);
+        if use_global_id {
+            GlobalId::new(self.conn.connection_id, local_id).encode(&self.conn.global_id_mac_key)
+        } else {
+            local_id.encode()
+        }
     }
 
     fn release_owned(&self, id: &rpc::ObjectId) -> Result<(), rpc::LookupError> {
-        let idx = crate::objmap::GenIdx::try_decode(id)?;
+        let idx = self.conn.id_into_local_idx(id)?;
+
         if !idx.is_strong() {
             return Err(rpc::LookupError::WrongType(id.clone()));
         }

crates/arti-rpcserver/src/globalid.rs

@@ -5,8 +5,6 @@
 //! 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};
@@ -86,6 +84,17 @@ impl MacKey {
 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;
+    /// The number of bytes used to encode a `GlobalId` in base-64 form.
+    // TODO: use div_ceil once it's stable.
+    pub(crate) const B64_ENCODED_LEN: usize = (Self::ENCODED_LEN * 8 + 5) / 6;
+
+    /// Create a new GlobalId from its parts.
+    pub(crate) fn new(connection: ConnectionId, local_id: GenIdx) -> GlobalId {
+        GlobalId {
+            connection,
+            local_id,
+        }
+    }
 
     /// Encode this ID in an unforgeable string that we can later use to
     /// uniquely identify an RPC object.
@@ -191,6 +200,9 @@ mod test {
         let gid2_decoded = GlobalId::try_decode(&mac_key, &enc2).unwrap();
         assert_eq!(gid2, gid2_decoded);
         assert_ne!(gid1_decoded, gid2_decoded);
+
+        assert_eq!(enc1.as_ref().len(), GlobalId::B64_ENCODED_LEN);
+        assert_eq!(enc2.as_ref().len(), GlobalId::B64_ENCODED_LEN);
     }
 
     #[test]

crates/arti-rpcserver/src/mgr.rs

@@ -78,6 +78,7 @@ impl RpcMgr {
         let connection = Arc::new(Connection::new(
             connection_id,
             inner.dispatch_table.clone(),
+            self.mac_key.clone(),
             client_obj,
         ));
         let old = inner.connections.insert(connection_id, connection.clone());