ECDSA with Public Key Recovery

[julmb]

This is a minimal set of changes that would fix #41 for my use case.

The primary use case for public key recovery seems to be cryptocurrencies. I have tried my best to make this as general as possible without bringing in any specifics of that use case. For example, many cryptocurrencies use secp256k1, where it is nearly impossible for the offset flag to be set, so it is often not even calculated.

I decided to go with an ExtendedSignature type rather than just a tuple. I also decided to represent the recovery information as two Bool fields rather than one numeric field consisting of two packed bits. The advantage of having two Bool fields are stronger types (guaranteeing exactly two bits of information), and easier manipulation of the individual bits (no bit operations like xor are required). This will also be useful in a potential future PR for signature normalization.

This is a draft PR for now since there are still several open questions:

1. Should I add tests? I am not sure if there are any established test suites for this, so the best I could do is calculate the recovery information with the current implementation. It would help establish correctness of the implementation, but it would at least help protect from regressions.
2. Should I add an algorithm that actually performs public key recovery (rather than just outputting the recovery information as part of the signing process). This would enable some closed-loop tests where we could test if the public key that was used can actually be recovered. However, I am not sure if I am knowledgeable enough to do this.
3. The current module contains several variations for each function (signDigestWith, signWith, signDigest, sign in decreasing order of generality). The recovery information adds one more variant. If we want to have all options, this would amount to 2³ = 8 functions in total. I can easily do this, but I do not know if it is a good idea. In an ideal world, maybe it would be better to have a small helper function that takes care of the generation of k, as well as requiring users to do the transformation from message into digest. However, these would be breaking changes so it seems like we are stuck with many different versions of the same function. I do not know if we should have all 8 versions or only a subset thereof.
4. Should extended signatures and public key recovery be added to the Crypto.PubKey.ECDSA module aswell?

[julmb]

From https://crypto.stackexchange.com/questions/18105/how-does-recovering-the-public-key-from-an-ecdsa-signature-work and https://www.secg.org/sec1-v2.pdf#subsubsection.4.1.6 it seems like for curves with cofactor > 1, more than one bit of information is needed for the X coordinate.

I have replaced the offset field in ExtendedSignature with an index field that should be enough to recover the public key for curves with cofactor > 1.

[julmb]

Oh boy, it looks like I bit off more than I can chew. Apparently, parity is not so simple for curves over binary fields. In this context, point negation is defined as:

pointNegate CurveF2m{} (Point x y) = Point x (x `addF2m` y)

Which means that if x has an unset least significant bit, then the parity of y does not change. This means that the naive odd y check cannot reliably tell a point and its complement point apart.

We will probably need something like this:

isPointOdd :: Curve -> Point -> Maybe Bool
isPointOdd _ PointO = Nothing
isPointOdd (CurveFP _) (Point _ y) = Just $ odd y
isPointOdd (CurveF2m (CurveBinary fx _)) (Point x y) = odd <$> divF2m fx y x

Together with some reasoning that this will never go wrong in valid signature generation scenarios.

[julmb]

Alright, I added:

1. Function pointDecompose that extracts fully general public key recovery information from a point on curves over both prime and binary fields. Adapted from SEC 1: Elliptic Curve Cryptography, Version 2.0, section 2.3.3.
2. Function pointCompose which does the inverse, assembling these pieces of information back into a point. Adapted from SEC 1: Elliptic Curve Cryptography, Version 2.0, section 2.3.4. This required a function to solve equations of the form x² + x = a over binary fields, which I got from https://math.stackexchange.com/a/4392472/38567. My understanding of this is a little shaky but it makes sense and it seems to work.
3. The actual public key recovery function as well as round-trip tests.

I really only need (1) for my application, but (2) and (3) serve as verification and allow for automated round-trip tests. Indeed, I found several gaps in my understanding when trying to implement (2) and (3). That being said, if they are deemed too much or too risky, I would be okay only having (1) accepted in this PR.

Considerations:

