Mandate that the new device commits to a particular identity key

proposals/4108-oidc-qr-login.md

@@ -787,8 +787,44 @@ At this point the new device knows that, subject to the user consenting, it shou
 
 3. **New device informs existing device that it wants to use the `device_authorization_grant`**
 
-The new device send the `verification_uri` and, if present, the `verification_uri_complete` over to the existing device and
-indicates that want to use protocol `device_authorization_grant` along with the `device_id` that will be used:
+At this point, the new device should ensure it has generated its Olm account, so that it has its Curve25519 and Ed25519
+device identity keys.
+
+It then sends a `m.login.protocol` message to the existing device, containing:
+
+- An indicator that it wants to use protocol `device_authorization_grant`
+- The `verification_uri`
+- The `verification_uri_complete`, if present
+- The device ID it will be using, which MUST equal the unpadded base64-encoded form of the public part of the Curve25519
+identity key that the new device has generated
+- A proof of ownership of the device ID, which is a base64-encoded form of the proof described below
+
+The new device proves it controls the Curve25519 key - with public part `Ip` and private part `Is` - by doing an ECDH
+between the private part of the Curve25519 identity key and the other device's secure channel ephemeral public key (`Ep`
+) to derive a shared secret.  `Ep` equates to either `Gp` or `Sp` from the secure channel set up depending on which
+device did the QR code scanning. This derived secret is then used to to construct a proof of ownership based on
+HMAC-SHA256. Due to the properties of ECDH, the existing device knows that the new device can only do this if it
+possesses the private part of the Curve25519 identity key.
+
+By requiring the device ID to equal the device identity key, we reduce the number of (unnecessarily) free parameters,
+allowing a user's E2EE devices to be uniquely identified only by their identity key, rather than by a (device ID,
+identity key) 2-tuple. This paves the way for potentially making this a strict requirement for all E2EE-supporting
+devices in a future iteration of the Matrix E2EE protocol. This would provide a marked increase in protocol robustness
+and reduces potential for implementation errors.
+
+Separately, the proof of ownership of the identity key ensures that the new device cannot submit a key it does not
+control, either by accident or maliciously. While this scenario doesn't represent an outright security
+compromise---because a device cannot decrypt traffic for an identity key it does not control---it further reduces the
+margin for implementation error.
+
+To calculate the proof the new device does:
+
+```
+SH := ECDH(Is, Ep)
+ProofKey := HKDF_SHA256(SH, "MATRIX_QR_CODE_LOGIN_PROOFKEY|" || Ip || "|" || Ep, salt=0, size=32)
+ProofBytes := HMAC_SHA256(ProofKey, "MATRIX_QR_CODE_PROOF_OF_POSSESSION")
+Proof := UnpaddedBase64Encode(ProofBytes)
+```
 
 *New device => Existing device via secure channel*
 
@@ -800,7 +836,8 @@ indicates that want to use protocol `device_authorization_grant` along with the
         "verification_uri": "https://auth-oidc.lab.element.dev/link",
         "verification_uri_complete": "https://auth-oidc.lab.element.dev/link?code=123456"
     },
