Merge pull request #468 from jagerman/fix-store-sig_ts

Fix store sig_timestamp handling

network-tests/test_store_retrieve.py

@@ -237,3 +237,64 @@ def test_store_retrieve_multiple(omq, random_sn, sk, exclude):
     assert len(r) == 1
     r = json.loads(r[0])
     assert len(r['messages']) == 11
+
+
+def test_store_sig_timestamp(omq, random_sn, sk, exclude):
+    """Tests that sig_timestamp is used properly for the signature both sig_timestamp and timestamp
+    are given."""
+    swarm = ss.get_swarm(omq, random_sn, sk)
+
+    sn = ss.random_swarm_members(swarm, 1, exclude)[0]
+    conn = omq.connect_remote(sn_address(sn))
+
+    ts = int(time.time() * 1000)
+    ns = 123
+    ttl = 86400000
+    exp = ts + ttl
+
+    # Should be fine: timestamp is current, and we sign with it (so timestamp is double double-duty
+    # as both the message timestamp, and the signature timestamp):
+    to_sign = f"store{ns}{ts}".encode()
+    s = omq.request_future(conn, 'storage.store', [json.dumps({
+        "pubkey": '05' + sk.verify_key.encode().hex(),
+        "namespace": ns,
+        "timestamp": ts,
+        "ttl": ttl,
+        "data": base64.b64encode("msg1".encode()).decode(),
+        "signature": sk.sign(to_sign, encoder=Base64Encoder).signature.decode()
+        }).encode()]).get()
+    assert len(s) == 1
+    s = json.loads(s[0])
+
+    assert 'hash' in s
+
+
+    # Simulate a 100s storage delay:
+    ts -= 100_000
+
+    # Fails because timestamp is too old for a store signature:
+    to_sign = f"store{ns}{ts}".encode()
+    s = omq.request_future(conn, 'storage.store', [json.dumps({
+        "pubkey": '05' + sk.verify_key.encode().hex(),
+        "namespace": ns,
+        "timestamp": ts,
+        "ttl": ttl,
+        "data": base64.b64encode("msg2".encode()).decode(),
+        "signature": sk.sign(to_sign, encoder=Base64Encoder).signature.decode()
+        }).encode()]).get()
+    assert s == [b'406', b'store signature timestamp too far from current time']
+
+    # This should work: sig_timestamp is current, timestamp is old:
+    sig_ts = int(time.time() * 1000)
+    to_sign = f"store{ns}{sig_ts}".encode()
+    s = omq.request_future(conn, 'storage.store', [json.dumps({
+        "pubkey": '05' + sk.verify_key.encode().hex(),
+        "namespace": ns,
+        "timestamp": ts,
+        "sig_timestamp": sig_ts,
+        "ttl": ttl,
+        "data": base64.b64encode("msg3".encode()).decode(),
+        "signature": sk.sign(to_sign, encoder=Base64Encoder).signature.decode()
+        }).encode()]).get()
+    assert len(s) == 1
+    s = json.loads(s[0])

oxenss/rpc/request_handler.cpp

@@ -533,14 +533,13 @@ void RequestHandler::process_client_req(rpc::store&& req, std::function<void(Res
             log::warning(logcat, err);
             return cb(Response{http::UNAUTHORIZED, err});
         }
-        if (req.timestamp < now - SIGNATURE_TOLERANCE ||
+        if (*req.sig_ts < now - SIGNATURE_TOLERANCE || *req.sig_ts > now + SIGNATURE_TOLERANCE) {
-            req.timestamp > now + SIGNATURE_TOLERANCE) {
             log::debug(
                     logcat,
-                    "store: invalid timestamp ({}s from now)",
+                    "store: invalid signature timestamp ({}s from now)",
                     duration_cast<seconds>(req.timestamp - now).count());
-            return cb(
+            return cb(Response{
-                    Response{http::NOT_ACCEPTABLE, "store timestamp too far from current time"sv});
+                    http::NOT_ACCEPTABLE, "store signature timestamp too far from current time"sv});
         }
 
         if (!verify_signature(
@@ -551,7 +550,7 @@ void RequestHandler::process_client_req(rpc::store&& req, std::function<void(Res
                     *req.signature,
                     "store",
                     req.msg_namespace == namespace_id::Default ? "" : to_string(req.msg_namespace),
-                    req.timestamp)) {
+                    *req.sig_ts)) {
             log::debug(logcat, "store: signature verification failed");
             return cb(Response{http::UNAUTHORIZED, "store signature verification failed"sv});
         }

oxenss/rpc/request_handler.h

@@ -41,10 +41,10 @@ inline constexpr auto TTL_MAXIMUM_PRIVATE = 30 * 24h;
 // future, and don't allow stores with an expiry in the past by more than this amount.
 inline constexpr auto STORE_TOLERANCE = 10s;
 
-// Tolerance for timestamp-dependent, signed requests (such as `delete_all`); we accept the
+// Tolerance for timestamp-dependent, signed requests (such as storing to a non-public namespace,
-// initial request if within SIGNATURE_TOLERANCE of now, and accept a recursive request if
+// `delete_all`, etc.); we accept the initial request if within SIGNATURE_TOLERANCE of now, and
-// within SIGNATURE_TOLERANCE_FORWARDED (generally slightly larger to account for swarm
+// accept a recursive request if within SIGNATURE_TOLERANCE_FORWARDED (generally slightly larger to
-// forwarding latency).
+// account for swarm forwarding latency).
 inline constexpr auto SIGNATURE_TOLERANCE = 60s;
 inline constexpr auto SIGNATURE_TOLERANCE_FORWARDED = 70s;