diff --git a/keystore2/src/crypto/Android.bp b/keystore2/src/crypto/Android.bp index 35fc5a90..c78ae418 100644 --- a/keystore2/src/crypto/Android.bp +++ b/keystore2/src/crypto/Android.bp @@ -74,7 +74,7 @@ rust_bindgen { "--allowlist-function", "AES_gcm_encrypt", "--allowlist-function", "AES_gcm_decrypt", "--allowlist-function", "CreateKeyId", - "--allowlist-function", "generateKeyFromPassword", + "--allowlist-function", "PBKDF2", "--allowlist-function", "HKDFExtract", "--allowlist-function", "HKDFExpand", "--allowlist-function", "ECDHComputeKey", diff --git a/keystore2/src/crypto/crypto.cpp b/keystore2/src/crypto/crypto.cpp index 7feeaff6..56d8de6c 100644 --- a/keystore2/src/crypto/crypto.cpp +++ b/keystore2/src/crypto/crypto.cpp @@ -141,7 +141,8 @@ bool AES_gcm_decrypt(const uint8_t* in, uint8_t* out, size_t len, const uint8_t* EVP_DecryptUpdate(ctx.get(), out_pos, &out_len, in, len); out_pos += out_len; if (!EVP_DecryptFinal_ex(ctx.get(), out_pos, &out_len)) { - ALOGE("Failed to decrypt blob; ciphertext or tag is likely corrupted"); + // No error log here; this is expected when trying two different keys to see which one + // works. The callers handle the error appropriately. return false; } out_pos += out_len; @@ -191,8 +192,7 @@ static constexpr size_t SALT_SIZE = 16; // Copied from system/security/keystore/user_state.cpp. -void generateKeyFromPassword(uint8_t* key, size_t key_len, const char* pw, size_t pw_len, - const uint8_t* salt) { +void PBKDF2(uint8_t* key, size_t key_len, const char* pw, size_t pw_len, const uint8_t* salt) { const EVP_MD* digest = EVP_sha256(); // SHA1 was used prior to increasing the key size diff --git a/keystore2/src/crypto/crypto.hpp b/keystore2/src/crypto/crypto.hpp index 4a161e6c..f67f6407 100644 --- a/keystore2/src/crypto/crypto.hpp +++ b/keystore2/src/crypto/crypto.hpp @@ -37,8 +37,7 @@ extern "C" { bool CreateKeyId(const uint8_t* key_blob, size_t len, km_id_t* out_id); // The salt parameter must be non-nullptr and point to 16 bytes of data. - void generateKeyFromPassword(uint8_t* key, size_t key_len, const char* pw, - size_t pw_len, const uint8_t* salt); + void PBKDF2(uint8_t* key, size_t key_len, const char* pw, size_t pw_len, const uint8_t* salt); #include "openssl/digest.h" #include "openssl/ec_key.h" diff --git a/keystore2/src/crypto/lib.rs b/keystore2/src/crypto/lib.rs index 84346511..09b84ec8 100644 --- a/keystore2/src/crypto/lib.rs +++ b/keystore2/src/crypto/lib.rs @@ -19,10 +19,10 @@ mod error; pub mod zvec; pub use error::Error; use keystore2_crypto_bindgen::{ - extractSubjectFromCertificate, generateKeyFromPassword, hmacSha256, randomBytes, - AES_gcm_decrypt, AES_gcm_encrypt, ECDHComputeKey, ECKEYGenerateKey, ECKEYMarshalPrivateKey, - ECKEYParsePrivateKey, ECPOINTOct2Point, ECPOINTPoint2Oct, EC_KEY_free, EC_KEY_get0_public_key, - EC_POINT_free, HKDFExpand, HKDFExtract, EC_KEY, EC_MAX_BYTES, EC_POINT, EVP_MAX_MD_SIZE, + extractSubjectFromCertificate, hmacSha256, randomBytes, AES_gcm_decrypt, AES_gcm_encrypt, + ECDHComputeKey, ECKEYGenerateKey, ECKEYMarshalPrivateKey, ECKEYParsePrivateKey, + ECPOINTOct2Point, ECPOINTPoint2Oct, EC_KEY_free, EC_KEY_get0_public_key, EC_POINT_free, + HKDFExpand, HKDFExtract, EC_KEY, EC_MAX_BYTES, EC_POINT, EVP_MAX_MD_SIZE, PBKDF2, }; use std::convert::TryFrom; use std::convert::TryInto; @@ -172,7 +172,7 @@ pub fn aes_gcm_encrypt(plaintext: &[u8], key: &[u8]) -> Result<(Vec, Vec } } -/// Represents a "password" that can be used to key the PBKDF2 algorithm. +/// A high-entropy synthetic password from which an AES key may be derived. pub enum Password<'a> { /// Borrow an existing byte array Ref(&'a [u8]), @@ -194,25 +194,28 @@ impl<'a> Password<'a> { } } - /// Generate a key from the given password and salt. - /// The salt must be exactly 16 bytes long. - /// Two key sizes are accepted: 16 and 32 bytes. - pub fn derive_key(&self, salt: &[u8], key_length: usize) -> Result { + /// Derives a key from the given password and salt, using PBKDF2 with 8192 iterations. + /// + /// The salt length must be 16 bytes, and the output key length must be 16 or 32 bytes. + /// + /// This function exists only for backwards compatibility reasons. Keystore now receives only + /// high-entropy synthetic passwords, which do not require key stretching. + pub fn derive_key_pbkdf2(&self, salt: &[u8], out_len: usize) -> Result { if salt.len() != SALT_LENGTH { return Err(Error::InvalidSaltLength); } - match key_length { + match out_len { AES_128_KEY_LENGTH | AES_256_KEY_LENGTH => {} _ => return Err(Error::InvalidKeyLength), } let pw = self.get_key(); - let mut result = ZVec::new(key_length)?; + let mut result = ZVec::new(out_len)?; // Safety: We checked that the salt is exactly 16 bytes long. The other pointers are valid, // and have matching lengths. unsafe { - generateKeyFromPassword( + PBKDF2( result.as_mut_ptr(), result.len(), pw.as_ptr() as *const std::os::raw::c_char, @@ -224,6 +227,13 @@ impl<'a> Password<'a> { Ok(result) } + /// Derives a key from the given high-entropy synthetic password and salt, using HKDF. + pub fn derive_key_hkdf(&self, salt: &[u8], out_len: usize) -> Result { + let prk = hkdf_extract(self.get_key(), salt)?; + let info = []; + hkdf_expand(out_len, &prk, &info) + } + /// Try to make another Password object with the same data. pub fn try_clone(&self) -> Result, Error> { Ok(Password::Owned(ZVec::try_from(self.get_key())?)) @@ -471,9 +481,7 @@ pub fn parse_subject_from_certificate(cert_buf: &[u8]) -> Result, Error> mod tests { use super::*; - use keystore2_crypto_bindgen::{ - generateKeyFromPassword, AES_gcm_decrypt, AES_gcm_encrypt, CreateKeyId, - }; + use keystore2_crypto_bindgen::{AES_gcm_decrypt, AES_gcm_encrypt, CreateKeyId, PBKDF2}; #[test] fn test_wrapper_roundtrip() { @@ -535,21 +543,15 @@ mod tests { } #[test] - fn test_generate_key_from_password() { + fn test_pbkdf2() { let mut key = vec![0; 16]; let pw = [0; 16]; let salt = [0; 16]; // SAFETY: The pointers are obtained from references so they are valid, the salt is the - // expected length, the other lengths match the lengths of the arrays, and - // `generateKeyFromPassword` doesn't access them after it returns. + // expected length, the other lengths match the lengths of the arrays, and `PBKDF2` doesn't + // access them after it returns. unsafe { - generateKeyFromPassword( - key.as_mut_ptr(), - key.len(), - pw.as_ptr(), - pw.len(), - salt.as_ptr(), - ); + PBKDF2(key.as_mut_ptr(), key.len(), pw.as_ptr(), pw.len(), salt.as_ptr()); } assert_ne!(key, vec![0; 16]); } diff --git a/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs b/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs index 8b8843dd..fb4c9ad7 100644 --- a/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs +++ b/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs @@ -144,7 +144,8 @@ fuzz_target!(|commands: Vec| { let _res = aes_gcm_encrypt(plaintext, key_aes_encrypt); } FuzzCommand::Password { pw, salt, key_length } => { - let _res = Password::from(pw).derive_key(salt, key_length % MAX_SIZE_MODIFIER); + let _res = + Password::from(pw).derive_key_pbkdf2(salt, key_length % MAX_SIZE_MODIFIER); } FuzzCommand::HkdfExtract { hkdf_secret, hkdf_salt } => { let _res = hkdf_extract(hkdf_secret, hkdf_salt); diff --git a/keystore2/src/legacy_blob.rs b/keystore2/src/legacy_blob.rs index 2ffcc711..2bb7f27b 100644 --- a/keystore2/src/legacy_blob.rs +++ b/keystore2/src/legacy_blob.rs @@ -1313,7 +1313,7 @@ impl LegacyBlobLoader { Blob { flags, value: BlobValue::PwEncrypted { iv, tag, data, salt, key_size } } => { if (flags & flags::ENCRYPTED) != 0 { let key = pw - .derive_key(&salt, key_size) + .derive_key_pbkdf2(&salt, key_size) .context(ks_err!("Failed to derive key from password."))?; let blob = aes_gcm_decrypt(&data, &iv, &tag, &key) .context(ks_err!("while trying to decrypt legacy super key blob."))?; @@ -1953,7 +1953,7 @@ mod test { std::fs::create_dir(&*temp_dir.build().push("user_0")).unwrap(); let pw: Password = PASSWORD.into(); - let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap()); + let pw_key = TestKey(pw.derive_key_pbkdf2(SUPERKEY_SALT, 32).unwrap()); let super_key = Arc::new(TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap())); @@ -2040,7 +2040,7 @@ mod test { std::fs::create_dir(&*temp_dir.build().push("user_0")).unwrap(); let pw: Password = PASSWORD.into(); - let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap()); + let pw_key = TestKey(pw.derive_key_pbkdf2(SUPERKEY_SALT, 32).unwrap()); let super_key = Arc::new(TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap())); @@ -2128,7 +2128,7 @@ mod test { std::fs::create_dir(&*temp_dir.build().push("user_0")).unwrap(); let pw: Password = PASSWORD.into(); - let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap()); + let pw_key = TestKey(pw.derive_key_pbkdf2(SUPERKEY_SALT, 32).unwrap()); let super_key = Arc::new(TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap())); diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs index 3992eb2f..9c17ccbf 100644 --- a/keystore2/src/super_key.rs +++ b/keystore2/src/super_key.rs @@ -532,11 +532,17 @@ impl SuperKeyManager { (Some(&EncryptedBy::Password), Some(salt), Some(iv), Some(tag)) => { // Note that password encryption is AES no matter the value of algorithm. let key = pw - .derive_key(salt, AES_256_KEY_LENGTH) - .context(ks_err!("Failed to generate key from password."))?; + .derive_key_hkdf(salt, AES_256_KEY_LENGTH) + .context(ks_err!("Failed to derive key from password."))?; - aes_gcm_decrypt(blob, iv, tag, &key) - .context(ks_err!("Failed to decrypt key blob."))? + aes_gcm_decrypt(blob, iv, tag, &key).or_else(|_e| { + // Handle old key stored before the switch to HKDF. + let key = pw + .derive_key_pbkdf2(salt, AES_256_KEY_LENGTH) + .context(ks_err!("Failed to derive key from password (PBKDF2)."))?; + aes_gcm_decrypt(blob, iv, tag, &key) + .context(ks_err!("Failed to decrypt key blob.")) + })? } (enc_by, salt, iv, tag) => { return Err(Error::Rc(ResponseCode::VALUE_CORRUPTED)).context(ks_err!( @@ -563,14 +569,20 @@ impl SuperKeyManager { } /// Encrypts the super key from a key derived from the password, before storing in the database. + /// This does not stretch the password; i.e., it assumes that the password is a high-entropy + /// synthetic password, not a low-entropy user provided password. pub fn encrypt_with_password( super_key: &[u8], pw: &Password, ) -> Result<(Vec, BlobMetaData)> { let salt = generate_salt().context("In encrypt_with_password: Failed to generate salt.")?; - let derived_key = pw - .derive_key(&salt, AES_256_KEY_LENGTH) - .context(ks_err!("Failed to derive password."))?; + let derived_key = if android_security_flags::fix_unlocked_device_required_keys_v2() { + pw.derive_key_hkdf(&salt, AES_256_KEY_LENGTH) + .context(ks_err!("Failed to derive key from password."))? + } else { + pw.derive_key_pbkdf2(&salt, AES_256_KEY_LENGTH) + .context(ks_err!("Failed to derive password."))? + }; let mut metadata = BlobMetaData::new(); metadata.add(BlobMetaEntry::EncryptedBy(EncryptedBy::Password)); metadata.add(BlobMetaEntry::Salt(salt)); diff --git a/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs b/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs index c2cd3adf..0335159a 100644 --- a/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs +++ b/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs @@ -174,7 +174,7 @@ fn keystore2_encrypted_characteristics() -> anyhow::Result<()> { // Create keystore file layout for user_99. let pw: Password = PASSWORD.into(); - let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap()); + let pw_key = TestKey(pw.derive_key_pbkdf2(SUPERKEY_SALT, 32).unwrap()); let super_key = TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap()); @@ -427,7 +427,7 @@ fn keystore2_encrypted_certificates() -> anyhow::Result<()> { // Create keystore file layout for user_98. let pw: Password = PASSWORD.into(); - let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap()); + let pw_key = TestKey(pw.derive_key_pbkdf2(SUPERKEY_SALT, 32).unwrap()); let super_key = TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap());