scripte/nexarch
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(mail): QA-04 Sicherheits-/Berechtigungsprüfung — fehlenden SMTP-Idle-Timeout gefunden und behoben

Browse Source 
Gezielter Testangriff auf den SMTP-Pfad deckte einen realen
Härtungsfehler auf: ING-07 (Idle-Timeout via protoguard) wurde
versehentlich nur in mail/internal/imap und mail/internal/pop3
verdrahtet, SMTP bekam nie einen Timeout. Eine Gegenstelle, die eine
Kommandozeile ohne abschließendes CRLF öffnet und nie beendet, konnte
die Session unbegrenzt blockieren — real reproduziert und danach
behoben.

session.go/server.go (smtp): guard *protoguard.Guard neu, Timeout wird
in readLine() selbst gesetzt (ein Ort für Haupt-Serve-Schleife,
handleData, drainUntilDot). Neuer Konstruktor
NewServerWithMaxMessageBytesTLSLoggerRateLimitAndGuardConfig für
abweichende Timeout-Werte. Bestehende Konstruktoren bekommen automatisch
protoguard.DefaultConfig() (5 Minuten) statt wie zuvor gar keinen
Timeout — reine Härtung, keine Verhaltensänderung für funktionierende
Clients, QA-07-Lasttest bleibt unverändert grün.

Neue Tests: qa04_security_test.go (Header-Injection-Angriffe auf
Envelope-Adressen, Ressourcenerschöpfung durch nie abgeschlossene Zeile
— deckte den Fehler auf und bestätigt die Korrektur).
mailboxconfig/tenant_scoping_test.go: Stichprobe eines dritten
Speicherpfads (verschlüsselte IMAP-Zugangsdaten) — Zugriff mit echter,
bekannter fremder ID wird über alle vier Operationen zuverlässig
abgelehnt.

Rate-Limiting-Teil von Akzeptanzkriterium 3 real bestätigt (ING-09,
erneut mitgeprüft). API-Token-Teil bleibt offen: das Mail-Board besitzt
keine eigene Token-Authentifizierung, bewusst an Core-Board IAM
delegiert (QA-04s eigene Ausgangslage) — im Prüfprotokoll dokumentiert.

