[#246] Make STARTTLS mandatory: support starttls.required in external.email

openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc

@@ -64,7 +64,8 @@ $ cp samples/misc/external.email.json conf/
     "writetimeout" : 300000,
     "connectiontimeout" : 300000,
     "starttls" : {
-        "enable" : true
+        "enable" : true,
+        "required" : true
     },
     "ssl" : {
         "enable" : false
@@ -122,7 +123,9 @@ If `"enable" : false`, you can leave the entries for `"username"` and `"password
 
 
 `starttls`::
-If `"enable" : true`, enables the use of the STARTTLS command (if supported by the server) to switch the connection to a TLS-protected connection before issuing any login commands. If the server does not support STARTTLS, the connection continues without the use of TLS.
+If `"enable" : true`, enables the use of the STARTTLS command (if supported by the server) to switch the connection to a TLS-protected connection before issuing any login commands. If the server does not support STARTTLS, the connection continues without the use of TLS, unless `required` is set.
++
+Set `"required" : true` to refuse to send when the server does not offer STARTTLS; `required` implies `enable`. With the default, `false`, an attacker on the network path can remove STARTTLS from the server's reply and so receive the SMTP credentials and the message in clear. Leave it `false` only for a server that does not support STARTTLS.
 +
 The SMTP server certificate is validated against the JVM trust store and must be issued for the configured `host`. On Java 17 and later the host name is matched only against the certificate's DNS subject alternative names (or, if it has none, its CN), so `host` must be a DNS name the certificate carries; a relay addressed by IP address is rejected even if its certificate has an IP address subject alternative name. Two optional settings relax that:
 +

openidm-external-email/pom.xml

@@ -108,6 +108,13 @@
             <artifactId>mockito-all</artifactId>
             <scope>test</scope>
         </dependency>
+        <!-- javax.activation-api (via jaxb-api) has no implementation classes; sending a message in a test needs them -->
+        <dependency>
+            <groupId>com.sun.activation</groupId>
+            <artifactId>javax.activation</artifactId>
+            <version>1.2.0</version>
+            <scope>test</scope>
+        </dependency>
 
         <dependency>
             <groupId>org.openidentityplatform.commons</groupId>

openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java

@@ -71,6 +71,11 @@ public class EmailClient {
     public static final String CONFIG_MAIL_SMTP_AUTH_USERNAME = "username";
     public static final String CONFIG_MAIL_SMTP_STARTTLS = "starttls";
     public static final String CONFIG_MAIL_SMTP_STARTTLS_ENABLE = "enable";
+    /**
+     * Fail instead of sending in clear when the server does not offer STARTTLS. Implies
+     * {@code enable}. Off by default: STARTTLS is then used only if the server offers it.
+     */
+    public static final String CONFIG_MAIL_SMTP_STARTTLS_REQUIRED = "required";
     /** Opt-in: accept any server certificate over STARTTLS. Never use outside development. */
     public static final String CONFIG_MAIL_SMTP_STARTTLS_TRUST_ALL = "trustAll";
     /**
@@ -98,9 +103,15 @@ public EmailClient(JsonValue config) throws RuntimeException {
         }
 
         JsonValue starttlsConfig = config.get(CONFIG_MAIL_SMTP_STARTTLS);
-        boolean startTLS = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_ENABLE).defaultTo(false).asBoolean();
+        boolean startTLSRequired =
+                starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_REQUIRED).defaultTo(false).asBoolean();
+        // JavaMail issues STARTTLS for "required" alone, so it implies "enable" and the trust settings below
+        boolean startTLS = startTLSRequired
+                || starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_ENABLE).defaultTo(false).asBoolean();
         if (startTLS) {
             props.put("mail.smtp.starttls.enable", String.valueOf(startTLS));
+            // when true, fail instead of continuing in clear if the server does not offer STARTTLS
+            props.put("mail.smtp.starttls.required", String.valueOf(startTLSRequired));
             // without this JavaMail 1.4.7 enables only TLSv1 for STARTTLS, which current JDKs disable
             props.put("mail.smtp.ssl.protocols", defaultTlsProtocols());
             configureStartTlsTrust(starttlsConfig);

openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java

@@ -16,28 +16,42 @@
 package org.forgerock.openidm.external.email.impl;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 import static org.forgerock.json.JsonValue.array;
 import static org.forgerock.json.JsonValue.field;
 import static org.forgerock.json.JsonValue.json;
 import static org.forgerock.json.JsonValue.object;
 
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
 import java.lang.reflect.Field;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
 import java.util.Properties;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import javax.mail.Session;
 import javax.net.ssl.SSLContext;
 
 import com.sun.mail.util.MailSSLSocketFactory;
 import org.forgerock.json.JsonValue;
+import org.forgerock.json.resource.BadRequestException;
 import org.testng.annotations.Test;
 
 /**
- * Tests for the STARTTLS trust settings of {@link EmailClient}.
+ * Tests for the STARTTLS settings of {@link EmailClient}.
  */
 public class EmailClientTest {
 
     private static final String SOCKET_FACTORY = "mail.smtp.ssl.socketFactory";
     private static final String CHECK_SERVER_IDENTITY = "mail.smtp.ssl.checkserveridentity";
+    private static final String STARTTLS_REQUIRED = "mail.smtp.starttls.required";
 
     @Test
     public void startTlsValidatesTheServerCertificateByDefault() throws Exception {
@@ -105,6 +119,61 @@ public void socketFactoryComesFromTheJavaMailInUse() {
                 .isEqualTo(Session.class.getProtectionDomain().getCodeSource().getLocation());
     }
 
+    @Test
+    public void startTlsIsOpportunisticByDefault() throws Exception {
+        Properties props = sessionProperties(json(object(
+                field("host", "smtp.example.com"),
+                field("starttls", object(field("enable", true))))));
+
+        assertThat(props.get(STARTTLS_REQUIRED)).isEqualTo("false");
+    }
+
+    @Test
+    public void startTlsRequiredRejectsServersWithoutStartTls() throws Exception {
+        Properties props = sessionProperties(json(object(
+                field("host", "smtp.example.com"),
+                field("starttls", object(field("enable", true), field("required", true))))));
+
+        assertThat(props.get(STARTTLS_REQUIRED)).isEqualTo("true");
+        assertThat(props.get(CHECK_SERVER_IDENTITY)).isEqualTo("true");
+    }
+
+    @Test
+    public void startTlsRequiredImpliesStartTls() throws Exception {
+        Properties props = sessionProperties(json(object(
+                field("host", "smtp.example.com"),
+                field("starttls", object(field("enable", false), field("required", true))))));
+
+        // JavaMail issues STARTTLS for required alone, so the trust settings must apply as well
+        assertThat(props.get("mail.smtp.starttls.enable")).isEqualTo("true");
+        assertThat(props.get(STARTTLS_REQUIRED)).isEqualTo("true");
+        assertThat(props.get(CHECK_SERVER_IDENTITY)).isEqualTo("true");
+    }
+
+    @Test
+    public void startTlsRequiredStopsBeforeMailWhenTheServerDoesNotOfferIt() throws Exception {
+        try (StartTlsStrippingServer server = new StartTlsStrippingServer()) {
+            EmailClient client = new EmailClient(server.config(true));
+            try {
+                client.send(message());
+                fail("sent over a connection without STARTTLS");
+            } catch (BadRequestException e) {
+                assertThat(e.getCause()).hasMessageContaining("STARTTLS is required");
+            }
+            assertThat(server.commands()).as("connected and read the EHLO reply").anyMatch(c -> c.startsWith("EHLO"));
+            assertThat(server.commands()).noneMatch(c -> c.startsWith("MAIL FROM"));
+        }
+    }
+
+    @Test
+    public void startTlsWithoutRequiredSendsInClearWhenTheServerDoesNotOfferIt() throws Exception {
+        try (StartTlsStrippingServer server = new StartTlsStrippingServer()) {
+            new EmailClient(server.config(false)).send(message());
+
+            assertThat(server.commands()).anyMatch(c -> c.startsWith("MAIL FROM"));
+        }
+    }
+
     @Test
     public void trustSettingsApplyOnlyWithStartTls() throws Exception {
         Properties props = sessionProperties(json(object(
@@ -116,6 +185,83 @@ public void trustSettingsApplyOnlyWithStartTls() throws Exception {
         assertThat(props.get(CHECK_SERVER_IDENTITY)).isNull();
     }
 
+    private static JsonValue message() {
+        return json(object(
+                field("from", "idm@example.com"),
+                field("to", "user@example.com"),
+                field("subject", "test"),
+                field("body", "test")));
+    }
+
+    /**
+     * An SMTP server whose EHLO reply does not offer STARTTLS, as seen by a client whose
+     * connection is tampered with on path; it accepts every message in clear.
+     */
+    private static final class StartTlsStrippingServer implements AutoCloseable {
+
+        private final ServerSocket serverSocket;
+        private final List<String> commands = new CopyOnWriteArrayList<>();
+        private final Thread thread;
+
+        StartTlsStrippingServer() throws IOException {
+            serverSocket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress());
+            thread = new Thread(this::serve, "fake-smtp");
+            thread.setDaemon(true);
+            thread.start();
+        }
+
+        JsonValue config(boolean required) {
+            return json(object(
+                    field("host", serverSocket.getInetAddress().getHostAddress()),
+                    field("port", String.valueOf(serverSocket.getLocalPort())),
+                    field("starttls", object(field("enable", true), field("required", required)))));
+        }
+
+        List<String> commands() throws InterruptedException {
+            thread.join(10_000);
+            return commands;
+        }
+
+        private void serve() {
+            try (Socket socket = serverSocket.accept();
+                 BufferedReader in = new BufferedReader(
+                         new InputStreamReader(socket.getInputStream(), StandardCharsets.US_ASCII));
+                 Writer out = new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.US_ASCII)) {
+                reply(out, "220 localhost ESMTP");
+                String line;
+                while ((line = in.readLine()) != null) {
+                    commands.add(line);
+                    if (line.startsWith("EHLO")) {
+                        reply(out, "250-localhost\r\n250 8BITMIME");
+                    } else if (line.equals("DATA")) {
+                        reply(out, "354 end with <CRLF>.<CRLF>");
+                        while ((line = in.readLine()) != null && !line.equals(".")) {
+                            // message content
+                        }
+                        reply(out, "250 OK");
+                    } else if (line.equals("QUIT")) {
+                        reply(out, "221 bye");
+                        return;
+                    } else {
+                        reply(out, "250 OK");
+                    }
+                }
+            } catch (IOException e) {
+                // the client closed the connection
+            }
+        }
+
+        private static void reply(Writer out, String reply) throws IOException {
+            out.write(reply + "\r\n");
+            out.flush();
+        }
+
+        @Override
+        public void close() throws IOException {
+            serverSocket.close();
+        }
+    }
+
     private static Properties sessionProperties(JsonValue config) throws Exception {
         EmailClient client = new EmailClient(config);
         Field session = EmailClient.class.getDeclaredField("session");

openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/settings/EmailConfigView.js

@@ -37,6 +37,8 @@ define([
         noBaseTemplate: true,
         events: {
             "click #emailAuth": "toggleUserPass",
+            "change #emailTLS": "syncStartTls",
+            "change #emailTLSRequired": "syncStartTls",
             "change #emailToggle": "toggleEmail",
             "change #emailAuthPassword": "updatePassword",
             "click #saveEmailConfig": "save"
@@ -105,6 +107,17 @@ define([
             this.$el.find("#smtpauth").slideToggle($(e.currentTarget).prop("checked"));
         },
 
+        // "required" implies STARTTLS (see EmailClient), so keep the two switches consistent
+        syncStartTls: function(e) {
+            var checked = $(e.currentTarget).prop("checked");
+
+            if (e.currentTarget.id === "emailTLSRequired" && checked) {
+                this.$el.find("#emailTLS").prop("checked", true);
+            } else if (e.currentTarget.id === "emailTLS" && !checked) {
+                this.$el.find("#emailTLSRequired").prop("checked", false);
+            }
+        },
+
         toggleEmail: function() {
             if (!this.$el.find("#emailToggle").is(":checked")) {
                 if (this.$el.find("#smtpauth").is(":visible")) {

openidm-ui/openidm-ui-admin/src/main/resources/templates/admin/settings/EmailConfigTemplate.html

@@ -1,4 +1,5 @@
 <!-- Copyright 2015 ForgeRock AS.
+	Portions Copyright 2026 3A Systems, LLC.
 	License terms: https://forgerock.org/cddlv1-0/ -->
 <form id="emailConfigForm" class="form clearfix panel-collapse-group" autocomplete="off">
     <div class="panel-body">
@@ -54,7 +55,18 @@
                             <div class="col-sm-6">
                                 <div class="checkbox checkbox-slider-primary checkbox-slider checkbox-slider--b checkbox-slider-md">
                                     <label>
-                                        <input type="checkbox" name="starttls.enable" id="emailTLS" value="true" {{#if config.starttls.enable}}checked{{/if}}/><span></span>
+                                        <input type="checkbox" name="starttls.enable" id="emailTLS" value="true" {{#if config.starttls.enable}}checked{{else}}{{#if config.starttls.required}}checked{{/if}}{{/if}}/><span></span>
+                                    </label>
+                                </div>
+                            </div>
+                        </div>
+
+                        <div class="form-group">
+                            <label for="emailTLSRequired" class="col-sm-3 control-label">{{t "templates.emailConfig.tlsRequired"}}</label>
+                            <div class="col-sm-6">
+                                <div class="checkbox checkbox-slider-primary checkbox-slider checkbox-slider--b checkbox-slider-md">
+                                    <label>
+                                        <input type="checkbox" name="starttls.required" id="emailTLSRequired" value="true" {{#if config.starttls.required}}checked{{/if}}/><span></span>
                                     </label>
                                 </div>
                             </div>

openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/settings/EmailConfigViewTest.js

@@ -18,8 +18,10 @@ define([
     "jquery",
     "sinon",
     "org/forgerock/openidm/ui/admin/settings/EmailConfigView",
-    "org/forgerock/openidm/ui/common/delegates/ConfigDelegate"
-], function ($, sinon, EmailConfigView, ConfigDelegate) {
+    "org/forgerock/openidm/ui/common/delegates/ConfigDelegate",
+    "org/forgerock/openidm/ui/common/util/ThemeManager",
+    "org/forgerock/commons/ui/common/main/ValidatorsManager"
+], function ($, sinon, EmailConfigView, ConfigDelegate, ThemeManager, ValidatorsManager) {
     QUnit.module('EmailConfigView Tests');
 
     QUnit.test("save keeps the STARTTLS keys the form does not edit", function (assert) {
@@ -50,4 +52,73 @@ define([
         assert.deepEqual(saved.starttls.trustedHosts, ["smtp.internal"], "trustedHosts is kept");
         assert.strictEqual(saved.starttls.trustAll, false, "trustAll is kept");
     });
+
+    // renders the real template with the given stored config
+    function renderStored(config, callback) {
+        var theme = sinon.stub(ThemeManager, "getTheme", function () {
+                return $.Deferred().resolve({});
+            }),
+            read = sinon.stub(ConfigDelegate, "readEntity", function () {
+                return $.Deferred().resolve(config);
+            }),
+            bind = sinon.stub(ValidatorsManager, "bindValidators"),
+            validate = sinon.stub(ValidatorsManager, "validateAllFields");
+
+        $("#qunit-fixture").html('<div id="emailContainer"></div>');
+        EmailConfigView.model = { externalEmailExists: false };
+        EmailConfigView.data = { config: {} };
+        EmailConfigView.render([], function () {
+            theme.restore();
+            read.restore();
+            bind.restore();
+            validate.restore();
+            callback();
+            EmailConfigView.undelegateEvents();
+        });
+    }
+
+    QUnit.test("a stored STARTTLS required renders Use STARTTLS on", function (assert) {
+        var done = assert.async();
+
+        renderStored({ host: "smtp.example.com", starttls: { required: true } }, function () {
+            assert.ok(EmailConfigView.$el.find("#emailTLS").prop("checked"), "required implies STARTTLS");
+            assert.ok(EmailConfigView.$el.find("#emailTLSRequired").prop("checked"), "required is shown");
+            done();
+        });
+    });
+
+    QUnit.test("the rendered Require STARTTLS switch saves false when unchecked", function (assert) {
+        var done = assert.async();
+
+        renderStored({ host: "smtp.example.com", starttls: { enable: true, required: true } }, function () {
+            var saved,
+                update = sinon.stub(ConfigDelegate, "updateEntity", function (id, config) {
+                    saved = config;
+                    return $.Deferred();
+                });
+
+            EmailConfigView.model.externalEmailExists = true;
+            EmailConfigView.$el.find("#emailTLSRequired").prop("checked", false);
+            EmailConfigView.save({ preventDefault: $.noop });
+            update.restore();
+
+            assert.strictEqual(saved.starttls.enable, true, "STARTTLS stays on");
+            assert.strictEqual(saved.starttls.required, false, "the template's switch overrides the stored true");
+            done();
+        });
+    });
+
+    QUnit.test("the STARTTLS switches stay consistent on change", function (assert) {
+        $("#qunit-fixture").html('<input type="checkbox" id="emailTLS">' +
+            '<input type="checkbox" id="emailTLSRequired">');
+        EmailConfigView.setElement($("#qunit-fixture"));
+
+        $("#emailTLSRequired").prop("checked", true).trigger("change");
+        assert.ok($("#emailTLS").prop("checked"), "checking required turns STARTTLS on");
+
+        $("#emailTLS").prop("checked", false).trigger("change");
+        assert.notOk($("#emailTLSRequired").prop("checked"), "turning STARTTLS off clears required");
+
+        EmailConfigView.undelegateEvents();
+    });
 });