Add public key and signature verification to browser-side API keys

content/common/experiments/api_key.h

Index: content/common/experiments/api_key.h
diff --git a/content/common/experiments/api_key.h b/content/common/experiments/api_key.h
index 6b06ea279c070871a45a32312fa53490b9a0a82e..dd211876dfbdf8b852d00879270d834247e331ec 100644
--- a/content/common/experiments/api_key.h
+++ b/content/common/experiments/api_key.h
@@ -56,6 +56,11 @@ class CONTENT_EXPORT ApiKey {
   bool ValidateOrigin(const std::string& origin) const;
   bool ValidateApiName(const std::string& api_name) const;
   bool ValidateDate(const base::Time& now) const;
+  bool ValidateSignature(const uint8_t* public_key) const;

[davidben, 2016/01/11 20:18:57]

Passing in a bare pointer without some indication of length seems iffy. I would
suggest one of:
- Use a base::StringPiece and check that the length is 32 in the function.
- Make the parameter a const uint8_t public_key[32] which at least tells the
reader they're responsible for passing in something of suitable size. (This
doesn't make a copy. C is weird.)

[iclelland, 2016/01/12 14:52:49]

I can certainly switch to StringPiece if that makes you feel better about it :)
The code is only called by this class, with a reference to the included public
key, and by tests, with a pointer to the public key in *that* class. It's not a
public method.

In the future, though, it will need to deal with data provided by the /chrome/
layer, so I may as well switch it now. 

(I had a length parameter, but it wasn't being used in any meaningful way, so it
was removed. Also, annotating the parameter size in the declaration looks like a
fun hack, but it's really no better than a code comment, I think.)

[davidben, 2016/01/13 20:50:39]

I'm fine with whatever. StringPiece or code comment or the funny array thing.
Also occurs to me that const uint8_t (&public_key)[32] would let the compiler
check it.

*shrug* I just wanted there to be some indication as to what the size is
supposed to be. Being protected, it's part of your class's API for (test-only)
subclasses.

[iclelland, 2016/01/13 21:30:11]

I'm good with StringPiece (other than all of the reinterpret_casts needed to use
it to uint_8 data). It may evolve into a PublicKey class at some point, but for
now it's just good to be able to pass around a single *thing* with an intrinsic
length property.

[davidben, 2016/01/13 22:18:13]

Yeah, having to add those casts drives me crazy. :-) (I will never forgive you,
C, for signed char...)

There's https://crbug.com/559302 but it's still unresolved.

+
+  static bool ValidateSignature(const std::string& signature_text,
+                                const std::string& data,
+                                const uint8_t* public_key);
 
  private:
   ApiKey(const std::string& signature,