diff --git a/mail/docs/QA-04-PRUEFPROTOKOLL.md b/mail/docs/QA-04-PRUEFPROTOKOLL.md new file mode 100644 index 0000000..04f2620 --- /dev/null +++ b/mail/docs/QA-04-PRUEFPROTOKOLL.md @@ -0,0 +1,134 @@ +# QA-04 — Sicherheits- & Berechtigungsprüfung: Prüfprotokoll + +Datum: 2026-09-01 +Host: 192.168.1.131 (Build/Test/Lint), rsync + ssh +Geprüfte Pakete: `mail/internal/smtp`, `mail/internal/mailer`, `mail/internal/storage`, `mail/internal/folderstate`, `mail/internal/mailboxconfig`, `mail/internal/ratelimit` + +## Umsetzung — echter Sicherheitsbefund und Korrektur + +Der gezielte Testangriff auf den SMTP-Pfad (Pflichtprüfung 1) deckte +einen REALEN Härtungsfehler auf, der VOR dieser Kachel unbemerkt +geblieben war: ING-07 (Idle-Timeout-Schutz) hatte `protoguard` nur in +`mail/internal/imap` und `mail/internal/pop3` verdrahtet — `mail/ +internal/smtp` bekam versehentlich NIE einen Idle-Timeout. Eine +Gegenstelle, die eine Kommandozeile ohne abschließendes CRLF öffnet und +nie beendet, konnte die Session unbegrenzt blockieren (bestätigt durch +`TestQA04_OversizedLineWithoutCRLFDoesNotHangOrCrash`, VOR der +Korrektur real reproduziert: Server antwortete nach 8s Wartezeit weder +mit Fehler noch Verbindungsende). + +**Korrektur** (`session.go`/`server.go`, `mail/internal/smtp`): `guard +*protoguard.Guard` als neues Feld, Idle-Timeout wird jetzt in +`readLine()` selbst gesetzt — EIN Ort für alle Aufrufer (Haupt-Serve- +Schleife, `handleData`, `drainUntilDot`), damit auch das Lesen des +DATA-Bodys geschützt ist. Neuer Konstruktor +`NewServerWithMaxMessageBytesTLSLoggerRateLimitAndGuardConfig` für +abweichende Timeout-Werte (Tests). Bestehende Konstruktoren bekommen +automatisch `protoguard.DefaultConfig()` (5 Minuten) statt wie zuvor +gar keinen Timeout — reine Härtung, keine Verhaltensänderung für +funktionierende Clients. + +## Pflichtprüfung 1: Gezielter Testangriff auf Header-Injection schlägt fehl + +`TestQA04_HeaderInjectionViaEnvelopeAddressRejected` +(`smtp/qa04_security_test.go`): NUL-Byte und Steuerzeichen in +RCPT TO/MAIL FROM werden mit `553`/`501` zurückgewiesen, Session bleibt +danach funktionsfähig, keine Nachricht erreicht den Sink. Ergänzt um +`TestQA04_OversizedLineWithoutCRLFDoesNotHangOrCrash` (Ressourcen- +erschöpfungsangriff, siehe Abschnitt "Umsetzung" — deckte den realen +Härtungsfehler auf und bestätigt nach der Korrektur zuverlässige +Reaktion binnen des konfigurierten Timeouts). Bereits bestehende, +unverändert gültige Nachweise aus ING-03/ING-06 werden mitgezählt: +CRLF-Injection in Betreff/Anzeigename (`mailer.TestHeaderWriter_ +RejectsControlCharsAndCRLFInSubjectAndDisplayName`), Dot-Stuffing +korrekt gegen DATA-Command-Smuggling (`smtp.TestData_ +MessageSizeCheckedBeforeAcceptance` u. a.), TLS-Downgrade-Angriffe +(`smtp.TestServer_RejectsLegacyTLSVersionAndWeakCiphers`, ING-06). + +Ergebnis: **BESTANDEN** — inklusive eines real gefundenen und +behobenen Härtungsfehlers. + +## Pflichtprüfung 2: Stichprobenprüfung mehrerer Speicherpfade auf Mandantentrennung + +Drei unabhängige Speicherpfade stichprobenartig geprüft: + +1. **Objekt-Storage** (`mail/internal/storage`, ARC-06): physische + Bucket-Trennung, bereits real gegen MinIO nachgewiesen + (`TestProvisionTenant_CreatesPhysicallySeparateBuckets`, + `TestAccessWithoutTenantContext_FailsBecauseNoBucketReferenceable` + — siehe `ARC-06-PRUEFPROTOKOLL.md`). +2. **Folder-State** (`mail/internal/folderstate`, ING-10): + `NextUID`/`Rebuild` für Mandant A verändern Mandant Bs Zustand + nachweislich nicht (`TestTenantScoping_ + NeverReturnsOrMutatesOtherTenantsFolderState`). +3. **Postfachkonfiguration** (`mail/internal/mailboxconfig`) — NEU für + diese Kachel, bislang nicht auditiert, besonders sensibel + (verschlüsselte IMAP-Zugangsdaten): `TestTenantScoping_ + ForeignKnownIDNeverAccessible` — Mandant B versucht mit einer ECHTEN, + bekannten ID aus Mandant As Zeile (realistischster Angriffsfall bei + fortlaufenden IDs in einer gemeinsamen Tabelle) auf + `List`/`GetDecryptedPassword`/`Update`/`Delete` zuzugreifen — jeder + Versuch liefert `ErrNotFound`, Mandant As Daten bleiben unverändert. + +Ergebnis: **BESTANDEN** in allen drei gezogenen Stichproben. + +## Pflichtprüfung 3: Test: API-Zugriff mit widerrufenem/fremdem Token wird verweigert + +**Teilweise nicht durchführbar, dokumentiert:** Das Mail-Modul besitzt +aktuell KEINE eigene HTTP-API mit Token-/Session-Authentifizierung — +jede vorhandene Schnittstelle (`web/mail-search`, SRC-04) verweist +explizit auf eine noch ausstehende "zentrale Session-/IAM-Anbindung +(Core-Board-Scope, nicht Bestandteil dieser Kachel)", konsistent mit +QA-04s eigener Ausgangslage: "Berührt Login-Tenant-Filter und +Privilege-Escalation – dafür ist bereits Core-Board IAM zuständig, hier +nur Mail-spezifische Aspekte prüfen." Es gibt daher keinen Prüfgegenstand +für "widerrufenes/fremdes API-Token" innerhalb des Mail-Boards — dieser +Teil bleibt **offen**, bis Core-Board IAM eine Token-Schnittstelle +liefert, gegen die geprüft werden kann. + +Der **Rate-Limiting-Teil** von Akzeptanzkriterium 3 ist dagegen real +vorhanden und geprüft (ING-09): `TestRateLimit_ +LoadExceedingLimitGetsRejectedWithRetryHint`, +`TestRateLimit_LegitUsageBelowThresholdUnaffected`, +`TestRateLimit_PerTenantIndependentAndEffective` — je einmal in IMAP, +POP3, SMTP, alle mit echten Nachweisen bestanden (siehe +`ING-09-PRUEFPROTOKOLL.md`), hier erneut mitgeprüft und bestätigt grün. + +Ergebnis: **Rate-Limiting-Teil BESTANDEN, API-Token-Teil OFFEN** +(kein Prüfgegenstand im Mail-Board vorhanden). + +## Akzeptanzkriterien + +1. **Prüfung bestätigt wirksame Härtung des SMTP-Pfads gegen bekannte + Angriffsmuster**: durch Pflichtprüfung 1 belegt — inklusive eines + real gefundenen und in dieser Kachel behobenen Härtungsfehlers + (fehlender Idle-Timeout). +2. **Prüfung bestätigt lückenlose Mandantentrennung im Speicherpfad**: + durch Pflichtprüfung 2 belegt (drei Speicherpfade, keine Lücke + gefunden). +3. **Prüfung bestätigt korrekt greifendes API-Token-/Rate-Limiting**: + Rate-Limiting-Teil durch Pflichtprüfung 3 belegt; API-Token-Teil + bleibt offen (kein Prüfgegenstand, siehe oben). + +## Build/Vet/Lint/Test — Gesamtmodul + +``` +go build ./... → OK +go vet ./... → OK +golangci-lint run ./... → 0 issues +go test ./... -p 1 (TEST_TENANT_DSN, TEST_MANTICORE_URL, TEST_S3_ENDPOINT/TEST_S3_ACCESS_KEY/TEST_S3_SECRET_KEY gesetzt) → alle Pakete ok +``` + +Keine Regression — insbesondere QA-07-Lasttest für SMTP bleibt nach der +Idle-Timeout-Korrektur unverändert grün (6057,8 Sessions/s, p95 48,2ms). + +## Ergebnis + +QA-04 erfüllt Akzeptanzkriterium 1 und 2 vollständig mit echten, +ausgeführten Nachweisen — inklusive eines real gefundenen und behobenen +Sicherheitsfehlers (fehlender SMTP-Idle-Timeout). Akzeptanzkriterium 3 +ist zur Hälfte (Rate-Limiting) erfüllt; die API-Token-Hälfte bleibt +offen, da im Mail-Board kein Prüfgegenstand existiert (bewusst an +Core-Board IAM delegiert, siehe QA-04s eigene Ausgangslage). Board wird +auf Basis der erfüllbaren Teile fortgeführt, das offene Element ist +hier und im Entscheidungsverlauf vermerkt. Freigeschaltet: QA-09. diff --git a/mail/internal/mailboxconfig/tenant_scoping_test.go b/mail/internal/mailboxconfig/tenant_scoping_test.go new file mode 100644 index 0000000..898a7bf --- /dev/null +++ b/mail/internal/mailboxconfig/tenant_scoping_test.go @@ -0,0 +1,82 @@ +package mailboxconfig + +import ( + "context" + "errors" + "testing" +) + +// TestTenantScoping_ForeignKnownIDNeverAccessible ist Teil der +// geforderten Pflichtprüfung "Stichprobenprüfung mehrerer Speicherpfade +// auf Mandantentrennung" (QA-04): dieser Speicherpfad ist besonders +// sensibel (verschlüsselte Postfach-Zugangsdaten). Anders als die +// Tenant-Scoping-Tests aus ING-10 (unterschiedliche Server-/Store- +// Instanzen je Mandant) prüft dieser Test den REALISTISCHSTEN +// Angriffsfall für eine geteilte Datenbank mit tenant_slug-Spalte: +// Mandant B kennt/errät eine gültige, echte ID aus Mandant As Zeile +// (z. B. durch fortlaufende IDs) und versucht direkt darauf +// zuzugreifen — muss an der tenant_slug-Prüfung in der WHERE-Klausel +// scheitern, nicht an einem geratenen falschen Wert. +func TestTenantScoping_ForeignKnownIDNeverAccessible(t *testing.T) { + store := setupStore(t) + ctx := context.Background() + + tenantA := "mandant-qa04-a" + tenantB := "mandant-qa04-b" + + idA := createTestMailbox(t, store, tenantA, "Postfach A") + + // Mandant B versucht, MIT DER ECHTEN, BEKANNTEN ID von Mandant A + // zuzugreifen — List gibt es aus Mandant Bs Sicht nicht. + listB, err := store.List(ctx, tenantB) + if err != nil { + t.Fatalf("List mandant b: %v", err) + } + for _, mb := range listB { + if mb.ID == idA { + t.Fatalf("mandant b sieht mandant As postfach in seiner eigenen liste") + } + } + + // Direkter Zugriff mit fremder, aber echter ID: GetDecryptedPassword. + _, err = store.GetDecryptedPassword(ctx, tenantB, idA) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("GetDecryptedPassword mit fremder mandant-b-anfrage auf mandant-a-id hätte ErrNotFound liefern müssen, habe: %v", err) + } + + // Direkter Zugriff mit fremder, aber echter ID: Update. + err = store.Update(ctx, tenantB, idA, UpdateInput{ + IMAPHost: "boese.example.com", + IMAPPort: 993, + FolderSelection: []string{"INBOX"}, + IntervalSeconds: 60, + }) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Update mit fremder mandant-b-anfrage auf mandant-a-id hätte ErrNotFound liefern müssen, habe: %v", err) + } + + // Direkter Zugriff mit fremder, aber echter ID: Delete. + err = store.Delete(ctx, tenantB, idA) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("Delete mit fremder mandant-b-anfrage auf mandant-a-id hätte ErrNotFound liefern müssen, habe: %v", err) + } + + // Kontrolle: Mandant As eigenes Postfach ist von alledem UNBERÜHRT + // (kein erfolgreicher Update/Delete-Seiteneffekt trotz Ablehnung). + stillA, err := store.List(ctx, tenantA) + if err != nil { + t.Fatalf("List mandant a nach angriffsversuchen: %v", err) + } + found := false + for _, ma := range stillA { + if ma.ID == idA { + found = true + if ma.IMAPHost == "boese.example.com" { + t.Fatalf("mandant Bs Update-Versuch hat mandant As postfach tatsächlich verändert") + } + } + } + if !found { + t.Fatalf("mandant As postfach ist nach den angriffsversuchen verschwunden") + } +} diff --git a/mail/internal/smtp/qa04_security_test.go b/mail/internal/smtp/qa04_security_test.go new file mode 100644 index 0000000..16d2beb --- /dev/null +++ b/mail/internal/smtp/qa04_security_test.go @@ -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:"}, + {"Steuerzeichen 0x01 in MAIL FROM", "MAIL FROM:"}, + } + + 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:") + + 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") + } +} diff --git a/mail/internal/smtp/server.go b/mail/internal/smtp/server.go index a836514..0a0dfab 100644 --- a/mail/internal/smtp/server.go +++ b/mail/internal/smtp/server.go @@ -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) } } diff --git a/mail/internal/smtp/session.go b/mail/internal/smtp/session.go index f5efc23..dc77aeb 100644 --- a/mail/internal/smtp/session.go +++ b/mail/internal/smtp/session.go @@ -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 != "" {