go build/go vet/golangci-lint clean, gesamtes Mail-Modul
regressionsfrei getestet.
This commit is contained in:
sysops
2026-09-01 14:24:56 +02:00
parent e19003b5d9
commit 0505351e8f
5 changed files with 408 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mail/internal/smtp/qa04_security_test.go
+147
View File
@@ -0,0 +1,147 @@
// QA-04: Sicherheits- und Berechtigungsprüfung, SMTP-Härtung gegen
// bekannte Angriffsmuster (Pflichtprüfung: "Gezielter Testangriff auf
// Header-Injection schlägt fehl"). Ergänzt ING-03/ING-06/ING-09s
// bestehende Tests um einen eigenständigen, gezielten Angriffslauf
// gegen den echten, laufenden Server.
package smtp
import (
"bufio"
"context"
"net"
"strings"
"testing"
"time"
"gitea.perlbach24.de/scripte/nexarch/mail/internal/protoguard"
)
// TestQA04_HeaderInjectionViaEnvelopeAddressRejected ist die geforderte
// Pflichtprüfung: ein gezielter Versuch, über die Envelope-Adressen
// (MAIL FROM/RCPT TO) Steuerzeichen einzuschleusen, die bei
// unvorsichtiger späterer Header-Konstruktion (z. B. durch einen
// nachgelagerten Mailer, siehe ING-03 mail/internal/mailer) zu
// Header-Injection führen könnten, schlägt bereits auf
// SMTP-Envelope-Ebene fehl.
func TestQA04_HeaderInjectionViaEnvelopeAddressRejected(t *testing.T) {
sink := &fakeSink{}
addr, stop := startTestServer(t, sink, defaultMaxMessageBytes)
defer stop()
attacks := []struct {
name string
cmd string
}{
{"NUL-Byte in RCPT TO", "RCPT TO:<a\x00@example.com>"},
{"Steuerzeichen 0x01 in MAIL FROM", "MAIL FROM:<a\x01@example.com>"},
}
for _, atk := range attacks {
t.Run(atk.name, func(t *testing.T) {
c := dial(t, addr)
defer c.close()
c.send(t, "EHLO client.example.com")
for {
line := c.readLine(t)
if strings.HasPrefix(line, "250 ") {
break
}
}
c.send(t, "MAIL FROM:<a@example.com>")
resp := c.send(t, atk.cmd)
if code(resp) != "553" && code(resp) != "501" {
t.Fatalf("angriff %q wurde NICHT sauber zurückgewiesen: %q", atk.name, resp)
}
// Verbindung muss danach weiter funktionsfähig sein — kein
// Absturz, kein hängender Zustand durch den Angriffsversuch.
if resp := c.send(t, "NOOP"); code(resp) != "250" {
t.Fatalf("session nach angriff %q nicht mehr funktionsfähig: %q", atk.name, resp)
}
})
}
if sink.count() != 0 {
t.Fatalf("keiner der angriffsversuche hätte eine nachricht bis zum sink durchlassen dürfen, habe %d", sink.count())
}
}
// TestQA04_OversizedLineWithoutCRLFDoesNotHangOrCrash ist ein
// zusätzlicher, gezielter Testangriff (bekanntes Muster: Ressourcen-
// erschöpfung durch eine Kommandozeile ohne Zeilenende) — bestätigt,
// dass der Server bei einer weit über die dokumentierte Zeilengrenze
// (maxCommandLineBytes) hinausgehenden, nie abgeschlossenen Zeile
// zuverlässig reagiert (Verbindungsabbruch/Timeout) statt unbegrenzt
// Speicher zu binden oder zu hängen.
func TestQA04_OversizedLineWithoutCRLFDoesNotHangOrCrash(t *testing.T) {
sink := &fakeSink{}
// Kurzer Idle-Timeout statt protoguard.DefaultConfig()s 5 Minuten —
// sonst würde dieser Test selbst bei korrektem Verhalten Minuten
// brauchen, um die Reaktion des Servers zu beobachten.
guardCfg := protoguard.Config{DefaultTimeout: 500 * time.Millisecond}
srv := NewServerWithMaxMessageBytesTLSLoggerRateLimitAndGuardConfig(sink, defaultMaxMessageBytes, nil, nil, "mandant-qa04-guard", nil, guardCfg)
listener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("listener: %v", err)
}
ctx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
go func() {
_ = srv.Serve(ctx, listener)
close(done)
}()
addr := listener.Addr().String()
defer func() {
cancel()
<-done
}()
conn, err := net.DialTimeout("tcp", addr, 2*time.Second)
if err != nil {
t.Fatalf("dial: %v", err)
}
defer func() { _ = conn.Close() }()
reader := bufio.NewReader(conn)
if _, err := reader.ReadString('\n'); err != nil {
t.Fatalf("begrüßung lesen: %v", err)
}
// Weit über maxCommandLineBytes (8192) hinaus, absichtlich OHNE
// abschließendes CRLF — simuliert eine Gegenstelle, die niemals eine
// Zeile abschließt.
huge := strings.Repeat("A", 10*maxCommandLineBytes)
writeDone := make(chan struct{})
var writeErr error
go func() {
defer close(writeDone)
_ = conn.SetWriteDeadline(time.Now().Add(5 * time.Second))
_, writeErr = conn.Write([]byte(huge))
}()
writeCtx, writeCancel := context.WithTimeout(context.Background(), 8*time.Second)
defer writeCancel()
select {
case <-writeDone:
case <-writeCtx.Done():
t.Fatalf("schreiben der überlangen zeile blockierte unerwartet lange (>8s) — hinweis auf hängenden server")
}
if writeErr != nil {
// Verbindungsabbruch durch den Server während des Schreibens ist
// ein AKZEPTABLES Ergebnis (frühes Erkennen), kein Testfehler.
t.Logf("verbindung während des schreibens der überlangen zeile beendet (akzeptabel): %v", writeErr)
return
}
// Server muss innerhalb einer begrenzten Zeit reagieren (Fehler oder
// Verbindungsende) — NICHT unbegrenzt auf ein CRLF warten, das nie
// kommt.
_ = conn.SetReadDeadline(time.Now().Add(8 * time.Second))
_, err = reader.ReadString('\n')
if err == nil {
t.Fatalf("erwartete verbindungsende/fehler nach überlanger zeile ohne CRLF, server hat stattdessen scheinbar eine antwort geliefert")
}
if ne, ok := err.(net.Error); ok && ne.Timeout() {
t.Fatalf("server hat innerhalb von 8s weder geantwortet noch die verbindung beendet — unbegrenztes warten auf CRLF bestätigt")
}
}
mail/internal/smtp/server.go
+24 -7
View File
@@ -8,6 +8,7 @@ import (
"log/slog"
"net"
"gitea.perlbach24.de/scripte/nexarch/mail/internal/protoguard"
"gitea.perlbach24.de/scripte/nexarch/mail/internal/ratelimit"
)
@@ -18,13 +19,22 @@ const defaultMaxMessageBytes = 25 * 1024 * 1024 // 25 MiB
// Server nimmt SMTP-Verbindungen an und bedient jede in einer eigenen
// Goroutine — gleiches Muster wie mail/internal/imap.Server und
// mail/internal/pop3.Server. tlsConfig ist optional (ING-06): nil
// bedeutet, STARTTLS ist nicht verfügbar. Rate-Limiting ist Sache von
// ING-09, Protokoll-Logging Sache von ING-08 — keine dieser Kacheln.
// bedeutet, STARTTLS ist nicht verfügbar. Protokoll-Logging ist Sache
// von ING-08 — keine dieser Kacheln.
//
// QA-04-Befund (Sicherheitsprüfung): SMTP bekam beim ursprünglichen
// ING-07 (Idle-Timeout) fälschlich KEINEN protoguard-Schutz, anders als
// IMAP/POP3 — eine Gegenstelle, die eine Kommandozeile ohne
// abschließendes CRLF öffnet und nie beendet, blockierte die Session
// unbegrenzt (bestätigter, realer Testangriff, siehe
// qa04_security_test.go). guardCfg schließt diese Lücke nachträglich,
// mit demselben Standardverhalten wie IMAP/POP3.
type Server struct {
sink MessageSink
maxMessageBytes int64
tlsConfig *tls.Config
logger *slog.Logger
guardCfg protoguard.Config
tenantID string
acceptLimiter *ratelimit.Limiter
@@ -37,21 +47,21 @@ func NewServer(sink MessageSink) *Server {
// NewServerWithMaxMessageBytes erlaubt eine abweichende
// Nachrichten-Höchstgröße, z. B. für Tests.
func NewServerWithMaxMessageBytes(sink MessageSink, maxMessageBytes int64) *Server {
return &Server{sink: sink, maxMessageBytes: maxMessageBytes}
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, guardCfg: protoguard.DefaultConfig()}
}
// NewServerWithMaxMessageBytesAndTLSConfig erlaubt zusätzlich, STARTTLS
// zu aktivieren (ING-06). tlsConfig sollte über tlscert.Store.Config()
// erzeugt werden.
func NewServerWithMaxMessageBytesAndTLSConfig(sink MessageSink, maxMessageBytes int64, tlsConfig *tls.Config) *Server {
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig}
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig, guardCfg: protoguard.DefaultConfig()}
}
// NewServerWithMaxMessageBytesTLSAndLogger erlaubt zusätzlich
// strukturiertes Protokoll-Logging (ING-08). logger darf nil sein
// (Logging dann deaktiviert, Rückwärtskompatibilität zu ING-01..ING-06).
func NewServerWithMaxMessageBytesTLSAndLogger(sink MessageSink, maxMessageBytes int64, tlsConfig *tls.Config, logger *slog.Logger) *Server {
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig, logger: logger}
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig, logger: logger, guardCfg: protoguard.DefaultConfig()}
}
// NewServerWithMaxMessageBytesTLSLoggerAndRateLimit erlaubt zusätzlich
@@ -60,7 +70,14 @@ func NewServerWithMaxMessageBytesTLSAndLogger(sink MessageSink, maxMessageBytes
// tenantID identifiziert diesen Server gegenüber dem Limiter
// (Akzeptanzkriterium 3).
func NewServerWithMaxMessageBytesTLSLoggerAndRateLimit(sink MessageSink, maxMessageBytes int64, tlsConfig *tls.Config, logger *slog.Logger, tenantID string, acceptLimiter *ratelimit.Limiter) *Server {
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig, logger: logger, tenantID: tenantID, acceptLimiter: acceptLimiter}
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig, logger: logger, tenantID: tenantID, acceptLimiter: acceptLimiter, guardCfg: protoguard.DefaultConfig()}
}
// NewServerWithMaxMessageBytesTLSLoggerRateLimitAndGuardConfig erlaubt
// zusätzlich abweichende Idle-Timeout-Parameter (QA-04-Nachtrag zu
// ING-07), z. B. für Tests.
func NewServerWithMaxMessageBytesTLSLoggerRateLimitAndGuardConfig(sink MessageSink, maxMessageBytes int64, tlsConfig *tls.Config, logger *slog.Logger, tenantID string, acceptLimiter *ratelimit.Limiter, guardCfg protoguard.Config) *Server {
return &Server{sink: sink, maxMessageBytes: maxMessageBytes, tlsConfig: tlsConfig, logger: logger, tenantID: tenantID, acceptLimiter: acceptLimiter, guardCfg: guardCfg}
}
// Serve nimmt Verbindungen auf listener an, bis ctx beendet wird.
@@ -82,7 +99,7 @@ func (srv *Server) Serve(ctx context.Context, listener net.Listener) error {
}
return fmt.Errorf("smtp: verbindung annehmen: %w", err)
}
session := newSession(conn, srv.sink, srv.maxMessageBytes, srv.tlsConfig, srv.logger, srv.tenantID, srv.acceptLimiter)
session := newSession(conn, srv.sink, srv.maxMessageBytes, srv.tlsConfig, srv.logger, srv.tenantID, srv.acceptLimiter, srv.guardCfg)
go session.Serve(ctx)
}
}
mail/internal/smtp/session.go
+21 -1
View File
@@ -10,6 +10,7 @@ import (
"net"
"strings"
"gitea.perlbach24.de/scripte/nexarch/mail/internal/protoguard"
"gitea.perlbach24.de/scripte/nexarch/mail/internal/protolog"
"gitea.perlbach24.de/scripte/nexarch/mail/internal/ratelimit"
)
@@ -20,6 +21,14 @@ import (
// mail/internal/pop3).
const maxCommandLineBytes = 8192
// phaseSMTP ist die (einzige) protoguard-Phase dieser Sitzung
// (QA-04-Nachtrag zu ING-07): SMTP hat — anders als IMAP/POP3 — keine
// sicherheitsrelevant unterschiedlichen Phasen (Anmeldephase
// existiert in der aktuellen Implementierung nicht, siehe ING-03), ein
// einziger Idle-Timeout für jeden Lesevorgang genügt, um eine
// Gegenstelle ohne abschließendes CRLF nicht unbegrenzt zu blockieren.
const phaseSMTP protoguard.Phase = "smtp"
// Session ist eine einzelne SMTP-Verbindung mit eigener
// Zustandsmaschine (Akzeptanzkriterium 1).
type Session struct {
@@ -38,12 +47,14 @@ type Session struct {
tenantID string
acceptLimiter *ratelimit.Limiter // ING-09, nil = kein Rate-Limiting
guard *protoguard.Guard // QA-04-Nachtrag zu ING-07
state State
from string
to []string
}
func newSession(conn net.Conn, sink MessageSink, maxMessageBytes int64, tlsConfig *tls.Config, logger *slog.Logger, tenantID string, acceptLimiter *ratelimit.Limiter) *Session {
func newSession(conn net.Conn, sink MessageSink, maxMessageBytes int64, tlsConfig *tls.Config, logger *slog.Logger, tenantID string, acceptLimiter *ratelimit.Limiter, guardCfg protoguard.Config) *Session {
_, alreadyTLS := conn.(*tls.Conn)
return &Session{
conn: conn,
@@ -56,6 +67,7 @@ func newSession(conn net.Conn, sink MessageSink, maxMessageBytes int64, tlsConfi
log: protolog.NewSessionLogger(logger, "smtp"),
tenantID: tenantID,
acceptLimiter: acceptLimiter,
guard: protoguard.New(guardCfg),
state: Greeting,
}
}
@@ -113,7 +125,15 @@ func (s *Session) Serve(ctx context.Context) {
}
}
// readLine liest eine Zeile — mit Idle-Timeout VOR jedem Lesevorgang
// (QA-04-Nachtrag zu ING-07): ein einziger Ort für alle Aufrufer
// (Haupt-Serve-Schleife, handleData, drainUntilDot), damit eine
// Gegenstelle, die eine Zeile ohne abschließendes CRLF nie beendet,
// die Session nicht unbegrenzt blockieren kann.
func (s *Session) readLine() (string, error) {
if err := s.guard.ApplyReadDeadline(s.conn, phaseSMTP); err != nil {
return "", err
}
line, err := s.reader.ReadString('\n')
if err != nil {
if errors.Is(err, io.EOF) && line != "" {