New issue
Advanced search Search tips

Issue 598021 link

Starred by 8 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 572734



Sign in to add a comment

Reporting for OCSP stapling ("Expect-OCSP")

Project Member Reported by est...@chromium.org, Mar 25 2016

Issue description

Give sites a way to opt-in to receive reports when Chrome makes connections to them without valid stapled OCSP responses. This will allow site operators to get feedback from their OCSP pipelines to find/fix bugs and misconfigurations. This will also help us (Chrome) evaluate the pros and cons of implementing Must-Staple.
 

Comment 1 by est...@chromium.org, May 12 2016

Owner: dadrian@google.com
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 12 2016

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

commit 144744b5cced306a06d167f2e34610378f6336ea
Author: engedy <engedy@chromium.org>
Date: Tue Jul 12 11:18:01 2016

Revert of Add CheckOCSPDateValid() to net/cert/internal (patchset #13 id:240001 of https://codereview.chromium.org/2091103002/ )

Reason for revert:
Causes reliable test failures on Linux Tests (dbg)(1)(32):

EncodeValuesTest.EncodeTimeAfterTimeTMax
EncodeValuesTest.EncodeTimeFromBeforeWindows

Original issue's description:
> Add CheckOCSPDateValid() to net/cert/internal
>
> Intended to be used internally by certificate validation as part of OCSP
> validity checks and Expect-Staple.
>
> BUG= 598021 
>
> Committed: https://crrev.com/687cf9fffda1a7eb45c80a596fa4f9e3e524f0e4
> Cr-Commit-Position: refs/heads/master@{#404708}

TBR=rsleevi@chromium.org,svaldez@chromium.org,estark@chromium.org,dadrian@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 598021 

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

[modify] https://crrev.com/144744b5cced306a06d167f2e34610378f6336ea/net/cert/internal/parse_ocsp.cc
[modify] https://crrev.com/144744b5cced306a06d167f2e34610378f6336ea/net/cert/internal/parse_ocsp.h
[modify] https://crrev.com/144744b5cced306a06d167f2e34610378f6336ea/net/cert/internal/parse_ocsp_unittest.cc
[delete] https://crrev.com/8466c81e38957bf0075c87e94403d963904492cd/net/der/encode_values.cc
[delete] https://crrev.com/8466c81e38957bf0075c87e94403d963904492cd/net/der/encode_values.h
[delete] https://crrev.com/8466c81e38957bf0075c87e94403d963904492cd/net/der/encode_values_unittest.cc
[modify] https://crrev.com/144744b5cced306a06d167f2e34610378f6336ea/net/net.gypi

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13 2016

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

commit 3e45becd1da65b4a3ff379fb794cc652d496e07e
Author: dadrian <dadrian@google.com>
Date: Wed Jul 13 04:15:08 2016

Reland of Add CheckOCSPDateValid() to net/cert/internal (patchset #1 id:1 of https://codereview.chromium.org/2138413002/ )

Reason for revert:
Fix broken 32-bit tests

Original issue's description:
> Revert of Add CheckOCSPDateValid() to net/cert/internal (patchset #13 id:240001 of https://codereview.chromium.org/2091103002/ )
>
> Reason for revert:
> Causes reliable test failures on Linux Tests (dbg)(1)(32):
>
> EncodeValuesTest.EncodeTimeAfterTimeTMax
> EncodeValuesTest.EncodeTimeFromBeforeWindows
>
> Original issue's description:
> > Add CheckOCSPDateValid() to net/cert/internal
> >
> > Intended to be used internally by certificate validation as part of OCSP
> > validity checks and Expect-Staple.
> >
> > BUG= 598021 
> >
> > Committed: https://crrev.com/687cf9fffda1a7eb45c80a596fa4f9e3e524f0e4
> > Cr-Commit-Position: refs/heads/master@{#404708}
>
> TBR=rsleevi@chromium.org,svaldez@chromium.org,estark@chromium.org,dadrian@google.com
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 598021 
>
> Committed: https://crrev.com/144744b5cced306a06d167f2e34610378f6336ea
> Cr-Commit-Position: refs/heads/master@{#404802}

TBR=rsleevi@chromium.org,svaldez@chromium.org,estark@chromium.org,engedy@chromium.org
BUG= 598021 

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

[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/cert/internal/parse_ocsp.cc
[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/cert/internal/parse_ocsp.h
[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/cert/internal/parse_ocsp_unittest.cc
[add] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/der/encode_values.cc
[add] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/der/encode_values.h
[add] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/der/encode_values_unittest.cc
[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/net.gypi

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e45becd1da65b4a3ff379fb794cc652d496e07e

commit 3e45becd1da65b4a3ff379fb794cc652d496e07e
Author: dadrian <dadrian@google.com>
Date: Wed Jul 13 04:15:08 2016

Reland of Add CheckOCSPDateValid() to net/cert/internal (patchset #1 id:1 of https://codereview.chromium.org/2138413002/ )

Reason for revert:
Fix broken 32-bit tests

Original issue's description:
> Revert of Add CheckOCSPDateValid() to net/cert/internal (patchset #13 id:240001 of https://codereview.chromium.org/2091103002/ )
>
> Reason for revert:
> Causes reliable test failures on Linux Tests (dbg)(1)(32):
>
> EncodeValuesTest.EncodeTimeAfterTimeTMax
> EncodeValuesTest.EncodeTimeFromBeforeWindows
>
> Original issue's description:
> > Add CheckOCSPDateValid() to net/cert/internal
> >
> > Intended to be used internally by certificate validation as part of OCSP
> > validity checks and Expect-Staple.
> >
> > BUG= 598021 
> >
> > Committed: https://crrev.com/687cf9fffda1a7eb45c80a596fa4f9e3e524f0e4
> > Cr-Commit-Position: refs/heads/master@{#404708}
>
> TBR=rsleevi@chromium.org,svaldez@chromium.org,estark@chromium.org,dadrian@google.com
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 598021 
>
> Committed: https://crrev.com/144744b5cced306a06d167f2e34610378f6336ea
> Cr-Commit-Position: refs/heads/master@{#404802}

TBR=rsleevi@chromium.org,svaldez@chromium.org,estark@chromium.org,engedy@chromium.org
BUG= 598021 

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

[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/cert/internal/parse_ocsp.cc
[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/cert/internal/parse_ocsp.h
[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/cert/internal/parse_ocsp_unittest.cc
[add] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/der/encode_values.cc
[add] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/der/encode_values.h
[add] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/der/encode_values_unittest.cc
[modify] https://crrev.com/3e45becd1da65b4a3ff379fb794cc652d496e07e/net/net.gypi

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 20 2016

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

commit 612337a1d7cadc52d0217b9f399eb1fab445d3e2
Author: dadrian <dadrian@google.com>
Date: Wed Jul 20 22:36:58 2016

Extract OCSPCertStatus::Status to standalone OCSPRevocationStatus, and add OCSPVerifyResult for tracking stapled OCSP responses cross-platform. OCSPVerifyResult is populated by CertVerifyProc, but is currently unused. In the future, it will be consumed by Expect-Staple reports.

This CL also updates mini-CA and the spawned test server to be able to send a wider range of OCSP responses. Since OCSP responses are short lived, test the new functionality in url_request_unittest.cc and dynamically generate OCSP responses.

BUG= 598021 

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

[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/chrome/browser/chromeos/login/test/https_forwarder.py
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/cert_verify_proc.cc
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/cert_verify_result.cc
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/cert_verify_result.h
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/internal/parse_ocsp.cc
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/internal/parse_ocsp.h
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/internal/parse_ocsp_unittest.cc
[add] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/ocsp_revocation_status.h
[add] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/ocsp_verify_result.cc
[add] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/cert/ocsp_verify_result.h
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/net.gypi
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/ssl/ssl_info.cc
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/ssl/ssl_info.h
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/test/spawned_test_server/base_test_server.cc
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/test/spawned_test_server/base_test_server.h
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/tools/testserver/minica.py
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/tools/testserver/testserver.py
[modify] https://crrev.com/612337a1d7cadc52d0217b9f399eb1fab445d3e2/net/url_request/url_request_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 21 2016

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

commit c87b456d823a4bd61599c5ce4512848a6f2be7b6
Author: dadrian <dadrian@google.com>
Date: Thu Jul 21 02:10:09 2016

Add the ability to send Expect-Staple reports.

This adds a |ProcessExpectStaple| method to TransportSecurityState that
implements reporting for Expect-Staple, using contents of SSLInfo and
OCSPVerifyResult. No report is sent if a valid staple is provided, or if
the remote host is not on the Expect-Staple preload list. This method is
not currently called by any socket implementation.

BUG= 598021 

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

[modify] https://crrev.com/c87b456d823a4bd61599c5ce4512848a6f2be7b6/net/http/transport_security_state.cc
[modify] https://crrev.com/c87b456d823a4bd61599c5ce4512848a6f2be7b6/net/http/transport_security_state.h
[modify] https://crrev.com/c87b456d823a4bd61599c5ce4512848a6f2be7b6/net/http/transport_security_state_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 25 2016

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

commit 3c0e49240847ea54265e17c7a8a1ecf73daeaeee
Author: dadrian <dadrian@google.com>
Date: Mon Jul 25 21:17:03 2016

Enable Expect-Staple in SSLClientSocket.

In TransportSecurityState, set |enable_static_expect_staple_| to true by default. Update SSLClientSocket to call TransportSecurityState::ProcessExpectStaple.

In ssl_client_socket_impl.cc, this also removes the if (|signed_certificate_timestamps_enabled_) check around extracting the OCSP response and setting the UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled"). Since SCTs are always enabled, this if statement was a noop.

This does not enable Expect-Staple for QUIC. See https://crbug.com/631101

BUG= 598021 

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

[modify] https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee/net/http/transport_security_state.cc
[modify] https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee/net/quic/quic_crypto_client_stream.cc
[modify] https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee/net/url_request/url_request_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 25 2016

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

commit 5b3cfa2861a8c2d452111225fb5f95c7b561b50c
Author: mpearson <mpearson@chromium.org>
Date: Mon Jul 25 21:58:13 2016

Revert of Enable Expect-Staple in SSLClientSocket. (patchset #3 id:40001 of https://codereview.chromium.org/2155753002/ )

Reason for revert:
The newly added test HTTPSOCSPTest.ExpectStapleReportNotSentOnValid fails on the Android bot:
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20ARMv6%20Builder/builds/2675

C  165.045s Main  Detailed Logs
C  165.045s Main  ********************************************************************************
C  165.063s Main  [FAIL] HTTPSOCSPTest.ExpectStapleReportNotSentOnValid:
C  165.063s Main  [ RUN      ] HTTPSOCSPTest.ExpectStapleReportNotSentOnValid
C  165.063s Main  [WARNING:embedded_test_server.cc(193)] Request not handled. Returning 404: /
C  165.063s Main  ../../net/url_request/url_request_unittest.cc:9342: Failure
C  165.063s Main  Value of: mock_report_sender.latest_report().empty()
C  165.063s Main    Actual: false
C  165.064s Main  Expected: true
C  165.064s Main  ../../net/url_request/url_request_unittest.cc:9343: Failure
C  165.064s Main  Value of: mock_report_sender.latest_report_uri()
C  165.064s Main    Actual: https://report.badssl.com/expect-staple
C  165.064s Main  Expected: GURL()
C  165.064s Main  Which is:
C  165.064s Main  [  FAILED  ] HTTPSOCSPTest.ExpectStapleReportNotSentOnValid (65 ms)

Original issue's description:
> Enable Expect-Staple in SSLClientSocket.
>
> In TransportSecurityState, set |enable_static_expect_staple_|
> to true by default. Update SSLClientSocket to call
> TransportSecurityState::ProcessExpectStaple.
>
> In ssl_client_socket_impl.cc, this also removes the if
> (|signed_certificate_timestamps_enabled_) check around
> extracting the OCSP response and setting the
> UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled"). Since
> SCTs are always enabled, this if statement was a noop.
>
> This does not enable Expect-Staple for QUIC. See
> https://crbug.com/631101
>
> BUG= 598021 
>
> Committed: https://crrev.com/3c0e49240847ea54265e17c7a8a1ecf73daeaeee
> Cr-Commit-Position: refs/heads/master@{#407575}

TBR=svaldez@chromium.org,rsleevi@chromium.org,estark@chromium.org,dadrian@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 598021 

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

[modify] https://crrev.com/5b3cfa2861a8c2d452111225fb5f95c7b561b50c/net/http/transport_security_state.cc
[modify] https://crrev.com/5b3cfa2861a8c2d452111225fb5f95c7b561b50c/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/5b3cfa2861a8c2d452111225fb5f95c7b561b50c/net/quic/quic_crypto_client_stream.cc
[modify] https://crrev.com/5b3cfa2861a8c2d452111225fb5f95c7b561b50c/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/5b3cfa2861a8c2d452111225fb5f95c7b561b50c/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/5b3cfa2861a8c2d452111225fb5f95c7b561b50c/net/url_request/url_request_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 26 2016

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

commit d476e65cb521b5da146dbdd51bde4fafdaa37468
Author: dadrian <dadrian@google.com>
Date: Tue Jul 26 21:33:24 2016

Enable Expect-Staple in SSLClientSocket.

In TransportSecurityState, set |enable_static_expect_staple_|
to true by default. Update SSLClientSocket to call
TransportSecurityState::ProcessExpectStaple. Implements
operator== for OCSPVerifyResult, to make sure OCSP data
is not accidentally lost.

In ssl_client_socket_impl.cc, this also removes the if
(|signed_certificate_timestamps_enabled_) check around
extracting the OCSP response and setting the
UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled"). Since
SCTs are always enabled, this if statement was a noop.

This does not enable Expect-Staple for QUIC. See
https://crbug.com/631101

BUG= 598021 

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

[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/cert/cert_verify_result.cc
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/cert/ocsp_verify_result.cc
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/cert/ocsp_verify_result.h
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/http/transport_security_state.cc
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/quic/quic_crypto_client_stream.cc
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/d476e65cb521b5da146dbdd51bde4fafdaa37468/net/url_request/url_request_unittest.cc

Owner: est...@chromium.org
Blocking: 572734
estark@: Should/can this be closed as Fixed? It looks like this hit M-54, I wasn't sure if there was follow-up work encompassed by this bug.
Status: Fixed (was: Assigned)
Labels: M-54

Sign in to add a comment