mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 23:42:00 +00:00
3853b7460c
H-3 in certctl-audit-report.md: caller-supplied From/To/Subject were
interpolated directly into the SMTP DATA payload and handed to
client.Mail / client.Rcpt with no sanitization, allowing an attacker
who controls any of those values to inject extra headers (Bcc:,
Reply-To:), split the message body (CRLFCRLF), or tamper with the
SMTP envelope. CWE-113.
Fix:
- New package helper internal/validation.ValidateHeaderValue(field,
value). Rejects CR ("\r"), LF ("\n"), and NUL ("\x00") with an error
that names the offending field but does NOT echo the raw value,
so log readers cannot be attacked with injected content. Silent
stripping was considered and rejected: authentication-relevant
headers must fail visibly.
- Two-layer defense in internal/connector/notifier/email/email.go:
(1) primary guard at the top of sendEmail / sendHTMLEmail, which
blocks tampering of the SMTP envelope (client.Mail, client.Rcpt)
since net/smtp does not sanitize those arguments; and
(2) defense-in-depth guard inside formatEmailMessage /
formatHTMLEmailMessage, catching any future caller that
bypasses sendEmail. Both format functions now return an error.
- Body content is intentionally NOT validated — CR/LF in body is legal
RFC 5322 content and net/smtp handles dot-stuffing.
Tests:
- internal/validation/headers_test.go: 3 functions (AcceptsSafeInput,
RejectsControlCharacters, DefaultFieldName) covering plain ASCII,
UTF-8 multibyte, tabs, typical email addresses, CRLF injection,
lone CR, lone LF, NUL, CRLFCRLF body split, trailing CR, leading LF.
Each reject case asserts the field name IS in the error and the
raw offending value IS NOT (anti-log-injection).
- internal/connector/notifier/email/email_test.go: added
TestEmail_FormatEmailMessage_RejectsCRLFInjection and
TestEmail_FormatHTMLEmailMessage_RejectsCRLFInjection. Existing
format tests updated for the new (bytes, error) signature.
Wire-format invariants preserved:
- SMTP DATA headers still use CRLF separators and RFC 1123Z Date
(unchanged).
- Content-Type headers unchanged (text/plain for plain, text/html +
MIME-Version: 1.0 for HTML).
- No change to message encoding or transport.
Verification (Go 1.25.9 linux-arm64, parent e9947dc):
- go build ./... clean
- go vet ./... clean
- go test -race ./internal/validation/... ok
- go test -race ./internal/connector/notifier/email/... ok
- go test -race ./internal/connector/notifier/webhook/... ok
- Per-layer coverage gates all pass:
validation 95.1% (+0.7 vs baseline 94.4%)
email 39.7% (+1.4 vs baseline 38.3%)
service 67.8% (unchanged)
handler 78.6% (unchanged)
middleware 80.0% (unchanged)
domain 92.7% (unchanged)
- govulncheck ./... No vulnerabilities found
- golangci-lint run ./internal/validation/... ./internal/connector/notifier/email/...
0 issues
Operational note: SMTP sends that would previously deliver a
tampered message now fail fast at the notifier with a clear error.
Operators who were relying on header-injection-shaped inputs (there
should be none in practice — all callers are internal certctl code)
will see "failed to format message: <field> contains disallowed
control character" in logs.
Scope: H-3 only. H-4 (webhook SSRF) follows in a separate commit.
71 lines
2.1 KiB
Go
71 lines
2.1 KiB
Go
package validation
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
func TestValidateHeaderValue_AcceptsSafeInput(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
field string
|
|
value string
|
|
}{
|
|
{"plain ASCII", "Subject", "Renewal reminder"},
|
|
{"empty string", "Reply-To", ""},
|
|
{"utf-8 multibyte", "Subject", "résumé — 日本語"},
|
|
{"tabs and spaces permitted", "Subject", "a\tb c"},
|
|
{"typical email address", "From", "alerts@example.com"},
|
|
{"long Subject within limits", "Subject", strings.Repeat("x", 998)},
|
|
}
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
if err := ValidateHeaderValue(tc.field, tc.value); err != nil {
|
|
t.Fatalf("expected nil error, got %v", err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestValidateHeaderValue_RejectsControlCharacters(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
field string
|
|
value string
|
|
}{
|
|
{"injected CRLF + header", "Subject", "hello\r\nBcc: attacker@example.com"},
|
|
{"lone LF", "From", "alice@example.com\nBcc: x@y"},
|
|
{"lone CR", "Subject", "hello\rworld"},
|
|
{"NUL byte", "To", "bob@example.com\x00extra"},
|
|
{"CRLFCRLF body injection", "Subject", "ping\r\n\r\nMalicious body"},
|
|
{"CR at end", "Subject", "trailing\r"},
|
|
{"LF at start", "Subject", "\nleading"},
|
|
}
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
err := ValidateHeaderValue(tc.field, tc.value)
|
|
if err == nil {
|
|
t.Fatalf("expected error rejecting control characters, got nil")
|
|
}
|
|
// Error must mention the field so operators can pinpoint the offender.
|
|
if !strings.Contains(err.Error(), tc.field) {
|
|
t.Errorf("expected error to mention field %q, got %q", tc.field, err.Error())
|
|
}
|
|
// Error must NOT leak the raw value back into logs.
|
|
if strings.Contains(err.Error(), tc.value) {
|
|
t.Errorf("error leaks raw value; expected redaction: %q", err.Error())
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestValidateHeaderValue_DefaultFieldName(t *testing.T) {
|
|
err := ValidateHeaderValue("", "bad\r\nvalue")
|
|
if err == nil {
|
|
t.Fatal("expected error for CRLF input, got nil")
|
|
}
|
|
if !strings.Contains(err.Error(), "header") {
|
|
t.Errorf("expected default field name 'header' in error, got %q", err.Error())
|
|
}
|
|
}
|