Server 2.0.0-alpha.5: security fix, a signature with every wrapped vault key

README.md

@@ -25,8 +25,8 @@ Standard library only: no `pip install`, no virtualenv, nothing to keep
 patched beyond Python itself.
 
 **Which apps it serves.** SecretSpace for Windows 2.x and SecretSpace for Linux
-0.x sync with it. The SecretSpace 3 test versions (Windows 3.0.0-alpha.7, Linux
-1.0.0-alpha.7) sync with the newest test version of the server, 2.0.0-alpha.4,
+0.x sync with it. The SecretSpace 3 test versions (Windows 3.0.0-alpha.8, Linux
+1.0.0-alpha.8) sync with the newest test version of the server, 2.0.0-alpha.5,
 through the accounts, sharing and recovery described under API v2 below.
 
 This repository is the server for **Linux**, run by systemd. The other
@@ -539,6 +539,7 @@ like a 2.x one, with the same revision, `If-Match` and history rules.
 | `/v2/vaults/<id>/members/<user>` | DELETE | session, yourself | leave the vault. **409** for its only owner. Anyone else is removed with `rotate` (an owner who tries this gets **400**) |
 | `/v2/vaults/<id>/rotate` | POST | session, owner | change the vault's key, and remove members in the same call: `{generation, remove, wrapped_keys, name, blob, revision}` (see below) |
 | `/v2/vaults/<id>/log` | GET | session, owner | the vault's log: `{lines}`, oldest first, the newest 200 |
+| `/v2/vaults/<id>/key-sig` | PUT | session, member | sign the wrapped key you already hold: `{key_sig}`, 64 bytes in base64. Only while that key has no signature (**409** otherwise) |
 
 Sessions are sent as `Authorization: Bearer <session>`. The server's token
 is not a session and opens no account; a session opens no 2.x vault. A vault
@@ -603,6 +604,31 @@ succeeds and the line is missing. A name with a space in it (`ann lee`) works
 in paths when it is sent percent-encoded (`ann%20lee`). `/health` now lists
 `"features": ["vaults", "sharing", "recovery"]`.
 
+Server **2.0.0-alpha.5** keeps a **signature with every wrapped vault key**.
+It goes with the Windows 3.0.0-alpha.8 and Linux 1.0.0-alpha.8 test versions,
+which sync only the Personal vault with an older server.
+
+- *Why.* The server stores each member's copy of a vault key but must never
+  be able to choose one. Before this version a dishonest server (or someone
+  who broke into it) could wrap a key of its own to a member's public key and
+  call it a new key generation; the device would then upload the vault under
+  that key, and the server could read it.
+- *How.* The account that wraps a key signs it (Ed25519): which vault, which
+  key generation, for whom, the wrapped key itself. `key_sig` travels with
+  `wrapped_key` in `POST /v2/vaults`, in `PUT /v2/vaults/<id>/members/<user>`
+  for a new member, and as `key_sigs` (one per member who stays) in `rotate`.
+  `/v2/me` returns `key_sig` and `key_signer` with each vault.
+- *Who checks.* The devices, not the server: it has no key to check with and
+  needs none. A device takes a key only when it is signed by its own account
+  or by someone it has reason to trust, and never one of an older generation.
+- `key_signer` is always the account whose session sent the key, never a
+  name from the request.
+- A vault from an earlier version has unsigned keys. A device that already
+  holds such a key signs it at its next sync (`PUT /v2/vaults/<id>/key-sig`),
+  after which the account's other devices take it. A signature made by an
+  owner is never replaced this way.
+- `/health` lists `"signed-keys"`.
+
 Server **2.0.0-alpha.4** lets an account be **recovered with its 12-word
 phrase** when the master password is forgotten. It goes with the Windows
 3.0.0-alpha.7 and Linux 1.0.0-alpha.7 test versions.

server/passman_server.py

@@ -260,6 +260,21 @@ def _archive(self, user: str, revision: int) -> None:
 #: version, a one-off X25519 public key, a nonce, the key and its tag.
 WRAPPED_KEY_LEN = 1 + 32 + 12 + 32 + 16
 
