Commit 94a42a3b authored by Fabrice de Gans-Riberi's avatar Fabrice de Gans-Riberi Committed by Commit Bot

Revert "Refactor platform keys service token ID"

This reverts commit 30df8d48.

Reason for revert: Broke linux-chromeos-chrome compile. See https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/6735

Original change's description:
> Refactor platform keys service token ID
> 
> This CL changes how platform keys service expects token IDs from the
> service consumers and makes it clear what to expect in special cases
> like empty/unprovided token IDs.
> 
> Before this CL, platform keys service was accepting token IDs as strings
> which is not as clear as enums especially for the cases of empty
> strings.
> 
> Bug: 1073512
> Change-Id: I4cbdfdc8f22b23ce0297314915e52804b2e495f3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2270069
> Commit-Queue: Omar Morsi <omorsi@google.com>
> Reviewed-by: Pavol Marko <pmarko@chromium.org>
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Reviewed-by: Michael Ershov <miersh@google.com>
> Cr-Commit-Position: refs/heads/master@{#784448}

TBR=emaxx@chromium.org,pmarko@chromium.org,miersh@google.com,omorsi@google.com

Change-Id: Ib163fce34664943e586b43a8c4a9f79eb59be260
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1073512
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277117Reviewed-by: default avatarFabrice de Gans-Riberi <fdegans@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784481}
parent bc8370f3
......@@ -149,12 +149,12 @@ std::string GetVaKeyNameForSpkac(CertScope scope, CertProfileId profile_id) {
}
}
platform_keys::TokenId GetPlatformKeysTokenId(CertScope scope) {
const char* GetPlatformKeysTokenId(CertScope scope) {
switch (scope) {
case CertScope::kUser:
return platform_keys::TokenId::kUser;
return platform_keys::kTokenIdUser;
case CertScope::kDevice:
return platform_keys::TokenId::kSystem;
return platform_keys::kTokenIdSystem;
}
}
......
......@@ -11,7 +11,6 @@
#include "base/callback_forward.h"
#include "base/optional.h"
#include "base/values.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
#include "chromeos/dbus/constants/attestation_constants.h"
#include "components/policy/proto/device_management_backend.pb.h"
#include "net/cert/x509_certificate.h"
......@@ -96,7 +95,7 @@ const char* GetPrefNameForSerialization(CertScope scope);
std::string GetKeyName(CertProfileId profile_id);
// Returns the key type for VA API calls for |scope|.
attestation::AttestationKeyType GetVaKeyType(CertScope scope);
platform_keys::TokenId GetPlatformKeysTokenId(CertScope scope);
const char* GetPlatformKeysTokenId(CertScope scope);
// The Verified Access APIs are used to generate key pairs. For user-specific
// key pairs, it is possible to reuse the key pair that is used for Verified
......
......@@ -73,7 +73,7 @@ struct CertificateTestHelper {
DCHECK(cert);
}
void GetCertificates(chromeos::platform_keys::TokenId token_id,
void GetCertificates(const std::string& token_id,
const platform_keys::GetCertificatesCallback& callback) {
auto result = std::make_unique<net::CertificateList>();
*result = cert_list;
......
......@@ -436,13 +436,13 @@ TEST_F(CertProvisioningWorkerTest, Success) {
EXPECT_REGISTER_KEY_OK(*mock_tpm_challenge_key, StartRegisterKeyStep);
EXPECT_SET_ATTRIBUTE_FOR_KEY_OK(SetAttributeForKey(
platform_keys::TokenId::kUser, GetPublicKey(),
platform_keys::kTokenIdUser, GetPublicKey(),
platform_keys::KeyAttributeType::CertificateProvisioningId,
kCertProfileId, _));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(SignRSAPKCS1Digest(
::testing::Optional(platform_keys::TokenId::kUser), kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(
SignRSAPKCS1Digest(platform_keys::kTokenIdUser, kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_FINISH_CSR_OK(ClientCertProvisioningFinishCsr(
kCertScopeStrUser, kCertProfileId, kCertProfileVersion, GetPublicKey(),
......@@ -453,7 +453,7 @@ TEST_F(CertProvisioningWorkerTest, Success) {
/*callback=*/_));
EXPECT_IMPORT_CERTIFICATE_OK(ImportCertificate(
platform_keys::TokenId::kUser, /*certificate=*/_, /*callback=*/_));
platform_keys::kTokenIdUser, /*certificate=*/_, /*callback=*/_));
EXPECT_CALL(*mock_invalidator, Unregister()).Times(1);
......@@ -490,9 +490,9 @@ TEST_F(CertProvisioningWorkerTest, NoVaSuccess) {
{
testing::InSequence seq;
EXPECT_CALL(*platform_keys_service_,
GenerateRSAKey(platform_keys::TokenId::kUser,
kNonVaKeyModulusLengthBits, /*callback=*/_))
EXPECT_CALL(
*platform_keys_service_,
GenerateRSAKey("user", kNonVaKeyModulusLengthBits, /*callback=*/_))
.Times(1)
.WillOnce(RunOnceCallback<2>(GetPublicKey(), ""));
......@@ -501,13 +501,13 @@ TEST_F(CertProvisioningWorkerTest, NoVaSuccess) {
/*callback=*/_));
EXPECT_SET_ATTRIBUTE_FOR_KEY_OK(SetAttributeForKey(
platform_keys::TokenId::kUser, GetPublicKey(),
platform_keys::kTokenIdUser, GetPublicKey(),
platform_keys::KeyAttributeType::CertificateProvisioningId,
kCertProfileId, _));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(SignRSAPKCS1Digest(
::testing::Optional(platform_keys::TokenId::kUser), kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(
SignRSAPKCS1Digest(platform_keys::kTokenIdUser, kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_FINISH_CSR_OK(ClientCertProvisioningFinishCsr(
kCertScopeStrUser, kCertProfileId, kCertProfileVersion, GetPublicKey(),
......@@ -518,7 +518,7 @@ TEST_F(CertProvisioningWorkerTest, NoVaSuccess) {
/*callback=*/_));
EXPECT_IMPORT_CERTIFICATE_OK(ImportCertificate(
platform_keys::TokenId::kUser, /*certificate=*/_, /*callback=*/_));
platform_keys::kTokenIdUser, /*certificate=*/_, /*callback=*/_));
EXPECT_CALL(callback_observer_,
Callback(cert_profile, CertProvisioningWorkerState::kSucceeded))
......@@ -576,7 +576,7 @@ TEST_F(CertProvisioningWorkerTest, TryLaterManualRetry) {
EXPECT_REGISTER_KEY_OK(*mock_tpm_challenge_key, StartRegisterKeyStep);
EXPECT_SET_ATTRIBUTE_FOR_KEY_OK(SetAttributeForKey(
platform_keys::TokenId::kSystem, GetPublicKey(),
platform_keys::kTokenIdSystem, GetPublicKey(),
platform_keys::KeyAttributeType::CertificateProvisioningId,
kCertProfileId, _));
......@@ -619,7 +619,7 @@ TEST_F(CertProvisioningWorkerTest, TryLaterManualRetry) {
/*callback=*/_));
EXPECT_IMPORT_CERTIFICATE_OK(ImportCertificate(
platform_keys::TokenId::kSystem, /*certificate=*/_, /*callback=*/_));
platform_keys::kTokenIdSystem, /*certificate=*/_, /*callback=*/_));
EXPECT_CALL(callback_observer_,
Callback(cert_profile, CertProvisioningWorkerState::kSucceeded))
......@@ -682,13 +682,13 @@ TEST_F(CertProvisioningWorkerTest, TryLaterWait) {
EXPECT_REGISTER_KEY_OK(*mock_tpm_challenge_key, StartRegisterKeyStep);
EXPECT_SET_ATTRIBUTE_FOR_KEY_OK(SetAttributeForKey(
platform_keys::TokenId::kUser, GetPublicKey(),
platform_keys::kTokenIdUser, GetPublicKey(),
platform_keys::KeyAttributeType::CertificateProvisioningId,
kCertProfileId, _));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(SignRSAPKCS1Digest(
::testing::Optional(platform_keys::TokenId::kUser), kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(
SignRSAPKCS1Digest(platform_keys::kTokenIdUser, kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_FINISH_CSR_TRY_LATER(
ClientCertProvisioningFinishCsr(
......@@ -724,7 +724,7 @@ TEST_F(CertProvisioningWorkerTest, TryLaterWait) {
EXPECT_DOWNLOAD_CERT_OK(ClientCertProvisioningDownloadCert);
EXPECT_IMPORT_CERTIFICATE_OK(ImportCertificate(
platform_keys::TokenId::kUser, /*certificate=*/_, /*callback=*/_));
platform_keys::kTokenIdUser, /*certificate=*/_, /*callback=*/_));
FastForwardBy(small_delay);
// Check that minimum wait time is not too small even if the server
......@@ -957,7 +957,7 @@ TEST_F(CertProvisioningWorkerTest, RemoveRegisteredKey) {
EXPECT_REGISTER_KEY_OK(*mock_tpm_challenge_key, StartRegisterKeyStep);
EXPECT_SET_ATTRIBUTE_FOR_KEY_FAIL(SetAttributeForKey(
platform_keys::TokenId::kUser, GetPublicKey(),
platform_keys::kTokenIdUser, GetPublicKey(),
platform_keys::KeyAttributeType::CertificateProvisioningId,
kCertProfileId, _));
......@@ -965,7 +965,7 @@ TEST_F(CertProvisioningWorkerTest, RemoveRegisteredKey) {
EXPECT_CALL(
*platform_keys_service_,
RemoveKey(platform_keys::TokenId::kUser,
RemoveKey(platform_keys::kTokenIdUser,
/*public_key_spki_der=*/GetPublicKey(), /*callback=*/_))
.Times(1)
.WillOnce(RunOnceCallback<2>(/*error_message=*/""));
......@@ -1104,13 +1104,13 @@ TEST_F(CertProvisioningWorkerTest, SerializationSuccess) {
EXPECT_REGISTER_KEY_OK(*mock_tpm_challenge_key, StartRegisterKeyStep);
EXPECT_SET_ATTRIBUTE_FOR_KEY_OK(SetAttributeForKey(
platform_keys::TokenId::kUser, GetPublicKey(),
platform_keys::kTokenIdUser, GetPublicKey(),
platform_keys::KeyAttributeType::CertificateProvisioningId,
kCertProfileId, _));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(SignRSAPKCS1Digest(
::testing::Optional(platform_keys::TokenId::kUser), kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_SIGN_RSAPKC1_DIGEST_OK(
SignRSAPKCS1Digest(platform_keys::kTokenIdUser, kDataToSign,
GetPublicKey(), kPkHashAlgo, /*callback=*/_));
EXPECT_FINISH_CSR_OK(ClientCertProvisioningFinishCsr(
kCertScopeStrUser, kCertProfileId, kCertProfileVersion, GetPublicKey(),
......@@ -1169,7 +1169,7 @@ TEST_F(CertProvisioningWorkerTest, SerializationSuccess) {
/*callback=*/_));
EXPECT_IMPORT_CERTIFICATE_OK(ImportCertificate(
platform_keys::TokenId::kUser, /*certificate=*/_, /*callback=*/_));
platform_keys::kTokenIdUser, /*certificate=*/_, /*callback=*/_));
pref_val = ParseJson("{}");
EXPECT_CALL(pref_observer, OnPrefValueUpdated(IsJson(pref_val))).Times(1);
......
......@@ -58,19 +58,18 @@ bool IsExtensionAllowlisted(const extensions::Extension* extension) {
#endif // defined(OS_CHROMEOS)
// Converts |token_ids| (string-based token identifiers used in the
// platformKeys API) to a vector of KeyPermissions::KeyLocation.
// platformKeys API) to a vector of KeyPermissions::KeyLocation. Currently only
// accepts |kTokenIdUser| and |kTokenIdSystem| as |token_ids| elements.
std::vector<KeyPermissions::KeyLocation> TokenIdsToKeyLocations(
const std::vector<platform_keys::TokenId>& token_ids) {
const std::vector<std::string>& token_ids) {
std::vector<KeyPermissions::KeyLocation> key_locations;
for (const auto& token_id : token_ids) {
switch (token_id) {
case platform_keys::TokenId::kUser:
key_locations.push_back(KeyPermissions::KeyLocation::kUserSlot);
break;
case platform_keys::TokenId::kSystem:
key_locations.push_back(KeyPermissions::KeyLocation::kSystemSlot);
break;
}
if (token_id == platform_keys::kTokenIdUser)
key_locations.push_back(KeyPermissions::KeyLocation::kUserSlot);
else if (token_id == platform_keys::kTokenIdSystem)
key_locations.push_back(KeyPermissions::KeyLocation::kSystemSlot);
else
NOTREACHED() << "Unknown platformKeys API token id " << token_id;
}
return key_locations;
}
......@@ -97,7 +96,7 @@ class ExtensionPlatformKeysService::GenerateKeyTask : public Task {
DONE,
};
GenerateKeyTask(platform_keys::TokenId token_id,
GenerateKeyTask(const std::string& token_id,
const std::string& extension_id,
const GenerateKeyCallback& callback,
KeyPermissions* key_permissions,
......@@ -120,7 +119,7 @@ class ExtensionPlatformKeysService::GenerateKeyTask : public Task {
protected:
virtual void GenerateKey(GenerateKeyCallback callback) = 0;
platform_keys::TokenId token_id_;
const std::string token_id_;
std::string public_key_spki_der_;
const std::string extension_id_;
GenerateKeyCallback callback_;
......@@ -202,7 +201,7 @@ class ExtensionPlatformKeysService::GenerateRSAKeyTask
// This key task generates an RSA key with the parameters |token_id| and
// |modulus_length| and registers it for the extension with id |extension_id|.
// The generated key will be passed to |callback|.
GenerateRSAKeyTask(platform_keys::TokenId token_id,
GenerateRSAKeyTask(const std::string& token_id,
unsigned int modulus_length,
const std::string& extension_id,
const GenerateKeyCallback& callback,
......@@ -232,7 +231,7 @@ class ExtensionPlatformKeysService::GenerateECKeyTask : public GenerateKeyTask {
// This Task generates an EC key with the parameters |token_id| and
// |named_curve| and registers it for the extension with id |extension_id|.
// The generated key will be passed to |callback|.
GenerateECKeyTask(platform_keys::TokenId token_id,
GenerateECKeyTask(const std::string& token_id,
const std::string& named_curve,
const std::string& extension_id,
const GenerateKeyCallback& callback,
......@@ -273,7 +272,7 @@ class ExtensionPlatformKeysService::SignTask : public Task {
// multiple times, also updates the permission to prevent any future signing
// operation of that extension using that same key. If an error occurs, an
// error message is passed to |callback| instead.
SignTask(base::Optional<platform_keys::TokenId> token_id,
SignTask(const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
bool raw_pkcs1,
......@@ -359,7 +358,7 @@ class ExtensionPlatformKeysService::SignTask : public Task {
base::BindRepeating(&SignTask::GotKeyLocation, base::Unretained(this)));
}
void GotKeyLocation(const std::vector<platform_keys::TokenId>& token_ids,
void GotKeyLocation(const std::vector<std::string>& token_ids,
const std::string& error_message) {
if (!error_message.empty()) {
next_step_ = Step::DONE;
......@@ -409,7 +408,7 @@ class ExtensionPlatformKeysService::SignTask : public Task {
Step next_step_ = Step::GET_EXTENSION_PERMISSIONS;
base::Optional<platform_keys::TokenId> token_id_;
const std::string token_id_;
const std::string data_;
const std::string public_key_spki_der_;
......@@ -603,7 +602,7 @@ class ExtensionPlatformKeysService::SelectTask : public Task {
}
void GotKeyLocations(const scoped_refptr<net::X509Certificate>& certificate,
const std::vector<platform_keys::TokenId>& token_ids,
const std::vector<std::string>& token_ids,
const std::string& error_message) {
if (!error_message.empty()) {
next_step_ = Step::DONE;
......@@ -780,7 +779,7 @@ void ExtensionPlatformKeysService::SetSelectDelegate(
}
void ExtensionPlatformKeysService::GenerateRSAKey(
platform_keys::TokenId token_id,
const std::string& token_id,
unsigned int modulus_length,
const std::string& extension_id,
const GenerateKeyCallback& callback) {
......@@ -791,7 +790,7 @@ void ExtensionPlatformKeysService::GenerateRSAKey(
}
void ExtensionPlatformKeysService::GenerateECKey(
platform_keys::TokenId token_id,
const std::string& token_id,
const std::string& named_curve,
const std::string& extension_id,
const GenerateKeyCallback& callback) {
......@@ -806,7 +805,7 @@ bool ExtensionPlatformKeysService::IsUsingSigninProfile() {
}
void ExtensionPlatformKeysService::SignDigest(
base::Optional<platform_keys::TokenId> token_id,
const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
platform_keys::KeyType key_type,
......@@ -821,7 +820,7 @@ void ExtensionPlatformKeysService::SignDigest(
}
void ExtensionPlatformKeysService::SignRSAPKCS1Raw(
base::Optional<platform_keys::TokenId> token_id,
const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
const std::string& extension_id,
......
......@@ -102,7 +102,7 @@ class ExtensionPlatformKeysService : public KeyedService {
// specifies the token to store the key pair on. |callback| will be invoked
// with the resulting public key or an error. Will only call back during the
// lifetime of this object.
void GenerateRSAKey(platform_keys::TokenId token_id,
void GenerateRSAKey(const std::string& token_id,
unsigned int modulus_length_bits,
const std::string& extension_id,
const GenerateKeyCallback& callback);
......@@ -112,7 +112,7 @@ class ExtensionPlatformKeysService : public KeyedService {
// token to store the key pair on. |callback| will be invoked with the
// resulting public key or an error. Will only call back during the lifetime
// of this object.
void GenerateECKey(platform_keys::TokenId token_id,
void GenerateECKey(const std::string& token_id,
const std::string& named_curve,
const std::string& extension_id,
const GenerateKeyCallback& callback);
......@@ -130,16 +130,14 @@ class ExtensionPlatformKeysService : public KeyedService {
// Digests |data|, applies PKCS1 padding if specified by |hash_algorithm| and
// chooses the signature algorithm according to |key_type| and signs the data
// with the private key matching |public_key_spki_der|. If a |token_id|
// is provided and the key is not found in that token, the operation aborts.
// If |token_id| is not provided (nullopt), all tokens available to the caller
// will be considered while searching for the key.
// If the extension does not have permissions for signing with this key, the
// operation aborts. In case of a one time permission (granted after
// with the private key matching |public_key_spki_der|. If a non empty token
// id is provided and the key is not found in that token, the operation
// aborts. If the extension does not have permissions for signing with this
// key, the operation aborts. In case of a one time permission (granted after
// generating the key), this function also removes the permission to prevent
// future signing attempts. |callback| will be invoked with the signature or
// an error message. Will only call back during the lifetime of this object.
void SignDigest(base::Optional<platform_keys::TokenId> token_id,
void SignDigest(const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
platform_keys::KeyType key_type,
......@@ -148,17 +146,18 @@ class ExtensionPlatformKeysService : public KeyedService {
const SignCallback& callback);
// Applies PKCS1 padding and afterwards signs the data with the private key
// matching |public_key_spki_der|. |data| is not digested. If a |token_id|
// is provided and the key is not found in that token, the operation aborts.
// If |token_id| is not provided (nullopt), all available tokens to the caller
// will be considered while searching for the key. The size of |data| (number
// of octets) must be smaller than k - 11, where k is the key size in octets.
// matching |public_key_spki_der|. |data| is not digested. If a non empty
// token id is provided and the key is not found in that token, the operation
// aborts.
// The size of |data| (number of octets) must be smaller than k - 11, where k
// is the key size in octets.
// If the extension does not have permissions for signing with this key, the
// operation aborts. In case of a one time permission (granted after
// generating the key), this function also removes the permission to prevent
// future signing attempts. |callback| will be invoked with the signature or
// an error message. Will only call back during the lifetime of this object.
void SignRSAPKCS1Raw(base::Optional<platform_keys::TokenId> token_id,
// future signing attempts.
// |callback| will be invoked with the signature or an error message.
// Will only call back during the lifetime of this object.
void SignRSAPKCS1Raw(const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
const std::string& extension_id,
......
......@@ -23,21 +23,21 @@ class MockPlatformKeysService : public PlatformKeysService {
MOCK_METHOD(void,
GenerateRSAKey,
(TokenId token_id,
(const std::string& token_id,
unsigned int modulus_length_bits,
const GenerateKeyCallback& callback),
(override));
MOCK_METHOD(void,
GenerateECKey,
(TokenId token_id,
(const std::string& token_id,
const std::string& named_curve,
const GenerateKeyCallback& callback),
(override));
MOCK_METHOD(void,
SignRSAPKCS1Digest,
(base::Optional<TokenId> token_id,
(const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
HashAlgorithm hash_algorithm,
......@@ -46,7 +46,7 @@ class MockPlatformKeysService : public PlatformKeysService {
MOCK_METHOD(void,
SignRSAPKCS1Raw,
(base::Optional<TokenId> token_id,
(const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
const SignCallback& callback),
......@@ -54,7 +54,7 @@ class MockPlatformKeysService : public PlatformKeysService {
MOCK_METHOD(void,
SignECDSADigest,
(base::Optional<TokenId> token_id,
(const std::string& token_id,
const std::string& data,
const std::string& public_key_spki_der,
HashAlgorithm hash_algorithm,
......@@ -69,31 +69,32 @@ class MockPlatformKeysService : public PlatformKeysService {
MOCK_METHOD(void,
GetCertificates,
(TokenId token_id, const GetCertificatesCallback& callback),
(const std::string& token_id,
const GetCertificatesCallback& callback),
(override));
MOCK_METHOD(void,
GetAllKeys,
(TokenId token_id, GetAllKeysCallback callback),
(const std::string& token_id, GetAllKeysCallback callback),
(override));
MOCK_METHOD(void,
ImportCertificate,
(TokenId token_id,
(const std::string& token_id,
const scoped_refptr<net::X509Certificate>& certificate,
const ImportCertificateCallback& callback),
(override));
MOCK_METHOD(void,
RemoveCertificate,
(TokenId token_id,
(const std::string& token_id,
const scoped_refptr<net::X509Certificate>& certificate,
const RemoveCertificateCallback& callback),
(override));
MOCK_METHOD(void,
RemoveKey,
(TokenId token_id,
(const std::string& token_id,
const std::string& public_key_spki_der,
RemoveKeyCallback callback),
(override));
......@@ -108,7 +109,7 @@ class MockPlatformKeysService : public PlatformKeysService {
MOCK_METHOD(void,
SetAttributeForKey,
(TokenId token_id,
(const std::string& token_id,
const std::string& public_key_spki_der,
KeyAttributeType attribute_type,
const std::string& attribute_value,
......@@ -117,7 +118,7 @@ class MockPlatformKeysService : public PlatformKeysService {
MOCK_METHOD(void,
GetAttributeForKey,
(TokenId token_id,
(const std::string& token_id,
const std::string& public_key_spki_der,
KeyAttributeType attribute_type,
GetAttributeForKeyCallback callback),
......
......@@ -17,6 +17,9 @@
namespace chromeos {
namespace platform_keys {
const char kTokenIdUser[] = "user";
const char kTokenIdSystem[] = "system";
namespace {
void IntersectOnWorkerThread(const net::CertificateList& certs1,
......
......@@ -15,8 +15,6 @@
#include "base/location.h"
#include "base/memory/weak_ptr.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
#include "base/task/post_task.h"
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/chromeos/login/test/device_state_mixin.h"
......@@ -60,6 +58,8 @@ namespace {
constexpr char kTestUserEmail[] = "test@example.com";
constexpr char kTestAffiliationId[] = "test_affiliation_id";
constexpr char kSystemToken[] = "system";
constexpr char kUserToken[] = "user";
enum class ProfileToUse {
// A Profile that belongs to a user that is not affiliated with the device (no
......@@ -80,7 +80,7 @@ struct TestConfig {
// The token IDs that are expected to be available. This will be checked by
// the GetTokens test, and operation for these tokens will be performed by the
// other tests.
std::vector<TokenId> token_ids;
std::vector<std::string> token_ids;
};
// Softoken NSS PKCS11 module (used for testing) allows only predefined key
......@@ -177,12 +177,12 @@ class ExecutionWaiter {
// Supports waiting for the result of PlatformKeysService::GetTokens.
class GetTokensExecutionWaiter
: public ExecutionWaiter<std::unique_ptr<std::vector<TokenId>>> {
: public ExecutionWaiter<std::unique_ptr<std::vector<std::string>>> {
public:
GetTokensExecutionWaiter() = default;
~GetTokensExecutionWaiter() = default;
const std::unique_ptr<std::vector<TokenId>>& token_ids() const {
const std::unique_ptr<std::vector<std::string>>& token_ids() const {
return std::get<0>(result_callback_args());
}
};
......@@ -330,18 +330,17 @@ class PlatformKeysServiceBrowserTest
}
// Returns the slot to be used depending on |token_id|.
PK11SlotInfo* GetSlot(TokenId token_id) {
switch (token_id) {
case TokenId::kSystem:
return system_nss_key_slot_mixin_.slot();
case TokenId::kUser:
return user_slot_.get();
PK11SlotInfo* GetSlot(const std::string& token_id) {
if (token_id == kSystemToken) {
return system_nss_key_slot_mixin_.slot();
}
DCHECK_EQ(token_id, kUserToken);
return user_slot_.get();
}
// Generates a key pair in the given |token_id| using platform keys service
// and returns the SubjectPublicKeyInfo string encoded in DER format.
std::string GenerateKeyPair(TokenId token_id) {
std::string GenerateKeyPair(const std::string& token_id) {
const unsigned int kKeySize = 2048;
GenerateKeyExecutionWaiter generate_key_waiter;
......@@ -405,7 +404,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GenerateRsaAndSign) {
const crypto::SignatureVerifier::SignatureAlgorithm signature_algorithm =
crypto::SignatureVerifier::RSA_PKCS1_SHA256;
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
GenerateKeyExecutionWaiter generate_key_waiter;
platform_keys_service()->GenerateRSAKey(token_id, kKeySize,
generate_key_waiter.GetCallback());
......@@ -439,10 +438,8 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, SetAndGetKeyAttribute) {
const KeyAttributeType kAttributeType =
KeyAttributeType::CertificateProvisioningId;
for (TokenId token_id : GetParam().token_ids) {
const int token_id_as_int = static_cast<int>(token_id);
const std::string attribute_value =
base::StringPrintf("test%d", token_id_as_int);
for (const std::string& token_id : GetParam().token_ids) {
const std::string kAttributeValue = "test" + token_id;
// Generate key pair.
const std::string public_key_spki_der = GenerateKeyPair(token_id);
......@@ -451,7 +448,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, SetAndGetKeyAttribute) {
// Set key attribute.
SetAttributeForKeyExecutionWaiter set_attribute_for_key_execution_waiter;
platform_keys_service()->SetAttributeForKey(
token_id, public_key_spki_der, kAttributeType, attribute_value,
token_id, public_key_spki_der, kAttributeType, kAttributeValue,
set_attribute_for_key_execution_waiter.GetCallback());
set_attribute_for_key_execution_waiter.Wait();
......@@ -464,7 +461,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, SetAndGetKeyAttribute) {
EXPECT_TRUE(get_attribute_for_key_execution_waiter.error_message().empty());
EXPECT_EQ(get_attribute_for_key_execution_waiter.attribute_value(),
attribute_value);
kAttributeValue);
}
}
......@@ -472,7 +469,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetUnsetKeyAttribute) {
const KeyAttributeType kAttributeType =
KeyAttributeType::CertificateProvisioningId;
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
// Generate key pair.
const std::string public_key_spki_der = GenerateKeyPair(token_id);
ASSERT_FALSE(public_key_spki_der.empty());
......@@ -496,7 +493,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
KeyAttributeType::CertificateProvisioningId;
const std::string kPublicKey = "Non Existing public key";
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
// Get key attribute.
GetAttributeForKeyExecutionWaiter get_attribute_for_key_execution_waiter;
platform_keys_service()->GetAttributeForKey(
......@@ -516,7 +513,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
const std::string kAttributeValue = "test";
const std::string kPublicKey = "Non Existing public key";
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
// Set key attribute.
SetAttributeForKeyExecutionWaiter set_attribute_for_key_execution_waiter;
platform_keys_service()->SetAttributeForKey(
......@@ -531,7 +528,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
RemoveKeyWithNoMatchingCertificates) {
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
// Generate first key pair.
const std::string public_key_1 = GenerateKeyPair(token_id);
ASSERT_FALSE(public_key_1.empty());
......@@ -558,7 +555,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
RemoveKeyWithMatchingCertificate) {
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
PK11SlotInfo* const slot = GetSlot(token_id);
// Assert that there are no certificates before importing.
......@@ -619,15 +616,15 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
// retrieves them.
IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetAllKeys) {
// Generate key pair in every token.
std::map<TokenId, std::string> token_key_map;
for (TokenId token_id : GetParam().token_ids) {
std::map<std::string, std::string> token_key_map;
for (const std::string& token_id : GetParam().token_ids) {
const std::string public_key_spki_der = GenerateKeyPair(token_id);
ASSERT_FALSE(public_key_spki_der.empty());
token_key_map[token_id] = public_key_spki_der;
}
// Only keys in the requested token should be retrieved.
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
GetAllKeysExecutionWaiter get_all_keys_waiter;
platform_keys_service()->GetAllKeys(token_id,
get_all_keys_waiter.GetCallback());
......@@ -642,7 +639,7 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest, GetAllKeys) {
IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
GetAllKeysWhenNoKeysGenerated) {
for (TokenId token_id : GetParam().token_ids) {
for (const std::string& token_id : GetParam().token_ids) {
GetAllKeysExecutionWaiter get_all_keys_waiter;
platform_keys_service()->GetAllKeys(token_id,
get_all_keys_waiter.GetCallback());
......@@ -657,11 +654,11 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServiceBrowserTest,
INSTANTIATE_TEST_SUITE_P(
AllSupportedProfileTypes,
PlatformKeysServiceBrowserTest,
::testing::Values(
TestConfig{ProfileToUse::kSigninProfile, {TokenId::kSystem}},
TestConfig{ProfileToUse::kUnaffiliatedUserProfile, {TokenId::kUser}},
TestConfig{ProfileToUse::kAffiliatedUserProfile,
{TokenId::kSystem, TokenId::kUser}}));
::testing::Values(TestConfig{ProfileToUse::kSigninProfile, {kSystemToken}},
TestConfig{ProfileToUse::kUnaffiliatedUserProfile,
{kUserToken}},
TestConfig{ProfileToUse::kAffiliatedUserProfile,
{kSystemToken, kUserToken}}));
} // namespace platform_keys
} // namespace chromeos
......@@ -7,7 +7,6 @@
#include <utility>
#include "base/bind.h"
#include "base/optional.h"
#include "base/values.h"
#include "chrome/browser/chromeos/platform_keys/extension_platform_keys_service.h"
#include "chrome/browser/chromeos/platform_keys/extension_platform_keys_service_factory.h"
......@@ -54,9 +53,8 @@ EnterprisePlatformKeysInternalGenerateKeyFunction::Run() {
api_epki::GenerateKey::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
base::Optional<chromeos::platform_keys::TokenId> platform_keys_token_id =
platform_keys::ApiIdToPlatformKeysTokenId(params->token_id);
if (!platform_keys_token_id)
std::string platform_keys_token_id;
if (!platform_keys::ValidateToken(params->token_id, &platform_keys_token_id))
return RespondNow(Error(platform_keys::kErrorInvalidToken));
chromeos::ExtensionPlatformKeysService* service =
......@@ -69,7 +67,7 @@ EnterprisePlatformKeysInternalGenerateKeyFunction::Run() {
EXTENSION_FUNCTION_VALIDATE(params->algorithm.modulus_length &&
*(params->algorithm.modulus_length) >= 0);
service->GenerateRSAKey(
platform_keys_token_id.value(), *(params->algorithm.modulus_length),
platform_keys_token_id, *(params->algorithm.modulus_length),
extension_id(),
base::Bind(
&EnterprisePlatformKeysInternalGenerateKeyFunction::OnGeneratedKey,
......@@ -77,7 +75,7 @@ EnterprisePlatformKeysInternalGenerateKeyFunction::Run() {
} else if (params->algorithm.name == "ECDSA") {
EXTENSION_FUNCTION_VALIDATE(params->algorithm.named_curve);
service->GenerateECKey(
platform_keys_token_id.value(), *(params->algorithm.named_curve),
platform_keys_token_id, *(params->algorithm.named_curve),
extension_id(),
base::Bind(
&EnterprisePlatformKeysInternalGenerateKeyFunction::OnGeneratedKey,
......@@ -109,16 +107,15 @@ EnterprisePlatformKeysGetCertificatesFunction::Run() {
std::unique_ptr<api_epk::GetCertificates::Params> params(
api_epk::GetCertificates::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
base::Optional<chromeos::platform_keys::TokenId> platform_keys_token_id =
platform_keys::ApiIdToPlatformKeysTokenId(params->token_id);
if (!platform_keys_token_id)
std::string platform_keys_token_id;
if (!platform_keys::ValidateToken(params->token_id, &platform_keys_token_id))
return RespondNow(Error(platform_keys::kErrorInvalidToken));
chromeos::platform_keys::PlatformKeysService* platform_keys_service =
chromeos::platform_keys::PlatformKeysServiceFactory::GetForBrowserContext(
browser_context());
platform_keys_service->GetCertificates(
platform_keys_token_id.value(),
platform_keys_token_id,
base::Bind(
&EnterprisePlatformKeysGetCertificatesFunction::OnGotCertificates,
this));
......@@ -156,9 +153,8 @@ EnterprisePlatformKeysImportCertificateFunction::Run() {
std::unique_ptr<api_epk::ImportCertificate::Params> params(
api_epk::ImportCertificate::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
base::Optional<chromeos::platform_keys::TokenId> platform_keys_token_id =
platform_keys::ApiIdToPlatformKeysTokenId(params->token_id);
if (!platform_keys_token_id)
std::string platform_keys_token_id;
if (!platform_keys::ValidateToken(params->token_id, &platform_keys_token_id))
return RespondNow(Error(platform_keys::kErrorInvalidToken));
const std::vector<uint8_t>& cert_der = params->certificate;
......@@ -179,7 +175,7 @@ EnterprisePlatformKeysImportCertificateFunction::Run() {
CHECK(platform_keys_service);
platform_keys_service->ImportCertificate(
platform_keys_token_id.value(), cert_x509,
platform_keys_token_id, cert_x509,
base::Bind(&EnterprisePlatformKeysImportCertificateFunction::
OnImportedCertificate,
this));
......@@ -203,9 +199,8 @@ EnterprisePlatformKeysRemoveCertificateFunction::Run() {
std::unique_ptr<api_epk::RemoveCertificate::Params> params(
api_epk::RemoveCertificate::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
base::Optional<chromeos::platform_keys::TokenId> platform_keys_token_id =
platform_keys::ApiIdToPlatformKeysTokenId(params->token_id);
if (!platform_keys_token_id)
std::string platform_keys_token_id;
if (!platform_keys::ValidateToken(params->token_id, &platform_keys_token_id))
return RespondNow(Error(platform_keys::kErrorInvalidToken));
const std::vector<uint8_t>& cert_der = params->certificate;
......@@ -226,7 +221,7 @@ EnterprisePlatformKeysRemoveCertificateFunction::Run() {
CHECK(platform_keys_service);
platform_keys_service->RemoveCertificate(
platform_keys_token_id.value(), cert_x509,
platform_keys_token_id, cert_x509,
base::Bind(&EnterprisePlatformKeysRemoveCertificateFunction::
OnRemovedCertificate,
this));
......@@ -260,8 +255,7 @@ EnterprisePlatformKeysInternalGetTokensFunction::Run() {
}
void EnterprisePlatformKeysInternalGetTokensFunction::OnGotTokens(
std::unique_ptr<std::vector<chromeos::platform_keys::TokenId>>
platform_keys_token_ids,
std::unique_ptr<std::vector<std::string>> platform_keys_token_ids,
const std::string& error_message) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!error_message.empty()) {
......@@ -270,14 +264,15 @@ void EnterprisePlatformKeysInternalGetTokensFunction::OnGotTokens(
}
std::vector<std::string> token_ids;
for (auto token_id : *platform_keys_token_ids) {
std::string api_token_id =
platform_keys::PlatformKeysTokenIdToApiId(token_id);
if (api_token_id.empty()) {
for (std::vector<std::string>::const_iterator it =
platform_keys_token_ids->begin();
it != platform_keys_token_ids->end(); ++it) {
std::string token_id = platform_keys::PlatformKeysTokenIdToApiId(*it);
if (token_id.empty()) {
Respond(Error(kEnterprisePlatformErrorInternal));
return;
}
token_ids.push_back(api_token_id);
token_ids.push_back(token_id);
}
Respond(ArgumentList(api_epki::GetTokens::Results::Create(token_ids)));
......
......@@ -10,7 +10,6 @@
#include <vector>
#include "base/memory/ref_counted.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
#include "chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h"
#include "extensions/browser/extension_function.h"
......@@ -86,9 +85,8 @@ class EnterprisePlatformKeysInternalGetTokensFunction
// Called when the list of tokens was determined. If an error occurred,
// |token_ids| will be NULL and instead |error_message| be set.
void OnGotTokens(
std::unique_ptr<std::vector<chromeos::platform_keys::TokenId>> token_ids,
const std::string& error_message);
void OnGotTokens(std::unique_ptr<std::vector<std::string>> token_ids,
const std::string& error_message);
DECLARE_EXTENSION_FUNCTION("enterprise.platformKeysInternal.getTokens",
ENTERPRISE_PLATFORMKEYSINTERNAL_GETTOKENS)
......
......@@ -10,7 +10,6 @@
#include "base/bind.h"
#include "base/logging.h"
#include "base/optional.h"
#include "base/stl_util.h"
#include "base/strings/string_piece.h"
#include "base/values.h"
......@@ -104,25 +103,29 @@ const char kErrorInvalidX509Cert[] =
const char kTokenIdUser[] = "user";
const char kTokenIdSystem[] = "system";
base::Optional<chromeos::platform_keys::TokenId> ApiIdToPlatformKeysTokenId(
const std::string& token_id) {
if (token_id == kTokenIdUser)
return chromeos::platform_keys::TokenId::kUser;
if (token_id == kTokenIdSystem)
return chromeos::platform_keys::TokenId::kSystem;
return base::nullopt;
// Returns whether |token_id| references a known Token.
bool ValidateToken(const std::string& token_id,
std::string* platform_keys_token_id) {
platform_keys_token_id->clear();
if (token_id == kTokenIdUser) {
*platform_keys_token_id = chromeos::platform_keys::kTokenIdUser;
return true;
}
if (token_id == kTokenIdSystem) {
*platform_keys_token_id = chromeos::platform_keys::kTokenIdSystem;
return true;
}
return false;
}
std::string PlatformKeysTokenIdToApiId(
chromeos::platform_keys::TokenId platform_keys_token_id) {
switch (platform_keys_token_id) {
case chromeos::platform_keys::TokenId::kUser:
return kTokenIdUser;
case chromeos::platform_keys::TokenId::kSystem:
return kTokenIdSystem;
}
const std::string& platform_keys_token_id) {
if (platform_keys_token_id == chromeos::platform_keys::kTokenIdUser)
return kTokenIdUser;
if (platform_keys_token_id == chromeos::platform_keys::kTokenIdSystem)
return kTokenIdSystem;
return std::string();
}
} // namespace platform_keys
......@@ -361,16 +364,11 @@ ExtensionFunction::ResponseAction PlatformKeysInternalSignFunction::Run() {
std::unique_ptr<api_pki::Sign::Params> params(
api_pki::Sign::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
base::Optional<chromeos::platform_keys::TokenId> platform_keys_token_id;
// If |params->token_id| is not specified (empty string), the key will be
// searched for in all available tokens.
if (!params->token_id.empty()) {
platform_keys_token_id =
platform_keys::ApiIdToPlatformKeysTokenId(params->token_id);
if (!platform_keys_token_id) {
return RespondNow(Error(platform_keys::kErrorInvalidToken));
}
std::string platform_keys_token_id;
if (!params->token_id.empty() &&
!platform_keys::ValidateToken(params->token_id,
&platform_keys_token_id)) {
return RespondNow(Error(platform_keys::kErrorInvalidToken));
}
chromeos::ExtensionPlatformKeysService* service =
......
......@@ -8,14 +8,12 @@
#include <string>
#include <vector>
#include "base/optional.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
#include "extensions/browser/extension_function.h"
namespace net {
class X509Certificate;
typedef std::vector<scoped_refptr<X509Certificate>> CertificateList;
} // namespace net
} // net
namespace extensions {
namespace platform_keys {
......@@ -23,15 +21,14 @@ namespace platform_keys {
extern const char kErrorInvalidToken[];
extern const char kErrorInvalidX509Cert[];
// Returns a known token if |token_id| is valid and returns nullopt for both
// empty or unknown |token_id|.
base::Optional<chromeos::platform_keys::TokenId> ApiIdToPlatformKeysTokenId(
const std::string& token_id);
// Returns whether |token_id| references a known Token.
bool ValidateToken(const std::string& token_id,
std::string* platform_keys_token_id);
// Converts a token id from ::chromeos::platform_keys to the platformKeys API
// token id.
std::string PlatformKeysTokenIdToApiId(
chromeos::platform_keys::TokenId platform_keys_token_id);
const std::string& platform_keys_token_id);
} // namespace platform_keys
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment