New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 815024 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 828447

Blocking:
issue 803774



Sign in to add a comment

Make SignedExchangeHandler support OCSP

Project Member Reported by horo@chromium.org, Feb 23 2018

Issue description

Make SignedExchangeHandler support OCSP
 

Comment 1 by horo@chromium.org, Feb 23 2018

Blocking: 803774
Owner: ksakamoto@chromium.org
Status: Assigned (was: Untriaged)
I'm taking over this from horo@.
Blockedon: 828447
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018

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

commit 4c759a7fd5d97e906d672e813b754cb014aa5d0b
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Tue Apr 10 03:01:04 2018

Introduce SignedExchangeCertificateChain

In preparation to add support for the new cert format, this patch
introduces SignedExchangeCertificateChain class that contains all
information of a certificate chain. Certificate parsing code in
SignedExchangeCertFetcher is moved to this new class.

Pure refactoring, no behavior change.

Bug:  815024 , 815025 
Change-Id: Iacb592279c9fef7afb40cb303ef81eebb4be34a3
Reviewed-on: https://chromium-review.googlesource.com/1002339
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549400}
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/BUILD.gn
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_cert_fetcher.cc
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_cert_fetcher.h
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_cert_fetcher_unittest.cc
[add] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_certificate_chain.cc
[add] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_certificate_chain.h
[rename] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_certificate_chain_fuzzer.cc
[add] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_certificate_chain_unittest.cc
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_handler.cc
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_handler.h
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/browser/web_package/signed_exchange_handler_unittest.cc
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/test/BUILD.gn
[rename] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/test/data/fuzzer_corpus/signed_exchange_certificate_chain_data/1
[rename] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/test/data/fuzzer_corpus/signed_exchange_certificate_chain_data/2
[rename] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/test/data/fuzzer_corpus/signed_exchange_certificate_chain_data/3
[rename] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/test/data/fuzzer_corpus/signed_exchange_certificate_chain_data/4
[modify] https://crrev.com/4c759a7fd5d97e906d672e813b754cb014aa5d0b/content/test/fuzzer/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, May 14 2018

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

commit 3f83270002b069388219fcc00c1ee93c020dea76
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Mon May 14 03:08:08 2018

Add support for new signed-exchange cert chain format

This adds a parser for the CBOR certificate chain format defined in [1].
SignedExchangeCertificateChain::Parse() takes a version enum and selects
a parser to use. For now, the new parser is used only by tests.

[1] https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cert-chain-format

Bug:  815024 , 815025 
Change-Id: Ia554ad3d086dbecd20294bdb6db03f37b60d67d9
Reviewed-on: https://chromium-review.googlesource.com/1002412
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558182}
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_cert_fetcher.cc
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_certificate_chain.cc
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_certificate_chain.h
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_certificate_chain_fuzzer.cc
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_certificate_chain_unittest.cc
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_consts.h
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/browser/web_package/signed_exchange_handler_unittest.cc
[add] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/test/data/fuzzer_corpus/signed_exchange_certificate_chain_data/wildcard_example.org.public.pem.cbor
[modify] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/test/data/htxg/README
[add] https://crrev.com/3f83270002b069388219fcc00c1ee93c020dea76/content/test/data/htxg/wildcard_example.org.public.pem.cbor

Project Member

Comment 6 by bugdroid1@chromium.org, May 25 2018

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

commit 081d150e19d10f6f4bcd83f432ee73f8869f17d6
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Fri May 25 00:42:46 2018

Add OCSP check for Signed Exchange

After this patch, SignedExchangeHandler starts accepting
"application/signed-exchange;v=b1" content-type in addition to v=b0.
(But Accept-Header still advertises only v=b0.)

For b1 signed exchanges, OCSP response from cert chain is passed to
CertVerifier::Verify(), and signed exchange without valid OCSP response
is rejected.

For now, b1 has minimal test coverage, but when we drop b0 support after
M68 branch cut, we'll be able to convert existing tests to b1.

This also makes IgnoreErrorsCertVerifier set OCSP results if request has
a non-empty ocsp response. This allows LayoutTests to work.

