diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..6d9b95a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,20 @@ +linters-settings: + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - importShadow + - ifElseChain + - hugeParam + +linters: + enable: + - unconvert + - gocritic + +run: + timeout: 5m diff --git a/.woodpecker.yml b/.woodpecker.yml index 20254fe..7e6d694 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -102,7 +102,7 @@ pipeline: registry: codeberg.org dockerfile: Dockerfile repo: codeberg.org/codeberg/pages-server - tag: [ latest, "${CI_COMMIT_TAG}" ] + tags: [ latest, "${CI_COMMIT_TAG}" ] username: from_secret: bot_user password: diff --git a/Justfile b/Justfile index f7ea414..7d72fe8 100644 --- a/Justfile +++ b/Justfile @@ -38,10 +38,10 @@ tool-gofumpt: fi test: - go test -race codeberg.org/codeberg/pages/server/... + go test -race codeberg.org/codeberg/pages/server/... codeberg.org/codeberg/pages/html/ test-run TEST: - go test -race -run "^{{TEST}}$" codeberg.org/codeberg/pages/server/... + go test -race -run "^{{TEST}}$" codeberg.org/codeberg/pages/server/... codeberg.org/codeberg/pages/html/ integration: go test -race -tags integration codeberg.org/codeberg/pages/integration/... diff --git a/cmd/main.go b/cmd/main.go index c76b538..2bb91a1 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -66,7 +66,7 @@ func Serve(ctx *cli.Context) error { } allowedCorsDomains := AllowedCorsDomains - if len(rawDomain) != 0 { + if rawDomain != "" { allowedCorsDomains = append(allowedCorsDomains, rawDomain) } diff --git a/html/error.go b/html/error.go index 826c42b..206b123 100644 --- a/html/error.go +++ b/html/error.go @@ -1,6 +1,7 @@ package html import ( + "html/template" "net/http" "strconv" "strings" @@ -14,16 +15,27 @@ func ReturnErrorPage(ctx *context.Context, msg string, statusCode int) { ctx.RespWriter.Header().Set("Content-Type", "text/html; charset=utf-8") ctx.RespWriter.WriteHeader(statusCode) - if msg == "" { - msg = errorBody(statusCode) - } else { - // TODO: use template engine - msg = strings.ReplaceAll(strings.ReplaceAll(ErrorPage, "%message%", msg), "%status%", http.StatusText(statusCode)) - } + msg = generateResponse(msg, statusCode) _, _ = ctx.RespWriter.Write([]byte(msg)) } +// TODO: use template engine +func generateResponse(msg string, statusCode int) string { + if msg == "" { + msg = strings.ReplaceAll(NotFoundPage, + "%status%", + strconv.Itoa(statusCode)+" "+errorMessage(statusCode)) + } else { + msg = strings.ReplaceAll( + strings.ReplaceAll(ErrorPage, "%message%", template.HTMLEscapeString(msg)), + "%status%", + http.StatusText(statusCode)) + } + + return msg +} + func errorMessage(statusCode int) string { message := http.StatusText(statusCode) @@ -36,10 +48,3 @@ func errorMessage(statusCode int) string { return message } - -// TODO: use template engine -func errorBody(statusCode int) string { - return strings.ReplaceAll(NotFoundPage, - "%status%", - strconv.Itoa(statusCode)+" "+errorMessage(statusCode)) -} diff --git a/html/error_test.go b/html/error_test.go new file mode 100644 index 0000000..f5da08c --- /dev/null +++ b/html/error_test.go @@ -0,0 +1,38 @@ +package html + +import ( + "net/http" + "strings" + "testing" +) + +func TestValidMessage(t *testing.T) { + testString := "requested blacklisted path" + statusCode := http.StatusForbidden + + expected := strings.ReplaceAll( + strings.ReplaceAll(ErrorPage, "%message%", testString), + "%status%", + http.StatusText(statusCode)) + actual := generateResponse(testString, statusCode) + + if expected != actual { + t.Errorf("generated response did not match: expected: '%s', got: '%s'", expected, actual) + } +} + +func TestMessageWithHtml(t *testing.T) { + testString := `abc 0 { // CSR stores the time when the renewal shall be tried again @@ -228,9 +233,9 @@ func retrieveCertFromDB(sni, mainDomainSuffix, dnsProvider string, acmeUseRateLi } go (func() { res.CSR = nil // acme client doesn't like CSR to be set - tlsCertificate, err = obtainCert(acmeClient, []string{string(sni)}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) + tlsCertificate, err = obtainCert(acmeClient, []string{sni}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) if err != nil { - log.Error().Msgf("Couldn't renew certificate for %s: %v", string(sni), err) + log.Error().Msgf("Couldn't renew certificate for %s: %v", sni, err) } })() } @@ -263,7 +268,7 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re defer obtainLocks.Delete(name) if acmeClient == nil { - return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", string(mainDomainSuffix), keyDatabase), nil + return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", mainDomainSuffix, keyDatabase), nil } // request actual cert @@ -277,6 +282,9 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re res, err = acmeClient.Certificate.Renew(*renew, true, false, "") if err != nil { log.Error().Err(err).Msgf("Couldn't renew certificate for %v, trying to request a new one", domains) + if acmeUseRateLimits { + acmeClientFailLimit.Take() + } res = nil } } @@ -297,21 +305,28 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re Bundle: true, MustStaple: false, }) + if acmeUseRateLimits && err != nil { + acmeClientFailLimit.Take() + } } if err != nil { log.Error().Err(err).Msgf("Couldn't obtain again a certificate or %v", domains) if renew != nil && renew.CertURL != "" { tlsCertificate, err := tls.X509KeyPair(renew.Certificate, renew.PrivateKey) - if err == nil && tlsCertificate.Leaf.NotAfter.After(time.Now()) { + if err != nil { + return mockCert(domains[0], err.Error(), mainDomainSuffix, keyDatabase), err + } + leaf, err := leaf(&tlsCertificate) + if err == nil && leaf.NotAfter.After(time.Now()) { // 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 { - return mockCert(domains[0], err.Error(), string(mainDomainSuffix), keyDatabase), err + return mockCert(domains[0], err.Error(), mainDomainSuffix, keyDatabase), err } return tlsCertificate, nil } } - return mockCert(domains[0], err.Error(), string(mainDomainSuffix), keyDatabase), err + return mockCert(domains[0], err.Error(), mainDomainSuffix, keyDatabase), err } log.Debug().Msgf("Obtained certificate for %v", domains) @@ -409,7 +424,7 @@ func SetupAcmeConfig(acmeAPI, acmeMail, acmeEabHmac, acmeEabKID string, acmeAcce func SetupCertificates(mainDomainSuffix, dnsProvider string, acmeConfig *lego.Config, acmeUseRateLimits, enableHTTPServer bool, challengeCache cache.SetGetKey, certDB database.CertDB) error { // getting main cert before ACME account so that we can fail here without hitting rate limits - mainCertBytes, err := certDB.Get(string(mainDomainSuffix)) + mainCertBytes, err := certDB.Get(mainDomainSuffix) if err != nil { return fmt.Errorf("cert database is not working") } @@ -453,7 +468,7 @@ func SetupCertificates(mainDomainSuffix, dnsProvider string, acmeConfig *lego.Co } if mainCertBytes == nil { - _, err = obtainCert(mainDomainAcmeClient, []string{"*" + string(mainDomainSuffix), string(mainDomainSuffix[1:])}, nil, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) + _, err = obtainCert(mainDomainAcmeClient, []string{"*" + mainDomainSuffix, mainDomainSuffix[1:]}, nil, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) if err != nil { log.Error().Err(err).Msg("Couldn't renew main domain certificate, continuing with mock certs only") } @@ -480,7 +495,7 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, mainDomainSuffi } tlsCertificates, err := certcrypto.ParsePEMBundle(res.Certificate) - if err != nil || !tlsCertificates[0].NotAfter.After(now) { + if err != nil || tlsCertificates[0].NotAfter.Before(now) { err := certDB.Delete(string(key)) if err != nil { log.Error().Err(err).Msgf("Deleting expired certificate for %q failed", string(key)) @@ -502,18 +517,18 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, mainDomainSuffi } // update main cert - res, err := certDB.Get(string(mainDomainSuffix)) + res, err := certDB.Get(mainDomainSuffix) if err != nil { log.Error().Msgf("Couldn't get cert for domain %q", mainDomainSuffix) } else if res == nil { - log.Error().Msgf("Couldn't renew certificate for main domain %q expected main domain cert to exist, but it's missing - seems like the database is corrupted", string(mainDomainSuffix)) + log.Error().Msgf("Couldn't renew certificate for main domain %q expected main domain cert to exist, but it's missing - seems like the database is corrupted", mainDomainSuffix) } else { tlsCertificates, err := certcrypto.ParsePEMBundle(res.Certificate) // renew main certificate 30 days before it expires - if !tlsCertificates[0].NotAfter.After(time.Now().Add(30 * 24 * time.Hour)) { + if tlsCertificates[0].NotAfter.Before(time.Now().Add(30 * 24 * time.Hour)) { go (func() { - _, err = obtainCert(mainDomainAcmeClient, []string{"*" + string(mainDomainSuffix), string(mainDomainSuffix[1:])}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) + _, err = obtainCert(mainDomainAcmeClient, []string{"*" + mainDomainSuffix, mainDomainSuffix[1:]}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) if err != nil { log.Error().Err(err).Msg("Couldn't renew certificate for main domain") } @@ -528,3 +543,12 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, mainDomainSuffi } } } + +// leaf returns the parsed leaf certificate, either from c.leaf or by parsing +// the corresponding c.Certificate[0]. +func leaf(c *tls.Certificate) (*x509.Certificate, error) { + if c.Leaf != nil { + return c.Leaf, nil + } + return x509.ParseCertificate(c.Certificate[0]) +} diff --git a/server/database/setup.go b/server/database/setup.go index 1c5a0af..097c63e 100644 --- a/server/database/setup.go +++ b/server/database/setup.go @@ -44,7 +44,7 @@ func (p aDB) Get(name string) (*certificate.Resource, error) { if resBytes == nil { return nil, nil } - if err = gob.NewDecoder(bytes.NewBuffer(resBytes)).Decode(cert); err != nil { + if err := gob.NewDecoder(bytes.NewBuffer(resBytes)).Decode(cert); err != nil { return nil, err } return cert, nil diff --git a/server/gitea/cache.go b/server/gitea/cache.go index b11a370..85cbcde 100644 --- a/server/gitea/cache.go +++ b/server/gitea/cache.go @@ -42,9 +42,8 @@ func (f FileResponse) IsEmpty() bool { return len(f.Body) != 0 } -func (f FileResponse) createHttpResponse(cacheKey string) (http.Header, int) { - header := make(http.Header) - var statusCode int +func (f FileResponse) createHttpResponse(cacheKey string) (header http.Header, statusCode int) { + header = make(http.Header) if f.Exists { statusCode = http.StatusOK diff --git a/server/handler/handler.go b/server/handler/handler.go index c556470..a944c7e 100644 --- a/server/handler/handler.go +++ b/server/handler/handler.go @@ -28,7 +28,7 @@ func Handler(mainDomainSuffix, rawDomain string, dnsLookupCache, canonicalDomainCache cache.SetGetKey, ) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { - log := log.With().Strs("Handler", []string{string(req.Host), req.RequestURI}).Logger() + log := log.With().Strs("Handler", []string{req.Host, req.RequestURI}).Logger() ctx := context.New(w, req) ctx.RespWriter.Header().Set("Server", "CodebergPages/"+version.Version) diff --git a/server/handler/handler_custom_domain.go b/server/handler/handler_custom_domain.go index 8ed0e69..e5fe606 100644 --- a/server/handler/handler_custom_domain.go +++ b/server/handler/handler_custom_domain.go @@ -55,7 +55,7 @@ func handleCustomDomain(log zerolog.Logger, ctx *context.Context, giteaClient *g // only redirect if the target is also a codeberg page! targetOwner, _, _ = dns.GetTargetFromDNS(strings.SplitN(canonicalDomain, "/", 2)[0], mainDomainSuffix, firstDefaultBranch, dnsLookupCache) if targetOwner != "" { - ctx.Redirect("https://"+canonicalDomain+string(targetOpt.TargetPath), http.StatusTemporaryRedirect) + ctx.Redirect("https://"+canonicalDomain+targetOpt.TargetPath, http.StatusTemporaryRedirect) return } diff --git a/server/handler/handler_sub_domain.go b/server/handler/handler_sub_domain.go index 3e4866b..e3b1933 100644 --- a/server/handler/handler_sub_domain.go +++ b/server/handler/handler_sub_domain.go @@ -30,7 +30,7 @@ func handleSubDomain(log zerolog.Logger, ctx *context.Context, giteaClient *gite if targetOwner == "www" { // www.codeberg.page redirects to codeberg.page // TODO: rm hardcoded - use cname? - ctx.Redirect("https://"+string(mainDomainSuffix[1:])+string(ctx.Path()), http.StatusPermanentRedirect) + ctx.Redirect("https://"+mainDomainSuffix[1:]+ctx.Path(), http.StatusPermanentRedirect) return } @@ -131,6 +131,6 @@ func handleSubDomain(log zerolog.Logger, ctx *context.Context, giteaClient *gite // Couldn't find a valid repo/branch html.ReturnErrorPage(ctx, - fmt.Sprintf("couldn't find a valid repo[%s]", targetRepo), - http.StatusFailedDependency) + fmt.Sprintf("could not find a valid repository[%s]", targetRepo), + http.StatusNotFound) } diff --git a/server/handler/handler_test.go b/server/handler/handler_test.go index 14c4b72..ed063b2 100644 --- a/server/handler/handler_test.go +++ b/server/handler/handler_test.go @@ -24,26 +24,27 @@ func TestHandlerPerformance(t *testing.T) { ) testCase := func(uri string, status int) { - req := httptest.NewRequest("GET", uri, nil) - w := httptest.NewRecorder() + t.Run(uri, func(t *testing.T) { + req := httptest.NewRequest("GET", uri, nil) + w := httptest.NewRecorder() - log.Printf("Start: %v\n", time.Now()) - start := time.Now() - testHandler(w, req) - end := time.Now() - log.Printf("Done: %v\n", time.Now()) + log.Printf("Start: %v\n", time.Now()) + start := time.Now() + testHandler(w, req) + end := time.Now() + log.Printf("Done: %v\n", time.Now()) - resp := w.Result() + resp := w.Result() - if resp.StatusCode != status { - t.Errorf("request failed with status code %d", resp.StatusCode) - } else { - t.Logf("request took %d milliseconds", end.Sub(start).Milliseconds()) - } + if resp.StatusCode != status { + t.Errorf("request failed with status code %d", resp.StatusCode) + } else { + t.Logf("request took %d milliseconds", end.Sub(start).Milliseconds()) + } + }) } - testCase("https://mondstern.codeberg.page/", 424) // TODO: expect 200 - testCase("https://mondstern.codeberg.page/", 424) // TODO: expect 200 - testCase("https://example.momar.xyz/", 424) // TODO: expect 200 - testCase("https://codeberg.page/", 424) // TODO: expect 200 + testCase("https://mondstern.codeberg.page/", 404) // TODO: expect 200 + testCase("https://codeberg.page/", 404) // TODO: expect 200 + testCase("https://example.momar.xyz/", 424) } diff --git a/server/handler/try.go b/server/handler/try.go index bae3c44..5a09b91 100644 --- a/server/handler/try.go +++ b/server/handler/try.go @@ -21,8 +21,8 @@ func tryUpstream(ctx *context.Context, giteaClient *gitea.Client, ) { // check if a canonical domain exists on a request on MainDomain if strings.HasSuffix(trimmedHost, mainDomainSuffix) { - canonicalDomain, _ := options.CheckCanonicalDomain(giteaClient, "", string(mainDomainSuffix), canonicalDomainCache) - if !strings.HasSuffix(strings.SplitN(canonicalDomain, "/", 2)[0], string(mainDomainSuffix)) { + canonicalDomain, _ := options.CheckCanonicalDomain(giteaClient, "", mainDomainSuffix, canonicalDomainCache) + if !strings.HasSuffix(strings.SplitN(canonicalDomain, "/", 2)[0], mainDomainSuffix) { canonicalPath := ctx.Req.RequestURI if options.TargetRepo != defaultPagesRepo { path := strings.SplitN(canonicalPath, "/", 3) @@ -35,8 +35,8 @@ func tryUpstream(ctx *context.Context, giteaClient *gitea.Client, } } - // add host for debugging - options.Host = string(trimmedHost) + // Add host for debugging. + options.Host = trimmedHost // Try to request the file from the Gitea API if !options.Upstream(ctx, giteaClient) { diff --git a/server/setup.go b/server/setup.go index e7194ed..282e692 100644 --- a/server/setup.go +++ b/server/setup.go @@ -15,13 +15,13 @@ func SetupHTTPACMEChallengeServer(challengeCache cache.SetGetKey) http.HandlerFu return func(w http.ResponseWriter, req *http.Request) { ctx := context.New(w, req) if strings.HasPrefix(ctx.Path(), challengePath) { - challenge, ok := challengeCache.Get(utils.TrimHostPort(ctx.Host()) + "/" + string(strings.TrimPrefix(ctx.Path(), challengePath))) + challenge, ok := challengeCache.Get(utils.TrimHostPort(ctx.Host()) + "/" + strings.TrimPrefix(ctx.Path(), challengePath)) if !ok || challenge == nil { ctx.String("no challenge for this token", http.StatusNotFound) } ctx.String(challenge.(string)) } else { - ctx.Redirect("https://"+string(ctx.Host())+string(ctx.Path()), http.StatusMovedPermanently) + ctx.Redirect("https://"+ctx.Host()+ctx.Path(), http.StatusMovedPermanently) } } } diff --git a/server/upstream/helper.go b/server/upstream/helper.go index 428976b..a84d4f0 100644 --- a/server/upstream/helper.go +++ b/server/upstream/helper.go @@ -13,7 +13,7 @@ import ( func (o *Options) GetBranchTimestamp(giteaClient *gitea.Client) (bool, error) { log := log.With().Strs("BranchInfo", []string{o.TargetOwner, o.TargetRepo, o.TargetBranch}).Logger() - if len(o.TargetBranch) == 0 { + if o.TargetBranch == "" { // Get default branch defaultBranch, err := giteaClient.GiteaGetRepoDefaultBranch(o.TargetOwner, o.TargetRepo) if err != nil { diff --git a/server/upstream/upstream.go b/server/upstream/upstream.go index b76b8e6..7c3c848 100644 --- a/server/upstream/upstream.go +++ b/server/upstream/upstream.go @@ -82,8 +82,8 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin // Check if the browser has a cached version if ctx.Response() != nil { - if ifModifiedSince, err := time.Parse(time.RFC1123, string(ctx.Response().Header.Get(headerIfModifiedSince))); err == nil { - if !ifModifiedSince.Before(o.BranchTimestamp) { + if ifModifiedSince, err := time.Parse(time.RFC1123, ctx.Response().Header.Get(headerIfModifiedSince)); err == nil { + if ifModifiedSince.After(o.BranchTimestamp) { ctx.RespWriter.WriteHeader(http.StatusNotModified) log.Trace().Msg("check response against last modified: valid") return true