From 03881382a4c286e3a7dc351f095f9147e89c2d6d Mon Sep 17 00:00:00 2001 From: Jean-Marie 'Histausse' Mineau Date: Thu, 18 Apr 2024 17:05:20 +0000 Subject: [PATCH] Add option to disable DNS ACME provider (#290) This PR add the `$NO_DNS_01` option (disabled by default) that removes the DNS ACME provider, and replaces the wildcard certificate by individual certificates obtained using the TLS ACME provider. This option allows an instance to work without having to manage access tokens for the DNS provider. On the flip side, this means that a certificate can be requested for each subdomains. To limit the risk of DOS, the existence of the user/org corresponding to a subdomain is checked before requesting a cert, however, this limitation is not enough for an forge with a high number of users/orgs. Co-authored-by: 6543 <6543@obermui.de> Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/290 Reviewed-by: Moritz Marquardt Co-authored-by: Jean-Marie 'Histausse' Mineau Co-committed-by: Jean-Marie 'Histausse' Mineau --- README.md | 1 + cli/flags.go | 5 +++++ config/config.go | 1 + config/setup.go | 3 +++ config/setup_test.go | 7 ++++++ server/acme/client.go | 4 ++-- server/certificates/acme_client.go | 7 ++---- server/certificates/certificates.go | 30 ++++++++++++++++++++------ server/database/xorm.go | 13 ------------ server/gitea/cache.go | 3 +++ server/gitea/client.go | 33 +++++++++++++++++++++++++++++ server/startup.go | 2 ++ 12 files changed, 83 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 7193691..f47a196 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,7 @@ and especially have a look at [this section of the haproxy.cfg](https://codeberg - `ENABLE_HTTP_SERVER` (default: false): Set this to true to enable the HTTP-01 challenge and redirect all other HTTP requests to HTTPS. Currently only works with port 80. - `DNS_PROVIDER` (default: use self-signed certificate): Code of the ACME DNS provider for the main domain wildcard. See for available values & additional environment variables. +- `NO_DNS_01` (default: `false`): Disable the use of ACME DNS. This means that the wildcard certificate is self-signed and all domains and subdomains will have a distinct certificate. Because this may lead to a rate limit from the ACME provider, this option is not recommended for Gitea/Forgejo instances with open registrations or a great number of users/orgs. - `LOG_LEVEL` (default: warn): Set this to specify the level of logging. ## Contributing to the development diff --git a/cli/flags.go b/cli/flags.go index 097cf4f..52e1c1c 100644 --- a/cli/flags.go +++ b/cli/flags.go @@ -178,6 +178,11 @@ var ( Usage: "Use DNS-Challenge for main domain. Read more at: https://go-acme.github.io/lego/dns/", EnvVars: []string{"DNS_PROVIDER"}, }, + &cli.BoolFlag{ + Name: "no-dns-01", + Usage: "Always use individual certificates instead of a DNS-01 wild card certificate", + EnvVars: []string{"NO_DNS_01"}, + }, &cli.StringFlag{ Name: "acme-account-config", Usage: "json file of acme account", diff --git a/config/config.go b/config/config.go index 6cb972b..0146e0f 100644 --- a/config/config.go +++ b/config/config.go @@ -42,5 +42,6 @@ type ACMEConfig struct { EAB_HMAC string EAB_KID string DNSProvider string + NoDNS01 bool `default:"false"` AccountConfigFile string `default:"acme-account.json"` } diff --git a/config/setup.go b/config/setup.go index e774084..6a2aa62 100644 --- a/config/setup.go +++ b/config/setup.go @@ -141,6 +141,9 @@ func mergeACMEConfig(ctx *cli.Context, config *ACMEConfig) { if ctx.IsSet("dns-provider") { config.DNSProvider = ctx.String("dns-provider") } + if ctx.IsSet("no-dns-01") { + config.NoDNS01 = ctx.Bool("no-dns-01") + } if ctx.IsSet("acme-account-config") { config.AccountConfigFile = ctx.String("acme-account-config") } diff --git a/config/setup_test.go b/config/setup_test.go index a863e2f..1a32740 100644 --- a/config/setup_test.go +++ b/config/setup_test.go @@ -166,6 +166,7 @@ func TestMergeConfigShouldReplaceAllExistingValuesGivenAllArgsExist(t *testing.T EAB_HMAC: "original", EAB_KID: "original", DNSProvider: "original", + NoDNS01: false, AccountConfigFile: "original", }, } @@ -205,6 +206,7 @@ func TestMergeConfigShouldReplaceAllExistingValuesGivenAllArgsExist(t *testing.T EAB_HMAC: "changed", EAB_KID: "changed", DNSProvider: "changed", + NoDNS01: true, AccountConfigFile: "changed", }, } @@ -243,6 +245,7 @@ func TestMergeConfigShouldReplaceAllExistingValuesGivenAllArgsExist(t *testing.T "--acme-eab-hmac", "changed", "--acme-eab-kid", "changed", "--dns-provider", "changed", + "--no-dns-01", "--acme-account-config", "changed", }, ) @@ -517,6 +520,7 @@ func TestMergeACMEConfigShouldReplaceAllExistingValuesGivenAllArgsExist(t *testi EAB_HMAC: "original", EAB_KID: "original", DNSProvider: "original", + NoDNS01: false, AccountConfigFile: "original", } @@ -530,6 +534,7 @@ func TestMergeACMEConfigShouldReplaceAllExistingValuesGivenAllArgsExist(t *testi EAB_HMAC: "changed", EAB_KID: "changed", DNSProvider: "changed", + NoDNS01: true, AccountConfigFile: "changed", } @@ -545,6 +550,7 @@ func TestMergeACMEConfigShouldReplaceAllExistingValuesGivenAllArgsExist(t *testi "--acme-eab-hmac", "changed", "--acme-eab-kid", "changed", "--dns-provider", "changed", + "--no-dns-01", "--acme-account-config", "changed", }, ) @@ -563,6 +569,7 @@ func TestMergeACMEConfigShouldReplaceOnlyOneValueExistingValueGivenOnlyOneArgExi {args: []string{"--acme-eab-hmac", "changed"}, callback: func(gc *ACMEConfig) { gc.EAB_HMAC = "changed" }}, {args: []string{"--acme-eab-kid", "changed"}, callback: func(gc *ACMEConfig) { gc.EAB_KID = "changed" }}, {args: []string{"--dns-provider", "changed"}, callback: func(gc *ACMEConfig) { gc.DNSProvider = "changed" }}, + {args: []string{"--no-dns-01"}, callback: func(gc *ACMEConfig) { gc.NoDNS01 = true }}, {args: []string{"--acme-account-config", "changed"}, callback: func(gc *ACMEConfig) { gc.AccountConfigFile = "changed" }}, } diff --git a/server/acme/client.go b/server/acme/client.go index 38e2785..d5c83d0 100644 --- a/server/acme/client.go +++ b/server/acme/client.go @@ -13,8 +13,8 @@ var ErrAcmeMissConfig = errors.New("ACME client has wrong config") func CreateAcmeClient(cfg config.ACMEConfig, enableHTTPServer bool, challengeCache cache.ICache) (*certificates.AcmeClient, error) { // check config - if (!cfg.AcceptTerms || cfg.DNSProvider == "") && cfg.APIEndpoint != "https://acme.mock.directory" { - return nil, fmt.Errorf("%w: you must set $ACME_ACCEPT_TERMS and $DNS_PROVIDER, unless $ACME_API is set to https://acme.mock.directory", ErrAcmeMissConfig) + if (!cfg.AcceptTerms || (cfg.DNSProvider == "" && !cfg.NoDNS01)) && cfg.APIEndpoint != "https://acme.mock.directory" { + return nil, fmt.Errorf("%w: you must set $ACME_ACCEPT_TERMS and $DNS_PROVIDER or $NO_DNS_01, unless $ACME_API is set to https://acme.mock.directory", ErrAcmeMissConfig) } if cfg.EAB_HMAC != "" && cfg.EAB_KID == "" { return nil, fmt.Errorf("%w: ACME_EAB_HMAC also needs ACME_EAB_KID to be set", ErrAcmeMissConfig) diff --git a/server/certificates/acme_client.go b/server/certificates/acme_client.go index d53e854..f42fd8f 100644 --- a/server/certificates/acme_client.go +++ b/server/certificates/acme_client.go @@ -56,11 +56,8 @@ func NewAcmeClient(cfg config.ACMEConfig, enableHTTPServer bool, challengeCache log.Error().Err(err).Msg("Can't create ACME client, continuing with mock certs only") } else { if cfg.DNSProvider == "" { - // using mock server, don't use wildcard certs - err := mainDomainAcmeClient.Challenge.SetTLSALPN01Provider(AcmeTLSChallengeProvider{challengeCache}) - if err != nil { - log.Error().Err(err).Msg("Can't create TLS-ALPN-01 provider") - } + // using mock wildcard certs + mainDomainAcmeClient = nil } else { // use DNS-Challenge https://go-acme.github.io/lego/dns/ provider, err := dns.NewDNSChallengeProviderByName(cfg.DNSProvider) diff --git a/server/certificates/certificates.go b/server/certificates/certificates.go index b638755..67219dd 100644 --- a/server/certificates/certificates.go +++ b/server/certificates/certificates.go @@ -33,6 +33,8 @@ func TLSConfig(mainDomainSuffix string, firstDefaultBranch string, keyCache, challengeCache, dnsLookupCache, canonicalDomainCache cache.ICache, certDB database.CertDB, + noDNS01 bool, + rawDomain string, ) *tls.Config { return &tls.Config{ // check DNS name & get certificate from Let's Encrypt @@ -64,9 +66,24 @@ func TLSConfig(mainDomainSuffix string, targetOwner := "" mayObtainCert := true + if strings.HasSuffix(domain, mainDomainSuffix) || strings.EqualFold(domain, mainDomainSuffix[1:]) { - // deliver default certificate for the main domain (*.codeberg.page) - domain = mainDomainSuffix + if noDNS01 { + // Limit the domains allowed to request a certificate to pages-server domains + // and domains for an existing user of org + if !strings.EqualFold(domain, mainDomainSuffix[1:]) && !strings.EqualFold(domain, rawDomain) { + targetOwner := strings.TrimSuffix(domain, mainDomainSuffix) + owner_exist, err := giteaClient.GiteaCheckIfOwnerExists(targetOwner) + mayObtainCert = owner_exist + if err != nil { + log.Error().Err(err).Msgf("Failed to check '%s' existence on the forge: %s", targetOwner, err) + mayObtainCert = false + } + } + } else { + // deliver default certificate for the main domain (*.codeberg.page) + domain = mainDomainSuffix + } } else { var targetRepo, targetBranch string targetOwner, targetRepo, targetBranch = dnsutils.GetTargetFromDNS(domain, mainDomainSuffix, firstDefaultBranch, dnsLookupCache) @@ -199,9 +216,6 @@ func (c *AcmeClient) retrieveCertFromDB(sni, mainDomainSuffix string, useDnsProv func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Resource, user string, useDnsProvider bool, mainDomainSuffix string, keyDatabase database.CertDB) (*tls.Certificate, error) { name := strings.TrimPrefix(domains[0], "*") - if useDnsProvider && domains[0] != "" && domains[0][0] == '*' { - domains = domains[1:] - } // lock to avoid simultaneous requests _, working := c.obtainLocks.LoadOrStore(name, struct{}{}) @@ -219,7 +233,11 @@ func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew defer c.obtainLocks.Delete(name) if acmeClient == nil { - return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", mainDomainSuffix, keyDatabase) + if useDnsProvider { + return mockCert(domains[0], "DNS ACME client is not defined", mainDomainSuffix, keyDatabase) + } else { + return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", mainDomainSuffix, keyDatabase) + } } // request actual cert diff --git a/server/database/xorm.go b/server/database/xorm.go index 217b6d1..63fa39e 100644 --- a/server/database/xorm.go +++ b/server/database/xorm.go @@ -52,7 +52,6 @@ func (x xDB) Close() error { func (x xDB) Put(domain string, cert *certificate.Resource) error { log.Trace().Str("domain", cert.Domain).Msg("inserting cert to db") - domain = integrationTestReplacements(domain) c, err := toCert(domain, cert) if err != nil { return err @@ -82,7 +81,6 @@ func (x xDB) Get(domain string) (*certificate.Resource, error) { if domain[:1] == "." { domain = "*" + domain } - domain = integrationTestReplacements(domain) cert := new(Cert) log.Trace().Str("domain", domain).Msg("get cert from db") @@ -99,7 +97,6 @@ func (x xDB) Delete(domain string) error { if domain[:1] == "." { domain = "*" + domain } - domain = integrationTestReplacements(domain) log.Trace().Str("domain", domain).Msg("delete cert from db") _, err := x.engine.ID(domain).Delete(new(Cert)) @@ -139,13 +136,3 @@ func supportedDriver(driver string) bool { return false } } - -// integrationTestReplacements is needed because integration tests use a single domain cert, -// while production use a wildcard cert -// TODO: find a better way to handle this -func integrationTestReplacements(domainKey string) string { - if domainKey == "*.localhost.mock.directory" { - return "localhost.mock.directory" - } - return domainKey -} diff --git a/server/gitea/cache.go b/server/gitea/cache.go index 4ecc5e0..97024a1 100644 --- a/server/gitea/cache.go +++ b/server/gitea/cache.go @@ -26,6 +26,9 @@ const ( // TODO: move as option into cache interface fileCacheTimeout = 5 * time.Minute + // ownerExistenceCacheTimeout specifies the timeout for the existence of a repo/org + ownerExistenceCacheTimeout = 5 * time.Minute + // fileCacheSizeLimit limits the maximum file size that will be cached, and is set to 1 MB by default. fileCacheSizeLimit = int64(1000 * 1000) ) diff --git a/server/gitea/client.go b/server/gitea/client.go index 4f7eaa6..5955bfb 100644 --- a/server/gitea/client.go +++ b/server/gitea/client.go @@ -28,6 +28,7 @@ const ( branchTimestampCacheKeyPrefix = "branchTime" defaultBranchCacheKeyPrefix = "defaultBranch" rawContentCacheKeyPrefix = "rawContent" + ownerExistenceKeyPrefix = "ownerExist" // pages server PagesCacheIndicatorHeader = "X-Pages-Cache" @@ -266,6 +267,38 @@ func (client *Client) GiteaGetRepoDefaultBranch(repoOwner, repoName string) (str return branch, nil } +func (client *Client) GiteaCheckIfOwnerExists(owner string) (bool, error) { + cacheKey := fmt.Sprintf("%s/%s", ownerExistenceKeyPrefix, owner) + + if exist, ok := client.responseCache.Get(cacheKey); ok && exist != nil { + return exist.(bool), nil + } + + _, resp, err := client.sdkClient.GetUserInfo(owner) + if resp.StatusCode == http.StatusOK && err == nil { + if err := client.responseCache.Set(cacheKey, true, ownerExistenceCacheTimeout); err != nil { + log.Error().Err(err).Msg("[cache] error on cache write") + } + return true, nil + } else if resp.StatusCode != http.StatusNotFound { + return false, err + } + + _, resp, err = client.sdkClient.GetOrg(owner) + if resp.StatusCode == http.StatusOK && err == nil { + if err := client.responseCache.Set(cacheKey, true, ownerExistenceCacheTimeout); err != nil { + log.Error().Err(err).Msg("[cache] error on cache write") + } + return true, nil + } else if resp.StatusCode != http.StatusNotFound { + return false, err + } + if err := client.responseCache.Set(cacheKey, false, ownerExistenceCacheTimeout); err != nil { + log.Error().Err(err).Msg("[cache] error on cache write") + } + return false, nil +} + func (client *Client) getMimeTypeByExtension(resource string) string { mimeType := mime.TypeByExtension(path.Ext(resource)) mimeTypeSplit := strings.SplitN(mimeType, ";", 2) diff --git a/server/startup.go b/server/startup.go index ffdabb7..149a07d 100644 --- a/server/startup.go +++ b/server/startup.go @@ -110,6 +110,8 @@ func Serve(ctx *cli.Context) error { cfg.Server.PagesBranches[0], keyCache, challengeCache, dnsLookupCache, canonicalDomainCache, certDB, + cfg.ACME.NoDNS01, + cfg.Server.RawDomain, )) interval := 12 * time.Hour