Merge cryptohome::RetrievedKeyData with cryptohome::KeyDefinition

chromeos/cryptohome/cryptohome_parameters.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROMEOS_CRYPTOHOME_CRYPTOHOME_PARAMETERS_H_
 #define CHROMEOS_CRYPTOHOME_CRYPTOHOME_PARAMETERS_H_
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/scoped_vector.h"
 #include "chromeos/chromeos_export.h"
 
 namespace cryptohome {
 
 enum AuthKeyPrivileges {
   PRIV_MOUNT = 1 << 0,              // Can mount with this key.
   PRIV_ADD = 1 << 1,                // Can add new keys.
   PRIV_REMOVE = 1 << 2,             // Can remove other keys.
   PRIV_MIGRATE = 1 << 3,            // Destroy all keys and replace with new.
   PRIV_AUTHORIZED_UPDATE = 1 << 4,  // Key can be updated in place.
   PRIV_DEFAULT = PRIV_MOUNT | PRIV_ADD | PRIV_REMOVE | PRIV_MIGRATE
 };
 
 // Identification of the user calling cryptohome method.
 struct CHROMEOS_EXPORT Identification {
   explicit Identification(const std::string& user_id);
 
   bool operator==(const Identification& other) const;
 
   std::string user_id;
 };
 
 // Definition of the key (e.g. password) for the cryptohome.
 // It contains authorization data along with extra parameters like perimissions

[Darren Krahn, 2014/09/02 18:21:53]

old code nit: perimissions -> permissions

[bartfab (slow), 2014/09/04 10:14:18]

Done.

 // associated with this key.
 struct CHROMEOS_EXPORT KeyDefinition {
-  KeyDefinition(const std::string& key,
+  enum Type {
+    TYPE_PASSWORD = 0
+  };
+
+  struct AuthorizationData {

[Darren Krahn, 2014/09/02 18:21:53]

IMO, this is under-documented in the protobuf and here's a chance to make it
easier to understand. A description of AuthorizationData and its intended use
would be great.

[bartfab (slow), 2014/09/04 10:14:18]

Unfortunately, I do not understand it either. I see that everyone always sets
this to |TYPE_HMACSHA256|. I have no idea what this is for and I do not know
when the other value would be used. Is there a doc that explains this somewhere?
I would be happy to distill it into a comment.

[Darren Krahn, 2014/09/04 23:13:23]

I shared the closest doc I could find. My understanding is that this allows a
key to be managed server-side -- or by a supervisor or some entity other than
the user. It is configured with AddKey and verified by cryptohome on UpdateKey.

Currently the only mechanism supported by cryptohome (AFAIK) is HMAC-SHA256 for
signing.

[bartfab (slow), 2014/09/15 12:04:44]

I cannot find any doc shared with me on that day. If you are OK with it, I will
land the CL as-is and check with Will on the exact purpose of AuthorizationData.
I do not want to add a description that ends up being incorrect.

+    enum Type {
+      TYPE_HMACSHA256 = 0,
+      TYPE_AES256CBC_HMACSHA256
+    };
+
+    struct Secret {
+      Secret(bool encrypt,

[Darren Krahn, 2014/09/02 18:21:53]

A constructor for the way we typically use this would be nice. E.g.

Secret(const std::string& hmac_key);

Also, because this is a struct, would a default constructor be appropriate?

[bartfab (slow), 2014/09/04 10:14:18]

Our typical use actuall always needs at least |encrypt|, |sign| and
|symmetric_key| and in some cases, the other two arguments as well. We never
construct a Secret with just a |symmetric_key|.
I added default constructors for all structs in this file.

+             bool sign,
+             const std::string& symmetric_key,
+             const std::string& public_key,
+             bool wrapped);
+
+      bool operator==(const Secret& other) const;
+
+      bool encrypt;
+      bool sign;
+      std::string symmetric_key;
+      std::string public_key;
+      bool wrapped;
+    };
+
+    AuthorizationData();

[Darren Krahn, 2014/09/02 18:21:53]

Same here, a typical usage constructor would be nice (in addition to this one).
E.g.

AuthorizationData(const std::string& hmac_key);

[bartfab (slow), 2014/09/04 10:14:18]

Done.

+    ~AuthorizationData();
+
+    bool operator==(const AuthorizationData& other) const;
+
+    Type type;
+    std::vector<Secret> secrets;
+  };
+
+  struct ProviderData {

[Darren Krahn, 2014/09/02 18:21:53]

Same here -- a bit of documentation would be great.

[bartfab (slow), 2014/09/04 10:14:18]

Done.

+    explicit ProviderData(const std::string& name);
+    explicit ProviderData(const ProviderData& other);
+    void operator=(const ProviderData& other);
+    ~ProviderData();
+
+    bool operator==(const ProviderData& other) const;
+
+    std::string name;
+    scoped_ptr<int64> number;
+    scoped_ptr<std::string> bytes;
+  };
+
+  KeyDefinition(const std::string& secret,
                 const std::string& label,
-                int /*AuthKeyPrivileges*/ privileges);
+                int privileges);

[Darren Krahn, 2014/09/02 18:21:53]

[optional] Default constructor here too? It just seems intuitive to me that a
struct can be declared like:

MyStruct my_struct;

[bartfab (slow), 2014/09/04 10:14:18]

Done.

   ~KeyDefinition();
 
   bool operator==(const KeyDefinition& other) const;
 
+  void AddSymmetricKey(bool encrypt,

[Darren Krahn, 2014/09/02 18:21:53]

It's not clear what this method does -- would it be better to call it
AddAuthorizationData or AddAuthorizationKey or AddAuthorizationHMACKey?

It's also not intuitive what the parameters actually mean. AFAIK, only 'sign'
with 'hmac' is supported so we could drop the two bools unless we intend to use
them in the short-medium term.

[bartfab (slow), 2014/09/04 10:14:18]

I removed the method as AuthorizationData() has a handy constructor now. We do
use both |sign| and |encrypt| by the way. The two places that create
authorization data (SupervisedUserCreationControllerNew and
ManagerPasswordService) create two keys each, an ecryption key and a signing
key.

[Darren Krahn, 2014/09/04 23:13:23]

Ok, I guess they are just future-proofing, currently the only encryption
mechanism is AES256CBC_HMAC256 and it's not supported by cryptohome.

+                       bool sign,
+                       const std::string& symmetric_key);
+
+  Type type;
   std::string label;
-
-  int revision;
-  std::string key;
-
-  std::string encryption_key;
-  std::string signature_key;
   // Privileges associated with key. Combination of |AuthKeyPrivileges| values.
   int privileges;
+  int revision;
+  std::string secret;
+
+  std::vector<AuthorizationData> authorization_data;
+  std::vector<ProviderData> provider_data;
 };
 
 // Authorization attempt data for user.
 struct CHROMEOS_EXPORT Authorization {
   Authorization(const std::string& key, const std::string& label);
   explicit Authorization(const KeyDefinition& key);
 
   bool operator==(const Authorization& other) const;
 
   std::string key;
   std::string label;
 };
 
-// Information about keys returned by GetKeyDataEx().
-struct CHROMEOS_EXPORT RetrievedKeyData {
-  enum Type {
-    TYPE_PASSWORD = 0
-  };
-
-  enum AuthorizationType {
-    AUTHORIZATION_TYPE_HMACSHA256 = 0,
-    AUTHORIZATION_TYPE_AES256CBC_HMACSHA256
-  };
-
-  struct ProviderData {
-    explicit ProviderData(const std::string& name);
-    ~ProviderData();
-
-    std::string name;
-    scoped_ptr<int64> number;
-    scoped_ptr<std::string> bytes;
-  };
-
-  RetrievedKeyData(Type type, const std::string& label, int64 revision);
-  ~RetrievedKeyData();
-
-  Type type;
-  std::string label;
-  // Privileges associated with key. Combination of |AuthKeyPrivileges| values.
-  int privileges;
-  int64 revision;
-  std::vector<AuthorizationType> authorization_types;
-  ScopedVector<ProviderData> provider_data;
-};
-
 // Parameters for Mount call.
 class CHROMEOS_EXPORT MountParameters {
  public:
   explicit MountParameters(bool ephemeral);
   ~MountParameters();
 
   bool operator==(const MountParameters& other) const;
 
   // If |true|, the mounted home dir will be backed by tmpfs. If |false|, the
   // ephemeral users policy decides whether tmpfs or an encrypted directory is
   // used as the backend.
   bool ephemeral;
 
   // If not empty, home dir will be created with these keys if it exist.
   std::vector<KeyDefinition> create_keys;
 };
 
 }  // namespace cryptohome
 
 #endif  // CHROMEOS_CRYPTOHOME_CRYPTOHOME_PARAMETERS_H_