New issue
Advanced search Search tips

Issue 615497 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Restrict Alternative Services to secure origins.

Project Member Reported by b...@chromium.org, May 27 2016

Issue description

Allowing Alternative Services for insecure origins might enable MitM attackers to inject response headers and potentially:
* redirect traffic permanently similarly to a DNS cache poisoning attack,
* redirect insecure scheme requests over a secure transport, and if there was a server bug in which sensitive information was transmitted over a secure transport even if the scheme was insecure, then the browser would have that information associated with the insecure request, and later may leak it over insecure transport.

It is not necessarily true that either of these attacks are possible, but allowing Alternative Services for insecure origins definitely makes it more complicated to reason about security issues.

Also see https://www.chromium.org/Home/chromium-security/prefer-secure-origins-for-powerful-new-features.

Note that the current implementation requires the Alternative Service protocol to be HTTP/2 or QUIC, so that is guaranteed to be secure.  It is the origin that is currently allowed to be insecure.
 

Comment 1 by b...@chromium.org, May 31 2016

Labels: -Pri-2 -M-53 M-52 Pri-1
Could we also initially restrict this to an origin trial?
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3dd56f4d98843611954b84fa28c1e884594fd05

commit e3dd56f4d98843611954b84fa28c1e884594fd05
Author: bnc <bnc@chromium.org>
Date: Wed Jun 01 10:37:11 2016

Disable AltSvc from insecure origin.

* Do not parse Alt-Svc response on insecure connections, add regression test.
* Do not follow in-memory Alt-Svc on insecure connections, add regression test.

This CL has a shortcut: |enable_alt_svc_for_insecure_origin|, which is false in
production but true in tests (except for the two regression tests).  Adapting
the existing tests is a lot of work, that will come in one or more follow-up
CLs.

BUG= 615497 

Review-Url: https://codereview.chromium.org/2026863002
Cr-Commit-Position: refs/heads/master@{#397097}

[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/http/http_network_session.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/http/http_network_session.h
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/http/http_network_transaction.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/http/http_stream_factory_impl.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/quic/quic_network_transaction_unittest.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/e3dd56f4d98843611954b84fa28c1e884594fd05/net/spdy/spdy_test_util_common.h

Project Member

Comment 4 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by b...@chromium.org, Jun 3 2016

Labels: -M-53 -MovedFrom-52 Merge-Request-52 M-52
Requesting merge for M52 (2743).  CL has been picked up by Canary 53.0.2756.0 one day ago.

Comment 6 by tin...@google.com, Jun 3 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 3 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44e83591c5ea7ee2eb03ac84ebb03cca574a900d

commit 44e83591c5ea7ee2eb03ac84ebb03cca574a900d
Author: Bence Béky <bnc@chromium.org>
Date: Fri Jun 03 17:46:09 2016

Disable AltSvc from insecure origin.

* Do not parse Alt-Svc response on insecure connections, add regression test.
* Do not follow in-memory Alt-Svc on insecure connections, add regression test.

This CL has a shortcut: |enable_alt_svc_for_insecure_origin|, which is false in
production but true in tests (except for the two regression tests).  Adapting
the existing tests is a lot of work, that will come in one or more follow-up
CLs.

TBR=rch@chromium.org

BUG= 615497 

Review-Url: https://codereview.chromium.org/2026863002
Cr-Commit-Position: refs/heads/master@{#397097}
(cherry picked from commit e3dd56f4d98843611954b84fa28c1e884594fd05)

Review URL: https://codereview.chromium.org/2030023005 .

Cr-Commit-Position: refs/branch-heads/2743@{#206}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/http/http_network_session.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/http/http_network_session.h
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/http/http_network_transaction.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/http/http_stream_factory_impl.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/quic/quic_network_transaction_unittest.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/44e83591c5ea7ee2eb03ac84ebb03cca574a900d/net/spdy/spdy_test_util_common.h

Comment 8 by b...@chromium.org, Jun 5 2016

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5b35f012b6d2e5c943cee9fad51e23a98ba1303

commit b5b35f012b6d2e5c943cee9fad51e23a98ba1303
Author: bnc <bnc@chromium.org>
Date: Thu Jun 30 03:49:20 2016

Do not load alternative service from disk for non-https servers.

Such records might exists from earlier installations that used to allow
alternative services for other schemes.  Includes regression test.

BUG= 615497 

Review-Url: https://codereview.chromium.org/2102973004
Cr-Commit-Position: refs/heads/master@{#403084}

[modify] https://crrev.com/b5b35f012b6d2e5c943cee9fad51e23a98ba1303/net/http/http_server_properties_manager.cc
[modify] https://crrev.com/b5b35f012b6d2e5c943cee9fad51e23a98ba1303/net/http/http_server_properties_manager.h
[modify] https://crrev.com/b5b35f012b6d2e5c943cee9fad51e23a98ba1303/net/http/http_server_properties_manager_unittest.cc

Sign in to add a comment