+#: With each wrapped key comes a signature by the account that wrapped it
+#: (Ed25519, 64 bytes). A device accepts a vault key only with a signature
+#: from itself or from someone it knows -- so a dishonest server cannot hand
+#: out a key of its own making and read what is then uploaded under it. The
+#: server only keeps and returns the signature; the devices check it.
+KEY_SIG_LEN = 64
+
+
+def _key_sig(value):
+    """An optional key signature as stored ("" when none came)."""
+    if value is None or value == "":
+        return ""
+    return _b64(_unb64(value, "The key's signature", KEY_SIG_LEN))
+
+
 #: A vault's name arrives sealed under the vault's key, so the server cannot
 #: read it: a nonce, the name (up to 200 bytes) and a tag.
 MIN_SEALED_NAME = 12 + 1 + 16
@@ -612,6 +627,8 @@ def _vault_view(self, username: str, vault_id: str):
         if view["kind"] != "personal":
             view["name"] = members.get("name", "")
             view["wrapped_key"] = member.get("wrapped_key", "")
+            view["key_sig"] = member.get("key_sig", "")
+            view["key_signer"] = member.get("key_signer", "")
         return view
 
     def me(self, username: str) -> dict:
@@ -805,6 +822,7 @@ def create_vault(self, username: str, body: dict) -> dict:
         name = self._sealed_name(body)
         wrapped = _b64(_unb64(body.get("wrapped_key"), "The vault key",
                               WRAPPED_KEY_LEN))
+        key_sig = _key_sig(body.get("key_sig"))
         with _lock:
             record = self.account(username)
             mine = list(record.get("vaults", []))
@@ -819,7 +837,9 @@ def create_vault(self, username: str, body: dict) -> dict:
             self._write_json(self._members_path(vault_id), {
                 "kind": "shared", "created_at": utc_now(),
                 "created_by": username, "name": name, "key_generation": 0,
-                "members": {username: {"role": "owner", "wrapped_key": wrapped}}})
+                "members": {username: {"role": "owner", "wrapped_key": wrapped,
+                                       "key_sig": key_sig,
+                                       "key_signer": username}}})
             record["vaults"] = mine + [vault_id]
             self._write_json(self._account_path(username), record)
         return self._vault_view(username, vault_id)
@@ -911,13 +931,15 @@ def put_member(self, actor: str, vault_id: str, name: str, body: dict) -> dict:
                 return {"username": who, "role": role, "added": False}
             wrapped = _b64(_unb64(body.get("wrapped_key"), "The vault key",
                                   WRAPPED_KEY_LEN))
+            key_sig = _key_sig(body.get("key_sig"))
             if len(people) >= MAX_MEMBERS_PER_VAULT:
                 raise V2Error(403, f"A vault can have {MAX_MEMBERS_PER_VAULT} "
                                    "members.")
             listed = list(target.get("vaults", []))
             if len(listed) >= MAX_VAULTS_LISTED:
                 raise V2Error(409, f"“{who}” is in too many vaults already.")
             people[who] = {"role": role, "wrapped_key": wrapped,
+                           "key_sig": key_sig, "key_signer": actor,
                            "added_by": actor, "added_at": utc_now()}
             self._write_json(self._members_path(vault_id), members)
             target["vaults"] = listed + [vault_id]
@@ -979,6 +1001,12 @@ def rotate(self, actor: str, vault_id: str, body: dict) -> dict:
                                    "meantime. Sync, then try again.")
             clean = {name: _b64(_unb64(wrapped[name], "A vault key",
                                        WRAPPED_KEY_LEN)) for name in staying}
+            sigs = body.get("key_sigs") or {}
+            if not isinstance(sigs, dict) or \
+                    (sigs and sorted(sigs) != sorted(staying)):
+                raise V2Error(400, "The keys' signatures do not match the "
+                                   "members who stay.")
+            sigs = {name: _key_sig(sigs.get(name)) for name in staying}
             name = self._sealed_name(body)
 
             current = int(self.vaults.meta(vault_id)["revision"])
@@ -1003,13 +1031,33 @@ def rotate(self, actor: str, vault_id: str, body: dict) -> dict:
                 del people[gone]
             for each in staying:
                 people[each]["wrapped_key"] = clean[each]
+                people[each]["key_sig"] = sigs[each]
+                people[each]["key_signer"] = actor
             members["key_generation"] = generation
             members["name"] = name
             self._write_json(self._members_path(vault_id), members)
             for gone in removed:
                 self._unlist(gone, vault_id)
         return self._vault_view(actor, vault_id)
 
+    def set_key_sig(self, username: str, vault_id: str, body: dict) -> None:
+        """
+        A member vouches for the wrapped key it already holds, for a vault
+        made before keys were signed -- so its other devices will accept
+        that key. Only while that key has no signature: one made by an
+        owner is never replaced.
+        """
+        sig = _key_sig(body.get("key_sig"))
+        if not sig:
+            raise V2Error(400, "The key's signature is missing.")
+        with _lock:
+            members = self._shared(vault_id)
+            entry = members.get("members", {}).get(username)
+            if entry.get("key_sig"):
+                raise V2Error(409, "This key is signed already.")
+            entry["key_sig"], entry["key_signer"] = sig, username
+            self._write_json(self._members_path(vault_id), members)
+
     def key_generation(self, vault_id: str) -> int:
         members = self._read_json(self._members_path(vault_id)) or {}
         return int(members.get("key_generation", 0))
@@ -1101,11 +1149,12 @@ def do_GET(self):
             # per account (2.0.0-alpha.2).
             # "sharing" = members, roles, key change and the audit log
             # (2.0.0-alpha.3). "recovery" = an account recovered with its
-            # 12-word phrase (2.0.0-alpha.4).
+            # 12-word phrase (2.0.0-alpha.4). "signed-keys" = a signature is
+            # kept with every wrapped vault key (2.0.0-alpha.5).
             return self._send(200, {"ok": True, "service": "passman-sync",
                                     "api": [1, 2],
                                     "features": ["vaults", "sharing",
-                                                 "recovery"]})
+                                                 "recovery", "signed-keys"]})
         if self._is_v2():
             return self._v2("GET")
 
@@ -1385,7 +1434,7 @@ def _v2_route(self, method, parts):
             # Not a member and no such vault answer alike, so a guess at an
             # id learns nothing.
             if role is None or tail not in ("", "meta", "name", "members",
-                                            "rotate", "log"):
+                                            "rotate", "log", "key-sig"):
                 return self._error(404, "No such vault.")
             if len(parts) == 4 and tail != "members":
                 return self._error(404, "No such endpoint.")
@@ -1479,9 +1528,15 @@ def log(accounts, vault_id, actor, address):
             return {"run": leave, "anyone": True}
         if (method, tail, count) == ("POST", "rotate", 3):
             return {"run": body_then(rotate), "anyone": False}
+        def key_sig(accounts, vault_id, actor, address, body):
+            accounts.set_key_sig(actor, vault_id, body)
+            return self._send(200, {})
+
         if (method, tail, count) == ("GET", "log", 3):
             return {"run": log, "anyone": False}
-        if tail in ("name", "rotate", "log") or method == "DELETE" \
+        if (method, tail, count) == ("PUT", "key-sig", 3):
+            return {"run": body_then(key_sig), "anyone": True}
+        if tail in ("name", "rotate", "log", "key-sig") or method == "DELETE" \
                 or (tail == "members" and (count == 4 or method != "GET")):
             return {"run": lambda *a: self._error(404, "No such endpoint."),
                     "anyone": True}

server/version.py

@@ -7,4 +7,4 @@
 in SecretSpace-Server-Windows, which has its own version.
 """
 
-__version__ = "2.0.0-alpha.4"
+__version__ = "2.0.0-alpha.5"

tests/test_server_http.py

@@ -108,7 +108,7 @@ def test_health_answers_without_a_token(server):
     status, _, body = server.request("GET", "/health", token=None)
     assert status == 200
     assert body == {"ok": True, "service": "passman-sync", "api": [1, 2],
-                    "features": ["vaults", "sharing", "recovery"]}
+                    "features": ["vaults", "sharing", "recovery", "signed-keys"]}
 
 
 @pytest.mark.parametrize("header", [