From 26e8a8acb91a0cfbd2f95bf7245e2eb87d533a2f Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 22 Aug 2025 18:44:54 -0400 Subject: [PATCH] bssl-crypto: Fix Ed25519 SPKI parser to check key types Otherwise it mistakenly interprets an X25519 SPKI as an Ed25519 key. Once that is fixed, a number of these error conditions are impossible. Change-Id: Iffd116252f1f632f8f917c56b1676069686eddbf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81587 Commit-Queue: Adam Langley Reviewed-by: Adam Langley Auto-Submit: David Benjamin Commit-Queue: David Benjamin --- rust/bssl-crypto/src/ed25519.rs | 35 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/rust/bssl-crypto/src/ed25519.rs b/rust/bssl-crypto/src/ed25519.rs index 73c90fbe7..6be050876 100644 --- a/rust/bssl-crypto/src/ed25519.rs +++ b/rust/bssl-crypto/src/ed25519.rs @@ -161,32 +161,23 @@ impl PublicKey { // Safety: cbs is valid per `parse_with_cbs`. |cbs| unsafe { bssl_sys::EVP_parse_public_key(cbs) }, )?); - - let mut out_len = 0; - // When the out buffer is null, `out_len` is set to the size of the raw public key. - // Safety: the arguments are valid. - let result = unsafe { - bssl_sys::EVP_PKEY_get_raw_public_key( - pkey.as_ffi_ptr(), - core::ptr::null_mut(), - &mut out_len, - ) - }; - if result != 1 { - return None; - } - if out_len != PUBLIC_KEY_LEN { + // Safety: `pkey` is a valid `EVP_PKEY`. + if unsafe { bssl_sys::EVP_PKEY_id(pkey.as_ffi_ptr()) } != bssl_sys::EVP_PKEY_ED25519 { return None; } - // When the out buffer is not null, the raw public key is written into it. - // Safety: the arguments are valid. + // Safety: `EVP_PKEY_get_raw_public_key` is passed a valid buffer. The + // (always true) assertion ensures the closure always initializes the + // array. let raw_pkey: [u8; PUBLIC_KEY_LEN] = unsafe { - with_output_array(|out, _| { + with_output_array(|out, mut out_len| { + // If `pkey` is an Ed25519 key, checked above, the raw public + // key must be available, and must be `PUBLIC_KEY_LEN` bytes. assert_eq!( 1, bssl_sys::EVP_PKEY_get_raw_public_key(pkey.as_ffi_ptr(), out, &mut out_len) ); + assert_eq!(out_len, PUBLIC_KEY_LEN); }) }; Some(PublicKey(raw_pkey)) @@ -268,6 +259,14 @@ mod test { assert!(PublicKey::from_der_subject_public_key_info(b"").is_none()); } + #[test] + fn der_subject_public_key_info_wrong_type() { + // This is an X25519 key, not an Ed25519 key. + let spki = test_helpers::decode_hex_into_vec("302a300506032b656e032100e6db6867583030db3594c1a424b15f7c726624ec26b3353b10a903a6d0ab1c4c"); + // `from_der_subject_public_key_info` should reject it. + assert!(PublicKey::from_der_subject_public_key_info(&spki).is_none()); + } + #[test] fn empty_msg() { // Test Case 1 from RFC test vectors: https://www.rfc-editor.org/rfc/rfc8032#section-7.1