MITM in secret connection

[liamsi]

I haven't fully verified this yet. But the write-up in the tendermint issue looks sane:
tendermint/tendermint#3010

[tarcieri]

For what it's worth, #111 documents another MitM attack: public keys are not validated by the KMS in any form yet.

[tarcieri]

Work is underway to address this: tendermint/tendermint#3315

[nuevax]

I believe Scone has a generic framework for it https://sconecontainers.github.io/ , https://sconedocs.github.io/technical_summary/#integration-with-secure-key-store Transparent TLS encryption

In my opinion Scone can obsolete a HSM and provide security around a key file. Some may reject SGX altogether, but I believe it is a very suitable solution.

[tarcieri]

@nuevax that isn't relevant to this particular issue

[tarcieri]

https://eprint.iacr.org/2019/526.pdf contains a Tamarin proof of the current Secret Connection protocol's security with low order points rejected.

It'd definitely be good to continue adding a transcript hash (#254), but given the existence of a proof of the security of the protocol with the simple mitigation, I think we can add that for now.

I think we can add a check that the computed secret is all zeroes, e.g:

https://github.com/tendermint/kms/blob/3b3bd7bcc2a3cbe11b3367852a54e4420cabea63/tendermint-rs/src/secret_connection.rs#L63

We could check:

if shared_secret.ct_eq([0; 32]) == 1 {
    return Err(...)
}

[tarcieri]

I've implemented a fix in #279 which implements both the blacklist for degenerate points as well as a constant-time check for an all-zero shared secret, a belt-and-suspenders approach which I believe should be sufficient to mitigate this particular issue.

It's set to auto-close when merged, however it would be good to double-check this with an auditor and/or cryptographer.

That said, the Tamarin proofs in https://eprint.iacr.org/2019/526.pdf make me fairly confident this should be sufficient.