Security/SA-1

From Snowblossom Wiki
Revision as of 18:12, 30 October 2020 by Fireduck (talk | contribs)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to: navigation, search

Overview

 Date: 2020.10.30
 Severity: none
 Description: ECDSA signing using SHA1 hash

Transaction signing for ECDSA uses SHA1 hash. This means, signed data is first hashed with SHA-1 and then the output of that hash is signed with ECDSA. SHA-1 is no longer considered a cryptographically secure hash algorithm. This is not considered a best practice or in fact something we picked. It is an oversight that we weren't using a stronger hash in the signing process. However, the impact is very low. See #Impact section.

The ECDSA signing is the default for seed type wallets or old wallets before seed support was added to the clients.

Suggested User Actions

None

Impact

So SHA-1 is considered not cryptographically sound because there is a collision attack on it. Basically that means that there is a way to find two different values X and Y such that hash_sha1(X) == hash_sha1(Y). In order for SHA-1 to be vulnerable the way we are using it, an attacker would have to do what is called a second preimage attack. This where there is an existing hash_sha1(A) = B and the attacker needs to find an alternate A' that also hashes to B. This is much harder and currently there is no known second preimage attack on SHA-1.

However, our situation is even better than that. In our transaction validation on Snowblossom, what we actually are doing is where the transaction bytes are T:

 T = bytes of TransactionInner protobuf
 TX_ID = hash_skein(T)   
 C = hash_sha1(TX_ID)
 S = sig_ecdsa(C, priv_key)


In Snowblossom block validation, the TransactionInner must be a valid protobuf that passes all the validation rules (inputs and outputs matching, etc), then it gets hashed to the transaction ID. Then that is signed using the private key.

So to actually attack this, even if SHA-1 were completely broken and an attacker could trivially find other values of TX_ID that hash to the same C and thus the signature validates, those TX_ID would have to be the output of a skein hash of a valid TransactionInner protobuf. Which means doing a second preimage attack on skein as well, and if they could do that, they could simply have their skein attack output the same TX_ID as the existing signature.

In short, there is no way to exploit this without also breaking Skein.

Developer Actions

Regardless of this being very minor, we should fix it anyways. There are two main ways we can address this:

  1. Introduce new signature types to replace current ECDSA and ECDSA_Compressed. This will involve a breaking protocol change (and associated SIP vote) and wallets having new addresses with the new signing type.
  2. Add a signature mode field to the SignatureEntry protobuf. This will also involve a breaking protocol change (and associated SIP vote) but as it would not change the SigSpecs, the addresses wouldn't change avoiding a lot of confusion. The software will just consider lacking the flag to the old mode, SHA1 signatures. And if present the field can specify SHA256 hashes (or even SHA3 if we want).

Option 2 seems to be the clear winner.