Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: crypto/x509: implement OCSP verifier #40017

Open
FiloSottile opened this issue Jul 3, 2020 · 6 comments
Open

proposal: crypto/x509: implement OCSP verifier #40017

FiloSottile opened this issue Jul 3, 2020 · 6 comments

Comments

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Jul 3, 2020

Although we have a golang.org/x/crypto/ocsp package, we don't in fact have an OCSP verifier. The existing package provides serialization and parsing, but not APIs for "get me an OCSP response for this certificate" and "given this certificate and this OCSP response tell me if it's revoked". (They are separate because you only want the latter when checking stapled responses.)

There is a lot of complexity, subtlety, and dark knowledge involved in OCSP unfortunately. Here are a few notes on things the verifier needs to do (from reading this thread):

  • check that the response is signed directly by the issuer (without needing the OCSP EKU) or that it's signed by a Delegated Responder issued directly by the issuer (with the OCSP EKU)
  • for Delegated Responders, not require the EKU to be enforced up the chain
  • check that the signer has the correct KeyUsage
  • for Delegated Responders, require them to be End Entity certificates (i.e. not a CA; this is an off-spec Mozilla check that protected them from the mess of CAs giving the OCSP EKU away to intermediates)
  • for Delegated Responders, maybe check that the id-pkix-ocsp-nocheck extension is present (this is a BR requirement, but if it's not an IETF requirement we might want to skip it)

There are definitely a lot more things to consider (for example, the id-pkix-ocsp-nocheck extension needs to be processed itself), the list above are just notes of things I learned from that one incident.

A difficult question is where to put the code, and how to surface it. We'll want to use it in crypto/tls for Must-Staple (#22274) but it feels like the wrong place for the code to live. An obvious answer would be golang.org/x/crypto/ocsp, but then we can't use it from crypto/x509 without an import loop. Should x509.VerifyOptions have an option to verify a stapled response? Probably, I feel like we'd regret doing OCSP verification separately from path building and certificate verification anyway. What about the API to fetch a response? Maybe that can stay in golang.org/x/crypto/ocsp, separating the concerns of obtaining responses and verifying them.

We should also look around the ecosystem, because surely someone had to implement this, and we should compare results.

@gopherbot gopherbot added this to the Proposal milestone Jul 3, 2020
@gopherbot gopherbot added the Proposal label Jul 3, 2020
@rolandshoemaker
Copy link
Contributor

@rolandshoemaker rolandshoemaker commented Jul 3, 2020

I think this would go a long way towards getting rid of the numerous traps x/crypto/ocsp currently presents users. It definitely feels like response/certificate validation should happen in Cerifitcate.Verify, we'll already have all the requisite information and it feels unlikely that users will want to check OCSP at some other time (either pre or post chain building).

For delegated responders it probably makes sense to ignore whether a delegated responder contains id-pkix-ocsp-nocheck, but it's something response fetching code would probably want to take into consideration. There is also the question of what to do if the EE being verified has id-pkix-ocsp-nocheck but the user provided an OCSP response to verify against (this is a weird corner case, since presumably if the CA put the ext in a leaf it wouldn't be generating an OCSP response for it, but I wouldn't put it past anyone).

Fetching code probably should live in x/crypto/ocsp (or a paired down version of the API could make its way into crypto/x509 as a way of dissuading users from using the more dangerous bits that lurk in x/crypto/ocsp entirely) since the request generation code is already mostly complete and all that would need to be added would be a wrapper around a http.Client which generates the request and extracts the responder URL from the AIA extension in the certificate to check.

There is a bit of a chicken and egg problem with the fetching code and the verification code. For generating the OCSP request you need the leaf issuer, which means you might want to do chain building before you request the OCSP response (otherwise you need to do some pre-processing to determine the issuer, or you just assume the second cert in any chain you were sent is the issuer...), but as previously mentioned it'd probably be nicer to be able to do OCSP verification during chain building. The other less desirable option would be to to add a new Certificate.VerifyOCSP (or something) method which only verifies the OCSP response, so you could do Verify chain -> Fetch OCSP -> Verify OCSP...

@vanbroup
Copy link
Contributor

@vanbroup vanbroup commented Jul 3, 2020

I vote to implement this in Cerifitcate.Verify during the chain building as @rolandshoemaker suggested. If you would go for something like Certificate.VerifyOCSP at some point you would get Certificate.VerifyCRL as not all PKI is based on OCSP.

Ideally VerifyOptions would have an option CheckStatus or similar to let it check the current certificate status either using OCSP or CRL whatever is available in the certificate.

The id-pkix-ocsp-nocheck should be taken into account by the OCSP client when verifying the response but can be ignored when validating certificates of the chain itself. This because, if the responder is using a CA signed response, it should not include the id-pkix-ocsp-nocheck extension and the CA certificate who signed the response should be checked for revocation with it's issuer.

We might also want to cache the response according to the NotAfter, Expires or Cache-Control information.

@sleevi
Copy link

@sleevi sleevi commented Jul 6, 2020

There is a bit of a chicken and egg problem with the fetching code and the verification code. For generating the OCSP request you need the leaf issuer, which means you might want to do chain building before you request the OCSP response (otherwise you need to do some pre-processing to determine the issuer, or you just assume the second cert in any chain you were sent is the issuer...), but as previously mentioned it'd probably be nicer to be able to do OCSP verification during chain building. The other less desirable option would be to to add a new Certificate.VerifyOCSP (or something) method which only verifies the OCSP response, so you could do Verify chain -> Fetch OCSP -> Verify OCSP...

Right, as Roland suggests, doing it during chain building is the 'desired' implementation.

RFC 4158, Section 3.5.9 discusses some of the trade-offs here. Note that, depending on how you want to handle nocheck, revocation is potentially ideally implemented via a 'reverse' path search. That is, as reflected in RFC 5280, 6.1.3, you revocation check the root, then the intermediate, then the leaf. If, during revocation checking the leaf, you encounter a delegated responder for the intermediate, you can either check just that responder (with the intermediate) or via CRL. Since you can get into responder bounces, you probably have to have a limit, or force a fallback to CRL, or just refuse anything with nocheck (as it seems Microsoft is doing, as best I can tell)

That is, if you have

Intermediate ----> Responder 1
              \--> Responder 2
               \-> Responder 3
                \> Server

It's possible for Server to be responded by Responder 1, who when you check status (i.e. it lacks nocheck), gets a response from Responder 2, which also lacks nocheck and gets a response from Responder 3... etc. So either requiring nocheck or profiling that down seems sensible, or requiring a CRL if nocheck isn't present.

@rolandshoemaker
Copy link
Contributor

@rolandshoemaker rolandshoemaker commented Jul 6, 2020

It feels like there are two different paths we can go down here, implementing verification with out-of-band fetching (i.e. verifying using a stapled, or otherwise fetched, response) or verification with in-band fetching (i.e. fetching responses during verification) These can share a good bit of code (basically the actual verification logic) but have different implementation complexities.

It seems like probably the path with the least friction, for now anyway, is implementation out-of-band verification first and then deciding on the more complex problem of adding in-band verification once that is complete.

@tialaramex
Copy link

@tialaramex tialaramex commented Jul 7, 2020

Fetching OCSP can introduce both privacy and usability surprises. Privacy: Your client is now talking to somebody unexpected (based on an HTTP URL in the AIA OCSP item) and it's telling them which certificate you're currently interested in. It is obliged to use plaintext HTTP for this transaction and although the answers should be signed (so a MITM can't fake the response) the query can be read by anybody on the path.

Usability: Sometimes (too often) OCSP servers aren't reachable. Sometimes this is because they are on the Internet and you can't reach some or all of the public Internet from where you are. Sometimes though it's just the OCSP server is down.

In contrast the Stapled scenario is pretty much worry free.

As a result I'd strongly urge that any opt-in / opt-out type behaviour focuses on these network behaviours (whether to fetch stuff, over HTTP, from some server we discovered in the AIA records) not on disabling / enabling OCSP checking itself.

In particular it's desirable that on the one hand a quiet piece of software doesn't end up broadcasting its existence to the world because it saw some random Let's Encrypt certificate, while on the other hand a certificate with OCSP-must-staple set is always rejected by a Go program if that OCSP response is missing.

@FiloSottile
Copy link
Member Author

@FiloSottile FiloSottile commented Jul 7, 2020

Yes, however we end up structuring the API, a verification by default will not contact the network.

I'd still like to put fetching in a separate package, but it sounds like it would require a chain, and the chain comes from Verify, but the response would be fed to Verify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.