From 06b22125e49427649527af411c27519c070d1871 Mon Sep 17 00:00:00 2001 From: Austin Tankiang Date: Wed, 26 Aug 2020 08:25:43 +0000 Subject: [PATCH] Convert DeviceToDeviceResponderOperations to BindOnce/OnceCallback Extract out the callbacks from CreateResponderAuthMessageContext and ValidateInitiatorAuthMessageContext structs to avoid making those structs move only. Bug: 1007662 Change-Id: I8ae410e796c21ff0d137eae3e0bab486b286ae1d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2374928 Reviewed-by: Kyle Horimoto Commit-Queue: Austin Tankiang Cr-Commit-Position: refs/heads/master@{#801683} --- .../device_to_device_operations_unittest.cc | 40 +++++------ .../device_to_device_responder_operations.cc | 72 +++++++++++-------- .../device_to_device_responder_operations.h | 13 ++-- 3 files changed, 70 insertions(+), 55 deletions(-) diff --git a/chromeos/services/secure_channel/device_to_device_operations_unittest.cc b/chromeos/services/secure_channel/device_to_device_operations_unittest.cc index 0f987f73e6901..ba1d9650541fa 100644 --- a/chromeos/services/secure_channel/device_to_device_operations_unittest.cc +++ b/chromeos/services/secure_channel/device_to_device_operations_unittest.cc @@ -92,7 +92,7 @@ class SecureChannelDeviceToDeviceOperationsTest : public testing::Test { // Note: FakeSecureMessageDelegate functions are synchronous. secure_message_delegate_.DeriveKey( local_session_private_key_, remote_session_public_key_, - base::Bind(&SaveMessageResult, &session_symmetric_key_)); + base::BindOnce(&SaveMessageResult, &session_symmetric_key_)); session_keys_ = SessionKeys(session_symmetric_key_); persistent_symmetric_key_ = "persistent symmetric key"; @@ -103,10 +103,10 @@ class SecureChannelDeviceToDeviceOperationsTest : public testing::Test { // Creates the initator's [Hello] message. std::string CreateHelloMessage() { std::string hello_message; - helper_->CreateHelloMessage(local_session_public_key_, - persistent_symmetric_key_, - &secure_message_delegate_, - base::Bind(&SaveMessageResult, &hello_message)); + helper_->CreateHelloMessage( + local_session_public_key_, persistent_symmetric_key_, + &secure_message_delegate_, + base::BindOnce(&SaveMessageResult, &hello_message)); EXPECT_FALSE(hello_message.empty()); return hello_message; } @@ -122,7 +122,7 @@ class SecureChannelDeviceToDeviceOperationsTest : public testing::Test { hello_message, remote_session_public_key_, remote_session_private_key_, persistent_responder_private_key, persistent_symmetric_key_, &secure_message_delegate_, - base::Bind(&SaveMessageResult, &remote_auth_message)); + base::BindOnce(&SaveMessageResult, &remote_auth_message)); EXPECT_FALSE(remote_auth_message.empty()); return remote_auth_message; } @@ -134,7 +134,7 @@ class SecureChannelDeviceToDeviceOperationsTest : public testing::Test { helper_->CreateInitiatorAuthMessage( session_keys_, persistent_symmetric_key_, remote_auth_message, &secure_message_delegate_, - base::Bind(&SaveMessageResult, &local_auth_message)); + base::BindOnce(&SaveMessageResult, &local_auth_message)); EXPECT_FALSE(local_auth_message.empty()); return local_auth_message; } @@ -161,8 +161,8 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, DeviceToDeviceResponderOperations::ValidateHelloMessage( CreateHelloMessage(), persistent_symmetric_key_, &secure_message_delegate_, - base::Bind(&SaveValidationResultWithKey, &validation_success, - &hello_public_key)); + base::BindOnce(&SaveValidationResultWithKey, &validation_success, + &hello_public_key)); EXPECT_TRUE(validation_success); EXPECT_EQ(local_session_public_key_, hello_public_key); @@ -175,8 +175,8 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, DeviceToDeviceResponderOperations::ValidateHelloMessage( "some random string", persistent_symmetric_key_, &secure_message_delegate_, - base::Bind(&SaveValidationResultWithKey, &validation_success, - &hello_public_key)); + base::BindOnce(&SaveValidationResultWithKey, &validation_success, + &hello_public_key)); EXPECT_FALSE(validation_success); EXPECT_TRUE(hello_public_key.empty()); @@ -193,8 +193,8 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, remote_auth_message, kResponderPersistentPublicKey, persistent_symmetric_key_, local_session_private_key_, hello_message, &secure_message_delegate_, - base::Bind(&SaveValidationResultWithSessionKeys, &validation_success, - &session_keys)); + base::BindOnce(&SaveValidationResultWithSessionKeys, &validation_success, + &session_keys)); EXPECT_TRUE(validation_success); EXPECT_EQ(session_keys_.initiator_encode_key(), @@ -214,8 +214,8 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, remote_auth_message, kResponderPersistentPublicKey, persistent_symmetric_key_, local_session_private_key_, "invalid hello message", &secure_message_delegate_, - base::Bind(&SaveValidationResultWithSessionKeys, &validation_success, - &session_keys)); + base::BindOnce(&SaveValidationResultWithSessionKeys, &validation_success, + &session_keys)); EXPECT_FALSE(validation_success); EXPECT_TRUE(session_keys.initiator_encode_key().empty()); @@ -233,8 +233,8 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, remote_auth_message, kResponderPersistentPublicKey, "invalid persistent symmetric key", local_session_private_key_, hello_message, &secure_message_delegate_, - base::Bind(&SaveValidationResultWithSessionKeys, &validation_success, - &session_keys)); + base::BindOnce(&SaveValidationResultWithSessionKeys, &validation_success, + &session_keys)); EXPECT_FALSE(validation_success); EXPECT_TRUE(session_keys.initiator_encode_key().empty()); @@ -252,7 +252,7 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, DeviceToDeviceResponderOperations::ValidateInitiatorAuthMessage( local_auth_message, session_keys_, persistent_symmetric_key_, remote_auth_message, &secure_message_delegate_, - base::Bind(&SaveValidationResult, &validation_success)); + base::BindOnce(&SaveValidationResult, &validation_success)); EXPECT_TRUE(validation_success); } @@ -268,7 +268,7 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, DeviceToDeviceResponderOperations::ValidateInitiatorAuthMessage( local_auth_message, session_keys_, persistent_symmetric_key_, "invalid remote auth", &secure_message_delegate_, - base::Bind(&SaveValidationResult, &validation_success)); + base::BindOnce(&SaveValidationResult, &validation_success)); EXPECT_FALSE(validation_success); } @@ -284,7 +284,7 @@ TEST_F(SecureChannelDeviceToDeviceOperationsTest, DeviceToDeviceResponderOperations::ValidateInitiatorAuthMessage( local_auth_message, session_keys_, "invalid persistent symmetric key", remote_auth_message, &secure_message_delegate_, - base::Bind(&SaveValidationResult, &validation_success)); + base::BindOnce(&SaveValidationResult, &validation_success)); EXPECT_FALSE(validation_success); } diff --git a/chromeos/services/secure_channel/device_to_device_responder_operations.cc b/chromeos/services/secure_channel/device_to_device_responder_operations.cc index fcead3d6806c7..0a33030e80ac6 100644 --- a/chromeos/services/secure_channel/device_to_device_responder_operations.cc +++ b/chromeos/services/secure_channel/device_to_device_responder_operations.cc @@ -33,18 +33,19 @@ const int kD2DProtocolVersion = 1; // Callback for DeviceToDeviceResponderOperations::ValidateHelloMessage(), // after the [Hello] message is unwrapped. void OnHelloMessageUnwrapped( - const DeviceToDeviceResponderOperations::ValidateHelloCallback& callback, + DeviceToDeviceResponderOperations::ValidateHelloCallback callback, bool verified, const std::string& payload, const securemessage::Header& header) { securegcm::InitiatorHello initiator_hello; if (!verified || !initiator_hello.ParseFromString(header.public_metadata()) || initiator_hello.protocol_version() != kD2DProtocolVersion) { - callback.Run(false, std::string()); + std::move(callback).Run(false, std::string()); return; } - callback.Run(true, initiator_hello.public_dh_key().SerializeAsString()); + std::move(callback).Run(true, + initiator_hello.public_dh_key().SerializeAsString()); } // Helper struct containing all the context needed to create the [Responder @@ -56,7 +57,6 @@ struct CreateResponderAuthMessageContext { std::string persistent_private_key; std::string persistent_symmetric_key; multidevice::SecureMessageDelegate* secure_message_delegate; - DeviceToDeviceResponderOperations::MessageCallback callback; std::string hello_public_key; std::string middle_message; }; @@ -65,26 +65,31 @@ struct CreateResponderAuthMessageContext { // message, declared in order in which they are called during the creation flow. void OnHelloMessageValidatedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, bool hello_message_validated, const std::string& hello_public_key); void OnInnerMessageCreatedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, const std::string& inner_message); void OnMiddleMessageCreatedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, const std::string& middle_message); void OnSessionSymmetricKeyDerivedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, const std::string& session_symmetric_key); // Called after the initiator's [Hello] message is unwrapped. void OnHelloMessageValidatedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, bool hello_message_validated, const std::string& hello_public_key) { if (!hello_message_validated) { PA_LOG(VERBOSE) << "Invalid [Hello] while creating [Responder Auth]"; - context.callback.Run(std::string()); + std::move(callback).Run(std::string()); return; } @@ -103,16 +108,18 @@ void OnHelloMessageValidatedForResponderAuth( context.secure_message_delegate->CreateSecureMessage( kPayloadFiller, context.persistent_private_key, create_options, - base::Bind(&OnInnerMessageCreatedForResponderAuth, context)); + base::BindOnce(&OnInnerMessageCreatedForResponderAuth, context, + std::move(callback))); } // Called after the inner-most layer of [Responder Auth] is created. void OnInnerMessageCreatedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, const std::string& inner_message) { if (inner_message.empty()) { PA_LOG(VERBOSE) << "Failed to create middle message for [Responder Auth]"; - context.callback.Run(std::string()); + std::move(callback).Run(std::string()); return; } @@ -123,16 +130,18 @@ void OnInnerMessageCreatedForResponderAuth( create_options.associated_data = context.hello_message; context.secure_message_delegate->CreateSecureMessage( inner_message, context.persistent_symmetric_key, create_options, - base::Bind(&OnMiddleMessageCreatedForResponderAuth, context)); + base::BindOnce(&OnMiddleMessageCreatedForResponderAuth, context, + std::move(callback))); } // Called after the middle layer of [Responder Auth] is created. void OnMiddleMessageCreatedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, const std::string& middle_message) { if (middle_message.empty()) { PA_LOG(ERROR) << "Error inner message while creating [Responder Auth]"; - context.callback.Run(std::string()); + std::move(callback).Run(std::string()); return; } @@ -141,17 +150,19 @@ void OnMiddleMessageCreatedForResponderAuth( context.middle_message = middle_message; context.secure_message_delegate->DeriveKey( context.session_private_key, context.hello_public_key, - base::Bind(&OnSessionSymmetricKeyDerivedForResponderAuth, context)); + base::BindOnce(&OnSessionSymmetricKeyDerivedForResponderAuth, context, + std::move(callback))); } // Called after the session symmetric key is derived, so we can create the outer // most layer of [Responder Auth]. void OnSessionSymmetricKeyDerivedForResponderAuth( CreateResponderAuthMessageContext context, + DeviceToDeviceResponderOperations::MessageCallback callback, const std::string& session_symmetric_key) { if (session_symmetric_key.empty()) { PA_LOG(ERROR) << "Error inner message while creating [Responder Auth]"; - context.callback.Run(std::string()); + std::move(callback).Run(std::string()); return; } @@ -165,7 +176,7 @@ void OnSessionSymmetricKeyDerivedForResponderAuth( context.session_public_key)) { PA_LOG(ERROR) << "Error parsing public key while creating [Responder Auth]"; PA_LOG(ERROR) << context.session_public_key; - context.callback.Run(std::string()); + std::move(callback).Run(std::string()); return; } responder_hello.set_protocol_version(kD2DProtocolVersion); @@ -185,7 +196,7 @@ void OnSessionSymmetricKeyDerivedForResponderAuth( context.secure_message_delegate->CreateSecureMessage( device_to_device_message.SerializeAsString(), SessionKeys(session_symmetric_key).responder_encode_key(), create_options, - context.callback); + std::move(callback)); } // Helper struct containing all the context needed to validate the [Initiator @@ -194,29 +205,30 @@ struct ValidateInitiatorAuthMessageContext { std::string persistent_symmetric_key; std::string responder_auth_message; multidevice::SecureMessageDelegate* secure_message_delegate; - DeviceToDeviceResponderOperations::ValidationCallback callback; }; // Called after the inner-most layer of [Initiator Auth] is unwrapped. void OnInnerMessageUnwrappedForInitiatorAuth( const ValidateInitiatorAuthMessageContext& context, + DeviceToDeviceResponderOperations::ValidationCallback callback, bool verified, const std::string& payload, const securemessage::Header& header) { if (!verified) PA_LOG(VERBOSE) << "Failed to inner [Initiator Auth] message."; - context.callback.Run(verified); + std::move(callback).Run(verified); } // Called after the outer-most layer of [Initiator Auth] is unwrapped. void OnOuterMessageUnwrappedForInitiatorAuth( const ValidateInitiatorAuthMessageContext& context, + DeviceToDeviceResponderOperations::ValidationCallback callback, bool verified, const std::string& payload, const securemessage::Header& header) { if (!verified) { PA_LOG(VERBOSE) << "Failed to verify outer [Initiator Auth] message"; - context.callback.Run(false); + std::move(callback).Run(false); return; } @@ -225,7 +237,7 @@ void OnOuterMessageUnwrappedForInitiatorAuth( if (!device_to_device_message.ParseFromString(payload) || device_to_device_message.sequence_number() != 1) { PA_LOG(VERBOSE) << "Failed to validate DeviceToDeviceMessage payload."; - context.callback.Run(false); + std::move(callback).Run(false); return; } @@ -237,7 +249,8 @@ void OnOuterMessageUnwrappedForInitiatorAuth( context.secure_message_delegate->UnwrapSecureMessage( device_to_device_message.message(), context.persistent_symmetric_key, unwrap_options, - base::Bind(&OnInnerMessageUnwrappedForInitiatorAuth, context)); + base::BindOnce(&OnInnerMessageUnwrappedForInitiatorAuth, context, + std::move(callback))); } } // namespace @@ -247,7 +260,7 @@ void DeviceToDeviceResponderOperations::ValidateHelloMessage( const std::string& hello_message, const std::string& persistent_symmetric_key, multidevice::SecureMessageDelegate* secure_message_delegate, - const ValidateHelloCallback& callback) { + ValidateHelloCallback callback) { // The [Hello] message has the structure: // { // header: , @@ -259,7 +272,7 @@ void DeviceToDeviceResponderOperations::ValidateHelloMessage( unwrap_options.signature_scheme = securemessage::HMAC_SHA256; secure_message_delegate->UnwrapSecureMessage( hello_message, persistent_symmetric_key, unwrap_options, - base::Bind(&OnHelloMessageUnwrapped, callback)); + base::BindOnce(&OnHelloMessageUnwrapped, std::move(callback))); } // static @@ -270,7 +283,7 @@ void DeviceToDeviceResponderOperations::CreateResponderAuthMessage( const std::string& persistent_private_key, const std::string& persistent_symmetric_key, multidevice::SecureMessageDelegate* secure_message_delegate, - const MessageCallback& callback) { + MessageCallback callback) { // The [Responder Auth] message has the structure: // { // header: , @@ -293,14 +306,14 @@ void DeviceToDeviceResponderOperations::CreateResponderAuthMessage( session_private_key, persistent_private_key, persistent_symmetric_key, - secure_message_delegate, - callback}; + secure_message_delegate}; // To create the [Responder Auth] message, we need to first parse the // initiator's [Hello] message and extract the initiator's session public key. DeviceToDeviceResponderOperations::ValidateHelloMessage( hello_message, persistent_symmetric_key, secure_message_delegate, - base::Bind(&OnHelloMessageValidatedForResponderAuth, context)); + base::BindOnce(&OnHelloMessageValidatedForResponderAuth, context, + std::move(callback))); } // static @@ -310,7 +323,7 @@ void DeviceToDeviceResponderOperations::ValidateInitiatorAuthMessage( const std::string& persistent_symmetric_key, const std::string& responder_auth_message, multidevice::SecureMessageDelegate* secure_message_delegate, - const ValidationCallback& callback) { + DeviceToDeviceResponderOperations::ValidationCallback callback) { // The [Initiator Auth] message has the structure: // { // header: Sig(payload1, session_symmetric_key) @@ -323,9 +336,9 @@ void DeviceToDeviceResponderOperations::ValidateInitiatorAuthMessage( // } // }, session_symmetric_key) // } - ValidateInitiatorAuthMessageContext context = { - persistent_symmetric_key, responder_auth_message, secure_message_delegate, - callback}; + ValidateInitiatorAuthMessageContext context = {persistent_symmetric_key, + responder_auth_message, + secure_message_delegate}; multidevice::SecureMessageDelegate::UnwrapOptions unwrap_options; unwrap_options.encryption_scheme = securemessage::AES_256_CBC; @@ -333,7 +346,8 @@ void DeviceToDeviceResponderOperations::ValidateInitiatorAuthMessage( secure_message_delegate->UnwrapSecureMessage( initiator_auth_message, session_keys.initiator_encode_key(), unwrap_options, - base::Bind(&OnOuterMessageUnwrappedForInitiatorAuth, context)); + base::BindOnce(&OnOuterMessageUnwrappedForInitiatorAuth, context, + std::move(callback))); } } // namespace secure_channel diff --git a/chromeos/services/secure_channel/device_to_device_responder_operations.h b/chromeos/services/secure_channel/device_to_device_responder_operations.h index d1bddb8596a30..87e2a49a90b15 100644 --- a/chromeos/services/secure_channel/device_to_device_responder_operations.h +++ b/chromeos/services/secure_channel/device_to_device_responder_operations.h @@ -43,15 +43,16 @@ class DeviceToDeviceResponderOperations { public: // Callback for operations that create a message. Invoked with the serialized // SecureMessage upon success or the empty string upon failure. - typedef base::Callback MessageCallback; + typedef base::OnceCallback MessageCallback; // Callback for operations that validates a message. - typedef base::Callback ValidationCallback; + typedef base::OnceCallback ValidationCallback; // Callback for ValidateHelloMessage. The first argument will be called with // the validation outcome. If validation succeeded, then the second argument // will contain the initiator's public key. - typedef base::Callback ValidateHelloCallback; + typedef base::OnceCallback + ValidateHelloCallback; // Validates that the [Hello] message, received from the initiator, // is properly signed and encrypted. @@ -67,7 +68,7 @@ class DeviceToDeviceResponderOperations { const std::string& hello_message, const std::string& persistent_symmetric_key, multidevice::SecureMessageDelegate* secure_message_delegate, - const ValidateHelloCallback& callback); + ValidateHelloCallback callback); // Creates the [Responder Auth] message: // |hello_message|: The initial [Hello] message that was sent, which is used @@ -91,7 +92,7 @@ class DeviceToDeviceResponderOperations { const std::string& persistent_private_key, const std::string& persistent_symmetric_key, multidevice::SecureMessageDelegate* secure_message_delegate, - const MessageCallback& callback); + MessageCallback callback); // Validates that the [Initiator Auth] message, received from the initiator, // is properly signed and encrypted. @@ -110,7 +111,7 @@ class DeviceToDeviceResponderOperations { const std::string& persistent_symmetric_key, const std::string& responder_auth_message, multidevice::SecureMessageDelegate* secure_message_delegate, - const ValidationCallback& callback); + ValidationCallback callback); private: DISALLOW_IMPLICIT_CONSTRUCTORS(DeviceToDeviceResponderOperations);