IAM-15: timing-safe-vergleich-als-projektweite-coding-konvention (internal/timingsafe, coding-guideline, audit bestehender vergleichsstellen)

docs/CODING-GUIDELINES-CORE.md

@@ -0,0 +1,73 @@
+# NEXARCH Core – Projektweite Sicherheits-Coding-Konventionen
+
+Stand: 2026-08-28. Ticket: IAM-15. Ergänzt `docs/TESTSTRATEGIE-CORE.md` (QA-01) um Coding-Regeln,
+die als Code-Review-Checkliste gelten — keine dieser Regeln ist optional oder "nur für ein Modul".
+
+## 1. Warum dieses Dokument existiert
+
+Eine sicherheitsrelevante Coding-Regel, die nur einmal an einer Stelle vorgemacht statt projektweit
+verankert wird, wird beim nächsten neuen Vergleich vergessen. Das gilt für jede Regel in diesem
+Dokument gleichermaßen — die erste Regel (SQL) ist bereits als Konvention etabliert, die zweite
+(timing-safe Vergleich, IAM-15) macht sie hier zum ersten Mal explizit schriftlich.
+
+## 2. Regel: Kein `fmt.Sprintf` für SQL-Bestandteile aus Nutzereingabe
+
+**Spalten-/Tabellennamen ausschließlich aus statischen Konstanten bzw. einem geschlossenen
+Enum/Switch-Typ, nie aus Nutzereingabe oder generischem String-Zusammenbau — auch nicht hinter
+einer Whitelist-Funktion.** Werte (nicht Bezeichner) gehören als Parameter (`$1`, `$2`, …) in die
+Query, niemals interpoliert.
+
+Lehre aus beiden Altsystemen (`known-issues-archivdms.md` Punkt 10, `known-issues-archivmail.md`
+Punkt 12): dynamische Tabellennamen via `fmt.Sprintf`, nur durch eine fragile Whitelist-Funktion
+abgesichert. Siehe DMS/Mail `SRC-11` für die board-spezifische Umsetzung dieser Regel im
+Suchindex-Kontext.
+
+**Referenzbeispiel (korrekt):** `internal/tenant/lifecycle.go`, `ProcessDueDeletions` — Statuswerte
+und IDs ausschließlich als Parameter (`$1`, `$2`, …), niemals interpoliert; der einzige Einsatz von
+`fmt.Sprintf` im Package baut einen **Datenbanknamen aus einem bereits validierten Slug**
+(`dbNameForSlug`, `slugPattern` in `tenant.go` erzwingt `^[a-z][a-z0-9_]{1,48}$` vor jeder
+Verwendung) — keine ungeprüfte Nutzereingabe erreicht die Query.
+
+## 3. Regel: Timing-safe Vergleich für jede sicherheitsrelevante Zugriffsentscheidung (IAM-15)
+
+**Jeder Vergleich, der eine sicherheitsrelevante Zugriffsentscheidung trifft — Passwort-Hash, Token,
+Signatur, 2FA-Code/-Wiederherstellungscode — nutzt einen timing-safe/constant-time Vergleich, nie
+den regulären `==`-Operator.** Ein naiver `==`-Vergleich zweier Byte-Folgen bricht bei der ersten
+abweichenden Stelle ab; die dadurch messbare Laufzeitdifferenz lässt sich aus der Ferne ausmessen und
+erlaubt ein Byte-für-Byte-Erraten des korrekten Werts (Timing-Angriff).
+
+**So wird es gemacht:** `internal/timingsafe` (dieses Ticket) bündelt die kanonische Implementierung
+(`crypto/subtle.ConstantTimeCompare`) für neue Vergleichsstellen:
+
+```go
+import "gitea.perlbach24.de/scripte/nexarch/internal/timingsafe"
+
+if !timingsafe.EqualString(providedCode, expectedCode) {
+    return ErrInvalid
+}
+```
+
+Ausnahme: `bcrypt.CompareHashAndPassword` (Passwort-Hashes) ist bereits von Haus aus timing-safe —
+hier ist kein zusätzlicher Wrapper nötig.
+
+### 3.1 Audit bestehender Vergleichsstellen (Prüfung 2)
+
+Durchgeführt 2026-08-28, Ergebnis: **alle bestehenden sicherheitsrelevanten Vergleichsstellen
+implementierten die Regel bereits korrekt**, unabhängig voneinander mit `crypto/subtle` — nichts
+musste korrigiert werden (Akzeptanzkriterium 3, „ggf.").
+
+| Ort | Was wird verglichen | Fundstelle |
+|---|---|---|
+| `internal/totp/totp.go`, `Validate` | TOTP-Code (2FA) | nutzte bereits `subtle.ConstantTimeCompare` direkt, in diesem Ticket auf `timingsafe.EqualString` umgestellt (erster Verwender des neuen Packages) |
+| `internal/webhook/dispatcher.go`, `VerifySignature` | HMAC-Webhook-Signatur | `subtle.ConstantTimeCompare(expectedBytes, gotBytes)` |
+| `internal/moduleregistry/credentials.go`, `Authenticate` | Service-Credential-Secret-Hash | eigene `timingSafeEqual`-Hilfsfunktion, gleiches Muster |
+| `internal/authtoken/token.go`, `Consume` (Passwort-Reset/Einladung) | Einmal-Token | Hash-Lookup über DB-Index (`WHERE token_hash = $1`), kein manueller Byte-Vergleich nötig — bei zufälligen, hochentropischen Token ist der indexierte Hash-Abgleich gleichwertig sicher |
+
+Neue Vergleichsstellen sollen `internal/timingsafe` verwenden, statt das Muster erneut inline zu
+duplizieren — bestehende Stellen müssen dafür nicht umgebaut werden (kein Umbau angrenzender
+Bereiche über Board-Branch-Grenzen hinweg).
+
+## 4. Wie diese Liste wächst
+
+Neue projektweite Sicherheits-Coding-Regeln werden hier ergänzt, sobald sie (wie SQL-Sprintf und
+timing-safe Vergleich) mehr als einmal unabhängig als Lehre auftauchen — nicht vorab spekulativ.

internal/timingsafe/timingsafe.go

@@ -0,0 +1,30 @@
+// Package timingsafe stellt die kanonische Implementierung der projektweiten
+// Coding-Konvention aus IAM-15 bereit: jeder Vergleich, der eine
+// sicherheitsrelevante Zugriffsentscheidung trifft (Passwort-Hash, Token,
+// Signatur, 2FA-Wiederherstellungscode), nutzt einen timing-safe/constant-time
+// Vergleich, nie den regulaeren ==-Operator. Siehe docs/CODING-GUIDELINES-CORE.md.
+//
+// Bestehende Vergleichsstellen (internal/totp, internal/webhook,
+// internal/moduleregistry) implementieren dasselbe Muster bereits inline mit
+// crypto/subtle direkt — dieses Package buendelt es fuer neue Vergleichsstellen,
+// ersetzt die bestehenden nicht zwangsweise (kein Umbau angrenzender Bereiche).
+package timingsafe
+
+import "crypto/subtle"
+
+// Equal vergleicht zwei Byte-Slices timing-safe. Unterschiedliche Laenge gilt
+// als "nicht gleich", ohne dass die Laufzeit dabei die Laenge verraet, die
+// zum Ergebnis gefuehrt hat, mehr als durch den Laengenunterschied ohnehin
+// unvermeidbar waere.
+func Equal(a, b []byte) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	return subtle.ConstantTimeCompare(a, b) == 1
+}
+
+// EqualString ist die String-Variante von Equal fuer den haeufigen Fall,
+// dass beide Seiten bereits als string vorliegen (z. B. TOTP-Codes).
+func EqualString(a, b string) bool {
+	return Equal([]byte(a), []byte(b))
+}

