Merge branch 'feature/iam-15-timing-safe-vergleich-als-projektweite-coding-konvention' into feature/qa-05-abnahme-compliance-pruefung-core
This commit is contained in:
@@ -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.
|
||||
Reference in New Issue
Block a user