Cup request sender: use x-cup-server-proof header when available.

After this change, the RequestSender is choosing between
x-cup-server-proof and etag value, and gives priority to the
x-cup-server-proof value if it is non-empty.

The values for both headers is read by the network stacks, and made
them available to the RequestSender, which implements the validation
mechanism in one place.

It's impractical to unit test this code, since the RequestSender
hardcodes the production public key and it can only transact CUP
with a production backend.

Bug: 606958
Change-Id: I5b0f2e9f2ba1e75ca4fd4c4becde848846541bfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2266918Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: S. Ganesh <ganesh@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782987}

chrome/updater/mac/net/network_fetcher.mm

@@ -155,6 +155,12 @@ using DownloadToFileCompleteCallback =
   if ([headers objectForKey:headerEtag]) {
     etag = [headers objectForKey:headerEtag];
   }
+  NSString* headerXCupServerProof = base::SysUTF8ToNSString(
+      update_client::NetworkFetcher::kHeaderXCupServerProof);
+  NSString* cupServerProof = @"";
+  if ([headers objectForKey:headerXCupServerProof]) {
+    cupServerProof = [headers objectForKey:headerXCupServerProof];
+  }
   int64_t retryAfterResult = -1;
   NSString* xRetryAfter = [headers
       objectForKey:base::SysUTF8ToNSString(
@@ -173,8 +179,8 @@ using DownloadToFileCompleteCallback =
       base::BindOnce(
           std::move(_postRequestCompleteCallback),
           std::make_unique<std::string>(base::SysNSStringToUTF8(dataToUse)),
-          error.code, std::string(base::SysNSStringToUTF8(etag)),
-          retryAfterResult));
+          error.code, base::SysNSStringToUTF8(etag),
+          base::SysNSStringToUTF8(cupServerProof), retryAfterResult));
 }
 
 @end

chrome/updater/mac/net/network_unittest.mm

@@ -51,9 +51,11 @@ class ChromeUpdaterNetworkMacTest : public ::testing::Test {
   void PostRequestCompleteCallback(std::unique_ptr<std::string> response_body,
                                    int net_error,
                                    const std::string& header_etag,
+                                   const std::string& header_x_cup_server_proof,
                                    int64_t xheader_retry_after_sec) {
     EXPECT_EQ(net_error, 0);
-    EXPECT_GT(header_etag.length(), 0u);
+    EXPECT_STREQ(header_etag.c_str(), "Wfhw789h");
+    EXPECT_STREQ(header_x_cup_server_proof.c_str(), "server-proof");
     EXPECT_EQ(xheader_retry_after_sec, 67);
     PostRequestCompleted();
   }
@@ -75,6 +77,7 @@ class ChromeUpdaterNetworkMacTest : public ::testing::Test {
     http_response->set_content_type("text/plain");
     http_response->AddCustomHeader("X-Retry-After", "67");
     http_response->AddCustomHeader("ETag", "Wfhw789h");
+    http_response->AddCustomHeader("X-Cup-Server-Proof", "server-proof");
     return http_response;
   }
 

chrome/updater/win/net/network_fetcher.cc

@@ -66,7 +66,8 @@ void NetworkFetcher::PostRequestComplete() {
   std::move(post_request_complete_callback_)
       .Run(std::make_unique<std::string>(network_fetcher_->GetResponseBody()),
            network_fetcher_->GetNetError(), network_fetcher_->GetHeaderETag(),
-           network_fetcher_->GetXHeaderRetryAfterSec());
+           network_fetcher_->GetHeaderXCupServerProof(),
+           network_fetcher_->GetHeaderXRetryAfterSec());
 }
 
 void NetworkFetcher::DownloadToFileComplete() {

chrome/updater/win/net/network_winhttp.cc

@@ -92,12 +92,17 @@ HRESULT NetworkFetcherWinHTTP::GetNetError() const {
 
 std::string NetworkFetcherWinHTTP::GetHeaderETag() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return etag_;
+  return header_etag_;
 }
 
-int64_t NetworkFetcherWinHTTP::GetXHeaderRetryAfterSec() const {
+std::string NetworkFetcherWinHTTP::GetHeaderXCupServerProof() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return xheader_retry_after_sec_;
+  return header_x_cup_server_proof_;
+}
+
+int64_t NetworkFetcherWinHTTP::GetHeaderXRetryAfterSec() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return header_x_retry_after_sec_;
 }
 
 base::FilePath NetworkFetcherWinHTTP::GetFilePath() const {
@@ -299,7 +304,16 @@ void NetworkFetcherWinHTTP::HeadersAvailable() {
   base::string16 etag;
   if (SUCCEEDED(QueryHeadersString(request_handle_.get(), WINHTTP_QUERY_ETAG,
                                    WINHTTP_HEADER_NAME_BY_INDEX, &etag))) {
-    etag_ = base::SysWideToUTF8(etag);
+    header_etag_ = base::SysWideToUTF8(etag);
+  }
+
+  base::string16 xheader_cup_server_proof;
+  if (SUCCEEDED(QueryHeadersString(
+          request_handle_.get(), WINHTTP_QUERY_CUSTOM,
+          base::SysUTF8ToWide(
+              update_client::NetworkFetcher::kHeaderXCupServerProof),
+          &xheader_cup_server_proof))) {
+    header_x_cup_server_proof_ = base::SysWideToUTF8(xheader_cup_server_proof);
   }
 
   int xheader_retry_after_sec = 0;
@@ -308,7 +322,7 @@ void NetworkFetcherWinHTTP::HeadersAvailable() {
           base::SysUTF8ToWide(
               update_client::NetworkFetcher::kHeaderXRetryAfter),
           &xheader_retry_after_sec))) {
-    xheader_retry_after_sec_ = xheader_retry_after_sec;
+    header_x_retry_after_sec_ = xheader_retry_after_sec;
   }
 
   std::move(fetch_started_callback_).Run(response_code, content_length);

