PlzNavigate: Do not disclose urls between cross-origin renderers.

third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

Index: third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation b/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
index 33d80ad254bd88de907fbd771918c82b74d9ab2a..30927f0f97b0a02137cb68ed2faff3ebf4662457 100644
--- a/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
+++ b/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
@@ -18,3 +18,13 @@ Bug(none) virtual/mojo-loading/http/tests/security/popup-allowed-by-sandbox-can-
 # Without PlzNavigate, the line number of the console message is missing.
 Bug(none) http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked.html [ Failure ]
 Bug(none) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/frame-src-redirect-blocked.html [ Failure ]
+
+# PlzNavigate: URLs are potentially disclosed across cross-origin renderers

[alexmos, 2017/05/10 22:33:08]

I'd also mention that this is about CSP violations somewhere in the comment.

[arthursonzogni, 2017/05/11 13:06:24]

Done.

+# Not to disclose |source_location| and/or |blocked url| between cross-origin

[alexmos, 2017/05/10 22:33:08]

nit: s/to disclose/disclosing/

[arthursonzogni, 2017/05/11 13:06:24]

Done.

+# renderers regresses the quality of error messages.

[alexmos, 2017/05/10 22:33:08]

Just so I understand why the error messages regress.  Is it because we now print
the origin rather than full URL in the expected console errors, because you also
changed |message| to use |safe_url| in ReportViolation?  Whereas on the renderer
side, we used to only strip the URL to the origin for CSP violation events, but
not for console messages, which were considered ok since they went to the
devtools process?  Nasko and I were discussing that we should just directly
print the console error from the browser process without passing it through the
renderer process, which would allow us to fix this -- perhaps it's worth filing
a bug to do this in a followup.

[arthursonzogni, 2017/05/11 13:06:24]

Here are the diff for the 3 tests that have regressed:

###[ form-action-src-get-blocked-with-redirect.html ]###
-CONSOLE ERROR: Refused to send form data to
'http://localhost:8000/navigation/resources/form-target.pl' because it violates
the following Content Security Policy directive: "form-action 127.0.0.1:8000".
+CONSOLE ERROR: Refused to send form data to 'http://localhost:8000/' because it
violates the following Content Security Policy directive: "form-action
127.0.0.1:8000".

###[ form-action-src-redirect-blocked.html ]###
-CONSOLE ERROR: Refused to send form data to
'http://localhost:8000/navigation/resources/form-target.pl' because it violates
the following Content Security Policy directive: "form-action 127.0.0.1:8000".
+CONSOLE ERROR: Refused to send form data to 'http://localhost:8000/' because it
violates the following Content Security Policy directive: "form-action
127.0.0.1:8000".

###[ frame-src-child-frame-navigates-to-blocked-origin.html ] ###
+ FAIL The frame's navigation is blocked. assert_equals: This shouldn't actually
be 7, since it happens in a cross-origin frame. :/ expected 7 but got 0

For the first two, it is because we print the origin instead of the full url in
console error messages.
For the last one, it is because the source_location line number has been
removed. By looking at the it more closely, it looks like what we have with
PlzNavigate is better :-)

There is one more test that has regressed:
###[ http/tests/misc/onload-detach-during-csp-frame-src-none.html ]###
-CONSOLE ERROR: Refused to frame 'data:,hello' because it violates the following
Content Security Policy directive: "frame-src 'none'".
+CONSOLE ERROR: Refused to frame '' because it violates the following Content
Security Policy directive: "frame-src 'none'".

I don't really know what to think of this one.

[alexmos, 2017/05/12 01:37:20]

Hmm, this raises an interesting point, and perhaps another thing for Mike to
comment on.  The current stripping logic in Blink's StripURLForUseInReport, if I
read it correctly, only applies to frame-src and object-src, whereas this CL
will also apply it to form-action.  Do we actually need to strip URLs for
form-action reports (where this isn't really an information leak from one
renderer to another)?
Is last one because we strip the data URL down to an origin, which is a unique
origin?  Though then I would've expected it to print as "null".

[arthursonzogni, 2017/05/15 12:20:46]

Yes, it is because the data URL origin is an unique origin.

+Bug(718942) http/tests/security/contentSecurityPolicy/1.1/form-action-src-get-blocked-with-redirect.html [ Failure ]
+Bug(718942) http/tests/security/contentSecurityPolicy/1.1/form-action-src-redirect-blocked.html [ Failure ]
+Bug(718942) http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin.html [ Failure ]
+Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/1.1/form-action-src-get-blocked-with-redirect.html [ Failure ]
+Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/1.1/form-action-src-redirect-blocked.html [ Failure ]
+Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/frame-src-child-frame-navigates-to-blocked-origin.html [ Failure ]