Snap for 13256841 from b41967607dd7f1a24a71d393dd2055ac8dc54b91 to 25Q2-release

Change-Id: I9eca7640e05f5aeaa0578a0d8b77a0992ad7e7fa
diff --git a/keystore2/keystore2.rc b/keystore2/keystore2.rc
index d7d6951..e669b18 100644
--- a/keystore2/keystore2.rc
+++ b/keystore2/keystore2.rc
@@ -13,3 +13,5 @@
     task_profiles ProcessCapacityHigh
     # The default memlock limit of 65536 bytes is too low for keystore.
     rlimit memlock unlimited unlimited
+    # Reboot to bootloader if Keystore crashes more than 4 times before `sys.boot_completed`.
+    critical window=0
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 008e6fe..e08a5f2 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -76,6 +76,11 @@
     // Write/update keystore.crash_count system property.
     metrics_store::update_keystore_crash_sysprop();
 
+    // Send KeyMint module information for attestations.
+    // Note that the information should be sent before code from modules starts running.
+    // (This is guaranteed by waiting for `keystore.module_hash.sent` == true during device boot.)
+    Maintenance::check_send_module_info();
+
     // Keystore 2.0 cannot change to the database directory (typically /data/misc/keystore) on
     // startup as Keystore 1.0 did because Keystore 2.0 is intended to run much earlier than
     // Keystore 1.0. Instead we set a global variable to the database path.
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 4116bc5..a0f5ee8 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -247,17 +247,42 @@
         {
             log::error!("SUPER_KEY.set_up_boot_level_cache failed:\n{:?}\n:(", e);
         }
+        Maintenance::call_on_all_security_levels("earlyBootEnded", |dev| dev.earlyBootEnded(), None)
+    }
 
+    /// Spawns a thread to send module info if it hasn't already been sent. The thread first waits
+    /// for the apex info to be available.
+    /// (Module info would have already been sent in the case of a Keystore restart.)
+    ///
+    /// # Panics
+    ///
+    /// This method, and methods it calls, panic on failure, because a failure to populate module
+    /// information will block the boot process from completing. In this method, this happens if:
+    /// - the `apexd.status` property is unable to be monitored
+    /// - the `keystore.module_hash.sent` property cannot be updated
+    pub fn check_send_module_info() {
+        if rustutils::system_properties::read_bool("keystore.module_hash.sent", false)
+            .unwrap_or(false)
+        {
+            log::info!("Module info has already been sent.");
+            return;
+        }
         if keystore2_flags::attest_modules() {
             std::thread::spawn(move || {
-                Self::watch_apex_info()
-                    .unwrap_or_else(|e| log::error!("watch_apex_info failed, preventing keystore.module_hash.sent from being set to true; this may therefore block boot: {e:?}"));
+                // Wait for apex info to be available before populating.
+                Self::watch_apex_info().unwrap_or_else(|e| {
+                    log::error!("failed to monitor apexd.status property: {e:?}");
+                    panic!("Terminating due to inaccessibility of apexd.status property, blocking boot: {e:?}");
+                });
             });
         } else {
             rustutils::system_properties::write("keystore.module_hash.sent", "true")
-                .context(ks_err!("failed to set keystore.module_hash.sent to true"))?;
+                .unwrap_or_else(|e| {
+                        log::error!("Failed to set keystore.module_hash.sent to true; this will therefore block boot: {e:?}");
+                        panic!("Crashing Keystore because it failed to set keystore.module_hash.sent to true (which blocks boot).");
+                    }
+                );
         }
-        Maintenance::call_on_all_security_levels("earlyBootEnded", |dev| dev.earlyBootEnded(), None)
     }
 
     /// Watch the `apexd.status` system property, and read apex module information once
@@ -273,18 +298,38 @@
             let value = w.read(|_name, value| Ok(value.to_string()));
             log::info!("property '{apex_prop}' is now '{value:?}'");
             if matches!(value.as_deref(), Ok("activated")) {
-                let modules =
-                    Self::read_apex_info().context(ks_err!("failed to read apex info"))?;
-                Self::set_module_info(modules).context(ks_err!("failed to set module info"))?;
-                rustutils::system_properties::write("keystore.module_hash.sent", "true")
-                    .context(ks_err!("failed to set keystore.module_hash.sent to true"))?;
-                break;
+                Self::read_and_set_module_info();
+                return Ok(());
             }
             log::info!("await a change to '{apex_prop}'...");
             w.wait(None).context(ks_err!("property wait failed"))?;
             log::info!("await a change to '{apex_prop}'...notified");
         }
