Fix #1053: Handle SMTP servers that don't support STARTTLS

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.")
 

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()

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)