Report DevTools issue for insecure attributionsrc redirects

Bug: 1347848
Change-Id: Ib050cd51cb714852d1f4dd781e02062086ab49c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3784973
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Quick-Run: Andrew Paseltiner <apaseltiner@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030199}

third_party/blink/renderer/core/frame/attribution_src_loader.cc

@@ -171,10 +171,17 @@ class AttributionSrcLoader::ResourceClient
                              uint64_t request_id);
 
  private:
-  void HandleSourceRegistration(const ResourceResponse& response,
-                                uint64_t request_id);
-  void HandleTriggerRegistration(const ResourceResponse& response,
-                                 uint64_t request_id);
+  [[nodiscard]] bool CheckReportingOrigin(
+      const SecurityOrigin& reporting_origin,
+      uint64_t request_id);
+  void HandleSourceRegistration(
+      const AtomicString& json,
+      scoped_refptr<const SecurityOrigin> reporting_origin,
+      uint64_t request_id);
+  void HandleTriggerRegistration(
+      const AtomicString& json,
+      scoped_refptr<const SecurityOrigin> reporting_origin,
+      uint64_t request_id);
 
   // RawResourceClient:
   String DebugName() const override;
@@ -499,82 +506,94 @@ void AttributionSrcLoader::ResourceClient::NotifyFinished(Resource* resource) {
 void AttributionSrcLoader::ResourceClient::HandleResponseHeaders(
     const ResourceResponse& response,
     uint64_t request_id) {
+  scoped_refptr<const SecurityOrigin> reporting_origin =
+      SecurityOrigin::Create(response.CurrentRequestUrl());
+
   const auto& headers = response.HttpHeaderFields();
 
-  bool can_process_source =
-      type_ == SrcType::kUndetermined || type_ == SrcType::kSource;
-  if (can_process_source &&
-      headers.Contains(http_names::kAttributionReportingRegisterSource)) {
-    type_ = SrcType::kSource;
-    HandleSourceRegistration(response, request_id);
-    return;
-  }
+  // TODO(apaseltiner): Report a DevTools issue when `type_` and `headers` do
+  // not correspond correctly.
 
   // TODO(johnidel): Consider surfacing an error when source and trigger headers
   // are present together.
-  bool can_process_trigger =
-      type_ == SrcType::kUndetermined || type_ == SrcType::kTrigger;
-  if (can_process_trigger &&
-      headers.Contains(http_names::kAttributionReportingRegisterTrigger)) {
-    type_ = SrcType::kTrigger;
-    HandleTriggerRegistration(response, request_id);
+
+  if (type_ == SrcType::kUndetermined || type_ == SrcType::kSource) {
+    const AtomicString& json =
+        headers.Get(http_names::kAttributionReportingRegisterSource);
+    if (!json.IsNull()) {
+      type_ = SrcType::kSource;
+      HandleSourceRegistration(json, std::move(reporting_origin), request_id);
+      return;
+    }
   }
+
+  if (type_ == SrcType::kUndetermined || type_ == SrcType::kTrigger) {
+    const AtomicString& json =
+        headers.Get(http_names::kAttributionReportingRegisterTrigger);
+    if (!json.IsNull()) {
+      type_ = SrcType::kTrigger;
+      HandleTriggerRegistration(json, std::move(reporting_origin), request_id);
+    }
+  }
+}
+
+bool AttributionSrcLoader::ResourceClient::CheckReportingOrigin(
+    const SecurityOrigin& reporting_origin,
+    uint64_t request_id) {
+  if (reporting_origin.IsPotentiallyTrustworthy())
+    return true;
+
+  LogAuditIssue(loader_->local_frame_->DomWindow(),
+                AttributionReportingIssueType::kUntrustworthyReportingOrigin,
+                /*element=*/nullptr, request_id,
+                /*invalid_parameter=*/reporting_origin.ToString());
+  return false;
 }
 
 void AttributionSrcLoader::ResourceClient::HandleSourceRegistration(
-    const ResourceResponse& response,
+    const AtomicString& json,
+    scoped_refptr<const SecurityOrigin> reporting_origin,
     uint64_t request_id) {
   DCHECK_EQ(type_, SrcType::kSource);
 
-  mojom::blink::AttributionSourceDataPtr source_data =
-      mojom::blink::AttributionSourceData::New();
-
-  // Verify the current url is trustworthy and capable of registering sources.
-  scoped_refptr<const SecurityOrigin> reporting_origin =
-      SecurityOrigin::Create(response.CurrentRequestUrl());
-  // TODO(apaseltiner): Report DevTools issue if this fails.
-  if (!reporting_origin->IsPotentiallyTrustworthy())
+  if (!CheckReportingOrigin(*reporting_origin, request_id))
     return;
-  source_data->reporting_origin = std::move(reporting_origin);
 
-  const AtomicString& source_json =
-      response.HttpHeaderField(http_names::kAttributionReportingRegisterSource);
+  auto source_data = mojom::blink::AttributionSourceData::New();
+
+  source_data->reporting_origin = std::move(reporting_origin);
 
   if (!attribution_response_parsing::ParseSourceRegistrationHeader(
-          source_json, *source_data)) {
+          json, *source_data)) {
     LogAuditIssue(loader_->local_frame_->DomWindow(),
                   AttributionReportingIssueType::kInvalidRegisterSourceHeader,
                   /*element=*/nullptr, request_id,
-                  /*invalid_parameter=*/source_json);
+                  /*invalid_parameter=*/json);
     return;
   }
 
   data_host_->SourceDataAvailable(std::move(source_data));
 }
 
 void AttributionSrcLoader::ResourceClient::HandleTriggerRegistration(
-    const ResourceResponse& response,
+    const AtomicString& json,
+    scoped_refptr<const SecurityOrigin> reporting_origin,
     uint64_t request_id) {
   DCHECK_EQ(type_, SrcType::kTrigger);
 
+  if (!CheckReportingOrigin(*reporting_origin, request_id))
+    return;
+
   auto trigger_data = mojom::blink::AttributionTriggerData::New();
 
-  // Verify the current url is trustworthy and capable of registering triggers.
-  scoped_refptr<const SecurityOrigin> reporting_origin =
-      SecurityOrigin::Create(response.CurrentRequestUrl());
-  // TODO(apaseltiner): Report DevTools issue if this fails.
-  if (!reporting_origin->IsPotentiallyTrustworthy())
-    return;
   trigger_data->reporting_origin = std::move(reporting_origin);
 
-  const AtomicString& trigger_json = response.HttpHeaderField(
-      http_names::kAttributionReportingRegisterTrigger);
   if (!attribution_response_parsing::ParseTriggerRegistrationHeader(
-          trigger_json, *trigger_data)) {
+          json, *trigger_data)) {
     LogAuditIssue(loader_->local_frame_->DomWindow(),
                   AttributionReportingIssueType::kInvalidRegisterTriggerHeader,
                   /*element=*/nullptr, request_id,
-                  /*invalid_parameter=*/trigger_json);
+                  /*invalid_parameter=*/json);
     return;
   }
 