chrome/updater/win/net/network_winhttp.h

@@ -66,7 +66,8 @@ class NetworkFetcherWinHTTP
   std::string GetResponseBody() const;
   HRESULT GetNetError() const;
   std::string GetHeaderETag() const;
-  int64_t GetXHeaderRetryAfterSec() const;
+  std::string GetHeaderXCupServerProof() const;
+  int64_t GetHeaderXRetryAfterSec() const;
   base::FilePath GetFilePath() const;
 
   // Returns the number of bytes retrieved from the network. This may be
@@ -133,8 +134,9 @@ class NetworkFetcherWinHTTP
   base::string16 content_type_;
   WriteDataCallback write_data_callback_;
   HRESULT net_error_ = S_OK;
-  std::string etag_;
-  int64_t xheader_retry_after_sec_ = -1;
+  std::string header_etag_;
+  std::string header_x_cup_server_proof_;
+  int64_t header_x_retry_after_sec_ = -1;
   std::vector<char> read_buffer_;
   std::string post_response_body_;
   base::FilePath file_path_;

components/update_client/net/network_impl.cc

@@ -139,6 +139,7 @@ void NetworkFetcherImpl::PostRequest(
             std::move(post_request_complete_callback)
                 .Run(std::move(response_body), simple_url_loader->NetError(),
                      GetStringHeader(simple_url_loader, kHeaderEtag),
+                     GetStringHeader(simple_url_loader, kHeaderXCupServerProof),
                      GetInt64Header(simple_url_loader, kHeaderXRetryAfter));
           },
           simple_url_loader_.get(), std::move(post_request_complete_callback)),

components/update_client/network.cc

@@ -7,6 +7,7 @@
 namespace update_client {
 
 constexpr char NetworkFetcher::kHeaderEtag[];
+constexpr char NetworkFetcher::kHeaderXCupServerProof[];
 constexpr char NetworkFetcher::kHeaderXRetryAfter[];
 
 }  // namespace update_client

components/update_client/network.h

