-
Notifications
You must be signed in to change notification settings - Fork 478
fix: auto-renew TLS certificates without restart #1337
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
Changes from 1 commit
b9a3549
f440273
5ad1e09
f94dfd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/rand" | ||
| "crypto/tls" | ||
| "encoding/hex" | ||
|
|
@@ -17,6 +18,7 @@ import ( | |
|
|
||
| _ "net/http/pprof" | ||
|
|
||
| "github.com/caddyserver/certmagic" | ||
| "github.com/projectdiscovery/goflags" | ||
| "github.com/projectdiscovery/gologger" | ||
| "github.com/projectdiscovery/gologger/levels" | ||
|
|
@@ -293,34 +295,33 @@ func main() { | |
| ) | ||
| switch { | ||
| case cliOptions.CertificatePath != "" && cliOptions.PrivateKeyPath != "": | ||
| var domain string | ||
| if len(cliOptions.Domains) > 0 { | ||
| domain = cliOptions.Domains[0] | ||
| } | ||
| acmeManagerTLS, acmeErr := acme.BuildTlsConfigWithCertAndKeyPaths(cliOptions.CertificatePath, cliOptions.PrivateKeyPath, domain) | ||
| if acmeErr != nil { | ||
| gologger.Error().Msgf("https will be disabled: %s", acmeErr) | ||
| reloader, reloaderErr := acme.NewCertReloader(cliOptions.CertificatePath, cliOptions.PrivateKeyPath) | ||
| if reloaderErr != nil { | ||
| gologger.Error().Msgf("https will be disabled: %s", reloaderErr) | ||
| } else { | ||
| tlsConfig = acmeManagerTLS | ||
| go reloader.Start(context.Background()) | ||
| tlsConfig = &tls.Config{ | ||
| GetCertificate: reloader.GetCertificate, | ||
| NextProtos: []string{"h2", "http/1.1"}, | ||
| } | ||
| } | ||
| case !cliOptions.SkipAcme && len(cliOptions.Domains) > 0: | ||
| var certs []tls.Certificate | ||
| var cmCfg *certmagic.Config | ||
| for idx, domain := range cliOptions.Domains { | ||
| trimmedDomain := strings.TrimSuffix(domain, ".") | ||
| hostmaster := serverOptions.Hostmasters[idx] | ||
| var acmeErr error | ||
| domainCerts, certFiles, acmeErr = acme.HandleWildcardCertificates(fmt.Sprintf("*.%s", trimmedDomain), hostmaster, acmeStore, cliOptions.Debug, cliOptions.Resolvers) | ||
| domainCerts, certFiles, cmCfg, acmeErr = acme.HandleWildcardCertificates(fmt.Sprintf("*.%s", trimmedDomain), hostmaster, acmeStore, cliOptions.Debug, cliOptions.Resolvers) | ||
| if acmeErr != nil { | ||
|
ShubhamRasal marked this conversation as resolved.
Outdated
|
||
| gologger.Error().Msgf("An error occurred while applying for a certificate, error: %v", acmeErr) | ||
| gologger.Error().Msgf("Could not generate certs for auto TLS, https will be disabled") | ||
| } else { | ||
| certs = append(certs, domainCerts...) | ||
| } | ||
| } | ||
| var tlsErr error | ||
| tlsConfig, tlsErr = acme.BuildTlsConfigWithCerts("", certs...) | ||
| if tlsErr != nil { | ||
| gologger.Error().Msgf("An error occurred while preparing tls configuration, error: %v", tlsErr) | ||
| if cmCfg != nil { | ||
| tlsConfig = &tls.Config{ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Missing TLS MinVersion in ACME configuration (CWE-327) — TLS configuration for ACME certificates does not specify MinVersion, allowing clients to negotiate TLS 1.2. While Go 1.22+ defaults to TLS 1.2 minimum, explicitly setting TLS 1.3 provides defense-in-depth. Suggested Fix |
||
| GetCertificate: cmCfg.GetCertificate, | ||
| NextProtos: []string{"h2", "http/1.1"}, | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| package acme | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "os" | ||
| "sync" | ||
| "sync/atomic" | ||
| "time" | ||
|
|
||
| "github.com/projectdiscovery/gologger" | ||
| ) | ||
|
|
||
| const defaultCertCheckInterval = 1 * time.Hour | ||
|
|
||
| func certCheckInterval() time.Duration { | ||
| if v := os.Getenv("CERT_CHECK_INTERVAL"); v != "" { | ||
| if d, err := time.ParseDuration(v); err == nil && d > 0 { | ||
| return d | ||
| } | ||
| gologger.Warning().Msgf("Invalid CERT_CHECK_INTERVAL %q, using default %s", v, defaultCertCheckInterval) | ||
| } | ||
| return defaultCertCheckInterval | ||
| } | ||
|
|
||
| // CertReloader watches a certificate/key file pair and reloads when files change on disk. | ||
| type CertReloader struct { | ||
| certPath string | ||
| keyPath string | ||
| cert atomic.Pointer[tls.Certificate] | ||
| modTime time.Time | ||
| mu sync.Mutex // protects reload; reads are lock-free via atomic | ||
| } | ||
|
|
||
| // NewCertReloader loads the initial certificate and returns a reloader. | ||
| func NewCertReloader(certPath, keyPath string) (*CertReloader, error) { | ||
| cert, err := tls.LoadX509KeyPair(certPath, keyPath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| modTime, _ := latestModTime(certPath, keyPath) | ||
|
|
||
| r := &CertReloader{ | ||
| certPath: certPath, | ||
| keyPath: keyPath, | ||
| modTime: modTime, | ||
| } | ||
| r.cert.Store(&cert) | ||
| return r, nil | ||
| } | ||
|
|
||
| // GetCertificate returns the current certificate. Safe for concurrent use. | ||
| func (r *CertReloader) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { | ||
| return r.cert.Load(), nil | ||
| } | ||
|
|
||
| // Start polls for certificate file changes and reloads when detected. | ||
| // Blocks until ctx is cancelled. | ||
| // The check interval is configurable via the CERT_CHECK_INTERVAL env variable (e.g. "30m", "2h"). | ||
| func (r *CertReloader) Start(ctx context.Context) { | ||
| interval := certCheckInterval() | ||
| gologger.Info().Msgf("Certificate reload check interval: %s", interval) | ||
| ticker := time.NewTicker(interval) | ||
| defer ticker.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-ticker.C: | ||
| r.tryReload() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (r *CertReloader) tryReload() { | ||
| r.mu.Lock() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the |
||
| defer r.mu.Unlock() | ||
|
|
||
| mt, err := latestModTime(r.certPath, r.keyPath) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 TOCTOU vulnerability in certificate reload mechanism (CWE-367) — The tryReload() function has a time-of-check-time-of-use (TOCTOU) race condition. It checks file modification time at line 81, then loads the certificate at line 90. An attacker with filesystem access could replace the certificate files with symlinks between these operations, causing the server to load certificates from an attacker-controlled location. Attack ExampleSuggested Fix |
||
| if err != nil { | ||
| gologger.Warning().Msgf("Could not stat certificate files: %s", err) | ||
| return | ||
| } | ||
| if !mt.After(r.modTime) { | ||
| return | ||
| } | ||
|
|
||
| cert, err := tls.LoadX509KeyPair(r.certPath, r.keyPath) | ||
| if err != nil { | ||
| gologger.Warning().Msgf("Could not reload certificate: %s", err) | ||
| return | ||
| } | ||
|
|
||
| r.cert.Store(&cert) | ||
| r.modTime = mt | ||
| gologger.Info().Msgf("Reloaded TLS certificate from %s", r.certPath) | ||
| } | ||
|
|
||
| // latestModTime returns the most recent modification time of the two files. | ||
| func latestModTime(certPath, keyPath string) (time.Time, error) { | ||
| certInfo, err := os.Stat(certPath) | ||
| if err != nil { | ||
| return time.Time{}, err | ||
| } | ||
| keyInfo, err := os.Stat(keyPath) | ||
| if err != nil { | ||
| return time.Time{}, err | ||
| } | ||
| if keyInfo.ModTime().After(certInfo.ModTime()) { | ||
| return keyInfo.ModTime(), nil | ||
| } | ||
| return certInfo.ModTime(), nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Missing TLS MinVersion allows downgrade to TLS 1.2 (CWE-327) — TLS configuration for custom certificates does not specify MinVersion, allowing clients to negotiate TLS 1.2. While Go 1.22+ defaults to TLS 1.2 minimum, explicitly setting TLS 1.3 provides defense-in-depth against future changes and protocol downgrade attacks.
Suggested Fix