fix(metrics): stop using user-agent string in flow id check

#6044
r=vbudhram,shane-tomlinson

server/lib/flow-metrics.js

@@ -17,7 +17,7 @@ module.exports = {
    */
   create (key, userAgent) {
     const salt = crypto.randomBytes(SALT_SIZE).toString('hex');
-    return createFlowEventData(key, salt, Date.now(), userAgent);
+    return createFlowEventData(key, salt, Date.now());
   },
 
   /**
@@ -31,26 +31,32 @@ module.exports = {
    */
   validate (key, flowId, flowBeginTime, userAgent) {
     const salt = flowId.substr(0, SALT_STRING_LENGTH);
-    const expected = createFlowEventData(key, salt, flowBeginTime, userAgent);
+    let expected = createFlowEventData(key, salt, flowBeginTime);
 
+    if (getFlowSignature(flowId) === getFlowSignature(expected.flowId)) {
+      return true;
+    }
+
+    // HACK: We're transitioning between flow id recipes so, just for one train,
+    //       fall back to trying the old way if the preceding check failed.
+    expected = createFlowEventData(key, salt, flowBeginTime, userAgent);
     return getFlowSignature(flowId) === getFlowSignature(expected.flowId);
   }
 };
 
-function createFlowEventData(key, salt, flowBeginTime, userAgent) {
+function createFlowEventData (key, ...data) {
+  const [ salt, flowBeginTime ] = data;
+  data[1] = flowBeginTime.toString(16);
+
   // Incorporate a hash of request metadata into the flow id,
   // so that receiving servers can cross-check the metrics bundle.
   const flowSignature = crypto.createHmac('sha256', key)
-    .update([
-      salt,
-      flowBeginTime.toString(16),
-      userAgent
-    ].join('\n'))
+    .update(data.join('\n'))
     .digest('hex')
     .substr(0, SALT_STRING_LENGTH);
 
   return {
-    flowBeginTime: flowBeginTime,
+    flowBeginTime,
     flowId: salt + flowSignature
   };
 }

tests/server/flow-metrics.js

@@ -64,14 +64,13 @@ suite.tests['create correctly generates a known test vector'] = () => {
   //   print binascii.hexlify('MozillaFirefox!!')
   //   print hmac.new('S3CR37', '\n'.join((
   //      binascii.hexlify('MozillaFirefox!!'),
-  //      hex(1451566800000)[2:],
-  //      'Firefox'
+  //      hex(1451566800000)[2:]
   //   )), hashlib.sha256).hexdigest()[:32]
   //
-  var expectedSalt = '4d6f7a696c6c6146697265666f782121';
-  var expectedHmac = 'c89d56556d22039fbbf54d34e0baf206';
+  const expectedSalt = '4d6f7a696c6c6146697265666f782121';
+  const expectedHmac = '2a204a6d26b009b26b3116f643d84c6f';
 
-  var flowEventData = flowMetrics.create(mockFlowIdKey, mockUserAgent);
+  const flowEventData = flowMetrics.create(mockFlowIdKey, mockUserAgent);
 
   assert.equal(flowEventData.flowBeginTime, 1451566800000);
   assert.equal(flowEventData.flowId, expectedSalt + expectedHmac);
@@ -85,11 +84,11 @@ suite.tests['create generates different flowIds for different keys'] = () => {
   assert.equal(flowEventData1.flowBeginTime, flowEventData2.flowBeginTime);
 };
 
-suite.tests['create generates different flowIds for different user agents'] = () => {
-  var flowEventData1 = flowMetrics.create(mockFlowIdKey, 'Firefox');
-  var flowEventData2 = flowMetrics.create(mockFlowIdKey, 'Chrome');
+suite.tests['create generates the same flowId for different user agents'] = () => {
+  const flowEventData1 = flowMetrics.create(mockFlowIdKey, 'Firefox');
+  const flowEventData2 = flowMetrics.create(mockFlowIdKey, 'Chrome');
 
-  assert.notEqual(flowEventData1.flowId, flowEventData2.flowId);
+  assert.equal(flowEventData1.flowId, flowEventData2.flowId);
   assert.equal(flowEventData1.flowBeginTime, flowEventData2.flowBeginTime);
 };
 
@@ -115,15 +114,14 @@ suite.tests['create generates different flowIds for different timestamps'] = ()
   assert.notEqual(flowEventData1.flowBeginTime, flowEventData2.flowBeginTime);
 };
 
-suite.tests['validate returns true for good data'] = () => {
+suite.tests['validate returns true for good data with user-agent'] = () => {
   // Force the mocks to return bad data to be sure it really works
   mockDateNow = 1478626838531;
   mockFlowIdKey = 'foo';
   mockUserAgent = 'bar';
   mockRandomBytes = 'baz';
 
   const result = flowMetrics.validate(
-    // Good data is from the create test
     'S3CR37',
     '4d6f7a696c6c6146697265666f782121c89d56556d22039fbbf54d34e0baf206',
     1451566800000,
@@ -133,6 +131,23 @@ suite.tests['validate returns true for good data'] = () => {
   assert.strictEqual(result, true);
 };
 
+suite.tests['validate returns true for good data without user-agent'] = () => {
+  // Force the mocks to return bad data to be sure it really works
+  mockDateNow = 1478626838531;
+  mockFlowIdKey = 'foo';
+  mockUserAgent = 'bar';
+  mockRandomBytes = 'baz';
+
+  const result = flowMetrics.validate(
+    // Good data is from the create test
+    'S3CR37',
+    '4d6f7a696c6c6146697265666f7821212a204a6d26b009b26b3116f643d84c6f',
+    1451566800000
+  );
+
+  assert.strictEqual(result, true);
+};
+
 suite.tests['validate returns false for a bad flow id'] = () => {
   // Force the mocks to return good data to be sure it really works
   mockDateNow = 1451566800000;