From 746e1be8ef78cf71effe9d4082a033be7fc8345f Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Wed, 5 Jul 2023 17:39:57 +0100 Subject: [PATCH 1/2] Cope with previously-emulated keys If a device has upgraded Android versions then the KeyMint device may also have been upgraded. If that's the case, then there may be keyblobs that were created in software on the old device, because it didn't support some feature. Watch out for these keys, and if encountered, try to import them into the current KeyMint device: - extract the key material from the key blob - add PKCS#8 wrapping for import Bug: 283077822 Bug: 296403357 Test: tested with ARC upgrade, see b/296403357 Change-Id: I146f7cfdaac9fe22b7bb6850b7e48ea113945902 --- keystore2/src/km_compat.rs | 7 +- keystore2/src/security_level.rs | 6 +- keystore2/src/sw_keyblob.rs | 228 +++++++++++++++++++++++++++++++- keystore2/src/utils.rs | 219 ++++++++++++++++++++++++++---- 4 files changed, 425 insertions(+), 35 deletions(-) diff --git a/keystore2/src/km_compat.rs b/keystore2/src/km_compat.rs index cd58fe44..03c9d027 100644 --- a/keystore2/src/km_compat.rs +++ b/keystore2/src/km_compat.rs @@ -37,6 +37,11 @@ use keystore2_crypto::{hmac_sha256, HMAC_SHA256_LEN}; /// final zero byte indicates that the blob is not software emulated.) pub const KEYMASTER_BLOB_HW_PREFIX: &[u8] = b"pKMblob\x00"; +/// Magic prefix used by the km_compat C++ code to mark a key that is owned by an +/// software emulation device that has been wrapped by km_compat. (The final one +/// byte indicates that the blob is software emulated.) +pub const KEYMASTER_BLOB_SW_PREFIX: &[u8] = b"pKMblob\x01"; + /// Key data associated with key generation/import. #[derive(Debug, PartialEq, Eq)] pub enum KeyImportData<'a> { @@ -94,7 +99,7 @@ fn wrap_keyblob(keyblob: &[u8]) -> anyhow::Result> { /// Return an unwrapped version of the provided `keyblob`, which may or may /// not be associated with the software emulation. -fn unwrap_keyblob(keyblob: &[u8]) -> KeyBlob { +pub fn unwrap_keyblob(keyblob: &[u8]) -> KeyBlob { if !keyblob.starts_with(KEYBLOB_PREFIX) { return KeyBlob::Raw(keyblob); } diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs index 830fbe11..8abff777 100644 --- a/keystore2/src/security_level.rs +++ b/keystore2/src/security_level.rs @@ -33,7 +33,7 @@ use crate::super_key::{KeyBlob, SuperKeyManager}; use crate::utils::{ check_device_attestation_permissions, check_key_permission, check_unique_id_attestation_permissions, is_device_id_attestation_tag, - key_characteristics_to_internal, uid_to_android_user, watchdog as wd, + key_characteristics_to_internal, uid_to_android_user, watchdog as wd, UNDEFINED_NOT_AFTER, }; use crate::{ database::{ @@ -79,10 +79,6 @@ pub struct KeystoreSecurityLevel { // Blob of 32 zeroes used as empty masking key. static ZERO_BLOB_32: &[u8] = &[0; 32]; -// Per RFC 5280 4.1.2.5, an undefined expiration (not-after) field should be set to GeneralizedTime -// 999912312359559, which is 253402300799000 ms from Jan 1, 1970. -const UNDEFINED_NOT_AFTER: i64 = 253402300799000i64; - impl KeystoreSecurityLevel { /// Creates a new security level instance wrapped in a /// BnKeystoreSecurityLevel proxy object. It also enables diff --git a/keystore2/src/sw_keyblob.rs b/keystore2/src/sw_keyblob.rs index 11a9b41c..dc828a0c 100644 --- a/keystore2/src/sw_keyblob.rs +++ b/keystore2/src/sw_keyblob.rs @@ -15,8 +15,6 @@ //! Code for parsing software-backed keyblobs, as emitted by the C++ reference implementation of //! KeyMint. -#![allow(dead_code)] - use crate::error::Error; use crate::ks_err; use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{ @@ -73,9 +71,135 @@ pub fn export_key( | KeyParameterValue::Algorithm(Algorithm::EC) => KeyFormat::PKCS8, _ => return Err(bloberr!("Unexpected algorithm {:?}", algo_val)), }; + + let key_material = match (format, algo_val) { + (KeyFormat::PKCS8, KeyParameterValue::Algorithm(Algorithm::EC)) => { + // Key material format depends on the curve. + let curve = get_tag_value(&combined, Tag::EC_CURVE) + .ok_or_else(|| bloberr!("Failed to determine curve for EC key!"))?; + match curve { + KeyParameterValue::EcCurve(EcCurve::CURVE_25519) => key_material, + KeyParameterValue::EcCurve(EcCurve::P_224) => { + pkcs8_wrap_nist_key(&key_material, EcCurve::P_224)? + } + KeyParameterValue::EcCurve(EcCurve::P_256) => { + pkcs8_wrap_nist_key(&key_material, EcCurve::P_256)? + } + KeyParameterValue::EcCurve(EcCurve::P_384) => { + pkcs8_wrap_nist_key(&key_material, EcCurve::P_384)? + } + KeyParameterValue::EcCurve(EcCurve::P_521) => { + pkcs8_wrap_nist_key(&key_material, EcCurve::P_521)? + } + _ => { + return Err(bloberr!("Unexpected EC curve {curve:?}")); + } + } + } + (KeyFormat::RAW, _) => key_material, + (format, algo) => { + return Err(bloberr!( + "Unsupported combination of {format:?} format for {algo:?} algorithm" + )); + } + }; Ok((format, key_material, combined)) } +/// DER-encoded `AlgorithmIdentifier` for a P-224 key. +const DER_ALGORITHM_ID_P224: &[u8] = &[ + 0x30, 0x10, // SEQUENCE (AlgorithmIdentifier) { + 0x06, 0x07, // OBJECT IDENTIFIER (algorithm) + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, // 1.2.840.10045.2.1 (ecPublicKey) + 0x06, 0x05, // OBJECT IDENTIFIER (param) + 0x2b, 0x81, 0x04, 0x00, 0x21, // 1.3.132.0.33 (secp224r1) } +]; + +/// DER-encoded `AlgorithmIdentifier` for a P-256 key. +const DER_ALGORITHM_ID_P256: &[u8] = &[ + 0x30, 0x13, // SEQUENCE (AlgorithmIdentifier) { + 0x06, 0x07, // OBJECT IDENTIFIER (algorithm) + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, // 1.2.840.10045.2.1 (ecPublicKey) + 0x06, 0x08, // OBJECT IDENTIFIER (param) + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, // 1.2.840.10045.3.1.7 (secp256r1) } +]; + +/// DER-encoded `AlgorithmIdentifier` for a P-384 key. +const DER_ALGORITHM_ID_P384: &[u8] = &[ + 0x30, 0x10, // SEQUENCE (AlgorithmIdentifier) { + 0x06, 0x07, // OBJECT IDENTIFIER (algorithm) + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, // 1.2.840.10045.2.1 (ecPublicKey) + 0x06, 0x05, // OBJECT IDENTIFIER (param) + 0x2b, 0x81, 0x04, 0x00, 0x22, // 1.3.132.0.34 (secp384r1) } +]; + +/// DER-encoded `AlgorithmIdentifier` for a P-384 key. +const DER_ALGORITHM_ID_P521: &[u8] = &[ + 0x30, 0x10, // SEQUENCE (AlgorithmIdentifier) { + 0x06, 0x07, // OBJECT IDENTIFIER (algorithm) + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, // 1.2.840.10045.2.1 (ecPublicKey) + 0x06, 0x05, // OBJECT IDENTIFIER (param) + 0x2b, 0x81, 0x04, 0x00, 0x23, // 1.3.132.0.35 (secp521r1) } +]; + +/// DER-encoded integer value zero. +const DER_VERSION_0: &[u8] = &[ + 0x02, // INTEGER + 0x01, // len + 0x00, // value 0 +]; + +/// Given a NIST curve EC key in the form of a DER-encoded `ECPrivateKey` +/// (RFC 5915 s3), wrap it in a DER-encoded PKCS#8 format (RFC 5208 s5). +fn pkcs8_wrap_nist_key(nist_key: &[u8], curve: EcCurve) -> Result> { + let der_alg_id = match curve { + EcCurve::P_224 => DER_ALGORITHM_ID_P224, + EcCurve::P_256 => DER_ALGORITHM_ID_P256, + EcCurve::P_384 => DER_ALGORITHM_ID_P384, + EcCurve::P_521 => DER_ALGORITHM_ID_P521, + _ => return Err(bloberr!("unknown curve {curve:?}")), + }; + + // Output format is: + // + // PrivateKeyInfo ::= SEQUENCE { + // version INTEGER, + // privateKeyAlgorithm AlgorithmIdentifier, + // privateKey OCTET STRING, + // } + // + // Start by building the OCTET STRING so we know its length. + let mut nist_key_octet_string = Vec::new(); + nist_key_octet_string.push(0x04); // OCTET STRING + add_der_len(&mut nist_key_octet_string, nist_key.len())?; + nist_key_octet_string.extend_from_slice(nist_key); + + let mut buf = Vec::new(); + buf.push(0x30); // SEQUENCE + add_der_len(&mut buf, DER_VERSION_0.len() + der_alg_id.len() + nist_key_octet_string.len())?; + buf.extend_from_slice(DER_VERSION_0); + buf.extend_from_slice(der_alg_id); + buf.extend_from_slice(&nist_key_octet_string); + Ok(buf) +} + +/// Append a DER-encoded length value to the given buffer. +fn add_der_len(buf: &mut Vec, len: usize) -> Result<()> { + if len <= 0x7f { + buf.push(len as u8) + } else if len <= 0xff { + buf.push(0x81); // One length octet to come + buf.push(len as u8); + } else if len <= 0xffff { + buf.push(0x82); // Two length octets to come + buf.push((len >> 8) as u8); + buf.push((len & 0xff) as u8); + } else { + return Err(bloberr!("Unsupported DER length {len}")); + } + Ok(()) +} + /// Plaintext key blob, with key characteristics. #[derive(PartialEq, Eq)] struct KeyBlob { @@ -809,4 +933,104 @@ mod tests { } } } + + #[test] + fn test_add_der_len() { + let tests = [ + (0, "00"), + (1, "01"), + (126, "7e"), + (127, "7f"), + (128, "8180"), + (129, "8181"), + (255, "81ff"), + (256, "820100"), + (257, "820101"), + (65535, "82ffff"), + ]; + for (input, want) in tests { + let mut got = Vec::new(); + add_der_len(&mut got, input).unwrap(); + assert_eq!(hex::encode(got), want, " for input length {input}"); + } + } + + #[test] + fn test_pkcs8_wrap_key_p256() { + // Key material taken from `ec_256_key` in + // hardware/interfaces/security/keymint/aidl/vts/function/KeyMintTest.cpp + let input = hex::decode(concat!( + "3025", // SEQUENCE (ECPrivateKey) + "020101", // INTEGER length 1 value 1 (version) + "0420", // OCTET STRING (privateKey) + "737c2ecd7b8d1940bf2930aa9b4ed3ff", + "941eed09366bc03299986481f3a4d859", + )) + .unwrap(); + let want = hex::decode(concat!( + // RFC 5208 s5 + "3041", // SEQUENCE (PrivateKeyInfo) { + "020100", // INTEGER length 1 value 0 (version) + "3013", // SEQUENCE length 0x13 (AlgorithmIdentifier) { + "0607", // OBJECT IDENTIFIER length 7 (algorithm) + "2a8648ce3d0201", // 1.2.840.10045.2.1 (ecPublicKey) + "0608", // OBJECT IDENTIFIER length 8 (param) + "2a8648ce3d030107", // 1.2.840.10045.3.1.7 (secp256r1) + // } end SEQUENCE (AlgorithmIdentifier) + "0427", // OCTET STRING (privateKey) holding... + "3025", // SEQUENCE (ECPrivateKey) + "020101", // INTEGER length 1 value 1 (version) + "0420", // OCTET STRING length 0x20 (privateKey) + "737c2ecd7b8d1940bf2930aa9b4ed3ff", + "941eed09366bc03299986481f3a4d859", + // } end SEQUENCE (ECPrivateKey) + // } end SEQUENCE (PrivateKeyInfo) + )) + .unwrap(); + let got = pkcs8_wrap_nist_key(&input, EcCurve::P_256).unwrap(); + assert_eq!(hex::encode(got), hex::encode(want), " for input {}", hex::encode(input)); + } + + #[test] + fn test_pkcs8_wrap_key_p521() { + // Key material taken from `ec_521_key` in + // hardware/interfaces/security/keymint/aidl/vts/function/KeyMintTest.cpp + let input = hex::decode(concat!( + "3047", // SEQUENCE length 0xd3 (ECPrivateKey) + "020101", // INTEGER length 1 value 1 (version) + "0442", // OCTET STRING length 0x42 (privateKey) + "0011458c586db5daa92afab03f4fe46a", + "a9d9c3ce9a9b7a006a8384bec4c78e8e", + "9d18d7d08b5bcfa0e53c75b064ad51c4", + "49bae0258d54b94b1e885ded08ed4fb2", + "5ce9", + // } end SEQUENCE (ECPrivateKey) + )) + .unwrap(); + let want = hex::decode(concat!( + // RFC 5208 s5 + "3060", // SEQUENCE (PrivateKeyInfo) { + "020100", // INTEGER length 1 value 0 (version) + "3010", // SEQUENCE length 0x10 (AlgorithmIdentifier) { + "0607", // OBJECT IDENTIFIER length 7 (algorithm) + "2a8648ce3d0201", // 1.2.840.10045.2.1 (ecPublicKey) + "0605", // OBJECT IDENTIFIER length 5 (param) + "2b81040023", // 1.3.132.0.35 (secp521r1) + // } end SEQUENCE (AlgorithmIdentifier) + "0449", // OCTET STRING (privateKey) holding... + "3047", // SEQUENCE (ECPrivateKey) + "020101", // INTEGER length 1 value 1 (version) + "0442", // OCTET STRING length 0x42 (privateKey) + "0011458c586db5daa92afab03f4fe46a", + "a9d9c3ce9a9b7a006a8384bec4c78e8e", + "9d18d7d08b5bcfa0e53c75b064ad51c4", + "49bae0258d54b94b1e885ded08ed4fb2", + "5ce9", + // } end SEQUENCE (ECPrivateKey) + // } end SEQUENCE (PrivateKeyInfo) + )) + .unwrap(); + let got = pkcs8_wrap_nist_key(&input, EcCurve::P_521).unwrap(); + assert_eq!(hex::encode(got), hex::encode(want), " for input {}", hex::encode(input)); + } } diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs index 4fd9c8dd..74a5ae6a 100644 --- a/keystore2/src/utils.rs +++ b/keystore2/src/utils.rs @@ -28,8 +28,8 @@ use crate::{ raw_device::KeyMintDevice, }; use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{ - IKeyMintDevice::IKeyMintDevice, KeyCharacteristics::KeyCharacteristics, - KeyParameter::KeyParameter as KmKeyParameter, Tag::Tag, + Algorithm::Algorithm, IKeyMintDevice::IKeyMintDevice, KeyCharacteristics::KeyCharacteristics, + KeyParameter::KeyParameter as KmKeyParameter, KeyParameterValue::KeyParameterValue, Tag::Tag, }; use android_os_permissions_aidl::aidl::android::os::IPermissionController; use android_security_apc::aidl::android::security::apc::{ @@ -49,6 +49,10 @@ use keystore2_apc_compat::{ use keystore2_crypto::{aes_gcm_decrypt, aes_gcm_encrypt, ZVec}; use std::iter::IntoIterator; +/// Per RFC 5280 4.1.2.5, an undefined expiration (not-after) field should be set to GeneralizedTime +/// 999912312359559, which is 253402300799000 ms from Jan 1, 1970. +pub const UNDEFINED_NOT_AFTER: i64 = 253402300799000i64; + /// This function uses its namesake in the permission module and in /// combination with with_calling_sid from the binder crate to check /// if the caller has the given keystore permission. @@ -166,6 +170,119 @@ pub fn key_characteristics_to_internal( .collect() } +/// Import a keyblob that is of the format used by the software C++ KeyMint implementation. After +/// successful import, invoke both the `new_blob_handler` and `km_op` closures. On success a tuple +/// of the `km_op`s result and the optional upgraded blob is returned. +fn import_keyblob_and_perform_op( + km_dev: &dyn IKeyMintDevice, + inner_keyblob: &[u8], + upgrade_params: &[KmKeyParameter], + km_op: KmOp, + new_blob_handler: NewBlobHandler, +) -> Result<(T, Option>)> +where + KmOp: Fn(&[u8]) -> Result, + NewBlobHandler: FnOnce(&[u8]) -> Result<()>, +{ + let (format, key_material, mut chars) = + crate::sw_keyblob::export_key(inner_keyblob, upgrade_params)?; + log::debug!( + "importing {:?} key material (len={}) with original chars={:?}", + format, + key_material.len(), + chars + ); + let asymmetric = chars.iter().any(|kp| { + kp.tag == Tag::ALGORITHM + && (kp.value == KeyParameterValue::Algorithm(Algorithm::RSA) + || (kp.value == KeyParameterValue::Algorithm(Algorithm::EC))) + }); + + // Combine the characteristics of the previous keyblob with the upgrade parameters (which might + // include special things like APPLICATION_ID / APPLICATION_DATA). + chars.extend_from_slice(upgrade_params); + + // Now filter out values from the existing keyblob that shouldn't be set on import, either + // because they are per-operation parameter or because they are auto-added by KeyMint itself. + let mut import_params: Vec = chars + .into_iter() + .filter(|kp| { + !matches!( + kp.tag, + Tag::ORIGIN + | Tag::ROOT_OF_TRUST + | Tag::OS_VERSION + | Tag::OS_PATCHLEVEL + | Tag::UNIQUE_ID + | Tag::ATTESTATION_CHALLENGE + | Tag::ATTESTATION_APPLICATION_ID + | Tag::ATTESTATION_ID_BRAND + | Tag::ATTESTATION_ID_DEVICE + | Tag::ATTESTATION_ID_PRODUCT + | Tag::ATTESTATION_ID_SERIAL + | Tag::ATTESTATION_ID_IMEI + | Tag::ATTESTATION_ID_MEID + | Tag::ATTESTATION_ID_MANUFACTURER + | Tag::ATTESTATION_ID_MODEL + | Tag::VENDOR_PATCHLEVEL + | Tag::BOOT_PATCHLEVEL + | Tag::DEVICE_UNIQUE_ATTESTATION + | Tag::ATTESTATION_ID_SECOND_IMEI + | Tag::NONCE + | Tag::MAC_LENGTH + | Tag::CERTIFICATE_SERIAL + | Tag::CERTIFICATE_SUBJECT + | Tag::CERTIFICATE_NOT_BEFORE + | Tag::CERTIFICATE_NOT_AFTER + ) + }) + .collect(); + + // Now that any previous values have been removed, add any additional parameters that needed for + // import. In particular, if we are generating/importing an asymmetric key, we need to make sure + // that NOT_BEFORE and NOT_AFTER are present. + if asymmetric { + import_params.push(KmKeyParameter { + tag: Tag::CERTIFICATE_NOT_BEFORE, + value: KeyParameterValue::DateTime(0), + }); + import_params.push(KmKeyParameter { + tag: Tag::CERTIFICATE_NOT_AFTER, + value: KeyParameterValue::DateTime(UNDEFINED_NOT_AFTER), + }); + } + log::debug!("import parameters={import_params:?}"); + + let creation_result = { + let _wp = watchdog::watch_millis( + "In utils::import_keyblob_and_perform_op: calling importKey.", + 500, + ); + map_km_error(km_dev.importKey(&import_params, format, &key_material, None)) + } + .context(ks_err!("Upgrade failed."))?; + + // Note that the importKey operation will produce key characteristics that may be different + // than are already stored in Keystore's SQL database. In particular, the KeyMint + // implementation will now mark the key as `Origin::IMPORTED` not `Origin::GENERATED`, and + // the security level for characteristics will now be `TRUSTED_ENVIRONMENT` not `SOFTWARE`. + // + // However, the DB metadata still accurately reflects the original origin of the key, and + // so we leave the values as-is (and so any `KeyInfo` retrieved in the Java layer will get the + // same results before and after import). + // + // Note that this also applies to the `USAGE_COUNT_LIMIT` parameter -- if the key has already + // been used, then the DB version of the parameter will be (and will continue to be) lower + // than the original count bound to the keyblob. This means that Keystore's policing of + // usage counts will continue where it left off. + + new_blob_handler(&creation_result.keyBlob).context(ks_err!("calling new_blob_handler."))?; + + km_op(&creation_result.keyBlob) + .map(|v| (v, Some(creation_result.keyBlob))) + .context(ks_err!("Calling km_op after upgrade.")) +} + /// Upgrade a keyblob then invoke both the `new_blob_handler` and the `km_op` closures. On success /// a tuple of the `km_op`s result and the optional upgraded blob is returned. fn upgrade_keyblob_and_perform_op( @@ -221,33 +338,81 @@ where km_op, new_blob_handler, ), - // Some devices have been known to upgrade their Keymaster device to be a KeyMint - // device with a new release of Android. If this is the case, then any pre-upgrade - // keyblobs will have the km_compat prefix attached to them. - // - // This prefix gets stripped by the km_compat layer when used pre-upgrade, but after - // the upgrade the keyblob will be passed as-is to the KeyMint device, which probably - // won't expect to see the km_compat prefix. - // - // So if a keyblob: - // a) gets rejected with INVALID_KEY_BLOB - // b) when sent to a KeyMint (not km_compat) device - // c) and has the km_compat magic prefix - // d) and was not a software-emulated key pre-upgrade - // then strip the prefix and attempt a key upgrade. Err(Error::Km(ErrorCode::INVALID_KEY_BLOB)) - if km_dev_version >= KeyMintDevice::KEY_MINT_V1 - && key_blob.starts_with(km_compat::KEYMASTER_BLOB_HW_PREFIX) => + if km_dev_version >= KeyMintDevice::KEY_MINT_V1 => { - log::info!("found apparent km_compat(Keymaster) blob, attempt strip-and-upgrade"); - let inner_keyblob = &key_blob[km_compat::KEYMASTER_BLOB_HW_PREFIX.len()..]; - upgrade_keyblob_and_perform_op( - km_dev, - inner_keyblob, - upgrade_params, - km_op, - new_blob_handler, - ) + // A KeyMint (not Keymaster via km_compat) device says that this is an invalid keyblob. + // + // This may be because the keyblob was created before an Android upgrade, and as part of + // the device upgrade the underlying Keymaster/KeyMint implementation has been upgraded. + // + // If that's the case, there are three possible scenarios: + if key_blob.starts_with(km_compat::KEYMASTER_BLOB_HW_PREFIX) { + // 1) The keyblob was created in hardware by the km_compat C++ code, using a prior + // Keymaster implementation, and wrapped. + // + // In this case, the keyblob will have the km_compat magic prefix, including the + // marker that indicates that this was a hardware-backed key. + // + // The inner keyblob should still be recognized by the hardware implementation, so + // strip the prefix and attempt a key upgrade. + log::info!( + "found apparent km_compat(Keymaster) HW blob, attempt strip-and-upgrade" + ); + let inner_keyblob = &key_blob[km_compat::KEYMASTER_BLOB_HW_PREFIX.len()..]; + upgrade_keyblob_and_perform_op( + km_dev, + inner_keyblob, + upgrade_params, + km_op, + new_blob_handler, + ) + } else if key_blob.starts_with(km_compat::KEYMASTER_BLOB_SW_PREFIX) { + // 2) The keyblob was created in software by the km_compat C++ code because a prior + // Keymaster implementation did not support ECDH (which was only added in KeyMint). + // + // In this case, the keyblob with have the km_compat magic prefix, but with the + // marker that indicates that this was a software-emulated key. + // + // The inner keyblob should be in the format produced by the C++ reference + // implementation of KeyMint. Extract the key material and import it into the + // current KeyMint device. + log::info!("found apparent km_compat(Keymaster) SW blob, attempt strip-and-import"); + let inner_keyblob = &key_blob[km_compat::KEYMASTER_BLOB_SW_PREFIX.len()..]; + import_keyblob_and_perform_op( + km_dev, + inner_keyblob, + upgrade_params, + km_op, + new_blob_handler, + ) + } else if let km_compat::KeyBlob::Wrapped(inner_keyblob) = + km_compat::unwrap_keyblob(key_blob) + { + // 3) The keyblob was created in software by km_compat.rs because a prior KeyMint + // implementation did not support a feature present in the current KeyMint spec. + // (For example, a curve 25519 key created when the device only supported KeyMint + // v1). + // + // In this case, the keyblob with have the km_compat.rs wrapper around it to + // indicate that this was a software-emulated key. + // + // The inner keyblob should be in the format produced by the C++ reference + // implementation of KeyMint. Extract the key material and import it into the + // current KeyMint device. + log::info!( + "found apparent km_compat.rs(KeyMint) SW blob, attempt strip-and-import" + ); + import_keyblob_and_perform_op( + km_dev, + inner_keyblob, + upgrade_params, + km_op, + new_blob_handler, + ) + } else { + Err(Error::Km(ErrorCode::INVALID_KEY_BLOB)).context(ks_err!("Calling km_op")) + } } r => r.map(|v| (v, None)).context(ks_err!("Calling km_op.")), } From 093811ef2221145defd1cd3621e813c6a7018f96 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Thu, 9 Nov 2023 08:32:02 +0000 Subject: [PATCH 2/2] Flag for import of previously-emulated keys Test: build Bug: 283077822 Change-Id: I28f673b6eb905c2953fbb91f2658ff224ca0e21c --- keystore2/aconfig/flags.aconfig | 8 ++++++++ keystore2/src/utils.rs | 11 +++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/keystore2/aconfig/flags.aconfig b/keystore2/aconfig/flags.aconfig index 5d2a422c..02716da5 100644 --- a/keystore2/aconfig/flags.aconfig +++ b/keystore2/aconfig/flags.aconfig @@ -15,3 +15,11 @@ flag { bug: "307460850" is_fixed_read_only: true } + +flag { + name: "import_previously_emulated_keys" + namespace: "hardware_backed_security" + description: "Include support for importing keys that were previously software-emulated into KeyMint" + bug: "283077822" + is_fixed_read_only: true +} \ No newline at end of file diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs index 74a5ae6a..174a22ba 100644 --- a/keystore2/src/utils.rs +++ b/keystore2/src/utils.rs @@ -367,7 +367,9 @@ where km_op, new_blob_handler, ) - } else if key_blob.starts_with(km_compat::KEYMASTER_BLOB_SW_PREFIX) { + } else if keystore2_flags::import_previously_emulated_keys() + && key_blob.starts_with(km_compat::KEYMASTER_BLOB_SW_PREFIX) + { // 2) The keyblob was created in software by the km_compat C++ code because a prior // Keymaster implementation did not support ECDH (which was only added in KeyMint). // @@ -386,9 +388,10 @@ where km_op, new_blob_handler, ) - } else if let km_compat::KeyBlob::Wrapped(inner_keyblob) = - km_compat::unwrap_keyblob(key_blob) - { + } else if let (true, km_compat::KeyBlob::Wrapped(inner_keyblob)) = ( + keystore2_flags::import_previously_emulated_keys(), + km_compat::unwrap_keyblob(key_blob), + ) { // 3) The keyblob was created in software by km_compat.rs because a prior KeyMint // implementation did not support a feature present in the current KeyMint spec. // (For example, a curve 25519 key created when the device only supported KeyMint