Revert of Minor cleanup in EasyUnlockClient (patchset #2 id:20001 of https://codereview.chromium.org/569813002/)

Reason for revert:
Looks like some ChromeOS code was missed here; it's unclear why bots didn't catch this.

Original issue's description:
> Minor cleanup in EasyUnlockClient
>
> * arg names in GenerateEcP256KeyPairCallback typedef were reversed
> * Make CreateSecureMEssage and UnwrapSecureMessage take structs (instead
> of having long list of arguments)
>
> BUG=None
> TEST=existing unittests
>
> Committed: https://crrev.com/35e3a9c7061205db58f57182f5ba5eb87be0f355
> Cr-Commit-Position: refs/heads/master@{#294954}

TBR=stevenjb@chromium.org,isherman@chromium.org,tbarzic@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=None

Review URL: https://codereview.chromium.org/577443004

Cr-Commit-Position: refs/heads/master@{#294968}

5 files changed, 106 insertions, 121 deletions

diff --git a/chromeos/dbus/easy_unlock_client.cc b/chromeos/dbus/easy_unlock_client.cc
index 8d171a8..ea07f1c 100644
--- a/chromeos/dbus/easy_unlock_client.cc
+++ b/chromeos/dbus/easy_unlock_client.cc
@@ -74,7 +74,13 @@ class EasyUnlockClientImpl : public EasyUnlockClient {
 
   // EasyUnlockClient override.
   virtual void CreateSecureMessage(const std::string& payload,
-                                   const CreateSecureMessageOptions& options,
+                                   const std::string& secret_key,
+                                   const std::string& associated_data,
+                                   const std::string& public_metadata,
+                                   const std::string& verification_key_id,
+                                   const std::string& decryption_key_id,
+                                   const std::string& encryption_type,
+                                   const std::string& signature_type,
                                    const DataCallback& callback) OVERRIDE {
     dbus::MethodCall method_call(
         easy_unlock::kEasyUnlockServiceInterface,
@@ -83,13 +89,13 @@ class EasyUnlockClientImpl : public EasyUnlockClient {
     // NOTE: DBus expects that data sent as string is UTF-8 encoded. This is
     //     not guaranteed here, so the method uses byte arrays.
     AppendStringAsByteArray(payload, &writer);
-    AppendStringAsByteArray(options.key, &writer);
-    AppendStringAsByteArray(options.associated_data, &writer);
-    AppendStringAsByteArray(options.public_metadata, &writer);
-    AppendStringAsByteArray(options.verification_key_id, &writer);
-    AppendStringAsByteArray(options.decryption_key_id, &writer);
-    writer.AppendString(options.encryption_type);
-    writer.AppendString(options.signature_type);
+    AppendStringAsByteArray(secret_key, &writer);
+    AppendStringAsByteArray(associated_data, &writer);
+    AppendStringAsByteArray(public_metadata, &writer);
+    AppendStringAsByteArray(verification_key_id, &writer);
+    AppendStringAsByteArray(decryption_key_id, &writer);
+    writer.AppendString(encryption_type);
+    writer.AppendString(signature_type);
     proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
                        base::Bind(&EasyUnlockClientImpl::OnData,
                                   weak_ptr_factory_.GetWeakPtr(),
@@ -98,7 +104,10 @@ class EasyUnlockClientImpl : public EasyUnlockClient {
 
   // EasyUnlockClient override.
   virtual void UnwrapSecureMessage(const std::string& message,
-                                   const UnwrapSecureMessageOptions& options,
+                                   const std::string& secret_key,
+                                   const std::string& associated_data,
+                                   const std::string& encryption_type,
+                                   const std::string& signature_type,
                                    const DataCallback& callback) OVERRIDE {
     dbus::MethodCall method_call(
         easy_unlock::kEasyUnlockServiceInterface,
@@ -107,10 +116,10 @@ class EasyUnlockClientImpl : public EasyUnlockClient {
     // NOTE: DBus expects that data sent as string is UTF-8 encoded. This is
     //     not guaranteed here, so the method uses byte arrays.
     AppendStringAsByteArray(message, &writer);
-    AppendStringAsByteArray(options.key, &writer);
-    AppendStringAsByteArray(options.associated_data, &writer);
-    writer.AppendString(options.encryption_type);
-    writer.AppendString(options.signature_type);
+    AppendStringAsByteArray(secret_key, &writer);
+    AppendStringAsByteArray(associated_data, &writer);
+    writer.AppendString(encryption_type);
+    writer.AppendString(signature_type);
     proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
                        base::Bind(&EasyUnlockClientImpl::OnData,
                                   weak_ptr_factory_.GetWeakPtr(),
@@ -165,14 +174,6 @@ class EasyUnlockClientImpl : public EasyUnlockClient {
 
 }  // namespace
 
-EasyUnlockClient::CreateSecureMessageOptions::CreateSecureMessageOptions() {}
-
-EasyUnlockClient::CreateSecureMessageOptions::~CreateSecureMessageOptions() {}
-
-EasyUnlockClient::UnwrapSecureMessageOptions::UnwrapSecureMessageOptions() {}
-
-EasyUnlockClient::UnwrapSecureMessageOptions::~UnwrapSecureMessageOptions() {}
-
 EasyUnlockClient::EasyUnlockClient() {
 }
 
diff --git a/chromeos/dbus/easy_unlock_client.h b/chromeos/dbus/easy_unlock_client.h
index 82057fc..9aab446 100644
--- a/chromeos/dbus/easy_unlock_client.h
+++ b/chromeos/dbus/easy_unlock_client.h
@@ -30,74 +30,15 @@ class CHROMEOS_EXPORT EasyUnlockClient : public DBusClient {
 
   typedef base::Callback<void(const std::string& data)> DataCallback;
 
-  // Callback for |GenerateEcP256KeyPair|. Carries the generated keys.
-  typedef base::Callback<void(const std::string& private_key,
-                              const std::string& public_key)>
+  // Callback for |GenerateEcP256KeyAgreement|. Carries the generated keys.
+  typedef base::Callback<void(const std::string& public_key,
+                              const std::string& private_key)>
       KeyPairCallback;
 
   // Generates ECDSA key pair using P256 curve.
   // The created keys should only be used with this client.
   virtual void GenerateEcP256KeyPair(const KeyPairCallback& callback) = 0;
 
-  // Parameters used to create a secure message.
-  struct CreateSecureMessageOptions {
-    CreateSecureMessageOptions();
-    ~CreateSecureMessageOptions();
-
-    // The key used to sign, and if needed, encrypt the message. If encryption
-    // is required, the key must be symetric.
-    std::string key;
-
-    // Data associated with the message. The data will not actually be added to
-    // the message, but it will be used while signing the message (the receiver
-    // will use the same data to authenticate the signature).
-    std::string associated_data;
-
-    // Metadata added to the message header.
-    std::string public_metadata;
-
-    // The key id added to the message header. Has to be set if the message is
-    // signed with private asymetric key. This value is used by the receiver to
-    // identify the key that should be used to verify the signature.
-    std::string verification_key_id;
-
-    // Key id added to the message header. Used by the message receiver to
-    // identify the key that should be used to decrypt the message.
-    std::string decryption_key_id;
-
-    // The encryption algorithm to use for encrypting the message.
-    std::string encryption_type;
-
-    // The algorithm to use to sign the message.
-    std::string signature_type;
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(CreateSecureMessageOptions);
-  };
-
-  // Parameters used to unwrap a securemessage.
-  struct UnwrapSecureMessageOptions {
-    UnwrapSecureMessageOptions();
-    ~UnwrapSecureMessageOptions();
-
-    // The key used to authenticate message signature and, if needed, decrypt
-    // the message. If the message is encrypted, only symetric key can be used.
-    std::string key;
-
-    // Data associated with the message. Message authentication will succeed
-    // only if the message was created with the same associated data.
-    std::string associated_data;
-
-    // The encryption algorithm to use for decrypting the message.
-    std::string encryption_type;
-
-    // The algorithm that should be used to verify the message signature.
-    std::string signature_type;
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(UnwrapSecureMessageOptions);
-  };
-
   // Given a private and a public key, creates a symetric secret key using
   // EC Diffe-Hellman key exchange. The provided keys come from different
   // asymetric key pairs, and are expected to be in the same format as the ones
@@ -110,22 +51,55 @@ class CHROMEOS_EXPORT EasyUnlockClient : public DBusClient {
   // Creates signed and, if specified, encrypted message in format used by Easy
   // Unlock.
   // |payload|: The cleartext message body.
-  // |options|: The message parameters used for creating the secure message.
+  // |key|: The key used to sign, and if needed, encrypt the message. If
+  //     encryption is required, the key must be symetric.
+  // |associated_data|: Data associated with the message. The data will not
+  //     actually be added to the message, but it will be used while
+  //     signing the message (the receiver will use the same data to
+  //     authenticate the signature).
+  // |public_metadata|: Metadata added to the message header.
+  // |verification_key_id|: The key id added to the message header. Has to be
+  //     set if the message is signed with private asymetric key. This value
+  //     is used by the receiver to identify the public key that should be used
+  //     to verify the signature.
+  // |decryption_key_id|: Key id added to the message header. Used by the
+  //     message receiver to identify the key that should be used to decrypt
+  //     the message.
+  // |encryption_type|: The encryption algorithm to use for encrypting the
+  //     message. (May be set to none).
+  // |signature_type|: The algorithm to use to sign the message.
   // |callback|: Called with the created message. On failure, the message will
   //     be empty.
   virtual void CreateSecureMessage(const std::string& payload,
-                                   const CreateSecureMessageOptions& options,
+                                   const std::string& secret_key,
+                                   const std::string& associated_data,
+                                   const std::string& public_metadata,
+                                   const std::string& verification_key_id,
+                                   const std::string& decryption_key_id,
+                                   const std::string& encryption_type,
+                                   const std::string& signature_type,
                                    const DataCallback& callback) = 0;
 
   // Authenticates and, if specified, decrypts a secure message.
   // |message|: The message to unwrap. It is in the same format as the message
   //     returned by |CreateSecureMessage|.
-  // |options|: The parameters that should be used to unwrap the message.
+  // |key|: The key used to authenticate message signature and, if needed,
+  //     decrypt the message. If the message is encrypted, only symetric key
+  //     can be used.
+  // |associated_data|: Data associated with the message. Message
+  //     authentication will succeed only if the message was created with the
+  //     associated data.
+  // |encryption_type|: The encryption algorithm to use for decrypting the
+  //     message. (May be set to none).
+  // |signature_type|: The algorithm to use to verify the message signature.
   // |callback|: Called with the cleartext message header and body in a signle
   //     protobuf. If the message could not be authenticated or decrypted, it
   //     will be called with an empty string.
   virtual void UnwrapSecureMessage(const std::string& message,
-                                   const UnwrapSecureMessageOptions& options,
+                                   const std::string& secret_key,
+                                   const std::string& associated_data,
+                                   const std::string& encryption_type,
+                                   const std::string& signature_type,
                                    const DataCallback& callback) = 0;
 
   // Factory function, creates a new instance and returns ownership.
diff --git a/chromeos/dbus/fake_easy_unlock_client.cc b/chromeos/dbus/fake_easy_unlock_client.cc
index 77d8cb0..fe0aee1 100644
--- a/chromeos/dbus/fake_easy_unlock_client.cc
+++ b/chromeos/dbus/fake_easy_unlock_client.cc
@@ -95,7 +95,13 @@ void FakeEasyUnlockClient::PerformECDHKeyAgreement(
 
 void FakeEasyUnlockClient::CreateSecureMessage(
     const std::string& payload,
-    const CreateSecureMessageOptions& options,
+    const std::string& key,
+    const std::string& associated_data,
+    const std::string& public_metadata,
+    const std::string& verification_key_id,
+    const std::string& decryption_key_id,
+    const std::string& encryption_type,
+    const std::string& signature_type,
     const DataCallback& callback) {
   callback.Run(base::StringPrintf(
       "{\"securemessage\": {"
@@ -109,18 +115,21 @@ void FakeEasyUnlockClient::CreateSecureMessage(
           "\"signature_type\": \"%s\""
       "}}",
       payload.c_str(),
-      options.key.c_str(),
-      options.associated_data.c_str(),
-      options.public_metadata.c_str(),
-      options.verification_key_id.c_str(),
-      options.decryption_key_id.c_str(),
-      options.encryption_type.c_str(),
-      options.signature_type.c_str()));
+      key.c_str(),
+      associated_data.c_str(),
+      public_metadata.c_str(),
+      verification_key_id.c_str(),
+      decryption_key_id.c_str(),
+      encryption_type.c_str(),
+      signature_type.c_str()));
 }
 
 void FakeEasyUnlockClient::UnwrapSecureMessage(
     const std::string& message,
-    const UnwrapSecureMessageOptions& options,
+    const std::string& key,
+    const std::string& associated_data,
+    const std::string& encryption_type,
+    const std::string& signature_type,
     const DataCallback& callback) {
   // TODO(tbarzic): Verify that |message| is in the format returned by
   // |CreateSecureMessage| and extract payload, metadata and
@@ -134,10 +143,10 @@ void FakeEasyUnlockClient::UnwrapSecureMessage(
           "\"signature_type\": \"%s\""
       "}}",
       message.c_str(),
-      options.key.c_str(),
-      options.associated_data.c_str(),
-      options.encryption_type.c_str(),
-      options.signature_type.c_str()));
+      key.c_str(),
+      associated_data.c_str(),
+      encryption_type.c_str(),
+      signature_type.c_str()));
 }
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/fake_easy_unlock_client.h b/chromeos/dbus/fake_easy_unlock_client.h
index 046689d..bc83207 100644
--- a/chromeos/dbus/fake_easy_unlock_client.h
+++ b/chromeos/dbus/fake_easy_unlock_client.h
@@ -29,10 +29,19 @@ class CHROMEOS_EXPORT FakeEasyUnlockClient : public EasyUnlockClient {
                                        const std::string& public_key,
                                        const DataCallback& callback) OVERRIDE;
   virtual void CreateSecureMessage(const std::string& payload,
-                                   const CreateSecureMessageOptions& options,
+                                   const std::string& key,
+                                   const std::string& associated_data,
+                                   const std::string& public_metadata,
+                                   const std::string& verification_key_id,
+                                   const std::string& decryption_key_id,
+                                   const std::string& encryption_type,
+                                   const std::string& signature_type,
                                    const DataCallback& callback) OVERRIDE;
   virtual void UnwrapSecureMessage(const std::string& message,
-                                   const UnwrapSecureMessageOptions& options,
+                                   const std::string& key,
+                                   const std::string& associated_data,
+                                   const std::string& encryption_type,
+                                   const std::string& signature_type,
                                    const DataCallback& callback) OVERRIDE;
 
  private:
diff --git a/chromeos/dbus/fake_easy_unlock_client_unittest.cc b/chromeos/dbus/fake_easy_unlock_client_unittest.cc
index 2be4024..7c344d9 100644
--- a/chromeos/dbus/fake_easy_unlock_client_unittest.cc
+++ b/chromeos/dbus/fake_easy_unlock_client_unittest.cc
@@ -203,19 +203,15 @@ TEST(FakeEasyUnlockClientTest, CreateSecureMessage) {
   chromeos::FakeEasyUnlockClient client;
 
   std::string message;
-
-  chromeos::EasyUnlockClient::CreateSecureMessageOptions options;
-  options.key = "KEY";
-  options.associated_data = "ASSOCIATED_DATA";
-  options.public_metadata = "PUBLIC_METADATA";
-  options.verification_key_id = "VERIFICATION_KEY_ID";
-  options.decryption_key_id = "DECRYPTION_KEY_ID";
-  options.encryption_type = "ENCRYPTION_TYPE";
-  options.signature_type = "SIGNATURE_TYPE";
-
   client.CreateSecureMessage(
       "PAYLOAD",
-      options,
+      "KEY",
+      "ASSOCIATED_DATA",
+      "PUBLIC_METADATA",
+      "VERIFICATION_KEY_ID",
+      "DECRYPTION_KEY_ID",
+      "ENCRYPTION_TYPE",
+      "SIGNATURE_TYPE",
       base::Bind(&RecordData, &message));
 
   const std::string expected_message(
@@ -236,16 +232,12 @@ TEST(FakeEasyUnlockClientTest, UnwrapSecureMessage) {
   chromeos::FakeEasyUnlockClient client;
 
   std::string message;
-
-  chromeos::EasyUnlockClient::UnwrapSecureMessageOptions options;
-  options.key = "KEY";
-  options.associated_data = "ASSOCIATED_DATA";
-  options.encryption_type = "ENCRYPTION_TYPE";
-  options.signature_type = "SIGNATURE_TYPE";
-
   client.UnwrapSecureMessage(
       "MESSAGE",
-      options,
+      "KEY",
+      "ASSOCIATED_DATA",
+      "ENCRYPTION_TYPE",
+      "SIGNATURE_TYPE",
       base::Bind(&RecordData, &message));
 
   const std::string expected_message(