@@ -31,6 +31,7 @@ class NetworkFetcher {
       base::OnceCallback<void(std::unique_ptr<std::string> response_body,
                               int net_error,
                               const std::string& header_etag,
+                              const std::string& header_x_cup_server_proof,
                               int64_t xheader_retry_after_sec)>;
   using DownloadToFileCompleteCallback =
       base::OnceCallback<void(int net_error, int64_t content_size)>;
@@ -38,9 +39,11 @@ class NetworkFetcher {
       base::OnceCallback<void(int response_code, int64_t content_length)>;
   using ProgressCallback = base::RepeatingCallback<void(int64_t current)>;
 
-  // The ETag header carries the ECSDA signature of the POST response, if
-  // signing has been used.
+  // The following two headers carry the ECSDA signature of the POST response,
+  // if signing has been used. Two headers are used for redundancy purposes.
+  // The value of the `X-Cup-Server-Proof` is preferred.
   static constexpr char kHeaderEtag[] = "ETag";
+  static constexpr char kHeaderXCupServerProof[] = "X-Cup-Server-Proof";
 
   // The server uses the optional X-Retry-After header to indicate that the
   // current request should not be attempted again.

components/update_client/request_sender.cc

@@ -8,6 +8,7 @@
 
 #include "base/base64.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
@@ -32,6 +33,18 @@ constexpr char kKeyPubBytesBase64[] =
 // The content type for all protocol requests.
 constexpr char kContentType[] = "application/json";
 
+// Returns the value of |response_cup_server_proof| or the value of
+// |response_etag|, if the former value is empty.
+const std::string& SelectCupServerProof(
+    const std::string& response_cup_server_proof,
+    const std::string& response_etag) {
+  if (response_cup_server_proof.empty()) {
+    DVLOG(3) << "Using etag as cup server proof.";
+    return response_etag;
+  }
+  return response_cup_server_proof;
+}
+
 }  // namespace
 
 RequestSender::RequestSender(scoped_refptr<Configurator> config)
@@ -96,20 +109,22 @@ void RequestSender::SendInternal() {
         base::BindOnce(&RequestSender::SendInternalComplete,
                        base::Unretained(this),
                        static_cast<int>(ProtocolError::URL_FETCHER_FAILED),
-                       std::string(), std::string(), 0));
+                       std::string(), std::string(), std::string(), 0));
   }
   network_fetcher_->PostRequest(
       url, request_body_, kContentType, request_extra_headers_,
       base::BindOnce(&RequestSender::OnResponseStarted, base::Unretained(this)),
-      base::BindRepeating([](int64_t current) {}),
+      base::DoNothing(),
       base::BindOnce(&RequestSender::OnNetworkFetcherComplete,
                      base::Unretained(this), url));
 }
 
-void RequestSender::SendInternalComplete(int error,
-                                         const std::string& response_body,
-                                         const std::string& response_etag,
-                                         int retry_after_sec) {
+void RequestSender::SendInternalComplete(
+    int error,
+    const std::string& response_body,
+    const std::string& response_etag,
+    const std::string& response_cup_server_proof,
+    int retry_after_sec) {
   VLOG(2) << "Omaha response received: " << response_body;
 
   if (!error) {
@@ -122,7 +137,9 @@ void RequestSender::SendInternalComplete(int error,
 
     DCHECK(use_signing_);
     DCHECK(signer_);
-    if (signer_->ValidateResponse(response_body, response_etag)) {
+    if (signer_->ValidateResponse(
+            response_body,
+            SelectCupServerProof(response_cup_server_proof, response_etag))) {
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::BindOnce(std::move(request_sender_callback_), 0,
                                     response_body, retry_after_sec));
@@ -157,6 +174,7 @@ void RequestSender::OnNetworkFetcherComplete(
     std::unique_ptr<std::string> response_body,
     int net_error,
     const std::string& header_etag,
+    const std::string& xheader_cup_server_proof,
     int64_t xheader_retry_after_sec) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
@@ -175,10 +193,11 @@ void RequestSender::OnNetworkFetcherComplete(
     retry_after_sec = base::saturated_cast<int>(xheader_retry_after_sec);
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(&RequestSender::SendInternalComplete,
-                                base::Unretained(this), error,
-                                response_body ? *response_body : std::string(),
-                                header_etag, retry_after_sec));
+      FROM_HERE,
+      base::BindOnce(&RequestSender::SendInternalComplete,
+                     base::Unretained(this), error,
+                     response_body ? *response_body : std::string(),
+                     header_etag, xheader_cup_server_proof, retry_after_sec));
 }
 
 void RequestSender::HandleSendError(int error, int retry_after_sec) {

components/update_client/request_sender.h

@@ -71,6 +71,7 @@ class RequestSender {
                                 std::unique_ptr<std::string> response_body,
                                 int net_error,
                                 const std::string& header_etag,
+                                const std::string& xheader_cup_server_proof,
                                 int64_t xheader_retry_after_sec);
 
   // Implements the error handling and url fallback mechanism.
@@ -81,6 +82,7 @@ class RequestSender {
   void SendInternalComplete(int error,
                             const std::string& response_body,
                             const std::string& response_etag,
+                            const std::string& response_cup_server_proof,
                             int retry_after_sec);
 
   // Helper function to handle a non-continuable error in Send.