Skip to content

Commit

Permalink
fix: reject obviously invalid email addresses from courier
Browse files Browse the repository at this point in the history
  • Loading branch information
alnr authored and hperl committed Nov 29, 2023
1 parent c25ddff commit 8cb9e4c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
4 changes: 4 additions & 0 deletions courier/smtp.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/tls"
"encoding/json"
"fmt"
"net/mail"
"net/textproto"
"strconv"
"time"
Expand Down Expand Up @@ -118,6 +119,9 @@ func (c *courier) QueueEmail(ctx context.Context, t EmailTemplate) (uuid.UUID, e
if err != nil {
return uuid.Nil, err
}
if _, err := mail.ParseAddress(recipient); err != nil {
return uuid.Nil, err
}

subject, err := t.EmailSubject(ctx)
if err != nil {
Expand Down
22 changes: 14 additions & 8 deletions courier/smtp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/rand"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"flag"
Expand All @@ -19,8 +20,6 @@ import (
"testing"
"time"

"crypto/x509"

"github.com/gofrs/uuid"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -55,21 +54,21 @@ func TestNewSMTP(t *testing.T) {
t.SkipNow()
}

//Should enforce StartTLS => dialer.StartTLSPolicy = gomail.MandatoryStartTLS and dialer.SSL = false
// Should enforce StartTLS => dialer.StartTLSPolicy = gomail.MandatoryStartTLS and dialer.SSL = false
smtp := setupCourier("smtp://foo:bar@my-server:1234/")
assert.Equal(t, smtp.SmtpDialer().StartTLSPolicy, gomail.MandatoryStartTLS, "StartTLS not enforced")
assert.Equal(t, smtp.SmtpDialer().SSL, false, "Implicit TLS should not be enabled")

//Should enforce TLS => dialer.SSL = true
// Should enforce TLS => dialer.SSL = true
smtp = setupCourier("smtps://foo:bar@my-server:1234/")
assert.Equal(t, smtp.SmtpDialer().SSL, true, "Implicit TLS should be enabled")

//Should allow cleartext => dialer.StartTLSPolicy = gomail.OpportunisticStartTLS and dialer.SSL = false
// Should allow cleartext => dialer.StartTLSPolicy = gomail.OpportunisticStartTLS and dialer.SSL = false
smtp = setupCourier("smtp://foo:bar@my-server:1234/?disable_starttls=true")
assert.Equal(t, smtp.SmtpDialer().StartTLSPolicy, gomail.OpportunisticStartTLS, "StartTLS is enforced")
assert.Equal(t, smtp.SmtpDialer().SSL, false, "Implicit TLS should not be enabled")

//Test cert based SMTP client auth
// Test cert based SMTP client auth
clientCert, clientKey, err := generateTestClientCert()
require.NoError(t, err)
defer os.Remove(clientCert.Name())
Expand All @@ -88,8 +87,8 @@ func TestNewSMTP(t *testing.T) {
assert.Equal(t, smtpWithCert.SmtpDialer().TLSConfig.ServerName, "my-server", "TLS config server name should match")
assert.Contains(t, smtpWithCert.SmtpDialer().TLSConfig.Certificates, clientPEM, "TLS config should contain client pem")

//error case: invalid client key
conf.Set(ctx, config.ViperKeyCourierSMTPClientKeyPath, clientCert.Name()) //mixup client key and client cert
// error case: invalid client key
conf.Set(ctx, config.ViperKeyCourierSMTPClientKeyPath, clientCert.Name()) // mixup client key and client cert
smtpWithCert = setupCourier("smtps://subdomain.my-server:1234/?server_name=my-server")
assert.Equal(t, len(smtpWithCert.SmtpDialer().TLSConfig.Certificates), 0, "TLS config certificates should be empty")
}
Expand Down Expand Up @@ -117,6 +116,13 @@ func TestQueueEmail(t *testing.T) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

_, err = c.QueueEmail(ctx, templates.NewTestStub(reg, &templates.TestStubModel{
To: "invalid-email",
Subject: "test-subject-1",
Body: "test-body-1",
}))
require.Error(t, err)

id, err := c.QueueEmail(ctx, templates.NewTestStub(reg, &templates.TestStubModel{
To: "[email protected]",
Subject: "test-subject-1",
Expand Down

0 comments on commit 8cb9e4c

Please sign in to comment.