New issue
Advanced search Search tips

Issue 795086 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

QUIC ProofVerifierChromium should check the digitalSignature key usage bit

Project Member Reported by davidben@chromium.org, Dec 14 2017

Issue description

CertVerifier and X.509 stacks in general aren't the ones using the leaf certificate, ultimately, so they cannot easily check that the leaf certificate is suitable for how the caller intends to use it.

In particular, pre-Ed25519 key types in X.509 didn't encode crypto operations into the key type as they should have. Instead, RSA signing and RSA decryption keys share a key type, and ECDSA and ECDH keys share a key type.

Instead, there is a separate Key Usage extension that tells you whether you should accept signatures from this certificate (the digitalSignature bit for things like signing and keyEncipherment for static RSA).
https://tools.ietf.org/html/rfc5280#section-4.2.1.3

BoringSSL checks this by hand deep in the TLS stack when it extracts the leaf public key. Analogously, probably ProofVerifierChromium::Job::VerifySignature is the right place for QUIC. Perhaps we add X509Certificate::CheckKeyUsage(what_bit_I_need)?

This would prevent us from accidentally using, say, an ECDH certificate in QUIC. It's largely a theoretical concern, but something we should be checking.
 

Comment 1 by rch@chromium.org, Dec 18 2017

Fascinating! So many edge cases :)

With QUIC moving to TLS 1.3, I would imagine the cert verification will be performed by BoringSSL deep in the TLS stack. At that point, I suspect we won't need ProofVerifierChromium. Does that sound right?
Yeah, after TLS 1.3, the only thing QUIC should need is a CertVerifier and we'll take care of the key usage mess.

Comment 3 by rch@chromium.org, Dec 18 2017

Makes sense. With TLS 1.3 support coming "soonish" and this issue being theoretical, should we just wait for 1.3 or would you recommend doing this sooner? Either option is fine with me.
*shrug* Adding the check is easy enough to do, once there's some suitable function for you all to call (cert folks, do you have any thoughts?). Dunno how soon "soonish" is for QUIC.

Comment 5 by rch@chromium.org, Dec 19 2017

Owner: davidben@chromium.org
Status: Assigned (was: Untriaged)
Fair enough! Happy to make the necessary changes to ProofVerifierChromium once a function is ready. (Which I take it is not yet?)

In that case, I'll assigned this to davidben to figure out how to implement said method (or who to redirect to :>)? Please assign it back to me then, if that works...
Project Member

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

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

commit 03c28a41214b81d3f81ab1f449057bef6a5743ae
Author: David Benjamin <davidben@chromium.org>
Date: Fri May 11 23:12:07 2018

Check keyUsage bits in QUIC and Web Packaging..

This introduces a wrapper for SignatureVerifier when the public key
comes from a certificate.

Bug:  795086 , 803774
Change-Id: I177e4a2b3ea1b94afbac914c051be9f2b8b67985
Reviewed-on: https://chromium-review.googlesource.com/924349
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558053}
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/content/browser/web_package/signed_exchange_signature_verifier.cc
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/BUILD.gn
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/cert/x509_util.cc
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/cert/x509_util.h
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/cert/x509_util_unittest.cc
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/README
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_p256.key
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_p256_both.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_p256_digitalsignature.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_p256_keyagreement.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_p256_no_extension.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_rsa.key
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_rsa_both.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_rsa_digitalsignature.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_rsa_keyencipherment.pem
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/certificates/key_usage_rsa_no_extension.pem
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/scripts/ee.cnf
[add] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/data/ssl/scripts/generate-key-usage-certs.sh
[modify] https://crrev.com/03c28a41214b81d3f81ab1f449057bef6a5743ae/net/quic/chromium/crypto/proof_verifier_chromium.cc

Status: Fixed (was: Assigned)
Labels: M-68

Sign in to add a comment