diff --git a/grr/core/grr_response_core/config/server.py b/grr/core/grr_response_core/config/server.py index 7506a6d124..b7f88e9ae2 100644 --- a/grr/core/grr_response_core/config/server.py +++ b/grr/core/grr_response_core/config/server.py @@ -49,6 +49,10 @@ config_lib.DEFINE_bool("Worker.smtp_starttls", False, "Enable TLS for the smtp connection.") +config_lib.DEFINE_bool( + "Worker.smtp_starttls_strict", False, + "Fail if STARTTLS is enabled but not supported by the server.") + config_lib.DEFINE_string("Worker.smtp_user", None, "Username for the smtp connection.") diff --git a/grr/server/grr_response_server/email_alerts.py b/grr/server/grr_response_server/email_alerts.py index 57f354f04f..7566bd8231 100644 --- a/grr/server/grr_response_server/email_alerts.py +++ b/grr/server/grr_response_server/email_alerts.py @@ -141,17 +141,45 @@ def SendEmail( int(config.CONFIG["Worker.smtp_port"]), ) s.ehlo() + if config.CONFIG["Worker.smtp_starttls"]: - s.starttls() - s.ehlo() + if s.has_extn("starttls"): + s.starttls() + s.ehlo() + else: + smtp_server = config.CONFIG["Worker.smtp_server"] + error_msg = ( + f"STARTTLS is enabled but not supported by {smtp_server}." + ) + + if config.CONFIG.get("Worker.smtp_starttls_strict", False): + raise EmailNotSentError( + f"{error_msg} Refusing to send email without encryption. " + "Use a server that supports STARTTLS or disable strict mode." + ) + + logging.warning( + "%s Continuing without encryption. Set smtp_starttls_strict " + "to enforce TLS.", error_msg + ) + if ( config.CONFIG["Worker.smtp_user"] and config.CONFIG["Worker.smtp_password"] ): - s.login( - config.CONFIG["Worker.smtp_user"], - config.CONFIG["Worker.smtp_password"], - ) + try: + s.login( + config.CONFIG["Worker.smtp_user"], + config.CONFIG["Worker.smtp_password"], + ) + except smtplib.SMTPException as login_error: + smtp_server = config.CONFIG["Worker.smtp_server"] + if config.CONFIG["Worker.smtp_starttls"] and not s.has_extn("starttls"): + raise EmailNotSentError( + f"Authentication failed on {smtp_server}. Server may require " + "STARTTLS but doesn't support it." + ) from login_error + raise s.sendmail(from_address, to_addresses + cc_addresses, msg.as_string()) s.quit() diff --git a/grr/server/grr_response_server/email_alerts_test.py b/grr/server/grr_response_server/email_alerts_test.py index d15c732492..ac02e0c07a 100644 --- a/grr/server/grr_response_server/email_alerts_test.py +++ b/grr/server/grr_response_server/email_alerts_test.py @@ -136,6 +136,99 @@ def testSendEmail(self): "CC: testcc@%s,testcc2@%s" % (testdomain, testdomain), message ) + def testSendEmailWithStarttlsNotSupported(self): + """Email should send when STARTTLS is not supported (non-strict mode).""" + with mock.patch("smtplib.SMTP") as mock_smtp: + with test_lib.ConfigOverrider({ + "Logging.domain": "test.com", + "Worker.smtp_starttls": True, + }): + smtp_conn = mock_smtp.return_value + smtp_conn.has_extn.return_value = False + + email_alerts.EMAIL_ALERTER.SendEmail( + "testto@example.com", "me@example.com", "test", "test message" + ) + + smtp_conn.has_extn.assert_called_with("starttls") + smtp_conn.starttls.assert_not_called() + smtp_conn.sendmail.assert_called_once() + + def testSendEmailWithStarttlsSupported(self): + """STARTTLS should be used when supported.""" + with mock.patch("smtplib.SMTP") as mock_smtp: + with test_lib.ConfigOverrider({ + "Logging.domain": "test.com", + "Worker.smtp_starttls": True, + }): + smtp_conn = mock_smtp.return_value + smtp_conn.has_extn.return_value = True + + email_alerts.EMAIL_ALERTER.SendEmail( + "testto@example.com", "me@example.com", "test", "test message" + ) + + smtp_conn.starttls.assert_called_once() + smtp_conn.sendmail.assert_called_once() + + def testSendEmailAuthFailsWithoutStarttls(self): + """Auth failure should provide helpful error when STARTTLS unavailable.""" + with mock.patch("smtplib.SMTP") as mock_smtp: + with test_lib.ConfigOverrider({ + "Logging.domain": "test.com", + "Worker.smtp_starttls": True, + "Worker.smtp_user": "testuser", + "Worker.smtp_password": "testpass", + }): + smtp_conn = mock_smtp.return_value + smtp_conn.has_extn.return_value = False + smtp_conn.login.side_effect = smtplib.SMTPAuthenticationError( + 535, b"Authentication failed" + ) + + with self.assertRaises(email_alerts.EmailNotSentError) as context: + email_alerts.EMAIL_ALERTER.SendEmail( + "testto@example.com", "me@example.com", "test", "test message" + ) + + self.assertIn("STARTTLS", str(context.exception)) + + def testSendEmailWithStarttlsStrictMode(self): + """Strict mode should fail when STARTTLS is not supported.""" + with mock.patch("smtplib.SMTP") as mock_smtp: + with test_lib.ConfigOverrider({ + "Logging.domain": "test.com", + "Worker.smtp_starttls": True, + "Worker.smtp_starttls_strict": True, + }): + smtp_conn = mock_smtp.return_value + smtp_conn.has_extn.return_value = False + + with self.assertRaises(email_alerts.EmailNotSentError): + email_alerts.EMAIL_ALERTER.SendEmail( + "testto@example.com", "me@example.com", "test", "test message" + ) + + smtp_conn.sendmail.assert_not_called() + + def testSendEmailWithStarttlsNonStrictMode(self): + """Non-strict mode should continue when STARTTLS is not supported.""" + with mock.patch("smtplib.SMTP") as mock_smtp: + with test_lib.ConfigOverrider({ + "Logging.domain": "test.com", + "Worker.smtp_starttls": True, + "Worker.smtp_starttls_strict": False, + }): + smtp_conn = mock_smtp.return_value + smtp_conn.has_extn.return_value = False + + email_alerts.EMAIL_ALERTER.SendEmail( + "testto@example.com", "me@example.com", "test", "test message" + ) + + smtp_conn.sendmail.assert_called_once() + smtp_conn.starttls.assert_not_called() + def main(argv): test_lib.main(argv)