Skip to content

encode pubkeys as hex, not base64

Luke Champine requested to merge spk-json into master

While working on the Ledger Sia app, I discovered that SiaPublicKeys use base64 in their JSON encoding. This is problematic for the Ledger app because the fonts on the Ledger Nano S represent l and I identically. A malicious computer could therefore fool the user by showing an l where the Nano S actually sent an I, or vice versa. Hex does not suffer from this problem since none of the 16 digits are homoglyphs. We can therefore avoid the problem by encoding SiaPublicKeys as hex everywhere.

Specifically, this PR changes the JSON encoding to match the String encoding, e.g. ed25519:abd0121dead123beef.... (As an added bonus, this encoding is also more compact.) UnmarshalJSON will support both encodings, of course, to preserve compatibility. Old clients will not recognize the new format, but JSON encodings are never sent over the network, so this should cause minimal breakage (only in the event of an old client loading new persist, or certain API calls being made to an old client).

EDIT: another question is whether we should switch JSON signatures to from b64 to hex as well. I consider this to be much less important, for two reasons. First, pubkeys are often seen by the user (e.g. to identify hosts), whereas signatures are not, so the representation is less important. Second, signatures are not vulnerable to the same l/I problem that affects pubkeys: you never need to compare a signature on the Ledger with a signature on a computer. The signature is either valid or it isn't. Still, just because I'm not aware of any attacks today doesn't mean no attacks are possible. Personally I'd be happy to eliminate base64 everywhere, but I'm not convinced it's worth it for signatures, since base64 significantly reduces their size (89 chars, vs. 128 in hex).

Edited by Luke Champine

Merge request reports