SignIdentity and GetECDHSessionKey

[andrewkozlik]

There are several issues with the way the SignIdentity message (SLIP-0013) and GetECDHSessionKey message (SLIP-0017) are handled in firmware. Some of these have been identified by Mathias Herberts, others internally.

1. According to RFC 3986 the proto part and the host part of a URI should be treated as case-insensitive. The SLIPs do not explain how to achieve that and the implementation treats both as case sensitive. We should amend the SLIPs to specify normalization of proto and host to lowercase in the derivation calculation and implement the conversion to lowercase in Trezor as well.

2. The :// string, as per RFC 3986 section 3, should only be included when the URI has an authority part. For example the simple URI foo:bar with a scheme of foo and a path of bar is a valid URI as per RFC 3986 but it is incorrectly reconstructed as foo://bar by serialize_identity(), turning it into a URI with scheme foo (correct) and host bar (incorrect).

3. According to the SLIPs proto:// is mandatory in the URI, but both Core and Legacy treat is as optional, i.e. if the proto parameter is absent, then the :// is not prepended.

4. According to the SLIPs when path is present in the identity, the URI should be constructed by appending a / followed by the path. However, neither Legacy nor Core firmware place the / before the path.

5. In Legacy firmware if host is absent from the identity, then layoutDecryptIdentity() does not display port, but cryptoIdentityFingerprint() does use port in the path computation.

6. Legacy firmware does not display path, while core firmware does.

7. The parameters for the device test that we are using here are weird host="Satoshi Nakamoto <satoshi@bitcoin.org>". I'd expect user= "satoshi", host="bitcoin.org".

8. Trezor does not implement any sanitation of the input, e.g. requiring port to be numeric or generally disallowing @, so host="satoshi@bitcoin.org" and user= "satoshi", host="bitcoin.org" probably gives the same result.

9. Trezor doesn't have a function to display a SLIP-0013 or SLIP-0017 derived public key or public key fingerprint along with the corresponding identity. Without this feature, users have no secure means to verify that the BIP 32 path of their key was generated for the correct identity.

Discussion notes

Original issue https://github.com/satoshilabs/trezor-firmware/issues/195.

Displaying the index on Trezor

We believe that displaying the index is not necessary. The index is not part of the URI identity, so not displaying it does not pose a security issue. The index allows the host application to access a set of keys for the given identity. By confirming the screen, the user approves access to the entire set for the given identity, although only one key is used.

Displaying the public key with which the agreement is performed

We believe that displaying this key would not improve the security in any way. Our assumption is that the remote key is ephemeral and the shared secret is used for decryption of a message that was received by the user. At present GetECDHSessionKey is used in trezor-agent's gpg and age and the usage there aligns with these assumptions.

[hbs]

The GetECDHSessionKey is related to SLIP-0017 rather than SLIP-0013