From 69a05e2d30440dafad90b4cfd228f575c9543363 Mon Sep 17 00:00:00 2001 From: Alexandre Gagneux <46828169+alexGNX@users.noreply.github.com> Date: Thu, 14 Apr 2022 20:32:41 +0200 Subject: [PATCH] feat: add certificate based authentication for smtp client (#2351) --- courier/smtp.go | 23 ++++++++++- courier/smtp_test.go | 85 ++++++++++++++++++++++++++++++++++++--- driver/config/config.go | 12 ++++++ embedx/config.schema.json | 15 ++++++- 4 files changed, 126 insertions(+), 9 deletions(-) diff --git a/courier/smtp.go b/courier/smtp.go index 75707d9c272b..2c49e1c2edbf 100644 --- a/courier/smtp.go +++ b/courier/smtp.go @@ -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) @@ -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 @@ -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 } diff --git a/courier/smtp_test.go b/courier/smtp_test.go index 626d2c45d175..842b28ee464e 100644 --- a/courier/smtp_test.go +++ b/courier/smtp_test.go @@ -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" @@ -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) @@ -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) { @@ -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 +} diff --git a/driver/config/config.go b/driver/config/config.go index 96d122be29d8..51ce7d670838 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -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" @@ -239,6 +241,8 @@ type ( } CourierConfigs interface { CourierSMTPURL() *url.URL + CourierSMTPClientCertPath() string + CourierSMTPClientKeyPath() string CourierSMTPFrom() string CourierSMTPFromName() string CourierSMTPHeaders() map[string]string @@ -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") } diff --git a/embedx/config.schema.json b/embedx/config.schema.json index c7198b52f0db..d9b3129d9ee3 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -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.",