Change origin trial token format

content/common/origin_trials/trial_token.cc

Index: content/common/origin_trials/trial_token.cc
diff --git a/content/common/origin_trials/trial_token.cc b/content/common/origin_trials/trial_token.cc
index e44f6f046a2c712eaa6144f10d5aec51d9c4e9bf..e04e46495ac81997e72883046d53105c3348016e 100644
--- a/content/common/origin_trials/trial_token.cc
+++ b/content/common/origin_trials/trial_token.cc
@@ -9,12 +9,14 @@
 #include <vector>
 
 #include "base/base64.h"
+#include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"

[chasej, 2016/04/07 15:52:30]

Which of these string headers are still needed? Now, with parsing as binary
and/or typed values retrieved via json, it seems many headers could be removed.

[iclelland, 2016/04/07 20:26:44]

Good catch -- none of them!
Done.

 #include "base/time/time.h"
+#include "base/values.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -25,81 +27,128 @@ namespace {
 // Version 1 is the only token version currently supported
 const uint8_t kVersion1 = 1;
 
-const char* kFieldSeparator = "|";
+// Version is a 1-byte field
+const size_t kVersionBytesLength = 1;
+
+// Version 1 field sizes and offsets
+const size_t kSignatureBytesLength = 512 >> 3;  // get from curve25519 instead?
+
+const size_t kVersionOffset = 0;
+const size_t kSignatureOffset = kVersionOffset + kVersionBytesLength;
+const size_t kPayloadLengthOffset = kSignatureOffset + kSignatureBytesLength;
+const size_t kPayloadOffset = kPayloadLengthOffset + 4;
 
 }  // namespace
 
 TrialToken::~TrialToken() {}
 
