TpmChallenge: Mark registered user keys as corporate

Bug: 1082459
Test: *TpmChallenge*
Change-Id: I4c53ecca6ca4403e266bea9ba745e1bd1e337f52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2449991
Commit-Queue: Michael Ershov <miersh@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814231}

chrome/browser/chromeos/attestation/mock_tpm_challenge_key_subtle.h

@@ -32,8 +32,7 @@ class MockTpmChallengeKeySubtle : public TpmChallengeKeySubtle {
 
   MOCK_METHOD(void,
               StartSignChallengeStep,
-              (const std::string& challenge,
-               TpmChallengeKeyCallback callback),
+              (const std::string& challenge, TpmChallengeKeyCallback callback),
               (override));
 
   MOCK_METHOD(void,
@@ -46,6 +45,7 @@ class MockTpmChallengeKeySubtle : public TpmChallengeKeySubtle {
               (AttestationKeyType key_type,
                bool will_register_key,
                const std::string& key_name,
+               const std::string& public_key,
                Profile* profile),
               (override));
 };

chrome/browser/chromeos/attestation/tpm_challenge_key_result.cc

@@ -62,6 +62,8 @@ const char TpmChallengeKeyResult::kDeviceWebBasedAttestationNotOobeErrorMsg[] =
     "Device web based attestation is only available on the OOBE screen.";
 const char TpmChallengeKeyResult::kGetPublicKeyFailedErrorMsg[] =
     "Failed to get public key.";
+const char TpmChallengeKeyResult::kMarkCorporateKeyFailedErrorMsg[] =
+    "Failed to mark key as corporate.";
 
 // static
 TpmChallengeKeyResult TpmChallengeKeyResult::MakeChallengeResponse(
@@ -136,6 +138,8 @@ const char* TpmChallengeKeyResult::GetErrorMessage() const {
       return kDeviceWebBasedAttestationNotOobeErrorMsg;
     case TpmChallengeKeyResultCode::kGetPublicKeyFailedError:
       return kGetPublicKeyFailedErrorMsg;
+    case TpmChallengeKeyResultCode::kMarkCorporateKeyFailedError:
+      return kMarkCorporateKeyFailedErrorMsg;
     case TpmChallengeKeyResultCode::kSuccess:
       // Not an error message.
       NOTREACHED();

chrome/browser/chromeos/attestation/tpm_challenge_key_result.h

@@ -34,7 +34,8 @@ enum class TpmChallengeKeyResultCode {
   kChallengeBadBase64Error = 16,
   kDeviceWebBasedAttestationNotOobeError = 17,
   kGetPublicKeyFailedError = 18,
-  kMaxValue = kGetPublicKeyFailedError,
+  kMarkCorporateKeyFailedError = 19,
+  kMaxValue = kMarkCorporateKeyFailedError,
 };
 
 // If |IsSuccess| returns false, |result_code| contains error code and
@@ -62,6 +63,7 @@ struct TpmChallengeKeyResult {
   static const char kChallengeBadBase64ErrorMsg[];
   static const char kDeviceWebBasedAttestationNotOobeErrorMsg[];
   static const char kGetPublicKeyFailedErrorMsg[];
+  static const char kMarkCorporateKeyFailedErrorMsg[];
 
   static TpmChallengeKeyResult MakeChallengeResponse(
       const std::string& challenge_response);

chrome/browser/chromeos/attestation/tpm_challenge_key_subtle.cc

@@ -13,6 +13,8 @@
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/attestation/attestation_ca_client.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
@@ -51,13 +53,15 @@ std::unique_ptr<TpmChallengeKeySubtle> TpmChallengeKeySubtleFactory::Create() {
 
 // static
 std::unique_ptr<TpmChallengeKeySubtle>
-TpmChallengeKeySubtleFactory::CreateForPreparedKey(AttestationKeyType key_type,
-                                                   bool will_register_key,
-                                                   const std::string& key_name,
-                                                   Profile* profile) {
+TpmChallengeKeySubtleFactory::CreateForPreparedKey(
+    AttestationKeyType key_type,
+    bool will_register_key,
+    const std::string& key_name,
+    const std::string& public_key,
+    Profile* profile) {
   auto result = TpmChallengeKeySubtleFactory::Create();
   result->RestorePreparedKeyState(key_type, will_register_key, key_name,
-                                  profile);
+                                  public_key, profile);
   return result;
 }
 
@@ -133,8 +137,10 @@ void TpmChallengeKeySubtleImpl::RestorePreparedKeyState(
     AttestationKeyType key_type,
     bool will_register_key,
     const std::string& key_name,
+    const std::string& public_key,
     Profile* profile) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!will_register_key || !public_key.empty());
 
   // For user keys, a |profile| is strictly necessary.
   DCHECK(key_type != KEY_USER || profile);
@@ -142,6 +148,7 @@ void TpmChallengeKeySubtleImpl::RestorePreparedKeyState(
   key_type_ = key_type;
   will_register_key_ = will_register_key;
   key_name_ = GetKeyNameWithDefault(key_type, key_name);
+  public_key_ = public_key;
   profile_ = profile;
 }
 
@@ -481,6 +488,10 @@ void TpmChallengeKeySubtleImpl::PrepareKeyFinished(
     return;
   }
 
+  if (profile_ && will_register_key_) {
+    public_key_ = prepare_key_result->data;
+  }
+
   std::move(callback_).Run(Result::MakePublicKey(prepare_key_result->data));
 }
 
@@ -557,6 +568,32 @@ void TpmChallengeKeySubtleImpl::RegisterKeyCallback(
     return;
   }
 
+  if (!profile_ || (key_type_ == AttestationKeyType::KEY_DEVICE)) {
+    std::move(callback_).Run(Result::MakeSuccess());
+    return;
+  }
+
+  // TODO(1082459, 1113115): For now only user keys are being explicitly marked
+  // as corporate. All device keys are assumed to be corporate as well. It is
+  // better to mark device keys as well, when there is a way to get
+  // KeyPermissionsService without a profile.
+  platform_keys::KeyPermissionsServiceFactory::GetForBrowserContext(profile_)
+      ->SetCorporateKey(
+          public_key_,
+          base::BindOnce(&TpmChallengeKeySubtleImpl::MarkCorporateKeyCallback,
+                         weak_factory_.GetWeakPtr()));
+}
+
+void TpmChallengeKeySubtleImpl::MarkCorporateKeyCallback(
+    platform_keys::Status status) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (status != platform_keys::Status::kSuccess) {
+    std::move(callback_).Run(
+        Result::MakeError(ResultCode::kMarkCorporateKeyFailedError));
+    return;
+  }
+
   std::move(callback_).Run(Result::MakeSuccess());
 }
 

chrome/browser/chromeos/attestation/tpm_challenge_key_subtle.h

@@ -14,6 +14,7 @@
 #include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "chrome/browser/chromeos/attestation/tpm_challenge_key_result.h"
+#include "chrome/browser/chromeos/platform_keys/platform_keys.h"
 #include "chromeos/attestation/attestation_flow.h"
 #include "chromeos/dbus/constants/attestation_constants.h"
 #include "chromeos/dbus/cryptohome/cryptohome_client.h"
@@ -44,6 +45,7 @@ class TpmChallengeKeySubtleFactory final {
       AttestationKeyType key_type,
       bool will_register_key,
       const std::string& key_name,
+      const std::string& public_key,
       Profile* profile);
 
   static void SetForTesting(std::unique_ptr<TpmChallengeKeySubtle> next_result);
@@ -107,10 +109,12 @@ class TpmChallengeKeySubtle {
   TpmChallengeKeySubtle() = default;
 
   // Restores internal state of the object as if it would be after
-  // |StartPrepareKeyStep|.
+  // |StartPrepareKeyStep|. |public_key| is required only if |will_register_key|
+  // is true.
   virtual void RestorePreparedKeyState(AttestationKeyType key_type,
                                        bool will_register_key,
                                        const std::string& key_name,
+                                       const std::string& public_key,
                                        Profile* profile) = 0;
 };
 
@@ -144,6 +148,7 @@ class TpmChallengeKeySubtleImpl final : public TpmChallengeKeySubtle {
   void RestorePreparedKeyState(AttestationKeyType key_type,
                                bool will_register_key,
                                const std::string& key_name,
+                               const std::string& public_key,
                                Profile* profile) override;
 
   void PrepareUserKey();
@@ -177,6 +182,7 @@ class TpmChallengeKeySubtleImpl final : public TpmChallengeKeySubtle {
   void SignChallengeCallback(bool success, const std::string& response);
 
   void RegisterKeyCallback(bool success, cryptohome::MountError return_code);
+  void MarkCorporateKeyCallback(platform_keys::Status status);
 
   // Returns a trusted value from CrosSettings indicating if the device
   // attestation is enabled.
@@ -211,6 +217,10 @@ class TpmChallengeKeySubtleImpl final : public TpmChallengeKeySubtle {
   // See the comment for TpmChallengeKey::BuildResponse for more context about
   // different cases of using this variable.
   std::string key_name_;
+  // In case the key is going to be registered, the public key is stored here
+  // (after PrepareKeyFinished method is finished). It is used to mark the key
+  // as corporate.
+  std::string public_key_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

chrome/browser/chromeos/attestation/tpm_challenge_key_subtle_unittest.cc

@@ -10,6 +10,9 @@
 #include "base/test/gmock_callback_support.h"
 #include "chrome/browser/chromeos/attestation/tpm_challenge_key_result.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/mock_key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/platform_keys.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
@@ -101,7 +104,7 @@ class TpmChallengeKeySubtleTest : public ::testing::Test {
   void InitSigninProfile();
   void InitUnaffiliatedProfile();
   void InitAffiliatedProfile();
-  void SetDefaultSettings();
+  void InitAfterProfileCreated();
 
   TestingProfile* CreateUserProfile(bool is_affiliated);
   TestingProfile* GetProfile();
@@ -135,6 +138,7 @@ class TpmChallengeKeySubtleTest : public ::testing::Test {
   StrictMock<chromeos::attestation::MockAttestationFlow> mock_attestation_flow_;
   cryptohome::MockAsyncMethodCaller* mock_async_method_caller_ = nullptr;
   chromeos::FakeCryptohomeClient cryptohome_client_;
+  platform_keys::MockKeyPermissionsService* key_permissions_service_ = nullptr;
 
   TestingProfileManager testing_profile_manager_;
   chromeos::FakeChromeUserManager fake_user_manager_;
@@ -169,27 +173,35 @@ TpmChallengeKeySubtleTest::~TpmChallengeKeySubtleTest() {
 void TpmChallengeKeySubtleTest::InitSigninProfile() {
   testing_profile_ =
       testing_profile_manager_.CreateTestingProfile(chrome::kInitialProfile);
-  SetDefaultSettings();
+  InitAfterProfileCreated();
 }
 
 void TpmChallengeKeySubtleTest::InitUnaffiliatedProfile() {
   testing_profile_ = CreateUserProfile(/*is_affiliated=*/false);
-  SetDefaultSettings();
+  InitAfterProfileCreated();
 }
 
 void TpmChallengeKeySubtleTest::InitAffiliatedProfile() {
   testing_profile_ = CreateUserProfile(/*is_affiliated=*/true);
-  SetDefaultSettings();
+  InitAfterProfileCreated();
   GetProfile()->GetTestingPrefService()->SetManagedPref(
       prefs::kAttestationEnabled, std::make_unique<base::Value>(true));
 }
 
-void TpmChallengeKeySubtleTest::SetDefaultSettings() {
+void TpmChallengeKeySubtleTest::InitAfterProfileCreated() {
   GetInstallAttributes()->SetCloudManaged("google.com", "device_id");
 
   GetCrosSettingsHelper()->ReplaceDeviceSettingsProviderWithStub();
   GetCrosSettingsHelper()->SetBoolean(chromeos::kDeviceAttestationEnabled,
                                       true);
+
+  key_permissions_service_ =
+      static_cast<platform_keys::MockKeyPermissionsService*>(
+          platform_keys::KeyPermissionsServiceFactory::GetInstance()
+              ->SetTestingFactoryAndUse(
+                  GetProfile(),
+                  base::BindRepeating(
+                      &platform_keys::BuildMockKeyPermissionsService)));
 }
 
 TestingProfile* TpmChallengeKeySubtleTest::CreateUserProfile(
@@ -516,6 +528,10 @@ TEST_F(TpmChallengeKeySubtleTest, UserKeyRegisteredSuccess) {
       .WillOnce(RunOnceCallback<3>(
           /*success=*/true, cryptohome::MOUNT_ERROR_NONE));
 
+  EXPECT_CALL(*key_permissions_service_,
+              SetCorporateKey(GetPublicKey(), /*callback=*/_))
+      .WillOnce(RunOnceCallback<1>(platform_keys::Status::kSuccess));
+
   RunThreeStepsAndExpect(key_type, /*will_register_key=*/true, key_name,
                          TpmChallengeKeyResult::MakeSuccess());
 }
@@ -546,7 +562,8 @@ TEST_F(TpmChallengeKeySubtleTest, RestorePreparedKeyState) {
 
   std::unique_ptr<TpmChallengeKeySubtle> challenge_key_subtle =
       TpmChallengeKeySubtleFactory::CreateForPreparedKey(
-          key_type, /*will_register_key=*/true, key_name, GetProfile());
+          key_type, /*will_register_key=*/true, key_name, GetPublicKey(),
+          GetProfile());
 
   EXPECT_CALL(
       *mock_async_method_caller_,
@@ -573,6 +590,10 @@ TEST_F(TpmChallengeKeySubtleTest, RestorePreparedKeyState) {
       .WillOnce(RunOnceCallback<3>(
           /*success=*/true, cryptohome::MOUNT_ERROR_NONE));
 
+  EXPECT_CALL(*key_permissions_service_,
+              SetCorporateKey(GetPublicKey(), /*callback=*/_))
+      .WillOnce(RunOnceCallback<1>(platform_keys::Status::kSuccess));
+
   {
     CallbackObserver callback_observer;
     challenge_key_subtle->StartRegisterKeyStep(callback_observer.GetCallback());
@@ -590,7 +611,8 @@ TEST_F(TpmChallengeKeySubtleTest, KeyRegistrationFailed) {
 
   std::unique_ptr<TpmChallengeKeySubtle> challenge_key_subtle =
       TpmChallengeKeySubtleFactory::CreateForPreparedKey(
-          key_type, /*will_register_key=*/true, key_name, GetProfile());
+          key_type, /*will_register_key=*/true, key_name, GetPublicKey(),
+          GetProfile());
 
   EXPECT_CALL(*mock_async_method_caller_, TpmAttestationRegisterKey)
       .WillOnce(

chrome/browser/chromeos/cert_provisioning/cert_provisioning_worker.cc

@@ -889,7 +889,7 @@ void CertProvisioningWorkerImpl::InitAfterDeserialization() {
       attestation::TpmChallengeKeySubtleFactory::CreateForPreparedKey(
           GetVaKeyType(cert_scope_),
           /*will_register_key=*/true, GetKeyName(cert_profile_.profile_id),
-          profile_);
+          public_key_, profile_);
 }
 
 void CertProvisioningWorkerImpl::RegisterForInvalidationTopic() {

chrome/browser/chromeos/cert_provisioning/cert_provisioning_worker_unittest.cc

@@ -1177,11 +1177,11 @@ TEST_F(CertProvisioningWorkerTest, SerializationSuccess) {
 
     mock_tpm_challenge_key = PrepareTpmChallengeKey();
 
-    EXPECT_CALL(
-        *mock_tpm_challenge_key,
-        RestorePreparedKeyState(attestation::AttestationKeyType::KEY_USER,
-                                /*will_register_key=*/true,
-                                GetKeyName(kCertProfileId), /*profile=*/_))
+    EXPECT_CALL(*mock_tpm_challenge_key,
+                RestorePreparedKeyState(
+                    attestation::AttestationKeyType::KEY_USER,
+                    /*will_register_key=*/true, GetKeyName(kCertProfileId),
+                    GetPublicKey(), /*profile=*/_))
         .Times(1);
 
     worker = CertProvisioningWorkerFactory::Get()->Deserialize(
@@ -1259,11 +1259,11 @@ TEST_F(CertProvisioningWorkerTest, SerializationSuccess) {
     EXPECT_CALL(*mock_invalidator, Register(kInvalidationTopic, _)).Times(1);
 
     mock_tpm_challenge_key = PrepareTpmChallengeKey();
-    EXPECT_CALL(
-        *mock_tpm_challenge_key,
-        RestorePreparedKeyState(attestation::AttestationKeyType::KEY_USER,
-                                /*will_register_key=*/true,
-                                GetKeyName(kCertProfileId), /*profile=*/_))
+    EXPECT_CALL(*mock_tpm_challenge_key,
+                RestorePreparedKeyState(
+                    attestation::AttestationKeyType::KEY_USER,
+                    /*will_register_key=*/true, GetKeyName(kCertProfileId),
+                    GetPublicKey(), /*profile=*/_))
         .Times(1);
 
     worker = CertProvisioningWorkerFactory::Get()->Deserialize(