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>(); } }