fix: Audit-Bugfixes (Auth/WAF/Firewall/Cluster/Renderer) — v1.2.94

Verifizierte Bugs aus dem Code-Audit behoben (je mit Test/Build/nft -c geprüft):
- session: IssueWithRoleTTL mutierte geteiltes s.TTL (Data-Race + falsche TTL) → interne issue(); -race-Test.
- auth: Fallback/Federation leiteten role/TOTP nicht aus DB ab (2FA-Bypass auf Secondary, Rolle aus Remote) → viaDB-Flag + DB-Re-Lookup.
- waf: TrustedProxies waren No-op (bogus-Direktive) → XFF-Auflösung im SPOE-Agent (rightmostXFF/ipMatchesAny); RuleExclusions/TrustedProxies validiert (Direktiven-Injection); GetForHost via net.SplitHostPort.
- firewall: Auto-Rule mit IPv6-DstIP erzeugte 'ip daddr <v6>' → bricht ganzes nft-Ruleset; jetzt familienbewusst (ip/ip6, ungültige raus).
- kea: 'interfaces': null bei 0 Subnets → leeres Array.
- cluster_repair: nodeHasPublication schluckte DB-Fehler (Resync auf falschem Node) → (bool,error) fail-closed; IPv6-Primary-URL via net.JoinHostPort.
- cluster_replication: Replikations-Passwort via stdin statt psql -c (nicht mehr in argv/Logs).
- wireguard: Config (Private Key) jetzt configgen.AtomicWrite VOR Symlink/enable; SkipReload-Feld.
- render.go: --no-reload jetzt für alle Renderer (squid/unbound/chrony/wireguard).
- radius: leeres Secret/Passwort + Newlines abgelehnt; freeradius confEscape strippt CR/LF.
- configorch: continue-on-error + errors.Join statt Abbruch mitten in der Sequenz.
- i18n: fehlender Key common.status (de/en).
Verworfen als kein Bug: WAF detection-'blocked' (DetectionOnly liefert keine Interruption), render secrets.New('') (nutzt Default-Masterkey), FanOut-Sort (nur Kommentar), pg_hba (durch nft abgesichert).
Offen/bewusst zurückgestellt (low/risk): AlertWriter-Close (langlebiger Worker, vernachlässigbar), Rolling-Update-Kleinkram (sudoers-gebundener Script-Pfad / GET-State).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Debian
2026-06-06 10:55:21 +02:00
parent 5f92851a96
commit df31bfa720
22 changed files with 338 additions and 97 deletions

View File

@@ -1 +1 @@
1.2.93 1.2.94

View File

