Hide some navigation timing info when cross-origin redirects are present #7105

noamr · 2021-09-23T12:58:41Z

When a navigation includes cross-origin redirects, the navigation timing
entry should not include information about redirect timing and internal
network timing, as that may expose cross-origin timing information.

Closes #7104

Depends on whatwg/fetch#1309

[x] At least two implementers are interested (and none opposed):

Part of is already implemented and tested, but has been omitted when refactoring the navigation timing spec into HTML.

[x] Tests already cover this: navigation-timing/test_timing_xserver_redirect.html
…
[] Implementation bugs are filed:
Chrome: 1063370
Firefox: 1733044
Safari: TBD

Will have to close these bugs for now if we agree to keep the status quo, which is what's reflected in this PR.

(See WHATWG Working Mode: Changes for more details.)

/browsing-the-web.html ( diff )
/infrastructure.html ( diff )

domenic · 2021-09-25T19:35:45Z

Can you restore the PR template for this change, by editing the original post?

noamr · 2021-09-26T08:04:57Z

Can you restore the PR template for this change, by editing the original post?

Done

Exposing connection timing info such as domainLookupStart may expose the fact that a cross-origin redirect has occured, information which should be hidden. This is fixed in whatwg/html#7105, and this modifies the test to account for the new behavior.

annevk · 2021-09-28T12:18:29Z

To be clear, the template @domenic is referring to is https://github.com/whatwg/html/blob/main/PULL_REQUEST_TEMPLATE.md. I don't see it in OP.

annevk

@yoavweiss should review this as well or perhaps this should wait for @domenic who I believe reviewed the last round of navigation timing additions.

source

annevk · 2021-09-28T12:29:45Z

Also, thanks for working on this!

yoavweiss · 2021-09-29T06:26:13Z

PR preview seems to be failing with a build error. Are you able to build this locally?

noamr · 2021-09-29T06:35:16Z

PR preview seems to be failing with a build error. Are you able to build this locally?

It builds locally, I rebased on forced push with hope.

yoavweiss

Looks good, but I can't find the "opaque timing info" algorithm, which seems critical

source

noamr · 2021-09-29T06:50:17Z

Looks good, but I can't find the "opaque timing info" algorithm, which seems critical

Yea, this depends on whatwg/fetch#1309.

yoavweiss

LGTM

noamr · 2021-09-30T05:21:47Z

Trying to figure out whether exposing secureConnectionStart et al after cross-origin redirects is actually OK (as per current behavior), since the document who can read these attributes is from the same URL that the connection is relevant to. So in essence it doesn't expose any new cross-origin information at all.

Also, in that case, redirectStart and redirectEnd are values that can be extrapolated, since that final server knows the difference between navigationStart and the timing of its own request.

So I think we should either go with this (hiding all TAO-protected info when cross-origin redirects are present), or also expose redirectStart and redirectEnd when cross-origin redirects are part of the navigation.

If we choose to keep the current behavior, I will amend this PR to match it (it needs to special-case redirectStart and redirectEnd) and drop the WPT and browser issues.

@yoavweiss @sefeng211 @npm1 @annevk thoughts?

annevk · 2021-09-30T09:59:53Z

I think at least in theory a connection can encompass multiple origins (if the certificate is valid for all of them, though not all user agents support this), how does secureConnectionStart deal with that?

noamr · 2021-09-30T10:24:14Z

I think at least in theory a connection can encompass multiple origins (if the certificate is valid for all of them, though not all user agents support this), how does secureConnectionStart deal with that?

It would clamp secureConnectionStart to fetchStart - it would look like the secure connection was immediate.

… 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

noamr · 2021-10-05T12:47:05Z

This is on hold until we figure out w3c/navigation-timing#160

noamr · 2022-01-17T11:00:53Z

This is on hold until we figure out w3c/navigation-timing#160

Since w3c/navigation-timing#160 is not close to resolution, for now continuing with this to at least reflect the current implementations.

When a navigation includes cross-origin redirects, the navigation timing entry should not include information about redirect timing and internal network timing, as that may expose cross-origin timing information. This is already implemented and tested, but has been omitted when refactoring the navigation timing spec into HTML. Closes whatwg#7104

noamr · 2022-02-08T14:40:20Z

Looking at this again, I don't think it makes sense to TAO-protect anything in navigation timing as long as navigation start is the time origin. The connection info is anyway only relevant to the current origin, and redirectStart / redirectEnd can be derived from fetchStart not being zero.

Trying to figure out whether exposing secureConnectionStart et al after cross-origin redirects is actually OK (as per current behavior), since the document who can read these attributes is from the same URL that the connection is relevant to. So in essence it doesn't expose any new cross-origin information at all.

Also, in that case, redirectStart and redirectEnd are values that can be extrapolated, since that final server knows the difference between navigationStart and the timing of its own request.