Bug:  815024 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I611d68c1d4f26b1f97ea81f8f9f9b89ba3ad0d84
Reviewed-on: https://chromium-review.googlesource.com/1060933
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561709}
[modify] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/content/browser/web_package/signed_exchange_handler.cc
[modify] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/content/browser/web_package/signed_exchange_handler.h
[modify] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/content/browser/web_package/signed_exchange_handler_unittest.cc
[modify] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/services/network/ignore_errors_cert_verifier.cc
[add] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/WebKit/LayoutTests/http/tests/loading/htxg/htxg-location-b1.html
[add] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/WebKit/LayoutTests/http/tests/loading/htxg/resources/127.0.0.1.pem.cbor
[modify] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/WebKit/LayoutTests/http/tests/loading/htxg/resources/README.md
[add] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/WebKit/LayoutTests/http/tests/loading/htxg/resources/htxg-location.sxg
[add] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/WebKit/LayoutTests/virtual/htxg-origin-trial-with-network-service/http/tests/loading/htxg/htxg-location-b1-expected.txt
[add] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/WebKit/LayoutTests/virtual/htxg-origin-trial/http/tests/loading/htxg/htxg-location-b1-expected.txt
[modify] https://crrev.com/081d150e19d10f6f4bcd83f432ee73f8869f17d6/third_party/blink/tools/apache_config/mime.types

Project Member

Comment 7 by bugdroid1@chromium.org, May 25 2018

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

commit ae70ba5e012c667a074f964115e4b7d4e6848779
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Fri May 25 09:31:47 2018

Use ERR_INVALID_SIGNED_EXCHANGE for Signed Exchange OCSP failure

To distinguish the signed exchange error from network layer errors.

Bug:  815024 
Change-Id: I165b8d44938575c2c5092c4f0012c1f58122ba79
Reviewed-on: https://chromium-review.googlesource.com/1073182
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561816}
[modify] https://crrev.com/ae70ba5e012c667a074f964115e4b7d4e6848779/content/browser/web_package/signed_exchange_handler.cc
[modify] https://crrev.com/ae70ba5e012c667a074f964115e4b7d4e6848779/content/browser/web_package/signed_exchange_handler_unittest.cc

Cc: eroman@chromium.org
+cc eroman, since during https://chromium-review.googlesource.com/c/chromium/src/+/1116406 it came up that it would be useful to have on-the-fly OCSP generation in the EmbeddedTestServer
Labels: -Pri-3 SignedExchange-b1 Pri-2
I noticed that the DevTools support for SXG doesn't show the attached OCSP response (in the "ocsp" field of the cert-chain+cbor). It would be useful to show some parsed metadata from this blob, e.g. in the "View certificate" dialog, since I could imagine a few ways that web devs could make mistakes here. Does this seem like a reasonable idea? Is this the right bug for that or should I open a new one?
#c10:
That sounds like a good idea. Could you file a new bug?
After http://crrev.com/c/1205915 devtools says
 "OCSP check failed: OCSP Response was expired or not yet valid."
instead of
 "OCSP check failed. response status: 6, revocation status: 2".

net/ doesn't expose the parsed OCSP structure, so adding OCSP viewer in devtools will require some extra work. Maybe we can just show hexdump of the OCSP response so that developer can use "openssl ocsp -resp_text" to see its content?

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 5

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

commit d4296790c9d3671f48012bb7eff7b3681ed94ceb
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Wed Sep 05 08:06:04 2018

Signed Exchange: Show detailed OCSP error messages on devtools

Before this patch, devtools message for OCSP verification error was just
two integer status codes (OCSPVerifyResult and OCSPRevocationStatus enum
values). This patch converts these status codes to developer-friendly
error messages.

Bug:  815024 
Change-Id: Ide3593c61698f0bebe87f8c5ef7a8c69f55afebf
Reviewed-on: https://chromium-review.googlesource.com/1205915
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588793}
[modify] https://crrev.com/d4296790c9d3671f48012bb7eff7b3681ed94ceb/content/browser/web_package/signed_exchange_handler.cc
[modify] https://crrev.com/d4296790c9d3671f48012bb7eff7b3681ed94ceb/content/browser/web_package/signed_exchange_request_handler_browsertest.cc

Cc: twif...@chromium.org
#c12: That update may be enough, thanks. Would you still like me to file a new bug for additional debug info?

I also figured out a command-line way of parsing the OCSP response out of the CBOR using https://github.com/dflemstr/rq:

  curl -s path.to/certurl | rq -q -c 'get "[1].ocsp"' | tr -d \" | xxd -r -p | openssl ocsp -respin - -text -noverify

So the hexdump became less critical.
Wow that one-liner is nice. I'm thinking about adding a small tool to WICG/webpackage reference implementation that can dump / diagnose cert chains.

Probably we don't need a new bug. Thanks!

Status: Fixed (was: Assigned)
Closing this.

Re #c15, now WICG/webpackage has "dump-certurl" tool which can be used to diagnose cert errors.

Sign in to add a comment