-    "device_id": "ABCDEFGH"
+    "device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI",
+    "device_id_proof": "$base64_encoded_proof_of_identity_key_ownership"
 }
 ```
 
@@ -847,7 +884,7 @@ sequenceDiagram
         N->>+OP: POST /auth/device client_id=xyz&scope=openid+urn:matrix:api:*+urn:matrix:device:ABCDEFGH...
         OP->>-N: 200 OK {"user_code": "123456",<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"expires_in_ms": 120000, "device_code": "XYZ", "interval": 1}
         note over N: 3) New device informs existing device of choice of protocol:
-        N->>Z: SecureSend({"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...}, "device_id": "ABCDEFGH"})
+        N->>Z: SecureSend({"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...,<br>"device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI", "device_id_proof": "$base64_encoded_proof_of_identity_key_ownership"})
 
     deactivate N
     end
@@ -859,7 +896,7 @@ sequenceDiagram
     end
 
     rect rgba(0,255,0, 0.1)
-        Z->>E: SecureReceive() => {"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...}, "device_id": "ABCDEFGH"}
+        Z->>E: SecureReceive() => {"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...},<br>"device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI", "device_id_proof": "$base64_encoded_proof_of_identity_key_ownership"}
     end
 
     rect rgba(255,0,0, 0.1)
@@ -917,7 +954,7 @@ sequenceDiagram
         N->>+OP: POST /auth/device client_id=xyz&scope=openid+urn:matrix:api:*+urn:matrix:device:ABCDEFGH...
         OP->>-N: 200 OK {"user_code": "123456",<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"expires_in_ms": 120000, "device_code": "XYZ", "interval": 1}
         note over N: 3) New device informs existing device of choice of protocol:
-        N->>Z: SecureSend({"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...}, "device_id": "ABCDEFGH"})
+        N->>Z: SecureSend({"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...},<br>"device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI", "device_id_proof": "$base64_encoded_proof_of_identity_key_ownership"})
 
     deactivate N
     end
@@ -928,7 +965,7 @@ sequenceDiagram
     #end
 
     rect rgba(0,255,0, 0.1)
-        Z->>E: SecureReceive() => {"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...}, "device_id": "ABCDEFGH"}
+        Z->>E: SecureReceive() => {"type": "m.login.protocol", "protocol": "device_authorization_grant",<br> "device_authorization_grant":{<br>"verification_uri_complete": "https://id.matrix.org/device/abcde",<br>"verification_uri": ...},<br>"device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI", "device_id_proof": "$base64_encoded_proof_of_identity_key_ownership"}
     end
 
     # alt if New device scanned QR code
@@ -977,11 +1014,25 @@ normal for OIDC with MSC3861.
 5. **User is asked by OIDC Provider to consent on existing device**
 
 On receipt of the `m.login.protocol` message above, and having completed step 7 of the secure channel establishment, the
-existing device then asserts that there is no existing device corresponding to the `device_id` from the
-`m.login.protocol` message.
+existing device then verifies the `device_id_proof` and asserts that there is no existing device corresponding to the
+`device_id` from the `m.login.protocol` message.
 
-It does so by calling [GET /_matrix/client/v3/devices/<device_id>](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3devicesdeviceid)
-and expecting to receive an HTTP 404 response.
+The existing device does the following to verify the proof:
+
+```
+ProofBytes := UnpaddedBase64_Decode(device_id_proof)
+
+SH := ECDH(Es, Ip)
+ProofKey := HKDF_SHA256(SH, "MATRIX_QR_CODE_LOGIN_PROOFKEY|" || Ip || "|" || Ep, salt=0, size=32)
+
+unless HMAC_SHA256(ProofKey, "MATRIX_QR_CODE_PROOF_OF_POSSESSION") == ProofBytes:
+    FAIL
+```
+
+If the proof does not match then the login request should be rejected with an `m.login.failure` and reason `device_proof_failed`.
+
+To asset the device does not already exist it calls [GET /_matrix/client/v3/devices/<device_id>](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3devicesdeviceid)
+and expects to receive an HTTP 404 response.
 
 If the device already exists then the login request should be rejected with an `m.login.failure` and reason `device_already_exists`.
 
@@ -1018,6 +1069,11 @@ sequenceDiagram
 
     rect rgba(0,255,0, 0.1)
 
+        note over E: Existing device validates the proof of device key ownership
+        alt proof invalid
+            E->>N: SecureSend({ "type": "m.login.failure", "reason": "device_proof_failed" })
+        end
+
         E->>HS: GET /_matrix/client/v3/devices/{device_id}
         alt device already exists
             HS->>E: 200 OK
@@ -1073,15 +1129,15 @@ If checked successfully then the existing device sends the following secrets to
 
 This is achieved as following:
 
-1. **Existing device confirms that the new device has indeed logged in successfully**
+1. **Existing device confirms that a device with the previously committed-to device ID has indeed logged in successfully**
 
 On receipt of an `m.login.success` message the existing device queries the homeserver to check that the is a device online
 with the corresponding device_id (from the `m.login.protocol` message).
 
 It does so by calling [GET /_matrix/client/v3/devices/<device_id>](https://spec.matrix.org/v1.9/client-server-api/#get_matrixclientv3devicesdeviceid)
 and expecting to receive an HTTP 200 response.
 
-If the device isn't immediately visible it can repeat the `GET` request for up to, say, 10 seconds to allow for any latency.
+If the device isn't immediately visible it can repeat the `GET` request for up to, say, 10 seconds to allow for some latency.
 
 If no device is found then the process should be stopped.
 
@@ -1128,14 +1184,14 @@ Content-Type: application/json
             "m.olm.v1.curve25519-aes-sha2",
             "m.megolm.v1.aes-sha2"
         ],
-        "device_id": "SGKMSRAGBF",
+        "device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI",
         "keys": {
-            "curve25519:SGKMSRAGBF": "I11VOe5quKuH/YjdOqn5VcW06fvPIJQ9JX8ryj6ario",
-            "ed25519:SGKMSRAGBF": "b8gROFh+UIHLD/obY0+IlxoWiGtYVhKdqixvw4QHcN8"
+            "curve25519:3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI",
+            "ed25519:3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI": "b8gROFh+UIHLD/obY0+IlxoWiGtYVhKdqixvw4QHcN8"
         },
         "signatures": {
             "@testing_35:morpheus.localhost": {
-                "ed25519:SGKMSRAGBF": "ziHEUIsHnrYBH4CqYpN1JC/ex3t4VG3zvo16D8ORqN6yAErpsKsnd/5LDdZERIOB1MGffKGfCL6ny5V7rT9FCQ",
+                "ed25519:3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI": "ziHEUIsHnrYBH4CqYpN1JC/ex3t4VG3zvo16D8ORqN6yAErpsKsnd/5LDdZERIOB1MGffKGfCL6ny5V7rT9FCQ",
                 "ed25519:bkYgAVUNqvuyy8b1w09utJNJxBvK3hZB65xxoLPVzFol": "p257k0tfPF98OIDuXnFSJS2DmVlxO4sgTHdF41DTdZBCpTZfPwok6iASo3xMRKdyy3WMEgkQ6lzhEyRKKZBGBQ"
             }
         },
@@ -1239,6 +1295,7 @@ Fields:
 |`protocol`|required `string`|One of: `device_authorization_grant`|
 |`device_authorization_grant`|Required `object` where `protocol` is `device_authorization_grant`|These values are taken from the RFC8628 Device Authorization Response that the new device received from the OIDC Provider: <table> <tr> <td><strong>Field</strong> </td> <td><strong>Type</strong> </td> </tr> <tr> <td><code>verification_uri</code> </td> <td>required <code>string</code> </td> </tr> <tr> <td><code>verification_uri_complete</code> </td> <td><code>string</code> </td> </tr></table>|
 |`device_id`|required `string`|The device ID that the new device will use|
+|`device_id_proof`|required `string`|New device's proof of identity key ownership, base64-encoded|
 
 Example:
 
@@ -1250,7 +1307,8 @@ Example:
         "verification_uri_complete": "https://id.matrix.org/device/abcde",
         "verification_uri": "..."
     },
-    "device_id": "ABCDEFGH"
+    "device_id": "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI",
+    "device_id_proof": "$base64_encoded_proof_of_identity_key_ownership"
 }
 ```
 