third_party/blink/web_tests/http/tests/inspector-protocol/attribution-reporting/insecure-attributionsrc-redirect-source.js

@@ -0,0 +1,18 @@
+// Copyright 2022 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.
+
+(async function(testRunner) {
+  const {page, dp} = await testRunner.startBlank(
+      `Test that an attributionsrc that redirects to an insecure origin and tries to register a source triggers an issue.`);
+
+  await dp.Audits.enable();
+
+  await page.loadHTML(
+      `<img attributionsrc="https://devtools.test:8443/inspector-protocol/attribution-reporting/resources/redirect-to-insecure-origin-and-register-source.php">`);
+
+  const issuePromise = dp.Audits.onceIssueAdded();
+  const issue = await issuePromise;
+  testRunner.log(issue.params.issue, 'Issue reported: ', ['request']);
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/attribution-reporting/insecure-attributionsrc-redirect-trigger.js

@@ -0,0 +1,18 @@
+// Copyright 2022 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.
+
+(async function(testRunner) {
+  const {page, dp} = await testRunner.startBlank(
+      `Test that an attributionsrc that redirects to an insecure origin and tries to register a trigger triggers an issue.`);
+
+  await dp.Audits.enable();
+
+  await page.loadHTML(
+      `<img attributionsrc="https://devtools.test:8443/inspector-protocol/attribution-reporting/resources/redirect-to-insecure-origin-and-register-trigger.php">`);
+
+  const issuePromise = dp.Audits.onceIssueAdded();
+  const issue = await issuePromise;
+  testRunner.log(issue.params.issue, 'Issue reported: ', ['request']);
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/attribution-reporting/resources/redirect-to-insecure-origin-and-register-source.php

@@ -0,0 +1,3 @@
+<?php
+header('Location: http://devtools.test:8000/inspector-protocol/attribution-reporting/resources/register-source.php');
+?>

third_party/blink/web_tests/http/tests/inspector-protocol/attribution-reporting/resources/redirect-to-insecure-origin-and-register-trigger.php

@@ -0,0 +1,3 @@
+<?php
+header('Location: http://devtools.test:8000/inspector-protocol/attribution-reporting/resources/register-trigger.php');
+?>

third_party/blink/web_tests/http/tests/inspector-protocol/attribution-reporting/resources/register-source.php

@@ -0,0 +1,3 @@
+<?php
+header('Attribution-Reporting-Register-Source: {"source_event_id":"0","destination":"https://a.example"}');
+?>

third_party/blink/web_tests/platform/generic/http/tests/inspector-protocol/attribution-reporting/insecure-attributionsrc-redirect-source-expected.txt

@@ -0,0 +1,12 @@
+Test that an attributionsrc that redirects to an insecure origin and tries to register a source triggers an issue.
+Issue reported: {
+    code : AttributionReportingIssue
+    details : {
+        attributionReportingIssueDetails : {
+            invalidParameter : http://devtools.test:8000
+            request : <object>
+            violationType : UntrustworthyReportingOrigin
+        }
+    }
+}
+

third_party/blink/web_tests/platform/generic/http/tests/inspector-protocol/attribution-reporting/insecure-attributionsrc-redirect-trigger-expected.txt

@@ -0,0 +1,12 @@
+Test that an attributionsrc that redirects to an insecure origin and tries to register a trigger triggers an issue.
+Issue reported: {
+    code : AttributionReportingIssue
+    details : {
+        attributionReportingIssueDetails : {
+            invalidParameter : http://devtools.test:8000
+            request : <object>
+            violationType : UntrustworthyReportingOrigin
+        }
+    }
+}
+