From 8eefeb6788efef16fb5944b22b78ea7286cd9eab Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 9 Aug 2016 16:12:22 -0600 Subject: [PATCH] Begin improved OCSP stapling by persisting staple to disk --- caddytls/certificates.go | 13 ++----- caddytls/crypto.go | 82 +++++++++++++++++++++++++++++++++++++--- caddytls/maintain.go | 25 +++++++----- 3 files changed, 95 insertions(+), 25 deletions(-) diff --git a/caddytls/certificates.go b/caddytls/certificates.go index d057a5e6b..e6d5510d3 100644 --- a/caddytls/certificates.go +++ b/caddytls/certificates.go @@ -10,7 +10,6 @@ import ( "sync" "time" - "github.com/xenolf/lego/acme" "golang.org/x/crypto/ocsp" ) @@ -183,19 +182,13 @@ func makeCertificate(certPEMBlock, keyPEMBlock []byte) (Certificate, error) { } } cert.NotAfter = leaf.NotAfter + cert.Certificate = tlsCert - // Staple OCSP - ocspBytes, ocspResp, err := acme.GetOCSPForCert(certPEMBlock) + err = stapleOCSP(&cert, certPEMBlock) if err != nil { - // An error here is not a problem because a certificate may simply - // not contain a link to an OCSP server. But we should log it anyway. - log.Printf("[WARNING] No OCSP stapling for %v: %v", cert.Names, err) - } else if ocspResp.Status == ocsp.Good { - tlsCert.OCSPStaple = ocspBytes - cert.OCSP = ocspResp + log.Printf("[WARNING] Stapling OCSP: %v", err) } - cert.Certificate = tlsCert return cert, nil } diff --git a/caddytls/crypto.go b/caddytls/crypto.go index 04ee226c2..0f371fa3b 100644 --- a/caddytls/crypto.go +++ b/caddytls/crypto.go @@ -13,11 +13,19 @@ import ( "encoding/pem" "errors" "fmt" + "hash/fnv" "io" + "io/ioutil" + "log" "math/big" "net" + "os" + "path/filepath" "time" + "golang.org/x/crypto/ocsp" + + "github.com/mholt/caddy" "github.com/xenolf/lego/acme" ) @@ -59,7 +67,7 @@ func savePrivateKey(key crypto.PrivateKey) ([]byte, error) { // stapleOCSP staples OCSP information to cert for hostname name. // If you have it handy, you should pass in the PEM-encoded certificate // bundle; otherwise the DER-encoded cert will have to be PEM-encoded. -// If you don't have the PEM blocks handy, just pass in nil. +// If you don't have the PEM blocks already, just pass in nil. // // Errors here are not necessarily fatal, it could just be that the // certificate doesn't have an issuer URL. @@ -73,13 +81,66 @@ func stapleOCSP(cert *Certificate, pemBundle []byte) error { pemBundle = bundle.Bytes() } - ocspBytes, ocspResp, err := acme.GetOCSPForCert(pemBundle) - if err != nil { - return err + var ocspBytes []byte + var ocspResp *ocsp.Response + var ocspErr error + var gotNewOCSP bool + + // First try to load OCSP staple from storage and see if + // we can still use it. + // TODO: Use Storage interface instead of disk directly + ocspFolder := filepath.Join(caddy.AssetsPath(), "ocsp") + ocspFileName := cert.Names[0] + "-" + fastHash(pemBundle) + ocspCachePath := filepath.Join(ocspFolder, ocspFileName) + cachedOCSP, err := ioutil.ReadFile(ocspCachePath) + if err == nil { + resp, err := ocsp.ParseResponse(cachedOCSP, nil) + if err == nil { + if freshOCSP(resp) { + // staple is still fresh; use it + ocspBytes = cachedOCSP + ocspResp = resp + } + } else { + // invalid contents; delete the file + err := os.Remove(ocspCachePath) + if err != nil { + log.Printf("[WARNING] Unable to delete invalid OCSP staple file: %v", err) + } + } } - cert.Certificate.OCSPStaple = ocspBytes - cert.OCSP = ocspResp + // If we couldn't get a fresh staple by reading the cache, + // then we need to request it from the OCSP responder + if ocspResp == nil || len(ocspBytes) == 0 { + ocspBytes, ocspResp, ocspErr = acme.GetOCSPForCert(pemBundle) + if ocspErr != nil { + // An error here is not a problem because a certificate may simply + // not contain a link to an OCSP server. But we should log it anyway. + // There's nothing else we can do to get OCSP for this certificate, + // so we can return here with the error. + return fmt.Errorf("no OCSP stapling for %v: %v", cert.Names, ocspErr) + } + gotNewOCSP = true + } + + // By now, we should have a response. If good, staple it to + // the certificate. If the OCSP response was not loaded from + // storage, we persist it for next time. + if ocspResp.Status == ocsp.Good { + cert.Certificate.OCSPStaple = ocspBytes + cert.OCSP = ocspResp + if gotNewOCSP { + err := os.MkdirAll(filepath.Join(caddy.AssetsPath(), "ocsp"), 0700) + if err != nil { + return fmt.Errorf("unable to make OCSP staple path for %v: %v", cert.Names, err) + } + err = ioutil.WriteFile(ocspCachePath, ocspBytes, 0644) + if err != nil { + return fmt.Errorf("unable to write OCSP staple file for %v: %v", cert.Names, err) + } + } + } return nil } @@ -235,6 +296,15 @@ func standaloneTLSTicketKeyRotation(c *tls.Config, ticker *time.Ticker, exitChan } } +// fastHash hashes input using a hashing algorithm that +// is fast, and returns the hash as a hex-encoded string. +// Do not use this for cryptographic purposes. +func fastHash(input []byte) string { + h := fnv.New32a() + h.Write([]byte(input)) + return fmt.Sprintf("%x", h.Sum32()) +} + const ( // NumTickets is how many tickets to hold and consider // to decrypt TLS sessions. diff --git a/caddytls/maintain.go b/caddytls/maintain.go index 4d01bf4ef..ec56aaef2 100644 --- a/caddytls/maintain.go +++ b/caddytls/maintain.go @@ -17,11 +17,11 @@ const ( // RenewInterval is how often to check certificates for renewal. RenewInterval = 12 * time.Hour - // OCSPInterval is how often to check if OCSP stapling needs updating. - OCSPInterval = 1 * time.Hour - // RenewDurationBefore is how long before expiration to renew certificates. RenewDurationBefore = (24 * time.Hour) * 30 + + // OCSPInterval is how often to check if OCSP stapling needs updating. + OCSPInterval = 1 * time.Hour ) // maintainAssets is a permanently-blocking function @@ -154,6 +154,10 @@ func RenewManagedCertificates(allowPrompts bool) (err error) { // UpdateOCSPStaples updates the OCSP stapling in all // eligible, cached certificates. +// +// OCSP maintenance strives to abide the relevant points on +// Ryan Sleevi's recommendations for good OCSP support: +// https://gist.github.com/sleevi/5efe9ef98961ecfb4da8 func UpdateOCSPStaples() { // Create a temporary place to store updates // until we release the potentially long-lived @@ -187,12 +191,9 @@ func UpdateOCSPStaples() { var lastNextUpdate time.Time if cert.OCSP != nil { - // start checking OCSP staple about halfway through validity period for good measure lastNextUpdate = cert.OCSP.NextUpdate - refreshTime := cert.OCSP.ThisUpdate.Add(lastNextUpdate.Sub(cert.OCSP.ThisUpdate) / 2) - - // since OCSP is already stapled, we need only check if we're in that "refresh window" - if time.Now().Before(refreshTime) { + if freshOCSP(cert.OCSP) { + // no need to update staple if ours is still fresh continue } } @@ -201,7 +202,7 @@ func UpdateOCSPStaples() { if err != nil { if cert.OCSP != nil { // if there was no staple before, that's fine; otherwise we should log the error - log.Printf("[ERROR] Checking OCSP for %v: %v", cert.Names, err) + log.Printf("[ERROR] Checking OCSP: %v", err) } continue } @@ -229,3 +230,9 @@ func UpdateOCSPStaples() { } certCacheMu.Unlock() } + +func freshOCSP(resp *ocsp.Response) bool { + // start checking OCSP staple about halfway through validity period for good measure + refreshTime := resp.ThisUpdate.Add(resp.NextUpdate.Sub(resp.ThisUpdate) / 2) + return time.Now().Before(refreshTime) +}