Change the app link signature encoding to hex

[RiccardoM]

Feature description

Currently when we verify an application link result, we assume that the plain text value has been encoded using UTF-8 (see here). Unfortunately, this currently makes it impossible to sign something using the Ledger or any common wallet due to the fact that they do not allow to sign arbitrary strings.

Implementation proposal

In order to fix this issue, I think we should adopt the same strategy that we currently use for the chain links: require that the plain text is encoded using the hex format (see here). This would make it easier for developers to use any arbitrary value as the plain text, even a full transaction that is then signed using the Ledger.

Note that this change requires a change to all data sources as well to make sure they decode the plain text as a HEX instead of UTF-8 before verifying the signature.

Please @bragaz @dadamu let me know what you think about this before we can make an ADR.

[dadamu]

@RiccardoM
The chain link's plain text to sign is also UTF-8 encoded string of address as default, it can be changed to other custom string without UTF-8 encoded so it has same problem in chain link. I think chain link plain text should be also changed to hex-encoded if following this issue.

However, I think it is better to remain UTF-8 encoded. Is it able to force to convert the plain text to UTF-8 string before signing?

[RiccardoM]

@RiccardoM The chain link's plain text to sign is also UTF-8 encoded string of address as default, it can be changed to other custom string without UTF-8 encoded so it has same problem in chain link. I think chain link plain text should be also changed to hex-encoded if following this issue.

You're right, I just checked here and it assumes the plain text is UTF-8 encoded

However, I think it is better to remain UTF-8 encoded. Is it able to force to convert the plain text to UTF-8 string before signing?

Unfortunately this issue is not about encoding only, but also about the signature verification process. To better understand this, let's assume we keep the UTF-8 encoding as it is right now, and that we want to create an app/chain link using the Ledger as a signer.

As per design, the Ledger can sign only Cosmos transactions. You give to it a transaction as a byte array and it signs it. It doesn't allow the signature of arbitrary data. So to build an app/chain link we need to provide it with a Cosmos transaction as a byte array. What would happen is:

1. I give the Ledger a Cosmos transaction as a byte array
2. I encode the signed bytes as UTF-8 and the signature as hex

To be honest, I don't think that encoding the transaction to a UTF-8 string would always work. We might have some problems here due to the fact that UTF-8 does not support all characters. Reading here we get that:

ISO Latin-1, a subset of UNICODE, is not a subset of UTF-8.

Which means that if the transaction contains a particular memo field with ISO Latin-1 characters, it will not get converted to a UTF-8 byte array properly.

For this reason, I think we should convert both app and chain links to require HEX-encoded plain texts instead in order to support even Ledger signers

[dadamu]

@RiccardoM
Thanks for explaining. I am clear now. I agree with changing plain text to hex encoded before signing.

[leobragaz]

@RiccardoM @dadamu From what I've read, this problem extend also to chain-link plain-text right?
If so, I guess we should open an ADR for both and standardise the behaviour of the two in order to accept only hex-encoded plain text. Make sense?

[RiccardoM]

@bragaz I think a single ADR for both of them is great instead of having two different ADRs. Whoever wishes can go on

[dadamu]

@RiccardoM I can draft it later.

[leobragaz]

@bragaz I think a single ADR for both of them is great instead of having two different ADRs. Whoever wishes can go on

@RiccardoM Yes I meant one 👍

[dadamu]

@RiccardoM @bragaz Deeply re-thinking about it when writing the ADR. If we change the plain text of the old links which is UTF-8 encoded to hex before verifying the signature, the old links are all invalid since their signatures are simply signed with UTF-8 string bytes. So, all of old links must be re-created to sign the signature in hex bytes.

How about we force the plain text to be hex-encoded string to the new link, then remain the old links used the old proof since they have already passed the verification.
For instance, the signing process of chain link in CLI tool will be like:

// generateChainLinkJSON returns build a new ChainLinkJSON intance using the provided mnemonic and chain configuration
func generateChainLinkJSON(mnemonic string, chain chainlinktypes.Chain) (profilescliutils.ChainLinkJSON, error) {
	
       ...

        // force to change the value to hex-encoded string as plain text automatically
	plainText := hex.EncodeToString([]byte(value))
	sig, pubkey, err := keyBase.Sign(keyName, []byte(plainText))
	if err != nil {
		return profilescliutils.ChainLinkJSON{}, err
	}

	return profilescliutils.NewChainLinkJSON(
		profilestypes.NewBech32Address(addr, chain.Prefix),
		profilestypes.NewProof(pubkey, hex.EncodeToString(sig), plainText),
		profilestypes.NewChainConfig(chain.Name),
	), nil
}

What do you think @RiccardoM, @bragaz

[RiccardoM]

@dadamu Changing from UTF-8 to HEX for new links shouldn't be a problem to old links. Once a link is created, the signature is checked only during the creation process and never after that. If we want to keep things consistent on-chain, then we can write a migration script that takes all the currently stored links and transforms them into HEX-encoded values. This can be done as follow:

1. Iterate over all the chain and application links
2. Convert the plain text by doing link.PlainText = hex.EncodeToString([]byte(link.PlainText))

This will make all the links backward compatible as well.