-        Ok(())
+    }
+
+    /// Read apex information (which is assumed to be present) and propagate module
+    /// information to KeyMint instances.
+    ///
+    /// # Panics
+    ///
+    /// This method panics on failure, because a failure to populate module information
+    /// will block the boot process from completing.  This happens if:
+    /// - apex information is not available (precondition)
+    /// - KeyMint instances fail to accept module information
+    /// - the `keystore.module_hash.sent` property cannot be updated
+    fn read_and_set_module_info() {
+        let modules = Self::read_apex_info().unwrap_or_else(|e| {
+            log::error!("failed to read apex info: {e:?}");
+            panic!("Terminating due to unavailability of apex info, blocking boot: {e:?}");
+        });
+        Self::set_module_info(modules).unwrap_or_else(|e| {
+            log::error!("failed to set module info: {e:?}");
+            panic!("Terminating due to KeyMint not accepting module info, blocking boot: {e:?}");
+        });
+        rustutils::system_properties::write("keystore.module_hash.sent", "true").unwrap_or_else(|e| {
+            log::error!("failed to set keystore.module_hash.sent property: {e:?}");
+            panic!("Terminating due to failure to set keystore.module_hash.sent property, blocking boot: {e:?}");
+        });
     }
 
     fn read_apex_info() -> Result<Vec<ModuleInfo>> {
diff --git a/keystore2/test_utils/attestation/Android.bp b/keystore2/test_utils/attestation/Android.bp
index 0ac5630..fb4dc7e 100644
--- a/keystore2/test_utils/attestation/Android.bp
+++ b/keystore2/test_utils/attestation/Android.bp
@@ -41,6 +41,7 @@
     name: "libkeystore_attestation",
     defaults: ["libkeystore_attestation_defaults"],
     vendor_available: true,
+    min_sdk_version: "35",
 }
 
 rust_test {
diff --git a/provisioner/rkp_factory_extraction_lib.h b/provisioner/rkp_factory_extraction_lib.h
index 3515f48..f6f21f5 100644
--- a/provisioner/rkp_factory_extraction_lib.h
+++ b/provisioner/rkp_factory_extraction_lib.h
@@ -33,6 +33,18 @@
 // Challenge size must be between 32 and 64 bytes inclusive.
 constexpr size_t kChallengeSize = 64;
 
+// How CSRs should be validated when the rkp_factory_extraction_tool's "self_test"
+// flag is set to "true".
+struct CsrValidationConfig {
+    // Names of IRemotelyProvisionedComponent instances for which degenerate DICE
+    // chains are allowed.
+    std::unordered_set<std::string>* allow_degenerate_irpc_names;
+
+    // Names of IRemotelyProvisionedComponent instances for which UDS certificate
+    // chains are required to be present in the CSR.
+    std::unordered_set<std::string>* require_uds_certs_irpc_names;
+};
+
 // Contains a the result of an operation that should return cborData on success.
 // Returns an an error message and null cborData on error.
 template <typename T> struct CborResult {
diff --git a/provisioner/rkp_factory_extraction_tool.cpp b/provisioner/rkp_factory_extraction_tool.cpp
index 599b52a..f65e0ae 100644
--- a/provisioner/rkp_factory_extraction_tool.cpp
+++ b/provisioner/rkp_factory_extraction_tool.cpp
@@ -40,17 +40,20 @@
 
 DEFINE_string(output_format, "build+csr", "How to format the output. Defaults to 'build+csr'.");
 DEFINE_bool(self_test, true,
-            "If true, this tool performs a self-test, validating the payload for correctness. "
-            "This checks that the device on the factory line is producing valid output "
-            "before attempting to upload the output to the device info service.");
-DEFINE_bool(allow_degenerate, true,
-            "If true, self_test validation will allow degenerate DICE chains in the CSR.");
+            "Whether to validate the output for correctness. If enabled, this checks that the "
+            "device on the factory line is producing valid output before attempting to upload the "
+            "output to the device info service. Defaults to true.");
+DEFINE_string(allow_degenerate, "",
+              "Comma-delimited list of names of IRemotelyProvisionedComponent instances for which "
+              "self_test validation allows degenerate DICE chains in the CSR. Example: "
+              "avf,default,strongbox. Defaults to the empty string.");
 DEFINE_string(serialno_prop, "ro.serialno",
-              "The property of getting serial number. Defaults to 'ro.serialno'.");
+              "System property from which the serial number should be retrieved. Defaults to "
+              "'ro.serialno'.");
 DEFINE_string(require_uds_certs, "",
-              "The comma-delimited names of remotely provisioned "
-              "components whose UDS certificate chains are required to be present in the CSR. "
-              "Example: avf,default,strongbox");
+              "Comma-delimited list of names of IRemotelyProvisionedComponent instances for which "
+              "UDS certificate chains are required to be present in the CSR. Example: "
+              "avf,default,strongbox. Defaults to the empty string.");
 
 namespace {
 
@@ -84,7 +87,7 @@
 }
 
 void getCsrForIRpc(const char* descriptor, const char* name, IRemotelyProvisionedComponent* irpc,
-                   bool requireUdsCerts) {
+                   bool allowDegenerate, bool requireUdsCerts) {
     auto fullName = getFullServiceName(descriptor, name);
     // AVF RKP HAL is not always supported, so we need to check if it is supported before
     // generating the CSR.
@@ -96,8 +99,7 @@
         }
     }
 
