Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add legacy feature with AffineCoordinates trait #65

Closed
wants to merge 1 commit into from

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Jul 7, 2020

Protocols like ECDSA need access to affine coordinates: the x-coordinate for ECDSA itself, but also the y-coordinate for things like recovering public keys from ECDSA signatures (as seen in Ethereum).

This commit adds support for accessing affine coordinates for use in these protocols, taking into account some feedback from the previous attempt to do this (#50):

  • Trait is placed inside a legacy module with comments strongly suggesting it should only be used for implementing legacy protocols which explicitly need coordinate access
  • Feature is hidden from https://docs.rs to discourage (mis)use
  • Coordinates are represented using a FieldElementBytes type alias which is also defined in the legacy module

/cc @nickray

Protocols like ECDSA need access to affine coordinates: the x-coordinate
for ECDSA itself, but also the y-coordinate for things like recovering
public keys from ECDSA signatures (as seen in Ethereum).

This commit adds support for accessing affine coordinates for use
in these protocols, taking into account some feedback from the previous
attempt to do this (#50):

- Trait is placed inside a `legacy` module with comments strongly
  suggesting it should only be used for implementing legacy protocols
  which explicitly need coordinate access
- Feature is hidden from https://docs.rs to discourage (mis)use
- Coordinates are represented using a `FieldElementBytes` type alias
  which is also defined in the `legacy` module
@tarcieri tarcieri requested review from tuxxy and str4d July 7, 2020 16:35
@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2020

Codecov Report

Merging #65 into master will decrease coverage by 0.09%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
- Coverage   81.46%   81.37%   -0.10%     
==========================================
  Files          12       12              
  Lines        1392     1428      +36     
==========================================
+ Hits         1134     1162      +28     
- Misses        258      266       +8     
Impacted Files Coverage Δ
k256/src/arithmetic.rs 83.78% <0.00%> (-1.15%) ⬇️
p256/src/arithmetic.rs 81.81% <0.00%> (-1.12%) ⬇️
k256/src/arithmetic/scalar.rs 90.81% <0.00%> (+3.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd3516f...1f1eaf7. Read the comment docs.

@tarcieri tarcieri force-pushed the legacy-affine-coordinate-access branch 2 times, most recently from 738a829 to 1f1eaf7 Compare July 11, 2020 15:55
@tarcieri
Copy link
Member Author

tarcieri commented Jul 11, 2020

I've been poking at using this in some local branches and thought I'd spell out the immediate concrete use cases for each of the coordinates.

ECDSA signing

Needs access to the x-coordinate.

@nickray's quick 'n' dirty ECDSA signing implementation (#57) serializes the field element to an integer-as-bytestring and then reduces it to an element of the Scalar field for use in subsequent operations:

https://github.com/RustCrypto/elliptic-curves/pull/57/files#diff-e6b3b4deef9b63467f2493d3bb13a7b2R58-R65

        // Geometrically, x-coordinate is an element of the base field.
        // to_bytes lifts this to a big-endian integer.
        // This integer is then reduced to an element of the scalar field (n < p).
        let x_coordinate: [u8; 32] = ephemeral_point.x.to_bytes();
        let r = Scalar::from_hash(x_coordinate);

ECDSA public key recovery

Needs access to the y-coordinate to compute the "recovery ID", but like point compression only really needs to know if the y-coordinate is odd.

@tarcieri
Copy link
Member Author

I prefer RustCrypto/signatures#96 to this PR for the purposes of ECDSA.

@tarcieri
Copy link
Member Author

The elliptic-curve crate has been moved to the https://github.com/RustCrypto/traits repo.

If anyone is still interested in this feature, this PR needs to be reopened there.

@tarcieri tarcieri closed this Jul 17, 2020
@tarcieri tarcieri deleted the legacy-affine-coordinate-access branch July 17, 2020 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants