Open pboguslawski opened 3 years ago
/cc @FiloSottile
Another serious bug in OCSP response verification algo: ParseResponse from ocsp package does not check if embedded OCSP responder certificate is valid for signing OCSP responses; according to
https://tools.ietf.org/html/rfc6960#section-4.2.2.2
such cert must include id-kp-OCSPSigning in an extended key usage (not required only if response is signed by CA cert directly). Just checked that OCSP package in go1.15.2 linux/amd64 accepts OCSP response signed using simple personal certificate (no OCSPSigning EKU in this cert) signed by same CA.
OpenSSL in such scenario returns error like
OCSP routines:ocsp_check_delegated:missing ocspsigning usage:../crypto/ocsp/ocsp_vfy.c:329
Please do full embedded responder cert verification (i.e. expiration like above, EKU), not just signatures.
The following dirty workaround outside of ParseResponse works for us:
ocspResponse, err := ocsp.ParseResponse(httpResponseBody, issuer)
if err != nil {
# throw OCSP verification error
}
// Verify OCSP responder certificate if embedded in OCSP response
// (https://github.com/golang/go/issues/43522 workaround).
if ocspResponse.Certificate != nil {
// Verify chain up to issuer.
oscpResponderCACert := x509.NewCertPool()
oscpResponderCACert.AddCert(issuer)
opts := x509.VerifyOptions{
Roots: oscpResponderCACert,
KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageOCSPSigning},
}
if chains, err := ocspResponse.Certificate.Verify(opts); err == nil {
// 1 chain with 2 certs (leaf and issuer) should be returned
// on verification success; treat other results as an error.
if len(chains) != 1 {
# throw OCSP verification error
}
if len(chains[0]) != 2 {
# throw OCSP verification error
}
} else {
# throw OCSP verification error
}
}
Please verify and consider fixing inside ParseResponse routine.
As far as I can tell, this is a duplicate of #40017. ocsp.ParseResponse
simply parses an OCSP response, like x509.ParseCertificate
parses a certificate. We don't have an OCSP equivalent to x509.Verify
yet, which is where all chain and attribute verification would happen.
It is unfortunate the ocsp.ParseResponse
function does some signature verification, because indeed that makes it a pretty misleading API. I guess we could do the checks we can inside ocsp.ParseResponse
, but there is a lot involved in verifying an OCSP response, and I am not sure it will all fit in the ocsp.ParseResponse
API.
It is unfortunate the ocsp.ParseResponse function does some signature verification, because indeed that makes it a pretty misleading API.
Until OCSP Verify is implemented consider commenting ParseResponse to warn people that it is not doing full OCSP response verification just some elements of it. If you decide to implement separate Verify, consider calling it inside ParseResponse also to avoid loosing verification in existing apps that will use ParseResponse. If people see now that ParseResponse throws an error on sig verification, should not surprise them if other checks will be done as well in this method.
In case of OCSP reponse signed with embedded OCSP responder cert (not by CA cert directly) ParseResponse from ocsp package does not check if embedded OCSP responder certificate is expired.
It seems...
https://pkg.go.dev/golang.org/x/crypto/ocsp#ParseResponse https://github.com/golang/crypto/blob/master/ocsp/ocsp.go#L550
...only signatures are checked. This allows one to use old, expired OCSP responder certificate and its key to sign OCSP response and go application using ocsp.ParseResponse package will accept this response but should not (checked in go1.15.2 linux/amd64 but master sources seems to contain the same problem).
OpenSSL in such scenario throws
Response Verify Failure
error:Please see OpenSSL's OCSP response verification algo described on...
https://www.openssl.org/docs/man1.1.1/man1/ocsp.html#OCSP-Response-verification
...and do full responder cert verification, not just signatures.
Regards, Paweł