Skip to content

Commit

Permalink
feat: add certificate based authentication for smtp client (#2351)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexGNX committed Apr 14, 2022
1 parent 0a381fa commit 7200037
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 9 deletions.
23 changes: 21 additions & 2 deletions courier/smtp.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ type smtpClient struct {

func newSMTP(ctx context.Context, deps Dependencies) *smtpClient {
uri := deps.CourierConfig(ctx).CourierSMTPURL()
var tlsCertificates []tls.Certificate
clientCertPath := deps.CourierConfig(ctx).CourierSMTPClientCertPath()
clientKeyPath := deps.CourierConfig(ctx).CourierSMTPClientKeyPath()

if clientCertPath != "" && clientKeyPath != "" {
clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath)
if err == nil {
tlsCertificates = append(tlsCertificates, clientCert)
} else {
deps.Logger().
WithError(err).
Error("Unable to load tls certificate and private key for smtp client.")
}
}

password, _ := uri.User.Password()
port, _ := strconv.ParseInt(uri.Port(), 10, 0)
Expand All @@ -44,6 +58,11 @@ func newSMTP(ctx context.Context, deps Dependencies) *smtpClient {

sslSkipVerify, _ := strconv.ParseBool(uri.Query().Get("skip_ssl_verify"))

serverName := uri.Query().Get("server_name")
if serverName == "" {
serverName = uri.Hostname()
}

// SMTP schemes
// smtp: smtp clear text (with uri parameter) or with StartTLS (enforced by default)
// smtps: smtp with implicit TLS (recommended way in 2021 to avoid StartTLS downgrade attacks
Expand All @@ -54,13 +73,13 @@ func newSMTP(ctx context.Context, deps Dependencies) *smtpClient {
skipStartTLS, _ := strconv.ParseBool(uri.Query().Get("disable_starttls"))
if !skipStartTLS {
// #nosec G402 This is ok (and required!) because it is configurable and disabled by default.
dialer.TLSConfig = &tls.Config{InsecureSkipVerify: sslSkipVerify, ServerName: uri.Hostname()}
dialer.TLSConfig = &tls.Config{InsecureSkipVerify: sslSkipVerify, Certificates: tlsCertificates, ServerName: serverName}
// Enforcing StartTLS
dialer.StartTLSPolicy = gomail.MandatoryStartTLS
}
case "smtps":
// #nosec G402 This is ok (and required!) because it is configurable and disabled by default.
dialer.TLSConfig = &tls.Config{InsecureSkipVerify: sslSkipVerify, ServerName: uri.Hostname()}
dialer.TLSConfig = &tls.Config{InsecureSkipVerify: sslSkipVerify, Certificates: tlsCertificates, ServerName: serverName}
dialer.SSL = true
}

Expand Down
85 changes: 79 additions & 6 deletions courier/smtp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@ package courier_test

import (
"context"
"crypto/rand"
"crypto/rsa"
"crypto/tls"
"crypto/x509/pkix"
"encoding/pem"
"flag"
"fmt"
"io/ioutil"
"math/big"
"net/http"
"os"
"testing"
"time"

"crypto/x509"

"github.com/gofrs/uuid"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand All @@ -25,11 +35,10 @@ import (

func TestNewSMTP(t *testing.T) {
ctx := context.Background()
conf, reg := internal.NewFastRegistryWithMocks(t)

setupConfig := func(stringURL string) courier.Courier {
conf, reg := internal.NewFastRegistryWithMocks(t)
setupCourier := func(stringURL string) courier.Courier {
conf.MustSet(config.ViperKeyCourierSMTPURL, stringURL)

t.Logf("SMTP URL: %s", conf.CourierSMTPURL().String())

return courier.NewCourier(ctx, reg)
Expand All @@ -40,18 +49,42 @@ func TestNewSMTP(t *testing.T) {
}

//Should enforce StartTLS => dialer.StartTLSPolicy = gomail.MandatoryStartTLS and dialer.SSL = false
smtp := setupConfig("smtp://foo:bar@my-server:1234/")
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
smtp = setupConfig("smtps://foo:bar@my-server:1234/")
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
smtp = setupConfig("smtp://foo:bar@my-server:1234/?disable_starttls=true")
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
clientCert, clientKey, err := generateTestClientCert()
require.NoError(t, err)
defer os.Remove(clientCert.Name())
defer os.Remove(clientKey.Name())

conf.Set(config.ViperKeyCourierSMTPClientCertPath, clientCert.Name())
conf.Set(config.ViperKeyCourierSMTPClientKeyPath, clientKey.Name())

clientPEM, err := tls.LoadX509KeyPair(clientCert.Name(), clientKey.Name())
require.NoError(t, err)

smtpWithCert := setupCourier("smtps://subdomain.my-server:1234/?server_name=my-server")
assert.Equal(t, smtpWithCert.SmtpDialer().SSL, true, "Implicit TLS should be enabled")
assert.Equal(t, smtpWithCert.SmtpDialer().Host, "subdomain.my-server", "SMTP Dialer host should match")
assert.Equal(t, smtpWithCert.SmtpDialer().TLSConfig.ServerName, "my-server", "TLS config server name should match")
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(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")
}

func TestQueueEmail(t *testing.T) {
Expand Down Expand Up @@ -153,3 +186,43 @@ func TestQueueEmail(t *testing.T) {
assert.Contains(t, string(body), `"test-stub-header1":["foo"]`)
assert.Contains(t, string(body), `"test-stub-header2":["bar"]`)
}

func generateTestClientCert() (clientCert *os.File, clientKey *os.File, err error) {
var hostName *string = flag.String("host", "127.0.0.1", "Hostname to certify")
priv, err := rsa.GenerateKey(rand.Reader, 1024)
if err != nil {
return nil, nil, err
}
now := time.Now()
certTemplate := x509.Certificate{
SerialNumber: big.NewInt(1234),
Subject: pkix.Name{
CommonName: *hostName,
Organization: []string{"myorg"},
},
NotBefore: now.Add(-300 * time.Second),
NotAfter: now.Add(24 * time.Hour),
SubjectKeyId: []byte{1, 2, 3, 4},
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
}
cert, err := x509.CreateCertificate(rand.Reader, &certTemplate, &certTemplate, &priv.PublicKey, priv)
if err != nil {
return nil, nil, err
}
clientCert, err = ioutil.TempFile("./test", "testCert")
if err != nil {
return nil, nil, err
}

pem.Encode(clientCert, &pem.Block{Type: "CERTIFICATE", Bytes: cert})
clientCert.Close()

clientKey, err = ioutil.TempFile("./test", "testKey")
if err != nil {
return nil, nil, err
}
pem.Encode(clientKey, &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(priv)})
clientKey.Close()

return clientCert, clientKey, nil
}
12 changes: 12 additions & 0 deletions driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ const (
UnknownVersion = "unknown version"
ViperKeyDSN = "dsn"
ViperKeyCourierSMTPURL = "courier.smtp.connection_uri"
ViperKeyCourierSMTPClientCertPath = "courier.smtp.client_cert_path"
ViperKeyCourierSMTPClientKeyPath = "courier.smtp.client_key_path"
ViperKeyCourierTemplatesPath = "courier.template_override_path"
ViperKeyCourierTemplatesRecoveryInvalidEmail = "courier.templates.recovery.invalid.email"
ViperKeyCourierTemplatesRecoveryValidEmail = "courier.templates.recovery.valid.email"
Expand Down Expand Up @@ -239,6 +241,8 @@ type (
}
CourierConfigs interface {
CourierSMTPURL() *url.URL
CourierSMTPClientCertPath() string
CourierSMTPClientKeyPath() string
CourierSMTPFrom() string
CourierSMTPFromName() string
CourierSMTPHeaders() map[string]string
Expand Down Expand Up @@ -871,6 +875,14 @@ func (p *Config) SelfServiceFlowLogoutRedirectURL() *url.URL {
return p.p.RequestURIF(ViperKeySelfServiceLogoutBrowserDefaultReturnTo, p.SelfServiceBrowserDefaultReturnTo())
}

func (p *Config) CourierSMTPClientCertPath() string {
return p.p.StringF(ViperKeyCourierSMTPClientCertPath, "")
}

func (p *Config) CourierSMTPClientKeyPath() string {
return p.p.StringF(ViperKeyCourierSMTPClientKeyPath, "")
}

func (p *Config) CourierSMTPFrom() string {
return p.p.StringF(ViperKeyCourierSMTPFrom, "noreply@kratos.ory.sh")
}
Expand Down
15 changes: 14 additions & 1 deletion embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1494,11 +1494,24 @@
"smtp://foo:bar@my-mailserver:1234/ (Explicit StartTLS with certificate trust verification)",
"smtp://foo:bar@my-mailserver:1234/?skip_ssl_verify=true (NOT RECOMMENDED: Explicit StartTLS without certificate trust verification)",
"smtps://foo:bar@my-mailserver:1234/ (Implicit TLS with certificate trust verification)",
"smtps://foo:bar@my-mailserver:1234/?skip_ssl_verify=true (NOT RECOMMENDED: Implicit TLS without certificate trust verification)"
"smtps://foo:bar@my-mailserver:1234/?skip_ssl_verify=true (NOT RECOMMENDED: Implicit TLS without certificate trust verification)",
"smtps://subdomain.my-mailserver:1234/?server_name=my-mailserver (allows TLS to work if the server is hosted on a sudomain that uses a non-wildcard domain certificate)"
],
"type": "string",
"pattern": "^smtps?:\\/\\/.*"
},
"client_cert_path": {
"title": "SMTP Client certificate path",
"description": "Path of the client X.509 certificate, in case of certificate based client authentication to the SMTP server.",
"type": "string",
"default": ""
},
"client_key_path": {
"title": "SMTP Client private key path",
"description": "Path of the client certificate private key, in case of certificate based client authentication to the SMTP server",
"type": "string",
"default": ""
},
"from_address": {
"title": "SMTP Sender Address",
"description": "The recipient of an email will see this as the sender address.",
Expand Down

0 comments on commit 7200037

Please sign in to comment.