1. All sign* functions now calculate the extended signature.
   1. For curves over prime fields, this consists of a single bit check as well as calculating divMod instead of just mod in one place. The difference in performance should be negligible.
   2. For curves over binary fields, it is the same as above, except there is also a division y / x over the binary field. Division over binary fields involves GCD and can even fail. However, the case x = 0 is treated separately, and the other failure case involves gcd fx x /= 1, which from my understanding can only happen if x is denormalized. Since Point x y is the result of a pointMul operation, this should not be the case. The test suite also shows no failures like this. Still, it is worth looking out for as we do not want to introduce Nothing results when the same function would return Just before the change.
2. The extended signature essentially consists of (div x n, odd y), where Point x y is the result of pointMul curve k g. I do not know if these extra bits of information leak parts of k. I am assuming at least the parity of y to not be an issue since that is widely used in cryptocurrency transactions. However, it would be nice if someone more knowledgeable in cryptography could give more insight here. In case these pieces of information are considered sensitive, we should add additional documentation onto the ExtendedSignature type to advise users.

[julmb]

1. Should I add tests? I am not sure if there are any established test suites for this, so the best I could do is calculate the recovery information with the current implementation. It would help establish correctness of the implementation, but it would at least help protect from regressions.
2. Should I add an algorithm that actually performs public key recovery (rather than just outputting the recovery information as part of the signing process). This would enable some closed-loop tests where we could test if the public key that was used can actually be recovered. However, I am not sure if I am knowledgeable enough to do this.
3. The current module contains several variations for each function (signDigestWith, signWith, signDigest, sign in decreasing order of generality). The recovery information adds one more variant. If we want to have all options, this would amount to 2³ = 8 functions in total. I can easily do this, but I do not know if it is a good idea. In an ideal world, maybe it would be better to have a small helper function that takes care of the generation of k, as well as requiring users to do the transformation from message into digest. However, these would be breaking changes so it seems like we are stuck with many different versions of the same function. I do not know if we should have all 8 versions or only a subset thereof.
4. Should extended signatures and public key recovery be added to the Crypto.PubKey.ECDSA module aswell?

Revisiting the open questions from the original post:

1. Done. Implemented round-trip tests via public key recovery function.
2. Done. Added recoverDigest and recover functions.
3. I decided to add signExtendedDigestWith and signExtendedDigest, but not the convenience versions that perform the hashing. The rejection sampling logic for picking k is probably complex enough to warrant having a function version, while the hashing can easily be done at the call site.
4. I have not done this as I do not know what purpose the Crypto.PubKey.ECDSA module has compared to the more versatile Crypto.PubKey.ECC.ECDSA module.

As far as I am concerned, this PR could be merged, given appropriate review, since I am by no means an expert in cryptography and I do not want to introduce any vulnerabilities.

[kazu-yamamoto]

I believe that a pair of take and iterate should be replaced with replicate.

[julmb]

Here we are defining a list of iterated squares:

[fx, squareF2m fx, squareF2m (squareF2m fx), squareF2m (squareF2m (squareF2m fx)), ...]

And then we take the sum (foldr addF2m 0) of a prefix of that list. I do not know how to do this with replicate, since that would just give us the last item of that prefix.

[kazu-yamamoto]

My bad. I took it as repeat.

[kazu-yamamoto]

I would suggest to use `unsafeShiftR` 1 instead of div.

[julmb]

I can do that. I am assuming (!>>.) would also be okay?

[kazu-yamamoto]

It is not provided by old versions, sigh.
So, I recommend unsafeShiftR.

[julmb]

Ah, I see, that is unfortunate. I have changed it to use unsafeShiftR.

[kazu-yamamoto]

The following style is more readable to me:

quadraticF2m fx a
  | traceF2m fx a == 0 = Just $ halfTraceF2m fx a
  | otherwise = Nothing

[kazu-yamamoto]

I have confirmed that this PR does not give effect to my projects. 😀

[kazu-yamamoto]

Now LGTM!

[kazu-yamamoto]

I'm sure that the reason why CI failed is not relating to this PR.
So, I have merged this PR.
Thank you for your contribution!

[julmb]

Thank you for merging!

As for the CI failure, I had the same errors when I tried to run the tests locally. I had to add a cabal.project with these contents to make it work:

packages: .
tests: True

I do not understand what tests: True does or why it is necessary for crypton. I never needed it in my own projects to run tests.

[kazu-yamamoto]

Thanks.
I've pushed 81f2b0f.