bssl-crypto: Add support for serializing and deserializing compressed points
Also run `cargo fmt` on `bssl-crypto`.
Bug: b/419532530
Change-Id: I70a4424217eac32f3452c6feccd7c3f98596885f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/86707
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/rust/bssl-crypto/src/ec.rs b/rust/bssl-crypto/src/ec.rs
index 987c27b..5cd1a30 100644
--- a/rust/bssl-crypto/src/ec.rs
+++ b/rust/bssl-crypto/src/ec.rs
@@ -160,17 +160,7 @@
self.point
}
- /// Create a new point from an uncompressed X9.62 representation.
- ///
- /// (X9.62 is the standard representation of an elliptic-curve point that
- /// starts with an 0x04 byte.)
- pub fn from_x962_uncompressed(group: Group, x962: &[u8]) -> Option<Self> {
- const UNCOMPRESSED: u8 =
- bssl_sys::point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED as u8;
- if x962.first()? != &UNCOMPRESSED {
- return None;
- }
-
+ fn from_x962(group: Group, x962: &[u8]) -> Option<Self> {
let point = Self::new(group);
// Safety: `point` is valid by construction. `x962` is a valid memory
// buffer.
@@ -184,8 +174,9 @@
)
};
if result == 1 {
- // X9.62 format cannot represent the point at infinity, so this
- // should be moot, but `Point` must never contain infinity.
+ // The X9.62 encoding of the point at infinity is 0x00, but
+ // BoringSSL will never parse it. So this should be moot,
+ // but `Point` must never contain infinity.
assert_eq!(0, unsafe {
bssl_sys::EC_POINT_is_at_infinity(point.group, point.point)
});
@@ -195,10 +186,55 @@
}
}
+ /// Create a new point from an uncompressed X9.62 representation.
+ ///
+ /// (X9.62 is the standard representation of an elliptic-curve point that
+ /// starts with an 0x04 byte.)
+ pub fn from_x962_uncompressed(group: Group, x962: &[u8]) -> Option<Self> {
+ const UNCOMPRESSED: u8 =
+ bssl_sys::point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED as u8;
+ if x962.first()? != &UNCOMPRESSED {
+ return None;
+ }
+
+ Self::from_x962(group, x962)
+ }
+
pub fn to_x962_uncompressed(&self) -> Buffer {
// Safety: arguments are valid, `EC_KEY` ensures that the the group is
// correct for the point, and a `Point` is always finite.
- unsafe { to_x962_uncompressed(self.group, self.point) }
+ unsafe {
+ to_x962(
+ self.group,
+ self.point,
+ bssl_sys::point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED,
+ )
+ }
+ }
+
+ pub fn from_x962_compressed(group: Group, x962: &[u8]) -> Option<Self> {
+ // The first byte of the compressed format can be either 0x02 or 0x03,
+ // to indicate whether the y coordinate of the point is even or odd.
+ let first_byte = *x962.first()?;
+ if first_byte != 2 && first_byte != 3 {
+ return None;
+ }
+
+ Self::from_x962(group, x962)
+ }
+
+ /// WARNING: compressed form is rarely used and is not as well supported as
+ /// the uncompressed form.
+ pub fn to_x962_compressed(&self) -> Buffer {
+ // Safety: arguments are valid, `EC_KEY` ensures that the the group is
+ // correct for the point, and a `Point` is always finite.
+ unsafe {
+ to_x962(
+ self.group,
+ self.point,
+ bssl_sys::point_conversion_form_t::POINT_CONVERSION_COMPRESSED,
+ )
+ }
}
pub fn from_der_subject_public_key_info(group: Group, spki: &[u8]) -> Option<Self> {
@@ -422,7 +458,30 @@
let point = unsafe { bssl_sys::EC_KEY_get0_public_key(self.0) };
// Safety: arguments are valid, `EC_KEY` ensures that the the group is
// correct for the point, and a `Key` always holds a finite public point.
- unsafe { to_x962_uncompressed(group, point) }
+ unsafe {
+ to_x962(
+ group,
+ point,
+ bssl_sys::point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED,
+ )
+ }
+ }
+
+ /// WARNING: compressed form is rarely used and is not as well supported as
+ /// the uncompressed form.
+ pub fn to_x962_compressed(&self) -> Buffer {
+ // Safety: `self.0` is valid by construction.
+ let group = unsafe { bssl_sys::EC_KEY_get0_group(self.0) };
+ let point = unsafe { bssl_sys::EC_KEY_get0_public_key(self.0) };
+ // Safety: arguments are valid, `EC_KEY` ensures that the the group is
+ // correct for the point, and a `Key` always holds a finite public point.
+ unsafe {
+ to_x962(
+ group,
+ point,
+ bssl_sys::point_conversion_form_t::POINT_CONVERSION_COMPRESSED,
+ )
+ }
}
pub fn to_der_subject_public_key_info(&self) -> Buffer {
@@ -452,27 +511,31 @@
}
}
-/// Serialize a finite point to uncompressed X9.62 format.
+/// Serialize a finite point to X9.62 format.
///
/// Callers must ensure that the arguments are valid, that the point has the
/// specified group, and that the point is finite.
-unsafe fn to_x962_uncompressed(
+unsafe fn to_x962(
group: *const bssl_sys::EC_GROUP,
point: *const bssl_sys::EC_POINT,
+ form: bssl_sys::point_conversion_form_t,
) -> Buffer {
- cbb_to_buffer(65, |cbb| unsafe {
- // Safety: the caller must ensure that the arguments are valid.
- let result = bssl_sys::EC_POINT_point2cbb(
- cbb,
- group,
- point,
- bssl_sys::point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED,
- /*bn_ctx=*/ null_mut(),
- );
- // The public key is always finite, so `EC_POINT_point2cbb` only fails
- // if out of memory, which isn't handled by this crate.
- assert_eq!(result, 1);
- })
+ cbb_to_buffer(
+ // This length is just a hint and is tuned for P-256's output length.
+ if form == bssl_sys::point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED {
+ 1 + 32 + 32
+ } else {
+ 1 + 32
+ },
+ |cbb| unsafe {
+ // Safety: the caller must ensure that the arguments are valid.
+ let result =
+ bssl_sys::EC_POINT_point2cbb(cbb, group, point, form, /*bn_ctx=*/ null_mut());
+ // The public key is always finite, so `EC_POINT_point2cbb` only fails
+ // if out of memory, which isn't handled by this crate.
+ assert_eq!(result, 1);
+ },
+ )
}
unsafe fn to_der_subject_public_key_info(ec_key: *mut bssl_sys::EC_KEY) -> Buffer {
@@ -481,23 +544,37 @@
assert_eq!(1, unsafe {
bssl_sys::EVP_PKEY_set1_EC_KEY(pkey.as_ffi_ptr(), ec_key)
});
- cbb_to_buffer(65, |cbb| unsafe {
- // The arguments are valid so this will only fail if out of memory,
- // which this crate doesn't handle.
- assert_eq!(1, bssl_sys::EVP_marshal_public_key(cbb, pkey.as_ffi_ptr()));
- })
+ cbb_to_buffer(
+ // This length is just a hint and is tuned for P-256's output length.
+ 65,
+ |cbb| unsafe {
+ // The arguments are valid so this will only fail if out of memory,
+ // which this crate doesn't handle.
+ assert_eq!(1, bssl_sys::EVP_marshal_public_key(cbb, pkey.as_ffi_ptr()));
+ },
+ )
}
#[cfg(test)]
mod test {
use super::*;
- fn test_point_format<Serialize, Parse>(serialize_func: Serialize, parse_func: Parse)
- where
+ #[derive(PartialEq)]
+ enum Corruption {
+ ShouldBeDetected,
+ DontTest,
+ }
+
+ fn test_point_format<Serialize, Parse>(
+ group: Group,
+ serialize_func: Serialize,
+ parse_func: Parse,
+ corruption: Corruption,
+ ) where
Serialize: FnOnce(&Point) -> Buffer,
Parse: Fn(&[u8]) -> Option<Point>,
{
- let key = Key::generate(Group::P256);
+ let key = Key::generate(group);
let point = key.to_point();
let mut vec = serialize_func(&point).as_ref().to_vec();
@@ -509,9 +586,16 @@
assert!(parse_func(&vec.as_slice()[0..16]).is_none());
- vec[10] ^= 1;
+ // Messing with the first byte should always cause an error.
+ vec[0] ^= 64;
assert!(parse_func(vec.as_slice()).is_none());
- vec[10] ^= 1;
+ vec[0] ^= 64;
+
+ if corruption == Corruption::ShouldBeDetected {
+ vec[11] ^= 1;
+ assert!(parse_func(vec.as_slice()).is_none());
+ vec[11] ^= 1;
+ }
assert!(parse_func(b"").is_none());
}
@@ -522,16 +606,83 @@
assert!(Point::from_x962_uncompressed(Group::P256, x962).is_some());
test_point_format(
+ Group::P256,
|point| point.to_x962_uncompressed(),
|buf| Point::from_x962_uncompressed(Group::P256, buf),
+ Corruption::ShouldBeDetected,
);
+
+ test_point_format(
+ Group::P384,
+ |point| point.to_x962_uncompressed(),
+ |buf| Point::from_x962_uncompressed(Group::P384, buf),
+ Corruption::ShouldBeDetected,
+ );
+
+ test_point_format(
+ Group::P256,
+ |point| point.to_x962_compressed(),
+ |buf| Point::from_x962_compressed(Group::P256, buf),
+ // Flipping a bit in a compressed point has a reasonable chance of
+ // producing another valid point, so we can't run a bit-flip test
+ // in this case.
+ Corruption::DontTest,
+ );
+
+ test_point_format(
+ Group::P384,
+ |point| point.to_x962_compressed(),
+ |buf| Point::from_x962_compressed(Group::P384, buf),
+ // Flipping a bit in a compressed point has a reasonable chance of
+ // producing another valid point, so we can't run a bit-flip test
+ // in this case.
+ Corruption::DontTest,
+ );
+ }
+
+ #[test]
+ fn x962_crossing_formats() {
+ let point = Key::generate(Group::P256).to_point();
+ let uncompressed = point.to_x962_uncompressed();
+ let compressed = point.to_x962_compressed();
+
+ // Compressed points won't be accepted by the uncompressed function and
+ // vice-versa.
+ assert!(Point::from_x962_uncompressed(Group::P256, compressed.as_ref()).is_none());
+ assert!(Point::from_x962_compressed(Group::P256, uncompressed.as_ref()).is_none());
+ }
+
+ #[test]
+ fn x962_infinity_not_accepted() {
+ // 0x00 is the X9.62 encoding of the point at infinity.
+ let infinity = &[0];
+ assert!(Point::from_x962_uncompressed(Group::P256, infinity).is_none());
+ assert!(Point::from_x962_compressed(Group::P256, infinity).is_none());
+ }
+
+ #[test]
+ fn x962_empty() {
+ // The X9.62 functions look at the first byte to check the type. They
+ // must handle the case of an empty input correctly.
+ let empty = b"";
+ assert!(Point::from_x962_uncompressed(Group::P256, empty).is_none());
+ assert!(Point::from_x962_compressed(Group::P256, empty).is_none());
}
#[test]
fn spki() {
test_point_format(
+ Group::P256,
|point| point.to_der_subject_public_key_info(),
|buf| Point::from_der_subject_public_key_info(Group::P256, buf),
+ Corruption::ShouldBeDetected,
+ );
+
+ test_point_format(
+ Group::P384,
+ |point| point.to_der_subject_public_key_info(),
+ |buf| Point::from_der_subject_public_key_info(Group::P384, buf),
+ Corruption::ShouldBeDetected,
);
}
diff --git a/rust/bssl-crypto/src/ecdsa.rs b/rust/bssl-crypto/src/ecdsa.rs
index 4f30096..3aac227 100644
--- a/rust/bssl-crypto/src/ecdsa.rs
+++ b/rust/bssl-crypto/src/ecdsa.rs
@@ -59,6 +59,23 @@
self.point.to_x962_uncompressed()
}
+ /// Parse a public key in compressed X9.62 point format.
+ pub fn from_x962_compressed(x962: &[u8]) -> Option<Self> {
+ let point = ec::Point::from_x962_compressed(C::group(sealed::Sealed), x962)?;
+ Some(Self {
+ point,
+ marker: PhantomData,
+ })
+ }
+
+ /// Serialize this key as compressed X9.62 format.
+ ///
+ /// WARNING: compressed form is rarely used and is not as well supported as
+ /// the uncompressed form.
+ pub fn to_x962_compressed(&self) -> Buffer {
+ self.point.to_x962_compressed()
+ }
+
/// Parse a public key in SubjectPublicKeyInfo format.
/// (This is found in, e.g., X.509 certificates.)
pub fn from_der_subject_public_key_info(spki: &[u8]) -> Option<Self> {
@@ -191,6 +208,14 @@
self.key.to_x962_uncompressed()
}
+ /// Serialize the _public_ part of this key in compressed X9.62 format.
+ ///
+ /// WARNING: compressed form is rarely used and is not as well supported as
+ /// the uncompressed form.
+ pub fn to_x962_compressed(&self) -> Buffer {
+ self.key.to_x962_compressed()
+ }
+
/// Serialize this key in SubjectPublicKeyInfo format.
pub fn to_der_subject_public_key_info(&self) -> Buffer {
self.key.to_der_subject_public_key_info()
@@ -308,13 +333,36 @@
.is_err());
}
+ fn check_compressed<C: ec::Curve>() {
+ let signed_message = b"hello world";
+ let key = PrivateKey::<C>::generate();
+ let mut sig = key.sign(signed_message);
+ let mut sig_p1363 = key.sign_p1363(signed_message);
+
+ let public_key =
+ PublicKey::<C>::from_x962_compressed(key.to_x962_compressed().as_ref()).unwrap();
+ assert!(public_key.verify(signed_message, sig.as_slice()).is_ok());
+ assert!(public_key
+ .verify_p1363(signed_message, sig_p1363.as_slice())
+ .is_ok());
+
+ sig[10] ^= 1;
+ assert!(public_key.verify(signed_message, sig.as_slice()).is_err());
+ sig_p1363[10] ^= 1;
+ assert!(public_key
+ .verify_p1363(signed_message, sig_p1363.as_slice())
+ .is_err());
+ }
+
#[test]
fn p256() {
check_curve::<P256>();
+ check_compressed::<P256>();
}
#[test]
fn p384() {
check_curve::<P384>();
+ check_compressed::<P384>();
}
}
