Closes what the security review left as an accepted risk: with the
spam filter down - it is attached fail-open - nothing removes verdict
headers a sender wrote himself, and the inbound journal recorded his
verdict as the filter's. The owner asked whether rspamd could add a
header that proves the others are ours (2026-10-12) and agreed to a
signature: a fixed mark could be written by a sender as easily as the
verdict.
- inbound.FilterKey: a key of 32 random bytes, 64 hex characters, in
/data/antispam/filter.key, made on the first start with the relay
on. 0644 like the list files beside it: the filter is another user
in another container.
- deploy/antispam/rspamd/milter_headers.conf: a custom routine reads
the key through the directory the sidecar already mounts and adds
X-SelfPost-Filter, the HMAC-SHA256 of the message's queue id, hex;
a header of that name from the sender is removed.
- internal/milter/inbound.go: the verdict headers are kept as they
are read, and at end-of-message - when the queue id is known - they
count only if the signature fits (FilterSignature, hmac.Equal). No
key, no queue id, no signature or a wrong one: the row is that of
mail nobody judged (accept, selfpost, no detail).
The signature goes on to the upstream with the mail and is of no use
there: it fits one queue id.
Tried in a throwaway rspamd 3.11 container on the test server before
the Go side was written: the routine loads, signs
(2aa1ca6d... for key 0123...cdef and queue id 4A1B2C3D5E, the value
python's hmac gives and TestFilterSignatureIsWhatTheFilterWrites
pins), removes a sender's header of the same name, and adds nothing
without a queue id.
Tests: signed, unsigned, made-up, another message's, another key's,
the sender's followed by the real one, the next message on the
connection, no queue id; the key made once, read after, replaced when
damaged, left alone by the lists. e2e: filter.key is there, 644, 65
bytes.
docs/security.md: the accepted risk is replaced by a narrower one (the
key is readable on the host); Mail path describes the signature.
Checked on the test server, uncommitted tree: go vet ./... ok,
go test ./... ok (22 packages), make e2e ok.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
package inbound
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"net/netip"
|
||||
"os"
|
||||
@@ -401,3 +403,45 @@ func (l *Lists) Usage() (map[int64]Usage, error) {
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// FilterKeyFile is the file, beside the list files, that holds the key the
|
||||
// spam filter signs messages with.
|
||||
const FilterKeyFile = "filter.key"
|
||||
|
||||
// FilterKey returns the key the spam filter signs each message with, creating
|
||||
// it on first use: 32 random bytes as 64 hex characters on one line, in the
|
||||
// directory the filter's container mounts.
|
||||
//
|
||||
// The filter is attached so that mail flows when it is down. Then nothing
|
||||
// removes verdict headers a sender wrote himself, and the inbound journal
|
||||
// would record his verdict as the filter's. So the filter signs each message
|
||||
// with this key and the journal reads a verdict only next to a signature it
|
||||
// can recompute (internal/milter, FilterSignature).
|
||||
//
|
||||
// The file is readable by anyone on the host, like the list files and for the
|
||||
// same reason: the filter is another user in another container. What the key
|
||||
// protects is the truth of a monitoring page, against senders on the internet;
|
||||
// someone with a shell on the host is not who it is kept from.
|
||||
func FilterKey(dir string) (string, error) {
|
||||
path := filepath.Join(dir, FilterKeyFile)
|
||||
if raw, err := os.ReadFile(path); err == nil {
|
||||
if key := strings.TrimSpace(string(raw)); len(key) >= 32 && !strings.ContainsAny(key, " \t\r\n") {
|
||||
return key, nil
|
||||
}
|
||||
// Empty or cut short: not a key anyone should sign with. Replaced below.
|
||||
} else if !os.IsNotExist(err) {
|
||||
return "", fmt.Errorf("read %s: %w", path, err)
|
||||
}
|
||||
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||
return "", fmt.Errorf("create %s: %w", dir, err)
|
||||
}
|
||||
buf := make([]byte, 32)
|
||||
if _, err := rand.Read(buf); err != nil {
|
||||
return "", fmt.Errorf("generate the filter key: %w", err)
|
||||
}
|
||||
key := hex.EncodeToString(buf)
|
||||
if err := atomicfile.Write(path, []byte(key+"\n"), 0o644); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return key, nil
|
||||
}
|
||||
|
||||
@@ -395,3 +395,46 @@ func TestListable(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The key the filter signs with is made once and then read, and is readable by
|
||||
// the filter, which is another user in another container.
|
||||
func TestFilterKey(t *testing.T) {
|
||||
dir := filepath.Join(t.TempDir(), "antispam")
|
||||
key, err := FilterKey(dir)
|
||||
if err != nil || len(key) != 64 || strings.Trim(key, "0123456789abcdef") != "" {
|
||||
t.Fatalf("FilterKey = %q, %v", key, err)
|
||||
}
|
||||
again, err := FilterKey(dir)
|
||||
if err != nil || again != key {
|
||||
t.Errorf("a second call = %q, %v; want the same key", again, err)
|
||||
}
|
||||
raw, err := os.ReadFile(filepath.Join(dir, FilterKeyFile))
|
||||
if err != nil || string(raw) != key+"\n" {
|
||||
t.Errorf("file = %q, %v", raw, err)
|
||||
}
|
||||
if info, _ := os.Stat(filepath.Join(dir, FilterKeyFile)); os.PathSeparator == '/' && info.Mode().Perm() != 0o644 {
|
||||
t.Errorf("mode = %o", info.Mode().Perm())
|
||||
}
|
||||
|
||||
// A file that is not a key is replaced, not signed with.
|
||||
if err := os.WriteFile(filepath.Join(dir, FilterKeyFile), []byte("short\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fresh, err := FilterKey(dir)
|
||||
if err != nil || len(fresh) != 64 || fresh == key {
|
||||
t.Errorf("after a damaged file: %q, %v", fresh, err)
|
||||
}
|
||||
|
||||
// The lists leave it alone.
|
||||
st, err := store.Open(filepath.Join(t.TempDir(), "test.db"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer st.Close()
|
||||
if err := NewLists(st, dir).Sync(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if kept, _ := FilterKey(dir); kept != fresh {
|
||||
t.Errorf("the key changed when the lists were written")
|
||||
}
|
||||
}
|
||||
|
||||
+59
-22
@@ -2,6 +2,9 @@ package milter
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"log"
|
||||
"net"
|
||||
"net/textproto"
|
||||
@@ -33,10 +36,12 @@ type InboundStore interface {
|
||||
type inboundSession struct {
|
||||
milter.NoOpMilter
|
||||
rec InboundStore
|
||||
// filter says whether an external filter is attached in front of this
|
||||
// milter. Without one the verdict headers below can only have been written
|
||||
// by the sender, and are not read.
|
||||
filter bool
|
||||
// key is what the filter signs each message with (FilterSignature), or
|
||||
// empty when no filter is attached. A verdict is read only next to a
|
||||
// signature that fits: the filter is attached so that mail flows when it is
|
||||
// down, and then the verdict headers of a message are whatever its sender
|
||||
// wrote.
|
||||
key string
|
||||
|
||||
// clientIP is taken at Connect and again from {client_addr} with every
|
||||
// MAIL FROM: a session the library made after a non-continue reply never
|
||||
@@ -49,6 +54,7 @@ type inboundSession struct {
|
||||
action string // X-Rspamd-Action
|
||||
result string // X-Spamd-Result
|
||||
seen bool // any verdict header of the filter
|
||||
sig string // X-SelfPost-Filter
|
||||
}
|
||||
|
||||
func (s *inboundSession) Connect(host, family string, port uint16, addr net.IP, m *milter.Modifier) (milter.Response, error) {
|
||||
@@ -73,10 +79,12 @@ func (s *inboundSession) RcptTo(rcpt string, m *milter.Modifier) (milter.Respons
|
||||
return milter.RespContinue, nil
|
||||
}
|
||||
|
||||
// Header keeps the first Subject and the filter's verdict. Postfix hands the
|
||||
// message to its milters one after another at end-of-message, each seeing what
|
||||
// the one before added, which is how the filter's headers arrive here. The
|
||||
// filter replaces headers of these names, so the last one read is its own.
|
||||
// Header keeps the first Subject, the filter's verdict and its signature.
|
||||
// Postfix hands the message to its milters one after another at
|
||||
// end-of-message, each seeing what the one before added, which is how the
|
||||
// filter's headers arrive here. The filter replaces headers of these names, so
|
||||
// the last one read is its own - when it is up. Whether it was is decided at
|
||||
// end-of-message, by the signature.
|
||||
func (s *inboundSession) Header(name, value string, m *milter.Modifier) (milter.Response, error) {
|
||||
switch textproto.CanonicalMIMEHeaderKey(name) {
|
||||
case "Subject":
|
||||
@@ -84,21 +92,50 @@ func (s *inboundSession) Header(name, value string, m *milter.Modifier) (milter.
|
||||
s.subject = mailhdr.DecodeSubject(value)
|
||||
}
|
||||
case "X-Rspamd-Action":
|
||||
if s.filter {
|
||||
s.action, s.seen = value, true
|
||||
}
|
||||
s.action, s.seen = value, true
|
||||
case "X-Spamd-Result":
|
||||
if s.filter {
|
||||
s.result, s.seen = value, true
|
||||
}
|
||||
s.result, s.seen = value, true
|
||||
case signatureHeader:
|
||||
s.sig = strings.TrimSpace(value)
|
||||
}
|
||||
return milter.RespContinue, nil
|
||||
}
|
||||
|
||||
// SignatureHeader is the header the filter signs a message in
|
||||
// (deploy/antispam/rspamd/milter_headers.conf writes it).
|
||||
const SignatureHeader = "X-SelfPost-Filter"
|
||||
|
||||
var signatureHeader = textproto.CanonicalMIMEHeaderKey(SignatureHeader)
|
||||
|
||||
// FilterSignature is what the filter writes into SignatureHeader for a message:
|
||||
// the HMAC-SHA256 of its queue id under the key SelfPost shares with the
|
||||
// filter, in hex. The key is used as the text it is stored as. A signature
|
||||
// fits one queue id, so the header going on to the upstream with the mail
|
||||
// gives nothing to whoever reads it there.
|
||||
func FilterSignature(key, queueID string) string {
|
||||
mac := hmac.New(sha256.New, []byte(key))
|
||||
mac.Write([]byte(queueID))
|
||||
return hex.EncodeToString(mac.Sum(nil))
|
||||
}
|
||||
|
||||
// signed reports whether the message carries the filter's signature for its
|
||||
// queue id. No key, no queue id or no signature is "no".
|
||||
func (s *inboundSession) signed(queueID string) bool {
|
||||
if s.key == "" || queueID == "" || s.sig == "" {
|
||||
return false
|
||||
}
|
||||
return hmac.Equal([]byte(strings.ToLower(s.sig)), []byte(FilterSignature(s.key, queueID)))
|
||||
}
|
||||
|
||||
// Body fires at end-of-message: the message is about to be queued for its
|
||||
// upstream. "Continue" lets it through as "accept" would, and keeps this
|
||||
// session for the connection's next message.
|
||||
func (s *inboundSession) Body(m *milter.Modifier) (milter.Response, error) {
|
||||
// The queue id is known only now, and with it whether the verdict headers
|
||||
// read on the way here are the filter's.
|
||||
if !s.signed(macro(m, "i")) {
|
||||
s.seen = false
|
||||
}
|
||||
s.record()
|
||||
s.reset()
|
||||
return milter.RespContinue, nil
|
||||
@@ -111,7 +148,7 @@ func (s *inboundSession) Abort(m *milter.Modifier) error {
|
||||
|
||||
func (s *inboundSession) reset() {
|
||||
s.from, s.rcpts, s.subject = "", nil, ""
|
||||
s.action, s.result, s.seen = "", "", false
|
||||
s.action, s.result, s.seen, s.sig = "", "", false, ""
|
||||
}
|
||||
|
||||
// record writes one row per recipient at an inbound domain. Other recipients
|
||||
@@ -169,9 +206,8 @@ var (
|
||||
// rspamdVerdict reduces the filter's headers to an action and a one-line
|
||||
// summary: the action, the score against the threshold, and the first few
|
||||
// symbols that carried weight. It reads only what it recognises - the headers
|
||||
// are the filter's when it is up, but the sender's own when it is not (it is
|
||||
// attached fail-open), so nothing of them is kept verbatim except through
|
||||
// these patterns.
|
||||
// are read only next to the filter's signature, and even so nothing of them is
|
||||
// kept verbatim except through these patterns.
|
||||
func rspamdVerdict(actionHeader, resultHeader string) (action, detail string) {
|
||||
action = strings.ToLower(strings.TrimSpace(actionHeader))
|
||||
for _, r := range action {
|
||||
@@ -229,11 +265,12 @@ func rspamdVerdict(actionHeader, resultHeader string) (action, detail string) {
|
||||
}
|
||||
|
||||
// ServeInbound runs the inbound journal milter on ln until ctx is cancelled.
|
||||
// filter says whether an external filter is attached in front of it
|
||||
// (INBOUND_ANTISPAM_MILTER). It returns nil on a clean shutdown.
|
||||
func ServeInbound(ctx context.Context, ln net.Listener, rec InboundStore, filter bool) error {
|
||||
// key is what the external filter in front of it signs messages with, or
|
||||
// empty when none is attached (INBOUND_ANTISPAM_MILTER): no verdict is then
|
||||
// read at all. It returns nil on a clean shutdown.
|
||||
func ServeInbound(ctx context.Context, ln net.Listener, rec InboundStore, key string) error {
|
||||
srv := &milter.Server{
|
||||
NewMilter: func() milter.Milter { return &inboundSession{rec: rec, filter: filter} },
|
||||
NewMilter: func() milter.Milter { return &inboundSession{rec: rec, key: key} },
|
||||
Actions: 0, // read-only
|
||||
Protocol: milter.OptNoBody, // headers and end-of-message, not the body
|
||||
}
|
||||
|
||||
@@ -60,9 +60,18 @@ func message(t *testing.T, s *inboundSession, from string, rcpts []string, heade
|
||||
return resp
|
||||
}
|
||||
|
||||
// testKey is the key the tests' filter signs with; the messages they replay
|
||||
// have the queue id ABC123.
|
||||
const testKey = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
|
||||
|
||||
var testSig = [2]string{SignatureHeader, FilterSignature(testKey, "ABC123")}
|
||||
|
||||
func inboundSessionFor(t *testing.T, rec InboundStore, filter bool) *inboundSession {
|
||||
t.Helper()
|
||||
s := &inboundSession{rec: rec, filter: filter}
|
||||
s := &inboundSession{rec: rec}
|
||||
if filter {
|
||||
s.key = testKey
|
||||
}
|
||||
if _, err := s.Connect("mx.example.net", "tcp4", 0, net.ParseIP("203.0.113.7"), mods(nil)); err != nil {
|
||||
t.Fatalf("Connect: %v", err)
|
||||
}
|
||||
@@ -116,6 +125,7 @@ func TestInboundReadsTheFiltersVerdict(t *testing.T) {
|
||||
{"Subject", "Cheap pills"},
|
||||
{"X-Rspamd-Action", "add header"},
|
||||
{"X-Spamd-Result", spamdResult},
|
||||
testSig,
|
||||
})
|
||||
if len(rec.rows) != 1 {
|
||||
t.Fatalf("rows = %+v", rec.rows)
|
||||
@@ -133,7 +143,7 @@ func TestInboundQuarantineAction(t *testing.T) {
|
||||
rec := &fakeInbound{domains: map[string]bool{"in.example.com": true}}
|
||||
s := inboundSessionFor(t, rec, true)
|
||||
message(t, s, "sender@example.net", []string{"a@in.example.com"},
|
||||
[][2]string{{"x-rspamd-action", "quarantine"}})
|
||||
[][2]string{{"x-rspamd-action", "quarantine"}, {"x-selfpost-filter", strings.ToUpper(testSig[1])}})
|
||||
if len(rec.rows) != 1 || rec.rows[0].Decision != store.SpamDecisionQuarantine || rec.rows[0].Detail != "quarantine" {
|
||||
t.Errorf("rows = %+v", rec.rows)
|
||||
}
|
||||
@@ -180,7 +190,7 @@ func TestInboundResetsBetweenMessages(t *testing.T) {
|
||||
rec := &fakeInbound{domains: map[string]bool{"in.example.com": true}}
|
||||
s := inboundSessionFor(t, rec, true)
|
||||
message(t, s, "one@example.net", []string{"a@in.example.com"}, [][2]string{
|
||||
{"Subject", "First"}, {"X-Rspamd-Action", "add header"},
|
||||
{"Subject", "First"}, {"X-Rspamd-Action", "add header"}, testSig,
|
||||
})
|
||||
message(t, s, "two@example.net", []string{"b@in.example.com"}, nil)
|
||||
if len(rec.rows) != 2 {
|
||||
@@ -259,7 +269,7 @@ func TestInboundClientAddressFromTheMailMacro(t *testing.T) {
|
||||
// cost it its session.
|
||||
func TestInboundOnlyEverContinues(t *testing.T) {
|
||||
rec := &fakeInbound{domains: map[string]bool{"in.example.com": true}}
|
||||
s := &inboundSession{rec: rec, filter: true}
|
||||
s := &inboundSession{rec: rec, key: testKey}
|
||||
check := func(stage string, resp milter.Response, err error) {
|
||||
t.Helper()
|
||||
if err != nil || resp != milter.RespContinue {
|
||||
@@ -277,3 +287,64 @@ func TestInboundOnlyEverContinues(t *testing.T) {
|
||||
resp, err = s.Body(mods(nil))
|
||||
check("end of message", resp, err)
|
||||
}
|
||||
|
||||
// The filter is attached so that mail flows when it is down, and then the
|
||||
// verdict headers of a message are its sender's own. A verdict is recorded only
|
||||
// next to the filter's signature for this message's queue id.
|
||||
func TestInboundReadsAVerdictOnlyWithTheFiltersSignature(t *testing.T) {
|
||||
verdict := [][2]string{{"X-Rspamd-Action", "quarantine"}, {"X-Spamd-Result", spamdResult}}
|
||||
for name, c := range map[string]struct {
|
||||
extra [][2]string
|
||||
signed bool
|
||||
}{
|
||||
"signed": {[][2]string{testSig}, true},
|
||||
"no signature": {nil, false},
|
||||
"a made-up signature": {[][2]string{{SignatureHeader, strings.Repeat("ab", 32)}}, false},
|
||||
"another message's signature": {[][2]string{{SignatureHeader, FilterSignature(testKey, "OTHER1")}}, false},
|
||||
"a signature under another key": {[][2]string{{SignatureHeader, FilterSignature(strings.Repeat("f", 64), "ABC123")}}, false},
|
||||
"the sender's first, then the real": {[][2]string{{SignatureHeader, "forged"}, testSig}, true},
|
||||
} {
|
||||
rec := &fakeInbound{domains: map[string]bool{"in.example.com": true}}
|
||||
s := inboundSessionFor(t, rec, true)
|
||||
message(t, s, "sender@example.net", []string{"a@in.example.com"}, append(append([][2]string{}, verdict...), c.extra...))
|
||||
if len(rec.rows) != 1 {
|
||||
t.Fatalf("%s: rows = %+v", name, rec.rows)
|
||||
}
|
||||
got := rec.rows[0]
|
||||
if c.signed && (got.Engine != store.SpamEngineRspamd || got.Decision != store.SpamDecisionQuarantine) {
|
||||
t.Errorf("%s: a signed verdict was not read: %+v", name, got)
|
||||
}
|
||||
if !c.signed && (got.Engine != store.SpamEngineSelfPost || got.Decision != store.SpamDecisionAccept || got.Detail != "") {
|
||||
t.Errorf("%s: an unsigned verdict was recorded: %+v", name, got)
|
||||
}
|
||||
}
|
||||
|
||||
// A signature is of no use to the next message on the connection.
|
||||
rec := &fakeInbound{domains: map[string]bool{"in.example.com": true}}
|
||||
s := inboundSessionFor(t, rec, true)
|
||||
message(t, s, "one@example.net", []string{"a@in.example.com"}, append(append([][2]string{}, verdict...), testSig))
|
||||
message(t, s, "two@example.net", []string{"a@in.example.com"}, verdict)
|
||||
if len(rec.rows) != 2 || rec.rows[1].Engine != store.SpamEngineSelfPost {
|
||||
t.Errorf("rows = %+v", rec.rows)
|
||||
}
|
||||
|
||||
// With no queue id nothing can be checked, so nothing is trusted.
|
||||
rec = &fakeInbound{domains: map[string]bool{"in.example.com": true}}
|
||||
s = inboundSessionFor(t, rec, true)
|
||||
_, _ = s.MailFrom("sender@example.net", mods(nil))
|
||||
_, _ = s.RcptTo("a@in.example.com", mods(nil))
|
||||
_, _ = s.Header("X-Rspamd-Action", "reject", mods(nil))
|
||||
_, _ = s.Header(SignatureHeader, FilterSignature(testKey, ""), mods(nil))
|
||||
_, _ = s.Body(mods(nil))
|
||||
if len(rec.rows) != 1 || rec.rows[0].Engine != store.SpamEngineSelfPost {
|
||||
t.Errorf("no queue id: rows = %+v", rec.rows)
|
||||
}
|
||||
}
|
||||
|
||||
// What rspamd 3.11 wrote for this key and queue id in a throwaway container
|
||||
// (dev trial, 2026-10-12): the two ends must compute the same thing.
|
||||
func TestFilterSignatureIsWhatTheFilterWrites(t *testing.T) {
|
||||
if got := FilterSignature(testKey, "4A1B2C3D5E"); got != "2aa1ca6dd1545d3f16c8c0c29ba59bfc28c050224ac2c5c953bd2617f73b6de2" {
|
||||
t.Errorf("FilterSignature = %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user