Use P521 curve instead of P256

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

3 files changed, 16 insertions, 16 deletions

diff --git a/keystore2/src/crypto/crypto.cpp b/keystore2/src/crypto/crypto.cpp
index e4a1ac31..5d360a1e 100644
--- a/keystore2/src/crypto/crypto.cpp
+++ b/keystore2/src/crypto/crypto.cpp
@@ -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);
diff --git a/keystore2/src/crypto/lib.rs b/keystore2/src/crypto/lib.rs
index db23d1ff..5f8a2ef2 100644
--- a/keystore2/src/crypto/lib.rs
+++ b/keystore2/src/crypto/lib.rs
@@ -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) };
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index 9fb267a1..e02d9bcf 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -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()),
                 )