-scoped_ptr<TrialToken> TrialToken::Parse(const std::string& token_text) {
-  if (token_text.empty()) {
+// Static
+scoped_ptr<TrialToken> TrialToken::From(const std::string& token_text,
+                                        base::StringPiece public_key) {
+  scoped_ptr<std::string> token_payload = Extract(token_text, public_key);
+  if (!token_payload) {
+    return nullptr;
+  }
+  return Parse(*token_payload);
+}
+
+scoped_ptr<std::string> TrialToken::Extract(const std::string& token_payload,

[chasej, 2016/04/07 15:52:30]

Move to match new declaration order.

[iclelland, 2016/04/07 20:26:45]

Done.

+                                            base::StringPiece public_key) {
+  if (token_payload.empty()) {
     return nullptr;
   }
 
-  // Extract the version from the token. The version must be the first part of
-  // the token, separated from the remainder, as:
-  // version|<version-specific contents>
-  size_t version_end = token_text.find(kFieldSeparator);
-  if (version_end == std::string::npos) {
+  // Token is base64-encoded; decode first.
+  std::string token_contents;
+  if (!base::Base64Decode(token_payload, &token_contents)) {
     return nullptr;
   }
 
-  std::string version_string = token_text.substr(0, version_end);
-  unsigned int version = 0;
-  if (!base::StringToUint(version_string, &version) || version > UINT8_MAX) {
+  // Token must be large enough to contain a version, signature, and payload
+  // length.
+  if (token_contents.length() < kPayloadOffset) {
     return nullptr;
   }
 
-  // Only version 1 currently supported
+  // Only version 1 currently supported.
+  uint8_t version = token_contents[0];
   if (version != kVersion1) {
     return nullptr;
   }
 
+  // Extract the length of the signed data (Big-endian).
+  uint32_t payload_length =
+      ((uint8_t)token_contents[kPayloadLengthOffset] << 24) +
+      ((uint8_t)token_contents[kPayloadLengthOffset + 1] << 16) +
+      ((uint8_t)token_contents[kPayloadLengthOffset + 2] << 8) +
+      ((uint8_t)token_contents[kPayloadLengthOffset + 3]);
+
+  // Validate that the stated length matches the actual payload length.
+  if (payload_length != token_contents.length() - kPayloadOffset) {
+    return nullptr;
+  }
+
   // Extract the version-specific contents of the token
-  std::string token_contents = token_text.substr(version_end + 1);
-
-  // The contents of a valid version 1 token should resemble:
-  // signature|origin|feature_name|expiry_timestamp
-  std::vector<std::string> parts =
-      SplitString(token_contents, kFieldSeparator, base::KEEP_WHITESPACE,
-                  base::SPLIT_WANT_ALL);
-  if (parts.size() != 4) {
+  std::string version_string =
+      token_contents.substr(kVersionOffset, kVersionBytesLength);
+  std::string signature =
+      token_contents.substr(kSignatureOffset, kSignatureBytesLength);
+  std::string payload_length_string =
+      token_contents.substr(kPayloadLengthOffset, 4);
+  std::string payload = token_contents.substr(kPayloadOffset, payload_length);
+
+  // The data which is covered by the signature is (version + length + payload)
+  std::string signed_data = version_string + payload_length_string + payload;
+
+  // Validate the signature on the data before proceeding
+  if (!TrialToken::ValidateSignature(signature, signed_data, public_key)) {
     return nullptr;
   }
 
-  const std::string& signature = parts[0];
-  const std::string& origin_string = parts[1];
-  const std::string& feature_name = parts[2];
-  const std::string& expiry_string = parts[3];
+  // Return just the payload, as a new string.
+  return make_scoped_ptr(new std::string(payload));
+}
 
-  uint64_t expiry_timestamp;
-  if (!base::StringToUint64(expiry_string, &expiry_timestamp)) {
+scoped_ptr<TrialToken> TrialToken::Parse(const std::string& token_json) {

[chasej, 2016/04/07 15:52:30]

Move to match new declaration order.

[iclelland, 2016/04/07 20:26:44]

Done.

+  // signed_data is JSON-encoded
+  scoped_ptr<base::DictionaryValue> datadict =
+      base::DictionaryValue::From(base::JSONReader::Read(token_json));
+  if (!datadict) {
     return nullptr;
   }
 
+  std::string origin_string;
+  std::string feature_name;
+  int expiry_timestamp = 0;
+  datadict->GetString("origin", &origin_string);
+  datadict->GetString("feature", &feature_name);
+  datadict->GetInteger("expiry", &expiry_timestamp);
+
   // Ensure that the origin is a valid (non-unique) origin URL
   url::Origin origin = url::Origin(GURL(origin_string));
   if (origin.unique()) {
     return nullptr;
   }
 
-  // Signed data is (origin + "|" + feature_name + "|" + expiry).
-  std::string data = token_contents.substr(signature.length() + 1);
+  // Ensure that the feature name is a valid string
+  if (feature_name.empty()) {
+    return nullptr;
+  }
 
-  return make_scoped_ptr(new TrialToken(version, signature, data, origin,
-                                        feature_name, expiry_timestamp));
-}
+  // Ensure that the expiry timestamp is a valid (positive) integer
+  if (expiry_timestamp <= 0) {
+    return nullptr;
+  }
 
-bool TrialToken::IsAppropriate(const url::Origin& origin,
-                               base::StringPiece feature_name) const {
-  return ValidateOrigin(origin) && ValidateFeatureName(feature_name);
+  return make_scoped_ptr(
+      new TrialToken(origin, feature_name, expiry_timestamp));
 }
 
-bool TrialToken::IsValid(const base::Time& now,
-                         base::StringPiece public_key) const {
-  // TODO(iclelland): Allow for multiple signing keys, and iterate over all
-  // active keys here. https://crbug.com/543220
-  return ValidateDate(now) && ValidateSignature(public_key);
+bool TrialToken::IsValidForFeature(const url::Origin& origin,
+                                   base::StringPiece feature_name,
+                                   const base::Time& now) const {
+  return ValidateOrigin(origin) && ValidateFeatureName(feature_name) &&
+         ValidateDate(now);
 }
 
 bool TrialToken::ValidateOrigin(const url::Origin& origin) const {
@@ -114,23 +163,13 @@ bool TrialToken::ValidateDate(const base::Time& now) const {
   return expiry_time_ > now;
 }
 
-bool TrialToken::ValidateSignature(base::StringPiece public_key) const {
-  return ValidateSignature(signature_, data_, public_key);
-}
-
 // static
-bool TrialToken::ValidateSignature(const std::string& signature_text,
+bool TrialToken::ValidateSignature(const std::string& signature,
                                    const std::string& data,
                                    base::StringPiece public_key) {
   // Public key must be 32 bytes long for Ed25519.
   CHECK_EQ(public_key.length(), 32UL);
 
-  std::string signature;
-  // signature_text is base64-encoded; decode first.
-  if (!base::Base64Decode(signature_text, &signature)) {
-    return false;
-  }
-
   // Signature must be 64 bytes long
   if (signature.length() != 64) {
     return false;
@@ -143,16 +182,10 @@ bool TrialToken::ValidateSignature(const std::string& signature_text,
   return (result != 0);
 }
 
-TrialToken::TrialToken(uint8_t version,
-                       const std::string& signature,
-                       const std::string& data,
-                       const url::Origin& origin,
+TrialToken::TrialToken(const url::Origin& origin,
                        const std::string& feature_name,
                        uint64_t expiry_timestamp)
-    : version_(version),
-      signature_(signature),
-      data_(data),
-      origin_(origin),
+    : origin_(origin),
       feature_name_(feature_name),
       expiry_time_(base::Time::FromDoubleT(expiry_timestamp)) {}