Ledger backend is not on par with tendermint votes/proposals #171

adrianbrink · 2019-02-14T13:11:25Z

This seems to be a larger problem with the KMS implementation. Currently the sign_bytes that are returned in session.rs/sign() change depending on whether the KMS is compiled with the YubiHSM backend or the Ledger backend. This should never be the case since the KMS should be agnostic to the backend being used.

So I've added logging after the sign_bytes method in called in session.rs/sign(). With the YubiHSM it results in
Proposal: 119
Prevote: 109 or 110
Precommit: 109 or 110

With the ledger it results in:
Proposal: 120
Prevote: 37

However the sign() method is called before it ever reaches any specific backend HSM. One reason for these errors could be that due to the conditional compilation with "--features" the KMS generates different data depending on which features are turned on.

@tarcieri @liamsi I have no idea why this happens. With the Yubi HSM it works fine and sign_bytes returns the correct values, whereas if you don't compile with yubihsm the returned sign_bytes are wrong.

The second issue here is that the Ledger validator application does not expect these sign_bytes, since they are divergent from the specification. The reason why this only appears with the ledger is that the YubiHSM will just sign whatever it is given, but the ledger actually decodes the bytes. @jleni

The text was updated successfully, but these errors were encountered:

adrianbrink · 2019-02-14T13:25:05Z

I've managed to get the same sign bytes for SignProposal if I add ledger and yubihsm to the default features. The ones for SignVote are still different.

