NEL: Don't generate reports about insecure requests |
||||
Issue descriptionThe NEL spec requires that we only generate reports about requests to "Potentially Trustworthy" origins: https://w3c.github.io/network-error-logging/#generate-a-network-error-report Right now we're generating reports about *all* requests, even insecure HTTP ones. We're correctly ignoring any policies unless they come in via HTTPS; but if you receive a policy via HTTPS and then make an HTTP request to the same origin, we'll currently create a report about it. We might want to consider allowing that in the spec, but until then, we should make sure our implementation matches what the spec requires.
,
Nov 26
,
Nov 26
,
Nov 27
It looks like we create the report but it gets discarded and doesn't get queued: https://cs.chromium.org/chromium/src/net/network_error_logging/network_error_logging_service.cc?l=213 Am I missing something? Is it a problem that we create the report, even if it doesn't end up getting sent?
,
Nov 27
[chlily]: Looks to me like you're right. [dcreager]: Is there any more action that needs to be taken here?
,
Nov 27
I guess it would also make sense to move the check up a step and discard it in HttpNetworkTransaction::GenerateNetworkErrorLoggingReport() to avoid generating a report that we're not going to send anyway.
,
Nov 27
I agree that it seems inelegant to create a report that we will immediately drop, but if you drop the report in HttpNetworkTransaction the histogram count won’t get updated. Not sure how to square that. I’d be happy with either approach, as long as it’s well documented in NetworkErrorLoggingService what the expectations are. Re other actions: add a test case in http_network_transaction_unittest that makes an HTTPS request to install a NEL policy, and then an HTTP request to verify that no report is generated (and that the histogram count is updated).
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/00196ab622b5ea471c32703ff17926e035d66e53 commit 00196ab622b5ea471c32703ff17926e035d66e53 Author: Lily Chen <chlily@chromium.org> Date: Tue Dec 04 19:52:29 2018 NEL: Don't generate reports about insecure requests If a request is to an insecure origin, we don't queue a Reporting report for it anyway, so don't generate the report in the first place. TBR=holte@chromium.org Bug: 903893 Change-Id: Iec27462db5a4483821338570e7630122150ba2c2 Reviewed-on: https://chromium-review.googlesource.com/c/1352671 Commit-Queue: Lily Chen <chlily@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#613657} [modify] https://crrev.com/00196ab622b5ea471c32703ff17926e035d66e53/net/http/http_network_transaction.cc [modify] https://crrev.com/00196ab622b5ea471c32703ff17926e035d66e53/net/http/http_network_transaction_unittest.cc [modify] https://crrev.com/00196ab622b5ea471c32703ff17926e035d66e53/net/network_error_logging/network_error_logging_service.cc [modify] https://crrev.com/00196ab622b5ea471c32703ff17926e035d66e53/net/network_error_logging/network_error_logging_service.h [modify] https://crrev.com/00196ab622b5ea471c32703ff17926e035d66e53/net/network_error_logging/network_error_logging_service_unittest.cc [modify] https://crrev.com/00196ab622b5ea471c32703ff17926e035d66e53/tools/metrics/histograms/histograms.xml
,
Dec 4
|
||||
►
Sign in to add a comment |
||||
Comment 1 by mmenke@chromium.org
, Nov 26Owner: ----
Status: Available (was: Assigned)