Skip to content

k256+p256: handle identity point in GroupEncoding #446

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.

Sign up for GitHub

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

Merged
tarcieri merged 1 commit into from
Oct 2, 2021
Merged

k256+p256: handle identity point in GroupEncoding #446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions k256/src/arithmetic/affine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,15 @@ impl GroupEncoding for AffinePoint {
type Repr = CompressedPoint;

fn from_bytes(bytes: &Self::Repr) -> CtOption<Self> {
let tag = bytes[0];
let y_is_odd = tag.ct_eq(&sec1::Tag::CompressedOddY.into());
let is_compressed_point = y_is_odd | tag.ct_eq(&sec1::Tag::CompressedEvenY.into());
Self::decompress(FieldBytes::from_slice(&bytes[1..]), y_is_odd)
.and_then(|point| CtOption::new(point, is_compressed_point))
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)
})
Comment on lines +135 to +142
Copy link
Member Author

@tarcieri tarcieri Oct 2, 2021

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.

.and_then(|point| Self::from_encoded_point(&point))
}

fn from_bytes_unchecked(bytes: &Self::Repr) -> CtOption<Self> {
Expand All @@ -145,7 +149,10 @@ impl GroupEncoding for AffinePoint {
}

fn to_bytes(&self) -> Self::Repr {
CompressedPoint::clone_from_slice(self.to_encoded_point(true).as_bytes())
let encoded = self.to_encoded_point(true);
let mut result = CompressedPoint::default();
result[..encoded.len()].copy_from_slice(encoded.as_bytes());
result
}
}

Expand Down Expand Up @@ -241,7 +248,7 @@ mod tests {
use super::AffinePoint;
use crate::EncodedPoint;
use elliptic_curve::{
group::prime::PrimeCurveAffine,
group::{prime::PrimeCurveAffine, GroupEncoding},
sec1::{FromEncodedPoint, ToEncodedPoint},
};
use hex_literal::hex;
Expand Down Expand Up @@ -300,4 +307,15 @@ mod tests {
let basepoint = AffinePoint::generator();
assert_eq!((-(-basepoint)), basepoint);
}

#[test]
fn identity_encoding() {
// This is technically an invalid SEC1 encoding, but is preferable to panicking.
assert_eq!([0; 33], AffinePoint::identity().to_bytes().as_slice());
assert!(bool::from(
AffinePoint::from_bytes(&AffinePoint::identity().to_bytes())
.unwrap()
.is_identity()
))
}
}
4 changes: 2 additions & 2 deletions k256/src/arithmetic/projective.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl GroupEncoding for ProjectivePoint {
type Repr = CompressedPoint;

fn from_bytes(bytes: &Self::Repr) -> CtOption<Self> {
<AffinePoint as GroupEncoding>::from_bytes(bytes).map(|point| point.into())
<AffinePoint as GroupEncoding>::from_bytes(bytes).map(Into::into)
}

fn from_bytes_unchecked(bytes: &Self::Repr) -> CtOption<Self> {
Expand All @@ -307,7 +307,7 @@ impl GroupEncoding for ProjectivePoint {
}

fn to_bytes(&self) -> Self::Repr {
CompressedPoint::clone_from_slice(self.to_affine().to_encoded_point(true).as_bytes())
self.to_affine().to_bytes()
}
}

Expand Down
33 changes: 26 additions & 7 deletions p256/src/arithmetic/affine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,17 @@ impl DecompressPoint<NistP256> for AffinePoint {
impl GroupEncoding for AffinePoint {
type Repr = CompressedPoint;

/// NOTE: not constant-time with respect to identity point
fn from_bytes(bytes: &Self::Repr) -> CtOption<Self> {
let tag = bytes[0];
let y_is_odd = tag.ct_eq(&sec1::Tag::CompressedOddY.into());
let is_compressed_point = y_is_odd | tag.ct_eq(&sec1::Tag::CompressedEvenY.into());
Self::decompress(FieldBytes::from_slice(&bytes[1..]), y_is_odd)
.and_then(|point| CtOption::new(point, is_compressed_point))
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)
})
.and_then(|point| Self::from_encoded_point(&point))
}

fn from_bytes_unchecked(bytes: &Self::Repr) -> CtOption<Self> {
Expand All @@ -145,7 +150,10 @@ impl GroupEncoding for AffinePoint {
}

fn to_bytes(&self) -> Self::Repr {
CompressedPoint::clone_from_slice(self.to_encoded_point(true).as_bytes())
let encoded = self.to_encoded_point(true);
let mut result = CompressedPoint::default();
result[..encoded.len()].copy_from_slice(encoded.as_bytes());
result
}
}

Expand Down Expand Up @@ -277,7 +285,7 @@ mod tests {
use super::AffinePoint;
use crate::EncodedPoint;
use elliptic_curve::{
group::prime::PrimeCurveAffine,
group::{prime::PrimeCurveAffine, GroupEncoding},
sec1::{FromEncodedPoint, ToCompactEncodedPoint, ToEncodedPoint},
};
use hex_literal::hex;
Expand Down Expand Up @@ -378,4 +386,15 @@ mod tests {
let res = point.to_encoded_point(false);
assert_eq!(res.as_bytes(), UNCOMPACT_BASEPOINT);
}

#[test]
fn identity_encoding() {
// This is technically an invalid SEC1 encoding, but is preferable to panicking.
assert_eq!([0; 33], AffinePoint::identity().to_bytes().as_slice());
assert!(bool::from(
AffinePoint::from_bytes(&AffinePoint::identity().to_bytes())
.unwrap()
.is_identity()
))
}
}
8 changes: 2 additions & 6 deletions p256/src/arithmetic/projective.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl GroupEncoding for ProjectivePoint {
type Repr = CompressedPoint;

fn from_bytes(bytes: &Self::Repr) -> CtOption<Self> {
<AffinePoint as GroupEncoding>::from_bytes(bytes).map(|point| point.into())
<AffinePoint as GroupEncoding>::from_bytes(bytes).map(Into::into)
}

fn from_bytes_unchecked(bytes: &Self::Repr) -> CtOption<Self> {
Expand All @@ -75,11 +75,7 @@ impl GroupEncoding for ProjectivePoint {
}

fn to_bytes(&self) -> Self::Repr {
let bytes = self.to_affine().to_encoded_point(true);
let bytes = bytes.as_bytes();
let mut result = CompressedPoint::default();
result[..bytes.len()].copy_from_slice(bytes);
result
self.to_affine().to_bytes()
}
}

Expand Down