author | Alberto Bertogli
<albertito@blitiri.com.ar> 2016-09-12 05:08:53 UTC |
committer | Alberto Bertogli
<albertito@blitiri.com.ar> 2016-10-09 23:50:56 UTC |
parent | 941eb9315cb5e007e0822994d8268545519fd3bc |
chasquid.go | +21 | -10 |
chasquid_test.go | +76 | -26 |
internal/envelope/envelope.go | +3 | -3 |
test/t-01-simple_local/msmtprc | +4 | -1 |
test/t-01-simple_local/run.sh | +8 | -0 |
test/t-02-exim/config/exim4.in | +7 | -0 |
test/t-02-exim/msmtprc | +1 | -1 |
test/t-02-exim/run.sh | +1 | -0 |
test/util/mail_diff | +10 | -5 |
diff --git a/chasquid.go b/chasquid.go index 7c93b6c..47925f2 100644 --- a/chasquid.go +++ b/chasquid.go @@ -22,6 +22,7 @@ import ( "blitiri.com.ar/go/chasquid/internal/auth" "blitiri.com.ar/go/chasquid/internal/config" "blitiri.com.ar/go/chasquid/internal/courier" + "blitiri.com.ar/go/chasquid/internal/envelope" "blitiri.com.ar/go/chasquid/internal/queue" "blitiri.com.ar/go/chasquid/internal/set" "blitiri.com.ar/go/chasquid/internal/systemd" @@ -320,6 +321,7 @@ func (s *Server) serve(l net.Listener, mode SocketMode) { mode: mode, tlsConfig: s.tlsConfig, userDBs: s.userDBs, + localDomains: s.localDomains, deadline: time.Now().Add(s.connTimeout), commandTimeout: s.commandTimeout, queue: s.queue, @@ -356,8 +358,9 @@ type Conn struct { // Are we using TLS? onTLS bool - // User databases - taken from the server at creation time. - userDBs map[string]*userdb.DB + // User databases and local domains, taken from the server at creation time. + userDBs map[string]*userdb.DB + localDomains *set.String // Have we successfully completed AUTH? completedAuth bool @@ -560,6 +563,14 @@ func (c *Conn) MAIL(params string) (code int, msg string) { // but that's not according to the RFC. We reset the envelope instead. c.resetEnvelope() + // If the source is local, check that it completed auth for that user. + if e.Address != "<>" && envelope.DomainIn(e.Address, c.localDomains) { + user, domain := envelope.Split(e.Address) + if user != c.authUser || domain != c.authDomain { + return 503, "user not authorized" + } + } + c.mailFrom = e.Address return 250, "You feel like you are being watched" } @@ -572,6 +583,11 @@ func (c *Conn) RCPT(params string) (code int, msg string) { return 500, "unknown command" } + // RFC says 100 is the minimum limit for this, but it seems excessive. + if len(c.rcptTo) > 100 { + return 503, "too many recipients" + } + // TODO: Write our own parser (we have different needs, mail.ParseAddress // is useful for other things). // Allow utf8, but prevent "control" characters. @@ -585,16 +601,11 @@ func (c *Conn) RCPT(params string) (code int, msg string) { return 503, "sender not yet given" } - // RFC says 100 is the minimum limit for this, but it seems excessive. - if len(c.rcptTo) > 100 { - return + remoteDst := !envelope.DomainIn(e.Address, c.localDomains) + if remoteDst && !c.completedAuth { + return 503, "relay not allowed" } - // TODO: do we allow receivers without a domain? - // TODO: check the case: - // - local recipient, always ok - // - external recipient, only ok if mailFrom is local (needs auth) - c.rcptTo = append(c.rcptTo, e.Address) return 250, "You have an eerie feeling..." } diff --git a/chasquid_test.go b/chasquid_test.go index 14f494c..243d83d 100644 --- a/chasquid_test.go +++ b/chasquid_test.go @@ -24,16 +24,18 @@ import ( // Flags. var ( - externalServerAddr = flag.String("external_server_addr", "", - "address of the external server to test (defaults to use internal)") + externalSMTPAddr = flag.String("external_smtp_addr", "", + "SMTP server address to test (defaults to use internal)") + externalSubmissionAddr = flag.String("external_submission_addr", "", + "submission server address to test (defaults to use internal)") ) var ( - // Server address. - // We default to an internal one, but may get overriden via - // --external_server_addr. + // Server addresses. + // We default to internal ones, but may get overriden via flags. // TODO: Don't hard-code the default. - srvAddr = "127.0.0.1:13453" + smtpAddr = "127.0.0.1:13444" + submissionAddr = "127.0.0.1:13999" // TLS configuration to use in the clients. // Will contain the generated server certificate as root CA. @@ -44,8 +46,14 @@ var ( // === Tests === // -func mustDial(tb testing.TB, useTLS bool) *smtp.Client { - c, err := smtp.Dial(srvAddr) +func mustDial(tb testing.TB, mode SocketMode, useTLS bool) *smtp.Client { + addr := "" + if mode == ModeSMTP { + addr = smtpAddr + } else { + addr = submissionAddr + } + c, err := smtp.Dial(addr) if err != nil { tb.Fatalf("smtp.Dial: %v", err) } @@ -73,18 +81,23 @@ func sendEmail(tb testing.TB, c *smtp.Client) { func sendEmailWithAuth(tb testing.TB, c *smtp.Client, auth smtp.Auth) { var err error + from := "from@from" if auth != nil { if err = c.Auth(auth); err != nil { tb.Errorf("Auth: %v", err) } + + // If we authenticated, we must use the user as from, as the server + // checks otherwise. + from = "testuser@localhost" } - if err = c.Mail("from@from"); err != nil { + if err = c.Mail(from); err != nil { tb.Errorf("Mail: %v", err) } - if err = c.Rcpt("to@to"); err != nil { + if err = c.Rcpt("to@localhost"); err != nil { tb.Errorf("Rcpt: %v", err) } @@ -104,19 +117,19 @@ func sendEmailWithAuth(tb testing.TB, c *smtp.Client, auth smtp.Auth) { } func TestSimple(t *testing.T) { - c := mustDial(t, false) + c := mustDial(t, ModeSMTP, false) defer c.Close() sendEmail(t, c) } func TestSimpleTLS(t *testing.T) { - c := mustDial(t, true) + c := mustDial(t, ModeSMTP, true) defer c.Close() sendEmail(t, c) } func TestManyEmails(t *testing.T) { - c := mustDial(t, true) + c := mustDial(t, ModeSMTP, true) defer c.Close() sendEmail(t, c) sendEmail(t, c) @@ -124,15 +137,26 @@ func TestManyEmails(t *testing.T) { } func TestAuth(t *testing.T) { - c := mustDial(t, true) + c := mustDial(t, ModeSubmission, true) + defer c.Close() + + auth := smtp.PlainAuth("", "testuser@localhost", "testpasswd", "127.0.0.1") + sendEmailWithAuth(t, c, auth) +} + +func TestAuthOnSMTP(t *testing.T) { + c := mustDial(t, ModeSMTP, true) defer c.Close() auth := smtp.PlainAuth("", "testuser@localhost", "testpasswd", "127.0.0.1") + + // At least for now, we allow AUTH over the SMTP port to avoid unnecessary + // complexity, so we expect it to work. sendEmailWithAuth(t, c, auth) } func TestWrongMailParsing(t *testing.T) { - c := mustDial(t, false) + c := mustDial(t, ModeSMTP, false) defer c.Close() addrs := []string{"from", "a b c", "a @ b", "<x>", "<x y>", "><"} @@ -155,7 +179,7 @@ func TestWrongMailParsing(t *testing.T) { } func TestNullMailFrom(t *testing.T) { - c := mustDial(t, false) + c := mustDial(t, ModeSMTP, false) defer c.Close() addrs := []string{"", "<>", " <>", " < > "} @@ -167,7 +191,7 @@ func TestNullMailFrom(t *testing.T) { } func TestRcptBeforeMail(t *testing.T) { - c := mustDial(t, false) + c := mustDial(t, ModeSMTP, false) defer c.Close() if err := c.Rcpt("to@to"); err == nil { @@ -175,6 +199,28 @@ func TestRcptBeforeMail(t *testing.T) { } } +func TestLocalHasAuthenticated(t *testing.T) { + c := mustDial(t, ModeSubmission, false) + defer c.Close() + + if err := c.Mail("from@localhost"); err == nil { + t.Errorf("Accepted non-authenticated local mail") + } +} + +func TestRelayForbidden(t *testing.T) { + c := mustDial(t, ModeSMTP, false) + defer c.Close() + + if err := c.Mail("from@somewhere"); err != nil { + t.Errorf("Mail: %v", err) + } + + if err := c.Rcpt("to@somewhere"); err == nil { + t.Errorf("Accepted relay email") + } +} + func simpleCmd(t *testing.T, c *smtp.Client, cmd string, expected int) { if err := c.Text.PrintfLine(cmd); err != nil { t.Fatalf("Failed to write %s: %v", cmd, err) @@ -186,7 +232,7 @@ func simpleCmd(t *testing.T, c *smtp.Client, cmd string, expected int) { } func TestSimpleCommands(t *testing.T) { - c := mustDial(t, false) + c := mustDial(t, ModeSMTP, false) defer c.Close() simpleCmd(t, c, "HELP", 214) simpleCmd(t, c, "NOOP", 250) @@ -195,7 +241,7 @@ func TestSimpleCommands(t *testing.T) { } func TestReset(t *testing.T) { - c := mustDial(t, false) + c := mustDial(t, ModeSMTP, false) defer c.Close() if err := c.Mail("from@from"); err != nil { @@ -212,7 +258,7 @@ func TestReset(t *testing.T) { } func TestRepeatedStartTLS(t *testing.T) { - c, err := smtp.Dial(srvAddr) + c, err := smtp.Dial(smtpAddr) if err != nil { t.Fatalf("smtp.Dial: %v", err) } @@ -231,7 +277,7 @@ func TestRepeatedStartTLS(t *testing.T) { // func BenchmarkManyEmails(b *testing.B) { - c := mustDial(b, false) + c := mustDial(b, ModeSMTP, false) defer c.Close() b.ResetTimer() @@ -245,7 +291,7 @@ func BenchmarkManyEmails(b *testing.B) { func BenchmarkManyEmailsParallel(b *testing.B) { b.RunParallel(func(pb *testing.PB) { - c := mustDial(b, false) + c := mustDial(b, ModeSMTP, false) defer c.Close() for pb.Next() { @@ -355,8 +401,9 @@ func realMain(m *testing.M) int { flag.Parse() defer glog.Flush() - if *externalServerAddr != "" { - srvAddr = *externalServerAddr + if *externalSMTPAddr != "" { + smtpAddr = *externalSMTPAddr + submissionAddr = *externalSubmissionAddr tlsConfig = &tls.Config{ InsecureSkipVerify: true, } @@ -379,16 +426,19 @@ func realMain(m *testing.M) int { s.Hostname = "localhost" s.MaxDataSize = 50 * 1024 * 1025 s.AddCerts(tmpDir+"/cert.pem", tmpDir+"/key.pem") - s.AddAddr(srvAddr, ModeSMTP) + s.AddAddr(smtpAddr, ModeSMTP) + s.AddAddr(submissionAddr, ModeSubmission) udb := userdb.New("/dev/null") udb.AddUser("testuser", "testpasswd") + s.AddDomain("localhost") s.AddUserDB("localhost", udb) go s.ListenAndServe() } - waitForServer(srvAddr) + waitForServer(smtpAddr) + waitForServer(submissionAddr) return m.Run() } diff --git a/internal/envelope/envelope.go b/internal/envelope/envelope.go index c1d4233..4c31214 100644 --- a/internal/envelope/envelope.go +++ b/internal/envelope/envelope.go @@ -9,7 +9,7 @@ import ( ) // Split an user@domain address into user and domain. -func split(addr string) (string, string) { +func Split(addr string) (string, string) { ps := strings.SplitN(addr, "@", 2) if len(ps) != 2 { return addr, "" @@ -19,12 +19,12 @@ func split(addr string) (string, string) { } func UserOf(addr string) string { - user, _ := split(addr) + user, _ := Split(addr) return user } func DomainOf(addr string) string { - _, domain := split(addr) + _, domain := Split(addr) return domain } diff --git a/test/t-01-simple_local/msmtprc b/test/t-01-simple_local/msmtprc index 00190f0..949e099 100644 --- a/test/t-01-simple_local/msmtprc +++ b/test/t-01-simple_local/msmtprc @@ -1,7 +1,7 @@ account default host testserver -port 1025 +port 1587 tls on tls_trust_file config/domains/testserver/cert.pem @@ -12,6 +12,9 @@ auth on user user@testserver password secretpassword +account smtpport : default +port 1025 + account baduser : default user unknownuser@testserver password secretpassword diff --git a/test/t-01-simple_local/run.sh b/test/t-01-simple_local/run.sh index eb8f501..d52e6dd 100755 --- a/test/t-01-simple_local/run.sh +++ b/test/t-01-simple_local/run.sh @@ -7,6 +7,7 @@ init generate_certs_for testserver +mkdir -p .logs chasquid -v=2 --log_dir=.logs --config_dir=config & wait_until_ready 1025 @@ -16,6 +17,13 @@ wait_for_file .mail/someone@testserver mail_diff content .mail/someone@testserver +# At least for now, we allow AUTH over the SMTP port to avoid unnecessary +# complexity, so we expect it to work. +if ! run_msmtp -a smtpport someone@testserver < content 2> /dev/null; then + echo "ERROR: failed auth on the SMTP port" + exit 1 +fi + if run_msmtp -a baduser someone@testserver < content 2> /dev/null; then echo "ERROR: successfully sent an email with a bad password" exit 1 diff --git a/test/t-02-exim/config/exim4.in b/test/t-02-exim/config/exim4.in index 7623b43..1304f0c 100644 --- a/test/t-02-exim/config/exim4.in +++ b/test/t-02-exim/config/exim4.in @@ -31,6 +31,13 @@ acl_check_data: accept +# Rewrite envelope-from to server@srv-exim. +# This is so when we redirect, we don't use user@srv-chasquid in the +# envelope-from (we're not authorized to send mail on behalf of +# @srv-chasquid). +begin rewrite +user@srv-chasquid server@srv-exim F + # Forward all incoming email to chasquid (running on :1025 in this test). begin routers diff --git a/test/t-02-exim/msmtprc b/test/t-02-exim/msmtprc index d5e75c1..f24e87c 100644 --- a/test/t-02-exim/msmtprc +++ b/test/t-02-exim/msmtprc @@ -1,7 +1,7 @@ account default host srv-chasquid -port 1025 +port 1587 tls on tls_trust_file config/domains/srv-chasquid/cert.pem diff --git a/test/t-02-exim/run.sh b/test/t-02-exim/run.sh index a9803dc..a1c2caa 100755 --- a/test/t-02-exim/run.sh +++ b/test/t-02-exim/run.sh @@ -37,6 +37,7 @@ generate_certs_for srv-chasquid # Launch chasquid at port 1025 (in config). # Use outgoing port 2025 which is where exim will be at. # Bypass MX lookup, so it can find srv-exim (via our host alias). +mkdir -p .logs chasquid -v=2 --log_dir=.logs --config_dir=config \ --testing__outgoing_smtp_port=2025 \ --testing__bypass_mx_lookup & diff --git a/test/util/mail_diff b/test/util/mail_diff index 5e9667e..da87547 100755 --- a/test/util/mail_diff +++ b/test/util/mail_diff @@ -25,10 +25,15 @@ for h, val in expected.items(): if expected.get_payload() != msg.get_payload(): diff = True - exp = expected.get_payload().splitlines() - got = msg.get_payload().splitlines() - print("Payload differs:") - for l in difflib.ndiff(exp, got): - print(l) + + if expected.is_multipart() != msg.is_multipart(): + print("Multipart differs, expected %s, got %s" % ( + expected.is_multipart(), msg.is_multipart())) + elif not msg.is_multipart(): + exp = expected.get_payload().splitlines() + got = msg.get_payload().splitlines() + print("Payload differs:") + for l in difflib.ndiff(exp, got): + print(l) sys.exit(0 if not diff else 1)