From b7f311eeb670a048e8efe504daf6fbba8bd3f38b Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Wed, 15 May 2019 18:55:18 -0700
Subject: [PATCH] makeCLists: change key encoding scheme

refs #45
---
 src/kernel/commsSlots/state/makeCLists.js | 61 ++++++++++++-----------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/src/kernel/commsSlots/state/makeCLists.js b/src/kernel/commsSlots/state/makeCLists.js
index 381b3d40cf2..646eedb651f 100644
--- a/src/kernel/commsSlots/state/makeCLists.js
+++ b/src/kernel/commsSlots/state/makeCLists.js
@@ -11,13 +11,11 @@
 export function makeCLists() {
   const state = new Map();
 
-  function checkIfAlreadyExists(incomingWireMessageObj, kernelToMeSlot) {
-    const slot = state.get(JSON.stringify(incomingWireMessageObj));
-    const outgoing = state.get(JSON.stringify(kernelToMeSlot));
+  function checkIfAlreadyExists(incomingWireMessageKey, kernelToMeKey) {
+    const slot = state.get(incomingWireMessageKey);
+    const outgoing = state.get(kernelToMeKey);
     if (slot || outgoing) {
-      throw new Error(
-        `${JSON.stringify(kernelToMeSlot)} already exists in clist`,
-      );
+      throw new Error(`${kernelToMeKey} already exists in clist`);
     }
   }
 
@@ -42,11 +40,20 @@ export function makeCLists() {
     };
   }
 
-  function createIncomingWireMessageObj(otherMachineName, youToMeSlot) {
-    return {
-      otherMachineName,
-      youToMeSlot, // could be a your-ingress, your-egress
-    };
+  function createIncomingWireMessageKey(otherMachineName, youToMeSlot) {
+    // TODO: we need an encoding scheme that is both stable and
+    // collision-free under adversarial attack. Any encoding scheme that can
+    // be decoded unambiguously is sufficiently collision-free. This first
+    // used JSON.stringify, which is collision-free, but its stability
+    // depends upon the order in which the object keys were added. "djson" (a
+    // library that provides deterministic JSON encoding) would be stable,
+    // but importing it into a SES environment sounds like an adventure. For
+    // now, we use a cheap concatenation that is stable but not
+    // collision-free. However, 'otherMachineName' will generally be a public
+    // key, which won't have hyphens, so the attacker is not likely to be
+    // able to force a collision with other machines, which is the only kind
+    // of collision that could get them unintended access. See #45 for more.
+    return `incoming-${otherMachineName}-${youToMeSlot.type}-${youToMeSlot.id}`;
   }
 
   function createOutgoingWireMessageObj(otherMachineName, meToYouSlot) {
@@ -56,19 +63,23 @@ export function makeCLists() {
     };
   }
 
+  function createKernelToMeKey(kernelToMeSlot) {
+    return `kernel-${kernelToMeSlot.type}-${kernelToMeSlot.id}`;
+  }
+
   // takes youToMeSlot, returns kernelToMeSlot
   function mapIncomingWireMessageToKernelSlot(otherMachineName, youToMeSlot) {
-    return state.get(
-      JSON.stringify(
-        createIncomingWireMessageObj(otherMachineName, youToMeSlot),
-      ),
-    );
+    const key = createIncomingWireMessageKey(otherMachineName, youToMeSlot);
+    if (!state.has(key)) {
+      console.log(`unable to find key ${key}`);
+    }
+    return state.get(key);
   }
 
   // takes kernelToMeSlot, returns meToYouSlot and machineName
   // we don't know the otherMachineName
   function mapKernelSlotToOutgoingWireMessage(kernelToMeSlot) {
-    return state.get(JSON.stringify(kernelToMeSlot));
+    return state.get(createKernelToMeKey(kernelToMeSlot));
   }
 
   // kernelToMeSlot can have type: import, export or promise
@@ -83,7 +94,7 @@ export function makeCLists() {
   // 2) to translate something that we get over the wire (youToMeSlot)
   //    into a kernelToMeSlot.
   function add(otherMachineName, kernelToMeSlot, youToMeSlot, meToYouSlot) {
-    const incomingWireMessageObj = createIncomingWireMessageObj(
+    const incomingWireMessageKey = createIncomingWireMessageKey(
       otherMachineName,
       youToMeSlot,
     );
@@ -91,16 +102,10 @@ export function makeCLists() {
       otherMachineName,
       meToYouSlot,
     );
-    checkIfAlreadyExists(
-      incomingWireMessageObj,
-      outgoingWireMessageObj,
-      kernelToMeSlot,
-    );
-    // TODO: serialize these more stably, since JSON will depend on the order
-    // in which the properties were added. Maybe `${type}-${id}` or
-    // djson.stringify.
-    state.set(JSON.stringify(kernelToMeSlot), outgoingWireMessageObj);
-    state.set(JSON.stringify(incomingWireMessageObj), kernelToMeSlot);
+    const kernelToMeKey = createKernelToMeKey(kernelToMeSlot);
+    checkIfAlreadyExists(incomingWireMessageKey, kernelToMeKey);
+    state.set(kernelToMeKey, outgoingWireMessageObj);
+    state.set(incomingWireMessageKey, kernelToMeSlot);
   }
 
   return {