Add base::UnguessableToken

base/nonce.h

+// Copyright 2016 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 BASE_NONCE_H_
+#define BASE_NONCE_H_
+
+#include <stdint.h>
+#include <string.h>
+
+#include "base/base_export.h"
+#include "base/format_macros.h"
+#include "base/hash.h"
+#include "base/strings/stringprintf.h"
+
+namespace base {
+
+// An Nonce is a cryptographically strong, randomly generated, unguessable 128
+// bit token.
+//
+// PLEASE MAKE SURE YOUR USAGE OF THIS CLASS IS SECURE!

[danakj, 2016/09/12 20:41:57]

You should explain how to make sure or this comment is scary but not actionable.

[tguilbert, 2016/09/12 23:53:33]

Updated.

+//
+struct BASE_EXPORT Nonce {
+  Nonce() : high(0), low(0){};

[danakj, 2016/09/12 20:41:57]

 = default;

move the defn to a .cc file

[tguilbert, 2016/09/12 23:53:33]

I have updated it to be "= default". If you have no objection, I will leave the
= default here, because moving it to the .cc file is considered to be a "user
defined" ctor, and std::is_pod<Nonce>::value would no longer be true. (Following
info I found in this post
http://stackoverflow.com/questions/4178175/what-are-aggregates-and-pods-and-h...)

+  Nonce(uint64_t high, uint64_t low) : high(high), low(low){};

[danakj, 2016/09/12 20:41:57]

dittos

[tguilbert, 2016/09/12 23:53:33]

Done.

+
+  // Generates a unique, strongly random, unguessable nonce.
+  static Nonce Generate();

[danakj, 2016/09/12 20:41:57]

maybe this should go first in the class so people find and use this.

[Fady Samuel, 2016/09/12 20:47:47]

Do we want to expose manually setting the nonce to potentially something
insecure? I'm not sure how I feel about this constructor:

Nonce(uint64_t high, uint64_t low) 

[tguilbert, 2016/09/12 23:53:33]

I agree with you and have changed the class to be harder to misuse.

+
+  bool is_null() const { return high == 0 && low == 0; };

[danakj, 2016/09/12 20:41:57]

no ; after a function body

[tguilbert, 2016/09/12 23:53:34]

Done.

+
+  std::string ToString() const {
+    return base::StringPrintf("(%" PRIu64 ":%" PRIu64 ")", high, low);

[danakj, 2016/09/12 20:41:57]

move this fn defn to a .cc file

[tguilbert, 2016/09/12 23:53:33]

Done.

+  }
+
+  // Acceptable hash for use in a std::unordered_map,
+  // but not cryptographically secure.
+  size_t hash() const { return base::HashInts64(high, low); }

[danakj, 2016/09/12 20:41:57]

Think this should be part of the hash struct, rather than part of this class.

[tguilbert, 2016/09/12 23:53:34]

Done.

+
+  bool operator<(const Nonce& other) const {
+    return high == other.high ? low < other.low : high < other.high;

[danakj, 2016/09/12 20:41:57]

use std::tie

[tguilbert, 2016/09/12 23:53:33]

Done.

+  }
+
+  bool operator==(const Nonce& other) const {
+    return high == other.high && low == other.low;
+  }
+
+  bool operator!=(const Nonce& other) const { return !operator==(other); }
+
+  // Note: two uint64_t are used instead of uint8_t[16], in order to have a

[danakj, 2016/09/12 20:41:57]

Two

[tguilbert, 2016/09/12 23:53:33]

Done.

+  // simpler ToString(), is_null() and hash() functions.
+  uint64_t high;

[Fady Samuel, 2016/09/12 20:47:47]

I'm a bit worried about exposing these because that invites the possibility of
changing them in client code after creation.

Personally I think it might make more sense to make these private and only
provide readonly accessors?

I'm not sure how we'd expose this to serialization code though. We could either
make serialization code a friend (layering violation) or continue to expose
Nonce(uint64_t high, uint64_t low). WDYT?

[tguilbert, 2016/09/12 23:53:33]

I agree with you. I wasn't sure in which ways one could misuse this class, but
moving the Nonce(uint64_t, uint64_t) to private forces users to be very
deliberate about their usage of the class.

I have added a Deserialize() function that I know I will use when converting
back from JNI (where sending two longs is simpler than checking the size of byte
arrays). Otherwise, the class is a POD class (and I have added unit test that
checks that std::is_pod<Nonce>::value is true, thanks to the info found
here:http://stackoverflow.com/questions/4178175/what-are-aggregates-and-pods-and-how-why-are-they-special/7189821#7189821).
The UT makes sure it safe to simply memcpy the class for deserialization.

+  uint64_t low;
+};
+
+}  // namespace base
+
+namespace std {
+// Specialize default hash for use in std::unordered_map.

[danakj, 2016/09/12 20:41:57]

You can provider a struct to do the hash if you like, but it should not be in
std::. https://google.github.io/styleguide/cppguide.html#std_hash

[tguilbert, 2016/09/12 23:53:33]

Done. TY!

+template <>
+struct hash<base::Nonce> {
+  size_t operator()(const base::Nonce& nonce) const { return nonce.hash(); }
+};
+}
+
+#endif  // BASE_NONCE_H_