-    auto [request, errMsg] =
-        getCsr(name, irpc, FLAGS_self_test, FLAGS_allow_degenerate, requireUdsCerts);
+    auto [request, errMsg] = getCsr(name, irpc, FLAGS_self_test, allowDegenerate, requireUdsCerts);
     if (!request) {
         std::cerr << "Unable to build CSR for '" << fullName << "': " << errMsg << ", exiting."
                   << std::endl;
@@ -131,11 +133,25 @@
         exit(-1);
     }
 
-    auto requireUdsCertsRpcNames = static_cast<std::unordered_set<std::string>*>(context);
-    auto requireUdsCerts = requireUdsCertsRpcNames->count(name) != 0;
-    requireUdsCertsRpcNames->erase(name);
+    auto csrValidationConfig = static_cast<CsrValidationConfig*>(context);
+    bool allowDegenerateFieldNotNull = csrValidationConfig->allow_degenerate_irpc_names != nullptr;
+    bool allowDegenerate = allowDegenerateFieldNotNull &&
+                           csrValidationConfig->allow_degenerate_irpc_names->count(name) > 0;
+    bool requireUdsCertsFieldNotNull = csrValidationConfig->require_uds_certs_irpc_names != nullptr;
+    bool requireUdsCerts = requireUdsCertsFieldNotNull &&
+                           csrValidationConfig->require_uds_certs_irpc_names->count(name) > 0;
+
+    // Record the fact that this IRemotelyProvisionedComponent instance was found by removing it
+    // from the sets in the context.
+    if (allowDegenerateFieldNotNull) {
+        csrValidationConfig->allow_degenerate_irpc_names->erase(name);
+    }
+    if (requireUdsCertsFieldNotNull) {
+        csrValidationConfig->require_uds_certs_irpc_names->erase(name);
+    }
+
     getCsrForIRpc(IRemotelyProvisionedComponent::descriptor, name, rkpService.get(),
-                  requireUdsCerts);
+                  allowDegenerate, requireUdsCerts);
 }
 
 }  // namespace
@@ -143,21 +159,38 @@
 int main(int argc, char** argv) {
     gflags::ParseCommandLineFlags(&argc, &argv, /*remove_flags=*/true);
 
-    auto requireUdsCertsRpcNames = parseCommaDelimited(FLAGS_require_uds_certs);
+    auto allowDegenerateIRpcNames = parseCommaDelimited(FLAGS_allow_degenerate);
+    auto requireUdsCertsIRpcNames = parseCommaDelimited(FLAGS_require_uds_certs);
+    CsrValidationConfig csrValidationConfig = {
+        .allow_degenerate_irpc_names = &allowDegenerateIRpcNames,
+        .require_uds_certs_irpc_names = &requireUdsCertsIRpcNames,
+    };
 
     AServiceManager_forEachDeclaredInstance(IRemotelyProvisionedComponent::descriptor,
-                                            &requireUdsCertsRpcNames, getCsrForInstance);
+                                            &csrValidationConfig, getCsrForInstance);
 
     // Append drm CSRs
     for (auto const& [name, irpc] : android::mediadrm::getDrmRemotelyProvisionedComponents()) {
-        auto requireUdsCerts = requireUdsCertsRpcNames.count(name) != 0;
-        requireUdsCertsRpcNames.erase(name);
-        getCsrForIRpc(IDrmFactory::descriptor, name.c_str(), irpc.get(), requireUdsCerts);
+        bool allowDegenerate = allowDegenerateIRpcNames.count(name) != 0;
+        allowDegenerateIRpcNames.erase(name);
+        auto requireUdsCerts = requireUdsCertsIRpcNames.count(name) != 0;
+        requireUdsCertsIRpcNames.erase(name);
+        getCsrForIRpc(IDrmFactory::descriptor, name.c_str(), irpc.get(), allowDegenerate,
+                      requireUdsCerts);
     }
 
-    for (auto const& rpcName : requireUdsCertsRpcNames) {
-        std::cerr << "WARNING: You requested to enforce the presence of UDS Certs for '" << rpcName
-                  << "', but no Remotely Provisioned Component had that name." << std::endl;
+    // Print a warning for IRemotelyProvisionedComponent instance names that were passed
+    // in as parameters to the "require_uds_certs" and "allow_degenerate" flags but were
+    // ignored because no instances with those names were found.
+    for (const auto& irpcName : allowDegenerateIRpcNames) {
+        std::cerr << "WARNING: You requested special handling of 'self_test' validation checks "
+                  << "for '" << irpcName << "' via the 'allow_degenerate' flag but no such "
+                  << "IRemotelyProvisionedComponent instance exists." << std::endl;
+    }
+    for (const auto& irpcName : requireUdsCertsIRpcNames) {
+        std::cerr << "WARNING: You requested special handling of 'self_test' validation checks "
+                  << "for '" << irpcName << "' via the 'require_uds_certs' flag but no such "
+                  << "IRemotelyProvisionedComponent instance exists." << std::endl;
     }
 
     return 0;