So I think we should either go with this (hiding all TAO-protected info when cross-origin redirects are present), or also expose redirectStart and redirectEnd when cross-origin redirects are part of the navigation.

If we choose to keep the current behavior, I will amend this PR to match it (it needs to special-case redirectStart and redirectEnd) and drop the WPT and browser issues.

@yoavweiss @sefeng211 @npm1 @annevk thoughts?

Looking at this again, I don't think it makes sense to TAO-protect anything in navigation timing as long as navigation start is the time origin. The connection info is anyway only relevant to the current origin, and redirectStart / redirectEnd can be derived from fetchStart not being zero. Since the main reason to keep the time origin the way it is is backwards compatibility, maybe we should keep the status quo (hiding only redirectStart and redirectEnd) and add it to the spec even though it doesn't provide any real cross-origin protection.

noamr · 2022-02-08T15:02:58Z

Updated PR to send the "has cross origin redirects" info to nav-timing, where the decision to hide redirectStart and redirectEnd is made (as it's an API decision rather than a real cross-origin issue)

In conjunction with w3c/navigation-timing#170

noamr · 2022-02-24T09:36:47Z

Closed by w3c/navigation-timing#170

noamr force-pushed the nav-timing-cors branch from 65b5fb1 to 27caec8 Compare September 23, 2021 13:11

whatwg deleted a comment Sep 25, 2021

noamr mentioned this pull request Sep 27, 2021

Test that we don't indirectly expose redirect timing web-platform-tests/wpt#30978

Closed

annevk reviewed Sep 28, 2021

View reviewed changes

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

source Outdated Show resolved Hide resolved

yoavweiss reviewed Sep 29, 2021

View reviewed changes

source Outdated Show resolved Hide resolved

noamr force-pushed the nav-timing-cors branch from 67257bd to 4dc9d8f Compare September 29, 2021 06:49

yoavweiss approved these changes Sep 29, 2021

View reviewed changes

This was referenced Sep 29, 2021

Better integration with ResourceTiming w3c/navigation-timing#117

Closed

Provide a TAO-bypass for Navigation timing w3c/resource-timing#220

Open

noamr mentioned this pull request Sep 30, 2021

Using navigationStart as a baseline may expose cross-origin timing information w3c/navigation-timing#160

Open

noamr force-pushed the nav-timing-cors branch from 4dc9d8f to dbbabf2 Compare January 17, 2022 10:59

noamr added 3 commits February 8, 2022 16:11

Fix nits

6bd3cd9

nit

6e5a67b

Removed extra character

a59b8c5

noamr force-pushed the nav-timing-cors branch from dbbabf2 to a59b8c5 Compare February 8, 2022 14:12

noamr mentioned this pull request Feb 8, 2022

Hide redirectStart and redirectEnd for navigations with cross-origin redirects w3c/navigation-timing#170

Merged

noamr closed this Feb 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide some navigation timing info when cross-origin redirects are present #7105

Hide some navigation timing info when cross-origin redirects are present #7105

noamr commented Sep 23, 2021 •

edited by pr-preview bot

domenic commented Sep 25, 2021

noamr commented Sep 26, 2021

annevk commented Sep 28, 2021

annevk left a comment

annevk commented Sep 28, 2021

yoavweiss commented Sep 29, 2021

noamr commented Sep 29, 2021

yoavweiss left a comment

noamr commented Sep 29, 2021

yoavweiss left a comment

noamr commented Sep 30, 2021 •

edited

annevk commented Sep 30, 2021

noamr commented Sep 30, 2021

noamr commented Oct 5, 2021

noamr commented Jan 17, 2022

noamr commented Feb 8, 2022

noamr commented Feb 8, 2022

noamr commented Feb 24, 2022 •

edited

Hide some navigation timing info when cross-origin redirects are present #7105

Hide some navigation timing info when cross-origin redirects are present #7105

Conversation

noamr commented Sep 23, 2021 • edited by pr-preview bot

domenic commented Sep 25, 2021

noamr commented Sep 26, 2021

annevk commented Sep 28, 2021

annevk left a comment

Choose a reason for hiding this comment

annevk commented Sep 28, 2021

yoavweiss commented Sep 29, 2021

noamr commented Sep 29, 2021

yoavweiss left a comment

Choose a reason for hiding this comment

noamr commented Sep 29, 2021

yoavweiss left a comment

Choose a reason for hiding this comment

noamr commented Sep 30, 2021 • edited

annevk commented Sep 30, 2021

noamr commented Sep 30, 2021

noamr commented Oct 5, 2021

noamr commented Jan 17, 2022

noamr commented Feb 8, 2022

noamr commented Feb 8, 2022

noamr commented Feb 24, 2022 • edited

noamr commented Sep 23, 2021 •

edited by pr-preview bot

noamr commented Sep 30, 2021 •

edited

noamr commented Feb 24, 2022 •

edited