tequilaOS/platform_system_security
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Use P521 curve instead of P256

Browse source 
Certification may require the use of a larger elliptic curve.

Devices that took a dogfood/beta version of Android S without this
change will experience two problems:

* old P256 keys will be present unused in the database
* in the unlikely event that a screen-lock bound key was created
  while the device was locked before taking the update with this change
  and then not used until after, the key won't be decryptable.

Since these problems don't affect production users, I don't think
the significant complexity that would be needed to fix them is worth it.

Bug: 191759985
Test: keystore2_test
Test: atest android.keystore.cts.CipherTest#
    testEmptyPlaintextEncryptsAndDecryptsWhenUnlockedRequired
Ignore-AOSP-First: problem in sc-dev, no merge path from AOSP
Change-Id: If1938bb8eddc148c7f8888006e7eb7c8e9a5a806
This commit is contained in:
Paul Crowley 2021-06-22 08:16:01 -07:00 committed by [6;7~
parent 15b7f67665
commit 31eebb3d0a
3 changed files with 16 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
keystore2/src/crypto/crypto.cpp
View file

@ -225,7 +225,7 @@ int ECDHComputeKey(void* out, const EC_POINT* pub_key, const EC_KEY* priv_key) {
EC_KEY* ECKEYGenerateKey() {
EC_KEY* key = EC_KEY_new();
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
EC_KEY_set_group(key, group);
auto result = EC_KEY_generate_key(key);
if (result == 0) {
@ -251,7 +251,7 @@ size_t ECKEYMarshalPrivateKey(const EC_KEY* priv_key, uint8_t* buf, size_t len)
EC_KEY* ECKEYParsePrivateKey(const uint8_t* buf, size_t len) {
CBS cbs;
CBS_init(&cbs, buf, len);
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
auto result = EC_KEY_parse_private_key(&cbs, group);
EC_GROUP_free(group);
if (result != nullptr && CBS_len(&cbs) != 0) {
@ -262,7 +262,7 @@ EC_KEY* ECKEYParsePrivateKey(const uint8_t* buf, size_t len) {
}
size_t ECPOINTPoint2Oct(const EC_POINT* point, uint8_t* buf, size_t len) {
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
point_conversion_form_t form = POINT_CONVERSION_UNCOMPRESSED;
auto result = EC_POINT_point2oct(group, point, form, buf, len, nullptr);
EC_GROUP_free(group);
@ -270,7 +270,7 @@ size_t ECPOINTPoint2Oct(const EC_POINT* point, uint8_t* buf, size_t len) {
}
EC_POINT* ECPOINTOct2Point(const uint8_t* buf, size_t len) {
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
EC_POINT* point = EC_POINT_new(group);
auto result = EC_POINT_oct2point(group, point, buf, len, nullptr);
EC_GROUP_free(group);

6
keystore2/src/crypto/lib.rs
View file

@ -346,7 +346,7 @@ pub fn ec_key_generate_key() -> Result<ECKey, Error> {
/// Calls the boringssl EC_KEY_marshal_private_key function.
pub fn ec_key_marshal_private_key(key: &ECKey) -> Result<ZVec, Error> {
let len = 39; // Empirically observed length of private key
let len = 73; // Empirically observed length of private key
let mut buf = ZVec::new(len)?;
// Safety: the key is valid.
// This will not write past the specified length of the buffer; if the
@ -381,8 +381,8 @@ pub fn ec_key_get0_public_key(key: &ECKey) -> BorrowedECPoint {
/// Calls the boringssl EC_POINT_point2oct.
pub fn ec_point_point_to_oct(point: &EC_POINT) -> Result<Vec<u8>, Error> {
// We fix the length to 65 (1 + 2 * field_elem_size), as we get an error if it's too small.
let len = 65;
// We fix the length to 133 (1 + 2 * field_elem_size), as we get an error if it's too small.
let len = 133;
let mut buf = vec![0; len];
// Safety: EC_POINT_point2oct writes at most len bytes. The point is valid.
let result = unsafe { ECPOINTPoint2Oct(point, buf.as_mut_ptr(), len) };

18
keystore2/src/super_key.rs
View file

@ -68,8 +68,8 @@ type UserId = u32;
pub enum SuperEncryptionAlgorithm {
/// Symmetric encryption with AES-256-GCM
Aes256Gcm,
/// Public-key encryption with ECDH P-256
EcdhP256,
/// Public-key encryption with ECDH P-521
EcdhP521,
}
/// A particular user may have several superencryption keys in the database, each for a
@ -96,9 +96,9 @@ pub const USER_SCREEN_LOCK_BOUND_KEY: SuperKeyType = SuperKeyType {
/// Key used for ScreenLockBound keys; the corresponding superencryption key is loaded in memory
/// each time the user enters their LSKF, and cleared from memory each time the device is locked.
/// Asymmetric, so keys can be encrypted when the device is locked.
pub const USER_SCREEN_LOCK_BOUND_ECDH_KEY: SuperKeyType = SuperKeyType {
alias: "USER_SCREEN_LOCK_BOUND_ECDH_KEY",
algorithm: SuperEncryptionAlgorithm::EcdhP256,
pub const USER_SCREEN_LOCK_BOUND_P521_KEY: SuperKeyType = SuperKeyType {
alias: "USER_SCREEN_LOCK_BOUND_P521_KEY",
algorithm: SuperEncryptionAlgorithm::EcdhP521,
};
/// Superencryption to apply to a new key.
@ -468,7 +468,7 @@ impl SuperKeyManager {
tag.is_some(),
)),
},
SuperEncryptionAlgorithm::EcdhP256 => {
SuperEncryptionAlgorithm::EcdhP521 => {
match (metadata.public_key(), metadata.salt(), metadata.iv(), metadata.aead_tag()) {
(Some(public_key), Some(salt), Some(iv), Some(aead_tag)) => {
ECDHPrivateKey::from_private_key(&key.key)
@ -753,7 +753,7 @@ impl SuperKeyManager {
} else {
// Symmetric key is not available, use public key encryption
let loaded =
db.load_super_key(&USER_SCREEN_LOCK_BOUND_ECDH_KEY, user_id).context(
db.load_super_key(&USER_SCREEN_LOCK_BOUND_P521_KEY, user_id).context(
"In handle_super_encryption_on_key_init: load_super_key failed.",
)?;
let (key_id_guard, key_entry) = loaded.ok_or_else(Error::sys).context(
@ -836,7 +836,7 @@ impl SuperKeyManager {
.context("In get_or_create_super_key: Failed to generate AES 256 key.")?,
None,
),
SuperEncryptionAlgorithm::EcdhP256 => {
SuperEncryptionAlgorithm::EcdhP521 => {
let key = ECDHPrivateKey::generate()
.context("In get_or_create_super_key: Failed to generate ECDH key")?;
(
@ -903,7 +903,7 @@ impl SuperKeyManager {
Self::get_or_create_super_key(
db,
user_id,
&USER_SCREEN_LOCK_BOUND_ECDH_KEY,
&USER_SCREEN_LOCK_BOUND_P521_KEY,
password,
Some(aes.clone()),
)