Fix all open CodeQL code scanning alerts

pkg/diagnosticlog/diagnosticlog.go

@@ -21,7 +21,9 @@ func SQLSummary(query string) string {
 }
 
 func SafeLogValue(rawValue string) string {
-	cleanValue := strings.TrimSpace(rawValue)
+	cleanValue := strings.ReplaceAll(rawValue, "\r", "")
+	cleanValue = strings.ReplaceAll(cleanValue, "\n", "")
+	cleanValue = strings.TrimSpace(cleanValue)
 	if cleanValue == "" {
 		return "-"
 	}

pkg/diagnosticlog/diagnosticlog_test.go

@@ -0,0 +1,16 @@
+package diagnosticlog
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestSafeLogValueRemovesLogEntryDelimiters(t *testing.T) {
+	safeValue := SafeLogValue("first\r\nforged\x00entry")
+	if strings.ContainsAny(safeValue, "\r\n\x00") {
+		t.Fatalf("safe log value contains a control character: %q", safeValue)
+	}
+	if safeValue != "firstforgedentry" {
+		t.Fatalf("safe log value = %q, want %q", safeValue, "firstforgedentry")
+	}
+}

pkg/dirprotect/dirprotect.go

@@ -1,13 +1,16 @@
 package dirprotect
 
 import (
+	"crypto/hmac"
 	"crypto/sha256"
-	"crypto/subtle"
 	"fmt"
+	"hash/fnv"
 	"path"
 	"strconv"
 	"strings"
 	"time"
+
+	"golang.org/x/crypto/bcrypt"
 )
 
 type Rule struct {
@@ -40,18 +43,29 @@ func HasProtectedPrefix(pagePath, protectedPrefix string) bool {
 	return pagePath == protectedPrefix || strings.HasPrefix(pagePath, protectedPrefix+"/")
 }
 
-func Hash(password string) string {
-	hashedBytes := sha256.Sum256([]byte("sitebrush page password\n" + password))
-	return fmt.Sprintf("sha256:%x", hashedBytes)
+func Hash(password string) (string, error) {
+	hashedBytes, err := bcrypt.GenerateFromPassword(passwordPrehash(password), bcrypt.DefaultCost)
+	if err != nil {
+		return "", fmt.Errorf("hash page password: %w", err)
+	}
+	return string(hashedBytes), nil
 }
 
 func Matches(storedHash, password string) bool {
-	return strings.TrimSpace(storedHash) == Hash(password)
+	err := bcrypt.CompareHashAndPassword([]byte(strings.TrimSpace(storedHash)), passwordPrehash(password))
+	return err == nil
+}
+
+func passwordPrehash(password string) []byte {
+	prehash := hmac.New(sha256.New, []byte("sitebrush page password bcrypt prehash v1"))
+	_, _ = prehash.Write([]byte(password))
+	return prehash.Sum(nil)
 }
 
 func CookieName(domain, pagePath string) string {
-	hashedBytes := sha256.Sum256([]byte(NormalizeDomain(domain) + "\n" + CleanPath(pagePath)))
-	return "sitebrush_page_password_" + fmt.Sprintf("%x", hashedBytes)[:16]
+	cookieIdentifier := fnv.New64a()
+	_, _ = cookieIdentifier.Write([]byte(NormalizeDomain(domain) + "\n" + CleanPath(pagePath)))
+	return "sitebrush_page_password_" + fmt.Sprintf("%016x", cookieIdentifier.Sum64())
 }
 
 func BoundSessionToken(rule Rule, clientIP, userAgent string, issuedAt time.Time) string {
@@ -80,7 +94,7 @@ func BoundSessionTokenValid(rule Rule, token, clientIP, userAgent string, now ti
 		return false
 	}
 	expectedToken := BoundSessionToken(rule, clientIP, userAgent, issuedAt)
-	return subtle.ConstantTimeCompare([]byte(expectedToken), []byte(strings.TrimSpace(token))) == 1
+	return hmac.Equal([]byte(expectedToken), []byte(strings.TrimSpace(token)))
 }
 
 func FailureDomain(domain, pagePath string) string {
@@ -171,13 +185,13 @@ func NormalizeDomain(domain string) string {
 }
 
 func boundSessionSignature(rule Rule, clientIP string, issuedUnix int64) string {
-	hashedBytes := sha256.Sum256([]byte(strings.Join([]string{
+	signature := hmac.New(sha256.New, []byte(strings.TrimSpace(rule.PasswordHash)))
+	_, _ = signature.Write([]byte(strings.Join([]string{
 		"sitebrush page password session v2",
 		NormalizeDomain(rule.Domain),
 		CleanPath(rule.Path),
-		strings.TrimSpace(rule.PasswordHash),
 		strings.TrimSpace(clientIP),
 		strconv.FormatInt(issuedUnix, 10),
 	}, "\n")))
-	return fmt.Sprintf("%x", hashedBytes)
+	return fmt.Sprintf("%x", signature.Sum(nil))
 }

pkg/dirprotect/dirprotect_test.go

@@ -8,8 +8,8 @@ import (
 
 func TestFindBestRuleUsesPathBoundaries(t *testing.T) {
 	rules := []Rule{
-		{Domain: "localhost", Path: "/passport", PasswordHash: Hash("secret")},
-		{Domain: "localhost", Path: "/passport/deep", PasswordHash: Hash("deeper")},
+		{Domain: "localhost", Path: "/passport", PasswordHash: mustHashForTest(t, "secret")},
+		{Domain: "localhost", Path: "/passport/deep", PasswordHash: mustHashForTest(t, "deeper")},
 	}
 
 	rule, found := FindBestRule("localhost", "/passport/deep/page", rules)
@@ -26,7 +26,7 @@ func TestFindBestRuleUsesPathBoundaries(t *testing.T) {
 }
 
 func TestPrefixFileRoundTrip(t *testing.T) {
-	rule := Rule{Domain: "localhost", Path: "/passport", PasswordHash: Hash("secret")}
+	rule := Rule{Domain: "localhost", Path: "/passport", PasswordHash: mustHashForTest(t, "secret")}
 	body := PrefixFileBody([]Rule{rule})
 	parsedRule, found := FindBestRuleInPrefixData("localhost", "/passport/one", body)
 	if !found {
@@ -41,7 +41,7 @@ func TestPrefixFileRoundTrip(t *testing.T) {
 }
 
 func TestBoundSessionTokenRequiresSameClientConditions(t *testing.T) {
-	rule := Rule{Domain: "Example.COM.", Path: "/passport", PasswordHash: Hash("secret")}
+	rule := Rule{Domain: "Example.COM.", Path: "/passport", PasswordHash: mustHashForTest(t, "secret")}
 	issuedAt := time.Unix(1_700_000_000, 0).UTC()
 	token := BoundSessionToken(rule, "198.51.100.10", "Test Browser", issuedAt)
 	if !strings.HasPrefix(token, "v2:") {
@@ -59,10 +59,36 @@ func TestBoundSessionTokenRequiresSameClientConditions(t *testing.T) {
 }
 
 func TestBoundSessionTokenExpiresAfterTTL(t *testing.T) {
-	rule := Rule{Domain: "localhost", Path: "/passport", PasswordHash: Hash("secret")}
+	rule := Rule{Domain: "localhost", Path: "/passport", PasswordHash: mustHashForTest(t, "secret")}
 	issuedAt := time.Unix(1_700_000_000, 0).UTC()
 	token := BoundSessionToken(rule, "198.51.100.10", "Test Browser", issuedAt)
 	if BoundSessionTokenValid(rule, token, "198.51.100.10", "Test Browser", issuedAt.Add(time.Hour+time.Second), time.Hour) {
 		t.Fatal("token should expire after the configured TTL")
 	}
 }
+
+func TestHashUsesPasswordHashingAlgorithm(t *testing.T) {
+	password := strings.Repeat("long password ", 20)
+	passwordHash := mustHashForTest(t, password)
+	if strings.HasPrefix(passwordHash, "sha256:") {
+		t.Fatalf("password hash uses legacy fast digest: %s", passwordHash)
+	}
+	if !Matches(passwordHash, password) {
+		t.Fatal("password hash did not match its password")
+	}
+	if Matches(passwordHash, "incorrect") {
+		t.Fatal("password hash matched an incorrect password")
+	}
+	if Matches("sha256:09e591c2a216e7264dd10d10b65a0092d7a0c5b5c9927642df8c29c10c30aeb2", "secret") {
+		t.Fatal("legacy fast password digest was accepted")
+	}
+}
+
+func mustHashForTest(t *testing.T, password string) string {
+	t.Helper()
+	passwordHash, err := Hash(password)
+	if err != nil {
+		t.Fatalf("hash password: %v", err)
+	}
+	return passwordHash
+}