Skip to content

Use PK11_CreatePrivateKeyFromTemplate instead of the CreateGenericObject/FindKeyByKeyID workaround #7455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Frosne wants to merge 1 commit into from
+14 −98
Closed

Use PK11_CreatePrivateKeyFromTemplate instead of the CreateGenericObject/FindKeyByKeyID workaround #7455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions components/support/rc_crypto/nss/nss_sys/src/bindings/pk11pub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ extern "C" {
isSensitive: PRBool,
wincx: *mut c_void,
) -> *mut SECKEYPrivateKey;
pub fn PK11_FindKeyByKeyID(
pub fn PK11_CreatePrivateKeyFromTemplate(
slot: *mut PK11SlotInfo,
keyID: *mut SECItem,
template: *const CK_ATTRIBUTE,
count: c_uint,
wincx: *mut c_void,
) -> *mut SECKEYPrivateKey;
pub fn PK11_ListFixedKeysInSlot(
Expand Down Expand Up @@ -155,13 +156,6 @@ extern "C" {
outLen: *mut c_uint,
length: c_uint,
) -> SECStatus;
pub fn PK11_DestroyGenericObject(object: *mut PK11GenericObject) -> SECStatus;
pub fn PK11_CreateGenericObject(
slot: *mut PK11SlotInfo,
pTemplate: *const CK_ATTRIBUTE,
count: c_int,
token: PRBool,
) -> *mut PK11GenericObject;
pub fn PK11_ReadRawAttribute(
type_: u32, /* PK11ObjectType */
object: *mut c_void,
Expand Down
100 changes: 11 additions & 89 deletions components/support/rc_crypto/nss/src/pk11/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

use crate::{
error::*,
pk11::slot::{generate_random, get_internal_slot},
pk11::slot::get_internal_slot,
util::{map_nss_secstatus, ScopedPtr},
};
use std::{
ops::Deref,
os::raw::{c_int, c_uchar, c_uint, c_void},
os::raw::{c_uint, c_void},
ptr,
};

Expand All @@ -24,12 +24,6 @@ scoped_ptr!(
nss_sys::SECKEYPublicKey,
nss_sys::SECKEY_DestroyPublicKey
);
scoped_ptr!(
GenericObject,
nss_sys::PK11GenericObject,
nss_sys::PK11_DestroyGenericObject
);

scoped_ptr!(
Certificate,
nss_sys::CERTCertificate,
Expand Down Expand Up @@ -79,9 +73,6 @@ pub(crate) unsafe trait Pkcs11Object: ScopedPtr {
}
}

unsafe impl Pkcs11Object for GenericObject {
const PK11_OBJECT_TYPE: nss_sys::PK11ObjectType = nss_sys::PK11ObjectType::PK11_TypeGeneric;
}
unsafe impl Pkcs11Object for PrivateKey {
const PK11_OBJECT_TYPE: nss_sys::PK11ObjectType = nss_sys::PK11ObjectType::PK11_TypePrivKey;
}
Expand All @@ -102,74 +93,18 @@ impl PrivateKey {
Ok(unsafe { PublicKey::from_ptr(nss_sys::SECKEY_ConvertToPublicKey(self.as_mut_ptr()))? })
}

// To protect against key ID collisions, PrivateKeyFromPrivateKeyTemplate
// generates a random ID for each key. The given template must contain an
// attribute slot for a key ID, but it must consist of a null pointer and have a
// length of 0.
pub(crate) fn from_private_key_template(
mut template: Vec<nss_sys::CK_ATTRIBUTE>,
) -> Result<Self> {
// Generate a random 160-bit object ID. This ID must be unique.
let mut obj_id_buf = vec![0u8; 160 / 8];
generate_random(&mut obj_id_buf)?;
let mut obj_id = nss_sys::SECItem {
type_: nss_sys::SECItemType::siBuffer as u32,
data: obj_id_buf.as_ptr() as *mut c_uchar,
len: c_uint::try_from(obj_id_buf.len())?,
};
// Creates a SECKEYPrivateKey directly from a fully-specified PKCS#11 private
// key template. NSS creates the backing session object and returns a key that
// owns it, destroying it when the key is destroyed.
pub(crate) fn from_private_key_template(template: Vec<nss_sys::CK_ATTRIBUTE>) -> Result<Self> {
let slot = get_internal_slot()?;
let mut pre_existing_key = unsafe {
nss_sys::PK11_FindKeyByKeyID(slot.as_mut_ptr(), &mut obj_id, std::ptr::null_mut())
};
if !pre_existing_key.is_null() {
// Note that we can't just call SECKEY_DestroyPrivateKey here because that
// will destroy the PKCS#11 object that is backing a preexisting key (that
// we still have a handle on somewhere else in memory). If that object were
// destroyed, cryptographic operations performed by that other key would
// fail.
unsafe {
destroy_private_key_without_destroying_pkcs11_object(pre_existing_key);
}
// Try again with a new ID (but only once - collisions are very unlikely).
generate_random(&mut obj_id_buf)?;
pre_existing_key = unsafe {
nss_sys::PK11_FindKeyByKeyID(slot.as_mut_ptr(), &mut obj_id, std::ptr::null_mut())
};
if !pre_existing_key.is_null() {
unsafe {
destroy_private_key_without_destroying_pkcs11_object(pre_existing_key);
}
return Err(ErrorKind::InternalError.into());
}
}
let template_len = c_int::try_from(template.len())?;
let id_attr: &mut nss_sys::CK_ATTRIBUTE = template
.iter_mut()
.find(|&&mut attr| {
attr.type_ == (nss_sys::CKA_ID as nss_sys::CK_ATTRIBUTE_TYPE)
&& attr.pValue.is_null()
&& attr.ulValueLen == 0
})
.ok_or(ErrorKind::InternalError)?;
id_attr.pValue = obj_id_buf.as_mut_ptr() as *mut c_void;
id_attr.ulValueLen = nss_sys::CK_ULONG::try_from(obj_id_buf.len())?;
// We use `PK11_CreateGenericObject` instead of `PK11_CreateManagedGenericObject`
// to leak the reference on purpose because `PK11_FindKeyByKeyID` will take
// ownership of it.
let _obj = unsafe {
GenericObject::from_ptr(nss_sys::PK11_CreateGenericObject(
slot.as_mut_ptr(),
template.as_mut_ptr(),
template_len,
nss_sys::PR_FALSE,
))?
};
// Have NSS translate the object to a private key.
let count = c_uint::try_from(template.len())?;
Ok(unsafe {
PrivateKey::from_ptr(nss_sys::PK11_FindKeyByKeyID(
PrivateKey::from_ptr(nss_sys::PK11_CreatePrivateKeyFromTemplate(
slot.as_mut_ptr(),
&mut obj_id,
std::ptr::null_mut(),
template.as_ptr(),
count,
ptr::null_mut(),
))?
})
}
Expand Down Expand Up @@ -214,16 +149,3 @@ impl Drop for ScopedSECItem {
}
}
}

// This helper function will release the memory backing a SECKEYPrivateKey and
// any resources acquired in its creation. It will leave the backing PKCS#11
// object untouched, however. This should only be called from
// PrivateKeyFromPrivateKeyTemplate.
// From: https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/dom/crypto/CryptoKey.cpp#80
unsafe fn destroy_private_key_without_destroying_pkcs11_object(
key: *mut nss_sys::SECKEYPrivateKey,
) {
assert!(!key.is_null());
nss_sys::PK11_FreeSlot((*key).pkcs11Slot);
nss_sys::PORT_FreeArena((*key).arena, nss_sys::PR_TRUE);
}