Signed messages should return a signature that has recoverable public key

[aulneau]

Talking with @pradel about how to use the new signed messages feature in thing like ceramic.network, we realized that the signatures the wallet is currently sending down to the apps are ones that make it impossible to recover the public key from the signature.

The wallet is using signECDSA versus signWithKey.

Sign with Key allows for recoverable public keys from signatures:

export function signWithKey(privateKey: StacksPrivateKey, input: string): MessageSignature {
  const [rawSignature, recoveryParam] = signSync(input, privateKey.data.slice(0, 32), {
    canonical: true,
    recovered: true,
  });
  const signature = Signature.fromHex(rawSignature);
  const coordinateValueBytes = 32;
  const r = leftPadHexToLength(signature.r.toString(16), coordinateValueBytes * 2);
  const s = leftPadHexToLength(signature.s.toString(16), coordinateValueBytes * 2);

  if (recoveryParam === undefined || recoveryParam === null) {
    throw new Error('"signature.recoveryParam" is not set');
  }
  const recoveryParamHex = intToHexString(recoveryParam, 1);
  const recoverableSignatureString = recoveryParamHex + r + s;
  return createMessageSignature(recoverableSignatureString);
}

Where as signECDSA does not:

export function signECDSA(
  privateKey: string,
  content: string | Buffer
): {
  publicKey: string;
  signature: string;
} {
  const contentBuffer = content instanceof Buffer ? content : Buffer.from(content);
  const publicKey = getPublicKeyFromPrivate(privateKey);
  const contentHash = hashSha256Sync(contentBuffer);
  const signature = signSync(contentHash, privateKey);

  return {
    signature: utils.bytesToHex(signature),
    publicKey,
  };
}

We can also see that Marvins example implementation correctly uses signWithKey https://github.com/MarvinJanssen/stx-signed-structured-data/blob/4c5714e2b9957db0037863d15c3640a0524abe20/scripts/deps.ts#L28

I'd suggested shipping a change to where you switch out these functions.

[markmhendrickson]

Thanks for this input 🙏 Is this a progressive enhancement of sorts, or should this be in place before developers start using message signing generally?

[aulneau]

@markmhx i think it is not a progressive enhancement, but should be a requirement. perhaps @pradel can chime in too, or @MarvinJanssen

[pradel]

@markmhx yeah this should be shipped asap before apps start using it in prod (it would be a breaking change otherwise).

[markmhendrickson]

@aulneau @MarvinJanssen Mind helping us test the build here as well? #2420

@pradel says it checks out 💪

[MarvinJanssen]

Good find @aulneau & @pradel!

Quick observation. signWithKey returns the signature in vrs order. publicKeyFromSignature likewise uses/expects this order. However, Clarity functions secp256k1-recover? and secp256k1-verify expect the signature in rsv order. Since the main focus of (SIP018) signed messages was to (at some point) post them to the chain, my reference library returns the signature in rsv order:

https://github.com/MarvinJanssen/stx-signed-structured-data/blob/4c5714e2b9957db0037863d15c3640a0524abe20/scripts/deps.ts#L29

return Buffer.from(data.slice(2) + data.slice(0, 2), 'hex');

The SIP document also assumes rsv order:

• https://github.com/MarvinJanssen/sips/blob/feat/signed-structured-data/sips/sip-018/sip-018-signed-structured-data.md#definitions
• https://github.com/MarvinJanssen/sips/blob/feat/signed-structured-data/sips/sip-018/sip-018-signed-structured-data.md#typescript-implementation

I therefore think it makes sense to return the signature in rsv order lest we confuse end-developers.

[beguene]

@aulneau The wallet return the ECDSA signature and the publicKey in an explicit way like this

So it's easily verifiable and it's clear for the developer that he can just verify the signature with the pubKey (with no extra step) and the message hash. Anyone who has used or is familiar with cryptographic primitives for signing messages and verifying signature will be cognisant of this even without any blockchain knowledge.

Besides
secp256k1-verify from clarity expects an ECDSA signature and a publicKey.

(secp256k1-verify message-hash signature public-key)

https://docs.stacks.co/en/write-smart-contracts/language-functions#secp256k1-verify

and the corresponding code in the stack blockchain confirm this

pub fn secp256k1_verify(
    message_arr: &[u8],
    serialized_signature_arr: &[u8],
    pubkey_arr: &[u8],
)

https://github.com/stacks-network/stacks-blockchain/blob/26bfd5fcdc1f25106288f18ced05290b569f6abb/stacks-common/src/util/secp256k1.rs#L396
which internally calls

int secp256k1_ecdsa_verify(const secp256k1_context* ctx, const secp256k1_ecdsa_signature *sig, const unsigned char *msghash32, const secp256k1_pubkey *pubkey)

(via ffi binding )
https://github.com/bitcoin-core/secp256k1/blob/aa5d34a8fe99b1f69306be20819f337dbd3283db/src/secp256k1.c#L442

secp256k1_ecdsa_verify(self.ctx, sig.as_c_ptr(), msg.as_c_ptr(), pk.as_c_ptr()) 

Note that signECDSA returns in the signature in vrs order (via the DER format) which is also the order expected by secp256k1-verify

In the case of a blockchain like Bitcoin or Stack with restricted access to resources (space and compute) it makes sense to encode both the signature and the publicKey in a single 'recoverable signature'. But for a web app or backend service that has no such restriction I think this is unnecessary optimisation and complexity.

[janniks]

I believe Clarity's secp256k1-verify is RSV:

• uses first 64 bytes for verification, while neglecting 65th bytes (V or more correctly, the recovery id)
• 65th byte, if present, needs to be <= 3

[aulneau]

But for a web app or backend service that has no such restriction I think this is unnecessary optimisation and complexity.

I disagree with this statement personally -- this is a significant assumption on use case. @pradel gave a perfectly reasonable example of a real use case within 1 day of this feature being released, and @MarvinJanssen gave additional context on what the stacks-blockchain and SIP-018 expects too.

I think if the wallet does not provide a recoverable signature in RSV format, it would not be SIP-018 compliant, unless I'm misunderstanding it -- would love to hear from @MarvinJanssen on this.

@beguene I'd love to understand where the complexity is around this? Seems like a very reasonable request imo.

Note that signECDSA returns in the signature in vrs order (via the DER format) which is also the order expected by secp256k1-verify

Can you go into more detail about what you mean by this? this does not seem to be the case (see @janniks and @MarvinJanssen's comments)

[MarvinJanssen]

@janniks and @aulneau are correct. It appears the Stacks blockchain uses vrs order for transactions and rsv order for the Clarity functions. See also this comment: hirosystems/stacks.js#1263 (comment)

I think if the wallet does not provide a recoverable signature in RSV format, it would not be SIP-018 compliant, unless I'm misunderstanding it -- would love to hear from @MarvinJanssen on this.

Not just SIP018 but it also makes it incompatible with the Clarity functions per the above comment. Additionally, without the recovered: true option, signSync drops the recovery byte:
https://github.com/paulmillr/noble-secp256k1/blob/450ebc76ef73ff375c8ce8252468d3a04e87c7f5/index.ts#L1148-L1157

[beguene]

My bad indeed secp256k1-verify function is rsv and takes a recoverable signature 🤦
Therefore it then makes sense then to send this format back to the dApp to be easily reused for smart contract.
Sorry for the confusion.

[jruffer]

Will this be fixed in all the places it is called...it's everywhere... hirowallet, stacks.js, blockstacks, stacks-wallet-web etc.