[Crypto] fix KMS signing bug for long messages

[tarakby]

Closes: #317

Description

Fixes a bug when messages signed by Google KMS are longer than 65536 bytes.
If the message is longer than the KMS limit, pre-hash the message outside KMS.

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the standards mentioned here.
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[devbugging]

Thank you! LGTM

[bluesign]

If the message is longer than the KMS limit, pre-hash the message outside KMS.

Sorry the naive question but why not always pre-hash ? This way you can also support SHA3

[tarakby]

@bluesign
Good question. Although hashing does not involve a private key, it is still a sensitive operation and an important step in signing (the security of the signature algorithm depends on the hash function security). It is therefore recommended to perform hashing within the boundaries of the trusted platform.
In this case the HSM is the trusted platform, and it is better to fit as much signing steps into it as possible. This helps reducing the attack windows.
In some cases/designs, we may want to pre-hash outside the trusted hardware for different reasons, that's why some HSMs allow signing hashes.

[bluesign]

@tarakby

Although hashing does not involve a private key, it is still a sensitive operation and an important step in signing. (the security of the signature algorithm depends on the hash function security).

If I understand correctly, hash is the "weakest link" here it means.

It is therefore recommended to perform hashing within the boundaries of the trusted platform. In this case the HSM is the trusted platform, and it is better to fit as much signing steps into it as possible. This helps reducing the attack windows.

Here I think we differ. In Flow use case, other use cases I am not sure, only attack is to confuse Transaction with Hash. (which is only possible here actually )

So I specially craft a Transaction, which looks innocent, but transaction canonical form will be equal to the hash of my evil transaction.

But in this case minimum Flow Transaction Payload length but more importantly transaction domain tag prevent those attacks. ( I cannot send hash to HSM at the first place when in transaction (plaintext) mode without domain tag )

[tarakby]

I agree that on Flow a hash (32 bytes) cannot be confused as a signable transaction payload (at least 32 bytes DST + other data).

only attack is to confuse Transaction with Hash. So I specially craft a Transaction, which looks innocent, but transaction canonical form will be equal to the hash of my evil transaction.

I don't think I understood the attack path though. Can you please formulate it differently ?

The added security by hashing on HSM is mainly to guarantee that the hashing computation and signable hash are trusted. This avoids the HSM signing some hash sent from outside. An attack may still tamper with the original message on the application device, but at least it can't tamper with the hash itself, hence reducing the tampering window.

[tarakby]

Regarding SHA3 support: In theory, it is possible to pre-hash using SHA3 and only process the following steps on HSM as you mentioned. For now we restricted the support to SHA2 for consistency because the ECDSA keys generated on KMS specify the hashing algo and for now only include SHA2.

[bluesign]

I was thinking more on what is feeling not ok for me, I think I figured it out. ( After a lot of coffee )

Consider a system with flow-go-sdk: ( let's say a web REST service for signing getting arbitrary transaction data )

User - Service - KMS - Flow Network: let's say service is super dummy and just passing data to KMS. ( or let's say simply exposing KMS methods to web, which just validating byte array is a Transaction )

We have 1 method: hashAndSign(m []byte), then hashAndSign(m []byte) is only vulnerable in the case of hash collision or bad implementation on hashing algorithm.

Collision case ( on SHA ) can be ignored as this is not our concern ( as it will be similar bad in KMS as ours ), that leaves us with bad implementation on hashing algorithm. (causing a prefix collision attack, preimage attack etc )

in case of pre-image attack:
this is very unlikely ( as we validate it is a transaction etc ) But it is game over: if hash(evil_transaction) = kms_hash(innocent_transaction) So KMS doesn't make difference in this case ( considering we use the same hash algorithm in software of flow-go )

In case of prefix collision attack:
If we have bad hashing algorithm, and google has the correct, worse case is that we will reject valid signatures.

So sending KSM here is obviously has benefits. ( for prefix collision attack)

But now, when we add the size limit (65536), aren't we back to square one ?

Considering hashing algorithm is buggy and vulnerable to preimage attack, as we can choose p1 and p2 in preimage attack, and only search for m1 and m2, we can keep both payloads over 65536 easily.

PS: I think we can most likely say that Transaction structure is not vulnerable to the preimage attack but it is not relevant in this discussion.

[tarakby]

I see what kind of issues you were considering.
Here is a high level of the situation as I see it:

We have 1 method: hashAndSign(m []byte)

I would consider the method as sign(m []byte) which includes hashing as an internal step (a step that does not involve the private key). Here we're discussing whether sign would be split over two platforms.

Reusing the same flow you described: User - Service - KMS - Network, we should first assume the network (the party that verifies signatures) is using a secure hash function hash_network (both algorithm and implementation). Otherwise, multiple attack paths would open up, whatever signing implementation we use. hash_network is resistant to 1st pre-image and collisions (therefore 2nd pre-image too).

Now we're comparing:

1. hash the message on service - send hash to KMS - finish signing on KMS
2. send message to KMS - sign on KMS

In both options, KMS does not check the data it receives corresponds to a valid transaction. The service is the party that is supposed to guarantee that. Let's call h_user the hash output signed by the user (either in (1) or (2))
There will be an issue in general if h_user = hash_network(malicious_transaction) i.e, the user authorizing malicious_transaction implicitly by signing h_user.

We can also make the assumption that KMS implements the same hash_network. Signing malicious_transaction can only happen if something goes wrong at the service level :

• in (1), the service is tricked to replace message by malicious_transaction, or the service is tricked to send hash_network(malicious_transaction) to KMS (using a faulty hash function on service or any other method).
• in (2), the service is tricked to replace message by malicious_transaction.

I think there is no need in this case to consider the collision properties of hash_service. It is enough to get KMS to sign hash_network(malicious_transaction).

(2) has less windows than (1) to trick the service, as it reduces the attack window to changing message. Since it is not possible to implement (2) for long messages, it is fine to fall back to (1) in that case.

[tarakby]

FYI, it seems to me that AWS HSM interface does not allow pre-hashing when signing (had a quick look at the API so I might be wrong) https://docs.aws.amazon.com/cloudhsm/latest/userguide/key_mgmt_util-sign.html

[pgebheim]

FYI, it seems to me that AWS HSM interface does not allow pre-hashing when signing (had a quick look at the API so I might be wrong) https://docs.aws.amazon.com/cloudhsm/latest/userguide/key_mgmt_util-sign.html

Iirc the hsm also includes a tighter limit on payload size.

[bluesign]

FYI, it seems to me that AWS HSM interface does not allow pre-hashing when signing (had a quick look at the API so I might be wrong) https://docs.aws.amazon.com/cloudhsm/latest/userguide/key_mgmt_util-sign.html

https://docs.aws.amazon.com/cloudhsm/latest/userguide/pkcs11-mechanisms.html#mech3-2

[3.2] AWS CloudHSM approaches hashing differently based on the Client SDK. For Client SDK 5, we do all hashing for supported functions in software and we do not copy to the HSM, meaning that there is no data size limit. For Client SDK 3, where we do the hashing depends on data size and whether you’re using single-part or multipart operations.

@pgebheim correct they seem to do 16kb max