Chrome Participated Tamper Detect

components/data_reduction_proxy/browser/data_reduction_proxy_tamper_detect.cc

+// 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.
+
+#include "components/data_reduction_proxy/browser/data_reduction_proxy_tamper_detect.h"
+
+#include <algorithm>
+#include <cstring>
+
+#include "base/base64.h"
+#include "base/md5.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
+#include "base/strings/string_number_conversions.h"
+#include "components/data_reduction_proxy/common/data_reduction_proxy_headers.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_util.h"
+
+#if defined(OS_ANDROID)
+#include "net/android/network_library.h"
+#endif
+
+// Macro for UMA reporting. Depending on |scheme_is_https|, it first reports to

[bengr, 2014/07/28 21:56:47]

I can't parse "it first reports to histograms events". Do you mean it reports
either to https_histogram or http_histogram depending on the scheme and adds
events by carrier id.

[xingx1, 2014/07/30 03:44:18]

Changed.

Two things: 
1) report to |http_histogram| by |carrier_id| and then report total number to
|http_histogram|_Total;
2) HTTP and HTTPS responses are reported to different histograms separately.

+// histogram events |https_histogram| or |http_histogram| by |carrier_id|; then
+// reports total counts to |https_histogram|_Total or |http_histogram|_Total.
+#define REPORT_TAMPER_DETECTION_UMA(scheme_is_https, http_histogram, https_histogram, carrier_id) \
+  do { \
+    if (scheme_is_https) { \
+      UMA_HISTOGRAM_SPARSE_SLOWLY(https_histogram, carrier_id); \
+      UMA_HISTOGRAM_COUNTS(https_histogram "_Total", 1); \
+    } else { \
+      UMA_HISTOGRAM_SPARSE_SLOWLY(http_histogram, carrier_id); \
+      UMA_HISTOGRAM_COUNTS(http_histogram "_Total", 1); \
+    }\
+  } while (0)
+
+namespace data_reduction_proxy {
+
+// static
+bool DataReductionProxyTamperDetection::DetectAndReport(
+    const net::HttpResponseHeaders* headers,
+    const bool is_secure_scheme) {

[bengr, 2014/07/28 21:56:48]

scheme_is_https

[xingx1, 2014/07/30 03:44:18]

Done. Also with .h file

+  DCHECK(headers);

[bengr, 2014/07/28 21:56:48]

are headers ever NULL? When does that happen?

[xingx1, 2014/07/30 03:44:18]

It should not happen.

Done.

+  if (!headers)
+    return false;
+
+  // If the fingerprint of the Chrome-Proxy header is absent, abort tamper

[bengr, 2014/07/28 21:56:49]

So a middlebox that strips fingerprints won't be detected, right? Add a comment
in the .h that says this. It would be better if the client could determine if a
response should have fingerprints, but that might be hard.

[xingx1, 2014/07/30 03:44:19]

Done.

+  // detection.
+  std::string chrome_proxy_fingerprint;
+  if (!GetDataReductionProxyActionValue(
+      headers,
+      kChromeProxyActionFingerprintChromeProxy,
+      &chrome_proxy_fingerprint))
+    return false;
+
+  // Gets the Chrome-Proxy header values.
+  std::vector<std::string> chrome_proxy_header_values =
+      GetHeaderValues(headers, "Chrome-Proxy");
+
+  // Removes header's fingerprint for generating the fingerprint of received
+  // Chrome-Proxy header later.
+  RemoveChromeProxyFingerprint(&chrome_proxy_header_values);
+
+  // Get carrier ID.
+  unsigned carrier_id = 0;
+#if defined(OS_ANDROID)
+  base::StringToUint(net::android::GetTelephonyNetworkOperator(), &carrier_id);
+#endif
+
+  DataReductionProxyTamperDetection tamper_detection(
+      headers,
+      is_secure_scheme,
+      carrier_id,
+      &chrome_proxy_header_values);
+
+  // Checks if the Chrome-Proxy header has been tampered with.
+  if (tamper_detection.IsChromeProxyHeaderTampered(chrome_proxy_fingerprint)) {
+    tamper_detection.ReportChromeProxyHeaderTamperedUMA();
+    return true;
+  }
+
+  // Since the Chrome-Proxy header has not been tampered with, reports the
+  // number of responses that other fingerprints will be checked.

[bengr, 2014/07/28 21:56:47]

I don't understand. Please clarify.

[xingx1, 2014/07/30 03:44:17]

Changed.

+  REPORT_TAMPER_DETECTION_UMA(
+      is_secure_scheme,
+      "DataReductionProxy.HTTPSHeaderTamperDetection",
+      "DataReductionProxy.HTTPHeaderTamperDetection",
+      carrier_id);
+
+  bool tampered = false;
+  std::map<std::string, FingerprintCode>::iterator i;
+  for (i = tamper_detection.fingerprint_name_code_map_.begin();
+       i != tamper_detection.fingerprint_name_code_map_.end(); ++i) {
+    std::string fingerprint;
+    if (!GetDataReductionProxyActionValue(
+        headers, i->first, &fingerprint)) {
+      continue;
+    }
+
+    switch (i->second) {
+    case VIA:
+      bool has_chrome_proxy_via_header;
+      if (tamper_detection.IsViaHeaderTampered(
+          fingerprint, &has_chrome_proxy_via_header)) {
+        tamper_detection.ReportViaHeaderTamperedUMA(
+            has_chrome_proxy_via_header);
+        tampered = true;
+      }
+      break;
+    case OTHERHEADERS:
+      if (tamper_detection.AreOtherHeadersTampered(fingerprint)) {
+        tamper_detection.ReportOtherHeadersTamperedUMA();
+        tampered = true;
+      }
+      break;
+    case CONTENTLENGTH:
+      if (tamper_detection.IsContentLengthHeaderTampered(fingerprint)) {
+        tamper_detection.ReportContentLengthHeaderTamperedUMA();
+        tampered = true;
+      }
+      break;
+    default:
+      NOTREACHED();
+      break;
+    }
+  }
+  return tampered;

[bengr, 2014/07/28 21:56:48]

might be better to return a status than a bool.

[xingx1, 2014/07/30 03:44:19]

Acknowledged.

+}
+
+// Constructor initializes the map of fingerprint name to code.

[bengr, 2014/07/28 21:56:49]

names to codes

[xingx1, 2014/07/30 03:44:18]

Done.

+DataReductionProxyTamperDetection::DataReductionProxyTamperDetection(
+    const net::HttpResponseHeaders* headers,
+    const bool is_secure,

[bengr, 2014/07/28 21:56:47]

scheme_is_https

[xingx1, 2014/07/30 03:44:18]

Done.

+    const unsigned carrier_id,
+    std::vector<std::string>* values)
+    : response_headers_(headers),
+      is_secure_scheme_(is_secure),
+      carrier_id_(carrier_id),
+      clean_chrome_proxy_header_values_(values) {
+  DCHECK(headers);
+  fingerprint_name_code_map_ = std::map<std::string, FingerprintCode>();

[bengr, 2014/07/28 21:56:47]

Why do you need this?

[xingx1, 2014/07/30 03:44:17]

Previously need this for "SWITCH", now I changed to "IF", so removed.

+  fingerprint_name_code_map_
+      [kChromeProxyActionFingerprintVia] = VIA;
+  fingerprint_name_code_map_
+      [kChromeProxyActionFingerprintOtherHeaders] = OTHERHEADERS;
+  fingerprint_name_code_map_
+      [kChromeProxyActionFingerprintContentLength] = CONTENTLENGTH;
+};
+
+DataReductionProxyTamperDetection::~DataReductionProxyTamperDetection() {};
+
+// Checks whether the Chrome-Proxy header has been tampered with. |fingerprint|
+// is the fingerprint received from the data reduction proxy, which is Base64
+// encoded. Decodes it first. Then calculates the fingerprint of received
+// Chrome-Proxy header, and compares the two to see whether they are equal or
+// not. Note that |clean_chrome_proxy_header_values_| holds the values of the
+// Chrome-Proxy header with its own fingerprint removed, so it's the correct
+// values to calculate fingerprint of received Chrome-Proxy header.
+bool DataReductionProxyTamperDetection::IsChromeProxyHeaderTampered(
+    const std::string& fingerprint) const {
+  std::string received_fingerprint;
+  if (!base::Base64Decode(fingerprint, &received_fingerprint))
+    return true;
+  // Calculates the MD5 hash value of Chrome-Proxy.
+  std::string actual_fingerprint = GetMD5(
+      ValuesToSortedString(clean_chrome_proxy_header_values_));
+
+  return received_fingerprint != actual_fingerprint;
+}
+
+void DataReductionProxyTamperDetection::ReportChromeProxyHeaderTamperedUMA()
+    const {
+  REPORT_TAMPER_DETECTION_UMA(
+      is_secure_scheme_,
+      "DataReductionProxy.HTTPSHeaderTampered_ChromeProxy",
+      "DataReductionProxy.HTTPHeaderTampered_ChromeProxy",
+      carrier_id_);
+}
+
+// Checks whether there are other proxies/middleboxes' name after the data

[bengr, 2014/07/28 21:56:47]

name -> named 

[xingx1, 2014/07/30 03:44:17]

Done.

+// reduction proxy's name in Via header. |has_chrome_proxy_via_header| marks
+// that whether the data reduction proxy's Via header occurs or not.
+bool DataReductionProxyTamperDetection::IsViaHeaderTampered(
+    const std::string& fingerprint, bool* has_chrome_proxy_via_header) const {
+  bool has_intermediary;
+  *has_chrome_proxy_via_header = HasDataReductionProxyViaHeader(
+      response_headers_,
+      &has_intermediary);
+
+  if (*has_chrome_proxy_via_header)
+    return !has_intermediary;
+  return false;
+}
+
+void DataReductionProxyTamperDetection::ReportViaHeaderTamperedUMA(
+    bool has_chrome_proxy) const {
+  // The Via header of the data reduction proxy is missing.
+  if (!has_chrome_proxy) {
+    REPORT_TAMPER_DETECTION_UMA(
+        is_secure_scheme_,
+        "DataReductionProxy.HTTPSHeaderTampered_Via_Missing",
+        "DataReductionProxy.HTTPHeaderTampered_Via_Missing",
+        carrier_id_);
+    return;
+  }
+
+  REPORT_TAMPER_DETECTION_UMA(
+      is_secure_scheme_,
+      "DataReductionProxy.HTTPSHeaderTampered_Via",
+      "DataReductionProxy.HTTPHeaderTampered_Via",
+      carrier_id_);
+}
+
+// Checks whether values of a predefined list of headers have been modified. At
+// the data reduction proxy side, it constructs a canonical representation of
+// values of a list headers. The fingerprint is constructed as follows:

[bengr, 2014/07/28 21:56:47]

list headers -> list of headers

[xingx1, 2014/07/30 03:44:18]

Done.

+// 1) for each header, gets the string representation of its values (same to

[bengr, 2014/07/28 21:56:49]

same to -> same as  OR "using" if that is true

[xingx1, 2014/07/30 03:44:17]

Done. I'm explaining fingerprint generation, which happens at Flywheel side. 

+//    ValuesToSortedString);
+// 2) concatenates all header's string representation with a ";" delimiter,

[bengr, 2014/07/28 21:56:48]

representation -> representations
with a ';' delimiter -> using ";" as a delimiter

[xingx1, 2014/07/30 03:44:17]

Done.

+//    respect to the order of the header list;

[bengr, 2014/07/28 21:56:48]

what does this mean? "keeping the headers in the order in which they were
received"

[xingx1, 2014/07/30 03:44:19]

I mean the order of that header list.
Header list is: header_a, header_b, header_c.
Then when we concatenate the string representation of each header, we need to
concatenate them according to header list's order, i.e., string representation
of header_a first, then header_b, then header_c.

But I think it's clear so I removed the sentence.

+// 3) calculates the MD5 hash value of above concatenated string;
+// 4) appends the header names to the fingerprint, with a delimiter "|".
+// The constructed fingerprint looks like:
+//     [hashed_fingerprint]|header_name1|header_namer2:...
+//
+// To check whether such fingerprint matches the response that the Chromium

[bengr, 2014/07/28 21:56:49]

such a  fingerprint

[xingx1, 2014/07/30 03:44:17]

Done.

+// client receives, the Chromium client firstly extracts the header names. For

[bengr, 2014/07/28 21:56:47]

the Chromium client -> the client

[xingx1, 2014/07/30 03:44:19]

Done.

+// each header, gets its string representation (by ValuesToSortedString),
+// concatenates them and calculates the MD5 hash value. Compares such hash

[bengr, 2014/07/28 21:56:47]

such --> the

[xingx1, 2014/07/30 03:44:18]

Done.

+// value to the fingerprint received from the data reduction proxy.
+bool DataReductionProxyTamperDetection::AreOtherHeadersTampered(

[bengr, 2014/07/28 21:56:48]

ValidateOtherHeaders

[xingx1, 2014/07/30 03:44:17]

Done.

+    const std::string& fingerprint) const {
+  std::string received_fingerprint;
+  DCHECK(fingerprint.size());

[bengr, 2014/07/28 21:56:48]

DCHECK(!fingerprint.empty());

[xingx1, 2014/07/30 03:44:19]

Done.

+
+  // "|" delimiter would not occur in base64 as well as header names.
+  net::HttpUtil::ValuesIterator it(fingerprint.begin(),
+                                   fingerprint.end(), '|');

[bengr, 2014/07/28 21:56:48]

can you fit all of these params on one line? E.g.,.

net::HttpUtil::ValuesIterator it(
    fingerprint.begin(), fingerprint.end(), '|');

[xingx1, 2014/07/30 03:44:18]

Done.

+
+  // The first value from fingerprint is the base64 encoded fingerprint; the
+  // following values are the header names included in fingerprint calculation.

[bengr, 2014/07/28 21:56:47]

in -> in the

[xingx1, 2014/07/30 03:44:18]

Done.

+  // Make sure there is [base64fingerprint] and it can be decoded.

[bengr, 2014/07/28 21:56:47]

Add a note that you ignore the first value.

[xingx1, 2014/07/30 03:44:18]

Done.

+  if (!(it.GetNext() &&
+        base::Base64Decode(it.value(), &received_fingerprint))) {
+    NOTREACHED();
+    return true;
+  }
+
+  std::string header_values;
+  // Enumerates the list of headers.
+  while (it.GetNext()) {
+    // Gets values of one header.
+    std::vector<std::string> response_header_values =
+        GetHeaderValues(response_headers_, it.value());
+    // Sorts the values and concatenate them, with delimiter ";". ";" would not
+    // occur in header values,
+    header_values += ValuesToSortedString(&response_header_values)  + ";";

[bengr, 2014/07/28 21:56:48]

Why not have ValuesToSortedSTring add the ";" to the end?

[xingx1, 2014/07/30 03:44:18]

ValuesToSortedString generates a representation of a header. In this
ValiadateOtherHeaders function, we are dealing with multiple headers, each
header's string is generated by ValuesToSortedString, we use delimiter ";" to
concatenate strings of multiple headers:
[representation of header A];[representation of header B];[representation of
header C];

I think right now it's clearer, because we have representation for each header,
and the delimiter.

+  }
+
+  // Calculates the MD5 hash of the concatenated string.
+  std::string actual_fingerprint = GetMD5(header_values);

[bengr, 2014/07/28 21:56:47]

you could avoid a copy by passing a pointer to the result into GetMD5.

[xingx1, 2014/07/30 03:44:17]

Done.

+
+  return received_fingerprint != actual_fingerprint;
+}
+
+void DataReductionProxyTamperDetection::ReportOtherHeadersTamperedUMA() const {
+  REPORT_TAMPER_DETECTION_UMA(
+      is_secure_scheme_,
+      "DataReductionProxy.HTTPSHeaderTampered_OtherHeaders",
+      "DataReductionProxy.HTTPHeaderTampered_OtherHeaders",
+      carrier_id_);
+}
+
+// Checks whether the Content-Length value is different from what the data
+// reduction proxy sends. Reports it as modified only if Content-Length can be
+// decoded as an integer at both ends and such two numbers are not equal.

[bengr, 2014/07/28 21:56:48]

I don't understand this last sentence.

[xingx1, 2014/07/30 03:44:19]

Done.

+bool DataReductionProxyTamperDetection::IsContentLengthHeaderTampered(
+    const std::string& fingerprint) const {
+  int received_content_length_fingerprint, actual_content_length;
+  // If Content-Length value from data reduction proxy does not exist or it

[bengr, 2014/07/28 21:56:47]

from the data reduction...

[xingx1, 2014/07/30 03:44:18]

Done.

+  // cannot be converted to an integer, abort.
+  if (base::StringToInt(fingerprint, &received_content_length_fingerprint)) {
+    std::string actual_content_length_string;
+    // If there is no Content-Length header received, abort.

[bengr, 2014/07/28 21:56:48]

You can move the "abort" first, here and elsewhere. E.g., "Abort if no
Content-Length header was received."

[xingx1, 2014/07/30 03:44:18]

Done. all other places.

+    if (response_headers_->GetNormalizedHeader("Content-Length",
+        &actual_content_length_string)) {
+      // If the Content-Length value cannot be converted to integer, abort.
+      if (!base::StringToInt(actual_content_length_string,
+                             &actual_content_length)) {
+        return false;
+      }
+
+      return received_content_length_fingerprint != actual_content_length;
+    }
+  }
+  return false;
+}
+
+void DataReductionProxyTamperDetection::ReportContentLengthHeaderTamperedUMA()
+    const {
+  // Gets MIME type of the response and reports to UMA histograms separately.
+  // Divides MIME types into 4 groups: JavaScript, CSS, Images, and others.
+  REPORT_TAMPER_DETECTION_UMA(
+      is_secure_scheme_,
+      "DataReductionProxy.HTTPSHeaderTampered_ContentLength",
+      "DataReductionProxy.HTTPHeaderTampered_ContentLength",
+      carrier_id_);
+
+  // Gets MIME type.
+  std::string mime_type;
+  response_headers_->GetMimeType(&mime_type);
+
+  // Reports tampered JavaScript.
+  if (mime_type.compare("text/javascript") == 0 ||
+      mime_type.compare("application/x-javascript") == 0 ||
+      mime_type.compare("application/javascript") == 0) {
+    REPORT_TAMPER_DETECTION_UMA(
+        is_secure_scheme_,
+        "DataReductionProxy.HTTPSHeaderTampered_ContentLength_JS",
+        "DataReductionProxy.HTTPHeaderTampered_ContentLength_JS",
+        carrier_id_);
+  }
+  // Reports tampered CSSs.
+  else if (mime_type.compare("text/css") == 0) {
+    REPORT_TAMPER_DETECTION_UMA(
+        is_secure_scheme_,
+        "DataReductionProxy.HTTPSHeaderTampered_ContentLength_CSS",
+        "DataReductionProxy.HTTPHeaderTampered_ContentLength_CSS",
+        carrier_id_);
+  }
+  // Reports tampered images.
+  else if (mime_type.find("image/") == 0) {
+    REPORT_TAMPER_DETECTION_UMA(
+        is_secure_scheme_,
+        "DataReductionProxy.HTTPSHeaderTampered_ContentLength_Image",
+        "DataReductionProxy.HTTPHeaderTampered_ContentLength_Image",
+        carrier_id_);
+  }
+  // Reports tampered other MIME types.
+  else {
+    REPORT_TAMPER_DETECTION_UMA(
+        is_secure_scheme_,
+        "DataReductionProxy.HTTPSHeaderTampered_ContentLength_Other",
+        "DataReductionProxy.HTTPHeaderTampered_ContentLength_Other",
+        carrier_id_);
+  }
+}
+
+DataReductionProxyTamperDetection::FingerprintCode
+    DataReductionProxyTamperDetection::GetFingerprintCode(
+    const std::string& fingerprint_name) {
+  std::map<std::string, FingerprintCode>::iterator it =
+      fingerprint_name_code_map_.find(fingerprint_name);
+
+  if (it != fingerprint_name_code_map_.end())
+    return it->second;
+  return NONEXIST;
+}
+
+// Removes the Chrome-Proxy header's fingerprint (action name

[bengr, 2014/07/28 21:56:48]

What if there is no such fingerprint? If you expect there to always be such a
fingerprint, verify that there is.

[xingx1, 2014/07/30 03:44:18]

Done.

+// |kFingerprintChromeProxy|) from its values vector.
+void DataReductionProxyTamperDetection::RemoveChromeProxyFingerprint(
+    std::vector<std::string>* values) {
+  DCHECK(values);
+  if (!values) return;
+
+  std::string chrome_proxy_fingerprint_prefix = std::string(
+      kChromeProxyActionFingerprintChromeProxy) + "=";
+
+  for (size_t i = 0; i < values->size(); ++i) {
+    if ((*values)[i].find(chrome_proxy_fingerprint_prefix) == 0) {
+      values->erase(values->begin() + i);
+      break;
+    }
+  }
+}
+
+// We construct a canonical representation of the header so that reordered
+// header values will produce the same fingerprint. The fingerprint is
+// constructed as follows:
+// 1) sorts the values;

[bengr, 2014/07/28 21:56:49]

1) sort the values

