From a8f6bbda85a0f53127aa7853db467d657376ec15 Mon Sep 17 00:00:00 2001 From: crapStone Date: Tue, 30 Apr 2024 23:18:10 +0200 Subject: [PATCH] fix leaf nil reference --- flake.lock | 6 ++-- server/certificates/certificates.go | 50 +++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/flake.lock b/flake.lock index b14ba48..f4fec7d 100644 --- a/flake.lock +++ b/flake.lock @@ -19,11 +19,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1714030708, - "narHash": "sha256-JOGPOxa8N6ySzB7SQBsh0OVz+UXZriyahgvfNHMIY0Y=", + "lastModified": 1714314149, + "narHash": "sha256-yNAevSKF4krRWacmLUsLK7D7PlfuY3zF0lYnGYNi9vQ=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "b0d52b31f7f4d80f8bf38f0253652125579c35ff", + "rev": "cf8cc1201be8bc71b7cbbbdaf349b22f4f99c7ae", "type": "github" }, "original": { diff --git a/server/certificates/certificates.go b/server/certificates/certificates.go index cf11078..4acde2e 100644 --- a/server/certificates/certificates.go +++ b/server/certificates/certificates.go @@ -37,7 +37,7 @@ func TLSConfig(mainDomainSuffix string, noDNS01 bool, rawDomain string, ) *tls.Config { - keyCache, err := lru.New[string, tls.Certificate](32) + keyCache, err := lru.New[string, *tls.Certificate](32) if err != nil { panic(err) // This should only happen if 32 < 0 at the time of writing, which should be reason enough to panic. } @@ -112,12 +112,21 @@ func TLSConfig(mainDomainSuffix string, } if tlsCertificate, ok := keyCache.Get(domain); ok { - if shouldRenewCert(&tlsCertificate, 7) { + if tlsCertificate.Leaf == nil { + log.Error().Msg("Leaf is nil") + tlsCertificate.Leaf, err = x509.ParseCertificate(tlsCertificate.Certificate[0]) + if err != nil { + return nil, fmt.Errorf("error parsing leaf tlsCert: %w", err) + } + } + // if Leaf == nil the certificate can't be in the cache for a long time so we just ignore it + if tlsCertificate.Leaf != nil && tlsCertificate.Leaf.NotAfter.Before(time.Now().Add(7*24*time.Hour)) { // if cert is up for renewal remove it from the cache keyCache.Remove(domain) } else { + log.Error().Msg("Cert is not expired") // we can use an existing certificate object - return &tlsCertificate, nil + return tlsCertificate, nil } } @@ -143,7 +152,7 @@ func TLSConfig(mainDomainSuffix string, } } - keyCache.Add(domain, *tlsCertificate) + keyCache.Add(domain, tlsCertificate) return tlsCertificate, nil }, NextProtos: []string{ @@ -195,13 +204,13 @@ func (c *AcmeClient) retrieveCertFromDB(sni, mainDomainSuffix string, useDnsProv // TODO: document & put into own function if !strings.EqualFold(sni, mainDomainSuffix) { - tlsCertificate.Leaf, err = x509.ParseCertificate(tlsCertificate.Certificate[0]) + tlsCertificate.Leaf, err = leaf(&tlsCertificate) if err != nil { - return nil, fmt.Errorf("error parsing leaf tlsCert: %w", err) + return nil, err } // renew certificates 7 days before they expire - if shouldRenewCert(&tlsCertificate, 7) { + if tlsCertificate.Leaf.NotAfter.Before(time.Now().Add(7 * 24 * time.Hour)) { // TODO: use ValidTill of custom cert struct if res.CSR != nil && len(res.CSR) > 0 { // CSR stores the time when the renewal shall be tried again @@ -237,6 +246,10 @@ func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew if err != nil { return nil, fmt.Errorf("certificate failed in synchronous request: %w", err) } + cert.Leaf, err = leaf(cert) + if err != nil { + return nil, err + } return cert, nil } defer c.obtainLocks.Delete(name) @@ -300,6 +313,7 @@ func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew } leaf, err := leaf(&tlsCertificate) if err == nil && leaf.NotAfter.After(time.Now()) { + tlsCertificate.Leaf = leaf // avoid sending a mock cert instead of a still valid cert, instead abuse CSR field to store time to try again at renew.CSR = []byte(strconv.FormatInt(time.Now().Add(6*time.Hour).Unix(), 10)) if err := keyDatabase.Put(name, renew); err != nil { @@ -323,6 +337,10 @@ func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew if err != nil { return nil, err } + tlsCertificate.Leaf, err = leaf(&tlsCertificate) + if err != nil { + return nil, err + } return &tlsCertificate, nil } @@ -343,11 +361,6 @@ func SetupMainDomainCertificates(mainDomainSuffix string, acmeClient *AcmeClient return nil } -// shouldRenewCert returns true if the validity date of the cert is less than the given number of days in the future -func shouldRenewCert(cert *tls.Certificate, days uint) bool { - return cert.Leaf.NotAfter.Before(time.Now().Add(time.Duration(days) * 24 * time.Hour)) -} - func MaintainCertDB(ctx context.Context, interval time.Duration, acmeClient *AcmeClient, mainDomainSuffix string, certDB database.CertDB) { for { // delete expired certs that will be invalid until next clean up @@ -402,11 +415,20 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, acmeClient *Acm } } -// leaf returns the parsed leaf certificate, either from c.leaf or by parsing +// leaf returns the parsed leaf certificate, either from c.Leaf or by parsing // the corresponding c.Certificate[0]. +// After successfuly parsing the cert c.Leaf gets set to the parsed cert. func leaf(c *tls.Certificate) (*x509.Certificate, error) { if c.Leaf != nil { return c.Leaf, nil } - return x509.ParseCertificate(c.Certificate[0]) + + leaf, err := x509.ParseCertificate(c.Certificate[0]) + if err != nil { + return nil, fmt.Errorf("tlsCert - failed to parse leaf: %w", err) + } + + c.Leaf = leaf + + return leaf, err }