13:18:53 [DEBUG] tmkms::session: started handling request ...
13:18:54 [DEBUG] tmkms::session: got sign proposal request
13:18:54 [DEBUG] tmkms::session: got sign request
13:18:54 [DEBUG] tmkms::session: sign_bytes for request: [119, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 191, 103, 198, 145, 194, 126, 71, 202, 43, 138, 157, 77, 130, 186, 161, 188, 97,
 53, 37, 92, 124, 110, 78, 222, 47, 18, 148, 103, 202, 210, 142, 122, 18, 36, 10, 32, 229, 245, 202, 169, 32, 97, 200, 158, 211, 240, 106, 182, 197, 35, 14, 157, 114, 199, 37, 118, 2, 22, 36, 232, 216, 110, 41, 153, 215, 79, 219, 39, 16,
1, 50, 12, 8, 190, 213, 149, 227, 5, 16, 248, 190, 156, 140, 3, 58, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
13:18:54 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
Ledger is trying to sign: [119, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 191, 103, 198, 145, 194, 126, 71, 202, 43, 138, 157, 77, 130, 186, 161, 188, 97, 53, 37, 92, 124, 110, 78, 222,
 47, 18, 148, 103, 202, 210, 142, 122, 18, 36, 10, 32, 229, 245, 202, 169, 32, 97, 200, 158, 211, 240, 106, 182, 197, 35, 14, 157, 114, 199, 37, 118, 2, 22, 36, 232, 216, 110, 41, 153, 215, 79, 219, 39, 16, 1, 50, 12, 8, 190, 213, 149, 22
7, 5, 16, 248, 190, 156, 140, 3, 58, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
Received an error
13:18:54 [ERROR] [test-yubi@tcp://127.0.0.1:26658] signing operation failed: received an invalid signature: received an invalid signature
13:18:55 [INFO] KMS node ID: 3DD79F0F25B3F71423DABEF47F43BAF3ABEEC68B
13:18:55 [DEBUG] tmkms::session: test-yubi: Connecting to 127.0.0.1:26658...
13:18:55 [INFO] [test-yubi@tcp://127.0.0.1:26658] connected to validator successfully

sign_bytes for SignProposal with the Yubi:

[119, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52,
8, 17, 118, 206, 143, 191, 25, 150, 215, 173, 178, 131, 254, 28, 94, 18, 36, 10, 32, 148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15, 16, 1, 50, 12
, 8, 230, 185, 149, 227, 5, 16, 240, 150, 158, 195, 3, 58, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]

sign_bytes for SignProposal with the Ledger given that all yubihsm stuff is compiled as well:

[119, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 191, 103, 198, 145, 194, 126, 71, 202, 43, 138, 157, 77, 130, 186, 161, 188, 97,
 53, 37, 92, 124, 110, 78, 222, 47, 18, 148, 103, 202, 210, 142, 122, 18, 36, 10, 32, 229, 245, 202, 169, 32, 97, 200, 158, 211, 240, 106, 182, 197, 35, 14, 157, 114, 199, 37, 118, 2, 22, 36, 232, 216, 110, 41, 153, 215, 79, 219, 39, 16,
1, 50, 12, 8, 190, 213, 149, 227, 5, 16, 248, 190, 156, 140, 3, 58, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]

However on the above sign_bytes the ledger fails as you can see from the first log from tmkms.

adrianbrink · 2019-02-14T13:26:32Z

However for SignVote with compiled ledger and yubihsm features the sign_bytes are wrong.

13:18:57 [DEBUG] tmkms::session: got sign vote request
13:18:57 [DEBUG] tmkms::session: got sign request
13:18:57 [DEBUG] tmkms::session: sign_bytes for request: [36, 8, 1, 17, 1, 0, 0, 0, 0, 0, 0, 0, 42, 12, 8, 193, 213, 149, 227, 5, 16, 176, 252, 185, 140, 3, 50, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
13:18:57 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
Ledger is trying to sign: [36, 8, 1, 17, 1, 0, 0, 0, 0, 0, 0, 0, 42, 12, 8, 193, 213, 149, 227, 5, 16, 176, 252, 185, 140, 3, 50, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
Received an error
13:18:57 [ERROR] [test-yubi@tcp://127.0.0.1:26658] signing operation failed: received an invalid signature: received an invalid signature
13:18:58 [INFO] KMS node ID: 3DD79F0F25B3F71423DABEF47F43BAF3ABEEC68B

I can't explain why the conditional compilation of the yubihsm or the ledger would cause different sign_bytes to appear.

jleni · 2019-02-14T13:26:48Z

Yes, but still.. that first byte is unexpected and out of spec.
I am looking at tendermint-rs right now.

adrianbrink · 2019-02-14T13:27:18Z

Oh 119 is not in the spec?

jleni · 2019-02-14T13:30:45Z

well.. at least the spec for how votes/proposals where going to be serialized.
https://github.com/cosmos/ledger-cosmos/blob/aaae52d2692a586eae171577152378c3b756593f/tests/val/vote_parser.cpp#L83

The ledger will expect field_number and later content

liamsi · 2019-02-14T13:45:24Z

It seems like the ledger test-vectors are not up to date. That is easy to fix. The other problem (sign bytes seem to differ depending on if compiled with yubihsm or not) is rather mysterious tho.

jleni · 2019-02-14T13:46:42Z

Ok, the issue is now clear :)
When serializing, the code is using cp.encode_length_delimited(sign_bytes)?;
so the first "magic" byte is the number of bytes.
we need to drop that when sending to the HSMs

jleni · 2019-02-14T13:48:55Z

The issue is in this line
https://github.com/tendermint/kms/blob/752184b5bfa139cde4c9013d836e0f3cda897110/tendermint-rs/src/amino_types/proposal.rs#L118

jleni · 2019-02-14T13:49:36Z

I will write a fix + additional test and check other types too.

liamsi · 2019-02-14T13:49:40Z

OK, wait: In the test-vectors you've linked MarshalBinaryBare is used (see https://github.com/tendermint/tendermint/blob/28d75ec8016b7fb043735cbe4c4d92ec73355de7/types/vote_test.go#L99-L138), the actual sign-bytes use

func (vote *Vote) SignBytes(chainID string) []byte {
	bz, err := cdc.MarshalBinaryLengthPrefixed(CanonicalizeVote(chainID, vote))
	if err != nil {
		panic(err)
	}
	return bz
}

which corresponds to encode_length_delimited (see: https://github.com/tendermint/tendermint/blob/28d75ec8016b7fb043735cbe4c4d92ec73355de7/types/vote.go#L72-L78).

jleni · 2019-02-14T13:51:45Z

ok, so should I then update the test vectors and the spec for the ledger app?

liamsi · 2019-02-14T13:53:12Z

Yes, sorry that the test vectors skip the length. I think initially they were length encoded. Must have been changed in PR (in tm).

adrianbrink · 2019-02-14T14:01:28Z

This is amazing. Thank you very much for looking into this.

@liamsi What is the best way to investigate why the YubiHSM feature is needed for correct compilation?

liamsi · 2019-02-14T14:02:27Z

@jleni: I'll submit a PR to tendermint to match the actual sign bytes.

@liamsi What is the best way to investigate why the YubiHSM feature is needed for correct compilation?

No idea. I'll look into it after the PR.

liamsi · 2019-02-14T14:24:42Z

tendermint/tendermint#3312

liamsi · 2019-02-14T15:23:00Z

I can't reproduce why the bytes differ depending on how you compile (tried with all features available here). Can you elaborate a bit how you've compiled it?

adrianbrink · 2019-02-14T15:42:11Z

Cargo.toml

signatory = { version = "0.11", features = ["ed25519"] }
signatory-dalek = "0.11"
signatory-yubihsm = { version = "0.11", optional = true }
signatory-ledger-cosval = { version = "0.11", optional = true }
subtle-encoding = "0.3"
tendermint = { version = "0.2", path = "tendermint-rs" }

[patch.crates-io]
signatory = { path = "/Users/adrian/code/tendermint/signatory" }
signatory-dalek = { path = "/Users/adrian/code/tendermint/signatory/signatory-dalek" }
signatory-yubihsm = { path = "/Users/adrian/code/tendermint/signatory/signatory-yubihsm" }
signatory-ledger-cosval = { path = "/Users/adrian/code/tendermint/signatory/signatory-ledger-cosval" }

[dev-dependencies]
tempfile = "3"
rand = "0.6"

[features]
default = ["softsign", "yubihsm"]
ledger = ["signatory-ledger-cosval"]
softsign = []
yubihsm = ["signatory-yubihsm/usb"] # USB only for now
yubihsm-mock = ["yubihsm", "signatory-yubihsm/mockhsm"]

# Enable integer overflow checks in release builds for security reasons
[profile.release]
overflow-checks = true

I added logging output to session.rs/sign().

    /// Perform a digital signature operation
    fn sign<T: TendermintRequest + Debug>(&mut self, mut request: T) -> Result<Response, KmsError> {
        debug!("got sign request");
        request.validate()?;

        let mut to_sign = vec![];
        request.sign_bytes(self.chain_id, &mut to_sign)?;
        debug!("sign_bytes for request: {:?}", to_sign);

        // TODO(ismail): figure out which key to use here instead of taking the only key
        // from keyring here:
        let sig = KeyRing::sign(None, &to_sign)?;

        request.set_signature(&sig);
        debug!("successfully signed request:\n {:?}", request);
        Ok(request.build_response())
    }

I'm logging the sign_bytes and below I'll paste the sign_bytes.

Compiling with YubiHSM enabled

Compile tmkms with cargo build, which by default builds the default features including the yubihsm.
Setup a local 1 node testnet.
Start tmkms with ./target/debug/tmkms start -c tmkms.toml -v
Start gaiad

tmkms.toml

# Example KMS configuration file
#
# Copy this to 'tmkms.toml' and edit for your own purposes

## Yubi ##
[[validator]]
addr = "tcp://127.0.0.1:26658" # or "unix:///path/to/socket"
chain_id = "test-yubi"
reconnect = true # true is the default
secret_key = "/Users/adrian/.tmkms/secret_connection.key"

[[providers.yubihsm]]
adapter = { type = "usb" }
auth = { key = 1, password = "password" } # Default YubiHSM admin credentials. Change ASAP!
keys = [{ id = "test-yubi", key = 42 }]
serial_number = "0007550214" # identify serial number of a specific YubiHSM to connect to

Truncated output from tmkms:

SignProposalRequest

15:30:34 [DEBUG] tmkms::session: started handling request ...
15:30:35 [DEBUG] tmkms::session: got sign proposal request
15:30:35 [DEBUG] tmkms::session: got sign request
15:30:35 [DEBUG] tmkms::session: sign_bytes for request: [119, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52,
8, 17, 118, 206, 143, 191, 25, 150, 215, 173, 178, 131, 254, 28, 94, 18, 36, 10, 32, 148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15, 16, 1, 50, 1$, 8, 155, 147, 150, 227, 5, 16, 232, 173, 214, 231, 1, 58, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
15:30:35 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
15:30:35 [DEBUG] yubihsm::session: session=0 n=3 uuid=bf6f58a4-3c86-4606-a0f4-53c284ecc889 cmd=SignEddsa
15:30:35 [DEBUG] tmkms::session: successfully signed request:
 SignProposalRequest { proposal: Some(Proposal { msg_type: 32, height: 1, round: 0, pol_round: -1, block_id: Some(BlockId { hash: [109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52, 8, 17, 118, 206, 143, 191,
25, 150, 215, 173, 178, 131, 254, 28, 94], parts_header: Some(PartsSetHeader { total: 1, hash: [148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15] }$ }), timestamp: Some(TimeMsg { seconds: 1550158235, nanos: 485857000 }), signature: [200, 221, 211, 254, 193, 249, 110, 3, 251, 121, 78, 3, 118, 35, 143, 2, 116, 33, 188, 32, 254, 169, 18, 164, 63, 146, 207, 31, 175, 234, 194, 120, 160, $6, 38, 131, 223, 158, 170, 82, 233, 217, 239, 186, 112, 192, 199, 52, 199, 20, 119, 38, 244, 11, 42, 96, 68, 202, 77, 19, 171, 221, 232, 6] }) }
15:30:35 [DEBUG] tmkms::session: ... success handling request

SignVote Request (Prevote)

15:30:35 [DEBUG] tmkms::session: started handling request ...
15:30:35 [DEBUG] tmkms::session: got sign vote request
15:30:35 [DEBUG] tmkms::session: got sign request
15:30:35 [DEBUG] tmkms::session: sign_bytes for request: [110, 8, 1, 17, 1, 0, 0, 0, 0, 0, 0, 0, 34, 72, 10, 32, 109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52, 8, 17, 118, 206, 143, 191, 25, 150, 215, 173,
 178, 131, 254, 28, 94, 18, 36, 10, 32, 148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15, 16, 1, 42, 12, 8, 155, 147, 150, 227, 5, 16, 248, 227, 253
, 176, 2, 50, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
15:30:35 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
15:30:35 [DEBUG] yubihsm::session: session=0 n=4 uuid=ead0f371-4489-47df-87f8-f8ef2542f146 cmd=SignEddsa
15:30:35 [DEBUG] tmkms::session: successfully signed request:
 SignVoteRequest { vote: Some(Vote { vote_type: 1, height: 1, round: 0, block_id: Some(BlockId { hash: [109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52, 8, 17, 118, 206, 143, 191, 25, 150, 215, 173, 178, 131
, 254, 28, 94], parts_header: Some(PartsSetHeader { total: 1, hash: [148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15] }) }), timestamp: Some(TimeMs
g { seconds: 1550158235, nanos: 639595000 }), validator_address: [216, 240, 13, 101, 177, 130, 4, 73, 202, 6, 11, 225, 5, 25, 195, 131, 173, 199, 34, 218], validator_index: 0, signature: [163, 185, 99, 121, 55, 52, 21, 5, 79, 85, 55, 227,
 149, 219, 71, 152, 20, 14, 173, 99, 105, 198, 127, 72, 165, 159, 183, 25, 53, 126, 122, 105, 50, 87, 186, 56, 181, 57, 129, 247, 10, 221, 191, 196, 159, 177, 72, 93, 208, 223, 88, 133, 255, 102, 83, 137, 23, 239, 49, 53, 147, 222, 38, 14
] }) }
15:30:35 [DEBUG] tmkms::session: ... success handling request

SignVote Request (Precommit)

15:30:35 [DEBUG] tmkms::session: started handling request ...
15:30:35 [DEBUG] tmkms::session: got sign vote request
15:30:35 [DEBUG] tmkms::session: got sign request
15:30:35 [DEBUG] tmkms::session: sign_bytes for request: [110, 8, 2, 17, 1, 0, 0, 0, 0, 0, 0, 0, 34, 72, 10, 32, 109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52, 8, 17, 118, 206, 143, 191, 25, 150, 215, 173,
 178, 131, 254, 28, 94, 18, 36, 10, 32, 148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15, 16, 1, 42, 12, 8, 155, 147, 150, 227, 5, 16, 200, 224, 207
, 249, 2, 50, 9, 116, 101, 115, 116, 45, 121, 117, 98, 105]
15:30:35 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
15:30:35 [DEBUG] yubihsm::session: session=0 n=5 uuid=7e8b4b1f-7466-4d1f-9a16-bb2871ea56b4 cmd=SignEddsa
15:30:35 [DEBUG] tmkms::session: successfully signed request:
 SignVoteRequest { vote: Some(Vote { vote_type: 2, height: 1, round: 0, block_id: Some(BlockId { hash: [109, 184, 47, 185, 69, 249, 99, 187, 134, 206, 136, 198, 23, 145, 171, 194, 52, 8, 17, 118, 206, 143, 191, 25, 150, 215, 173, 178, 131
, 254, 28, 94], parts_header: Some(PartsSetHeader { total: 1, hash: [148, 8, 254, 116, 222, 40, 160, 178, 114, 15, 62, 50, 10, 187, 33, 66, 206, 190, 39, 140, 25, 50, 56, 61, 158, 0, 54, 81, 122, 146, 19, 15] }) }), timestamp: Some(TimeMs
g { seconds: 1550158235, nanos: 791933000 }), validator_address: [216, 240, 13, 101, 177, 130, 4, 73, 202, 6, 11, 225, 5, 25, 195, 131, 173, 199, 34, 218], validator_index: 0, signature: [14, 139, 52, 18, 88, 188, 117, 13, 152, 48, 205, 1
69, 252, 217, 168, 89, 102, 117, 218, 214, 102, 241, 107, 10, 109, 81, 188, 166, 151, 89, 138, 45, 116, 42, 31, 220, 195, 81, 66, 247, 227, 112, 220, 141, 7, 146, 136, 163, 196, 251, 30, 33, 153, 57, 19, 242, 250, 189, 54, 71, 201, 101, 8
7, 3] }) }
15:30:35 [DEBUG] tmkms::session: ... success handling request

Compiling with Ledger enabled but without YubiHSM

Compile tmkms with cargo build --features ledger
Setup a local 1 node testnet.
Start tmkms with ./target/debug/tmkms start -c tmkms.toml -v
Start gaiad

tmkms.toml

# Example KMS configuration file
#
# Copy this to 'tmkms.toml' and edit for your own purposes

# ## Ledger ##
[[validator]]
addr = "tcp://127.0.0.1:26658" # or "unix:///path/to/socket"
chain_id = "test-ledger"
reconnect = true # true is the default
secret_key = "/Users/adrian/.tmkms/secret_connection.key"

[[providers.ledger]]
active = true

Truncated output from tmkms

SignProposal Request

15:39:43 [DEBUG] tmkms::session: started handling request ...
15:39:44 [DEBUG] tmkms::session: got sign proposal request
15:39:44 [DEBUG] tmkms::session: got sign request
15:39:44 [DEBUG] tmkms::session: sign_bytes for request: [120, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 191, 103, 198, 145, 194, 126, 71, 202, 43, 138, 157, 77, 130, 186, 161, 188, 97$ 53, 37, 92, 124, 110, 78, 222, 47, 18, 148, 103, 202, 210, 142, 122, 18, 36, 10, 32, 229, 245, 202, 169, 32, 97, 200, 158, 211, 240, 106, 182, 197, 35, 14, 157, 114, 199, 37, 118, 2, 22, 36, 232, 216, 110, 41, 153, 215, 79, 219, 39, 16,
1, 50, 11, 8, 192, 151, 150, 227, 5, 16, 240, 138, 218, 99, 58, 11, 116, 101, 115, 116, 45, 108, 101, 100, 103, 101, 114]
15:39:44 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
Ledger is trying to sign: [120, 8, 32, 17, 1, 0, 0, 0, 0, 0, 0, 0, 33, 255, 255, 255, 255, 255, 255, 255, 255, 42, 72, 10, 32, 191, 103, 198, 145, 194, 126, 71, 202, 43, 138, 157, 77, 130, 186, 161, 188, 97, 53, 37, 92, 124, 110, 78, 222$ 47, 18, 148, 103, 202, 210, 142, 122, 18, 36, 10, 32, 229, 245, 202, 169, 32, 97, 200, 158, 211, 240, 106, 182, 197, 35, 14, 157, 114, 199, 37, 118, 2, 22, 36, 232, 216, 110, 41, 153, 215, 79, 219, 39, 16, 1, 50, 11, 8, 192, 151, 150, 2$7, 5, 16, 240, 138, 218, 99, 58, 11, 116, 101, 115, 116, 45, 108, 101, 100, 103, 101, 114]
Received an error
15:39:44 [ERROR] [test-ledger@tcp://127.0.0.1:26658] signing operation failed: received an invalid signature: received an invalid signature

SignVote Request

15:39:47 [DEBUG] tmkms::session: started handling request ...
15:39:47 [DEBUG] tmkms::session: got sign vote request
15:39:47 [DEBUG] tmkms::session: got sign request
15:39:47 [DEBUG] tmkms::session: sign_bytes for request: [37, 8, 1, 17, 1, 0, 0, 0, 0, 0, 0, 0, 42, 11, 8, 195, 151, 150, 227, 5, 16, 152, 223, 255, 99, 50, 11, 116, 101, 115, 116, 45, 108, 101, 100, 103, 101, 114]
15:39:47 [DEBUG] tmkms::keyring: Successfully got signer and now trying to sign message
Ledger is trying to sign: [37, 8, 1, 17, 1, 0, 0, 0, 0, 0, 0, 0, 42, 11, 8, 195, 151, 150, 227, 5, 16, 152, 223, 255, 99, 50, 11, 116, 101, 115, 116, 45, 108, 101, 100, 103, 101, 114]
Received an error
15:39:47 [ERROR] [test-ledger@tcp://127.0.0.1:26658] signing operation failed: received an invalid signature: received an invalid signature

The reason why the Ledger doesn't return a proper signature have been discussed above and will hopefully be fixed soon.

However, I don't understand why the first byte is different in the SignProposal Request is different and why the bytes for the SignVote Request are completely different. As far as I can tell nothing in the code leading up until the point of calling

        let mut to_sign = vec![];
        request.sign_bytes(self.chain_id, &mut to_sign)?;
        debug!("sign_bytes for request: {:?}", to_sign);

is branching on whether the YubiHSM or Ledger feature is enabled.
The only branching behaviour depending on the backend should happen in.

        // TODO(ismail): figure out which key to use here instead of taking the only key
        // from keyring here:
        let sig = KeyRing::sign(None, &to_sign)?;

jleni · 2019-02-14T19:43:39Z

Quick update:

Ledger Tendermint validator app to v0.6.0 follows the updated spec and can parse KMS messages
ledger-tendermint-rs can now handle this too
PR for signatory is here Adding ledger tendermint provider signatory#141
I have integrated ledger-tendermint-rs + signatory-ledger-rm into KMS. I still need to finish some basic ledgertm commands and I will send a PR with the integration into KMS (most likely tomorrow.)
Locally it is working well, however, once everything is merged we might want to review a few error cases such as possible disconnections, etc.

adrianbrink · 2019-02-14T21:14:08Z

Fantastic :-) I can't wait to test the entire pipeline.

I also created #172 which integrates the current ledger version into it. It needs updating with your changes though or we can discard it and pull in yours.

jleni · 2019-02-14T21:32:30Z

I am happy to take it and apply my changes on top. I will do it tomorrow morning then.

liamsi · 2019-02-20T11:30:21Z

I think this can be closed. This was fixed here: tendermint/signatory#141. And as far as I can see, the original concerns raised by Adrian (that this does depend on with which feature you compile with) do not hold.

Feel free to reopen if there is still an issue.

adrianbrink changed the title ~~Ledger receives different sign-bytes than yubi~~ sign_bytes are dependent on backing HSMs Feb 14, 2019

adrianbrink changed the title ~~sign_bytes are dependent on backing HSMs~~ sign_bytes are dependent on backing HSMs and they shouldn't be at all Feb 14, 2019

liamsi mentioned this issue Feb 14, 2019

Test-vectors in vote_test do not match sign-bytes tendermint/tendermint#3311

Closed

liamsi changed the title ~~sign_bytes are dependent on backing HSMs and they shouldn't be at all~~ Ledger backend is not on par with tendermint votes/proposal Feb 15, 2019

liamsi changed the title ~~Ledger backend is not on par with tendermint votes/proposal~~ Ledger backend is not on par with tendermint votes/proposals Feb 15, 2019

liamsi closed this as completed Feb 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ledger backend is not on par with tendermint votes/proposals #171

Ledger backend is not on par with tendermint votes/proposals #171

adrianbrink commented Feb 14, 2019 •

edited

Loading

adrianbrink commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

jleni commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

jleni commented Feb 14, 2019

liamsi commented Feb 14, 2019 •

edited

Loading

jleni commented Feb 14, 2019

jleni commented Feb 14, 2019

jleni commented Feb 14, 2019

liamsi commented Feb 14, 2019 •

edited

Loading

jleni commented Feb 14, 2019

liamsi commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

liamsi commented Feb 14, 2019

liamsi commented Feb 14, 2019

liamsi commented Feb 14, 2019

adrianbrink commented Feb 14, 2019 •

edited

Loading

jleni commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

jleni commented Feb 14, 2019

liamsi commented Feb 20, 2019

Ledger backend is not on par with tendermint votes/proposals #171

Ledger backend is not on par with tendermint votes/proposals #171

Comments

adrianbrink commented Feb 14, 2019 • edited Loading

adrianbrink commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

jleni commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

jleni commented Feb 14, 2019

liamsi commented Feb 14, 2019 • edited Loading

jleni commented Feb 14, 2019

jleni commented Feb 14, 2019

jleni commented Feb 14, 2019

liamsi commented Feb 14, 2019 • edited Loading

jleni commented Feb 14, 2019

liamsi commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

liamsi commented Feb 14, 2019

liamsi commented Feb 14, 2019

liamsi commented Feb 14, 2019

adrianbrink commented Feb 14, 2019 • edited Loading

jleni commented Feb 14, 2019

adrianbrink commented Feb 14, 2019

jleni commented Feb 14, 2019

liamsi commented Feb 20, 2019

adrianbrink commented Feb 14, 2019 •

edited

Loading

liamsi commented Feb 14, 2019 •

edited

Loading

liamsi commented Feb 14, 2019 •

edited

Loading

adrianbrink commented Feb 14, 2019 •

edited

Loading