Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not require the SMTPUTF8 extension when not needed #146

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dmarc/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func ExampleVerify() {

// Message to verify.
msg := strings.NewReader("From: <[email protected]>\r\nMore: headers\r\n\r\nBody\r\n")
msgFrom, _, _, err := message.From(slog.Default(), true, msg)
msgFrom, _, _, err := message.From(slog.Default(), true, msg, nil)
if err != nil {
log.Fatalf("parsing message for from header: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2176,7 +2176,7 @@ can be found in message headers.

data, err := io.ReadAll(os.Stdin)
xcheckf(err, "read message")
dmarcFrom, _, _, err := message.From(c.log.Logger, false, bytes.NewReader(data))
dmarcFrom, _, _, err := message.From(c.log.Logger, false, bytes.NewReader(data), nil)
xcheckf(err, "extract dmarc from message")

const ignoreTestMode = false
Expand Down
15 changes: 10 additions & 5 deletions message/from.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,22 @@ import (
// From headers may be present. From returns an error if there is not exactly
// one address. This address can be used for evaluating a DMARC policy against
// SPF and DKIM results.
func From(elog *slog.Logger, strict bool, r io.ReaderAt) (raddr smtp.Address, envelope *Envelope, header textproto.MIMEHeader, rerr error) {
func From(elog *slog.Logger, strict bool, r io.ReaderAt, p *Part) (raddr smtp.Address, envelope *Envelope, header textproto.MIMEHeader, rerr error) {
log := mlog.New("message", elog)

// ../rfc/7489:1243

// todo: only allow utf8 if enabled in session/message?

p, err := Parse(log.Logger, strict, r)
if err != nil {
// todo: should we continue with p, perhaps headers can be parsed?
return raddr, nil, nil, fmt.Errorf("parsing message: %v", err)
var err error
if p == nil {
var pp Part
pp, err = Parse(log.Logger, strict, r)
if err != nil {
// todo: should we continue with p, perhaps headers can be parsed?
return raddr, nil, nil, fmt.Errorf("parsing message: %v", err)
}
p = &pp
}
header, err = p.Header()
if err != nil {
Expand Down
128 changes: 100 additions & 28 deletions smtpserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"strings"
"sync"
"time"
"unicode"

"golang.org/x/exp/maps"
"golang.org/x/text/unicode/norm"
Expand Down Expand Up @@ -331,7 +332,8 @@ type conn struct {
futureRelease time.Time // MAIL FROM with HOLDFOR or HOLDUNTIL.
futureReleaseRequest string // For use in DSNs, either "for;" or "until;" plus original value. ../rfc/4865:305
has8bitmime bool // If MAIL FROM parameter BODY=8BITMIME was sent. Required for SMTPUTF8.
smtputf8 bool // todo future: we should keep track of this per recipient. perhaps only a specific recipient requires smtputf8, e.g. due to a utf8 localpart. we should decide ourselves if the message needs smtputf8, e.g. due to utf8 header values.
smtputf8 bool // todo future: we should keep track of this per recipient. perhaps only a specific recipient requires smtputf8, e.g. due to a utf8 localpart.
msgsmtputf8 bool // Is SMTPUTF8 required for the received message. Default to the same value as `smtputf8`, but is re-evaluated after the whole message (envelope and data) is received.
recipients []rcptAccount
}

Expand Down Expand Up @@ -372,6 +374,7 @@ func (c *conn) rset() {
c.futureReleaseRequest = ""
c.has8bitmime = false
c.smtputf8 = false
c.msgsmtputf8 = false
c.recipients = nil
}

Expand Down Expand Up @@ -764,7 +767,7 @@ func command(c *conn) {
c.cmd = cmdl
c.cmdStart = time.Now()

p := newParser(args, c.smtputf8, c)
p := newParser(args, c.msgsmtputf8, c)
lmeunier marked this conversation as resolved.
Show resolved Hide resolved
fn, ok := commands[cmdl]
if !ok {
c.cmd = "(unknown)"
Expand Down Expand Up @@ -1419,6 +1422,7 @@ func (c *conn) cmdMail(p *parser) {
case "SMTPUTF8":
// ../rfc/6531:213
c.smtputf8 = true
c.msgsmtputf8 = true
case "REQUIRETLS":
// ../rfc/8689:155
if !c.tls {
Expand Down Expand Up @@ -1468,7 +1472,7 @@ func (c *conn) cmdMail(p *parser) {
}

// We now know if we have to parse the address with support for utf8.
pp := newParser(rawRevPath, c.smtputf8, c)
pp := newParser(rawRevPath, c.msgsmtputf8, c)
rpath := pp.xbareReversePath()
pp.xempty()
pp = nil
Expand Down Expand Up @@ -1654,6 +1658,53 @@ func (c *conn) cmdRcpt(p *parser) {
c.bwritecodeline(smtp.C250Completed, smtp.SeAddr1Other0, "now on the list", nil)
}

// ../rfc/6531:497
func (c *conn) isSmtpUtf8Required(part *message.Part) bool {
lmeunier marked this conversation as resolved.
Show resolved Hide resolved
hasNonASCII := func(r io.Reader) bool {
lmeunier marked this conversation as resolved.
Show resolved Hide resolved
br := bufio.NewReader(r)
for {
b, err := br.ReadByte()
if err == io.EOF {
break
}
xcheckf(err, "read header")
if b > unicode.MaxASCII {
return true
}
}
return false
}
var hasNonASCIIPartHeader func(p *message.Part) bool
hasNonASCIIPartHeader = func(p *message.Part) bool {
if hasNonASCII(p.HeaderReader()) {
return true
}
for _, pp := range p.Parts {
if hasNonASCIIPartHeader(&pp) {
return true
}
}
return false
}

// Check "MAIL FROM"
if hasNonASCII(strings.NewReader(string(c.mailFrom.Localpart))) {
return true
}
// Check all "RCPT TO"
for _, rcpt := range c.recipients {
if hasNonASCII(strings.NewReader(string(rcpt.rcptTo.Localpart))) {
return true
}
}
// Check header in all message parts
if hasNonASCIIPartHeader(part) {
return true
}

return false
}

// ../rfc/5321:1992 ../rfc/5321:1098
func (c *conn) cmdData(p *parser) {
c.xneedHello()
Expand Down Expand Up @@ -1752,6 +1803,26 @@ func (c *conn) cmdData(p *parser) {
}
}

// Now that we have all the whole message (envelope + data), we can check if the SMTPUTF8 extension is required.
// The check happens only when the client required the SMTPUTF8 extension.
lmeunier marked this conversation as resolved.
Show resolved Hide resolved
var part *message.Part
if c.smtputf8 {
// Try to parse the message.
// Do nothing if something bad happen during Parse and Walk, just keep the current value for c.msgsmtputf8.
p, err := message.Parse(c.log.Logger, true, dataFile)
if err == nil {
// Message parsed without error. Keep the result to avoid parsing the message again.
part = &p
err = part.Walk(c.log.Logger, nil)
if err == nil {
c.msgsmtputf8 = c.isSmtpUtf8Required(part)
}
}
if c.smtputf8 != c.msgsmtputf8 {
c.log.Debug("SMTPUTF8 flag changed", slog.Bool("received SMTPUTF8", c.smtputf8), slog.Bool("evaluated SMTPUTF8", c.msgsmtputf8))
lmeunier marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Prepare "Received" header.
// ../rfc/5321:2051 ../rfc/5321:3302
// ../rfc/5321:3311 ../rfc/6531:578
Expand All @@ -1761,14 +1832,14 @@ func (c *conn) cmdData(p *parser) {
if c.submission {
// Hide internal hosts.
// todo future: make this a config option, where admins specify ip ranges that they don't want exposed. also see ../rfc/5321:4321
recvFrom = message.HeaderCommentDomain(mox.Conf.Static.HostnameDomain, c.smtputf8)
recvFrom = message.HeaderCommentDomain(mox.Conf.Static.HostnameDomain, c.msgsmtputf8)
} else {
if len(c.hello.IP) > 0 {
recvFrom = smtp.AddressLiteral(c.hello.IP)
} else {
// ASCII-only version added after the extended-domain syntax below, because the
// comment belongs to "BY" which comes immediately after "FROM".
recvFrom = c.hello.Domain.XName(c.smtputf8)
recvFrom = c.hello.Domain.XName(c.msgsmtputf8)
}
iprevctx, iprevcancel := context.WithTimeout(cmdctx, time.Minute)
var revName string
Expand All @@ -1787,24 +1858,24 @@ func (c *conn) cmdData(p *parser) {
}
name = strings.TrimSuffix(name, ".")
recvFrom += " ("
if name != "" && name != c.hello.Domain.XName(c.smtputf8) {
if name != "" && name != c.hello.Domain.XName(c.msgsmtputf8) {
recvFrom += name + " "
}
recvFrom += smtp.AddressLiteral(c.remoteIP) + ")"
if c.smtputf8 && c.hello.Domain.Unicode != "" {
if c.msgsmtputf8 && c.hello.Domain.Unicode != "" {
recvFrom += " (" + c.hello.Domain.ASCII + ")"
}
}
recvBy := mox.Conf.Static.HostnameDomain.XName(c.smtputf8)
recvBy := mox.Conf.Static.HostnameDomain.XName(c.msgsmtputf8)
recvBy += " (" + smtp.AddressLiteral(c.localIP) + ")" // todo: hide ip if internal?
if c.smtputf8 && mox.Conf.Static.HostnameDomain.Unicode != "" {
if c.msgsmtputf8 && mox.Conf.Static.HostnameDomain.Unicode != "" {
// This syntax is part of "VIA".
recvBy += " (" + mox.Conf.Static.HostnameDomain.ASCII + ")"
}

// ../rfc/3848:34 ../rfc/6531:791
with := "SMTP"
if c.smtputf8 {
if c.msgsmtputf8 {
with = "UTF8SMTP"
} else if c.ehlo {
with = "ESMTP"
Expand Down Expand Up @@ -1849,7 +1920,7 @@ func (c *conn) cmdData(p *parser) {
// handle it first, and leave the rest of the function for handling wild west
// internet traffic.
if c.submission {
c.submit(cmdctx, recvHdrFor, msgWriter, dataFile)
c.submit(cmdctx, recvHdrFor, msgWriter, dataFile, part)
} else {
c.deliver(cmdctx, recvHdrFor, msgWriter, iprevStatus, iprevAuthentic, dataFile)
}
Expand All @@ -1873,7 +1944,7 @@ func hasTLSRequiredNo(h textproto.MIMEHeader) bool {
}

// submit is used for mail from authenticated users that we will try to deliver.
func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWriter *message.Writer, dataFile *os.File) {
func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWriter *message.Writer, dataFile *os.File, part *message.Part) {
// Similar between ../smtpserver/server.go:/submit\( and ../webmail/webmail.go:/MessageSubmit\(

var msgPrefix []byte
Expand All @@ -1882,7 +1953,7 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
// for other users.
// We don't check the Sender field, there is no expectation of verification, ../rfc/7489:2948
// and with Resent headers it seems valid to have someone else as Sender. ../rfc/5322:1578
msgFrom, _, header, err := message.From(c.log.Logger, true, dataFile)
msgFrom, _, header, err := message.From(c.log.Logger, true, dataFile, part)
if err != nil {
metricSubmission.WithLabelValues("badmessage").Inc()
c.log.Infox("parsing message From address", err, slog.String("user", c.username))
Expand Down Expand Up @@ -1919,7 +1990,7 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
// ../rfc/5321:4131 ../rfc/6409:751
messageID := header.Get("Message-Id")
if messageID == "" {
messageID = mox.MessageIDGen(c.smtputf8)
messageID = mox.MessageIDGen(c.msgsmtputf8)
msgPrefix = append(msgPrefix, fmt.Sprintf("Message-Id: <%s>\r\n", messageID)...)
}

Expand Down Expand Up @@ -1960,7 +2031,7 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
if len(selectors) > 0 {
if canonical, err := mox.CanonicalLocalpart(msgFrom.Localpart, confDom); err != nil {
c.log.Errorx("determining canonical localpart for dkim signing", err, slog.Any("localpart", msgFrom.Localpart))
} else if dkimHeaders, err := dkim.Sign(ctx, c.log.Logger, canonical, msgFrom.Domain, selectors, c.smtputf8, store.FileMsgReader(msgPrefix, dataFile)); err != nil {
} else if dkimHeaders, err := dkim.Sign(ctx, c.log.Logger, canonical, msgFrom.Domain, selectors, c.msgsmtputf8, store.FileMsgReader(msgPrefix, dataFile)); err != nil {
c.log.Errorx("dkim sign for domain", err, slog.Any("domain", msgFrom.Domain))
metricServerErrors.WithLabelValues("dkimsign").Inc()
} else {
Expand All @@ -1969,14 +2040,14 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
}

authResults := message.AuthResults{
Hostname: mox.Conf.Static.HostnameDomain.XName(c.smtputf8),
Comment: mox.Conf.Static.HostnameDomain.ASCIIExtra(c.smtputf8),
Hostname: mox.Conf.Static.HostnameDomain.XName(c.msgsmtputf8),
Comment: mox.Conf.Static.HostnameDomain.ASCIIExtra(c.msgsmtputf8),
Methods: []message.AuthMethod{
{
Method: "auth",
Result: "pass",
Props: []message.AuthProp{
message.MakeAuthProp("smtp", "mailfrom", c.mailFrom.XString(c.smtputf8), true, c.mailFrom.ASCIIExtra(c.smtputf8)),
message.MakeAuthProp("smtp", "mailfrom", c.mailFrom.XString(c.msgsmtputf8), true, c.mailFrom.ASCIIExtra(c.msgsmtputf8)),
},
},
},
Expand Down Expand Up @@ -2010,7 +2081,7 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
}
xmsgPrefix := append([]byte(recvHdrFor(rcptTo)), msgPrefix...)
msgSize := int64(len(xmsgPrefix)) + msgWriter.Size
qm := queue.MakeMsg(*c.mailFrom, rcptAcc.rcptTo, msgWriter.Has8bit, c.smtputf8, msgSize, messageID, xmsgPrefix, c.requireTLS, now)
qm := queue.MakeMsg(*c.mailFrom, rcptAcc.rcptTo, msgWriter.Has8bit, c.msgsmtputf8, msgSize, messageID, xmsgPrefix, c.requireTLS, now)
if !c.futureRelease.IsZero() {
qm.NextAttempt = c.futureRelease
qm.FutureReleaseRequest = c.futureReleaseRequest
Expand All @@ -2032,6 +2103,7 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
slog.Any("mailfrom", *c.mailFrom),
slog.Any("rcptto", rcptAcc.rcptTo),
slog.Bool("smtputf8", c.smtputf8),
slog.Bool("msgsmtputf8", c.msgsmtputf8),
slog.Int64("msgsize", qml[i].Size))
}

Expand Down Expand Up @@ -2107,7 +2179,7 @@ func (c *conn) xlocalserveError(lp smtp.Localpart) {
func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgWriter *message.Writer, iprevStatus iprev.Status, iprevAuthentic bool, dataFile *os.File) {
// todo: in decision making process, if we run into (some) temporary errors, attempt to continue. if we decide to accept, all good. if we decide to reject, we'll make it a temporary reject.

msgFrom, envelope, headers, err := message.From(c.log.Logger, false, dataFile)
msgFrom, envelope, headers, err := message.From(c.log.Logger, false, dataFile, nil)
if err != nil {
c.log.Infox("parsing message for From address", err)
}
Expand All @@ -2129,7 +2201,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW

// We'll be building up an Authentication-Results header.
authResults := message.AuthResults{
Hostname: mox.Conf.Static.HostnameDomain.XName(c.smtputf8),
Hostname: mox.Conf.Static.HostnameDomain.XName(c.msgsmtputf8),
}

commentAuthentic := func(v bool) string {
Expand Down Expand Up @@ -2175,7 +2247,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
dkimctx, dkimcancel := context.WithTimeout(ctx, time.Minute)
defer dkimcancel()
// todo future: we could let user configure which dkim headers they require
dkimResults, dkimErr = dkim.Verify(dkimctx, c.log.Logger, c.resolver, c.smtputf8, dkim.DefaultPolicy, dataFile, ignoreTestMode)
dkimResults, dkimErr = dkim.Verify(dkimctx, c.log.Logger, c.resolver, c.msgsmtputf8, dkim.DefaultPolicy, dataFile, ignoreTestMode)
dkimcancel()
}()

Expand Down Expand Up @@ -2271,8 +2343,8 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
sig := base64.StdEncoding.EncodeToString(r.Sig.Signature)
sig = sig[:12] // Must be at least 8 characters and unique among the signatures.
props = []message.AuthProp{
message.MakeAuthProp("header", "d", r.Sig.Domain.XName(c.smtputf8), true, r.Sig.Domain.ASCIIExtra(c.smtputf8)),
message.MakeAuthProp("header", "s", r.Sig.Selector.XName(c.smtputf8), true, r.Sig.Selector.ASCIIExtra(c.smtputf8)),
message.MakeAuthProp("header", "d", r.Sig.Domain.XName(c.msgsmtputf8), true, r.Sig.Domain.ASCIIExtra(c.msgsmtputf8)),
message.MakeAuthProp("header", "s", r.Sig.Selector.XName(c.msgsmtputf8), true, r.Sig.Selector.ASCIIExtra(c.msgsmtputf8)),
message.MakeAuthProp("header", "a", r.Sig.Algorithm(), false, ""),
message.MakeAuthProp("header", "b", sig, false, ""), // ../rfc/6008:147
}
Expand Down Expand Up @@ -2318,7 +2390,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
}
var props []message.AuthProp
if spfIdentity != nil {
props = []message.AuthProp{message.MakeAuthProp("smtp", string(receivedSPF.Identity), spfIdentity.XName(c.smtputf8), true, spfIdentity.ASCIIExtra(c.smtputf8))}
props = []message.AuthProp{message.MakeAuthProp("smtp", string(receivedSPF.Identity), spfIdentity.XName(c.msgsmtputf8), true, spfIdentity.ASCIIExtra(c.msgsmtputf8))}
}
var spfComment string
if spfAuthentic {
Expand Down Expand Up @@ -2407,7 +2479,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
Comment: comment,
Props: []message.AuthProp{
// ../rfc/7489:1489
message.MakeAuthProp("header", "from", msgFrom.Domain.ASCII, true, msgFrom.Domain.ASCIIExtra(c.smtputf8)),
message.MakeAuthProp("header", "from", msgFrom.Domain.ASCII, true, msgFrom.Domain.ASCIIExtra(c.msgsmtputf8)),
},
}

Expand Down Expand Up @@ -2664,7 +2736,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
// Received-SPF header goes before Received. ../rfc/7208:2038
m.MsgPrefix = []byte(
xmox +
"Delivered-To: " + rcptAcc.rcptTo.XString(c.smtputf8) + "\r\n" + // ../rfc/9228:274
"Delivered-To: " + rcptAcc.rcptTo.XString(c.msgsmtputf8) + "\r\n" + // ../rfc/9228:274
"Return-Path: <" + c.mailFrom.String() + ">\r\n" + // ../rfc/5321:3300
rcptAuthResults.Header() +
receivedSPF.Header() +
Expand Down Expand Up @@ -2967,7 +3039,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
if len(deliverErrors) > 0 {
now := time.Now()
dsnMsg := dsn.Message{
SMTPUTF8: c.smtputf8,
SMTPUTF8: c.msgsmtputf8,
From: smtp.Path{Localpart: "postmaster", IPDomain: deliverErrors[0].rcptTo.IPDomain},
To: *c.mailFrom,
Subject: "mail delivery failure",
Expand Down
Loading