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.

Sign up for GitHub

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

fix: reject obviously invalid email addresses from courier #3646

Merged
merged 1 commit into from
Nov 29, 2023
Merged

fix: reject obviously invalid email addresses from courier #3646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions courier/smtp.go
View file
Open in desktop
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
View file
Open in desktop
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
Loading