internal/timingsafe/timingsafe_test.go

@@ -0,0 +1,36 @@
+package timingsafe
+
+import "testing"
+
+func TestEqual_SameBytes(t *testing.T) {
+	if !Equal([]byte("geheimnis"), []byte("geheimnis")) {
+		t.Fatal("identische Byte-Slices sollten gleich sein")
+	}
+}
+
+func TestEqual_DifferentBytes(t *testing.T) {
+	if Equal([]byte("geheimnis"), []byte("anders123")) {
+		t.Fatal("unterschiedliche Byte-Slices sollten ungleich sein")
+	}
+}
+
+func TestEqual_DifferentLength(t *testing.T) {
+	if Equal([]byte("kurz"), []byte("laengererstring")) {
+		t.Fatal("unterschiedliche Laenge sollte immer ungleich sein")
+	}
+}
+
+func TestEqual_EmptyVsEmpty(t *testing.T) {
+	if !Equal([]byte(""), []byte("")) {
+		t.Fatal("zwei leere Slices sollten gleich sein")
+	}
+}
+
+func TestEqualString_MatchesEqual(t *testing.T) {
+	if !EqualString("abc123", "abc123") {
+		t.Fatal("identische Strings sollten gleich sein")
+	}
+	if EqualString("abc123", "xyz789") {
+		t.Fatal("unterschiedliche Strings sollten ungleich sein")
+	}
+}

internal/totp/totp.go

@@ -7,12 +7,13 @@ import (
 	"crypto/hmac"
 	"crypto/rand"
 	"crypto/sha1"
-	"crypto/subtle"
 	"encoding/base32"
 	"encoding/binary"
 	"fmt"
 	"net/url"
 	"time"
+
+	"gitea.perlbach24.de/scripte/nexarch/internal/timingsafe"
 )
 
 // StepSeconds ist das TOTP-Zeitfenster (RFC-6238-Standard: 30 Sekunden).
@@ -70,7 +71,7 @@ func Validate(secret, code string, t time.Time) (bool, error) {
 
 	for delta := -DefaultSkewSteps; delta <= DefaultSkewSteps; delta++ {
 		candidate := hotp(key, uint64(counter+int64(delta)))
-		if subtle.ConstantTimeCompare([]byte(candidate), []byte(code)) == 1 {
+		if timingsafe.EqualString(candidate, code) {
 			return true, nil
 		}
 	}