From c0ebe3eb8cd5a3612edb76c2ea2ca5af6bbd22f6 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Tue, 10 Aug 2021 10:52:41 +1000 Subject: [PATCH] fix(notifier): use sane default connection timeout (#2273) --- config.template.yml | 35 +++++--- docs/configuration/notifier/smtp.md | 61 +++++++------ internal/commands/root.go | 2 +- internal/configuration/config.template.yml | 35 +++++--- internal/configuration/schema/notifier.go | 26 +++--- internal/configuration/validator/const.go | 1 + internal/configuration/validator/notifier.go | 4 + internal/notification/smtp_notifier.go | 90 +++++++++----------- internal/notification/smtp_notifier_test.go | 6 +- 9 files changed, 144 insertions(+), 116 deletions(-) diff --git a/config.template.yml b/config.template.yml index 3b8529c8a..b349511fe 100644 --- a/config.template.yml +++ b/config.template.yml @@ -541,20 +541,39 @@ notifier: ## - validate the SMTP server x509 certificate during the TLS handshake against the hosts trusted certificates ## (configure in tls section) smtp: - username: test - ## Password can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html - password: password + ## The SMTP host to connect to. host: 127.0.0.1 + + ## The port to connect to the SMTP host on. port: 1025 + + ## The connection timeout. + timeout: 5s + + ## The username used for SMTP authentication. + username: test + + ## The password used for SMTP authentication. + ## Can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html + password: password + + ## The address to send the email FROM. sender: admin@example.com + ## HELO/EHLO Identifier. Some SMTP Servers may reject the default of localhost. identifier: localhost + ## Subject configuration of the emails sent. {title} is replaced by the text from the notifier. subject: "[Authelia] {title}" + ## This address is used during the startup check to verify the email configuration is correct. ## It's not important what it is except if your email server only allows local delivery. startup_check_address: test@authelia.com + + ## By default we require some form of TLS. This disables this check though is not advised. disable_require_tls: false + + ## Disables sending HTML formatted emails. disable_html_emails: false tls: @@ -569,16 +588,6 @@ notifier: ## Minimum TLS version for either StartTLS or SMTPS. minimum_version: TLS1.2 - ## Sending an email using a Gmail account is as simple as the next section. - ## You need to create an app password by following: https://support.google.com/accounts/answer/185833?hl=en - # smtp: - # username: myaccount@gmail.com - # ## Password can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html - # password: yourapppassword - # sender: admin@example.com - # host: smtp.gmail.com - # port: 587 - ## ## Identity Providers ## diff --git a/docs/configuration/notifier/smtp.md b/docs/configuration/notifier/smtp.md index f9861d89e..421479e74 100644 --- a/docs/configuration/notifier/smtp.md +++ b/docs/configuration/notifier/smtp.md @@ -16,10 +16,11 @@ It can be configured as described below. notifier: disable_startup_check: false smtp: - username: test - password: password host: 127.0.0.1 port: 1025 + timeout: 5s + username: test + password: password sender: admin@example.com identifier: localhost subject: "[Authelia] {title}" @@ -34,27 +35,6 @@ notifier: ## Options -### username -
-type: string -{: .label .label-config .label-purple } -required: no -{: .label .label-config .label-green } -
- -The username sent for authentication with the SMTP server. Paired with the password. - -### password -
-type: string -{: .label .label-config .label-purple } -required: no -{: .label .label-config .label-green } -
- -The password sent for authentication with the SMTP server. Paired with the username. Can also be defined using a -[secret](../secrets.md) which is the recommended for containerized deployments. - ### host
type: integer @@ -82,6 +62,39 @@ required: yes The port the SMTP service is listening on. +### timeout +
+type: duration +{: .label .label-config .label-purple } +default: 5s +{: .label .label-config .label-blue } +required: no +{: .label .label-config .label-green } +
+ +The SMTP connection timeout. + +### username +
+type: string +{: .label .label-config .label-purple } +required: no +{: .label .label-config .label-green } +
+ +The username sent for authentication with the SMTP server. Paired with the password. + +### password +
+type: string +{: .label .label-config .label-purple } +required: no +{: .label .label-config .label-green } +
+ +The password sent for authentication with the SMTP server. Paired with the username. Can also be defined using a +[secret](../secrets.md) which is the recommended for containerized deployments. + ### sender
type: string @@ -93,7 +106,7 @@ required: no The address sent in the FROM header for the email. Basically who the email appears to come from. It should be noted that some SMTP servers require the username provided to have access to send from the specific address listed here. -### identifer +### identifier
type: string {: .label .label-config .label-purple } diff --git a/internal/commands/root.go b/internal/commands/root.go index 48fcb845d..8565233a5 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -120,7 +120,7 @@ func getProviders(config *schema.Configuration) (providers middlewares.Providers switch { case config.Notifier.SMTP != nil: - notifier = notification.NewSMTPNotifier(*config.Notifier.SMTP, autheliaCertPool) + notifier = notification.NewSMTPNotifier(config.Notifier.SMTP, autheliaCertPool) case config.Notifier.FileSystem != nil: notifier = notification.NewFileNotifier(*config.Notifier.FileSystem) default: diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index 3b8529c8a..b349511fe 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -541,20 +541,39 @@ notifier: ## - validate the SMTP server x509 certificate during the TLS handshake against the hosts trusted certificates ## (configure in tls section) smtp: - username: test - ## Password can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html - password: password + ## The SMTP host to connect to. host: 127.0.0.1 + + ## The port to connect to the SMTP host on. port: 1025 + + ## The connection timeout. + timeout: 5s + + ## The username used for SMTP authentication. + username: test + + ## The password used for SMTP authentication. + ## Can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html + password: password + + ## The address to send the email FROM. sender: admin@example.com + ## HELO/EHLO Identifier. Some SMTP Servers may reject the default of localhost. identifier: localhost + ## Subject configuration of the emails sent. {title} is replaced by the text from the notifier. subject: "[Authelia] {title}" + ## This address is used during the startup check to verify the email configuration is correct. ## It's not important what it is except if your email server only allows local delivery. startup_check_address: test@authelia.com + + ## By default we require some form of TLS. This disables this check though is not advised. disable_require_tls: false + + ## Disables sending HTML formatted emails. disable_html_emails: false tls: @@ -569,16 +588,6 @@ notifier: ## Minimum TLS version for either StartTLS or SMTPS. minimum_version: TLS1.2 - ## Sending an email using a Gmail account is as simple as the next section. - ## You need to create an app password by following: https://support.google.com/accounts/answer/185833?hl=en - # smtp: - # username: myaccount@gmail.com - # ## Password can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html - # password: yourapppassword - # sender: admin@example.com - # host: smtp.gmail.com - # port: 587 - ## ## Identity Providers ## diff --git a/internal/configuration/schema/notifier.go b/internal/configuration/schema/notifier.go index 9e582c27e..15aae34c3 100644 --- a/internal/configuration/schema/notifier.go +++ b/internal/configuration/schema/notifier.go @@ -1,5 +1,7 @@ package schema +import "time" + // FileSystemNotifierConfiguration represents the configuration of the notifier writing emails in a file. type FileSystemNotifierConfiguration struct { Filename string `koanf:"filename"` @@ -7,17 +9,18 @@ type FileSystemNotifierConfiguration struct { // SMTPNotifierConfiguration represents the configuration of the SMTP server to send emails with. type SMTPNotifierConfiguration struct { - Host string `koanf:"host"` - Port int `koanf:"port"` - Username string `koanf:"username"` - Password string `koanf:"password"` - Identifier string `koanf:"identifier"` - Sender string `koanf:"sender"` - Subject string `koanf:"subject"` - StartupCheckAddress string `koanf:"startup_check_address"` - DisableRequireTLS bool `koanf:"disable_require_tls"` - DisableHTMLEmails bool `koanf:"disable_html_emails"` - TLS *TLSConfig `koanf:"tls"` + Host string `koanf:"host"` + Port int `koanf:"port"` + Timeout time.Duration `koanf:"timeout"` + Username string `koanf:"username"` + Password string `koanf:"password"` + Identifier string `koanf:"identifier"` + Sender string `koanf:"sender"` + Subject string `koanf:"subject"` + StartupCheckAddress string `koanf:"startup_check_address"` + DisableRequireTLS bool `koanf:"disable_require_tls"` + DisableHTMLEmails bool `koanf:"disable_html_emails"` + TLS *TLSConfig `koanf:"tls"` } // NotifierConfiguration represents the configuration of the notifier to use when sending notifications to users. @@ -29,6 +32,7 @@ type NotifierConfiguration struct { // DefaultSMTPNotifierConfiguration represents default configuration parameters for the SMTP notifier. var DefaultSMTPNotifierConfiguration = SMTPNotifierConfiguration{ + Timeout: time.Second * 5, Subject: "[Authelia] {title}", Identifier: "localhost", TLS: &TLSConfig{ diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 95c7d2c7d..e238b7f5b 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -232,6 +232,7 @@ var ValidKeys = []string{ // SMTP Notifier Keys. "notifier.smtp.host", "notifier.smtp.port", + "notifier.smtp.timeout", "notifier.smtp.username", "notifier.smtp.password", "notifier.smtp.identifier", diff --git a/internal/configuration/validator/notifier.go b/internal/configuration/validator/notifier.go index 4ac7fd387..f7e745872 100644 --- a/internal/configuration/validator/notifier.go +++ b/internal/configuration/validator/notifier.go @@ -42,6 +42,10 @@ func validateSMTPNotifier(configuration *schema.SMTPNotifierConfiguration, valid validator.Push(fmt.Errorf(errFmtNotifierSMTPNotConfigured, "port")) } + if configuration.Timeout == 0 { + configuration.Timeout = schema.DefaultSMTPNotifierConfiguration.Timeout + } + if configuration.Sender == "" { validator.Push(fmt.Errorf(errFmtNotifierSMTPNotConfigured, "sender")) } diff --git a/internal/notification/smtp_notifier.go b/internal/notification/smtp_notifier.go index d2d149ce7..38a93f3a3 100644 --- a/internal/notification/smtp_notifier.go +++ b/internal/notification/smtp_notifier.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "errors" "fmt" + "net" "net/smtp" "strings" "time" @@ -16,34 +17,16 @@ import ( // SMTPNotifier a notifier to send emails to SMTP servers. type SMTPNotifier struct { - username string - password string - sender string - identifier string - host string - port int - disableRequireTLS bool - address string - subject string - startupCheckAddress string - client *smtp.Client - tlsConfig *tls.Config + configuration *schema.SMTPNotifierConfiguration + client *smtp.Client + tlsConfig *tls.Config } // NewSMTPNotifier creates a SMTPNotifier using the notifier configuration. -func NewSMTPNotifier(configuration schema.SMTPNotifierConfiguration, certPool *x509.CertPool) *SMTPNotifier { +func NewSMTPNotifier(configuration *schema.SMTPNotifierConfiguration, certPool *x509.CertPool) *SMTPNotifier { notifier := &SMTPNotifier{ - username: configuration.Username, - password: configuration.Password, - sender: configuration.Sender, - identifier: configuration.Identifier, - host: configuration.Host, - port: configuration.Port, - disableRequireTLS: configuration.DisableRequireTLS, - address: fmt.Sprintf("%s:%d", configuration.Host, configuration.Port), - subject: configuration.Subject, - startupCheckAddress: configuration.StartupCheckAddress, - tlsConfig: utils.NewTLSConfig(configuration.TLS, tls.VersionTLS12, certPool), + configuration: configuration, + tlsConfig: utils.NewTLSConfig(configuration.TLS, tls.VersionTLS12, certPool), } return notifier @@ -68,7 +51,7 @@ func (n *SMTPNotifier) startTLS() error { logger.Debug("Notifier SMTP STARTTLS completed without error") default: - switch n.disableRequireTLS { + switch n.configuration.DisableRequireTLS { case true: logger.Warn("Notifier SMTP server does not support STARTTLS and SMTP configuration is set to disable the TLS requirement (only useful for unauthenticated emails over plain text)") default: @@ -83,7 +66,7 @@ func (n *SMTPNotifier) startTLS() error { func (n *SMTPNotifier) auth() error { logger := logging.Logger() // Attempt AUTH if password is specified only. - if n.password != "" { + if n.configuration.Password != "" { _, ok := n.client.TLSConnectionState() if !ok { return errors.New("Notifier SMTP client does not support authentication over plain text and the connection is currently plain text") @@ -99,11 +82,11 @@ func (n *SMTPNotifier) auth() error { // Adaptively select the AUTH mechanism to use based on what the server advertised. if utils.IsStringInSlice("PLAIN", mechanisms) { - auth = smtp.PlainAuth("", n.username, n.password, n.host) + auth = smtp.PlainAuth("", n.configuration.Username, n.configuration.Password, n.configuration.Host) logger.Debug("Notifier SMTP client attempting AUTH PLAIN with server") } else if utils.IsStringInSlice("LOGIN", mechanisms) { - auth = newLoginAuth(n.username, n.password, n.host) + auth = newLoginAuth(n.configuration.Username, n.configuration.Password, n.configuration.Host) logger.Debug("Notifier SMTP client attempting AUTH LOGIN with server") } @@ -135,7 +118,7 @@ func (n *SMTPNotifier) compose(recipient, subject, body, htmlBody string) error logger := logging.Logger() logger.Debugf("Notifier SMTP client attempting to send email body to %s", recipient) - if !n.disableRequireTLS { + if !n.configuration.DisableRequireTLS { _, ok := n.client.TLSConnectionState() if !ok { return errors.New("Notifier SMTP client can't send an email over plain text connection") @@ -153,7 +136,7 @@ func (n *SMTPNotifier) compose(recipient, subject, body, htmlBody string) error now := time.Now() msg := "Date:" + now.Format(rfc5322DateTimeLayout) + "\n" + - "From: " + n.sender + "\n" + + "From: " + n.configuration.Sender + "\n" + "To: " + recipient + "\n" + "Subject: " + subject + "\n" + "MIME-version: 1.0\n" + @@ -188,33 +171,40 @@ func (n *SMTPNotifier) compose(recipient, subject, body, htmlBody string) error } // Dial the SMTP server with the SMTPNotifier config. -func (n *SMTPNotifier) dial() error { +func (n *SMTPNotifier) dial() (err error) { logger := logging.Logger() - logger.Debugf("Notifier SMTP client attempting connection to %s", n.address) + logger.Debugf("Notifier SMTP client attempting connection to %s:%d", n.configuration.Host, n.configuration.Port) - if n.port == 465 { + var ( + client *smtp.Client + conn net.Conn + ) + + dialer := &net.Dialer{ + Timeout: n.configuration.Timeout, + } + + if n.configuration.Port == 465 { logger.Infof("Notifier SMTP client using submissions port 465. Make sure the mail server you are connecting to is configured for submissions and not SMTPS.") - conn, err := tls.Dial("tcp", n.address, n.tlsConfig) + conn, err = tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("%s:%d", n.configuration.Host, n.configuration.Port), n.tlsConfig) if err != nil { return err } - - client, err := smtp.NewClient(conn, n.host) - if err != nil { - return err - } - - n.client = client } else { - client, err := smtp.Dial(n.address) + conn, err = dialer.Dial("tcp", fmt.Sprintf("%s:%d", n.configuration.Host, n.configuration.Port)) if err != nil { return err } - - n.client = client } + client, err = smtp.NewClient(conn, n.configuration.Host) + if err != nil { + return err + } + + n.client = client + logger.Debug("Notifier SMTP client connected successfully") return nil @@ -238,7 +228,7 @@ func (n *SMTPNotifier) StartupCheck() (bool, error) { defer n.cleanup() - if err := n.client.Hello(n.identifier); err != nil { + if err := n.client.Hello(n.configuration.Identifier); err != nil { return false, err } @@ -250,11 +240,11 @@ func (n *SMTPNotifier) StartupCheck() (bool, error) { return false, err } - if err := n.client.Mail(n.sender); err != nil { + if err := n.client.Mail(n.configuration.Sender); err != nil { return false, err } - if err := n.client.Rcpt(n.startupCheckAddress); err != nil { + if err := n.client.Rcpt(n.configuration.StartupCheckAddress); err != nil { return false, err } @@ -268,7 +258,7 @@ func (n *SMTPNotifier) StartupCheck() (bool, error) { // Send is used to send an email to a recipient. func (n *SMTPNotifier) Send(recipient, title, body, htmlBody string) error { logger := logging.Logger() - subject := strings.ReplaceAll(n.subject, "{title}", title) + subject := strings.ReplaceAll(n.configuration.Subject, "{title}", title) if err := n.dial(); err != nil { return err @@ -277,7 +267,7 @@ func (n *SMTPNotifier) Send(recipient, title, body, htmlBody string) error { // Always execute QUIT at the end once we're connected. defer n.cleanup() - if err := n.client.Hello(n.identifier); err != nil { + if err := n.client.Hello(n.configuration.Identifier); err != nil { return err } @@ -291,7 +281,7 @@ func (n *SMTPNotifier) Send(recipient, title, body, htmlBody string) error { } // Set the sender and recipient first. - if err := n.client.Mail(n.sender); err != nil { + if err := n.client.Mail(n.configuration.Sender); err != nil { logger.Debugf("Notifier SMTP failed while sending MAIL FROM (using sender) with error: %s", err) return err } diff --git a/internal/notification/smtp_notifier_test.go b/internal/notification/smtp_notifier_test.go index e9c2126ac..2e73a67de 100644 --- a/internal/notification/smtp_notifier_test.go +++ b/internal/notification/smtp_notifier_test.go @@ -25,12 +25,11 @@ func TestShouldConfigureSMTPNotifierWithTLS11AndDefaultHostname(t *testing.T) { sv := schema.NewStructValidator() validator.ValidateNotifier(config, sv) - notifier := NewSMTPNotifier(*config.SMTP, nil) + notifier := NewSMTPNotifier(config.SMTP, nil) assert.Equal(t, "smtp.example.com", notifier.tlsConfig.ServerName) assert.Equal(t, uint16(tls.VersionTLS11), notifier.tlsConfig.MinVersion) assert.False(t, notifier.tlsConfig.InsecureSkipVerify) - assert.Equal(t, "smtp.example.com:25", notifier.address) } func TestShouldConfigureSMTPNotifierWithServerNameOverrideAndDefaultTLS12(t *testing.T) { @@ -48,10 +47,9 @@ func TestShouldConfigureSMTPNotifierWithServerNameOverrideAndDefaultTLS12(t *tes sv := schema.NewStructValidator() validator.ValidateNotifier(config, sv) - notifier := NewSMTPNotifier(*config.SMTP, nil) + notifier := NewSMTPNotifier(config.SMTP, nil) assert.Equal(t, "smtp.golang.org", notifier.tlsConfig.ServerName) assert.Equal(t, uint16(tls.VersionTLS12), notifier.tlsConfig.MinVersion) assert.False(t, notifier.tlsConfig.InsecureSkipVerify) - assert.Equal(t, "smtp.example.com:25", notifier.address) }