sender check: the reply names no other domain (L8); which key signs, tested (stage 2)
L8: a From: line naming another sender of the server was refused with
"it names another sender of this server, <domain>" - a confirmation, to
whoever tries names, that the domain is hosted here. The reason now stops
before the domain; the line as written stays in the reply (the client's own
text) and on the refused message's page (header_from). Unit test: the reason
of every refusal names none of the server's other domains; e2e: the reason
of each sender-check refusal does not contain other.e2e.test.
Which key OpenDKIM signs with (owner, 2026-10-13): the check and OpenDKIM
read From: with two parsers, and their agreement was argued, never run.
internal/milter/testdata/from-lines-passed.txt holds the lines the check
passes for an application - bare and bracketed, quoted names with a comma,
escaped quotes and an address inside, an extension, three encoded charsets,
two folded lines, comments (nested too), a stranger's address in the name -
as Go-quoted strings with {domain}. The unit test wants each passed; the new
e2e case passed_from_lines_signed_for_their_domain_only sends each as app1
and wants every DKIM signature at the sink to be for the sender domain,
with one valid. All fifteen were. "(the shop) a@domain" - a comment before
the address, valid RFC 5322 - is refused by the check ("it must be one
address"); strict, not a hole, so it is not in the file. Groups are refused
too.
Audit plan lines ticked (two) with what was found, CHANGELOG [Unreleased],
roadmap 21/62.
Evidence on the test server for this tree (with a618ffb): go vet clean, go
test ./... 23 packages ok, make e2e exit 0 - the new case passed in 4 s.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -86,6 +86,13 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
|||||||
registry); the rows of a deleted domain are the global role's alone. The
|
registry); the rows of a deleted domain are the global role's alone. The
|
||||||
inbound log follows the same rule (audit 2026-10-12, L12).
|
inbound log follows the same rule (audit 2026-10-12, L12).
|
||||||
|
|
||||||
|
- **A refusal no longer says which other domain sends from this server.**
|
||||||
|
A `From:` line that named another sender of the server was refused with
|
||||||
|
that sender's domain in the reply, which confirmed to whoever tried names
|
||||||
|
that it is hosted here. The reply now stops at "it names another sender of
|
||||||
|
this server"; the line as written stays in the reply and on the refused
|
||||||
|
message's page (audit 2026-10-12, L8).
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
- **The delivery log measures its own guesses** (migration `0007`). A table
|
- **The delivery log measures its own guesses** (migration `0007`). A table
|
||||||
|
|||||||
@@ -690,7 +690,7 @@ has already run is never edited — the next file is added; the published
|
|||||||
about the name in DNS, and whoever holds the name now is its reader.
|
about the name in DNS, and whoever holds the name now is its reader.
|
||||||
- [ ] Docs: security.md § Roles (the guard) and the struck accepted risk, guide
|
- [ ] Docs: security.md § Roles (the guard) and the struck accepted risk, guide
|
||||||
§ Domain export, schema-migrations.md. — **Sonnet**
|
§ Domain export, schema-migrations.md. — **Sonnet**
|
||||||
- [ ] **Which key OpenDKIM signs with, seen and not reasoned** (owner,
|
- [x] **Which key OpenDKIM signs with, seen and not reasoned** (owner,
|
||||||
2026-10-13, from the Not verified list of from-header-binding). The check
|
2026-10-13, from the Not verified list of from-header-binding). The check
|
||||||
and OpenDKIM read `From:` with two parsers; that they agree was argued
|
and OpenDKIM read `From:` with two parsers; that they agree was argued
|
||||||
from the check's two rules, never run. An e2e case sends, as one
|
from the check's two rules, never run. An e2e case sends, as one
|
||||||
@@ -701,7 +701,12 @@ has already run is never edited — the next file is added; the published
|
|||||||
shared with the check's unit test, so a line added to one is run by the
|
shared with the check's unit test, so a line added to one is run by the
|
||||||
other. A disagreement found is fixed in the check before the cut, and
|
other. A disagreement found is fixed in the check before the cut, and
|
||||||
security.md § Mail path says the claim is tested. - **Opus**
|
security.md § Mail path says the claim is tested. - **Opus**
|
||||||
- [ ] **The 550 reply names no other domain** (L8): `sender.go:334` stops at
|
*Done 2026-10-14:* fifteen lines (`internal/milter/testdata/from-lines-passed.txt`),
|
||||||
|
each signed for the application's domain alone, the signature valid. No
|
||||||
|
disagreement. Groups are refused by the check, so none is in the file;
|
||||||
|
a comment before the address - `(the shop) a@domain`, valid RFC 5322 - is
|
||||||
|
refused too ("it must be one address"): strict, not a hole, left as it is.
|
||||||
|
- [x] **The 550 reply names no other domain** (L8): `sender.go:334` stops at
|
||||||
"it names another sender of this server"; the refused message's page
|
"it names another sender of this server"; the refused message's page
|
||||||
still shows the `From:` line as written (`header_from`), so the operator
|
still shows the `From:` line as written (`header_from`), so the operator
|
||||||
loses nothing. Test pins the reply text; e2e's sender-check negatives
|
loses nothing. Test pins the reply text; e2e's sender-check negatives
|
||||||
|
|||||||
+1
-1
@@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md).
|
|||||||
| from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) |
|
| from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) |
|
||||||
| panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) |
|
| panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) |
|
||||||
| schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) |
|
| schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) |
|
||||||
| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`; next stages 1+2 → `2.5.0`) | 19/62 | [audit-2026-10-12.md](audit-2026-10-12.md) |
|
| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`; next stages 1+2 → `2.5.0`) | 21/62 | [audit-2026-10-12.md](audit-2026-10-12.md) |
|
||||||
|
|
||||||
**Recommended order** (not binding; owner, 2026-10-09: feature order follows
|
**Recommended order** (not binding; owner, 2026-10-09: feature order follows
|
||||||
development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and
|
development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and
|
||||||
|
|||||||
@@ -331,7 +331,11 @@ func (s *senderSession) judge() (shown, why string, err error) {
|
|||||||
return shown, "", fmt.Errorf("signing table: %w", err)
|
return shown, "", fmt.Errorf("signing table: %w", err)
|
||||||
}
|
}
|
||||||
if signs {
|
if signs {
|
||||||
return printable(raw), "it names another sender of this server, " + d, nil
|
// Which one is not said: the reply would confirm to whoever
|
||||||
|
// tries names that another tenant sends from this server (audit
|
||||||
|
// 2026-10-12, L8). The line as written is in the reply and on
|
||||||
|
// the refused message's page.
|
||||||
|
return printable(raw), "it names another sender of this server", nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// And one that needs to know nothing of OpenDKIM's parser: outside quoted
|
// And one that needs to know nothing of OpenDKIM's parser: outside quoted
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package milter
|
|||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
@@ -144,6 +145,14 @@ func TestSenderCheck(t *testing.T) {
|
|||||||
if !strings.HasPrefix(text, "y 550 5.7.1 From: ") || !strings.Contains(text, c.wantText) {
|
if !strings.HasPrefix(text, "y 550 5.7.1 From: ") || !strings.Contains(text, c.wantText) {
|
||||||
t.Fatalf("reply %q, want a 550 5.7.1 saying %q", text, c.wantText)
|
t.Fatalf("reply %q, want a 550 5.7.1 saying %q", text, c.wantText)
|
||||||
}
|
}
|
||||||
|
// The reason names no other domain of the server (audit L8); the
|
||||||
|
// From: line before it is what the client wrote itself.
|
||||||
|
reason := text[strings.LastIndex(text, " - ")+3:]
|
||||||
|
for _, other := range []string{"example.com", "mail.example.org", "example.org"} {
|
||||||
|
if strings.Contains(reason, other) {
|
||||||
|
t.Fatalf("the reason %q names %s", reason, other)
|
||||||
|
}
|
||||||
|
}
|
||||||
if len(rows) != 1 || rows[0].Reason != store.ReasonFrom || rows[0].To != "someone@elsewhere.example" || rows[0].AppLogin != c.login {
|
if len(rows) != 1 || rows[0].Reason != store.ReasonFrom || rows[0].To != "someone@elsewhere.example" || rows[0].AppLogin != c.login {
|
||||||
t.Fatalf("log rows: %+v", rows)
|
t.Fatalf("log rows: %+v", rows)
|
||||||
}
|
}
|
||||||
@@ -271,3 +280,37 @@ func TestSenderCheckRefusalWithNullSenderHasItsDomain(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// passedFromLines reads testdata/from-lines-passed.txt with {domain} put in.
|
||||||
|
func passedFromLines(t *testing.T, domain string) []string {
|
||||||
|
t.Helper()
|
||||||
|
raw, err := os.ReadFile(filepath.Join("testdata", "from-lines-passed.txt"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
var out []string
|
||||||
|
for _, line := range strings.Split(string(raw), "\n") {
|
||||||
|
line = strings.TrimSpace(line)
|
||||||
|
if line == "" || strings.HasPrefix(line, "#") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
s, err := strconv.Unquote(line)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("%s: %v", line, err)
|
||||||
|
}
|
||||||
|
out = append(out, strings.ReplaceAll(s, "{domain}", domain))
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// Every line of the shared file is passed for the domain's application; the
|
||||||
|
// e2e then checks OpenDKIM signs each for that domain alone.
|
||||||
|
func TestSenderCheckPassesTheSharedLines(t *testing.T) {
|
||||||
|
bind := NewBindings(writeMaps(t, senderMaps))
|
||||||
|
for _, line := range passedFromLines(t, "shop.example.org") {
|
||||||
|
resp, rows := offer(t, bind, "shop", "shop@shop.example.org", [2]string{"From", line})
|
||||||
|
if resp != milter.RespContinue || len(rows) != 0 {
|
||||||
|
t.Errorf("From: %q refused: %s", line, replyText(resp))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+22
@@ -0,0 +1,22 @@
|
|||||||
|
# From: lines the sender check lets through for an application of {domain}
|
||||||
|
# (audit 2026-10-12, stage 2: which key OpenDKIM signs with, seen and not
|
||||||
|
# reasoned). One Go-quoted string per line; {domain} is the application's
|
||||||
|
# domain. Two tests read this file: the check's unit test
|
||||||
|
# (internal/milter/sender_test.go) wants each line passed, and the e2e
|
||||||
|
# (test/e2e/dkim_from_test.go) sends each one and wants the message signed
|
||||||
|
# for {domain} and nothing else. A line added here is run by both.
|
||||||
|
"a@{domain}"
|
||||||
|
"<a@{domain}>"
|
||||||
|
"\"The Shop\" <Orders@{domain}>"
|
||||||
|
"orders+48213@{domain}"
|
||||||
|
"\"shop@{domain}\" <a@{domain}>"
|
||||||
|
"\"Orders, returns\" <a@{domain}>"
|
||||||
|
"\"The \\\"Shop\\\"\" <a@{domain}>"
|
||||||
|
"\"someone@gmail.example\" <a@{domain}>"
|
||||||
|
"=?utf-8?b?0JzQsNCz0LDQt9C40L0=?= <a@{domain}>"
|
||||||
|
"=?windows-1251?B?zODj4Ofo7Q==?= <a@{domain}>"
|
||||||
|
"=?koi8-r?Q?=ED=C1=C7=C1=DA=C9=CE?= <a@{domain}>"
|
||||||
|
"\"A very long display name of the shop\"\r\n <a@{domain}>"
|
||||||
|
"=?utf-8?b?0JzQsNCz0LDQt9C40L0=?=\r\n\t<a@{domain}>"
|
||||||
|
"a@{domain} (the shop)"
|
||||||
|
"a@{domain} (the (nested) shop)"
|
||||||
@@ -0,0 +1,84 @@
|
|||||||
|
package e2e
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/emersion/go-msgauth/dkim"
|
||||||
|
)
|
||||||
|
|
||||||
|
// passedFromLines reads the From: lines the sender check passes, the file its
|
||||||
|
// unit test reads too, with the application's domain put in.
|
||||||
|
func passedFromLines(repoRoot, domain string) ([]string, error) {
|
||||||
|
raw, err := os.ReadFile(filepath.Join(repoRoot, "internal", "milter", "testdata", "from-lines-passed.txt"))
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
var out []string
|
||||||
|
for _, line := range strings.Split(string(raw), "\n") {
|
||||||
|
line = strings.TrimSpace(line)
|
||||||
|
if line == "" || strings.HasPrefix(line, "#") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
s, err := strconv.Unquote(line)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("%s: %w", line, err)
|
||||||
|
}
|
||||||
|
out = append(out, strings.ReplaceAll(s, "{domain}", domain))
|
||||||
|
}
|
||||||
|
return out, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// testSignedForItsDomainOnly: the sender check and OpenDKIM read From: with
|
||||||
|
// two parsers, and that they agree was argued, never run (audit 2026-10-12,
|
||||||
|
// stage 2). Every From: line the check passes for an application is sent as
|
||||||
|
// that application, and what arrives must be signed for its domain - with a
|
||||||
|
// signature that validates - and for no other: a line the check reads as the
|
||||||
|
// application's own and OpenDKIM as another sender's would be signed with
|
||||||
|
// that sender's key.
|
||||||
|
func testSignedForItsDomainOnly(t *testing.T, sc *scenario) {
|
||||||
|
lines, err := passedFromLines(h.repoRoot, senderDomain)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for _, line := range lines {
|
||||||
|
token := uniqueToken("dkimfrom")
|
||||||
|
res := attemptSend(sendAttempt{
|
||||||
|
authLogin: sc.appLogin, authPassword: sc.appPassword,
|
||||||
|
from: "alerts@" + senderDomain, headerFrom: line, to: recipient,
|
||||||
|
subject: "e2e " + token, body: "which key signs this",
|
||||||
|
})
|
||||||
|
if !res.ok() {
|
||||||
|
t.Errorf("From: %q was refused: %v", line, res.firstErr())
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
raw, err := findSinkMessage(h.stageDir, token, 30*time.Second)
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("From: %q never reached the sink: %v", line, err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
verifications, err := dkim.VerifyWithOptions(bytes.NewReader(raw), &dkim.VerifyOptions{LookupTXT: lookupTXTFunc()})
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("From: %q: dkim verify: %v", line, err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
valid := false
|
||||||
|
for _, v := range verifications {
|
||||||
|
if v.Domain != senderDomain {
|
||||||
|
t.Errorf("From: %q was signed for %s", line, v.Domain)
|
||||||
|
}
|
||||||
|
if v.Domain == senderDomain && v.Err == nil {
|
||||||
|
valid = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !valid {
|
||||||
|
t.Errorf("From: %q has no valid signature for %s (%d signature(s))", line, senderDomain, len(verifications))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -310,6 +310,9 @@ func TestE2E(t *testing.T) {
|
|||||||
run("sender_check_holds_the_from_line", func(t *testing.T) {
|
run("sender_check_holds_the_from_line", func(t *testing.T) {
|
||||||
testSenderCheck(t, sc)
|
testSenderCheck(t, sc)
|
||||||
})
|
})
|
||||||
|
run("passed_from_lines_signed_for_their_domain_only", func(t *testing.T) {
|
||||||
|
testSignedForItsDomainOnly(t, sc)
|
||||||
|
})
|
||||||
run("panel_down_defers_mail", func(t *testing.T) {
|
run("panel_down_defers_mail", func(t *testing.T) {
|
||||||
testPanelDownDefersMail(t, sc)
|
testPanelDownDefersMail(t, sc)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -83,6 +83,10 @@ func testSenderCheck(t *testing.T, sc *scenario) {
|
|||||||
// DATA for the client; it is permanent and says what is wrong.
|
// DATA for the client; it is permanent and says what is wrong.
|
||||||
if e := fmt.Sprint(res.dataErr); res.dataErr == nil || !strings.HasPrefix(e, "550 ") || !strings.Contains(e, "5.7.1 From:") {
|
if e := fmt.Sprint(res.dataErr); res.dataErr == nil || !strings.HasPrefix(e, "550 ") || !strings.Contains(e, "5.7.1 From:") {
|
||||||
t.Errorf("%s: want 550 5.7.1 naming the From: line at the end of DATA, got %v", c.name, res.firstErr())
|
t.Errorf("%s: want 550 5.7.1 naming the From: line at the end of DATA, got %v", c.name, res.firstErr())
|
||||||
|
} else if reason := e[strings.LastIndex(e, " - ")+3:]; strings.Contains(reason, otherDomain) {
|
||||||
|
// What the client wrote is said back; that the other domain is
|
||||||
|
// a sender of this server is not (audit 2026-10-12, L8).
|
||||||
|
t.Errorf("%s: the reason %q names %s", c.name, reason, otherDomain)
|
||||||
}
|
}
|
||||||
if _, err := findSinkMessage(h.stageDir, token, 2*time.Second); err == nil {
|
if _, err := findSinkMessage(h.stageDir, token, 2*time.Second); err == nil {
|
||||||
t.Errorf("%s: the refused message reached the sink", c.name)
|
t.Errorf("%s: the refused message reached the sink", c.name)
|
||||||
|
|||||||
Reference in New Issue
Block a user