Extract a CBB helper
This was nagging at me, so I thought I'd see what I could do. I'm not
worried about pinning now - the CBB by inspection is definitely
movable, and it would need to be altered in fairly weird ways.
But I am worried about the lifetime issues - that seems like an
accident waiting to happen, but can be exposed to the borrow checker
fairly easily.
(This is similar to commit 3194c466151c7d1c06b226f13d3c03298a27d40a.)
Bug: 299055662
Test: TH
Change-Id: Iada56f51800cd2af99e86b557805e65823a141da
diff --git a/rialto/src/requests/cbb.rs b/rialto/src/requests/cbb.rs
new file mode 100644
index 0000000..93cebe4
--- /dev/null
+++ b/rialto/src/requests/cbb.rs
@@ -0,0 +1,52 @@
+// Copyright 2023, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Helpers for using BoringSSL CBB (crypto byte builder) objects.
+use bssl_ffi::{CBB_init_fixed, CBB};
+use core::marker::PhantomData;
+use core::mem::MaybeUninit;
+
+/// Wraps a CBB that references a existing fixed-sized buffer; no memory is allocated, but the
+/// buffer cannot grow.
+pub struct CbbFixed<'a> {
+ cbb: CBB,
+ // The CBB contains a mutable reference to the buffer, disguised as a pointer.
+ // Make sure the borrow checker knows that.
+ _buffer: PhantomData<&'a mut [u8]>,
+}
+
+impl CbbFixed<'_> {
+ // Create a new CBB that writes to the given buffer.
+ pub fn new(buffer: &mut [u8]) -> Self {
+ let mut cbb = MaybeUninit::uninit();
+ // SAFETY: `CBB_init_fixed()` is infallible and always returns one.
+ // The buffer remains valid during the lifetime of `cbb`.
+ unsafe { CBB_init_fixed(cbb.as_mut_ptr(), buffer.as_mut_ptr(), buffer.len()) };
+ // SAFETY: `cbb` has just been initialized by `CBB_init_fixed()`.
+ let cbb = unsafe { cbb.assume_init() };
+ Self { cbb, _buffer: PhantomData }
+ }
+}
+
+impl AsRef<CBB> for CbbFixed<'_> {
+ fn as_ref(&self) -> &CBB {
+ &self.cbb
+ }
+}
+
+impl AsMut<CBB> for CbbFixed<'_> {
+ fn as_mut(&mut self) -> &mut CBB {
+ &mut self.cbb
+ }
+}
diff --git a/rialto/src/requests/ec_key.rs b/rialto/src/requests/ec_key.rs
index 1e1a35c..4578526 100644
--- a/rialto/src/requests/ec_key.rs
+++ b/rialto/src/requests/ec_key.rs
@@ -15,26 +15,14 @@
//! Contains struct and functions that wraps the API related to EC_KEY in
//! BoringSSL.
+use super::cbb::CbbFixed;
use alloc::vec::Vec;
-use bssl_ffi::BN_bn2bin_padded;
-use bssl_ffi::BN_clear_free;
-use bssl_ffi::BN_new;
-use bssl_ffi::CBB_flush;
-use bssl_ffi::CBB_init_fixed;
-use bssl_ffi::CBB_len;
-use bssl_ffi::EC_KEY_free;
-use bssl_ffi::EC_KEY_generate_key;
-use bssl_ffi::EC_KEY_get0_group;
-use bssl_ffi::EC_KEY_get0_public_key;
-use bssl_ffi::EC_KEY_marshal_private_key;
-use bssl_ffi::EC_KEY_new_by_curve_name;
-use bssl_ffi::EC_POINT_get_affine_coordinates;
-use bssl_ffi::NID_X9_62_prime256v1; // EC P-256 CURVE Nid
-use bssl_ffi::BIGNUM;
-use bssl_ffi::EC_GROUP;
-use bssl_ffi::EC_KEY;
-use bssl_ffi::EC_POINT;
-use core::mem::MaybeUninit;
+use bssl_ffi::{
+ BN_bn2bin_padded, BN_clear_free, BN_new, CBB_flush, CBB_len, EC_KEY_free, EC_KEY_generate_key,
+ EC_KEY_get0_group, EC_KEY_get0_public_key, EC_KEY_marshal_private_key,
+ EC_KEY_new_by_curve_name, EC_POINT_get_affine_coordinates, NID_X9_62_prime256v1, BIGNUM,
+ EC_GROUP, EC_KEY, EC_POINT,
+};
use core::ptr::{self, NonNull};
use core::result;
use coset::{iana, CoseKey, CoseKeyBuilder};
@@ -61,7 +49,9 @@
/// Creates a new EC P-256 key pair.
pub fn new_p256() -> Result<Self> {
// SAFETY: The returned pointer is checked below.
- let ec_key = unsafe { EC_KEY_new_by_curve_name(NID_X9_62_prime256v1) };
+ let ec_key = unsafe {
+ EC_KEY_new_by_curve_name(NID_X9_62_prime256v1) // EC P-256 CURVE Nid
+ };
let mut ec_key = NonNull::new(ec_key).map(Self).ok_or(
RequestProcessingError::BoringSSLCallFailed(BoringSSLApiName::EC_KEY_new_by_curve_name),
)?;
@@ -142,26 +132,20 @@
pub fn private_key(&self) -> Result<ZVec> {
const CAPACITY: usize = 256;
let mut buf = Zeroizing::new([0u8; CAPACITY]);
- // SAFETY: `CBB_init_fixed()` is infallible and always returns one.
- // The `buf` is never moved and remains valid during the lifetime of `cbb`.
- let mut cbb = unsafe {
- let mut cbb = MaybeUninit::uninit();
- CBB_init_fixed(cbb.as_mut_ptr(), buf.as_mut_ptr(), buf.len());
- cbb.assume_init()
- };
+ let mut cbb = CbbFixed::new(buf.as_mut());
let enc_flags = 0;
let ret =
// SAFETY: The function only write bytes to the buffer managed by the valid `CBB`
// object, and the key has been allocated by BoringSSL.
- unsafe { EC_KEY_marshal_private_key(&mut cbb, self.0.as_ptr(), enc_flags) };
+ unsafe { EC_KEY_marshal_private_key(cbb.as_mut(), self.0.as_ptr(), enc_flags) };
check_int_result(ret, BoringSSLApiName::EC_KEY_marshal_private_key)?;
// SAFETY: This is safe because the CBB pointer is a valid pointer initialized with
// `CBB_init_fixed()`.
- check_int_result(unsafe { CBB_flush(&mut cbb) }, BoringSSLApiName::CBB_flush)?;
+ check_int_result(unsafe { CBB_flush(cbb.as_mut()) }, BoringSSLApiName::CBB_flush)?;
// SAFETY: This is safe because the CBB pointer is initialized with `CBB_init_fixed()`,
// and it has been flushed, thus it has no active children.
- let len = unsafe { CBB_len(&cbb) };
+ let len = unsafe { CBB_len(cbb.as_ref()) };
Ok(buf
.get(0..len)
.ok_or(RequestProcessingError::BoringSSLCallFailed(BoringSSLApiName::CBB_len))?
diff --git a/rialto/src/requests/mod.rs b/rialto/src/requests/mod.rs
index 8162237..12838f3 100644
--- a/rialto/src/requests/mod.rs
+++ b/rialto/src/requests/mod.rs
@@ -15,6 +15,7 @@
//! This module contains functions for the request processing.
mod api;
+mod cbb;
mod ec_key;
mod pub_key;
mod rkp;