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

checkocsp: verify validity period of signing certificate #4852

Merged
merged 5 commits into from Jun 8, 2020
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 45 additions & 0 deletions test/ocsp/helper/helper.go
Expand Up @@ -197,6 +197,36 @@ func getOCSPURL(cert *x509.Certificate) (*url.URL, error) {
return ocspURL, nil
}

// checkSignerTimes checks that the OCSP response is within the
// validity window of whichever certificate signed it, and that that
// certificate is currently valid.
func checkSignerTimes(resp *ocsp.Response, issuer *x509.Certificate) error {
var ocspSigner = issuer
if delegatedSigner := resp.Certificate; delegatedSigner != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x/crypto/ocsp already does this signature checking for us, I think all we need to do is check that the certificate is still valid.

ocspSigner = delegatedSigner

fmt.Printf("Using delegated OCSP signer from response: %s\n",
base64.StdEncoding.EncodeToString(ocspSigner.Raw))
}

if resp.NextUpdate.After(ocspSigner.NotAfter) {
return fmt.Errorf("OCSP response is valid longer than OCSP signer (%s): %s is after %s",
ocspSigner.Subject, resp.NextUpdate, ocspSigner.NotAfter)
}
if resp.ThisUpdate.Before(ocspSigner.NotBefore) {
return fmt.Errorf("OCSP response's validity begins before the OCSP signer's (%s): %s is before %s",
ocspSigner.Subject, resp.ThisUpdate, ocspSigner.NotBefore)
}

if time.Now().After(ocspSigner.NotAfter) {
return fmt.Errorf("OCSP signer (%s) expired at %s", ocspSigner.Subject, ocspSigner.NotAfter)
}
if time.Now().Before(ocspSigner.NotBefore) {
return fmt.Errorf("OCSP signer (%s) not valid until %s", ocspSigner.NotBefore)
}
return nil
}

func parseAndPrint(respBytes []byte, cert, issuer *x509.Certificate, expectStatus int) (*ocsp.Response, error) {
fmt.Printf("\nDecoding body: %s\n", base64.StdEncoding.EncodeToString(respBytes))
resp, err := ocsp.ParseResponseForCert(respBytes, cert, issuer)
Expand All @@ -211,6 +241,12 @@ func parseAndPrint(respBytes []byte, cert, issuer *x509.Certificate, expectStatu
if timeTilExpiry < tooSoonDuration {
return nil, fmt.Errorf("NextUpdate is too soon: %s", timeTilExpiry)
}

err = checkSignerTimes(resp, issuer)
if err != nil {
return nil, fmt.Errorf("checking signature on delegated signer: %s", err)
}

fmt.Printf("\n")
fmt.Printf("Good response:\n")
fmt.Printf(" CertStatus %d\n", resp.Status)
Expand All @@ -222,5 +258,14 @@ func parseAndPrint(respBytes []byte, cert, issuer *x509.Certificate, expectStatu
fmt.Printf(" RevocationReason %d\n", resp.RevocationReason)
fmt.Printf(" SignatureAlgorithm %s\n", resp.SignatureAlgorithm)
fmt.Printf(" Extensions %#v\n", resp.Extensions)
if resp.Certificate == nil {
fmt.Printf(" Certificate: nil\n")
} else {
fmt.Printf(" Certificate:\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extremely nit: Printf with no formatting directive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somewhat surprised we don;t get a vet error for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go vet actually does spot this, but we don't run go vet in Travis because it's too noisy, and it's explicitly documented as something not recommended for CI.

But thanks for the suggestion on running it- I actually found a bug on another line.

fmt.Printf(" Subject: %s\n", resp.Certificate.Subject)
fmt.Printf(" Issuer: %s\n", resp.Certificate.Issuer)
fmt.Printf(" NotBefore: %s\n", resp.Certificate.NotBefore)
fmt.Printf(" NotAfter: %s\n", resp.Certificate.NotAfter)
}
return resp, nil
}