Files
chasquid/internal/smtpsrv/conn_test.go
Alberto Bertogli a996106eee smtpsrv: Strict CRLF enforcement in DATA contents
The RFCs are very clear that in DATA contents:

> CR and LF MUST only occur together as CRLF; they MUST NOT appear
> independently in the body.

https://www.rfc-editor.org/rfc/rfc5322#section-2.3
https://www.rfc-editor.org/rfc/rfc5321#section-2.3.8

Allowing "independent" CR and LF can cause a number of problems.

In particular, there is a new "SMTP smuggling attack" published recently
that involves the server incorrectly parsing the end of DATA marker
`\r\n.\r\n`, which an attacker can exploit to impersonate a server when
email is transmitted server-to-server.

https://www.postfix.org/smtp-smuggling.html
https://sec-consult.com/blog/detail/smtp-smuggling-spoofing-e-mails-worldwide/

Currently, chasquid is vulnerable to this attack, because Go's standard
libraries net/textproto and net/mail do not enforce CRLF strictly.

This patch fixes the problem by introducing a new "dot reader" function
that strictly enforces CRLF when reading dot-terminated data, used in
the DATA input processing.

When an invalid newline terminator is found, the connection is aborted
immediately because we cannot safely recover from that state.

We still keep the internal representation as LF-terminated for
convenience and simplicity.

However, the MDA courier is changed to pass CRLF-terminated lines, since
that is an external program which could be strict when receiving email
messages.

See https://github.com/albertito/chasquid/issues/47 for more details and
discussion.
2023-12-24 10:43:27 +00:00

155 lines
3.5 KiB
Go

package smtpsrv
import (
"net"
"os"
"testing"
"blitiri.com.ar/go/chasquid/internal/domaininfo"
"blitiri.com.ar/go/chasquid/internal/testlib"
"blitiri.com.ar/go/chasquid/internal/trace"
"blitiri.com.ar/go/spf"
)
func TestSecLevel(t *testing.T) {
// We can't simulate this externally because of the SPF record
// requirement, so do a narrow test on Conn.secLevelCheck.
// Create the directory by hand because we don't want to automatically
// chdir into it (it affects the fuzzing infrastructure).
dir, err := os.MkdirTemp("", "testlib_")
if err != nil {
t.Fatalf("Failed to create temp dir: %v\n", dir)
}
defer testlib.RemoveIfOk(t, dir)
dinfo, err := domaininfo.New(dir)
if err != nil {
t.Fatalf("Failed to create domain info: %v", err)
}
c := &Conn{
tr: trace.New("testconn", "testconn"),
dinfo: dinfo,
}
// No SPF, skip security checks.
c.spfResult = spf.None
c.onTLS = true
if !c.secLevelCheck("from@slc") {
t.Fatalf("TLS seclevel failed")
}
c.onTLS = false
if !c.secLevelCheck("from@slc") {
t.Fatalf("plain seclevel failed, even though SPF does not exist")
}
// Now the real checks, once SPF passes.
c.spfResult = spf.Pass
if !c.secLevelCheck("from@slc") {
t.Fatalf("plain seclevel failed")
}
c.onTLS = true
if !c.secLevelCheck("from@slc") {
t.Fatalf("TLS seclevel failed")
}
c.onTLS = false
if c.secLevelCheck("from@slc") {
t.Fatalf("plain seclevel worked, downgrade was allowed")
}
}
func TestIsHeader(t *testing.T) {
no := []string{
"a", "\n", "\n\n", " \n", " ",
"a:b", "a: b\nx: y",
"\na:b\n", " a\nb:c\n",
}
for _, s := range no {
if isHeader([]byte(s)) {
t.Errorf("%q accepted as header, should be rejected", s)
}
}
yes := []string{
"", "a:b\n",
"X-Post-Data: success\n",
}
for _, s := range yes {
if !isHeader([]byte(s)) {
t.Errorf("%q rejected as header, should be accepted", s)
}
}
}
func TestAddrLiteral(t *testing.T) {
// TCP addresses.
casesTCP := []struct {
addr net.IP
expected string
}{
{net.IPv4(1, 2, 3, 4), "1.2.3.4"},
{net.IPv4(0, 0, 0, 0), "0.0.0.0"},
{net.ParseIP("1.2.3.4"), "1.2.3.4"},
{net.ParseIP("2001:db8::68"), "IPv6:2001:db8::68"},
{net.ParseIP("::1"), "IPv6:::1"},
}
for _, c := range casesTCP {
tcp := &net.TCPAddr{
IP: c.addr,
Port: 12345,
}
s := addrLiteral(tcp)
if s != c.expected {
t.Errorf("%v: expected %q, got %q", tcp, c.expected, s)
}
}
// Non-TCP addresses. We expect these to match addr.String().
casesOther := []net.Addr{
&net.UDPAddr{
IP: net.ParseIP("1.2.3.4"),
Port: 12345,
},
}
for _, addr := range casesOther {
s := addrLiteral(addr)
if s != addr.String() {
t.Errorf("%v: expected %q, got %q", addr, addr.String(), s)
}
}
}
func TestSanitizeEHLODomain(t *testing.T) {
equal := []string{
"domain", "do.main", "do-main",
"1.2.3.4", "a:b:c", "[a:b:c]",
"abz", "AbZ",
}
for _, str := range equal {
if got := sanitizeEHLODomain(str); got != str {
t.Errorf("sanitizeEHLODomain(%q) returned %q, expected %q",
str, got, str)
}
}
invalid := []struct {
str string
expected string
}{
{"ñaca", "aca"}, {"a\nb", "ab"}, {"a\x00b", "ab"}, {"a\x7fb", "ab"},
{"a/z", "az"}, {"a;b", "ab"}, {"a$b", "ab"}, {"a^b", "ab"},
{"a b", "ab"}, {"a+b", "ab"}, {"a@b", "ab"}, {`a"b`, "ab"},
{`a\b`, "ab"},
}
for _, c := range invalid {
if got := sanitizeEHLODomain(c.str); got != c.expected {
t.Errorf("sanitizeEHLODomain(%q) returned %q, expected %q",
c.str, got, c.expected)
}
}
}