Drop ancient default salt value
A value for the salt is present in all code paths:
- All callers of `Password::derive_key` pass a `Some(..)` value for the
salt. Remove this `Some` everywhere and...
- Change the signature of `derive_key` to expect a `&[u8]` for `salt`
rather than an `Option<&[u8]>`.
- `Password::derive_key` is the only caller of `generateKeyFromPassword`
(via bindgen to C++-land), so...
- Make it clear that the C++ `generateKeyFromPassword()` function
expects 16 bytes of `salt`.
Bug: 172121323
Test: compile, TreeHugger
Change-Id: I41966cce96b5371785680c13bacfc5f95969372a
diff --git a/keystore2/src/crypto/crypto.cpp b/keystore2/src/crypto/crypto.cpp
index 34a9a40..6de3be7 100644
--- a/keystore2/src/crypto/crypto.cpp
+++ b/keystore2/src/crypto/crypto.cpp
@@ -192,16 +192,6 @@
void generateKeyFromPassword(uint8_t* key, size_t key_len, const char* pw, size_t pw_len,
const uint8_t* salt) {
- size_t saltSize;
- if (salt != nullptr) {
- saltSize = SALT_SIZE;
- } else {
- // Pre-gingerbread used this hardwired salt, readMasterKey will rewrite these when found
- salt = reinterpret_cast<const uint8_t*>("keystore");
- // sizeof = 9, not strlen = 8
- saltSize = sizeof("keystore");
- }
-
const EVP_MD* digest = EVP_sha256();
// SHA1 was used prior to increasing the key size
@@ -209,7 +199,7 @@
digest = EVP_sha1();
}
- PKCS5_PBKDF2_HMAC(pw, pw_len, salt, saltSize, 8192, digest, key_len, key);
+ PKCS5_PBKDF2_HMAC(pw, pw_len, salt, SALT_SIZE, 8192, digest, key_len, key);
}
// New code.
diff --git a/keystore2/src/crypto/crypto.hpp b/keystore2/src/crypto/crypto.hpp
index d66532f..4a161e6 100644
--- a/keystore2/src/crypto/crypto.hpp
+++ b/keystore2/src/crypto/crypto.hpp
@@ -36,6 +36,7 @@
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);
diff --git a/keystore2/src/crypto/lib.rs b/keystore2/src/crypto/lib.rs
index 14bdf04..e925180 100644
--- a/keystore2/src/crypto/lib.rs
+++ b/keystore2/src/crypto/lib.rs
@@ -197,24 +197,16 @@
/// 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: Option<&[u8]>, key_length: usize) -> Result<ZVec, Error> {
- let pw = self.get_key();
-
- let salt: *const u8 = match salt {
- Some(s) => {
- if s.len() != SALT_LENGTH {
- return Err(Error::InvalidSaltLength);
- }
- s.as_ptr()
- }
- None => std::ptr::null(),
- };
-
+ pub fn derive_key(&self, salt: &[u8], key_length: usize) -> Result<ZVec, Error> {
+ if salt.len() != SALT_LENGTH {
+ return Err(Error::InvalidSaltLength);
+ }
match key_length {
AES_128_KEY_LENGTH | AES_256_KEY_LENGTH => {}
_ => return Err(Error::InvalidKeyLength),
}
+ let pw = self.get_key();
let mut result = ZVec::new(key_length)?;
unsafe {
@@ -223,7 +215,7 @@
result.len(),
pw.as_ptr() as *const std::os::raw::c_char,
pw.len(),
- salt,
+ salt.as_ptr(),
)
};
@@ -541,9 +533,9 @@
fn test_generate_key_from_password() {
let mut key = vec![0; 16];
let pw = vec![0; 16];
- let mut salt = vec![0; 16];
+ let salt = vec![0; 16];
unsafe {
- generateKeyFromPassword(key.as_mut_ptr(), 16, pw.as_ptr(), 16, salt.as_mut_ptr());
+ generateKeyFromPassword(key.as_mut_ptr(), 16, pw.as_ptr(), 16, salt.as_ptr());
}
assert_ne!(key, vec![0; 16]);
}
diff --git a/keystore2/src/legacy_blob.rs b/keystore2/src/legacy_blob.rs
index d75bfd2..1c43a04 100644
--- a/keystore2/src/legacy_blob.rs
+++ b/keystore2/src/legacy_blob.rs
@@ -1348,7 +1348,7 @@
Blob { flags, value: BlobValue::PwEncrypted { iv, tag, data, salt, key_size } } => {
if (flags & flags::ENCRYPTED) != 0 {
let key = pw
- .derive_key(Some(&salt), key_size)
+ .derive_key(&salt, key_size)
.context("In load_super_key: Failed to derive key from password.")?;
let blob = aes_gcm_decrypt(&data, &iv, &tag, &key).context(
"In load_super_key: while trying to decrypt legacy super key blob.",
@@ -1993,7 +1993,7 @@
std::fs::create_dir(&*temp_dir.build().push("user_0")).unwrap();
let pw: Password = PASSWORD.into();
- let pw_key = TestKey(pw.derive_key(Some(SUPERKEY_SALT), 32).unwrap());
+ let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap());
let super_key =
Arc::new(TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap()));
@@ -2080,7 +2080,7 @@
std::fs::create_dir(&*temp_dir.build().push("user_0")).unwrap();
let pw: Password = PASSWORD.into();
- let pw_key = TestKey(pw.derive_key(Some(SUPERKEY_SALT), 32).unwrap());
+ let pw_key = TestKey(pw.derive_key(SUPERKEY_SALT, 32).unwrap());
let super_key =
Arc::new(TestKey(pw_key.decrypt(SUPERKEY_PAYLOAD, SUPERKEY_IV, SUPERKEY_TAG).unwrap()));
@@ -2168,7 +2168,7 @@
std::fs::create_dir(&*temp_dir.build().push("user_0")).unwrap();
let pw: Password = PASSWORD.into();
- let pw_key = TestKey(pw.derive_key(Some(SUPERKEY_SALT), 32).unwrap());
+ let pw_key = TestKey(pw.derive_key(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 74e3e56..dd22d8b 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -646,7 +646,7 @@
) {
(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(Some(salt), AES_256_KEY_LENGTH).context(
+ let key = pw.derive_key(salt, AES_256_KEY_LENGTH).context(
"In extract_super_key_from_key_entry: Failed to generate key from password.",
)?;
@@ -686,7 +686,7 @@
) -> Result<(Vec<u8>, BlobMetaData)> {
let salt = generate_salt().context("In encrypt_with_password: Failed to generate salt.")?;
let derived_key = pw
- .derive_key(Some(&salt), AES_256_KEY_LENGTH)
+ .derive_key(&salt, AES_256_KEY_LENGTH)
.context("In encrypt_with_password: Failed to derive password.")?;
let mut metadata = BlobMetaData::new();
metadata.add(BlobMetaEntry::EncryptedBy(EncryptedBy::Password));