Use correct log level for CheckCanonicalDomain (#162)

- Currently any error generated by requesting the `.domains` file of a repository would be logged under the info log level, which isn't the correct log level when we exclude the not found error.
- Use warn log level if the error isn't the not found error.

Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/162
Reviewed-by: Otto <otto@codeberg.org>

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/...

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 = generateResponse(msg, statusCode)
-		msg = errorBody(statusCode)
-	} else {
-		// TODO: use template engine
-		msg = strings.ReplaceAll(strings.ReplaceAll(ErrorPage, "%message%", msg), "%status%", http.StatusText(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))
-}

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<img src=1 onerror=alert("xss");`
+	escapedString := "abc&lt;img src=1 onerror=alert(&#34;xss&#34;);"
+	statusCode := http.StatusNotFound
+
+	expected := strings.ReplaceAll(
+		strings.ReplaceAll(ErrorPage, "%message%", escapedString),
+		"%status%",
+		http.StatusText(statusCode))
+	actual := generateResponse(testString, statusCode)
+
+	if expected != actual {
+		t.Errorf("generated response did not match: expected: '%s', got: '%s'", expected, actual)
+	}
+}

integration/get_test.go

@@ -31,7 +31,7 @@ func TestGetRedirect(t *testing.T) {
 func TestGetContent(t *testing.T) {
 	log.Println("=== TestGetContent ===")
 	// test get image
-	resp, err := getTestHTTPSClient().Get("https://magiclike.localhost.mock.directory:4430/images/827679288a.jpg")
+	resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/images/827679288a.jpg")
 	assert.NoError(t, err)
 	if !assert.EqualValues(t, http.StatusOK, resp.StatusCode) {
 		t.FailNow()
@@ -42,7 +42,7 @@ func TestGetContent(t *testing.T) {
 	assert.Len(t, resp.Header.Get("ETag"), 42)
 
 	// specify branch
-	resp, err = getTestHTTPSClient().Get("https://momar.localhost.mock.directory:4430/pag/@master/")
+	resp, err = getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/pag/@master/")
 	assert.NoError(t, err)
 	if !assert.NotNil(t, resp) {
 		t.FailNow()
@@ -53,7 +53,7 @@ func TestGetContent(t *testing.T) {
 	assert.Len(t, resp.Header.Get("ETag"), 44)
 
 	// access branch name contains '/'
-	resp, err = getTestHTTPSClient().Get("https://blumia.localhost.mock.directory:4430/pages-server-integration-tests/@docs~main/")
+	resp, err = getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/blumia/@docs~main/")
 	assert.NoError(t, err)
 	if !assert.EqualValues(t, http.StatusOK, resp.StatusCode) {
 		t.FailNow()
@@ -78,10 +78,28 @@ func TestCustomDomain(t *testing.T) {
 	assert.EqualValues(t, 106, getSize(resp.Body))
 }
 
+func TestCustomDomainRedirects(t *testing.T) {
+	log.Println("=== TestCustomDomainRedirects ===")
+	// test redirect from default pages domain to custom domain
+	resp, err := getTestHTTPSClient().Get("https://6543.localhost.mock.directory:4430/test_pages-server_custom-mock-domain/@main/README.md")
+	assert.NoError(t, err)
+	if !assert.NotNil(t, resp) {
+		t.FailNow()
+	}
+	assert.EqualValues(t, http.StatusTemporaryRedirect, resp.StatusCode)
+	assert.EqualValues(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
+	// TODO: custom port is not evaluated (witch does hurt tests & dev env only)
+	assert.EqualValues(t, "https://mock-pages.codeberg-test.org/@main/README.md", resp.Header.Get("Location"))
+	assert.EqualValues(t, `https:/codeberg.org/6543/test_pages-server_custom-mock-domain/src/branch/main/README.md; rel="canonical"; rel="canonical"`, resp.Header.Get("Link"))
+
+	// TODO: test redirect from an custom domain to the primary custom domain (www.example.com -> example.com)
+	//       (cover bug https://codeberg.org/Codeberg/pages-server/issues/153)
+}
+
 func TestGetNotFound(t *testing.T) {
 	log.Println("=== TestGetNotFound ===")
 	// test custom not found pages
-	resp, err := getTestHTTPSClient().Get("https://crystal.localhost.mock.directory:4430/pages-404-demo/blah")
+	resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/pages-404-demo/blah")
 	assert.NoError(t, err)
 	if !assert.NotNil(t, resp) {
 		t.FailNow()
@@ -95,7 +113,7 @@ func TestGetNotFound(t *testing.T) {
 func TestFollowSymlink(t *testing.T) {
 	log.Printf("=== TestFollowSymlink ===\n")
 
-	resp, err := getTestHTTPSClient().Get("https://6543.localhost.mock.directory:4430/tests_for_pages-server/@main/link")
+	resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/tests_for_pages-server/@main/link")
 	assert.NoError(t, err)
 	if !assert.NotNil(t, resp) {
 		t.FailNow()
@@ -111,7 +129,7 @@ func TestFollowSymlink(t *testing.T) {
 func TestLFSSupport(t *testing.T) {
 	log.Printf("=== TestLFSSupport ===\n")
 
-	resp, err := getTestHTTPSClient().Get("https://6543.localhost.mock.directory:4430/tests_for_pages-server/@main/lfs.txt")
+	resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/tests_for_pages-server/@main/lfs.txt")
 	assert.NoError(t, err)
 	if !assert.NotNil(t, resp) {
 		t.FailNow()

server/certificates/certificates.go

@@ -163,6 +163,9 @@ var acmeClientOrderLimit = equalizer.NewTokenBucket(25, 15*time.Minute)
 // rate limit is 20 / second, we want 5 / second (especially as one cert takes at least two requests)
 var acmeClientRequestLimit = equalizer.NewTokenBucket(5, 1*time.Second)
 
+// rate limit is 5 / hour https://letsencrypt.org/docs/failed-validation-limit/
+var acmeClientFailLimit = equalizer.NewTokenBucket(5, 1*time.Hour)
+
 type AcmeTLSChallengeProvider struct {
 	challengeCache cache.SetGetKey
 }
@@ -278,6 +281,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
 		}
 	}
@@ -298,12 +304,19 @@ 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 {
@@ -529,3 +542,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])
+}

server/handler/handler_custom_domain.go

@@ -54,7 +54,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, dnsLookupCache)
 			if targetOwner != "" {
-				ctx.Redirect("https://"+canonicalDomain+targetOpt.TargetPath, http.StatusTemporaryRedirect)
+				ctx.Redirect("https://"+canonicalDomain+"/"+targetOpt.TargetPath, http.StatusTemporaryRedirect)
 				return
 			}
 

server/upstream/domains.go

@@ -44,9 +44,13 @@ func (o *Options) CheckCanonicalDomain(giteaClient *gitea.Client, actualDomain,
 					valid = true
 				}
 			}
+		} else {
+			if err != gitea.ErrorNotFound {
+				log.Error().Err(err).Msgf("could not read %s of %s/%s", canonicalDomainConfig, o.TargetOwner, o.TargetRepo)
 			} else {
 				log.Info().Err(err).Msgf("could not read %s of %s/%s", canonicalDomainConfig, o.TargetOwner, o.TargetRepo)
 			}
+		}
 		domains = append(domains, o.TargetOwner+mainDomainSuffix)
 		if domains[len(domains)-1] == actualDomain {
 			valid = true