signature verification fails on second message

[agoric]

signature verification fails on second message

[warner]

I've tracked this far enough to tell that the client is signing a different message than the chain is validating. The cosmos-sdk appears to use the dreaded "sign the canonical serialization" pattern, in which the bytes being sent over the wire are not the same bytes that the signer uses when computing the signature (and hopefully the verifier can reconstruct the same byte sequence). My biggest concern about this pattern is that the recipient may choose to act upon a decoded form of the received bytes, even though the signature they verified was on something different. If there is any variation in the encoding or parsing, an attacker might be able to replace a signed message on the wire with something very similar, but that has different semantics.

In this case, the sender (our ag-cosmos-helper, which acts like nscli in the cosmos-sdk Name Service tutorial) produces a message with the following JSON structure:

{
 account_number: "0",
 chain_id: "agoric",
 fee: { amount: [], gas: "200000" },
 memo: "",
 msgs: [
  {
    Ack: 1,
    Messages: [],
    Nums: [],
    Peer: (pubkey)
    Submitter: (pubkey)
  }],
 sequence: "1"
}

In MakeSignature, the body being signed (msg.Bytes()) is:

00000000  7b 22 61 63 63 6f 75 6e  74 5f 6e 75 6d 62 65 72  |{"account_number|
00000010  22 3a 22 30 22 2c 22 63  68 61 69 6e 5f 69 64 22  |":"0","chain_id"|
00000020  3a 22 61 67 6f 72 69 63  22 2c 22 66 65 65 22 3a  |:"agoric","fee":|
00000030  7b 22 61 6d 6f 75 6e 74  22 3a 5b 5d 2c 22 67 61  |{"amount":[],"ga|
00000040  73 22 3a 22 32 30 30 30  30 30 22 7d 2c 22 6d 65  |s":"200000"},"me|
00000050  6d 6f 22 3a 22 22 2c 22  6d 73 67 73 22 3a 5b 7b  |mo":"","msgs":[{|
00000060  22 41 63 6b 22 3a 31 2c  22 4d 65 73 73 61 67 65  |"Ack":1,"Message|
00000070  73 22 3a 5b 5d 2c 22 4e  75 6d 73 22 3a 5b 5d 2c  |s":[],"Nums":[],|
00000080  22 50 65 65 72 22 3a 22  63 6f 73 6d 6f 73 31 66  |"Peer":"cosmos1f|
00000090  33 30 76 65 6d 76 36 72  75 7a 38 38 74 34 73 78  |30vemv6ruz88t4sx|
000000a0  66 6a 68 74 67 75 67 36  79 76 6e 6a 6e 33 6a 64  |fjhtgug6yvnjn3jd|
000000b0  34 74 61 78 6a 22 2c 22  53 75 62 6d 69 74 74 65  |4taxj","Submitte|
000000c0  72 22 3a 22 63 6f 73 6d  6f 73 31 66 33 30 76 65  |r":"cosmos1f30ve|
000000d0  6d 76 36 72 75 7a 38 38  74 34 73 78 66 6a 68 74  |mv6ruz88t4sxfjht|
000000e0  67 75 67 36 79 76 6e 6a  6e 33 6a 64 34 74 61 78  |gug6yvnjn3jd4tax|
000000f0  6a 22 7d 5d 2c 22 73 65  71 75 65 6e 63 65 22 3a  |j"}],"sequence":|
00000100  22 31 22 7d                                       |"1"}|

The actual transmitted bytes (maybe "amino" encoded?) are smaller (txBytes inside CompleteAndBroadcastTxCli):

00000000  c3 01 f0 62 5d ee 0a 4b  07 0f de 94 0a 2d 63 6f  |...b]..K.....-co|
00000010  73 6d 6f 73 31 66 33 30  76 65 6d 76 36 72 75 7a  |smos1f30vemv6ruz|
00000020  38 38 74 34 73 78 66 6a  68 74 67 75 67 36 79 76  |88t4sxfjhtgug6yv|
00000030  6e 6a 6e 33 6a 64 34 74  61 78 6a 20 01 2a 14 4c  |njn3jd4taxj .*.L|
00000040  5e cc ed 9a 1f 04 73 ae  b0 32 65 75 a3 88 d1 19  |^.....s..2eu....|
00000050  39 4e 32 12 04 10 c0 9a  0c 1a 6a 0a 26 eb 5a e9  |9N2.......j.&.Z.|
00000060  87 21 02 39 46 8a 4b 4e  c8 85 0b 36 6e 0c da bf  |.!.9F.KN...6n...|
00000070  5a 9a ab 7b be 5b cd 06  03 60 6d 52 14 c0 91 e0  |Z..{.[...`mR....|
00000080  d6 4e b1 12 40 02 4a 58  7a a2 2f 1c 1f 97 f4 35  |[email protected]./....5|
00000090  25 90 79 e6 94 c7 1d 1f  97 0d 8a 64 2e b1 73 2a  |%.y........d..s*|
000000a0  00 49 c0 4a 5c 25 28 a0  33 47 d8 ce f5 9f 92 cc  |.I.J\%(.3G......|
000000b0  d7 37 c1 a1 92 b4 0c bb  11 0b e6 85 70 e6 94 04  |.7..........p...|
000000c0  fb ab 51 c5 58                                    |..Q.X|

When this arrives on the chain node, the x/auth/ante.go NewAnteHandler calls processSig with a signBytes that differs in the way the empty Messages and Nums lists are encoded (they show up as null instead of []):

00000000  7b 22 61 63 63 6f 75 6e  74 5f 6e 75 6d 62 65 72  |{"account_number|
00000010  22 3a 22 30 22 2c 22 63  68 61 69 6e 5f 69 64 22  |":"0","chain_id"|
00000020  3a 22 61 67 6f 72 69 63  22 2c 22 66 65 65 22 3a  |:"agoric","fee":|
00000030  7b 22 61 6d 6f 75 6e 74  22 3a 5b 5d 2c 22 67 61  |{"amount":[],"ga|
00000040  73 22 3a 22 32 30 30 30  30 30 22 7d 2c 22 6d 65  |s":"200000"},"me|
00000050  6d 6f 22 3a 22 22 2c 22  6d 73 67 73 22 3a 5b 7b  |mo":"","msgs":[{|
00000060  22 41 63 6b 22 3a 31 2c  22 4d 65 73 73 61 67 65  |"Ack":1,"Message|
00000070  73 22 3a 6e 75 6c 6c 2c  22 4e 75 6d 73 22 3a 6e  |s":null,"Nums":n|
00000080  75 6c 6c 2c 22 50 65 65  72 22 3a 22 63 6f 73 6d  |ull,"Peer":"cosm|
00000090  6f 73 31 66 33 30 76 65  6d 76 36 72 75 7a 38 38  |os1f30vemv6ruz88|
000000a0  74 34 73 78 66 6a 68 74  67 75 67 36 79 76 6e 6a  |t4sxfjhtgug6yvnj|
000000b0  6e 33 6a 64 34 74 61 78  6a 22 2c 22 53 75 62 6d  |n3jd4taxj","Subm|
000000c0  69 74 74 65 72 22 3a 22  63 6f 73 6d 6f 73 31 66  |itter":"cosmos1f|
000000d0  33 30 76 65 6d 76 36 72  75 7a 38 38 74 34 73 78  |30vemv6ruz88t4sx|
000000e0  66 6a 68 74 67 75 67 36  79 76 6e 6a 6e 33 6a 64  |fjhtgug6yvnjn3jd|
000000f0  34 74 61 78 6a 22 7d 5d  2c 22 73 65 71 75 65 6e  |4taxj"}],"sequen|
00000100  63 65 22 3a 22 31 22 7d                           |ce":"1"}|

The difference between this and what MakeSignature used is, of course, enough to make the signature invalid, and the message is rejected.

I suspect this has something to do with interactions between our msgs.go definitions of GetSignBytes(), MsgDeliverInbound, and NewMsgDeliverInbound(). I imagine the amino encoding isn't representing "empty" in the same way as the JSON serialization does it.

[warner]

@michaelfig and I tracked this down to the Amino decoder providing us with a MsgDeliverInbound struct that contained Messages= nil instead of an empty array. This might be by design.. we're looking at the go-amino bugs to figure it out. The upshot is that when we send Messages: [] from the client, the chain gets Messages: nil, which then serializes into Messages:null in GetSignBytes. That difference breaks the signatures.

We ruled out json.Marshal causing the problem by asking it to serialize a structure with an empty array, and it created "Messages":[] as expected. json.Marshal only put null in the output when it got nil as an input, which is why we think it's the amino pathway. It might be either the encoder or the decoder, we can't yet distinguish between the two.

@michaelfig landed a patch (e214661) to compensate for this by replacing the nil with an empty array in GetSignBytes, which makes the signatures work again. So we can close this bug, although MFig is going to file a bug on go-amino to see if we can get the problem addressed upstream.

[michaelfig]

I have raised this issue at tendermint/go-amino#275

[warner]

in the old repo. this was cosmic-swingset issue 5