diff --git a/caddyfile/parse.go b/caddyfile/parse.go index c0ec2d4c..3b06e8b8 100644 --- a/caddyfile/parse.go +++ b/caddyfile/parse.go @@ -100,10 +100,9 @@ func (p *parser) begin() error { if p.definedMacros == nil { p.definedMacros = map[string][]Token{} } - if p.definedMacros[name] != nil { - p.Errf("redeclaration of previously declared macro %s", name) + if _, found := p.definedMacros[name]; found { + return p.Errf("redeclaration of previously declared macro %s", name) } - // consume all tokens til matched close brace tokens, err := p.macroTokens() if err != nil { diff --git a/caddytls/certificates.go b/caddytls/certificates.go index 2f24e884..05af914f 100644 --- a/caddytls/certificates.go +++ b/caddytls/certificates.go @@ -128,8 +128,10 @@ func (cfg *Config) CacheManagedCertificate(domain string) (Certificate, error) { // cacheUnmanagedCertificatePEMFile loads a certificate for host using certFile // and keyFile, which must be in PEM format. It stores the certificate in -// memory. The Managed and OnDemand flags of the certificate will be set to -// false. +// memory after evicting any other entries in the cache keyed by the names +// on this certificate. In other words, it replaces existing certificates keyed +// by the names on this certificate. The Managed and OnDemand flags of the +// certificate will be set to false. // // This function is safe for concurrent use. func cacheUnmanagedCertificatePEMFile(certFile, keyFile string) error { @@ -137,6 +139,16 @@ func cacheUnmanagedCertificatePEMFile(certFile, keyFile string) error { if err != nil { return err } + + // since this is manually managed, this call might be part of a reload after + // the owner renewed a certificate; so clear cache of any previous cert first, + // otherwise the renewed certificate may never be loaded + certCacheMu.Lock() + for _, name := range cert.Names { + delete(certCache, name) + } + certCacheMu.Unlock() + cacheCertificate(cert) return nil } diff --git a/caddytls/crypto.go b/caddytls/crypto.go index 3ebc4be4..3036834c 100644 --- a/caddytls/crypto.go +++ b/caddytls/crypto.go @@ -151,6 +151,13 @@ func stapleOCSP(cert *Certificate, pemBundle []byte) error { // the certificate. If the OCSP response was not loaded from // storage, we persist it for next time. if ocspResp.Status == ocsp.Good { + if ocspResp.NextUpdate.After(cert.NotAfter) { + // uh oh, this OCSP response expires AFTER the certificate does, that's kinda bogus. + // it was the reason a lot of Symantec-validated sites (not Caddy) went down + // in October 2017. https://twitter.com/mattiasgeniar/status/919432824708648961 + return fmt.Errorf("invalid: OCSP response for %v valid after certificate expiration (%s)", + cert.Names, cert.NotAfter.Sub(ocspResp.NextUpdate)) + } cert.Certificate.OCSPStaple = ocspBytes cert.OCSP = ocspResp if gotNewOCSP { diff --git a/caddytls/maintain.go b/caddytls/maintain.go index a657f0c7..9e42fc87 100644 --- a/caddytls/maintain.go +++ b/caddytls/maintain.go @@ -334,8 +334,15 @@ func DeleteOldStapleFiles() { // meaning that it is not expedient to get an // updated response from the OCSP server. func freshOCSP(resp *ocsp.Response) bool { + nextUpdate := resp.NextUpdate + // If there is an OCSP responder certificate, and it expires before the + // OCSP response, use its expiration date as the end of the OCSP + // response's validity period. + if resp.Certificate != nil && resp.Certificate.NotAfter.Before(nextUpdate) { + nextUpdate = resp.Certificate.NotAfter + } // start checking OCSP staple about halfway through validity period for good measure - refreshTime := resp.ThisUpdate.Add(resp.NextUpdate.Sub(resp.ThisUpdate) / 2) + refreshTime := resp.ThisUpdate.Add(nextUpdate.Sub(resp.ThisUpdate) / 2) return time.Now().Before(refreshTime) }