Skip to content
This repository has been archived by the owner on Dec 23, 2020. It is now read-only.

Support lower-s form conversion for ECDSA signatures #215

Open
cheeseandcereal opened this issue Dec 17, 2019 · 3 comments
Open

Support lower-s form conversion for ECDSA signatures #215

cheeseandcereal opened this issue Dec 17, 2019 · 3 comments

Comments

@cheeseandcereal
Copy link

cheeseandcereal commented Dec 17, 2019

Because of the prevalence of libsecp256k1 with bitcoin/ethereum, more and more people are using those libraries for ecdsa. When verifying signatures, libsecp256k1 enforces a signature's s to be in lower-s form to essentially help protect signature malleability.

See the comment in its include header for more info:

https://github.com/bitcoin-core/secp256k1/blob/d644dda5c9dbdecee52d1aa259235510fdc2d4ee/include/secp256k1.h#L483-L513

Because of this, I would propose a feature be added that either converts the s value of ECSignatures to their lower-s form automatically upon signature creation (if necessary), or at least provide a public api function to do so for easier compatibility with creating signatures that can be consistently verified by these libraries.

Here's some pseudocode that would do the latter option:

ECSignature normalizeECSignature(ECSignature currentSignature, ECDomainParameters curveParams) {
  BigInt normalizedS = currentSignature.s;
  if (normalizedS.compareTo(curveParams.n >> 1) > 0) {
    normalizedS = curveParams.n - normalizedS;
  }
  return ECSignature(currentSignature.r, normalizedS);
}
@stevenroose
Copy link
Member

Sure, that makes sense. I would have a method on ECSignature called normalize() that just normalizes the signature so no new signature has to be created.

@cheeseandcereal
Copy link
Author

cheeseandcereal commented Dec 18, 2019

An ECSignature doesn't contain data about its ECDomainParameters which are required for doing this calculation. Would a better alternative be providing an optional normalize boolean when actually creating the signature? Or should a normalize() function be added to ECSignature which simply requires taking in an ECDomainParameters object? (Or as a 3rd alternative, simply add a reference to the ECDomainParameters on the ECSignature object itself when creating it, so it can have that information already available to call a normalize() function with no additional params)

@stevenroose
Copy link
Member

That's a good point. I would not add a ref to the signature for sure.

Of the other two options, it seems like we could do both. normalize(ECDomainParameters) seems fine, but a bool to normalize from the start seems better. They can both exist, though.

Btw, I'm about to move this project to a new repo, so keep your PR branch around in case you'd have to re-do.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants