diff --git a/README.md b/README.md index 251e9d4..9493a45 100644 --- a/README.md +++ b/README.md @@ -522,7 +522,7 @@ To use ZeroSSL's API instead, use the [`ZeroSSLIssuer`](https://pkg.go.dev/githu magic := certmagic.NewDefault() magic.Issuers = []certmagic.Issuer{ - certmagic.NewZeroSSLIssuer(magic, certmagic.ZeroSSLIssuer{ + certmagic.ZeroSSLIssuer{ APIKey: "", }), } diff --git a/acmeclient.go b/acmeclient.go index ed904b9..1727f1e 100644 --- a/acmeclient.go +++ b/acmeclient.go @@ -161,8 +161,8 @@ func (iss *ACMEIssuer) newACMEClient(useTestCA bool) (*acmez.Client, error) { if err != nil { return nil, err } - if u.Scheme != "https" && !isLoopback(u.Host) && !isInternal(u.Host) { - return nil, fmt.Errorf("%s: insecure CA URL (HTTPS required)", caURL) + if u.Scheme != "https" && !SubjectIsInternal(u.Host) { + return nil, fmt.Errorf("%s: insecure CA URL (HTTPS required for non-internal CA)", caURL) } client := &acmez.Client{ diff --git a/acmeissuer.go b/acmeissuer.go index b5e837c..219e5a5 100644 --- a/acmeissuer.go +++ b/acmeissuer.go @@ -323,14 +323,32 @@ func (iss *ACMEIssuer) isAgreed() bool { // PreCheck performs a few simple checks before obtaining or // renewing a certificate with ACME, and returns whether this -// batch is eligible for certificates if using Let's Encrypt. -// It also ensures that an email address is available. +// batch is eligible for certificates. It also ensures that an +// email address is available if possible. +// +// IP certificates via ACME are defined in RFC 8738. func (am *ACMEIssuer) PreCheck(ctx context.Context, names []string, interactive bool) error { - publicCA := strings.Contains(am.CA, "api.letsencrypt.org") || strings.Contains(am.CA, "acme.zerossl.com") || strings.Contains(am.CA, "api.pki.goog") + publicCAsAndIPCerts := map[string]bool{ // map of public CAs to whether they support IP certificates (last updated: Q1 2024) + "api.letsencrypt.org": false, // https://community.letsencrypt.org/t/certificate-for-static-ip/84/2?u=mholt + "acme.zerossl.com": false, // only supported via their API, not ACME endpoint + "api.pki.goog": true, // https://pki.goog/faq/#faq-IPCerts + "api.buypass.com": false, // https://community.buypass.com/t/h7hm76w/buypass-support-for-rfc-8738 + "acme.ssl.com": false, + } + var publicCA, ipCertAllowed bool + for caSubstr, ipCert := range publicCAsAndIPCerts { + if strings.Contains(am.CA, caSubstr) { + publicCA, ipCertAllowed = true, ipCert + break + } + } if publicCA { for _, name := range names { if !SubjectQualifiesForPublicCert(name) { - return fmt.Errorf("subject does not qualify for a public certificate: %s", name) + return fmt.Errorf("subject '%s' does not qualify for a public certificate", name) + } + if !ipCertAllowed && SubjectIsIP(name) { + return fmt.Errorf("subject '%s' cannot have public IP certificate from %s (if CA's policy has changed, please notify the developers in an issue)", name, am.CA) } } } diff --git a/cache.go b/cache.go index 152fe18..2dc65c0 100644 --- a/cache.go +++ b/cache.go @@ -16,7 +16,7 @@ package certmagic import ( "fmt" - weakrand "math/rand" // seeded elsewhere + weakrand "math/rand" "strings" "sync" "time" diff --git a/certificates.go b/certificates.go index 7fea812..8540f23 100644 --- a/certificates.go +++ b/certificates.go @@ -386,8 +386,8 @@ func SubjectQualifiesForCert(subj string) bool { // SubjectQualifiesForPublicCert returns true if the subject // name appears eligible for automagic TLS with a public -// CA such as Let's Encrypt. For example: localhost and IP -// addresses are not eligible because we cannot obtain certs +// CA such as Let's Encrypt. For example: internal IP addresses +// and localhost are not eligible because we cannot obtain certs // for those names with a public CA. Wildcard names are // allowed, as long as they conform to CABF requirements (only // one wildcard label, and it must be the left-most label). @@ -395,13 +395,9 @@ func SubjectQualifiesForPublicCert(subj string) bool { // must at least qualify for a certificate return SubjectQualifiesForCert(subj) && - // localhost, .localhost TLD, and .local TLD are ineligible + // loopback hosts and internal IPs are ineligible !SubjectIsInternal(subj) && - // cannot be an IP address (as of yet), see - // https://community.letsencrypt.org/t/certificate-for-static-ip/84/2?u=mholt - !SubjectIsIP(subj) && - // only one wildcard label allowed, and it must be left-most, with 3+ labels (!strings.Contains(subj, "*") || (strings.Count(subj, "*") == 1 && @@ -416,12 +412,55 @@ func SubjectIsIP(subj string) bool { } // SubjectIsInternal returns true if subj is an internal-facing -// hostname or address. +// hostname or address, including localhost/loopback hosts. +// Ports are ignored, if present. func SubjectIsInternal(subj string) bool { + subj = strings.ToLower(strings.TrimSuffix(hostOnly(subj), ".")) return subj == "localhost" || strings.HasSuffix(subj, ".localhost") || strings.HasSuffix(subj, ".local") || - strings.HasSuffix(subj, ".home.arpa") + strings.HasSuffix(subj, ".home.arpa") || + isInternalIP(subj) +} + +// isInternalIP returns true if the IP of addr +// belongs to a private network IP range. addr +// must only be an IP or an IP:port combination. +func isInternalIP(addr string) bool { + privateNetworks := []string{ + "127.0.0.0/8", // IPv4 loopback + "0.0.0.0/16", + "10.0.0.0/8", // RFC1918 + "172.16.0.0/12", // RFC1918 + "192.168.0.0/16", // RFC1918 + "169.254.0.0/16", // RFC3927 link-local + "::1/7", // IPv6 loopback + "fe80::/10", // IPv6 link-local + "fc00::/7", // IPv6 unique local addr + } + host := hostOnly(addr) + ip := net.ParseIP(host) + if ip == nil { + return false + } + for _, privateNetwork := range privateNetworks { + _, ipnet, _ := net.ParseCIDR(privateNetwork) + if ipnet.Contains(ip) { + return true + } + } + return false +} + +// hostOnly returns only the host portion of hostport. +// If there is no port or if there is an error splitting +// the port off, the whole input string is returned. +func hostOnly(hostport string) string { + host, _, err := net.SplitHostPort(hostport) + if err != nil { + return hostport // OK; probably had no port to begin with + } + return host } // MatchWildcard returns true if subject (a candidate DNS name) diff --git a/certificates_test.go b/certificates_test.go index d378d0d..12221dd 100644 --- a/certificates_test.go +++ b/certificates_test.go @@ -143,7 +143,8 @@ func TestSubjectQualifiesForPublicCert(t *testing.T) { {"Sub.Example.COM", true}, {"127.0.0.1", false}, {"127.0.1.5", false}, - {"69.123.43.94", false}, + {"1.2.3.4", true}, + {"69.123.43.94", true}, {"::1", false}, {"::", false}, {"0.0.0.0", false}, @@ -166,7 +167,7 @@ func TestSubjectQualifiesForPublicCert(t *testing.T) { {"foo.bar.home.arpa", false}, {"192.168.1.3", false}, {"10.0.2.1", false}, - {"169.112.53.4", false}, + {"169.112.53.4", true}, {"$hostname", false}, {"%HOSTNAME%", false}, {"{hostname}", false}, diff --git a/certmagic.go b/certmagic.go index 860f5e7..5843893 100644 --- a/certmagic.go +++ b/certmagic.go @@ -302,52 +302,6 @@ type OnDemandConfig struct { hostAllowlist map[string]struct{} } -// isLoopback returns true if the hostname of addr looks -// explicitly like a common local hostname. addr must only -// be a host or a host:port combination. -func isLoopback(addr string) bool { - host := hostOnly(addr) - return host == "localhost" || - strings.Trim(host, "[]") == "::1" || - strings.HasPrefix(host, "127.") -} - -// isInternal returns true if the IP of addr -// belongs to a private network IP range. addr -// must only be an IP or an IP:port combination. -// Loopback addresses are considered false. -func isInternal(addr string) bool { - privateNetworks := []string{ - "10.0.0.0/8", - "172.16.0.0/12", - "192.168.0.0/16", - "fc00::/7", - } - host := hostOnly(addr) - ip := net.ParseIP(host) - if ip == nil { - return false - } - for _, privateNetwork := range privateNetworks { - _, ipnet, _ := net.ParseCIDR(privateNetwork) - if ipnet.Contains(ip) { - return true - } - } - return false -} - -// hostOnly returns only the host portion of hostport. -// If there is no port or if there is an error splitting -// the port off, the whole input string is returned. -func hostOnly(hostport string) string { - host, _, err := net.SplitHostPort(hostport) - if err != nil { - return hostport // OK; probably had no port to begin with - } - return host -} - // PreChecker is an interface that can be optionally implemented by // Issuers. Pre-checks are performed before each call (or batch of // identical calls) to Issue(), giving the issuer the option to ensure diff --git a/config.go b/config.go index 26cf3a4..48823da 100644 --- a/config.go +++ b/config.go @@ -599,7 +599,7 @@ func (cfg *Config) obtainCert(ctx context.Context, name string, interactive bool // are compliant, so their CSR requirements just needlessly add friction, complexity, // and inefficiency for clients. CommonName has been deprecated for 25+ years. useCSR := csr - if _, ok := issuer.(*ZeroSSLIssuer); ok { + if issuer.IssuerKey() == zerosslIssuerKey { useCSR, err = cfg.generateCSR(privKey, []string{name}, true) if err != nil { return err diff --git a/crypto.go b/crypto.go index 4823fd5..879f6ae 100644 --- a/crypto.go +++ b/crypto.go @@ -281,7 +281,7 @@ func hashCertificateChain(certChain [][]byte) string { func namesFromCSR(csr *x509.CertificateRequest) []string { var nameSet []string // TODO: CommonName should not be used (it has been deprecated for 25+ years, - // but Sectigo CA still requires it to be filled out and not overlap SANs...) + // but ZeroSSL CA still requires it to be filled out and not overlap SANs...) if csr.Subject.CommonName != "" { nameSet = append(nameSet, csr.Subject.CommonName) } diff --git a/go.mod b/go.mod index 44bf00d..4b76d47 100644 --- a/go.mod +++ b/go.mod @@ -2,8 +2,10 @@ module github.com/caddyserver/certmagic go 1.22.0 +toolchain go1.22.2 + require ( - github.com/caddyserver/zerossl v0.1.1 + github.com/caddyserver/zerossl v0.1.2 github.com/klauspost/cpuid/v2 v2.2.7 github.com/libdns/libdns v0.2.2 github.com/mholt/acmez/v2 v2.0.0-beta.2 diff --git a/go.sum b/go.sum index 19290f4..dd86e5d 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/caddyserver/zerossl v0.1.1 h1:yQL7QXZnEb/ddH6JsNPGBANETUMHPFlAV5+a+Epxgbo= -github.com/caddyserver/zerossl v0.1.1/go.mod h1:wtiJEHbdvunr40ZzhXlnIkOB8Xj4eKtBKizCcZitJiQ= +github.com/caddyserver/zerossl v0.1.2 h1:tlEu1VzWGoqcCpivs9liKAKhfpJWYJkHEMmlxRbVAxE= +github.com/caddyserver/zerossl v0.1.2/go.mod h1:wtiJEHbdvunr40ZzhXlnIkOB8Xj4eKtBKizCcZitJiQ= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/klauspost/cpuid/v2 v2.0.12/go.mod h1:g2LTdtYhdyuGPqyWyv7qRAmj1WBqxuObKfj5c0PQa7c= diff --git a/zerosslissuer.go b/zerosslissuer.go index 419969d..10df015 100644 --- a/zerosslissuer.go +++ b/zerosslissuer.go @@ -26,26 +26,17 @@ import ( "time" "github.com/caddyserver/zerossl" + "github.com/mholt/acmez/v2" "github.com/mholt/acmez/v2/acme" "go.uber.org/zap" ) -// NewZeroSSLIssuer returns a ZeroSSL issuer with default values filled in -// for empty fields in the template. -func NewZeroSSLIssuer(cfg *Config, template ZeroSSLIssuer) *ZeroSSLIssuer { - if cfg == nil { - panic("cannot make valid ZeroSSLIssuer without an associated CertMagic config") - } - template.config = cfg - template.logger = defaultLogger.Named("zerossl") - return &template -} - // ZeroSSLIssuer can get certificates from ZeroSSL's API. (To use ZeroSSL's ACME // endpoint, use the ACMEIssuer instead.) Note that use of the API is restricted // by payment tier. type ZeroSSLIssuer struct { // The API key (or "access key") for using the ZeroSSL API. + // REQUIRED. APIKey string // How many days the certificate should be valid for. @@ -63,8 +54,13 @@ type ZeroSSLIssuer struct { // validation, set this field. CNAMEValidation *DNSManager - config *Config - logger *zap.Logger + // Where to store verification material temporarily. + // Set this on all instances in a cluster to the same + // value to enable distributed verification. + Storage Storage + + // An optional (but highly recommended) logger. + Logger *zap.Logger } // Issue obtains a certificate for the given csr. @@ -72,7 +68,12 @@ func (iss *ZeroSSLIssuer) Issue(ctx context.Context, csr *x509.CertificateReques client := iss.getClient() identifiers := namesFromCSR(csr) - logger := iss.logger.With(zap.Strings("identifiers", identifiers)) + + logger := iss.Logger + if logger == nil { + logger = zap.NewNop() + } + logger = logger.With(zap.Strings("identifiers", identifiers)) logger.Info("creating certificate") @@ -134,16 +135,19 @@ func (iss *ZeroSSLIssuer) Issue(ctx context.Context, csr *x509.CertificateReques }), } - distSolver := distributedSolver{ - storage: iss.config.Storage, - storageKeyIssuerPrefix: "zerossl", - solver: httpVerifier, + var solver acmez.Solver = httpVerifier + if iss.Storage != nil { + solver = distributedSolver{ + storage: iss.Storage, + storageKeyIssuerPrefix: iss.IssuerKey(), + solver: httpVerifier, + } } - if err = distSolver.Present(ctx, acme.Challenge{}); err != nil { + if err = solver.Present(ctx, acme.Challenge{}); err != nil { return nil, fmt.Errorf("presenting token for verification: %v", err) } - defer distSolver.CleanUp(ctx, acme.Challenge{}) + defer solver.CleanUp(ctx, acme.Challenge{}) } else { verificationMethod = zerossl.CNAMEVerification logger = logger.With(zap.String("verification_method", string(verificationMethod))) @@ -248,9 +252,7 @@ func (iss *ZeroSSLIssuer) getHTTPPort() int { } // IssuerKey returns the unique issuer key for ZeroSSL. -func (iss *ZeroSSLIssuer) IssuerKey() string { - return "zerossl" -} +func (iss *ZeroSSLIssuer) IssuerKey() string { return zerosslIssuerKey } // Revoke revokes the given certificate. Only do this if there is a security or trust // concern with the certificate. @@ -274,6 +276,7 @@ func (iss *ZeroSSLIssuer) Revoke(ctx context.Context, cert CertificateResource, const ( zerosslAPIBase = "https://" + zerossl.BaseURL + "/acme" zerosslValidationPathPrefix = "/.well-known/pki-validation/" + zerosslIssuerKey = "zerossl" ) // Interface guards