mirrors/maddy
1
0
Fork 0
mirror of https://github.com/foxcpp/maddy.git synced 2025-04-04 21:47:40 +03:00
Code Issues Projects Releases Packages Wiki Activity

target/remote: Attempt TLS without authentication instead of plaintext

Browse source 
TLS without authentication is still better than no TLS at all.

To save latency in transactions with a misconfigured recipient server
that cannot use TLS at all but still advertises STARTTLS support,
downgrade to non-authenticated TLS is attempted only on verification
errors (x509.UnknownAuthorityError or x509.HostnameError) and malformed
certificate errors (x509.ConstraintViolationError and
x509.CertificateInvalidError). In all other cases 'remote' module
fallbacks to plaintext directly.

While rearranging code to support this, some additional changes were
made to allow simplier implementation of security levels idea from #178.

See https://tools.ietf.org/html/rfc7435.
See #178.
This commit is contained in:
fox.cpp 2019-12-13 00:26:50 +03:00
parent c7f3e0caaa
commit eb8a974b8f
No known key found for this signature in database
GPG key ID: E76D97CCEDE90B6C
8 changed files with 379 additions and 220 deletions
Download patch file Download diff file Expand all files Collapse all files

10
internal/smtpconn/smtpconn.go
View file

@ -120,9 +120,9 @@ func (c *C) wrapClientErr(err error, serverName string) error {
}
// Connect actually estabilishes the network connection with the remote host.
func (c *C) Connect(ctx context.Context, endp config.Endpoint, starttls bool) (didTLS bool, err error) {
func (c *C) Connect(ctx context.Context, endp config.Endpoint, starttls bool, tlsConfig *tls.Config) (didTLS bool, err error) {
// TODO: Helper function to try multiple endpoints?
didTLS, cl, err := c.attemptConnect(ctx, endp, starttls)
didTLS, cl, err := c.attemptConnect(ctx, endp, starttls, tlsConfig)
if err != nil {
return false, c.wrapClientErr(err, endp.Host)
}
@ -149,7 +149,7 @@ func (err TLSError) Unwrap() error {
return err.Err
}
func (c *C) attemptConnect(ctx context.Context, endp config.Endpoint, starttls bool) (didTLS bool, cl *smtp.Client, err error) {
func (c *C) attemptConnect(ctx context.Context, endp config.Endpoint, starttls bool, tlsConfig *tls.Config) (didTLS bool, cl *smtp.Client, err error) {
var conn net.Conn
conn, err = c.Dialer(ctx, endp.Network(), endp.Address())
if err != nil {
@ -157,7 +157,7 @@ func (c *C) attemptConnect(ctx context.Context, endp config.Endpoint, starttls b
}
if endp.IsTLS() {
cfg := c.TLSConfig.Clone()
cfg := tlsConfig.Clone()
cfg.ServerName = endp.Host
conn = tls.Client(conn, cfg)
}
@ -182,7 +182,7 @@ func (c *C) attemptConnect(ctx context.Context, endp config.Endpoint, starttls b
return false, cl, nil
}
cfg := c.TLSConfig.Clone()
cfg := tlsConfig.Clone()
cfg.ServerName = endp.Host
if err := cl.StartTLS(cfg); err != nil {
// After the handshake failure, the connection may be in a bad state.

16
internal/smtpconn/smtputf8_test.go
View file

@ -46,7 +46,7 @@ func TestSMTPUTF8_Sender_UTF8_Punycode(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -75,7 +75,7 @@ func TestSMTPUTF8_Rcpt_UTF8_Punycode(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -104,7 +104,7 @@ func TestSMTPUTF8_Sender_UTF8_Reject(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -127,7 +127,7 @@ func TestSMTPUTF8_Rcpt_UTF8_Reject(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -149,7 +149,7 @@ func TestSMTPUTF8_Sender_UTF8_Domain(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -177,7 +177,7 @@ func TestSMTPUTF8_Rcpt_UTF8_Domain(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -206,7 +206,7 @@ func TestSMTPUTF8_Sender_UTF8_Username(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()
@ -235,7 +235,7 @@ func TestSMTPUTF8_Rcpt_UTF8_Username(t *testing.T) {
Scheme: "tcp",
Host: "127.0.0.1",
Port: testPort,
}, false); err != nil {
}, false, nil); err != nil {
t.Fatal(err)
}
defer c.Close()

184
internal/target/remote/connect.go
View file

@ -3,6 +3,7 @@ package remote
import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
@ -29,15 +30,11 @@ type mxConn struct {
// May be nil if MTA-STS is disabled.
stsFut *future.Future
// Future value holding the TLSA lookup results for this MX.
// May be nil if DANE is disabled.
tlsaFut *future.Future
// The MX record is DNSSEC-signed and was verified by the used resolver.
dnssecOk bool
}
func (rd *remoteDelivery) checkPolicies(ctx context.Context, mx string, didTLS bool, conn mxConn) error {
func (rd *remoteDelivery) checkPolicies(ctx context.Context, mx string, didTLS, tlsPKIX bool, conn mxConn) error {
var (
authenticated bool
requireTLS bool
@ -103,11 +100,11 @@ func (rd *remoteDelivery) checkPolicies(ctx context.Context, mx string, didTLS b
Message: fmt.Sprintf("Failed to estabilish the MX record (%s) authenticity", mx),
}
}
if requireTLS && !didTLS {
if requireTLS && (!didTLS || !tlsPKIX) {
return &exterrors.SMTPError{
Code: 550,
EnhancedCode: exterrors.EnhancedCode{5, 7, 1},
Message: fmt.Sprintf("TLS is required but unsupported or failed (mx = %s)", mx),
Message: "TLS is required but unsupported or failed (enforced by MTA-STS)",
}
}
@ -115,6 +112,104 @@ func (rd *remoteDelivery) checkPolicies(ctx context.Context, mx string, didTLS b
return nil
}
func isVerifyError(err error) bool {
_, ok := err.(x509.UnknownAuthorityError)
if ok {
return true
}
_, ok = err.(x509.HostnameError)
if ok {
return true
}
_, ok = err.(x509.ConstraintViolationError)
if ok {
return true
}
_, ok = err.(x509.CertificateInvalidError)
return ok
}
func (rd *remoteDelivery) attemptMX(ctx context.Context, conn mxConn, record *net.MX) error {
var (
daneCtx context.Context
tlsaFut *future.Future
)
if rd.rt.dane {
var daneCancel func()
daneCtx, daneCancel = context.WithCancel(ctx)
defer daneCancel()
tlsaFut = future.New()
go func() {
tlsaFut.Set(rd.lookupTLSA(daneCtx, record.Host))
}()
}
var (
tlsCfg *tls.Config
tlsPKIX = true
)
if rd.rt.tlsConfig != nil {
tlsCfg = rd.rt.tlsConfig.Clone()
tlsCfg.ServerName = record.Host
}
rd.Log.DebugMsg("trying", "mx", record.Host, "domain", conn.domain)
retry:
// smtpconn.C default TLS behavior is not useful for us, we want to handle
// TLS errors separately hence starttls=false.
_, err := conn.Connect(ctx, config.Endpoint{
Host: record.Host,
Port: smtpPort,
}, false, nil)
if err != nil {
return err
}
starttlsOk, _ := conn.Client().Extension("STARTTLS")
if starttlsOk && tlsCfg != nil {
if err := conn.Client().StartTLS(tlsCfg); err != nil {
// Attempt TLS without authentication. It is still better than
// plaintext and we might be able to actually authenticate the
// server using DANE-EE/DANE-TA later.
//
// tlsPKIX is to avoid looping forever if the same verify error
// happens with InsecureSkipVerify too (e.g. certificate is *too*
// broken).
if isVerifyError(err) && tlsPKIX {
rd.Log.Error("TLS verify error, trying without authentication", err, "mx", record.Host, "domain", conn.domain)
tlsCfg.InsecureSkipVerify = true
tlsPKIX = false
conn.Close()
goto retry
}
rd.Log.Error("TLS error, trying plaintext", err, "mx", record.Host, "domain", conn.domain)
tlsCfg = nil
tlsPKIX = false
conn.Close()
goto retry
}
}
tlsState, didTLS := conn.Client().TLSConnectionState()
if err := rd.checkPolicies(ctx, record.Host, didTLS, tlsPKIX, conn); err != nil {
conn.Close()
return err
}
_, err = rd.verifyDANE(ctx, tlsaFut, tlsState)
if err != nil {
conn.Close()
return err
}
return nil
}
func (rd *remoteDelivery) connectionForDomain(ctx context.Context, domain string) (*smtpconn.C, error) {
domain = strings.ToLower(domain)
@ -128,7 +223,6 @@ func (rd *remoteDelivery) connectionForDomain(ctx context.Context, domain string
}
conn.Dialer = rd.rt.dialer
conn.TLSConfig = rd.rt.tlsConfig
conn.Log = rd.Log
conn.Hostname = rd.rt.hostname
conn.AddrInSMTPMsg = true
@ -154,17 +248,9 @@ func (rd *remoteDelivery) connectionForDomain(ctx context.Context, domain string
}
conn.dnssecOk = dnssecOk
var (
lastErr error
daneCancel func()
)
var lastErr error
region = trace.StartRegion(ctx, "remote/Connect+TLS")
for _, record := range records {
if daneCancel != nil {
daneCancel()
daneCancel = nil
}
if record.Host == "." {
return nil, &exterrors.SMTPError{
Code: 556,
@ -173,73 +259,17 @@ func (rd *remoteDelivery) connectionForDomain(ctx context.Context, domain string
}
}
if rd.rt.dane {
var daneCtx context.Context
daneCtx, daneCancel = context.WithCancel(ctx)
conn.tlsaFut = future.New()
go func() {
conn.tlsaFut.Set(rd.lookupTLSA(daneCtx, record.Host))
}()
}
rd.Log.DebugMsg("trying", "mx", record.Host, "domain", domain)
didTLS, err := conn.Connect(ctx, config.Endpoint{
Host: record.Host,
Port: smtpPort,
}, true)
authErr := rd.checkPolicies(ctx, record.Host, didTLS, conn)
if err != nil {
lastErr = err
// If there was a TLS error and MX auth does not seem to complain
// about plaintext - reconnect without TLS.
if _, ok := err.(smtpconn.TLSError); ok && authErr == nil {
// Check if DANE is here and prevents our fallback.
if err := rd.verifyDANE(ctx, conn, tls.ConnectionState{}); err != nil {
lastErr = err
continue
}
rd.Log.Error("TLS error, falling back to plaintext", err,
"mx", record.Host, "domain", domain)
_, err := conn.Connect(ctx, config.Endpoint{
Host: record.Host,
Port: smtpPort,
}, false)
if err != nil {
// That's odd, but whatever.
continue
}
} else {
continue
if err := rd.attemptMX(ctx, conn, record); err != nil {
if len(records) != 0 {
rd.Log.Error("cannot use MX", err, "mx", record.Host, "domain", domain)
}
}
if authErr != nil {
conn.Close()
lastErr = authErr
continue
}
tlsState, _ := conn.Client().TLSConnectionState()
if err := rd.verifyDANE(ctx, conn, tlsState); err != nil {
conn.Close()
lastErr = err
continue
}
break
}
region.End()
// Cancel it to make staticcheck happy. verifyDANE call in loop above
// will wait for it to complete anyway.
if daneCancel != nil {
daneCancel()
}
// Stil not connected? Bail out.
if conn.Client() == nil {
return nil, &exterrors.SMTPError{
@ -254,8 +284,6 @@ func (rd *remoteDelivery) connectionForDomain(ctx context.Context, domain string
}
}
rd.Log.DebugMsg("connected", "mx", conn.ServerName(), "domain", domain)
if err := conn.Mail(ctx, rd.mailFrom, rd.msgMeta.SMTPOpts); err != nil {
conn.Close()
return nil, err

67
internal/target/remote/dane.go
View file

@ -3,9 +3,11 @@ package remote
import (
"context"
"crypto/tls"
"crypto/x509"
"github.com/foxcpp/maddy/internal/dns"
"github.com/foxcpp/maddy/internal/exterrors"
"github.com/foxcpp/maddy/internal/future"
)
func (rd *remoteDelivery) lookupTLSA(ctx context.Context, host string) ([]dns.TLSA, error) {
@ -26,17 +28,17 @@ func (rd *remoteDelivery) lookupTLSA(ctx context.Context, host string) ([]dns.TL
return recs, nil
}
func (rd *remoteDelivery) verifyDANE(ctx context.Context, conn mxConn, connState tls.ConnectionState) error {
func (rd *remoteDelivery) verifyDANE(ctx context.Context, tlsaFut *future.Future, connState tls.ConnectionState) (overridePKIX bool, err error) {
// DANE is disabled.
if conn.tlsaFut == nil {
return nil
if tlsaFut == nil {
return false, nil
}
recsI, err := conn.tlsaFut.GetContext(ctx)
recsI, err := tlsaFut.GetContext(ctx)
if err != nil {
// No records.
if err == dns.ErrNotFound {
return nil
return false, nil
}
// Lookup error here indicates a resolution failure or may also
@ -46,27 +48,22 @@ func (rd *remoteDelivery) verifyDANE(ctx context.Context, conn mxConn, connState
// We assume DANE failure in both cases as a safety measure.
// However, there is a possibility of a temporary error condition,
// so we mark it as such.
return exterrors.WithTemporary(err, true)
return false, exterrors.WithTemporary(err, true)
}
recs := recsI.([]dns.TLSA)
return verifyDANE(recs, connState)
}
func verifyDANE(recs []dns.TLSA, connState tls.ConnectionState) error {
requireTLS := func() error {
if !connState.HandshakeComplete {
return &exterrors.SMTPError{
Code: 550,
EnhancedCode: exterrors.EnhancedCode{5, 7, 1},
Message: "TLS is required but unsupported or failed (DANE)",
TargetName: "remote",
Misc: map[string]interface{}{
"remote_server": connState.ServerName,
},
}
}
return nil
func verifyDANE(recs []dns.TLSA, connState tls.ConnectionState) (overridePKIX bool, err error) {
tlsErr := &exterrors.SMTPError{
Code: 550,
EnhancedCode: exterrors.EnhancedCode{5, 7, 1},
Message: "TLS is required but unsupported or failed (enforced by DANE)",
TargetName: "remote",
Misc: map[string]interface{}{
"remote_server": connState.ServerName,
},
}
// See https://tools.ietf.org/html/rfc6698#appendix-B.2
@ -77,12 +74,12 @@ func verifyDANE(recs []dns.TLSA, connState tls.ConnectionState) error {
// We assume upstream resolver will generate an error if the DNSSEC
// signature is bogus so this case is "DNSSEC-authenticated denial of existence".
if len(recs) == 0 {
return nil
return false, nil
}
// Require TLS even if all records are not usable, per Section 2.2 of RFC 7672.
if err := requireTLS(); err != nil {
return err
if !connState.HandshakeComplete {
return false, tlsErr
}
// Ignore invalid records.
@ -105,32 +102,30 @@ func verifyDANE(recs []dns.TLSA, connState tls.ConnectionState) error {
for _, rec := range validRecs {
switch rec.Usage {
case 0, 2: // CA constraint (PKIX-TA) and Trust Anchor Assertion (DANE-TA)
// For DANE-TA, this should override failing PKIX check, but
// currently it is not done and I (fox.cpp) do not see a way to
// implement this. Perhaps we can get away without doing so since
// no public mail server is going to use invalid certs, right?
// TODO
chains := connState.VerifiedChains
if len(chains) == 0 { // Happens if InsecureSkipVerify=true
chains = [][]*x509.Certificate{connState.PeerCertificates}
}
for _, chain := range connState.VerifiedChains {
// TODO: DANE-TA requires ServerName match so do not override PKIX
// unless it matches.
for _, chain := range chains {
for _, cert := range chain {
if cert.IsCA && rec.Verify(cert) == nil {
return nil
return false, nil
}
}
}
case 1, 3: // Service certificate constraint (PKIX-EE) and Domain issued certificate (DANE-EE)
// Comment for DANE-TA above applies here too for DANE-EE.
// Also, SMTP DANE profile does not require ServerName match for
// DANE-EE, but we do not override it too.
if rec.Verify(connState.PeerCertificates[0]) == nil {
return nil
return rec.Usage == 3, nil
}
}
}
// There are valid records, but none matched.
return &exterrors.SMTPError{
return false, &exterrors.SMTPError{
Code: 550,
EnhancedCode: exterrors.EnhancedCode{5, 7, 0},
Message: "No matching TLSA records",

125
internal/target/remote/dane_test.go
View file

@ -43,6 +43,25 @@ func targetWithExtResolver(t *testing.T, zones map[string]mockdns.Zone) (*mockdn
}
}
func tlsaRecord(name string, usage, matchType, selector uint8, cert string) map[miekgdns.Type][]miekgdns.RR {
return map[miekgdns.Type][]miekgdns.RR{
miekgdns.Type(miekgdns.TypeTLSA): []miekgdns.RR{
&miekgdns.TLSA{
Hdr: miekgdns.RR_Header{
Name: name,
Class: miekgdns.ClassINET,
Rrtype: miekgdns.TypeTLSA,
Ttl: 9999,
},
Usage: usage,
MatchingType: matchType,
Selector: selector,
Certificate: cert,
},
},
}
}
func TestRemoteDelivery_DANE_Ok(t *testing.T) {
clientCfg, be, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv.Close()
@ -57,22 +76,9 @@ func TestRemoteDelivery_DANE_Ok(t *testing.T) {
},
"_25._tcp.mx.example.invalid.": {
AD: true,
Misc: map[miekgdns.Type][]miekgdns.RR{
miekgdns.Type(miekgdns.TypeTLSA): []miekgdns.RR{
&miekgdns.TLSA{
Hdr: miekgdns.RR_Header{
Name: "_25._tcp.mx.example.invalid.",
Class: miekgdns.ClassINET,
Rrtype: miekgdns.TypeTLSA,
Ttl: 9999,
},
Usage: 3,
MatchingType: 1,
Selector: 1,
Certificate: "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf",
},
},
},
Misc: tlsaRecord(
"_25._tcp.mx.example.invalid.",
3, 1, 1, "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf"),
},
}
@ -99,22 +105,9 @@ func TestRemoteDelivery_DANE_NonADIgnore(t *testing.T) {
A: []string{"127.0.0.1"},
},
"_25._tcp.mx.example.invalid.": {
Misc: map[miekgdns.Type][]miekgdns.RR{
miekgdns.Type(miekgdns.TypeTLSA): []miekgdns.RR{
&miekgdns.TLSA{
Hdr: miekgdns.RR_Header{
Name: "_25._tcp.mx.example.invalid.",
Class: miekgdns.ClassINET,
Rrtype: miekgdns.TypeTLSA,
Ttl: 9999,
},
Usage: 3,
MatchingType: 1,
Selector: 1,
Certificate: "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf",
},
},
},
Misc: tlsaRecord(
"_25._tcp.mx.example.invalid.",
3, 1, 1, "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf"),
},
}
@ -141,22 +134,9 @@ func TestRemoteDelivery_DANE_Mismatch(t *testing.T) {
},
"_25._tcp.mx.example.invalid.": {
AD: true,
Misc: map[miekgdns.Type][]miekgdns.RR{
miekgdns.Type(miekgdns.TypeTLSA): []miekgdns.RR{
&miekgdns.TLSA{
Hdr: miekgdns.RR_Header{
Name: "_25._tcp.mx.example.invalid.",
Class: miekgdns.ClassINET,
Rrtype: miekgdns.TypeTLSA,
Ttl: 9999,
},
Usage: 3,
MatchingType: 1,
Selector: 1,
Certificate: "b5b4d02f996f6385ebe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf",
},
},
},
Misc: tlsaRecord(
"_25._tcp.mx.example.invalid.",
3, 1, 1, "ffb5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf"),
},
}
@ -245,22 +225,9 @@ func TestRemoteDelivery_DANE_NoTLS(t *testing.T) {
},
"_25._tcp.mx.example.invalid.": {
AD: true,
Misc: map[miekgdns.Type][]miekgdns.RR{
miekgdns.Type(miekgdns.TypeTLSA): []miekgdns.RR{
&miekgdns.TLSA{
Hdr: miekgdns.RR_Header{
Name: "_25._tcp.mx.example.invalid.",
Class: miekgdns.ClassINET,
Rrtype: miekgdns.TypeTLSA,
Ttl: 9999,
},
Usage: 3,
MatchingType: 1,
Selector: 1,
Certificate: "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf",
},
},
},
Misc: tlsaRecord(
"_25._tcp.mx.example.invalid.",
3, 1, 1, "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf"),
},
}
dnsSrv, tgt := targetWithExtResolver(t, zones)
@ -292,31 +259,25 @@ func TestRemoteDelivery_DANE_TLSError(t *testing.T) {
},
"_25._tcp.mx.example.invalid.": {
AD: true,
Misc: map[miekgdns.Type][]miekgdns.RR{
miekgdns.Type(miekgdns.TypeTLSA): []miekgdns.RR{
&miekgdns.TLSA{
Hdr: miekgdns.RR_Header{
Name: "_25._tcp.mx.example.invalid.",
Class: miekgdns.ClassINET,
Rrtype: miekgdns.TypeTLSA,
Ttl: 9999,
},
Usage: 3,
MatchingType: 1,
Selector: 1,
Certificate: "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf",
},
},
},
Misc: tlsaRecord(
"_25._tcp.mx.example.invalid.",
3, 1, 1, "a9b5cb4d02f996f6385debe9a8952f1af1f4aec7eae0f37c2cd6d0d8ee8391cf"),
},
}
dnsSrv, tgt := targetWithExtResolver(t, zones)
defer dnsSrv.Close()
tgt.mxAuth[AuthDNSSEC] = struct{}{}
tgt.dane = true
tgt.tlsConfig = &tls.Config{}
// tgt.tlsConfig not set to trust the test server certificate.
// Cause failure through version incompatibility.
tgt.tlsConfig = &tls.Config{
MaxVersion: tls.VersionTLS12,
MinVersion: tls.VersionTLS12,
}
srv.TLSConfig.MinVersion = tls.VersionTLS11
srv.TLSConfig.MaxVersion = tls.VersionTLS11
// DANE should prevent the fallback to plaintext.
_, err := testutils.DoTestDeliveryErr(t, tgt, "test@example.com", []string{"test@example.invalid"})
if err == nil {
t.Error("Expected an error, got none")

96
internal/target/remote/mxauth_test.go
View file

@ -2,6 +2,7 @@ package remote
import (
"context"
"crypto/tls"
"errors"
"net"
"strconv"
@ -246,6 +247,101 @@ func TestRemoteDelivery_AuthMX_MTASTS_Fail(t *testing.T) {
}
}
func TestRemoteDelivery_AuthMX_MTASTS_NoTLS(t *testing.T) {
be1, srv1 := testutils.SMTPServer(t, "127.0.0.1:"+smtpPort)
defer srv1.Close()
defer testutils.CheckSMTPConnLeak(t, srv1)
zones := map[string]mockdns.Zone{
"example.invalid.": {
MX: []net.MX{{Host: "mx.example.invalid.", Pref: 10}},
},
"mx.example.invalid.": {
A: []string{"127.0.0.1"},
},
}
resolver := &mockdns.Resolver{Zones: zones}
tgt := Target{
name: "remote",
hostname: "mx.example.com",
tlsConfig: &tls.Config{},
resolver: &mockdns.Resolver{Zones: zones},
dialer: resolver.DialContext,
extResolver: nil,
requireMXAuth: true,
mxAuth: map[string]struct{}{AuthMTASTS: {}},
mtastsGet: func(ctx context.Context, domain string) (*mtasts.Policy, error) {
if domain != "example.invalid" {
return nil, errors.New("Wrong domain in lookup")
}
return &mtasts.Policy{
Mode: mtasts.ModeEnforce,
MX: []string{"mx.example.invalid"},
}, nil
},
Log: testutils.Logger(t, "remote"),
}
_, err := testutils.DoTestDeliveryErr(t, &tgt, "test@example.com", []string{"test@example.invalid"})
if err == nil {
t.Fatal("Expected an error, got none")
}
if be1.MailFromCounter != 0 {
t.Fatal("MAIL FROM issued for server failing authentication")
}
}
func TestRemoteDelivery_AuthMX_MTASTS_RequirePKIX(t *testing.T) {
_, be1, srv1 := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv1.Close()
defer testutils.CheckSMTPConnLeak(t, srv1)
zones := map[string]mockdns.Zone{
"example.invalid.": {
MX: []net.MX{{Host: "mx.example.invalid.", Pref: 10}},
},
"mx.example.invalid.": {
A: []string{"127.0.0.1"},
},
}
resolver := &mockdns.Resolver{Zones: zones}
tgt := Target{
name: "remote",
hostname: "mx.example.com",
// Client not configured to trust the server cert.
tlsConfig: &tls.Config{},
resolver: &mockdns.Resolver{Zones: zones},
dialer: resolver.DialContext,
extResolver: nil,
requireMXAuth: true,
mxAuth: map[string]struct{}{AuthMTASTS: {}},
mtastsGet: func(ctx context.Context, domain string) (*mtasts.Policy, error) {
if domain != "example.invalid" {
return nil, errors.New("Wrong domain in lookup")
}
return &mtasts.Policy{
Mode: mtasts.ModeEnforce,
MX: []string{"mx.example.invalid"},
}, nil
},
Log: testutils.Logger(t, "remote"),
}
_, err := testutils.DoTestDeliveryErr(t, &tgt, "test@example.com", []string{"test@example.invalid"})
if err == nil {
t.Fatal("Expected an error, got none")
}
if be1.MailFromCounter != 0 {
t.Fatal("MAIL FROM issued for server failing authentication")
}
}
func TestRemoteDelivery_AuthMX_MTASTS_NoPolicy(t *testing.T) {
clientCfg, be1, srv1 := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv1.Close()

98
internal/target/remote/remote_test.go
View file

@ -850,7 +850,7 @@ func TestRemoteDelivery_Split_BodyErr_NonAtomic(t *testing.T) {
}
func TestRemoteDelivery_TLSErrFallback(t *testing.T) {
_, be, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
clientCfg, be, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv.Close()
defer testutils.CheckSMTPConnLeak(t, srv)
zones := map[string]mockdns.Zone{
@ -863,13 +863,19 @@ func TestRemoteDelivery_TLSErrFallback(t *testing.T) {
}
resolver := &mockdns.Resolver{Zones: zones}
// Cause failure through version incompatibility.
clientCfg.MaxVersion = tls.VersionTLS12
clientCfg.MinVersion = tls.VersionTLS12
srv.TLSConfig.MinVersion = tls.VersionTLS11
srv.TLSConfig.MaxVersion = tls.VersionTLS11
tgt := Target{
name: "remote",
hostname: "mx.example.com",
resolver: &mockdns.Resolver{Zones: zones},
dialer: resolver.DialContext,
extResolver: nil,
tlsConfig: &tls.Config{},
tlsConfig: clientCfg,
Log: testutils.Logger(t, "remote"),
}
@ -937,7 +943,44 @@ func TestRemoteDelivery_RequireTLS_Present(t *testing.T) {
}
func TestRemoteDelivery_RequireTLS_NoErrFallback(t *testing.T) {
_, _, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
clientCfg, _, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv.Close()
defer testutils.CheckSMTPConnLeak(t, srv)
zones := map[string]mockdns.Zone{
"example.invalid.": {
MX: []net.MX{{Host: "mx.example.invalid.", Pref: 10}},
},
"mx.example.invalid.": {
A: []string{"127.0.0.1"},
},
}
resolver := &mockdns.Resolver{Zones: zones}
// Cause failure through version incompatibility.
clientCfg.MaxVersion = tls.VersionTLS12
clientCfg.MinVersion = tls.VersionTLS12
srv.TLSConfig.MinVersion = tls.VersionTLS11
srv.TLSConfig.MaxVersion = tls.VersionTLS11
tgt := Target{
name: "remote",
hostname: "mx.example.com",
resolver: &mockdns.Resolver{Zones: zones},
dialer: resolver.DialContext,
extResolver: nil,
tlsConfig: clientCfg,
requireTLS: true,
Log: testutils.Logger(t, "remote"),
}
_, err := testutils.DoTestDeliveryErr(t, &tgt, "test@example.com", []string{"test@example.invalid"})
if err == nil {
t.Fatal("Expected an error, got none")
}
}
func TestRemoteDelivery_TLS_FallbackNoVerify(t *testing.T) {
_, be, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv.Close()
defer testutils.CheckSMTPConnLeak(t, srv)
zones := map[string]mockdns.Zone{
@ -956,15 +999,52 @@ func TestRemoteDelivery_RequireTLS_NoErrFallback(t *testing.T) {
resolver: &mockdns.Resolver{Zones: zones},
dialer: resolver.DialContext,
extResolver: nil,
tlsConfig: &tls.Config{},
requireTLS: true,
// Client is not configured to trust the server cert.
tlsConfig: &tls.Config{},
Log: testutils.Logger(t, "remote"),
}
testutils.DoTestDelivery(t, &tgt, "test@example.com", []string{"test@example.invalid"})
be.CheckMsg(t, 0, "test@example.com", []string{"test@example.invalid"})
// But it should still be delivered over TLS.
if !be.Messages[0].State.TLS.HandshakeComplete {
t.Fatal("Message was not delivered over TLS")
}
}
func TestRemoteDelivery_TLS_FallbackPlaintext(t *testing.T) {
clientCfg, be, srv := testutils.SMTPServerSTARTTLS(t, "127.0.0.1:"+smtpPort)
defer srv.Close()
defer testutils.CheckSMTPConnLeak(t, srv)
zones := map[string]mockdns.Zone{
"example.invalid.": {
MX: []net.MX{{Host: "mx.example.invalid.", Pref: 10}},
},
"mx.example.invalid.": {
A: []string{"127.0.0.1"},
},
}
resolver := &mockdns.Resolver{Zones: zones}
// Cause failure through version incompatibility.
clientCfg.MaxVersion = tls.VersionTLS12
clientCfg.MinVersion = tls.VersionTLS12
srv.TLSConfig.MinVersion = tls.VersionTLS11
srv.TLSConfig.MaxVersion = tls.VersionTLS11
tgt := Target{
name: "remote",
hostname: "mx.example.com",
resolver: &mockdns.Resolver{Zones: zones},
dialer: resolver.DialContext,
extResolver: nil,
tlsConfig: clientCfg,
Log: testutils.Logger(t, "remote"),
}
_, err := testutils.DoTestDeliveryErr(t, &tgt, "test@example.com", []string{"test@example.invalid"})
if err == nil {
t.Fatal("Expected an error, got none")
}
testutils.DoTestDelivery(t, &tgt, "test@example.com", []string{"test@example.invalid"})
be.CheckMsg(t, 0, "test@example.com", []string{"test@example.invalid"})
}
func TestMain(m *testing.M) {

3
internal/target/smtp_downstream/smtp_downstream.go
View file

@ -152,13 +152,12 @@ func (d *delivery) connect(ctx context.Context) error {
var lastErr error
conn := smtpconn.New()
conn.TLSConfig = &d.u.tlsConfig
conn.Log = d.log
conn.Hostname = d.u.hostname
conn.AddrInSMTPMsg = false
for _, endp := range d.u.endpoints {
didTLS, err := conn.Connect(ctx, endp, d.u.attemptStartTLS)
didTLS, err := conn.Connect(ctx, endp, d.u.attemptStartTLS, &d.u.tlsConfig)
if err != nil {
if len(d.u.endpoints) != 1 {
d.log.Msg("connect error", err, "downstream_server", net.JoinHostPort(endp.Host, endp.Port))