Skip to content

Commit

Permalink
When creating an AES PKCS#11 key, test it with a dummy encryption to …
Browse files Browse the repository at this point in the history
…ensure that the token can perform AES-GCM. (#563)

Cherry-pick from main of 8cb888d

It is possible for PKCS#11 implementations to support AES keys only with AES-CBC etc but not with AES-GCM. Before this change, creating the key would succeed, and then encrypting with it would fail, at which point it would be too late.

With this change, we do a dummy encruption with the key just to be sure that it can be used with AES-GCM. If that fails, we continue to the next location, usually the filesystem ie openssl keys.
  • Loading branch information
arsing authored Oct 25, 2023
1 parent c247a7d commit 79aae50
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 9 deletions.
51 changes: 42 additions & 9 deletions key/aziot-keys/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,22 +612,23 @@ fn create_inner(
.open_session(pkcs11_slot, uri.pin.clone())
.map_err(crate::implementation::err_external)?;

match create_method {
let pkcs11_object = match create_method {
CreateMethod::Generate => {
let result =
pkcs11_session.generate_key(uri.object_label.as_deref(), usage);
let result = pkcs11_session
.clone()
.generate_key(uri.object_label.as_deref(), usage);

#[allow(clippy::unnested_or_patterns)]
match result {
Ok(_) => return Ok(()),
Ok(pkcs11_key) => pkcs11_key,

Err(pkcs11::GenerateKeyError::GenerateKeyFailed(
pkcs11_sys::CKR_FUNCTION_NOT_SUPPORTED,
)) |
// Some PKCS#11 implementations like Cryptoauthlib don't support `C_GenerateKey(CKM_GENERIC_SECRET_KEY_GEN)`
Err(pkcs11::GenerateKeyError::GenerateKeyFailed(
pkcs11_sys::CKR_MECHANISM_INVALID,
)) => (),
)) => continue,

Err(err) => return Err(crate::implementation::err_external(err)),
}
Expand All @@ -636,17 +637,49 @@ fn create_inner(
CreateMethod::Import(bytes) => {
// TODO: Verify if CAL actually smashes the stack for keys that are too large,
// and if not, if it returns a better error than CKR_GENERAL_ERROR
let result =
pkcs11_session.import_key(bytes, uri.object_label.as_deref(), usage);
let result = pkcs11_session.clone().import_key(
bytes,
uri.object_label.as_deref(),
usage,
);
match result {
Ok(_) => return Ok(()),
Ok(pkcs11_key) => pkcs11_key,

// No better error from some PKCS#11 implementations like Cryptoauthlib than CKR_GENERAL_ERROR
Err(pkcs11::ImportKeyError::CreateObjectFailed(_)) => (),
Err(pkcs11::ImportKeyError::CreateObjectFailed(_)) => continue,

Err(err) => return Err(crate::implementation::err_external(err)),
}
}
};

if let pkcs11::KeyUsage::Aes = usage {
// At this point we've successfully created an AES key but we don't know if the token supports AES-GCM specifically or not.
// So we do a dummy encryption.

let iv: &[u8; 12] = b"123456789012";
let aad = b"someaad";
let plaintext = b"someplaintext";
// We can use the block size of AES-256-GCM from openssl even though the token didn't necessarily use AES-256-GCM,
// because all AES ciphers have the same block size by definition.
let cipher = openssl::symm::Cipher::aes_256_gcm();
let mut ciphertext = vec![0_u8; plaintext.len() + cipher.block_size() + 16];

if pkcs11_object
.encrypt(iv, aad, plaintext, &mut ciphertext)
.is_ok()
{
return Ok(());
}

// Delete the object we just created...
if let Some(object_label) = &uri.object_label {
let _ = pkcs11_session.clone().delete_key(object_label);
}

// ... and continue to the next location.
} else {
return Ok(());
}
}
}
Expand Down
1 change: 1 addition & 0 deletions key/aziot-keys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
clippy::default_trait_access,
clippy::doc_markdown, // clippy wants "IoT" in a code fence
clippy::let_and_return,
clippy::let_underscore_drop,
clippy::let_unit_value,
clippy::missing_safety_doc,
clippy::shadow_unrelated,
Expand Down

0 comments on commit 79aae50

Please sign in to comment.