Reworking bssl_crypto: x25519
Change-Id: Ib9fc874e1c5d540dda91a454681dad809e8d6d14
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65167
Reviewed-by: Bob Beck <bbe@google.com>
Reviewed-by: Maurice Lam <yukl@google.com>
Reviewed-by: Nabil Wadih <nwadih@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/rust/bssl-crypto/src/lib.rs b/rust/bssl-crypto/src/lib.rs
index e53469d..022f5a3 100644
--- a/rust/bssl-crypto/src/lib.rs
+++ b/rust/bssl-crypto/src/lib.rs
@@ -28,6 +28,8 @@
extern crate core;
+use core::ffi::c_void;
+
/// Authenticated Encryption with Additional Data algorithms.
pub mod aead;
@@ -52,7 +54,6 @@
/// Random number generation.
pub mod rand;
-/// X25519 elliptic curve operations.
pub mod x25519;
/// Memory-manipulation operations.
@@ -68,7 +69,70 @@
#[cfg(test)]
mod test_helpers;
+/// FfiSlice exists to provide `as_ffi_ptr` on slices. Calling `as_ptr` on an
+/// empty Rust slice may return the alignment of the type, rather than NULL, as
+/// the pointer. When passing pointers into C/C++ code, that is not a valid
+/// pointer. Thus this method should be used whenever passing a pointer to a
+/// slice into BoringSSL code.
+trait FfiSlice {
+ fn as_ffi_ptr(&self) -> *const u8;
+ fn as_ffi_void_ptr(&self) -> *const c_void {
+ self.as_ffi_ptr() as *const c_void
+ }
+}
+
+impl FfiSlice for [u8] {
+ fn as_ffi_ptr(&self) -> *const u8 {
+ if self.is_empty() {
+ core::ptr::null()
+ } else {
+ self.as_ptr()
+ }
+ }
+}
+
+impl<const N: usize> FfiSlice for [u8; N] {
+ fn as_ffi_ptr(&self) -> *const u8 {
+ if N == 0 {
+ core::ptr::null()
+ } else {
+ self.as_ptr()
+ }
+ }
+}
+
+/// See the comment [`FfiSlice`].
+trait FfiMutSlice {
+ fn as_mut_ffi_ptr(&mut self) -> *mut u8;
+ fn as_ffi_void_ptr(&mut self) -> *mut c_void {
+ self.as_mut_ffi_ptr() as *mut c_void
+ }
+}
+
+impl FfiMutSlice for [u8] {
+ fn as_mut_ffi_ptr(&mut self) -> *mut u8 {
+ if self.is_empty() {
+ core::ptr::null_mut()
+ } else {
+ self.as_mut_ptr()
+ }
+ }
+}
+
+impl<const N: usize> FfiMutSlice for [u8; N] {
+ fn as_mut_ffi_ptr(&mut self) -> *mut u8 {
+ if N == 0 {
+ core::ptr::null_mut()
+ } else {
+ self.as_mut_ptr()
+ }
+ }
+}
+
/// This is a helper struct which provides functions for passing slices over FFI.
+///
+/// Deprecated: use `FfiSlice` which adds less noise and lets one grep for `as_ptr`
+/// as a sign of something to check.
struct CSlice<'a>(&'a [u8]);
impl<'a> From<&'a [u8]> for CSlice<'a> {
@@ -93,6 +157,9 @@
}
/// This is a helper struct which provides functions for passing mutable slices over FFI.
+///
+/// Deprecated: use `FfiMutSlice` which adds less noise and lets one grep for
+/// `as_ptr` as a sign of something to check.
struct CSliceMut<'a>(&'a mut [u8]);
impl CSliceMut<'_> {
@@ -179,3 +246,47 @@
/// Returns a raw pointer to the wrapped value.
fn as_ptr(&self) -> *mut Self::CType;
}
+
+/// Wrap a closure that initializes an output buffer and return that buffer as
+/// an array. Requires that the closure fully initialize the given buffer.
+///
+/// Safety: the closure must fully initialize the array.
+unsafe fn with_output_array<const N: usize, F>(func: F) -> [u8; N]
+where
+ F: FnOnce(*mut u8, usize),
+{
+ let mut out_uninit = core::mem::MaybeUninit::<[u8; N]>::uninit();
+ let out_ptr = if N != 0 {
+ out_uninit.as_mut_ptr() as *mut u8
+ } else {
+ core::ptr::null_mut()
+ };
+ func(out_ptr, N);
+ // Safety: `func` promises to fill all of `out_uninit`.
+ unsafe { out_uninit.assume_init() }
+}
+
+/// Wrap a closure that initializes an output buffer and return that buffer as
+/// an array. The closure returns a [`core::ffi::c_int`] and, if the return value
+/// is not one, then the initialization is assumed to have failed and [None] is
+/// returned. Otherwise, this function requires that the closure fully
+/// initialize the given buffer.
+///
+/// Safety: the closure must fully initialize the array if it returns one.
+unsafe fn with_output_array_fallible<const N: usize, F>(func: F) -> Option<[u8; N]>
+where
+ F: FnOnce(*mut u8, usize) -> core::ffi::c_int,
+{
+ let mut out_uninit = core::mem::MaybeUninit::<[u8; N]>::uninit();
+ let out_ptr = if N != 0 {
+ out_uninit.as_mut_ptr() as *mut u8
+ } else {
+ core::ptr::null_mut()
+ };
+ if func(out_ptr, N) == 1 {
+ // Safety: `func` promises to fill all of `out_uninit` if it returns one.
+ unsafe { Some(out_uninit.assume_init()) }
+ } else {
+ None
+ }
+}
diff --git a/rust/bssl-crypto/src/x25519.rs b/rust/bssl-crypto/src/x25519.rs
index 9ee449b..8f6440d 100644
--- a/rust/bssl-crypto/src/x25519.rs
+++ b/rust/bssl-crypto/src/x25519.rs
@@ -13,11 +13,40 @@
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+//! Diffie-Hellman over curve25519.
+//!
//! X25519 is the Diffie-Hellman primitive built from curve25519. It is sometimes referred to as
-//! “curve25519”, but “X25519” is a more precise name. See http://cr.yp.to/ecdh.html and
-//! https://tools.ietf.org/html/rfc7748.
+//! “curve25519”, but “X25519” is a more precise name. See <http://cr.yp.to/ecdh.html> and
+//! <https://tools.ietf.org/html/rfc7748>.
+//!
+//! ```
+//! use bssl_crypto::x25519;
+//!
+//! // Alice generates her key pair.
+//! let (alice_public_key, alice_private_key) = x25519::PrivateKey::generate();
+//! // Bob generates his key pair.
+//! let (bob_public_key, bob_private_key) = x25519::PrivateKey::generate();
+//!
+//! // If Alice obtains Bob's public key somehow, she can compute their
+//! // shared key:
+//! let shared_key = alice_private_key.compute_shared_key(&bob_public_key);
+//!
+//! // Alice can then derive a key (e.g. by using HKDF), which should include
+//! // at least the two public keys. Then shen can send a message to Bob
+//! // including her public key and an AEAD-protected blob. Bob can compute the
+//! // same shared key given Alice's public key:
+//! let shared_key2 = bob_private_key.compute_shared_key(&alice_public_key);
+//! assert_eq!(shared_key, shared_key2);
+//!
+//! // This is an _unauthenticated_ exchange which is vulnerable to an
+//! // active attacker. See, for example,
+//! // http://www.noiseprotocol.org/noise.html for an example of building
+//! // real protocols from a Diffie-Hellman primitive.
+//! ```
-use alloc::borrow::ToOwned;
+use crate::with_output_array;
+use crate::with_output_array_fallible;
+use crate::FfiSlice;
/// Number of bytes in a private key in X25519
pub const PRIVATE_KEY_LEN: usize = bssl_sys::X25519_PRIVATE_KEY_LEN as usize;
@@ -26,190 +55,99 @@
/// Number of bytes in a shared secret derived with X25519
pub const SHARED_KEY_LEN: usize = bssl_sys::X25519_SHARED_KEY_LEN as usize;
-/// Error while performing a X25519 Diffie-Hellman key exchange.
-#[derive(Debug)]
-pub struct DiffieHellmanError;
+/// X25519 public keys are simply 32-byte strings.
+pub type PublicKey = [u8; PUBLIC_KEY_LEN];
-/// A struct containing a X25519 key pair.
-pub struct PrivateKey {
- private_key: [u8; PRIVATE_KEY_LEN],
- public_key: [u8; PUBLIC_KEY_LEN],
-}
+/// An X25519 private key (a 32-byte string).
+pub struct PrivateKey(pub [u8; PRIVATE_KEY_LEN]);
-impl PrivateKey {
- /// Derives a shared secrect from this private key and the given public key.
- pub fn diffie_hellman(
- &self,
- other_public_key: &PublicKey,
- ) -> Result<SharedSecret, DiffieHellmanError> {
- let mut shared_key_uninit = core::mem::MaybeUninit::<[u8; SHARED_KEY_LEN]>::uninit();
- // Safety:
- // - private_key and other_public_key are Rust 32-byte arrays
- // - shared_key_uninit is just initialized above to a 32 byte array
- let result = unsafe {
- bssl_sys::X25519(
- shared_key_uninit.as_mut_ptr() as *mut u8,
- self.private_key.as_ptr(),
- other_public_key.0.as_ptr(),
- )
- };
- if result == 1 {
- // Safety:
- // - `shared_key_uninit` is initialized by `X25519` above, and we checked that it
- // succeeded
- let shared_key = unsafe { shared_key_uninit.assume_init() };
- Ok(crate::ecdh::SharedSecret(shared_key))
- } else {
- Err(DiffieHellmanError)
- }
- }
-
- /// Generate a new key pair for use in a Diffie-Hellman key exchange.
- pub fn generate() -> Self {
- let mut public_key_uninit = core::mem::MaybeUninit::<[u8; PUBLIC_KEY_LEN]>::uninit();
- let mut private_key_uninit = core::mem::MaybeUninit::<[u8; PRIVATE_KEY_LEN]>::uninit();
- // Safety:
- // - private_key_uninit and public_key_uninit are allocated to 32-bytes
- let (public_key, private_key) = unsafe {
- bssl_sys::X25519_keypair(
- public_key_uninit.as_mut_ptr() as *mut u8,
- private_key_uninit.as_mut_ptr() as *mut u8,
- );
- // Safety: Initialized by `X25519_keypair` above
- (
- public_key_uninit.assume_init(),
- private_key_uninit.assume_init(),
- )
- };
- Self {
- private_key,
- public_key,
- }
- }
-
- /// Tries to convert the given bytes into a private key.
- pub fn from_private_bytes(private_key_bytes: &[u8; PRIVATE_KEY_LEN]) -> Self {
- let mut public_key_uninit = core::mem::MaybeUninit::<[u8; PUBLIC_KEY_LEN]>::uninit();
- let private_key: [u8; PRIVATE_KEY_LEN] = private_key_bytes.to_owned();
- // Safety:
- // - private_key and public_key are Rust 32-byte arrays
- let public_key = unsafe {
- bssl_sys::X25519_public_from_private(
- public_key_uninit.as_mut_ptr() as *mut _,
- private_key.as_ptr(),
- );
- public_key_uninit.assume_init()
- };
- Self {
- private_key,
- public_key,
- }
- }
-}
-
-impl<'a> From<&'a PrivateKey> for PublicKey {
- fn from(value: &'a PrivateKey) -> Self {
- Self(value.public_key)
- }
-}
-
-/// A public key for X25519 elliptic curve.
-#[derive(Debug, PartialEq, Eq)]
-pub struct PublicKey([u8; PUBLIC_KEY_LEN]);
-
-impl PublicKey {
- /// Converts this public key to its byte representation.
- pub fn to_bytes(&self) -> [u8; PUBLIC_KEY_LEN] {
- self.0
- }
-
- /// Returns a reference to the byte representation of this public key.
- pub fn as_bytes(&self) -> &[u8; PUBLIC_KEY_LEN] {
+impl AsRef<[u8]> for PrivateKey {
+ fn as_ref(&self) -> &[u8] {
&self.0
}
}
-impl From<&[u8; 32]> for PublicKey {
- fn from(value: &[u8; 32]) -> Self {
- Self(*value)
+impl PrivateKey {
+ /// Derive the shared key between this private key and a peer's public key.
+ /// Don't use the shared key directly, rather use a KDF and also include
+ /// the two public values as inputs.
+ ///
+ /// Will fail and produce `None` if the peer's public key is a point of
+ /// small order. It is safe to react to this in non-constant time.
+ pub fn compute_shared_key(&self, other_public_key: &PublicKey) -> Option<[u8; SHARED_KEY_LEN]> {
+ // Safety: `X25519` indeed writes `SHARED_KEY_LEN` bytes.
+ unsafe {
+ with_output_array_fallible(|out, _| {
+ bssl_sys::X25519(out, self.0.as_ffi_ptr(), other_public_key.as_ffi_ptr())
+ })
+ }
+ }
+
+ /// Generate a new key pair.
+ pub fn generate() -> (PublicKey, PrivateKey) {
+ let mut public_key_uninit = core::mem::MaybeUninit::<[u8; PUBLIC_KEY_LEN]>::uninit();
+ let mut private_key_uninit = core::mem::MaybeUninit::<[u8; PRIVATE_KEY_LEN]>::uninit();
+ // Safety:
+ // - private_key_uninit and public_key_uninit are the correct length.
+ unsafe {
+ bssl_sys::X25519_keypair(
+ public_key_uninit.as_mut_ptr() as *mut u8,
+ private_key_uninit.as_mut_ptr() as *mut u8,
+ );
+ // Safety: Initialized by `X25519_keypair` just above.
+ (
+ public_key_uninit.assume_init(),
+ PrivateKey(private_key_uninit.assume_init()),
+ )
+ }
+ }
+
+ /// Compute the public key corresponding to this private key.
+ pub fn to_public(&self) -> PublicKey {
+ // Safety: `X25519_public_from_private` indeed fills an entire [`PublicKey`].
+ unsafe {
+ with_output_array(|out, _| {
+ bssl_sys::X25519_public_from_private(out, self.0.as_ffi_ptr());
+ })
+ }
}
}
-/// Shared secret derived from a Diffie-Hellman key exchange. Don't use the shared key directly,
-/// rather use a KDF and also include the two public values as inputs.
-type SharedSecret = crate::ecdh::SharedSecret<SHARED_KEY_LEN>;
-
#[cfg(test)]
#[allow(clippy::unwrap_used)]
mod tests {
- use crate::{
- test_helpers::decode_hex,
- x25519::{PrivateKey, PublicKey},
- };
+ use crate::{test_helpers::decode_hex, x25519::PrivateKey};
#[test]
- fn x25519_test_diffie_hellman() {
+ fn known_vector() {
// wycheproof/testvectors/x25519_test.json tcId 1
- let public_key_bytes: [u8; 32] =
+ let public_key: [u8; 32] =
decode_hex("504a36999f489cd2fdbc08baff3d88fa00569ba986cba22548ffde80f9806829");
- let private_key =
- decode_hex("c8a9d5a91091ad851c668b0736c1c9a02936c0d3ad62670858088047ba057475");
+ let private_key = PrivateKey(decode_hex(
+ "c8a9d5a91091ad851c668b0736c1c9a02936c0d3ad62670858088047ba057475",
+ ));
let expected_shared_secret: [u8; 32] =
decode_hex("436a2c040cf45fea9b29a0cb81b1f41458f863d0d61b453d0a982720d6d61320");
- let public_key = PublicKey::from(&public_key_bytes);
- let private_key = PrivateKey::from_private_bytes(&private_key);
-
- let shared_secret = private_key.diffie_hellman(&public_key).unwrap();
- assert_eq!(expected_shared_secret, shared_secret.to_bytes());
+ let shared_secret = private_key.compute_shared_key(&public_key).unwrap();
+ assert_eq!(expected_shared_secret, shared_secret);
}
#[test]
- fn x25519_generate_diffie_hellman_matches() {
- let private_key_1 = PrivateKey::generate();
- let private_key_2 = PrivateKey::generate();
- let public_key_1 = PublicKey::from(&private_key_1);
- let public_key_2 = PublicKey::from(&private_key_2);
-
- let diffie_hellman_1 = private_key_1.diffie_hellman(&public_key_2).unwrap();
- let diffie_hellman_2 = private_key_2.diffie_hellman(&public_key_1).unwrap();
-
- assert_eq!(diffie_hellman_1.to_bytes(), diffie_hellman_2.to_bytes());
+ fn all_zero_public_key() {
+ assert!(PrivateKey::generate()
+ .1
+ .compute_shared_key(&[0u8; 32])
+ .is_none());
}
#[test]
- fn x25519_test_diffie_hellman_zero_public_key() {
- // wycheproof/testvectors/x25519_test.json tcId 32
- let public_key_bytes =
- decode_hex("0000000000000000000000000000000000000000000000000000000000000000");
- let private_key =
- decode_hex("88227494038f2bb811d47805bcdf04a2ac585ada7f2f23389bfd4658f9ddd45e");
- let public_key = PublicKey::from(&public_key_bytes);
- let private_key = PrivateKey::from_private_bytes(&private_key);
-
- let shared_secret = private_key.diffie_hellman(&public_key);
- assert!(shared_secret.is_err());
- }
-
- #[test]
- fn x25519_public_key_byte_conversion() {
- let public_key_bytes =
- decode_hex("504a36999f489cd2fdbc08baff3d88fa00569ba986cba22548ffde80f9806829");
- let public_key = PublicKey::from(&public_key_bytes);
- assert_eq!(public_key_bytes, public_key.to_bytes());
- }
-
- #[test]
- fn x25519_test_public_key_from_private_key() {
+ fn to_public() {
// Taken from https://www.rfc-editor.org/rfc/rfc7748.html#section-6.1
let public_key_bytes =
decode_hex("8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a");
- let private_key_bytes =
- decode_hex("77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a");
- let private_key = PrivateKey::from_private_bytes(&private_key_bytes);
-
- assert_eq!(
- PublicKey::from(&public_key_bytes),
- PublicKey::from(&private_key)
- );
+ let private_key = PrivateKey(decode_hex(
+ "77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a",
+ ));
+ assert_eq!(public_key_bytes, private_key.to_public());
}
}
