Keystore 2.0: Add support for the new CERTIFICATE_* tags.

Test: Keystore CTS tests
Change-Id: Ifbecd4517e8b6fb143283ed3f815aed4812a3c4a
diff --git a/keystore2/src/crypto/certificate_utils.cpp b/keystore2/src/crypto/certificate_utils.cpp
index 4b0dca4..4aed224 100644
--- a/keystore2/src/crypto/certificate_utils.cpp
+++ b/keystore2/src/crypto/certificate_utils.cpp
@@ -42,17 +42,30 @@
 DEFINE_OPENSSL_OBJECT_POINTER(AUTHORITY_KEYID);
 DEFINE_OPENSSL_OBJECT_POINTER(BASIC_CONSTRAINTS);
 DEFINE_OPENSSL_OBJECT_POINTER(X509_ALGOR);
+DEFINE_OPENSSL_OBJECT_POINTER(BIGNUM);
 
 }  // namespace
 
-std::variant<CertUtilsError, X509_NAME_Ptr> makeCommonName(const std::string& name) {
+constexpr const char kDefaultCommonName[] = "Default Common Name";
+
+std::variant<CertUtilsError, X509_NAME_Ptr>
+makeCommonName(std::optional<std::reference_wrapper<const std::vector<uint8_t>>> name) {
+    if (name) {
+        const uint8_t* p = name->get().data();
+        X509_NAME_Ptr x509_name(d2i_X509_NAME(nullptr, &p, name->get().size()));
+        if (!x509_name) {
+            return CertUtilsError::MemoryAllocation;
+        }
+        return x509_name;
+    }
+
     X509_NAME_Ptr x509_name(X509_NAME_new());
     if (!x509_name) {
-        return CertUtilsError::BoringSsl;
+        return CertUtilsError::MemoryAllocation;
     }
     if (!X509_NAME_add_entry_by_txt(x509_name.get(), "CN", MBSTRING_ASC,
-                                    reinterpret_cast<const uint8_t*>(name.c_str()), name.length(),
-                                    -1 /* loc */, 0 /* set */)) {
+                                    reinterpret_cast<const uint8_t*>(kDefaultCommonName),
+                                    sizeof(kDefaultCommonName) - 1, -1 /* loc */, 0 /* set */)) {
         return CertUtilsError::BoringSsl;
     }
     return x509_name;
@@ -159,14 +172,11 @@
 // Callers should pass an empty X509_Ptr and check the return value for CertUtilsError::Ok (0)
 // before accessing the result.
 std::variant<CertUtilsError, X509_Ptr>
-makeCertRump(const uint32_t serial, const char subject[], const uint64_t activeDateTimeMilliSeconds,
+makeCertRump(std::optional<std::reference_wrapper<const std::vector<uint8_t>>> serial,
+             std::optional<std::reference_wrapper<const std::vector<uint8_t>>> subject,
+             const uint64_t activeDateTimeMilliSeconds,
              const uint64_t usageExpireDateTimeMilliSeconds) {
 
-    // Sanitize pointer arguments.
-    if (!subject || strlen(subject) == 0) {
-        return CertUtilsError::InvalidArgument;
-    }
-
     // Create certificate structure.
     X509_Ptr certificate(X509_new());
     if (!certificate) {
@@ -178,9 +188,23 @@
         return CertUtilsError::BoringSsl;
     }
 
+    BIGNUM_Ptr bn_serial;
+    if (serial) {
+        bn_serial = BIGNUM_Ptr(BN_bin2bn(serial->get().data(), serial->get().size(), nullptr));
+        if (!bn_serial) {
+            return CertUtilsError::MemoryAllocation;
+        }
+    } else {
+        bn_serial = BIGNUM_Ptr(BN_new());
+        if (!bn_serial) {
+            return CertUtilsError::MemoryAllocation;
+        }
+        BN_zero(bn_serial.get());
+    }
+
     // Set the certificate serialNumber
     ASN1_INTEGER_Ptr serialNumber(ASN1_INTEGER_new());
-    if (!serialNumber || !ASN1_INTEGER_set(serialNumber.get(), serial) ||
+    if (!serialNumber || !BN_to_ASN1_INTEGER(bn_serial.get(), serialNumber.get()) ||
         !X509_set_serialNumber(certificate.get(), serialNumber.get() /* Don't release; copied */))
         return CertUtilsError::BoringSsl;
 
@@ -215,7 +239,9 @@
 }
 
 std::variant<CertUtilsError, X509_Ptr>
-makeCert(const EVP_PKEY* evp_pkey, const uint32_t serial, const char subject[],
+makeCert(const EVP_PKEY* evp_pkey,
+         std::optional<std::reference_wrapper<const std::vector<uint8_t>>> serial,
+         std::optional<std::reference_wrapper<const std::vector<uint8_t>>> subject,
          const uint64_t activeDateTimeMilliSeconds, const uint64_t usageExpireDateTimeMilliSeconds,
          bool addSubjectKeyIdEx, std::optional<KeyUsageExtension> keyUsageEx,
          std::optional<BasicConstraintsExtension> basicConstraints) {
diff --git a/keystore2/src/crypto/include/certificate_utils.h b/keystore2/src/crypto/include/certificate_utils.h
index 1e80d80..e31ebc4 100644
--- a/keystore2/src/crypto/include/certificate_utils.h
+++ b/keystore2/src/crypto/include/certificate_utils.h
@@ -80,7 +80,7 @@
  * `signCert` or `signCertWith`.
  * @param evp_pkey The public key that the certificate is issued for.
  * @param serial The certificate serial number.
- * @param subject The subject common name.
+ * @param subject The X509 name encoded subject common name.
  * @param activeDateTimeMilliSeconds The not before date in epoch milliseconds.
  * @param usageExpireDateTimeMilliSeconds The not after date in epoch milliseconds.
  * @param addSubjectKeyIdEx If true, adds the subject key id extension.
@@ -89,14 +89,14 @@
  * @return CertUtilsError::Ok on success.
  */
 std::variant<CertUtilsError, X509_Ptr>
-makeCert(const EVP_PKEY* evp_pkey,                                    //
-         const uint32_t serial,                                       //
-         const char subject[],                                        //
-         const uint64_t activeDateTimeMilliSeconds,                   //
-         const uint64_t usageExpireDateTimeMilliSeconds,              //
-         bool addSubjectKeyIdEx,                                      //
-         std::optional<KeyUsageExtension> keyUsageEx,                 //
-         std::optional<BasicConstraintsExtension> basicConstraints);  //
+makeCert(const EVP_PKEY* evp_pkey,                                                   //
+         std::optional<std::reference_wrapper<const std::vector<uint8_t>>> serial,   //
+         std::optional<std::reference_wrapper<const std::vector<uint8_t>>> subject,  //
+         const uint64_t activeDateTimeMilliSeconds,                                  //
+         const uint64_t usageExpireDateTimeMilliSeconds,                             //
+         bool addSubjectKeyIdEx,                                                     //
+         std::optional<KeyUsageExtension> keyUsageEx,                                //
+         std::optional<BasicConstraintsExtension> basicConstraints);                 //
 
 /**
  * Takes the subject name from `signingCert` and sets it as issuer name in `cert`.
diff --git a/keystore2/src/crypto/tests/certificate_utils_test.cpp b/keystore2/src/crypto/tests/certificate_utils_test.cpp
index 2df9ce5..119c3fa 100644
--- a/keystore2/src/crypto/tests/certificate_utils_test.cpp
+++ b/keystore2/src/crypto/tests/certificate_utils_test.cpp
@@ -173,8 +173,8 @@
         .isCertificationKey = true,
     };
 
-    auto certV = makeCert(pkey.get(), 1, "Me", now_ms - kValidity, now_ms + kValidity,
-                          true /* subject key id extension */, keyUsage, bcons);
+    auto certV = makeCert(pkey.get(), std::nullopt, std::nullopt, now_ms - kValidity,
+                          now_ms + kValidity, true /* subject key id extension */, keyUsage, bcons);
     ASSERT_TRUE(std::holds_alternative<X509_Ptr>(certV));
     auto& cert = std::get<X509_Ptr>(certV);
     ASSERT_TRUE(!setIssuer(cert.get(), cert.get(), true));
@@ -272,8 +272,8 @@
         .isCertificationKey = true,
     };
 
-    auto certV = makeCert(pkey.get(), 1, "Me", now_ms - kValidity, now_ms + kValidity,
-                          true /* subject key id extension */, keyUsage, bcons);
+    auto certV = makeCert(pkey.get(), std::nullopt, std::nullopt, now_ms - kValidity,
+                          now_ms + kValidity, true /* subject key id extension */, keyUsage, bcons);
     ASSERT_TRUE(std::holds_alternative<X509_Ptr>(certV));
     auto& cert = std::get<X509_Ptr>(certV);
     ASSERT_TRUE(!setIssuer(cert.get(), cert.get(), true));
diff --git a/keystore2/src/key_parameter.rs b/keystore2/src/key_parameter.rs
index 93de6f2..117dea8 100644
--- a/keystore2/src/key_parameter.rs
+++ b/keystore2/src/key_parameter.rs
@@ -948,9 +948,23 @@
     #[key_param(tag = RESET_SINCE_ID_ROTATION, field = BoolValue)]
     ResetSinceIdRotation,
     /// Used to deliver a cryptographic token proving that the user
-    ///  confirmed a signing request
+    /// confirmed a signing request
     #[key_param(tag = CONFIRMATION_TOKEN, field = Blob)]
     ConfirmationToken(Vec<u8>),
+    /// Used to deliver the certificate serial number to the KeyMint instance
+    /// certificate generation.
+    #[key_param(tag = CERTIFICATE_SERIAL, field = Blob)]
+    CertificateSerial(Vec<u8>),
+    /// Used to deliver the certificate subject to the KeyMint instance
+    /// certificate generation. This must be DER encoded X509 name.
+    #[key_param(tag = CERTIFICATE_SUBJECT, field = Blob)]
+    CertificateSubject(Vec<u8>),
+    /// Used to deliver the not before date in milliseconds to KeyMint during key generation/import.
+    #[key_param(tag = CERTIFICATE_NOT_BEFORE, field = DateTime)]
+    CertificateNotBefore(i64),
+    /// Used to deliver the not after date in milliseconds to KeyMint during key generation/import.
+    #[key_param(tag = CERTIFICATE_NOT_AFTER, field = DateTime)]
+    CertificateNotAfter(i64),
 }
 }
 
diff --git a/keystore2/src/km_compat/km_compat.cpp b/keystore2/src/km_compat/km_compat.cpp
index 601baf1..93a8b70 100644
--- a/keystore2/src/km_compat/km_compat.cpp
+++ b/keystore2/src/km_compat/km_compat.cpp
@@ -30,6 +30,8 @@
 #include <keymasterV4_1/Keymaster3.h>
 #include <keymasterV4_1/Keymaster4.h>
 
+#include <chrono>
+
 #include "certificate_utils.h"
 
 using ::aidl::android::hardware::security::keymint::Algorithm;
@@ -50,6 +52,9 @@
 namespace V4_1 = ::android::hardware::keymaster::V4_1;
 namespace KMV1 = ::aidl::android::hardware::security::keymint;
 
+using namespace std::chrono_literals;
+using std::chrono::duration_cast;
+
 // Utility functions
 
 ScopedAStatus convertErrorCode(KMV1::ErrorCode result) {
@@ -579,21 +584,34 @@
     CBS cbs;
     CBS_init(&cbs, key.data(), key.size());
     auto pkey = EVP_parse_public_key(&cbs);
+
     // makeCert
-    // TODO: Get the serial and subject from key params once the tags are added.  Also use new tags
-    // for the two datetime parameters once we get those.
+    std::optional<std::reference_wrapper<const std::vector<uint8_t>>> subject;
+    if (auto blob = getParam(keyParams, KMV1::TAG_CERTIFICATE_SUBJECT)) {
+        subject = *blob;
+    }
+
+    std::optional<std::reference_wrapper<const std::vector<uint8_t>>> serial;
+    if (auto blob = getParam(keyParams, KMV1::TAG_CERTIFICATE_SERIAL)) {
+        serial = *blob;
+    }
 
     uint64_t activation = 0;
-    if (auto date = getParam(keyParams, KMV1::TAG_ACTIVE_DATETIME)) {
+    if (auto date = getParam(keyParams, KMV1::TAG_CERTIFICATE_NOT_BEFORE)) {
         activation = *date;
+    } else {
+        return KMV1::ErrorCode::MISSING_NOT_BEFORE;
     }
-    uint64_t expiration = std::numeric_limits<uint64_t>::max();
-    if (auto date = getParam(keyParams, KMV1::TAG_USAGE_EXPIRE_DATETIME)) {
+
+    uint64_t expiration;
+    if (auto date = getParam(keyParams, KMV1::TAG_CERTIFICATE_NOT_AFTER)) {
         expiration = *date;
+    } else {
+        return KMV1::ErrorCode::MISSING_NOT_AFTER;
     }
 
     auto certOrError = keystore::makeCert(
-        pkey, 42, "TODO", activation, expiration, false /* intentionally left blank */,
+        pkey, serial, subject, activation, expiration, false /* intentionally left blank */,
         std::nullopt /* intentionally left blank */, std::nullopt /* intentionally left blank */);
     if (std::holds_alternative<keystore::CertUtilsError>(certOrError)) {
         LOG(ERROR) << __func__ << ": Failed to make certificate";
diff --git a/keystore2/src/km_compat/km_compat_type_conversion.h b/keystore2/src/km_compat/km_compat_type_conversion.h
index 5fdca91..b36b78a 100644
--- a/keystore2/src/km_compat/km_compat_type_conversion.h
+++ b/keystore2/src/km_compat/km_compat_type_conversion.h
@@ -734,7 +734,11 @@
         }
         break;
     case KMV1::Tag::RSA_OAEP_MGF_DIGEST:
-        // Does not exist in KM < KeyMint 1.0.
+    case KMV1::Tag::CERTIFICATE_SERIAL:
+    case KMV1::Tag::CERTIFICATE_SUBJECT:
+    case KMV1::Tag::CERTIFICATE_NOT_BEFORE:
+    case KMV1::Tag::CERTIFICATE_NOT_AFTER:
+        // These tags do not exist in KM < KeyMint 1.0.
         break;
     }
     return V4_0::KeyParameter{.tag = V4_0::Tag::INVALID};
diff --git a/keystore2/src/km_compat/lib.rs b/keystore2/src/km_compat/lib.rs
index d264e7a..097e6d4 100644
--- a/keystore2/src/km_compat/lib.rs
+++ b/keystore2/src/km_compat/lib.rs
@@ -76,6 +76,10 @@
         creation_result
     }
 
+    // Per RFC 5280 4.1.2.5, an undefined expiration (not-after) field should be set to GeneralizedTime
+    // 999912312359559, which is 253402300799000 ms from Jan 1, 1970.
+    const UNDEFINED_NOT_AFTER: i64 = 253402300799000i64;
+
     fn generate_rsa_key(legacy: &dyn IKeyMintDevice, encrypt: bool, attest: bool) -> Vec<u8> {
         let mut kps = vec![
             KeyParameter {
@@ -97,6 +101,14 @@
                 tag: Tag::PURPOSE,
                 value: KeyParameterValue::KeyPurpose(KeyPurpose::SIGN),
             },
+            KeyParameter {
+                tag: Tag::CERTIFICATE_NOT_BEFORE,
+                value: KeyParameterValue::DateTime(0),
+            },
+            KeyParameter {
+                tag: Tag::CERTIFICATE_NOT_AFTER,
+                value: KeyParameterValue::DateTime(UNDEFINED_NOT_AFTER),
+            },
         ];
         if encrypt {
             kps.push(KeyParameter {
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index f6d8108..ec133f8 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -68,6 +68,10 @@
 // Blob of 32 zeroes used as empty masking key.
 static ZERO_BLOB_32: &[u8] = &[0; 32];
 
+// Per RFC 5280 4.1.2.5, an undefined expiration (not-after) field should be set to GeneralizedTime
+// 999912312359559, which is 253402300799000 ms from Jan 1, 1970.
+const UNDEFINED_NOT_AFTER: i64 = 253402300799000i64;
+
 impl KeystoreSecurityLevel {
     /// Creates a new security level instance wrapped in a
     /// BnKeystoreSecurityLevel proxy object. It also
@@ -305,17 +309,39 @@
         })
     }
 
-    fn add_attestation_parameters(uid: u32, params: &[KeyParameter]) -> Result<Vec<KeyParameter>> {
+    fn add_certificate_parameters(uid: u32, params: &[KeyParameter]) -> Result<Vec<KeyParameter>> {
         let mut result = params.to_vec();
+        // If there is an attestation challenge we need to get an application id.
         if params.iter().any(|kp| kp.tag == Tag::ATTESTATION_CHALLENGE) {
             let aaid = keystore2_aaid::get_aaid(uid).map_err(|e| {
-                anyhow!(format!("In add_attestation_parameters: get_aaid returned status {}.", e))
+                anyhow!(format!("In add_certificate_parameters: get_aaid returned status {}.", e))
             })?;
             result.push(KeyParameter {
                 tag: Tag::ATTESTATION_APPLICATION_ID,
                 value: KeyParameterValue::Blob(aaid),
             });
         }
+
+        // If we are generating/importing an asymmetric key, we need to make sure
+        // that NOT_BEFORE and NOT_AFTER are present.
+        match params.iter().find(|kp| kp.tag == Tag::ALGORITHM) {
+            Some(KeyParameter { tag: _, value: KeyParameterValue::Algorithm(Algorithm::RSA) })
+            | Some(KeyParameter { tag: _, value: KeyParameterValue::Algorithm(Algorithm::EC) }) => {
+                if !params.iter().any(|kp| kp.tag == Tag::CERTIFICATE_NOT_BEFORE) {
+                    result.push(KeyParameter {
+                        tag: Tag::CERTIFICATE_NOT_BEFORE,
+                        value: KeyParameterValue::DateTime(0),
+                    })
+                }
+                if !params.iter().any(|kp| kp.tag == Tag::CERTIFICATE_NOT_AFTER) {
+                    result.push(KeyParameter {
+                        tag: Tag::CERTIFICATE_NOT_AFTER,
+                        value: KeyParameterValue::DateTime(UNDEFINED_NOT_AFTER),
+                    })
+                }
+            }
+            _ => {}
+        }
         Ok(result)
     }
 
@@ -346,7 +372,7 @@
         // generate_key requires the rebind permission.
         check_key_permission(KeyPerm::rebind(), &key, &None).context("In generate_key.")?;
 
-        let params = Self::add_attestation_parameters(caller_uid, params)
+        let params = Self::add_certificate_parameters(caller_uid, params)
             .context("In generate_key: Trying to get aaid.")?;
 
         let km_dev: Box<dyn IKeyMintDevice> = self.keymint.get_interface()?;
@@ -386,7 +412,7 @@
         // import_key requires the rebind permission.
         check_key_permission(KeyPerm::rebind(), &key, &None).context("In import_key.")?;
 
-        let params = Self::add_attestation_parameters(caller_uid, params)
+        let params = Self::add_certificate_parameters(caller_uid, params)
             .context("In import_key: Trying to get aaid.")?;
 
         let format = params