@@ -7,6 +7,7 @@ import (
"encoding/json" "encoding/json"
"flag" "flag"
"fmt" "fmt"
"net"
"net/http" "net/http"
"os" "os"
"os/exec" "os/exec"
@@ -359,7 +360,9 @@ END $$;`, egSubName, egSubName, egSubName, egSubName)
"CREATE SUBSCRIPTION %s CONNECTION '%s' PUBLICATION %s WITH (copy_data = true, enabled = true);", "CREATE SUBSCRIPTION %s CONNECTION '%s' PUBLICATION %s WITH (copy_data = true, enabled = true);",
egSubName, connStr, egPubName, egSubName, connStr, egPubName,
) )
if err := psqlDBExec("edgeguard", createSQL); err != nil { // Via stdin (nicht -c), damit das Replikations-Passwort nicht in der
// Prozess-Argv (ps/proc) oder in PG-log_statement landet.
if err := psqlDBExecStdin("edgeguard", createSQL); err != nil {
fmt.Fprintf(os.Stderr, "cluster-setup-standby: create subscription: %v\n", err) fmt.Fprintf(os.Stderr, "cluster-setup-standby: create subscription: %v\n", err)
return 1 return 1
} }
@@ -470,7 +473,7 @@ func fetchReplicationCreds(host string, agentPort int, tlsDir string) (*pgReplic
}, },
} }
url := fmt.Sprintf("https://%s:%d/agent/cluster/pg-replication-info", host, agentPort) url := "https://" + net.JoinHostPort(host, strconv.Itoa(agentPort)) + "/agent/cluster/pg-replication-info"
resp, err := client.Get(url) resp, err := client.Get(url)
if err != nil { if err != nil {
return nil, fmt.Errorf("GET %s: %w", url, err) return nil, fmt.Errorf("GET %s: %w", url, err)
@@ -516,7 +519,7 @@ func syncMasterKey(host string, agentPort int, tlsDir string) error {
}, },
}, },
} }
url := fmt.Sprintf("https://%s:%d/agent/cluster/master-key", host, agentPort) url := "https://" + net.JoinHostPort(host, strconv.Itoa(agentPort)) + "/agent/cluster/master-key"
resp, err := client.Get(url) resp, err := client.Get(url)
if err != nil { if err != nil {
return fmt.Errorf("GET %s: %w", url, err) return fmt.Errorf("GET %s: %w", url, err)
@@ -568,6 +571,17 @@ func psqlDBExec(db, sql string) error {
return err return err
} }
// psqlDBExecStdin führt SQL über stdin (`-f -`) aus statt `-c`, damit
// Secrets im SQL nicht in der Prozess-Argv / PG-Statement-Logs erscheinen.
func psqlDBExecStdin(db, sql string) error {
cmd := buildPsqlCmd([]string{"-d", db, "-v", "ON_ERROR_STOP=1", "-f", "-"})
cmd.Stdin = strings.NewReader(sql)
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("%w: %s", err, strings.TrimSpace(string(out)))
}
return nil
}
// psqlDBRun führt psql-Kommandos gegen eine bestimmte Datenbank aus. // psqlDBRun führt psql-Kommandos gegen eine bestimmte Datenbank aus.
func psqlDBRun(db string, args []string) ([]byte, error) { func psqlDBRun(db string, args []string) ([]byte, error) {
baseArgs := []string{"-d", db} baseArgs := []string{"-d", db}

View File

@@ -68,6 +68,10 @@ func cmdRenderConfig(args []string) int {
if skipReload { if skipReload {
hap.SkipReload = true hap.SkipReload = true
fw.SkipReload = true fw.SkipReload = true
sq.SkipReload = true
wg.SkipReload = true
ub.SkipReload = true
cn.SkipReload = true
ke.SkipReload = true ke.SkipReload = true
fr.SkipReload = true fr.SkipReload = true
} }

View File

@@ -150,6 +150,7 @@ type AutoFWRule struct {
Proto string Proto string
Port int Port int
DstIP string DstIP string
L3 string // "ip"/"ip6" — gesetzt für DstIP-Rules (Familie); leer = agnostisch
Iface string // optional: scope auf ein iifname (z.B. DHCP udp/67 nur auf LAN) Iface string // optional: scope auf ein iifname (z.B. DHCP udp/67 nur auf LAN)
Comment string Comment string
} }
@@ -457,7 +458,21 @@ func (g *Generator) loadAutoRules(ctx context.Context) []AutoFWRule {
} }
} }
return out // Familien-Tag (ip/ip6) für DstIP-basierte Auto-Rules setzen; eine
// IPv6-Listen-Adresse muss `ip6 daddr` ergeben (sonst lehnt nft das
// gesamte Ruleset ab). Unparsebare DstIPs werden verworfen.
tagged := out[:0]
for _, r := range out {
if r.DstIP != "" {
fam := addrFamily(r.DstIP)
if fam == "" {
continue
}
r.L3 = fam
}
tagged = append(tagged, r)
}
return tagged
} }
// splitCSV — wie in den Service-renderern. // splitCSV — wie in den Service-renderern.

View File

@@ -15,7 +15,8 @@ func TestTemplate_autoRuleIface(t *testing.T) {
view := &View{ view := &View{
AutoRules: []AutoFWRule{ AutoRules: []AutoFWRule{
{Proto: "udp", Port: 67, Iface: "eth1", Comment: "DHCP (Kea) auf eth1"}, {Proto: "udp", Port: 67, Iface: "eth1", Comment: "DHCP (Kea) auf eth1"},
{Proto: "udp", Port: 53, DstIP: "10.0.0.1", Comment: "DNS"}, {Proto: "udp", Port: 53, DstIP: "10.0.0.1", L3: "ip", Comment: "DNS"},
{Proto: "udp", Port: 53, DstIP: "2001:db8::1", L3: "ip6", Comment: "DNS v6"},
}, },
} }
var buf bytes.Buffer var buf bytes.Buffer
@@ -27,9 +28,13 @@ func TestTemplate_autoRuleIface(t *testing.T) {
if !strings.Contains(out, `iifname "eth1" udp dport 67 accept comment "auto: DHCP (Kea) auf eth1"`) { if !strings.Contains(out, `iifname "eth1" udp dport 67 accept comment "auto: DHCP (Kea) auf eth1"`) {
t.Errorf("missing iface-scoped DHCP auto-rule\n----\n%s", out) t.Errorf("missing iface-scoped DHCP auto-rule\n----\n%s", out)
} }
// Regression: DstIP-Auto-Rule ohne Iface bleibt unverändert. // v4-DstIP-Auto-Rule: ip daddr.
if !strings.Contains(out, `ip daddr 10.0.0.1 udp dport 53 accept`) { if !strings.Contains(out, `ip daddr 10.0.0.1 udp dport 53 accept`) {
t.Errorf("DstIP auto-rule changed\n----\n%s", out) t.Errorf("v4 DstIP auto-rule wrong\n----\n%s", out)
}
// Fix #5: v6-DstIP muss `ip6 daddr` ergeben (sonst bricht nft das Ruleset).
if !strings.Contains(out, `ip6 daddr 2001:db8::1 udp dport 53 accept`) {
t.Errorf("v6 DstIP auto-rule must use ip6 daddr\n----\n%s", out)
} }
// Echte nft-Syntaxvalidierung (braucht root → via sudo, sonst skip). // Echte nft-Syntaxvalidierung (braucht root → via sudo, sonst skip).

View File

@@ -61,7 +61,7 @@ table inet edgeguard {
# editiert diese nicht. Wenn der Service entfernt/disabled # editiert diese nicht. Wenn der Service entfernt/disabled
# wird, ist die Rule beim nächsten Render weg. # wird, ist die Rule beim nächsten Render weg.
{{range .AutoRules}} {{range .AutoRules}}
{{if .Iface}}iifname "{{.Iface}}" {{end}}{{if .DstIP}}ip daddr {{.DstIP}} {{end}}{{.Proto}} dport {{.Port}} accept comment "auto: {{.Comment}}" {{if .Iface}}iifname "{{.Iface}}" {{end}}{{if .DstIP}}{{.L3}} daddr {{.DstIP}} {{end}}{{.Proto}} dport {{.Port}} accept comment "auto: {{.Comment}}"
{{end}} {{end}}
# ── Operator-defined rules ── # ── Operator-defined rules ──

View File

@@ -43,8 +43,12 @@ func New(pool *pgxpool.Pool, box *secrets.Box) *Generator {
func (g *Generator) Name() string { return "freeradius" } func (g *Generator) Name() string { return "freeradius" }
// confEscape escaped FreeRADIUS-double-quoted-Strings (Backslash + Quote). // confEscape escaped FreeRADIUS-double-quoted-Strings (Backslash + Quote)
// und strippt Steuerzeichen (CR/LF) als Defense-in-Depth gegen Zeilen-
// Injection — die Werte werden zwar schon im Handler validiert.
func confEscape(s string) string { func confEscape(s string) string {
s = strings.ReplaceAll(s, "\r", "")
s = strings.ReplaceAll(s, "\n", "")
s = strings.ReplaceAll(s, `\`, `\\`) s = strings.ReplaceAll(s, `\`, `\\`)
s = strings.ReplaceAll(s, `"`, `\"`) s = strings.ReplaceAll(s, `"`, `\"`)
return s return s

View File

@@ -110,6 +110,7 @@ func (h *AuthHandler) Login(c *gin.Context) {
actor, role := "", "admin" actor, role := "", "admin"
remote := c.ClientIP() remote := c.ClientIP()
var totpEnabled bool var totpEnabled bool
var viaDB bool // true wenn Rolle/TOTP bereits aus der DB-Row stammen
// 1. Try DB users table first. // 1. Try DB users table first.
if h.Users != nil { if h.Users != nil {
@@ -134,6 +135,7 @@ func (h *AuthHandler) Login(c *gin.Context) {
actor = ai.Email actor = ai.Email
role = ai.Role role = ai.Role
totpEnabled = ai.TOTPEnabled totpEnabled = ai.TOTPEnabled
viaDB = true
h.Users.RecordLogin(c.Request.Context(), ai.ID) h.Users.RecordLogin(c.Request.Context(), ai.ID)
} }
} }
@@ -168,6 +170,18 @@ func (h *AuthHandler) Login(c *gin.Context) {
return return
} }
// Bei Fallback (Setup-Store) / Federation (Primary) stammen role/TOTP
// NICHT aus der DB. Rolle + TOTP-Status autoritativ aus der lokalen
// (replizierten) users-Row ableiten — damit 2FA greift und die Rolle
// nie aus einer Remote-Payload kommt. Ist der User lokal (noch) nicht
// vorhanden (Replikations-Lag/DB aus), bleibt es beim Fallback-Wert.
if actor != "" && !viaDB && h.Users != nil {
if ai, err := h.Users.FindForAuth(c.Request.Context(), actor); err == nil {
role = ai.Role
totpEnabled = ai.TOTPEnabled
}
}
// TOTP gate: password OK but 2FA required → issue a short-lived pending // TOTP gate: password OK but 2FA required → issue a short-lived pending
// cookie and tell the UI to show the TOTP input. // cookie and tell the UI to show the TOTP input.
if totpEnabled { if totpEnabled {

View File

@@ -81,7 +81,14 @@ func (h *ClusterHandler) RepairReplication(c *gin.Context) {
return return
} }
if h.nodeHasPublication(ctx) { isPrimary, err := h.nodeHasPublication(ctx)
if err != nil {
// Primary/Subscriber-Status nicht ermittelbar → NICHT raten
// (sonst Resync auf dem falschen Node). Abbrechen.
response.Internal(c, fmt.Errorf("primary-status nicht ermittelbar: %w", err))
return
}
if isPrimary {
// Primary → an den Subscriber-Peer delegieren, mit eigener Adresse. // Primary → an den Subscriber-Peer delegieren, mit eigener Adresse.
if h.Aggregator == nil { if h.Aggregator == nil {
response.BadRequest(c, errors.New("kein mTLS-Aggregator verfügbar — Resync nicht delegierbar")) response.BadRequest(c, errors.New("kein mTLS-Aggregator verfügbar — Resync nicht delegierbar"))
@@ -162,7 +169,12 @@ func (h *ClusterHandler) startResync(ctx context.Context, primaryHost string) er
} }
// Niemals auf dem Primary (Publication-Quelle) resyncen — würde die // Niemals auf dem Primary (Publication-Quelle) resyncen — würde die
// eigene Config mit sich selbst überschreiben bzw. ist sinnlos. // eigene Config mit sich selbst überschreiben bzw. ist sinnlos.
if h.nodeHasPublication(ctx) { // Bei Statusfehler fail-closed (NICHT resyncen).
isPrimary, err := h.nodeHasPublication(ctx)
if err != nil {
return fmt.Errorf("publication-status nicht ermittelbar: %w", err)
}
if isPrimary {
return errors.New("dieser Node ist der Publication-Primary — Resync läuft nur auf einem Subscriber") return errors.New("dieser Node ist der Publication-Primary — Resync läuft nur auf einem Subscriber")
} }
if st := repairUnitState(); st == "activating" || st == "active" { if st := repairUnitState(); st == "activating" || st == "active" {
@@ -201,9 +213,9 @@ rm -f %[2]s
// nodeHasPublication prüft, ob dieser Node die Replikations-Publication // nodeHasPublication prüft, ob dieser Node die Replikations-Publication
// besitzt — das verlässliche Primary-Signal. pg_publication ist für jeden // besitzt — das verlässliche Primary-Signal. pg_publication ist für jeden
// DB-User lesbar (anders als pg_subscription). // DB-User lesbar (anders als pg_subscription).
func (h *ClusterHandler) nodeHasPublication(ctx context.Context) bool { func (h *ClusterHandler) nodeHasPublication(ctx context.Context) (bool, error) {
if h.Store == nil || h.Store.Pool == nil { if h.Store == nil || h.Store.Pool == nil {
return false return false, errors.New("no db pool")
} }
cctx, cancel := context.WithTimeout(ctx, 2*time.Second) cctx, cancel := context.WithTimeout(ctx, 2*time.Second)
defer cancel() defer cancel()
@@ -211,9 +223,9 @@ func (h *ClusterHandler) nodeHasPublication(ctx context.Context) bool {
if err := h.Store.Pool.QueryRow(cctx, if err := h.Store.Pool.QueryRow(cctx,
`SELECT EXISTS(SELECT 1 FROM pg_publication WHERE pubname = $1)`, repairPubName, `SELECT EXISTS(SELECT 1 FROM pg_publication WHERE pubname = $1)`, repairPubName,
).Scan(&exists); err != nil { ).Scan(&exists); err != nil {
return false return false, err
} }
return exists return exists, nil
} }
// repairStatusResponse spiegelt den Zustand der transienten Repair-Unit. // repairStatusResponse spiegelt den Zustand der transienten Repair-Unit.
@@ -231,7 +243,9 @@ type repairStatusResponse struct {
// Status vom Subscriber-Peer geholt (dort läuft der Job); sonst lokal. // Status vom Subscriber-Peer geholt (dort läuft der Job); sonst lokal.
func (h *ClusterHandler) RepairReplicationStatus(c *gin.Context) { func (h *ClusterHandler) RepairReplicationStatus(c *gin.Context) {
ctx := c.Request.Context() ctx := c.Request.Context()
if h.Store != nil && h.nodeHasPublication(ctx) && h.Aggregator != nil { // Status-Poll: bei Fehler kein 500 — einfach lokalen Status liefern.
isPrimary, _ := h.nodeHasPublication(ctx)
if h.Store != nil && isPrimary && h.Aggregator != nil {
if all, err := h.Store.List(ctx); err == nil { if all, err := h.Store.List(ctx); err == nil {
if peer := findOtherPeer(all, h.LocalID); peer != nil { if peer := findOtherPeer(all, h.LocalID); peer != nil {
results := h.Aggregator.FanOut(ctx, results := h.Aggregator.FanOut(ctx,

View File

@@ -148,11 +148,16 @@ func (b *clientBody) validate(creating bool) error {
return errors.New("ipaddr ist keine gültige IP/CIDR: " + b.IPAddr) return errors.New("ipaddr ist keine gültige IP/CIDR: " + b.IPAddr)
} }
} }
if creating && (b.Secret == nil || len(*b.Secret) < 6) { if creating && b.Secret == nil {
return errors.New("secret ist erforderlich (mind. 6 Zeichen)") return errors.New("secret ist erforderlich")
} }
if b.Secret != nil && *b.Secret != "" && len(*b.Secret) < 6 { if b.Secret != nil {
return errors.New("secret muss mind. 6 Zeichen haben") if len(*b.Secret) < 6 {
return errors.New("secret muss mind. 6 Zeichen haben (leer löscht es nicht)")
}
if strings.ContainsAny(*b.Secret, "\r\n") {
return errors.New("secret darf keine Zeilenumbrüche enthalten")
}
} }
return nil return nil
} }
@@ -266,6 +271,14 @@ func (b *userBody) validate(creating bool) error {
if creating && (b.Password == nil || *b.Password == "") { if creating && (b.Password == nil || *b.Password == "") {
return errors.New("password ist erforderlich") return errors.New("password ist erforderlich")
} }
if b.Password != nil {
if *b.Password == "" {
return errors.New("password darf nicht leer sein (löscht es nicht)")
}
if strings.ContainsAny(*b.Password, "\r\n") {
return errors.New("password darf keine Zeilenumbrüche enthalten")
}
}
return nil return nil
} }

View File

@@ -4,8 +4,11 @@ import (
"context" "context"
"errors" "errors"
"log/slog" "log/slog"
"net"
"net/http" "net/http"
"regexp"
"strconv" "strconv"
"strings"
"github.com/gin-gonic/gin" "github.com/gin-gonic/gin"
@@ -15,6 +18,10 @@ import (
wafsvc "git.netcell-it.de/projekte/edgeguard-native/internal/services/waf" wafsvc "git.netcell-it.de/projekte/edgeguard-native/internal/services/waf"
) )
// wafRuleIDRe erlaubt nur einzelne CRS-Rule-IDs oder Ranges ("942100" /
// "942100-942999") als Exclusion — verhindert SecLang-Direktiven-Injection.
var wafRuleIDRe = regexp.MustCompile(`^[0-9]{1,9}(-[0-9]{1,9})?$`)
// WafHandler exposes the per-domain WAF configuration REST API: // WafHandler exposes the per-domain WAF configuration REST API:
// //
// GET /waf/configs — list all configs (one per domain) // GET /waf/configs — list all configs (one per domain)
@@ -110,6 +117,27 @@ func (h *WafHandler) Upsert(c *gin.Context) {
if body.ExclusionNotes == nil { if body.ExclusionNotes == nil {
body.ExclusionNotes = map[string]string{} body.ExclusionNotes = map[string]string{}
} }
// Exclusions müssen reine Rule-IDs/Ranges sein (sonst Direktiven-Injection
// in die SecLang-Config via Newline).
for _, ex := range body.RuleExclusions {
if !wafRuleIDRe.MatchString(strings.TrimSpace(ex)) {
response.BadRequest(c, errors.New("ungültige Rule-Exclusion (nur IDs/Ranges erlaubt): "+ex))
return
}
}
// Trusted-Proxies müssen gültige IPs/CIDRs sein.
for _, p := range body.TrustedProxies {
p = strings.TrimSpace(p)
if p == "" {
continue
}
if net.ParseIP(p) == nil {
if _, _, err := net.ParseCIDR(p); err != nil {
response.BadRequest(c, errors.New("ungültiger Trusted-Proxy (IP/CIDR): "+p))
return
}
}
}
cfg := models.WafConfig{ cfg := models.WafConfig{
DomainID: domainID, DomainID: domainID,
Enabled: body.Enabled, Enabled: body.Enabled,

View File

@@ -119,7 +119,7 @@ func (g *Generator) buildConfig(ctx context.Context) (*keaConfig, *bool, error)
} }
ifaceSet := map[string]bool{} ifaceSet := map[string]bool{}
var ifaces []string ifaces := []string{} // nie nil → JSON "[]" statt "null" (Kea lehnt null ab)
var sn4 []subnet4 var sn4 []subnet4
for _, s := range subnets { for _, s := range subnets {

View File

@@ -44,6 +44,7 @@ func Run(ctx context.Context, gens []configgen.Generator, only []string) ([]Resu
whitelist[n] = true whitelist[n] = true
} }
out := make([]Result, 0, len(gens)) out := make([]Result, 0, len(gens))
var errs []error
for _, g := range gens { for _, g := range gens {
if len(whitelist) > 0 && !whitelist[g.Name()] { if len(whitelist) > 0 && !whitelist[g.Name()] {
out = append(out, Result{Name: g.Name(), Skipped: true}) out = append(out, Result{Name: g.Name(), Skipped: true})
@@ -52,11 +53,14 @@ func Run(ctx context.Context, gens []configgen.Generator, only []string) ([]Resu
err := g.Render(ctx) err := g.Render(ctx)
out = append(out, Result{Name: g.Name(), Err: err}) out = append(out, Result{Name: g.Name(), Err: err})
if err != nil && !errors.Is(err, configgen.ErrNotImplemented) { if err != nil && !errors.Is(err, configgen.ErrNotImplemented) {
// hard failure — surface it but return what's done so far // Weitermachen: die Generatoren sind unabhängig und reloaden
return out, fmt.Errorf("%s: %w", g.Name(), err) // inline (nft/Service-Reload sind atomar). Abbrechen würde die
// restlichen Dienste auf altem Stand lassen → halb angewandt.
// Stattdessen alle versuchen und Fehler gesammelt zurückgeben.
errs = append(errs, fmt.Errorf("%s: %w", g.Name(), err))
} }
} }
return out, nil return out, errors.Join(errs...)
} }
// Summarise turns the result slice into a human-readable multiline // Summarise turns the result slice into a human-readable multiline

View File

@@ -96,14 +96,15 @@ func loadOrCreateSecret(path string) ([]byte, error) {
return secret, nil return secret, nil
} }
// IssueWithRole returns a signed token for the given actor + role. // issue builds + signs a token with an explicit TTL. No shared-state
func (s *Signer) IssueWithRole(actor, role string) (string, *Token, error) { // mutation — safe for concurrent use of the shared Signer singleton.
func (s *Signer) issue(actor, role string, ttl time.Duration) (string, *Token, error) {
now := s.Now() now := s.Now()
t := Token{ t := Token{
Actor: actor, Actor: actor,
Role: role, Role: role,
Iat: now.Unix(), Iat: now.Unix(),
Exp: now.Add(s.TTL).Unix(), Exp: now.Add(ttl).Unix(),
} }
data, err := json.Marshal(t) data, err := json.Marshal(t)
if err != nil { if err != nil {
@@ -117,18 +118,20 @@ func (s *Signer) IssueWithRole(actor, role string) (string, *Token, error) {
return encoded, &t, nil return encoded, &t, nil
} }
// Issue is IssueWithRole with empty role. // IssueWithRole returns a signed token for the given actor + role.
func (s *Signer) Issue(actor string) (string, *Token, error) { func (s *Signer) IssueWithRole(actor, role string) (string, *Token, error) {
return s.IssueWithRole(actor, "") return s.issue(actor, role, s.TTL)
} }
// IssueWithRoleTTL issues a token with a custom TTL (overrides s.TTL for this call). // Issue is IssueWithRole with empty role.
func (s *Signer) Issue(actor string) (string, *Token, error) {
return s.issue(actor, "", s.TTL)
}
// IssueWithRoleTTL issues a token with a custom TTL — no longer mutates
// the shared Signer (previously a data race under concurrent logins).
func (s *Signer) IssueWithRoleTTL(actor, role string, ttl time.Duration) (string, *Token, error) { func (s *Signer) IssueWithRoleTTL(actor, role string, ttl time.Duration) (string, *Token, error) {
orig := s.TTL return s.issue(actor, role, ttl)
s.TTL = ttl
raw, tok, err := s.IssueWithRole(actor, role)
s.TTL = orig
return raw, tok, err
} }
// Verify checks a token. Returns ErrInvalidToken or ErrExpiredToken. // Verify checks a token. Returns ErrInvalidToken or ErrExpiredToken.

View File

@@ -0,0 +1,47 @@
package session
import (
"sync"
"sync/atomic"
"testing"
"time"
)
// TestSigner_TTLNotShared beweist Fix #1: IssueWithRoleTTL darf das geteilte
// s.TTL nicht mehr mutieren. Unter `go test -race` schlägt die alte Version
// als Data-Race an; zusätzlich prüfen wir, dass parallele normale Logins nie
// die kurze TOTP-TTL erben.
func TestSigner_TTLNotShared(t *testing.T) {
s := NewSigner([]byte("0123456789abcdef0123456789abcdef"), nil, time.Hour)
var wg sync.WaitGroup
var bad int32
for i := 0; i < 200; i++ {
wg.Add(2)
go func() {
defer wg.Done()
_, _, _ = s.IssueWithRoleTTL("a", "totp_pending", 2*time.Minute)
}()
go func() {
defer wg.Done()
_, tok, err := s.IssueWithRole("b", "admin")
if err != nil {
atomic.AddInt32(&bad, 1)
return
}
// Normale Session muss ~1h gelten, nie die 2-Min-TOTP-TTL.
if tok.Exp-tok.Iat < int64((30 * time.Minute).Seconds()) {
atomic.AddInt32(&bad, 1)
}
}()
}
wg.Wait()
if bad > 0 {
t.Fatalf("%d normale Tokens bekamen eine zu kurze TTL → geteilter Zustand", bad)
}
// TTL-Override wirkt weiterhin korrekt für den TOTP-Token.
_, ptok, _ := s.IssueWithRoleTTL("x", "totp_pending", 2*time.Minute)
if d := ptok.Exp - ptok.Iat; d > int64((3 * time.Minute).Seconds()) {
t.Fatalf("totp-pending TTL = %ds, want ~120s", d)
}
}

View File

@@ -66,14 +66,10 @@ func buildDirectives(cfg models.WafConfig, crsDir string) string {
} }
} }
// Trusted proxies: tell Coraza to trust X-Forwarded-For from these IPs. // Trusted proxies are NOT a SecLang directive — they are applied in the
for _, ip := range cfg.TrustedProxies { // SPOE agent (spoe.go): when the connection source is a trusted proxy,
ip = strings.TrimSpace(ip) // the real client IP is taken from X-Forwarded-For before Coraza sees
if ip != "" { // it. (Previously this loop emitted a bogus, unrelated directive.)
sb.WriteString(fmt.Sprintf("SecRemoteRulesFailAction Abort\n"))
_ = ip // used in custom rules below if needed
}
}
// Custom rules (appended last so they can override CRS). // Custom rules (appended last so they can override CRS).
if strings.TrimSpace(cfg.CustomRules) != "" { if strings.TrimSpace(cfg.CustomRules) != "" {

View File

@@ -3,6 +3,7 @@ package waf
import ( import (
"fmt" "fmt"
"log/slog" "log/slog"
"net"
"sync" "sync"
"github.com/corazawaf/coraza/v3" "github.com/corazawaf/coraza/v3"
@@ -12,8 +13,9 @@ import (
// DomainEngine bundles a Coraza WAF with its operating mode. // DomainEngine bundles a Coraza WAF with its operating mode.
type DomainEngine struct { type DomainEngine struct {
WAF coraza.WAF WAF coraza.WAF
Mode string // "detection" | "blocking" Mode string // "detection" | "blocking"
TrustedProxies []string // wenn src ∈ diese → echte Client-IP aus X-Forwarded-For
} }
// Manager holds per-domain Coraza engine instances. Engines are // Manager holds per-domain Coraza engine instances. Engines are
@@ -90,7 +92,7 @@ func (m *Manager) Reload(domains []DomainConfig) error {
if err != nil { if err != nil {
return fmt.Errorf("waf: build engine for %s: %w", dc.Hostname, err) return fmt.Errorf("waf: build engine for %s: %w", dc.Hostname, err)
} }
newEngines[dc.Hostname] = &DomainEngine{WAF: waf, Mode: dc.Config.Mode} newEngines[dc.Hostname] = &DomainEngine{WAF: waf, Mode: dc.Config.Mode, TrustedProxies: dc.Config.TrustedProxies}
slog.Info("waf: engine (re)loaded", slog.Info("waf: engine (re)loaded",
"host", dc.Hostname, "host", dc.Hostname,
"mode", dc.Config.Mode, "mode", dc.Config.Mode,
@@ -110,8 +112,9 @@ func (m *Manager) Reload(domains []DomainConfig) error {
// (nil, false) when the domain has no WAF or WAF is disabled. // (nil, false) when the domain has no WAF or WAF is disabled.
func (m *Manager) GetForHost(host string) (*DomainEngine, bool) { func (m *Manager) GetForHost(host string) (*DomainEngine, bool) {
// Strip port if present (e.g. "example.com:443" → "example.com"). // Strip port if present (e.g. "example.com:443" → "example.com").
if i := lastColon(host); i >= 0 { // SplitHostPort errors for a bare host or bare IPv6 literal → keep as-is.
host = host[:i] if h, _, err := net.SplitHostPort(host); err == nil {
host = h
} }
m.mu.RLock() m.mu.RLock()
de, ok := m.engines[host] de, ok := m.engines[host]
@@ -122,36 +125,3 @@ func (m *Manager) GetForHost(host string) (*DomainEngine, bool) {
return de, true return de, true
} }
// lastColon returns the index of the last ':' in s that looks like a
// port separator (after the final ']' for IPv6), or -1.
func lastColon(s string) int {
// IPv6 addresses in brackets: "[::1]:443"
if len(s) > 0 && s[0] == '[' {
if rb := lastByte(s, ']'); rb >= 0 && rb < len(s)-1 && s[rb+1] == ':' {
return rb + 1
}
return -1
}
// Plain host — only strip port if there's exactly one colon.
count := 0
idx := -1
for i, c := range s {
if c == ':' {
count++
idx = i
}
}
if count == 1 {
return idx
}
return -1
}
func lastByte(s string, b byte) int {
for i := len(s) - 1; i >= 0; i-- {
if s[i] == b {
return i
}
}
return -1
}

View File

@@ -3,6 +3,7 @@ package waf
import ( import (
"context" "context"
"log/slog" "log/slog"
"net"
"net/http" "net/http"
"strings" "strings"
@@ -75,6 +76,15 @@ func (a *SPOEAgent) handle(ctx context.Context, w *encoding.ActionWriter, m *enc
return // WAF not configured or disabled for this domain return // WAF not configured or disabled for this domain
} }
// Trusted-Proxy-Handling: stammt die Verbindung von einem konfigurierten
// Trusted-Proxy, ist die echte Client-IP das letzte X-Forwarded-For-Glied
// (das der Proxy angehängt hat), nicht die Proxy-IP selbst.
if clientIP != "" && len(de.TrustedProxies) > 0 && ipMatchesAny(clientIP, de.TrustedProxies) {
if real := rightmostXFF(rawHdrs); real != "" {
clientIP = real
}
}
tx := de.WAF.NewTransaction() tx := de.WAF.NewTransaction()
defer func() { defer func() {
tx.ProcessLogging() tx.ProcessLogging()
@@ -165,6 +175,53 @@ func (a *SPOEAgent) sendAlert(host, clientIP, method, uri string, mr types.Match
}) })
} }
// rightmostXFF gibt den letzten (vom nächstgelegenen Proxy angehängten)
// X-Forwarded-For-Eintrag zurück, sofern es eine gültige IP ist.
func rightmostXFF(rawHdrs string) string {
var val string
for _, line := range strings.Split(rawHdrs, "\n") {
line = strings.TrimRight(line, "\r")
idx := strings.IndexByte(line, ':')
if idx <= 0 {
continue
}
if strings.EqualFold(strings.TrimSpace(line[:idx]), "x-forwarded-for") {
val = strings.TrimSpace(line[idx+1:]) // letzter XFF-Header gewinnt
}
}
if val == "" {
return ""
}
parts := strings.Split(val, ",")
cand := strings.TrimSpace(parts[len(parts)-1])
if net.ParseIP(cand) == nil {
return ""
}
return cand
}
// ipMatchesAny prüft, ob ip exakt einer IP oder einem CIDR aus list entspricht.
func ipMatchesAny(ip string, list []string) bool {
parsed := net.ParseIP(ip)
if parsed == nil {
return false
}
for _, e := range list {
e = strings.TrimSpace(e)
if e == "" {
continue
}
if strings.Contains(e, "/") {
if _, n, err := net.ParseCIDR(e); err == nil && n.Contains(parsed) {
return true
}
} else if pe := net.ParseIP(e); pe != nil && pe.Equal(parsed) {
return true
}
}
return false
}
// parseHeaders splits HAProxy raw headers ("Name: value\r\n…") and // parseHeaders splits HAProxy raw headers ("Name: value\r\n…") and
// calls fn for each valid header line. // calls fn for each valid header line.
func parseHeaders(raw string, fn func(name, val string)) { func parseHeaders(raw string, fn func(name, val string)) {

37
internal/waf/spoe_test.go Normal file
View File

@@ -0,0 +1,37 @@
package waf
import "testing"
// Beweist Fix #2: Trusted-Proxy-XFF-Auflösung.
func TestRightmostXFF(t *testing.T) {
cases := map[string]string{
"X-Forwarded-For: 203.0.113.7": "203.0.113.7",
"X-Forwarded-For: 203.0.113.7, 10.0.0.1": "10.0.0.1", // rightmost
"x-forwarded-for: 1.2.3.4 , 5.6.7.8": "5.6.7.8",
"Host: x\r\nX-Forwarded-For: 2001:db8::1": "2001:db8::1",
"X-Forwarded-For: not-an-ip": "",
"User-Agent: foo": "",
"": "",
}
for raw, want := range cases {
if got := rightmostXFF(raw); got != want {
t.Errorf("rightmostXFF(%q) = %q, want %q", raw, got, want)
}
}
}
func TestIPMatchesAny(t *testing.T) {
list := []string{"10.0.0.5", "192.168.0.0/16", "2001:db8::/32"}
yes := []string{"10.0.0.5", "192.168.4.7", "2001:db8::abcd"}
no := []string{"10.0.0.6", "172.16.0.1", "2002::1", "garbage"}
for _, ip := range yes {
if !ipMatchesAny(ip, list) {
t.Errorf("ipMatchesAny(%q) = false, want true", ip)
}
}
for _, ip := range no {
if ipMatchesAny(ip, list) {
t.Errorf("ipMatchesAny(%q) = true, want false", ip)
}
}
}

View File

@@ -19,6 +19,7 @@ import (
"github.com/jackc/pgx/v5/pgxpool" "github.com/jackc/pgx/v5/pgxpool"
"git.netcell-it.de/projekte/edgeguard-native/internal/configgen"
"git.netcell-it.de/projekte/edgeguard-native/internal/models" "git.netcell-it.de/projekte/edgeguard-native/internal/models"
"git.netcell-it.de/projekte/edgeguard-native/internal/services/secrets" "git.netcell-it.de/projekte/edgeguard-native/internal/services/secrets"
wgsvc "git.netcell-it.de/projekte/edgeguard-native/internal/services/wireguard" wgsvc "git.netcell-it.de/projekte/edgeguard-native/internal/services/wireguard"
@@ -27,10 +28,11 @@ import (
const ConfDir = "/etc/edgeguard/wireguard" const ConfDir = "/etc/edgeguard/wireguard"
type Generator struct { type Generator struct {
Pool *pgxpool.Pool Pool *pgxpool.Pool
Box *secrets.Box Box *secrets.Box
Ifaces *wgsvc.InterfacesRepo Ifaces *wgsvc.InterfacesRepo
Peers *wgsvc.PeersRepo Peers *wgsvc.PeersRepo
SkipReload bool // nur Configs schreiben, keine wg-quick@-Service-Aktionen
} }
func New(pool *pgxpool.Pool, box *secrets.Box) *Generator { func New(pool *pgxpool.Pool, box *secrets.Box) *Generator {
@@ -151,8 +153,10 @@ func (g *Generator) Render(ctx context.Context) error {
continue continue
} }
_ = os.Remove(filepath.Join(ConfDir, e.Name())) _ = os.Remove(filepath.Join(ConfDir, e.Name()))
_ = stopWGQuick(ifaceName) if !g.SkipReload {
_ = disableWGQuick(ifaceName) _ = stopWGQuick(ifaceName)
_ = disableWGQuick(ifaceName)
}
} }
} }
return nil return nil
@@ -228,21 +232,31 @@ func (g *Generator) renderIface(ctx context.Context, ifc models.WireguardInterfa
} }
path := filepath.Join(ConfDir, ifc.Name+".conf") path := filepath.Join(ConfDir, ifc.Name+".conf")
// Config (enthält den Private Key) ZUERST atomar schreiben — vorher
// keinen Symlink/Service auf eine evtl. fehlende/abgeschnittene Datei
// zeigen lassen. AtomicWrite = temp+fsync+rename, 0600.
changed := true
if existing, err := os.ReadFile(path); err == nil && bytes.Equal(existing, body.Bytes()) {
changed = false
}
if changed {
if err := configgen.AtomicWrite(path, body.Bytes(), 0o600); err != nil {
return fmt.Errorf("write %s: %w", path, err)
}
}
if g.SkipReload {
return nil
}
// wg-quick@<iface>.service liest /etc/wireguard/<iface>.conf (Distro- // wg-quick@<iface>.service liest /etc/wireguard/<iface>.conf (Distro-
// Default), nicht unseren ConfDir. Wir lassen die Quelle of truth in // Default), nicht unseren ConfDir. Symlink via sudo (/etc/wireguard/
// /etc/edgeguard/wireguard/ und symlinken via sudo — /etc/wireguard/ // ist root:root 700). Das sudoers-Entry wird von postinst angelegt.
// ist root:root 700, daher braucht es sudo /bin/ln. Das sudoers-Entry
// wird von postinst angelegt.
if err := symlinkWGQuickConf(ifc.Name, path); err != nil { if err := symlinkWGQuickConf(ifc.Name, path); err != nil {
return fmt.Errorf("symlink: %w", err) return fmt.Errorf("symlink: %w", err)
} }
_ = enableWGQuick(ifc.Name) _ = enableWGQuick(ifc.Name)
if existing, err := os.ReadFile(path); err == nil && bytes.Equal(existing, body.Bytes()) { if !changed {
return startWGQuick(ifc.Name) return startWGQuick(ifc.Name)
} }
if err := os.WriteFile(path, body.Bytes(), 0o600); err != nil {
return fmt.Errorf("write %s: %w", path, err)
}
return restartWGQuick(ifc.Name) return restartWGQuick(ifc.Name)
} }

View File

@@ -1372,6 +1372,7 @@
"yes": "Ja", "yes": "Ja",
"no": "Nein", "no": "Nein",
"or": "oder", "or": "oder",
"status": "Status",
"save": "Speichern", "save": "Speichern",
"cancel": "Abbrechen", "cancel": "Abbrechen",
"loading": "Lädt …", "loading": "Lädt …",

View File

@@ -1372,6 +1372,7 @@
"yes": "Yes", "yes": "Yes",
"no": "No", "no": "No",
"or": "or", "or": "or",
"status": "Status",
"save": "Save", "save": "Save",
"cancel": "Cancel", "cancel": "Cancel",
"loading": "Loading …", "loading": "Loading …",