Bug 1733503 - Don't expose redirectStart and redirectEnd when there's a cross-origin redirect for PerformanceNavigationTiming r=smaug

The actual text for hiding these information is actually missing at the
moment. This is the pending PR https://github.com/whatwg/html/pull/7105
which should fix it.

In addition to this PR, the obsolete (obsolete, but still accurate) definition
for `redirectStart` and `redirectEnd` has specified that when there's a
cross-origin redirect, these timings should not be exposed.

Obsolete definition: https://w3c.github.io/navigation-timing/#dom-performancetiming-redirectstart

Differential Revision: https://phabricator.services.mozilla.com/D127200

dom/performance/PerformanceNavigationTiming.cpp

@@ -134,6 +134,18 @@ uint16_t PerformanceNavigationTiming::RedirectCount() const {
   return mTimingData->GetRedirectCount();
 }
 
+DOMHighResTimeStamp PerformanceNavigationTiming::RedirectStart(
+    Maybe<nsIPrincipal*>& aSubjectPrincipal) const {
+  return PerformanceResourceTiming::RedirectStart(
+      aSubjectPrincipal, true /* aEnsureSameOriginAndIgnoreTAO */);
+}
+
+DOMHighResTimeStamp PerformanceNavigationTiming::RedirectEnd(
+    Maybe<nsIPrincipal*>& aSubjectPrincipal) const {
+  return PerformanceResourceTiming::RedirectEnd(
+      aSubjectPrincipal, true /* aEnsureSameOriginAndIgnoreTAO */);
+}
+
 void PerformanceNavigationTiming::UpdatePropertiesFromHttpChannel(
     nsIHttpChannel* aHttpChannel, nsITimedChannel* aChannel) {
   mTimingData->SetPropertiesFromHttpChannel(aHttpChannel, aChannel);

dom/performance/PerformanceNavigationTiming.h

@@ -67,6 +67,12 @@ class PerformanceNavigationTiming final : public PerformanceResourceTiming {
   DOMHighResTimeStamp DomComplete() const;
   DOMHighResTimeStamp LoadEventStart() const;
   DOMHighResTimeStamp LoadEventEnd() const;
+
+  DOMHighResTimeStamp RedirectStart(
+      Maybe<nsIPrincipal*>& aSubjectPrincipal) const override;
+  DOMHighResTimeStamp RedirectEnd(
+      Maybe<nsIPrincipal*>& aSubjectPrincipal) const override;
+
   NavigationType Type() const;
   uint16_t RedirectCount() const;
 

dom/performance/PerformanceResourceTiming.cpp

@@ -114,8 +114,9 @@ bool PerformanceResourceTiming::TimingAllowedForCaller(
 }
 
 bool PerformanceResourceTiming::ReportRedirectForCaller(
-    Maybe<nsIPrincipal*>& aCaller) const {
-  if (mTimingData->ShouldReportCrossOriginRedirect()) {
+    Maybe<nsIPrincipal*>& aCaller, bool aEnsureSameOriginAndIgnoreTAO) const {
+  if (mTimingData->ShouldReportCrossOriginRedirect(
+          aEnsureSameOriginAndIgnoreTAO)) {
     return true;
   }
 

dom/performance/PerformanceResourceTiming.h

@@ -60,24 +60,40 @@ class PerformanceResourceTiming : public PerformanceEntry {
     return mTimingData->FetchStartHighRes(mPerformance);
   }
 
-  DOMHighResTimeStamp RedirectStart(
-      Maybe<nsIPrincipal*>& aSubjectPrincipal) const {
-    // We have to check if all the redirect URIs had the same origin (since
-    // there is no check in RedirectStartHighRes())
-    return ReportRedirectForCaller(aSubjectPrincipal)
+  DOMHighResTimeStamp RedirectStart(Maybe<nsIPrincipal*>& aSubjectPrincipal,
+                                    bool aEnsureSameOriginAndIgnoreTAO) const {
+    // We have to check if all the redirect URIs whether had the same origin or
+    // different origins with TAO headers set (since there is no check in
+    // RedirectStartHighRes())
+    return ReportRedirectForCaller(aSubjectPrincipal,
+                                   aEnsureSameOriginAndIgnoreTAO)
                ? mTimingData->RedirectStartHighRes(mPerformance)
                : 0;
   }
 
-  DOMHighResTimeStamp RedirectEnd(
+  virtual DOMHighResTimeStamp RedirectStart(
       Maybe<nsIPrincipal*>& aSubjectPrincipal) const {
-    // We have to check if all the redirect URIs had the same origin (since
-    // there is no check in RedirectEndHighRes())
-    return ReportRedirectForCaller(aSubjectPrincipal)
+    return RedirectStart(aSubjectPrincipal,
+                         false /* aEnsureSameOriginAndIgnoreTAO */);
+  }
+
+  DOMHighResTimeStamp RedirectEnd(Maybe<nsIPrincipal*>& aSubjectPrincipal,
+                                  bool aEnsureSameOriginAndIgnoreTAO) const {
+    // We have to check if all the redirect URIs whether had the same origin or
+    // different origins with TAO headers set (since there is no check in
+    // RedirectEndHighRes())
+    return ReportRedirectForCaller(aSubjectPrincipal,
+                                   aEnsureSameOriginAndIgnoreTAO)
                ? mTimingData->RedirectEndHighRes(mPerformance)
                : 0;
   }
 
+  virtual DOMHighResTimeStamp RedirectEnd(
+      Maybe<nsIPrincipal*>& aSubjectPrincipal) const {
+    return RedirectEnd(aSubjectPrincipal,
+                       false /* aEnsureSameOriginAndIgnoreTAO */);
+  }
+
   DOMHighResTimeStamp DomainLookupStart(
       Maybe<nsIPrincipal*>& aSubjectPrincipal) const {
     return TimingAllowedForCaller(aSubjectPrincipal)
@@ -170,7 +186,8 @@ class PerformanceResourceTiming : public PerformanceEntry {
   bool TimingAllowedForCaller(Maybe<nsIPrincipal*>& aCaller) const;
 
   // Check if cross-origin redirects should be reported to the caller.
-  bool ReportRedirectForCaller(Maybe<nsIPrincipal*>& aCaller) const;
+  bool ReportRedirectForCaller(Maybe<nsIPrincipal*>& aCaller,
+                               bool aEnsureSameOriginAndIgnoreTAO) const;
 
   nsString mInitiatorType;
   const UniquePtr<PerformanceTimingData> mTimingData;  // always non-null

dom/performance/PerformanceTiming.cpp

@@ -109,7 +109,7 @@ PerformanceTimingData::PerformanceTimingData(nsITimedChannel* aChannel,
       mDecodedBodySize(0),
       mRedirectCount(0),
       mAllRedirectsSameOrigin(true),
-      mReportCrossOriginRedirect(true),
+      mAllRedirectsPassTAO(true),
       mSecureConnection(false),
       mTimingAllowed(true),
       mInitialized(false) {
@@ -221,9 +221,7 @@ void PerformanceTimingData::SetPropertiesFromHttpChannel(
   }
 
   mTimingAllowed = CheckAllowedOrigin(aHttpChannel, aChannel);
-  bool redirectsPassCheck = false;
-  aChannel->GetAllRedirectsPassTimingAllowCheck(&redirectsPassCheck);
-  mReportCrossOriginRedirect = mTimingAllowed && redirectsPassCheck;
+  aChannel->GetAllRedirectsPassTimingAllowCheck(&mAllRedirectsPassTAO);
 
   aChannel->GetNativeServerTiming(mServerTiming);
 }
@@ -292,16 +290,22 @@ uint8_t PerformanceTimingData::GetRedirectCount() const {
   return mRedirectCount;
 }
 
-bool PerformanceTimingData::ShouldReportCrossOriginRedirect() const {
+bool PerformanceTimingData::ShouldReportCrossOriginRedirect(
+    bool aEnsureSameOriginAndIgnoreTAO) const {
   if (!StaticPrefs::dom_enable_performance() || !IsInitialized() ||
       nsContentUtils::ShouldResistFingerprinting()) {
     return false;
   }
 
+  if (!mTimingAllowed || mRedirectCount == 0) {
+    return false;
+  }
+
   // If the redirect count is 0, or if one of the cross-origin
   // redirects doesn't have the proper Timing-Allow-Origin header,
   // then RedirectStart and RedirectEnd will be set to zero
-  return (mRedirectCount != 0) && mReportCrossOriginRedirect;
+  return aEnsureSameOriginAndIgnoreTAO ? mAllRedirectsSameOrigin
+                                       : mAllRedirectsPassTAO;
 }
 
 DOMHighResTimeStamp PerformanceTimingData::AsyncOpenHighRes(

dom/performance/PerformanceTiming.h

@@ -147,7 +147,12 @@ class PerformanceTimingData final {
   // If this is false the values of redirectStart/End will be 0 This is false if
   // no redirects occured, or if any of the responses failed the
   // timing-allow-origin check in HttpBaseChannel::TimingAllowCheck
-  bool ShouldReportCrossOriginRedirect() const;
+  //
+  // If aEnsureSameOriginAndIgnoreTAO is false, it checks if all redirects pass
+  // TAO. When it is true, it checks if all redirects are same-origin and
+  // ignores the result of TAO.
+  bool ShouldReportCrossOriginRedirect(
+      bool aEnsureSameOriginAndIgnoreTAO) const;
 
   // Cached result of CheckAllowedOrigin. If false, security sensitive
   // attributes of the resourceTiming object will be set to 0
@@ -200,10 +205,7 @@ class PerformanceTimingData final {
 
   bool mAllRedirectsSameOrigin = false;
 
-  // If the resourceTiming object should have non-zero redirectStart and
-  // redirectEnd attributes. It is false if there were no redirects, or if any
-  // of the responses didn't pass the timing-allow-check
-  bool mReportCrossOriginRedirect = false;
+  bool mAllRedirectsPassTAO = false;
 
   bool mSecureConnection = false;
 
@@ -448,7 +450,7 @@ struct IPDLParamTraits<mozilla::dom::PerformanceTimingData> {
     WriteIPDLParam(aMsg, aActor, aParam.mDecodedBodySize);
     WriteIPDLParam(aMsg, aActor, aParam.mRedirectCount);
     WriteIPDLParam(aMsg, aActor, aParam.mAllRedirectsSameOrigin);
-    WriteIPDLParam(aMsg, aActor, aParam.mReportCrossOriginRedirect);
+    WriteIPDLParam(aMsg, aActor, aParam.mAllRedirectsPassTAO);
     WriteIPDLParam(aMsg, aActor, aParam.mSecureConnection);
     WriteIPDLParam(aMsg, aActor, aParam.mTimingAllowed);
     WriteIPDLParam(aMsg, aActor, aParam.mInitialized);
@@ -532,8 +534,7 @@ struct IPDLParamTraits<mozilla::dom::PerformanceTimingData> {
                        &aResult->mAllRedirectsSameOrigin)) {
       return false;
     }
-    if (!ReadIPDLParam(aMsg, aIter, aActor,
-                       &aResult->mReportCrossOriginRedirect)) {
+    if (!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mAllRedirectsPassTAO)) {
       return false;
     }
     if (!ReadIPDLParam(aMsg, aIter, aActor, &aResult->mSecureConnection)) {

testing/web-platform/meta/navigation-timing/nav2_test_redirect_chain_xserver_final_original_origin.html.ini

@@ -1,4 +0,0 @@
-[nav2_test_redirect_chain_xserver_final_original_origin.html]
-  [Navigation Timing 2 WPT]
-    expected: FAIL
-