diff options
author | tengs <tengs@chromium.org> | 2015-06-29 11:47:15 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-06-29 18:48:54 +0000 |
commit | 7555b0b6c4fdc89292cfb504a3f7802221e6d45d (patch) | |
tree | 20170415d6d1d0f9095f7dd6b0696d9e3fa3c14d /components/proximity_auth | |
parent | 839029f5f80f44b60d7402acafa87d6a272fa498 (diff) | |
download | chromium_src-7555b0b6c4fdc89292cfb504a3f7802221e6d45d.zip chromium_src-7555b0b6c4fdc89292cfb504a3f7802221e6d45d.tar.gz chromium_src-7555b0b6c4fdc89292cfb504a3f7802221e6d45d.tar.bz2 |
Fix CryptAuth enrollment logic.
The current enrollment flow is not quite right, so I needed to make some changes:
- Use the user's persistent key-pair to sign the inner message.
- Change the endpoint URLs
- Fill GcmDeviceInfo proto with fields that are validated by the server, such as the access token.
BUG=420315
TEST=manual and unit tests
Review URL: https://codereview.chromium.org/1151023005
Cr-Commit-Position: refs/heads/master@{#336602}
Diffstat (limited to 'components/proximity_auth')
17 files changed, 244 insertions, 79 deletions
diff --git a/components/proximity_auth/ble/proximity_auth_ble_system.cc b/components/proximity_auth/ble/proximity_auth_ble_system.cc index a564005..cfd2074 100644 --- a/components/proximity_auth/ble/proximity_auth_ble_system.cc +++ b/components/proximity_auth/ble/proximity_auth_ble_system.cc @@ -14,6 +14,7 @@ #include "components/proximity_auth/connection.h" #include "components/proximity_auth/cryptauth/base64url.h" #include "components/proximity_auth/cryptauth/cryptauth_client.h" +#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h" #include "components/proximity_auth/logging/logging.h" #include "components/proximity_auth/remote_device.h" #include "device/bluetooth/bluetooth_device.h" diff --git a/components/proximity_auth/cryptauth/BUILD.gn b/components/proximity_auth/cryptauth/BUILD.gn index 2da4244..ddfc973 100644 --- a/components/proximity_auth/cryptauth/BUILD.gn +++ b/components/proximity_auth/cryptauth/BUILD.gn @@ -33,6 +33,7 @@ source_set("cryptauth") { deps = [ "//base", + "//crypto", "//components/proximity_auth/logging", "//google_apis", "//net", diff --git a/components/proximity_auth/cryptauth/DEPS b/components/proximity_auth/cryptauth/DEPS index 717a290..1e58ca5 100644 --- a/components/proximity_auth/cryptauth/DEPS +++ b/components/proximity_auth/cryptauth/DEPS @@ -1,4 +1,5 @@ include_rules = [ + "+crypto", "+google_apis", "+net", ] diff --git a/components/proximity_auth/cryptauth/cryptauth_api_call_flow.cc b/components/proximity_auth/cryptauth/cryptauth_api_call_flow.cc index acefa12..c72e867 100644 --- a/components/proximity_auth/cryptauth/cryptauth_api_call_flow.cc +++ b/components/proximity_auth/cryptauth/cryptauth_api_call_flow.cc @@ -5,6 +5,7 @@ #include "components/proximity_auth/cryptauth/cryptauth_api_call_flow.h" #include "base/strings/string_number_conversions.h" +#include "components/proximity_auth/logging/logging.h" #include "net/url_request/url_fetcher.h" namespace proximity_auth { @@ -67,6 +68,10 @@ void CryptAuthApiCallFlow::ProcessApiCallFailure( } else { error_message = kRequestFailedError; } + + std::string response; + source->GetResponseAsString(&response); + PA_LOG(INFO) << "API call failed:\n" << response; error_callback_.Run(error_message); } diff --git a/components/proximity_auth/cryptauth/cryptauth_client.h b/components/proximity_auth/cryptauth/cryptauth_client.h index 7dfa236..d0bf1ab 100644 --- a/components/proximity_auth/cryptauth/cryptauth_client.h +++ b/components/proximity_auth/cryptauth/cryptauth_client.h @@ -5,10 +5,27 @@ #ifndef COMPONENTS_PROXIMITY_AUTH_CRYPTAUTH_CLIENT_H #define COMPONENTS_PROXIMITY_AUTH_CRYPTAUTH_CLIENT_H +#include <string> + #include "base/callback_forward.h" #include "base/macros.h" #include "base/memory/scoped_ptr.h" -#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h" + +namespace cryptauth { +class DeviceClassifier; +class GetMyDevicesRequest; +class GetMyDevicesResponse; +class FindEligibleUnlockDevicesRequest; +class FindEligibleUnlockDevicesResponse; +class SendDeviceSyncTickleRequest; +class SendDeviceSyncTickleResponse; +class ToggleEasyUnlockRequest; +class ToggleEasyUnlockResponse; +class SetupEnrollmentRequest; +class SetupEnrollmentResponse; +class FinishEnrollmentRequest; +class FinishEnrollmentResponse; +} namespace net { class URLRequestContextGetter; @@ -77,6 +94,10 @@ class CryptAuthClient { const cryptauth::FinishEnrollmentRequest& request, const FinishEnrollmentCallback& callback, const ErrorCallback& error_callback) = 0; + + // Returns the access token used to make the request. If no request has been + // made yet, this function will return an empty string. + virtual std::string GetAccessTokenUsed() = 0; }; // Interface for creating CryptAuthClient instances. Because each diff --git a/components/proximity_auth/cryptauth/cryptauth_client_impl.cc b/components/proximity_auth/cryptauth/cryptauth_client_impl.cc index 32ec704..73ab9e5 100644 --- a/components/proximity_auth/cryptauth/cryptauth_client_impl.cc +++ b/components/proximity_auth/cryptauth/cryptauth_client_impl.cc @@ -25,8 +25,8 @@ const char kFindEligibleUnlockDevicesPath[] = "deviceSync/findeligibleunlockdevices"; const char kSendDeviceSyncTicklePath[] = "deviceSync/senddevicesynctickle"; const char kToggleEasyUnlockPath[] = "deviceSync/toggleeasyunlock"; -const char kSetupEnrollmentPath[] = "enrollment/setupenrollment"; -const char kFinishEnrollmentPath[] = "enrollment/finishenrollment"; +const char kSetupEnrollmentPath[] = "enrollment/setup"; +const char kFinishEnrollmentPath[] = "enrollment/finish"; // Query string of the API URL indicating that the response should be in a // serialized protobuf format. @@ -104,6 +104,10 @@ void CryptAuthClientImpl::FinishEnrollment( MakeApiCall(kFinishEnrollmentPath, request, callback, error_callback); } +std::string CryptAuthClientImpl::GetAccessTokenUsed() { + return access_token_used_; +} + template <class RequestProto, class ResponseProto> void CryptAuthClientImpl::MakeApiCall( const std::string& request_path, @@ -144,6 +148,7 @@ void CryptAuthClientImpl::OnAccessTokenFetched( OnApiCallFailed("Failed to get a valid access token."); return; } + access_token_used_ = access_token; api_call_flow_->Start( CreateRequestUrl(request_path_), url_request_context_.get(), access_token, diff --git a/components/proximity_auth/cryptauth/cryptauth_client_impl.h b/components/proximity_auth/cryptauth/cryptauth_client_impl.h index aa19594..787254b 100644 --- a/components/proximity_auth/cryptauth/cryptauth_client_impl.h +++ b/components/proximity_auth/cryptauth/cryptauth_client_impl.h @@ -9,6 +9,7 @@ #include "components/proximity_auth/cryptauth/cryptauth_access_token_fetcher.h" #include "components/proximity_auth/cryptauth/cryptauth_api_call_flow.h" #include "components/proximity_auth/cryptauth/cryptauth_client.h" +#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h" #include "net/url_request/url_request_context_getter.h" class OAuth2TokenService; @@ -55,6 +56,7 @@ class CryptAuthClientImpl : public CryptAuthClient { void FinishEnrollment(const cryptauth::FinishEnrollmentRequest& request, const FinishEnrollmentCallback& callback, const ErrorCallback& error_callback) override; + std::string GetAccessTokenUsed() override; private: // Starts a call to the API given by |request_path|, with the templated @@ -104,6 +106,9 @@ class CryptAuthClientImpl : public CryptAuthClient { // URL path of the current request. std::string request_path_; + // The access token fetched by |access_token_fetcher_|. + std::string access_token_used_; + // Called when the current request fails. ErrorCallback error_callback_; diff --git a/components/proximity_auth/cryptauth/cryptauth_client_impl_unittest.cc b/components/proximity_auth/cryptauth/cryptauth_client_impl_unittest.cc index 57ccb53..8012912 100644 --- a/components/proximity_auth/cryptauth/cryptauth_client_impl_unittest.cc +++ b/components/proximity_auth/cryptauth/cryptauth_client_impl_unittest.cc @@ -316,7 +316,7 @@ TEST_F(ProximityAuthCryptAuthClientTest, ToggleEasyUnlockSuccess) { TEST_F(ProximityAuthCryptAuthClientTest, SetupEnrollmentSuccess) { ExpectRequest( "https://www.testgoogleapis.com/cryptauth/v1/enrollment/" - "setupenrollment?alt=proto"); + "setup?alt=proto"); std::string kApplicationId = "mkaes"; std::vector<std::string> supported_protocols; @@ -362,7 +362,7 @@ TEST_F(ProximityAuthCryptAuthClientTest, SetupEnrollmentSuccess) { TEST_F(ProximityAuthCryptAuthClientTest, FinishEnrollmentSuccess) { ExpectRequest( "https://www.testgoogleapis.com/cryptauth/v1/enrollment/" - "finishenrollment?alt=proto"); + "finish?alt=proto"); const char kEnrollmentSessionId[] = "enrollment_session_id"; const char kEnrollmentMessage[] = "enrollment_message"; @@ -540,4 +540,20 @@ TEST_F(ProximityAuthCryptAuthClientTest, DeviceClassifierIsSet) { EXPECT_EQ(kDeviceType, device_classifier.device_type()); } +TEST_F(ProximityAuthCryptAuthClientTest, GetAccessTokenUsed) { + EXPECT_TRUE(client_->GetAccessTokenUsed().empty()); + ExpectRequest( + "https://www.testgoogleapis.com/cryptauth/v1/deviceSync/" + "getmydevices?alt=proto"); + + cryptauth::GetMyDevicesResponse result_proto; + cryptauth::GetMyDevicesRequest request_proto; + request_proto.set_allow_stale_read(true); + client_->GetMyDevices( + request_proto, + base::Bind(&SaveResult<cryptauth::GetMyDevicesResponse>, &result_proto), + base::Bind(&NotCalled<std::string>)); + EXPECT_EQ(kAccessToken, client_->GetAccessTokenUsed()); +} + } // namespace proximity_auth diff --git a/components/proximity_auth/cryptauth/cryptauth_enroller.h b/components/proximity_auth/cryptauth/cryptauth_enroller.h index 5aaf28748..88bfd29 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enroller.h +++ b/components/proximity_auth/cryptauth/cryptauth_enroller.h @@ -17,11 +17,18 @@ class CryptAuthEnroller { public: virtual ~CryptAuthEnroller() {} - // Enrolls the device with |device_info| properties for a given - // |invocation_reason|. |callback| will be called with true if the enrollment - // succeeds and false otherwise. + // Enrolls the device: + // |user_public_key|: The user's persistent public key identifying the device. + // |user_private_key|: The corresponding private key to |user_public_key|. + // |device_info|: Contains information about the local device. Note that the + // enroller may change fields in this proto before it is finally uploaded. + // |invocation_reason|: The reason why the enrollment occurred. + // |callback|: Called will be called with true if the enrollment + // succeeds and false otherwise. typedef base::Callback<void(bool)> EnrollmentFinishedCallback; - virtual void Enroll(const cryptauth::GcmDeviceInfo& device_info, + virtual void Enroll(const std::string& user_public_key, + const std::string& user_private_key, + const cryptauth::GcmDeviceInfo& device_info, cryptauth::InvocationReason invocation_reason, const EnrollmentFinishedCallback& callback) = 0; }; diff --git a/components/proximity_auth/cryptauth/cryptauth_enroller_impl.cc b/components/proximity_auth/cryptauth/cryptauth_enroller_impl.cc index 729648d..c595be1 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enroller_impl.cc +++ b/components/proximity_auth/cryptauth/cryptauth_enroller_impl.cc @@ -5,9 +5,13 @@ #include "components/proximity_auth/cryptauth/cryptauth_enroller_impl.h" #include "base/bind.h" -#include "components/proximity_auth/cryptauth/cryptauth_client.h" +#include "components/proximity_auth/cryptauth/base64url.h" +#include "components/proximity_auth/cryptauth/cryptauth_client_impl.h" #include "components/proximity_auth/cryptauth/cryptauth_enrollment_utils.h" +#include "components/proximity_auth/cryptauth/fake_secure_message_delegate.h" #include "components/proximity_auth/cryptauth/secure_message_delegate.h" +#include "components/proximity_auth/logging/logging.h" +#include "crypto/sha2.h" namespace proximity_auth { @@ -15,7 +19,7 @@ namespace { // A successful SetupEnrollment or FinishEnrollment response should contain this // status string. -const char kResponseStatusOk[] = "OK"; +const char kResponseStatusOk[] = "ok"; // The name of the "gcmV1" protocol that the enrolling device supports. const char kSupportedEnrollmentTypeGcmV1[] = "gcmV1"; @@ -25,23 +29,13 @@ const int kGCMMetadataVersion = 1; // Returns true if |device_info| contains the required fields for enrollment. bool ValidateDeviceInfo(const cryptauth::GcmDeviceInfo& device_info) { - if (!device_info.has_user_public_key()) { - VLOG(1) << "Expected user_public_key field in GcmDeviceInfo."; - return false; - } - - if (!device_info.has_key_handle()) { - VLOG(1) << "Expected key_handle field in GcmDeviceInfo."; - return false; - } - if (!device_info.has_long_device_id()) { - VLOG(1) << "Expected long_device_id field in GcmDeviceInfo."; + PA_LOG(ERROR) << "Expected long_device_id field in GcmDeviceInfo."; return false; } if (!device_info.has_device_type()) { - VLOG(1) << "Expected device_type field in GcmDeviceInfo."; + PA_LOG(ERROR) << "Expected device_type field in GcmDeviceInfo."; return false; } @@ -61,9 +55,9 @@ std::string CreateEnrollmentPublicMetadata() { CryptAuthEnrollerImpl::CryptAuthEnrollerImpl( scoped_ptr<CryptAuthClientFactory> client_factory, - scoped_ptr<SecureMessageDelegate> secure_message_delegate_) + scoped_ptr<SecureMessageDelegate> secure_message_delegate) : client_factory_(client_factory.Pass()), - secure_message_delegate_(secure_message_delegate_.Pass()), + secure_message_delegate_(secure_message_delegate.Pass()), weak_ptr_factory_(this) { } @@ -71,15 +65,19 @@ CryptAuthEnrollerImpl::~CryptAuthEnrollerImpl() { } void CryptAuthEnrollerImpl::Enroll( + const std::string& user_public_key, + const std::string& user_private_key, const cryptauth::GcmDeviceInfo& device_info, cryptauth::InvocationReason invocation_reason, const EnrollmentFinishedCallback& callback) { if (!callback_.is_null()) { - VLOG(1) << "Enroll() already called. Do not reuse."; + PA_LOG(ERROR) << "Enroll() already called. Do not reuse."; callback.Run(false); return; } + user_public_key_ = user_public_key; + user_private_key_ = user_private_key; device_info_ = device_info; invocation_reason_ = invocation_reason; callback_ = callback; @@ -96,6 +94,7 @@ void CryptAuthEnrollerImpl::Enroll( void CryptAuthEnrollerImpl::OnKeyPairGenerated(const std::string& public_key, const std::string& private_key) { + PA_LOG(INFO) << "Ephemeral key pair generated, calling SetupEnrollment API."; session_public_key_ = public_key; session_private_key_ = private_key; @@ -113,19 +112,22 @@ void CryptAuthEnrollerImpl::OnKeyPairGenerated(const std::string& public_key, void CryptAuthEnrollerImpl::OnSetupEnrollmentSuccess( const cryptauth::SetupEnrollmentResponse& response) { if (response.status() != kResponseStatusOk) { - VLOG(1) << "Unexpected status for SetupEnrollment: " << response.status(); + PA_LOG(WARNING) << "Unexpected status for SetupEnrollment: " + << response.status(); callback_.Run(false); return; } if (response.infos_size() == 0) { - VLOG(1) << "No response info returned by server for SetupEnrollment"; + PA_LOG(ERROR) << "No response info returned by server for SetupEnrollment"; callback_.Run(false); return; } + PA_LOG(INFO) << "SetupEnrollment request succeeded: deriving symmetric key."; setup_info_ = response.infos(0); device_info_.set_enrollment_session_id(setup_info_.enrollment_session_id()); + secure_message_delegate_->DeriveKey( session_private_key_, setup_info_.server_ephemeral_key(), base::Bind(&CryptAuthEnrollerImpl::OnKeyDerived, @@ -133,27 +135,50 @@ void CryptAuthEnrollerImpl::OnSetupEnrollmentSuccess( } void CryptAuthEnrollerImpl::OnSetupEnrollmentFailure(const std::string& error) { - VLOG(1) << "SetupEnrollment API failed with error: " << error; + PA_LOG(WARNING) << "SetupEnrollment API failed with error: " << error; callback_.Run(false); } void CryptAuthEnrollerImpl::OnKeyDerived(const std::string& symmetric_key) { + PA_LOG(INFO) << "Derived symmetric key, " + << "encrypting enrollment data for upload."; + + // Make sure we're enrolling the same public key used below to sign the + // secure message. + device_info_.set_user_public_key(user_public_key_); + device_info_.set_key_handle(user_public_key_); + + // Hash the symmetric key and add it to the |device_info_| to be uploaded. + device_info_.set_device_master_key_hash( + crypto::SHA256HashString(symmetric_key)); + + // The server verifies that the access token set here and in the header + // of the FinishEnrollment() request are the same. + device_info_.set_oauth_token(cryptauth_client_->GetAccessTokenUsed()); + PA_LOG(INFO) << "Using access token: " << device_info_.oauth_token(); + symmetric_key_ = symmetric_key; SecureMessageDelegate::CreateOptions options; options.encryption_scheme = securemessage::NONE; options.signature_scheme = securemessage::ECDSA_P256_SHA256; - options.verification_key_id = session_public_key_; + options.verification_key_id = user_public_key_; // The inner message contains the signed device information that will be // sent to CryptAuth. secure_message_delegate_->CreateSecureMessage( - device_info_.SerializeAsString(), session_private_key_, options, + device_info_.SerializeAsString(), user_private_key_, options, base::Bind(&CryptAuthEnrollerImpl::OnInnerSecureMessageCreated, weak_ptr_factory_.GetWeakPtr())); } void CryptAuthEnrollerImpl::OnInnerSecureMessageCreated( const std::string& inner_message) { + if (inner_message.empty()) { + PA_LOG(ERROR) << "Error creating inner message"; + callback_.Run(false); + return; + } + SecureMessageDelegate::CreateOptions options; options.encryption_scheme = securemessage::AES_256_CBC; options.signature_scheme = securemessage::HMAC_SHA256; @@ -169,6 +194,8 @@ void CryptAuthEnrollerImpl::OnInnerSecureMessageCreated( void CryptAuthEnrollerImpl::OnOuterSecureMessageCreated( const std::string& outer_message) { + PA_LOG(INFO) << "SecureMessage created, calling FinishEnrollment API."; + cryptauth::FinishEnrollmentRequest request; request.set_enrollment_session_id(setup_info_.enrollment_session_id()); request.set_enrollment_message(outer_message); @@ -186,7 +213,8 @@ void CryptAuthEnrollerImpl::OnOuterSecureMessageCreated( void CryptAuthEnrollerImpl::OnFinishEnrollmentSuccess( const cryptauth::FinishEnrollmentResponse& response) { if (response.status() != kResponseStatusOk) { - VLOG(1) << "Unexpected status for FinishEnrollment: " << response.status(); + PA_LOG(WARNING) << "Unexpected status for FinishEnrollment: " + << response.status(); callback_.Run(false); } else { callback_.Run(true); @@ -195,7 +223,7 @@ void CryptAuthEnrollerImpl::OnFinishEnrollmentSuccess( void CryptAuthEnrollerImpl::OnFinishEnrollmentFailure( const std::string& error) { - VLOG(1) << "FinishEnrollment API failed with error: " << error; + PA_LOG(WARNING) << "FinishEnrollment API failed with error: " << error; callback_.Run(false); } diff --git a/components/proximity_auth/cryptauth/cryptauth_enroller_impl.h b/components/proximity_auth/cryptauth/cryptauth_enroller_impl.h index 51a0b2f..d34bf30 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enroller_impl.h +++ b/components/proximity_auth/cryptauth/cryptauth_enroller_impl.h @@ -10,11 +10,13 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "components/proximity_auth/cryptauth/cryptauth_enroller.h" +#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h" namespace proximity_auth { class CryptAuthClient; class CryptAuthClientFactory; +class CryptAuthClientFactoryImpl; class SecureMessageDelegate; // Implementation of CryptAuthEnroller to perform enrollment in two steps: @@ -31,11 +33,13 @@ class CryptAuthEnrollerImpl : public CryptAuthEnroller { // |crypto_delegate| is responsible for SecureMessage operations. CryptAuthEnrollerImpl( scoped_ptr<CryptAuthClientFactory> client_factory, - scoped_ptr<SecureMessageDelegate> secure_message_delegate_); + scoped_ptr<SecureMessageDelegate> secure_message_delegate); ~CryptAuthEnrollerImpl() override; // CryptAuthEnroller: - void Enroll(const cryptauth::GcmDeviceInfo& device_info, + void Enroll(const std::string& user_public_key, + const std::string& user_private_key, + const cryptauth::GcmDeviceInfo& device_info, cryptauth::InvocationReason invocation_reason, const EnrollmentFinishedCallback& callback) override; @@ -70,6 +74,10 @@ class CryptAuthEnrollerImpl : public CryptAuthEnroller { std::string session_public_key_; std::string session_private_key_; + // The user's persistent key-pair identifying the local device. + std::string user_public_key_; + std::string user_private_key_; + // Contains information of the device to enroll. cryptauth::GcmDeviceInfo device_info_; diff --git a/components/proximity_auth/cryptauth/cryptauth_enroller_impl_unittest.cc b/components/proximity_auth/cryptauth/cryptauth_enroller_impl_unittest.cc index bee79e4..9ab0dfb 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enroller_impl_unittest.cc +++ b/components/proximity_auth/cryptauth/cryptauth_enroller_impl_unittest.cc @@ -11,20 +11,22 @@ #include "testing/gtest/include/gtest/gtest.h" using ::testing::_; +using ::testing::Return; namespace proximity_auth { namespace { +const char kAccessTokenUsed[] = "access token used by CryptAuthClient"; + const char kClientSessionPublicKey[] = "throw away after one use"; const char kServerSessionPublicKey[] = "disposables are not eco-friendly"; -const char kClientPersistentPublicKey[] = "saves 50 trees a year"; cryptauth::InvocationReason kInvocationReason = cryptauth::INVOCATION_REASON_MANUAL; const int kGCMMetadataVersion = 1; const char kSupportedEnrollmentTypeGcmV1[] = "gcmV1"; -const char kResponseStatusOk[] = "OK"; +const char kResponseStatusOk[] = "ok"; const char kResponseStatusNotOk[] = "Your key was too bland."; const char kEnrollmentSessionId[] = "0123456789876543210"; const char kFinishEnrollmentError[] = "A hungry router ate all your packets."; @@ -39,8 +41,6 @@ cryptauth::GcmDeviceInfo GetDeviceInfo() { device_info.set_long_device_id(kDeviceId); device_info.set_device_type(kDeviceType); device_info.set_device_os_version(kDeviceOsVersion); - device_info.set_user_public_key(kClientPersistentPublicKey); - device_info.set_key_handle(kClientPersistentPublicKey); return device_info; } @@ -106,6 +106,11 @@ class ProximityAuthCryptAuthEnrollerTest enroller_(make_scoped_ptr(client_factory_), make_scoped_ptr(secure_message_delegate_)) { client_factory_->AddObserver(this); + + // This call is actually synchronous. + secure_message_delegate_->GenerateKeyPair( + base::Bind(&ProximityAuthCryptAuthEnrollerTest::OnKeyPairGenerated, + base::Unretained(this))); } // Starts the enroller. @@ -113,7 +118,7 @@ class ProximityAuthCryptAuthEnrollerTest secure_message_delegate_->set_next_public_key(kClientSessionPublicKey); enroller_result_.reset(); enroller_.Enroll( - device_info, kInvocationReason, + user_public_key_, user_private_key_, device_info, kInvocationReason, base::Bind(&ProximityAuthCryptAuthEnrollerTest::OnEnrollerCompleted, base::Unretained(this))); } @@ -158,10 +163,10 @@ class ProximityAuthCryptAuthEnrollerTest unwrap_options.encryption_scheme = securemessage::NONE; unwrap_options.signature_scheme = securemessage::ECDSA_P256_SHA256; secure_message_delegate_->UnwrapSecureMessage( - inner_message, kClientSessionPublicKey, unwrap_options, + inner_message, user_public_key_, unwrap_options, base::Bind(&SaveUnwrapResults, &verified, &inner_payload, &header)); EXPECT_TRUE(verified); - EXPECT_EQ(kClientSessionPublicKey, header.verification_key_id()); + EXPECT_EQ(user_public_key_, header.verification_key_id()); } // Check that the decrypted GcmDeviceInfo is correct. @@ -170,8 +175,8 @@ class ProximityAuthCryptAuthEnrollerTest EXPECT_EQ(kDeviceId, device_info.long_device_id()); EXPECT_EQ(kDeviceType, device_info.device_type()); EXPECT_EQ(kDeviceOsVersion, device_info.device_os_version()); - EXPECT_EQ(kClientPersistentPublicKey, device_info.user_public_key()); - EXPECT_EQ(kClientPersistentPublicKey, device_info.key_handle()); + EXPECT_EQ(user_public_key_, device_info.user_public_key()); + EXPECT_EQ(user_public_key_, device_info.key_handle()); EXPECT_EQ(kEnrollmentSessionId, device_info.enrollment_session_id()); } @@ -185,6 +190,15 @@ class ProximityAuthCryptAuthEnrollerTest ON_CALL(*client, FinishEnrollment(_, _, _)) .WillByDefault(Invoke( this, &ProximityAuthCryptAuthEnrollerTest::OnFinishEnrollment)); + + ON_CALL(*client, GetAccessTokenUsed()) + .WillByDefault(Return(kAccessTokenUsed)); + } + + void OnKeyPairGenerated(const std::string& public_key, + const std::string& private_key) { + user_public_key_ = public_key; + user_private_key_ = private_key; } void OnEnrollerCompleted(bool success) { @@ -221,6 +235,10 @@ class ProximityAuthCryptAuthEnrollerTest error_callback_ = error_callback; } + // The persistent user key-pair. + std::string user_public_key_; + std::string user_private_key_; + // Owned by |enroller_|. MockCryptAuthClientFactory* client_factory_; // Owned by |enroller_|. diff --git a/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.cc b/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.cc index e3179ac..2b87229 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.cc +++ b/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.cc @@ -6,8 +6,13 @@ #include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" +#include "base/time/clock.h" +#include "base/time/time.h" +#include "components/proximity_auth/cryptauth/cryptauth_enroller.h" #include "components/proximity_auth/cryptauth/pref_names.h" +#include "components/proximity_auth/cryptauth/secure_message_delegate.h" #include "components/proximity_auth/cryptauth/sync_scheduler_impl.h" +#include "components/proximity_auth/logging/logging.h" namespace proximity_auth { @@ -33,11 +38,16 @@ const double kEnrollmentMaxJitterRatio = 0.2; CryptAuthEnrollmentManager::CryptAuthEnrollmentManager( scoped_ptr<base::Clock> clock, scoped_ptr<CryptAuthEnrollerFactory> enroller_factory, - const cryptauth::GcmDeviceInfo& device_info) + const std::string& user_public_key, + const std::string& user_private_key, + const cryptauth::GcmDeviceInfo& device_info, + PrefService* pref_service) : clock_(clock.Pass()), enroller_factory_(enroller_factory.Pass()), + user_public_key_(user_public_key), + user_private_key_(user_private_key), device_info_(device_info), - pref_service_(nullptr), + pref_service_(pref_service), weak_ptr_factory_(this) { } @@ -54,11 +64,9 @@ void CryptAuthEnrollmentManager::RegisterPrefs(PrefRegistrySimple* registry) { cryptauth::INVOCATION_REASON_UNKNOWN); } -void CryptAuthEnrollmentManager::Start(PrefService* pref_service) { - pref_service_ = pref_service; - +void CryptAuthEnrollmentManager::Start() { bool is_recovering_from_failure = - pref_service->GetBoolean( + pref_service_->GetBoolean( prefs::kCryptAuthEnrollmentIsRecoveringFromFailure) || !IsEnrollmentValid(); @@ -171,8 +179,9 @@ void CryptAuthEnrollmentManager::OnSyncRequested( invocation_reason = cryptauth::INVOCATION_REASON_FAILURE_RECOVERY; } + PA_LOG(INFO) << "Making enrollment with reason: " << invocation_reason; cryptauth_enroller_->Enroll( - device_info_, invocation_reason, + user_public_key_, user_private_key_, device_info_, invocation_reason, base::Bind(&CryptAuthEnrollmentManager::OnEnrollmentFinished, weak_ptr_factory_.GetWeakPtr())); } diff --git a/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.h b/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.h index 5871a9a..f5558bc 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.h +++ b/components/proximity_auth/cryptauth/cryptauth_enrollment_manager.h @@ -8,18 +8,23 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" -#include "base/time/clock.h" #include "base/time/time.h" -#include "components/proximity_auth/cryptauth/cryptauth_client.h" -#include "components/proximity_auth/cryptauth/cryptauth_enroller.h" -#include "components/proximity_auth/cryptauth/secure_message_delegate.h" +#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h" #include "components/proximity_auth/cryptauth/sync_scheduler.h" class PrefRegistrySimple; class PrefService; +namespace base { +class Clock; +class Time; +} + namespace proximity_auth { +class CryptAuthEnroller; +class CryptAuthEnrollerFactory; + // This class manages the device's enrollment with CryptAuth, periodically // re-enrolling to keep the state on the server fresh. If an enrollment fails, // the manager will schedule the next enrollment more aggressively to recover @@ -42,22 +47,27 @@ class CryptAuthEnrollmentManager : public SyncScheduler::Delegate { // |clock|: Used to determine the time between sync attempts. // |enroller_factory|: Creates CryptAuthEnroller instances to perform each // enrollment attempt. + // |user_public_key|: The user's persistent public key identifying the device. + // |user_private_key|: The corresponding private key to |user_public_key|. // |device_info|: Contains information about the local device that will be // uploaded to CryptAuth with each enrollment request. + // |pref_service|: Contains preferences across browser restarts, and should + // have been registered through RegisterPrefs(). CryptAuthEnrollmentManager( scoped_ptr<base::Clock> clock, scoped_ptr<CryptAuthEnrollerFactory> enroller_factory, - const cryptauth::GcmDeviceInfo& device_info); + const std::string& user_public_key, + const std::string& user_private_key, + const cryptauth::GcmDeviceInfo& device_info, + PrefService* pref_service); ~CryptAuthEnrollmentManager() override; // Registers the prefs used by this class to the given |pref_service|. static void RegisterPrefs(PrefRegistrySimple* registry); - // Starts the manager backed by a |pref_service|, which shall already - // have registered the prefs through RegisterPrefs(). - // This function begins scheduling periodic enrollment attempts. - void Start(PrefService* pref_service); + // Begins scheduling periodic enrollment attempts. + void Start(); // Adds an observer. void AddObserver(Observer* observer); @@ -108,6 +118,10 @@ class CryptAuthEnrollmentManager : public SyncScheduler::Delegate { // Creates CryptAuthEnroller instances for each enrollment attempt. scoped_ptr<CryptAuthEnrollerFactory> enroller_factory_; + // The user's persistent key-pair identifying the local device. + std::string user_public_key_; + std::string user_private_key_; + // The local device information to upload to CryptAuth. const cryptauth::GcmDeviceInfo device_info_; diff --git a/components/proximity_auth/cryptauth/cryptauth_enrollment_manager_unittest.cc b/components/proximity_auth/cryptauth/cryptauth_enrollment_manager_unittest.cc index a4f0b10..b387ea6 100644 --- a/components/proximity_auth/cryptauth/cryptauth_enrollment_manager_unittest.cc +++ b/components/proximity_auth/cryptauth/cryptauth_enrollment_manager_unittest.cc @@ -7,6 +7,9 @@ #include "base/memory/weak_ptr.h" #include "base/prefs/testing_pref_service.h" #include "base/test/simple_test_clock.h" +#include "base/time/clock.h" +#include "base/time/time.h" +#include "components/proximity_auth/cryptauth/cryptauth_enroller.h" #include "components/proximity_auth/cryptauth/mock_sync_scheduler.h" #include "components/proximity_auth/cryptauth/pref_names.h" #include "testing/gmock/include/gmock/gmock.h" @@ -21,6 +24,10 @@ namespace proximity_auth { namespace { +// The user's persistent key pair identifying the local device. +const char kUserPublicKey[] = "user public key"; +const char kUserPrivateKey[] = "user private key"; + // The initial "Now" time for testing. const double kInitialTimeNowSeconds = 20000000; @@ -41,8 +48,10 @@ class MockCryptAuthEnroller : public CryptAuthEnroller { MockCryptAuthEnroller() {} ~MockCryptAuthEnroller() override {} - MOCK_METHOD3(Enroll, - void(const cryptauth::GcmDeviceInfo& device_info, + MOCK_METHOD5(Enroll, + void(const std::string& user_public_key, + const std::string& user_private_key, + const cryptauth::GcmDeviceInfo& device_info, cryptauth::InvocationReason invocation_reason, const EnrollmentFinishedCallback& callback)); @@ -83,10 +92,14 @@ class TestCryptAuthEnrollmentManager : public CryptAuthEnrollmentManager { TestCryptAuthEnrollmentManager( scoped_ptr<base::Clock> clock, scoped_ptr<CryptAuthEnrollerFactory> enroller_factory, - const cryptauth::GcmDeviceInfo& device_info) + const cryptauth::GcmDeviceInfo& device_info, + PrefService* pref_service) : CryptAuthEnrollmentManager(clock.Pass(), enroller_factory.Pass(), - device_info), + kUserPublicKey, + kUserPrivateKey, + device_info, + pref_service), scoped_sync_scheduler_(new NiceMock<MockSyncScheduler>()), weak_sync_scheduler_factory_(scoped_sync_scheduler_.get()) {} @@ -126,7 +139,8 @@ class ProximityAuthCryptAuthEnrollmentManagerTest enroller_factory_(new MockCryptAuthEnrollerFactory()), enrollment_manager_(make_scoped_ptr(clock_), make_scoped_ptr(enroller_factory_), - device_info_) {} + device_info_, + &pref_service_) {} // testing::Test: void SetUp() override { @@ -171,8 +185,9 @@ class ProximityAuthCryptAuthEnrollmentManagerTest cryptauth::InvocationReason expected_invocation_reason) { CryptAuthEnroller::EnrollmentFinishedCallback completion_callback; EXPECT_CALL(*next_cryptauth_enroller(), - Enroll(_, expected_invocation_reason, _)) - .WillOnce(SaveArg<2>(&completion_callback)); + Enroll(kUserPublicKey, kUserPrivateKey, _, + expected_invocation_reason, _)) + .WillOnce(SaveArg<4>(&completion_callback)); auto sync_request = make_scoped_ptr( new SyncScheduler::SyncRequest(enrollment_manager_.GetSyncScheduler())); @@ -219,7 +234,7 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, RegisterPrefs) { } TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, GetEnrollmentState) { - enrollment_manager_.Start(&pref_service_); + enrollment_manager_.Start(); ON_CALL(*sync_scheduler(), GetStrategy()) .WillByDefault(Return(SyncScheduler::Strategy::PERIODIC_REFRESH)); @@ -244,15 +259,24 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, GetEnrollmentState) { } TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, InitWithDefaultPrefs) { - EXPECT_CALL(*sync_scheduler(), - Start(clock_->Now() - base::Time::FromDoubleT(0), - SyncScheduler::Strategy::AGGRESSIVE_RECOVERY)); + scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock()); + clock->SetNow(base::Time::FromDoubleT(kInitialTimeNowSeconds)); + base::TimeDelta elapsed_time = clock->Now() - base::Time::FromDoubleT(0); TestingPrefServiceSimple pref_service; CryptAuthEnrollmentManager::RegisterPrefs(pref_service.registry()); - enrollment_manager_.Start(&pref_service); - EXPECT_FALSE(enrollment_manager_.IsEnrollmentValid()); - EXPECT_TRUE(enrollment_manager_.GetLastEnrollmentTime().is_null()); + + TestCryptAuthEnrollmentManager enrollment_manager( + clock.Pass(), make_scoped_ptr(new MockCryptAuthEnrollerFactory()), + device_info_, &pref_service); + + EXPECT_CALL( + *enrollment_manager.GetSyncScheduler(), + Start(elapsed_time, SyncScheduler::Strategy::AGGRESSIVE_RECOVERY)); + enrollment_manager.Start(); + + EXPECT_FALSE(enrollment_manager.IsEnrollmentValid()); + EXPECT_TRUE(enrollment_manager.GetLastEnrollmentTime().is_null()); } TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, InitWithExistingPrefs) { @@ -261,7 +285,7 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, InitWithExistingPrefs) { Start(clock_->Now() - base::Time::FromDoubleT(kLastEnrollmentTimeSeconds), SyncScheduler::Strategy::PERIODIC_REFRESH)); - enrollment_manager_.Start(&pref_service_); + enrollment_manager_.Start(); EXPECT_TRUE(enrollment_manager_.IsEnrollmentValid()); EXPECT_EQ(base::Time::FromDoubleT(kLastEnrollmentTimeSeconds), enrollment_manager_.GetLastEnrollmentTime()); @@ -277,7 +301,7 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, InitWithExpiredEnrollment) { kLastExpiredEnrollmentTimeSeconds), SyncScheduler::Strategy::AGGRESSIVE_RECOVERY)); - enrollment_manager_.Start(&pref_service_); + enrollment_manager_.Start(); EXPECT_FALSE(enrollment_manager_.IsEnrollmentValid()); EXPECT_EQ(base::Time::FromDoubleT(kLastExpiredEnrollmentTimeSeconds), enrollment_manager_.GetLastEnrollmentTime()); @@ -286,7 +310,7 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, InitWithExpiredEnrollment) { TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, EnrollmentSucceedsForFirstTime) { pref_service_.ClearPref(prefs::kCryptAuthEnrollmentLastEnrollmentTimeSeconds); - enrollment_manager_.Start(&pref_service_); + enrollment_manager_.Start(); EXPECT_FALSE(enrollment_manager_.IsEnrollmentValid()); auto completion_callback = @@ -302,7 +326,7 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, } TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, ForceEnrollment) { - enrollment_manager_.Start(&pref_service_); + enrollment_manager_.Start(); EXPECT_CALL(*sync_scheduler(), ForceSync()); enrollment_manager_.ForceEnrollmentNow(cryptauth::INVOCATION_REASON_MANUAL); @@ -318,7 +342,7 @@ TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, ForceEnrollment) { TEST_F(ProximityAuthCryptAuthEnrollmentManagerTest, EnrollmentFailsThenSucceeds) { - enrollment_manager_.Start(&pref_service_); + enrollment_manager_.Start(); base::Time old_enrollment_time = enrollment_manager_.GetLastEnrollmentTime(); // The first periodic enrollment fails. diff --git a/components/proximity_auth/cryptauth/mock_cryptauth_client.cc b/components/proximity_auth/cryptauth/mock_cryptauth_client.cc index 099082f..5566f9e 100644 --- a/components/proximity_auth/cryptauth/mock_cryptauth_client.cc +++ b/components/proximity_auth/cryptauth/mock_cryptauth_client.cc @@ -4,6 +4,7 @@ #include "base/callback.h" #include "components/proximity_auth/cryptauth/mock_cryptauth_client.h" +#include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h" namespace proximity_auth { diff --git a/components/proximity_auth/cryptauth/mock_cryptauth_client.h b/components/proximity_auth/cryptauth/mock_cryptauth_client.h index f137c00..adf2129 100644 --- a/components/proximity_auth/cryptauth/mock_cryptauth_client.h +++ b/components/proximity_auth/cryptauth/mock_cryptauth_client.h @@ -42,6 +42,7 @@ class MockCryptAuthClient : public CryptAuthClient { void(const cryptauth::FinishEnrollmentRequest& request, const FinishEnrollmentCallback& callback, const ErrorCallback& error_callback)); + MOCK_METHOD0(GetAccessTokenUsed, std::string()); private: DISALLOW_COPY_AND_ASSIGN(MockCryptAuthClient); |