[xingx1, 2014/07/30 03:44:17]

Done.

+// 2) concatenates sorted values with a "," delimiter.

[bengr, 2014/07/28 21:56:48]

concatenate

[xingx1, 2014/07/30 03:44:17]

Done.

+std::string DataReductionProxyTamperDetection::ValuesToSortedString(
+    std::vector<std::string>* values) {
+  std::string concatenated_values;

[bengr, 2014/07/28 21:56:49]

Why not make this a parameter. E.g., ValuesToSortedSTring(std::vector...,
std::string* concatenated_values);

[xingx1, 2014/07/30 03:44:18]

Done.

+  DCHECK(values);
+  if (!values) return "";
+
+  std::sort(values->begin(), values->end());
+  for (size_t i = 0; i < values->size(); ++i) {
+    // Concatenates with delimiter ",".
+    concatenated_values += (*values)[i] + ",";
+  }
+  return concatenated_values;
+}
+
+std::string DataReductionProxyTamperDetection::GetMD5(
+    const std::string &input) {
+  base::MD5Digest digest;
+  base::MD5Sum(input.c_str(), input.size(), &digest);
+  return std::string((char*)digest.a, ARRAYSIZE_UNSAFE(digest.a));

[bengr, 2014/07/28 21:56:48]

Likewise, the output could be a parameter of the method.

[xingx1, 2014/07/30 03:44:17]

To avoid return value copying, right? Done.

+}
+
+std::vector<std::string> DataReductionProxyTamperDetection::GetHeaderValues(
+    const net::HttpResponseHeaders* headers, const std::string& header_name) {
+  std::vector<std::string> values;
+  std::string value;
+  void* iter = NULL;
+  while (headers->EnumerateHeader(&iter, header_name, &value)) {
+    values.push_back(value);
+  }
+  return values;
+}
+
+}  // namespace data_reduction_proxy