Skip to content

Commit

Permalink
Bug 1733503 - Don't expose redirectStart and redirectEnd when there's…
Browse files Browse the repository at this point in the history
… 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 whatwg/html#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
  • Loading branch information
sefeng211 committed Oct 1, 2021
1 parent b12c503 commit e23dca5
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 30 deletions.
12 changes: 12 additions & 0 deletions dom/performance/PerformanceNavigationTiming.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions dom/performance/PerformanceNavigationTiming.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
5 changes: 3 additions & 2 deletions dom/performance/PerformanceResourceTiming.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
37 changes: 27 additions & 10 deletions dom/performance/PerformanceResourceTiming.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions dom/performance/PerformanceTiming.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ PerformanceTimingData::PerformanceTimingData(nsITimedChannel* aChannel,
mDecodedBodySize(0),
mRedirectCount(0),
mAllRedirectsSameOrigin(true),
mReportCrossOriginRedirect(true),
mAllRedirectsPassTAO(true),
mSecureConnection(false),
mTimingAllowed(true),
mInitialized(false) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(
Expand Down
17 changes: 9 additions & 8 deletions dom/performance/PerformanceTiming.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down

This file was deleted.

0 comments on commit e23dca5

Please sign in to comment.