Merge changes I1a855726,Ib7976671,I76e5ee5a into main am: 0e77b347e7

Original change: https://android-review.googlesource.com/c/platform/system/security/+/2912655

Change-Id: Ie1183739d25cccea727d3599c11827f81fc0c4d1
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

8 files changed, 60 insertions, 46 deletions

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<u8>, Vec<u8>
     }
 }
 
-/// 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<ZVec, Error> {
+    /// 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<ZVec, Error> {
         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<ZVec, Error> {
+        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<Password<'static>, Error> {
         Ok(Password::Owned(ZVec::try_from(self.get_key())?))
@@ -471,9 +481,7 @@ pub fn parse_subject_from_certificate(cert_buf: &[u8]) -> Result<Vec<u8>, 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<FuzzCommand>| {
                 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."))?;
-
-                    aes_gcm_decrypt(blob, iv, tag, &key)
-                        .context(ks_err!("Failed to decrypt key blob."))?
+                        .derive_key_hkdf(salt, AES_256_KEY_LENGTH)
+                        .context(ks_err!("Failed to derive key from password."))?;
+
+                    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<u8>, 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());