Protobuf Transaction Signing

[aaronc]

Problem Definition

The Cosmos SDK has historically used Amino JSON for signing of transactions whereas Amino binary is used for encoding.
During the SDK's migration to protobuf, we had made the preliminary decision to use a canonical protobuf JSON encoding for signing as described in https://github.com/regen-network/canonical-proto3.

As a consequence of #6030, the Cosmos SDK is moving in the direction of using protobuf's Any type for the transaction encoding and signing format. In this discussion, a number of participants have asked that we revisit the transaction signing discussion. The options that have been discussed/are available for consideration are outlined below.

It should be noted that it is theoretically possible to support more than one of the following options via an enum flag on the signature. Whether that should or should not be done is a related question.

Proposals

Feel free to suggest updates to these alternatives in the comments

(1) Protobuf canonical JSON

The official proto3 spec defines a canonical mapping to JSON. This is not really deterministic, however, so we define a canonical encoding on top of that using https://gibson042.github.io/canonicaljson-spec/.

Pros:

• By including field names, JSON is more self-describing than proto and is somewhat human-readable
• Can prevent certain user errors made when manually copying proto definitions or if proto definitions are changed between versions (shouldn't be done)
• Relatively easy migration for existing signing tools like the Ledger app
• Causes transaction verifiers to reject unknown fields which is generally correct

Cons:

• Converting to another format for signing may introduce transaction malleability vulnerabilities - i.e when different encoding representations map to a single signing representation. This is known to exist, at least for Timestamp, although that malleability is likely un-exploitable.
• Requires clients to have proto JSON and canonical JSON in addition to protobuf. There are very few implementations of canonical JSON (only go as far as we know) and some protobuf implementations don't implement proto JSON correctly (none of the Rust ones do for example)
• Doesn't bech32 encode addresses, pubkeys, etc. like Amino JSON
• By default, encodes fields in lowerCamelCase, i.e. some_field becomes someField
• Special treatment for well known types (e.g. Timestamp, Any, Duration, Struct) not necessarily supported well in existing libraries.
• And kind of renaming in field names leads to breaking changes of the message

(2) Protobuf canonical binary

This involves re-encoding the protobuf used for transaction encoding canonically for signing - meaning that fields must be ordered and defaults omitted. This is how Weave does signing.

Pros:

• Simpler for clients to implement, most protobuf implementations serialize things canonically
• Canonicalization could prevent a certain small class of user errors (if the wrong protobuf definition was used to encode a message)
• Renaming fields is a non-breaking change (as long as semantics don't change)
• Causes transaction verifiers to reject unknown fields which is generally correct

Cons:

• Introduces a subtle transaction malleability vulnerability if modules attempt to use proto2 semantics - i.e. interpret null/omitted differently from default/zero
• Not all protobuf implementations serialize things canonically and may require an additional canonicalization layer
• Doesn't prevent user errors made to reordering fields when copying or modifying proto files (like JSON does), but this should be much less of an issue using Any (because the full type URL is included and oneof's are not copied manually) and breaking change checkers like Buf/Prototool
• Hard to implement in the Ledger app for many/arbitrary message types

(3) Protobuf binary as encoded in transaction

This simply uses the protobuf encoding as broadcast in the transaction. This becomes a little easier for both signature creation and verification because of Any (although it could be done without Any too). Because Any wraps the raw bytes of the sdk.Msg, it is pretty easy to use these same exact bytes for signing and verification and only require that SignDoc itself is encoded canonically, rather than every Msg 

Pros:

• Simpler for clients to implement than even (2). All implementations should pretty much just work
• Is not vulnerable to proto2 semantics transaction malleability as (2)

Cons:

• Same as (2) in not preventing user errors related to manually copying or tampering with proto definitions, although less of an issue with Any
• Does not cause transaction verifiers to reject unknown fields (unlike 1, 2 & 4) which may lead to unexpected behavior for comments

(4) Amino JSON

This is how tx's are signed currently. The reason this is under consideration is because breaking Amino JSON signing would break many clients especially the ledger app. Transactions could still be encoded with protobuf and the /tx/encode endpoint could accept Amino JSON and return protobuf rather than amino binary for tx broadcasting - some upfront work would be required to enable this but it is possible

Pros:

• Doesn't break existing clients right away
• Doesn't introduce new transaction malleability issues that weren't already there
• Bech32 encodes addresses, pubkeys, etc. for better readability than (1)
• Causes transaction verifiers to reject unknown fields which is generally correct

Cons:

• Delays the deployment of protobuf signing, although Amino JSON signing could be enabled via a flag on Signature so maybe this is a non-issue
• Requires an additional layer on top of protobuf with information that isn't conveyed by the protobuf schema and so far lacks extensive library support

(5) Custom Proto JSON

Extend (1) to support custom encoding of certain types like bech32 addresses

Pros:

• The same benefits as Amino JSON for signing with embedded devices but derived from .proto files, i.e. human readable bech32 addresses and keys
• An easier migration path than (1)
• Does not try to re-use an encoding that was never designed to be deterministic
  (multiple signing algorithms?)

Cons:

• Requires a possibly even more complex custom JSON layer on top of protobuf plus extensions to protobuf field to indicate custom formatting like bech32
• Every signature verifier (on and off-chain) needs to maintain support all options (forever)

---

Related Question: Should we allow multiple signing algorithms?

Theoretically we can allow clients to use multiple signing algorithms and indicate which one they used as an enum flag on the Signature struct.

Pros:

• allows clients to sign with the method they feel are able to use and feel most secure with
• allows Amino JSON compatibility without delaying protobuf signing support

Cons:

• increases the number of vulnerability paths that need auditing
• increases maintenance requirements for supporting multiple paths* leaves too much responsibility to clients rather than making an opinionated, well-informed decision

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[aaronc]

I do want to say that after considering the above, my personal preference is to go with approach (3) (signing the raw binary) as default and to allow (4) Amino JSON for a limited period of time while clients transition.

(3) seems to offer the simplest UX for clients as well as the least number of malleability issues. Given that Any should minimize user errors from manually manipulating .proto files, I don't see that as as big of an issue as with Amino or the proto oneof approach.

Supporting (4) temporarily through a flag doesn't seem to introduce any new issues that aren't already here, and has the big benefit of not disrupting wallets, exchanges, etc. that don't have time to transition overnight.

[zmanian]

Thank you for writing this up @aaronc !

Here are the problems I see with 3.
Protobuf deserializing in embedded/wasm environments is extremely difficult. This is why we are actually avoiding protobuf in Armistice.

This basically means that you can't decode the transactions inside your Trusted Computing Base and you need to extend your trust boundaries around signing to enclude another system that will deserialize your proto bufs for you. This is going to be a disaster for practical security.

Strongly prefer 4 and 1. Because AMINO Json serialization in good enough form is pretty easy to implement and widely available. proto3 cannonical json actually looks okay but not really sure how wide spread support for this is. I think prost doesn't support this?

[aaronc]

@zmanian why is protobuf deserialization in embedded environments so hard? Have you looked at stuff like https://github.com/nanopb/nanopb? Even if that didn't work, I don't think a hand decoder should be too hard. The JSON stuff in the ledger app appears to be mostly hand coded.

I'm fine with enabling Amino JSON for compatibility, but we should have an alternative going forward that just requires .proto files.

[tarcieri]

Regarding this:

Theoretically we can allow clients to use multiple signing algorithms and indicate which one they used as an enum flag on the Signature struct.

...it's reminiscent of the alg field in JOSE JWT/JWS, which has been implicated in numerous security vulnerabilities (e.g. this recent one), namely via an ongoing history of implementation bugs which leverage an attacker-controlled alg into tricking a verifier into using the wrong algorithm.

An alternative is to make alg a property of public keys instead of signatures, ala X.509 Subject Public Key Info (SPKI).

This gives you a strong binding between an algorithm and a public key, known a priori to the verifier, and with that, a signature can name the "SPKI" (or SPKI hash)

[zmanian]

Some discussion here
https://www.reddit.com/r/rust/comments/aequik/is_there_a_no_std_compatible_protobuf_library_out/

My guess would be that nanopb would be too big for the ledger.

Adding a C deserialization library seems very much against our goals of minimizing C in the TCB.

[zmanian]

Also pretty strongly support the idea of binding the serialization system to the public key to support forward migration!

[tarcieri]

@aaronc

why is protobuf deserialization in embedded environments so hard?

The Protobuf Schema language lacks descriptions for the (maximum) size of variable-length fields.

This means it isn't possible to codegen a struct with a fixed-sized APDU-like structure which is typically used in heapless (e.g. microcontroller) environments.

Libraries which do support Protobufs in embedded environments therefore tend to work a level of abstraction below a typical Protobuf library which generates message types directly from the schema. This is also bad from a code size perspective, because it punts all of the work of size-checking the underlying fields to the end user.

[aaronc]

...it's reminiscent of the alg field in JOSE JWT/JWS, which has been implicated in numerous security vulnerabilities (e.g. this recent one), namely via an ongoing history of implementation bugs which leverage an attacker-controlled alg into tricking a verifier into using the wrong algorithm.

Okay, so that's definitely a consideration for supporting multiple algorithms. I do want to note that we're talking about supporting maybe 2-3 algorithms and none wouldn't be one of them. JWT seems to support ~20.

Also pretty strongly support the idea of binding the serialization system to the public key to support forward migration!

Would there be an option to change the binding to a newer system?

https://www.reddit.com/r/rust/comments/aequik/is_there_a_no_std_compatible_protobuf_library_out/

In that thread it seems there are at least 2 users who hacked together a way to do it. So definitely possibly, just not standardized in a library yet.

The Protobuf Schema language lacks descriptions for the (maximum) size of variable-length fields.

This means it isn't possible to codegen a struct with a fixed-sized APDU-like structure which is typically used in heapless (e.g. microcontroller) environments.

How is this any different from JSON? Strings/byte arrays have no max length in JSON either. If you had fixed sized arrays, you would need to truncate strings with either json or pb. But I'm not even sure why you'd need to do that sort of copying into a struct. From my quick glance at the ledger app source code, it seems like one of the key things it's trying to do is display info in fields to users. You could do the same thing in protobuf with zero allocation. Just iteratively navigate through the message like you would with JSON and keep track of what level of nesting you're at (which should be possible with a fixed depth). Strings should be easier to extract from protobuf because you don't need to copy to remove escape chars. And addresses need a small array to convert from bytes to bech32.

Anyway, I definitely can understand the concern of not wanting to rewrite a bunch of firmware, thus my support for keeping an Amino JSON option. But these problems with protobuf to me seem solvable...

[tarcieri]

How is this any different from JSON?

JSON and Protobufs are both problematic in this regard. There are some optimizations you can do for JSON as a proper context-free grammar, but ultimately on platforms with low memory the main one is ensuring either JSON or Protobufs have a well-known fixed-sized structure.

Just iteratively navigate through the message like you would with JSON and keep track of what level of nesting you're at

I've been working on this sort of pushdown automaton in Veriform, as it were.

For some extreme low-end embedded environments, even that sort of thing is too much (our target is a 500+MHz Cortex-A environment, whereas the problematic environments are much lower clocked Cortex-M ones with much smaller stacks)

[ethanfrey]

I want to jump in with an argument against Amino JSON. Basically, if we keep using that, we will need to support that tooling on all platforms. Much less work than binary Amino, but over 2 years there has been amazing little work by the core team to port any of Amino, and assuming all this magically happens "from the community" is wishful thinking. If we need to stay with Amino JSON, then I would say that porting client side libraries in major languages (not just JS and Rust, but say Java/Kotlin and ObjC/Swift at the minimum, ideally python and some more) comes into scope as part of this migration. If we can use tooling that already works out of the box in all these languages, than that is much easier.

The strong valid criticism I see above is that it is hard to parse Protobuf in an HSM.

My guess would be that nanopb would be too big for the ledger.
Adding a C deserialization library seems very much against our goals of minimizing C in the TCB.

I encourage you to look at the Ledger app that Juan (same dev who wrote Cosmos Ledger app) wrote for IOV: https://github.com/iov-one/ledger-iov There isn't too much code there for the parsing. Actually grabbing a few fields from a predefined protobuf format is quite easy, and doesn't require parsing arbitrary structs into memory.

It parses the Protobuf signing format we use for transactions and displays it to the users. It didn't take him that long to do it (I believe less than the original Cosmos app). I just want to say that using protobuf is not some new and untested idea, but was used for over 2 years now and on a mainnet. That IOV has no clue about business and marketing doesn't mean the code there has no technical merit. I advise you to borrow liberally.

[aaronc]

JSON and Protobufs are both problematic in this regard. There are some optimizations you can do for JSON as a proper context-free grammar, but ultimately on platforms with low memory the main one is ensuring either JSON or Protobufs have a well-known fixed-sized structure.

Okay, well sounds like this problem would only be solved by something like ASN.1 which isn't really an option. But either way sounds like we can actually deal with the memory problem with an event driven parser as opposed to decoding into a struct. So then the issue becomes code size, right?

Here I can see one potential argument in favor of JSON. JSON is self-describing so if you don't have the schema, you could still iteratively display each raw field on the JSON on a device like the ledger. To do that in protobuf, you would need to include the schema for every type or limit support to just a few types to limit code size. Is that one of the tradeoffs you're seeing @tarcieri ?

One point I will grant to Amino JSON is that it bech32 encodes addresses, pub keys, etc. Say you wanted to iteratively display every element of a JSON object in the ledger without the schema, with protobuf you would get base64 whenever you don't know the schema. Maybe that's not a hard problem to solve, but I do see it as a valid good thing about Amino JSON.

To do this with protobuf we would need a custom JSON serialization format which maybe indicated the bech32 type of bytes fields with an extension (i.e. bytes key = 1 [(cosmos_proto.bech32_type) = "valpub"]). That's maybe not a bad idea to include in the .proto files anyway, but as @ethanfrey noted the more custom work clients need to do the bigger the burden.

Anyway, I will include this as a new option (5) and update the pros and cons of the other options to reflect this discussion.

I do still think there is an elegance to approach (3) and if it did work for embedded devices, maybe following the example in https://github.com/iov-one/ledger-iov, that might make everyone's lives easier.

[zmanian]

This self describing nature of Javascript allows the Cosmos app to work any number of chains out of the box.

[ethanfrey]

This self describing nature of Javascript allows the Cosmos app to work any number of chains out of the box.

That and the fact that they all use a superset of the schema the ledger app understands.

It checked for eg. .msgs[0].type == "cosmos-sdk/send" and then .msgs[0].data.amount[0].amount for example. If we encoded it in JSON but with different keys, this wouldn't work.

Note that with option (3), protobuf is also self-describing to a degree. At least you can check the type they claim and will not mix up cosmos-sdk/send and cosmos-sdk/burn. It will not know how to display any type that it was not compiled for. But then again the JSON parser wouldn't either - it can just display the raw JSON to the user. Is that mode actually used (display raw JSON of sign bytes to end user). If this is a common use-case (pass raw bytes to the end user to interpret), then there is a big bonus for JSON. Otherwise, I don't see how self-describing helps an app that checks hardcoded fields - it just helps avoid mixups (like the Any type field does)