Skip to content

Conversation

@tarcieri
Copy link
Member

@tarcieri tarcieri commented Oct 2, 2021

GroupEncoding uses a fixed-width Repr whereas SEC1 is variable-width with respect to compressed points versus the identity.

This change allows 33-bytes of zeroes to be used as the identity. Technically that's not a valid SEC1 encoding: the SEC1 encoding for the identity is 1-byte: 0x00. However, this is the best we can do with a fixed-width encoding.

See also: #443 and #444

`GroupEncoding` uses a fixed-width `Repr` whereas SEC1 is variable-width
with respect to compressed points versus the identity.

This change allows 33-bytes of zeroes to be used as the identity.
Technically that's not a valid SEC1 encoding: the SEC1 encoding for the
identity is 1-byte: 0x00. However, this is the best we can do with a
fixed-width encoding.

See also: #443 and #444
Comment on lines +135 to +142
EncodedPoint::from_bytes(bytes)
.map(|point| CtOption::new(point, Choice::from(1)))
.unwrap_or_else(|_| {
// SEC1 identity encoding is technically 1-byte 0x00, but the
// `GroupEncoding` API requires a fixed-width `Repr`
let is_identity = bytes.ct_eq(&Self::Repr::default());
CtOption::new(EncodedPoint::identity(), is_identity)
})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this is a branch, but it's one which only occurs with respect to the SEC1 tag.

The important part to ensure is actually constant-time is the point decoding, which happens regardless of which branch is taken through the initial SEC1 decoding.

@codecov-commenter
Copy link

Codecov Report

Merging #446 (d00e74e) into master (3b1d44a) will increase coverage by 0.45%.
The diff coverage is 71.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
+ Coverage   64.58%   65.03%   +0.45%     
==========================================
  Files          28       28              
  Lines        3586     3598      +12     
==========================================
+ Hits         2316     2340      +24     
+ Misses       1270     1258      -12     
Impacted Files Coverage Δ
k256/src/arithmetic/projective.rs 77.04% <0.00%> (ø)
p256/src/arithmetic/projective.rs 77.30% <50.00%> (+0.41%) ⬆️
k256/src/arithmetic/affine.rs 83.63% <78.57%> (+6.18%) ⬆️
p256/src/arithmetic/affine.rs 90.13% <78.57%> (+4.02%) ⬆️

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 3b1d44a...d00e74e. Read the comment docs.

@tarcieri tarcieri merged commit b66283c into master Oct 2, 2021
@tarcieri tarcieri deleted the k256+p256/handle-groupencoding-identity branch October 2, 2021 19:31
@tarcieri tarcieri mentioned this pull request Dec 14, 2021
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.

3 participants