@@ -1280,7 +1338,7 @@ Fields:
 |Field|Type||
 |--- |--- |--- |
 |`type`|required `string`|`m.login.failure`|
-|`reason`|required `string`| One of: <table> <tr> <td><strong>Value</strong> </td> <td><strong>Description</strong> </td> </tr><tr> <td><code>authorization_expired</code> </td> <td>The Device Authorization Grant expired</td> </tr> <tr> <td><code>device_already_exists</code> </td> <td>The device ID specified by the new client already exists in the Homeserver provided device list</td> </tr><tr><td><code>device_not_found</code></td><td>The new device is not present in the device list as returned by the Homeserver</td></tr><tr><td><code>unexpected_message_received</code></td><td>Sent by either device to indicate that they received a message of a type that they weren't expecting</td></tr><tr><td><code>unsupported_protocol</code></td><td>Sent by a device where no suitable protocol is available or the requested protocol requested is not supported</td></tr><tr><td><code>user_cancelled</code></td><td>Sent by either new or existing device to indicate that the user has cancelled the login</td></tr></table>|
+|`reason`|required `string`| One of: <table> <tr> <td><strong>Value</strong> </td> <td><strong>Description</strong> </td> </tr><tr> <td><code>authorization_expired</code> </td> <td>The Device Authorization Grant expired</td> </tr> <tr> <td><code>device_already_exists</code> </td> <td>The device ID specified by the new client already exists in the Homeserver provided device list</td> </tr><tr><td><code>device_proof_failed</code></td><td>The proof of device key ownership failed</td></tr><tr><tr><td><code>device_not_found</code></td><td>The new device is not present in the device list as returned by the Homeserver</td></tr><tr><td><code>unexpected_message_received</code></td><td>Sent by either device to indicate that they received a message of a type that they weren't expecting</td></tr><tr><td><code>unsupported_protocol</code></td><td>Sent by a device where no suitable protocol is available or the requested protocol requested is not supported</td></tr><tr><td><code>user_cancelled</code></td><td>Sent by either new or existing device to indicate that the user has cancelled the login</td></tr></table>|
 |`homeserver`|`string`| When the existing device is sending this it can include the Base URL of the homeserver so that the new device can at least save the user the hassle of typing it in|
 
 Example: