Modify data_reduction_proxy_header to support tamper detection logic.

components/data_reduction_proxy/common/data_reduction_proxy_headers.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/common/data_reduction_proxy_headers.h"
 
 #include <string>
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/proxy/proxy_service.h"
 
 using base::StringPiece;
 using base::TimeDelta;
 using net::ProxyService;
 
+namespace {
+const char kChromeProxyHeader[] = "chrome-proxy";
+}  // namespace
+
 namespace data_reduction_proxy {
 
+const char kChromeProxyActionFingerprintChromeProxy[]   = "fcp";
+const char kChromeProxyActionFingerprintVia[]           = "fvia";
+const char kChromeProxyActionFingerprintOtherHeaders[]  = "foh";
+const char kChromeProxyActionFingerprintContentLength[] = "fcl";

[bengr, 2014/07/29 17:50:28]

I really don't like that you expose these names. Is there really no way to
provide methods that access what you need to know about them? E.g.,
bool HasChromeProxyFingerprint(...) or
bool GetChromeProxyFingerprint(std::string* fingerprint)

[xingx1, 2014/07/30 03:32:55]

Adding additional functions so that these constants would not be exposed to
namespace.

Done.

+
+bool GetDataReductionProxyActionValue(
+    const net::HttpResponseHeaders* headers,
+    const std::string& action_prefix,
+    std::string* action_value) {
+  DCHECK(!action_prefix.empty());

[bengr, 2014/07/29 17:50:28]

You should also check that header is not NULL.

[xingx1, 2014/07/30 03:32:55]

Done.

+  void* iter = NULL;
+  std::string value;
+  std::string prefix = action_prefix + "=";

[bengr, 2014/07/29 17:50:27]

Define "=" as kActionValueDelimiter in the anonymous namespace and use here and
below.

[xingx1, 2014/07/30 03:32:55]

Done.

+
+  while (headers->EnumerateHeader(&iter, kChromeProxyHeader, &value)) {
+    // ">=" to allow empty action value.

[bengr, 2014/07/29 17:50:27]

I don't think an empty value should be legal. Look at RFC2616, etc., to see
which pattern is used for other headers, e.g., cache-control.

[xingx1, 2014/07/30 03:32:56]

Done.

Previously I'm allowing empty action value to sort of send a command from
Flywheel without any parameter, say "via=" means "please check whether there is
intermediary on Via header".

+    if (value.size() >= prefix.size()) {
+      if (LowerCaseEqualsASCII(value.begin(),
+                               value.begin() + prefix.size(),
+                               prefix.c_str())) {
+        if (action_value)
+          *action_value = value.substr(prefix.size());
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 bool GetDataReductionProxyBypassDuration(
     const net::HttpResponseHeaders* headers,
     const std::string& action_prefix,
     base::TimeDelta* duration) {
+  DCHECK(!action_prefix.empty());
+  DCHECK(action_prefix[action_prefix.size() - 1] != '=');

[bolian, 2014/07/29 17:03:50]

same here, you don't need this DCHECK. Why not just allow '=' in the prefix.

[bengr, 2014/07/29 17:50:27]

I asked for the '=' to be separate from the prefix. This looks a little hacky
though.

[xingx1, 2014/07/30 03:32:55]

Done.

[xingx1, 2014/07/30 03:32:56]

I coded DCHECK there for now.

   void* iter = NULL;
   std::string value;
-  std::string name = "chrome-proxy";
+  std::string prefix = action_prefix + "=";
 
-  while (headers->EnumerateHeader(&iter, name, &value)) {
-    if (value.size() > action_prefix.size()) {
+  while (headers->EnumerateHeader(&iter, kChromeProxyHeader, &value)) {
+    if (value.size() > prefix.size()) {
       if (LowerCaseEqualsASCII(value.begin(),
-                               value.begin() + action_prefix.size(),
-                               action_prefix.c_str())) {
+                               value.begin() + prefix.size(),
+                               prefix.c_str())) {
         int64 seconds;
         if (!base::StringToInt64(
-                StringPiece(value.begin() + action_prefix.size(), value.end()),
+                StringPiece(value.begin() + prefix.size(), value.end()),
                 &seconds) || seconds < 0) {
           continue;  // In case there is a well formed instruction.
         }
         *duration = TimeDelta::FromSeconds(seconds);
         return true;
       }
     }
   }
   return false;
 }
 
 bool GetDataReductionProxyInfo(const net::HttpResponseHeaders* headers,
                                DataReductionProxyInfo* proxy_info) {
   DCHECK(proxy_info);
   proxy_info->bypass_all = false;
   proxy_info->bypass_duration = TimeDelta();
   // Support header of the form Chrome-Proxy: bypass|block=<duration>, where
   // <duration> is the number of seconds to wait before retrying
   // the proxy. If the duration is 0, then the default proxy retry delay
   // (specified in |ProxyList::UpdateRetryInfoOnFallback|) will be used.
   // 'bypass' instructs Chrome to bypass the currently connected data reduction
   // proxy, whereas 'block' instructs Chrome to bypass all available data
   // reduction proxies.
 
   // 'block' takes precedence over 'bypass', so look for it first.
   // TODO(bengr): Reduce checks for 'block' and 'bypass' to a single loop.
   if (GetDataReductionProxyBypassDuration(
-      headers, "block=", &proxy_info->bypass_duration)) {
+      headers, "block", &proxy_info->bypass_duration)) {

[bengr, 2014/07/29 17:50:28]

Add to the anonymous namespace:
const char kChromeProxyActionBlock[] = "block";
const char kChromeProxyActionBypass[] = "bypass";

[xingx1, 2014/07/30 03:32:55]

Done.

     proxy_info->bypass_all = true;
     return true;
   }
 
   // Next, look for 'bypass'.
   if (GetDataReductionProxyBypassDuration(
-      headers, "bypass=", &proxy_info->bypass_duration)) {
+      headers, "bypass", &proxy_info->bypass_duration)) {
     return true;
   }
   return false;
 }
 
-bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers) {
+bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers,
+                                    bool* has_intermediary) {
   const size_t kVersionSize = 4;
   const char kDataReductionProxyViaValue[] = "Chrome-Compression-Proxy";
   size_t value_len = strlen(kDataReductionProxyViaValue);
   void* iter = NULL;
   std::string value;
 
   // Case-sensitive comparison of |value|. Assumes the received protocol and the
   // space following it are always |kVersionSize| characters. E.g.,
   // 'Via: 1.1 Chrome-Compression-Proxy'
   while (headers->EnumerateHeader(&iter, "via", &value)) {
     if (value.size() >= kVersionSize + value_len &&
-        !value.compare(kVersionSize, value_len, kDataReductionProxyViaValue))
+        !value.compare(kVersionSize, value_len, kDataReductionProxyViaValue)) {
+      if (has_intermediary)

[bengr, 2014/07/29 17:50:27]

Add a comment here saying that we presume an intermediary if there is another
via header after the data reduction proxy's.

[xingx1, 2014/07/30 03:32:56]

Done.

+        *has_intermediary = !(headers->EnumerateHeader(&iter, "via", &value));
       return true;
+    }
   }
 
   // TODO(bengr): Remove deprecated header value.
   const char kDeprecatedDataReductionProxyViaValue[] =
       "1.1 Chrome Compression Proxy";
   iter = NULL;
   while (headers->EnumerateHeader(&iter, "via", &value))
-    if (value == kDeprecatedDataReductionProxyViaValue)
+    if (value == kDeprecatedDataReductionProxyViaValue) {
+      if (has_intermediary)
+        *has_intermediary = !(headers->EnumerateHeader(&iter, "via", &value));
       return true;
+    }
 
   return false;
 }
 
 const int kShortBypassMaxSeconds = 59;
 const int kMediumBypassMaxSeconds = 300;
 net::ProxyService::DataReductionProxyBypassType
 GetDataReductionProxyBypassType(
     const net::HttpResponseHeaders* headers,
     DataReductionProxyInfo* data_reduction_proxy_info) {
@@ skipping 17 matching lines @@
   if (headers->response_code() == net::HTTP_BAD_GATEWAY)
     return ProxyService::STATUS_502_HTTP_BAD_GATEWAY;
   if (headers->response_code() == net::HTTP_SERVICE_UNAVAILABLE)
     return ProxyService::STATUS_503_HTTP_SERVICE_UNAVAILABLE;
   // TODO(kundaji): Bypass if Proxy-Authenticate header value cannot be
   // interpreted by data reduction proxy.
   if (headers->response_code() == net::HTTP_PROXY_AUTHENTICATION_REQUIRED &&
       !headers->HasHeader("Proxy-Authenticate")) {
     return ProxyService::MALFORMED_407;
   }
-  if (!HasDataReductionProxyViaHeader(headers) &&
+  if (!HasDataReductionProxyViaHeader(headers, NULL) &&
       (headers->response_code() != net::HTTP_NOT_MODIFIED)) {
     // A Via header might not be present in a 304. Since the goal of a 304
     // response is to minimize information transfer, a sender in general
     // should not generate representation metadata other than Cache-Control,
     // Content-Location, Date, ETag, Expires, and Vary.
 
     // The proxy Via header might also not be present in a 4xx response.
     // Separate this case from other responses that are missing the header.
     if (headers->response_code() >= net::HTTP_BAD_REQUEST &&
         headers->response_code() < net::HTTP_INTERNAL_SERVER_ERROR) {
       return ProxyService::MISSING_VIA_HEADER_4XX;
     }
     return ProxyService::MISSING_VIA_HEADER_OTHER;
   }
   // There is no bypass event.
   return ProxyService::BYPASS_EVENT_TYPE_MAX;
 }
 
 }  // namespace data_reduction_proxy