From 2af4b5a589e3b1515b21068a1370204750bf0d33 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 13:53:13 +0300 Subject: [PATCH 1/5] Harden GitHub workflows and SMTP STARTTLS trust - Set an explicit read-only GITHUB_TOKEN permissions block on the build, deploy and release workflows, elevating only the jobs that need it (wiki/docs push and release tag: contents:write; ghcr push: packages:write) - Pin the third-party docker/* and softprops/action-gh-release actions to commit SHAs - EmailClient: stop trusting every SMTP server certificate over STARTTLS; validation is now the default, with opt-in starttls.trustedHosts / starttls.trustAll settings (documented) Resolves CodeQL alerts #711-#716, #718-#738, #920, #921 (actions) and #22 (java/insecure-smtp-ssl). --- .github/workflows/build.yml | 18 ++-- .github/workflows/deploy.yml | 4 + .github/workflows/release.yml | 36 +++++--- .../asciidoc/integrators-guide/chap-mail.adoc | 9 +- .../external/email/impl/EmailClient.java | 47 ++++++++-- .../external/email/impl/EmailClientTest.java | 88 +++++++++++++++++++ 6 files changed, 173 insertions(+), 29 deletions(-) create mode 100644 openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 10c27101bf..9fe9f84199 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,6 +20,8 @@ on: concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +permissions: + contents: read jobs: build-maven: runs-on: ${{ matrix.os }} @@ -313,20 +315,20 @@ jobs: echo "release_version=$git_version_last" >> $GITHUB_ENV - name: Docker meta id: meta - uses: docker/metadata-action@v6 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: images: | localhost:5000/${{ github.repository }} tags: | type=raw,value=${{ env.release_version }} - name: Set up QEMU - uses: docker/setup-qemu-action@v4 + uses: docker/setup-qemu-action@99012661954931238ded8c8b007157a8430204e1 # v4.4.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 with: driver-opts: network=host - name: Build image (default) - uses: docker/build-push-action@v7 + uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 continue-on-error: true with: context: . @@ -361,7 +363,7 @@ jobs: echo "release_version=$git_version_last" >> $GITHUB_ENV - name: Docker meta id: meta - uses: docker/metadata-action@v6 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: images: | localhost:5000/${{ github.repository }} @@ -369,14 +371,14 @@ jobs: type=raw,value=alpine type=raw,value=${{ env.release_version }}-alpine - name: Set up QEMU - uses: docker/setup-qemu-action@v4 + uses: docker/setup-qemu-action@99012661954931238ded8c8b007157a8430204e1 # v4.4.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 with: driver-opts: network=host - name: Build image continue-on-error: true - uses: docker/build-push-action@v7 + uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 with: context: . file: ./Dockerfile-alpine diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index bddaecbf0a..2a073ae35a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -22,11 +22,15 @@ on: concurrency: group: ${{ github.workflow }}-${{ github.event.workflow_run.head_branch }} cancel-in-progress: false +permissions: + contents: read jobs: deploy-maven: if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event=='push' }} runs-on: 'ubuntu-latest' + permissions: + contents: write # the docs push to the repository wiki uses github.token steps: - name: Print github context env: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 23c5e681ab..fb27ba8b46 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,9 +27,13 @@ on: concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: false +permissions: + contents: read jobs: release-maven: runs-on: 'ubuntu-latest' + permissions: + contents: write # release:prepare pushes the release tag, action-gh-release creates the release, docs go to the wiki steps: - name: Print github context env: @@ -75,7 +79,7 @@ jobs: if: ${{ env.MAVEN_USERNAME!='' && env.MAVEN_PASSWORD!='' }} run: mvn --batch-mode -Darguments="-Dgpg.passphrase=${{ secrets.GPG_PASSPHRASE }}" -DsignTag=true -DtagNameFormat="${{ github.event.inputs.releaseVersion }}" -DreleaseVersion=${{ github.event.inputs.releaseVersion }} -DdevelopmentVersion=${{ github.event.inputs.developmentVersion }} release:prepare release:perform --file pom.xml - name: Release on GitHub - uses: softprops/action-gh-release@v3 + uses: softprops/action-gh-release@efb35369e0ad2afab669f228072c1b0d510eae64 # v3.0.3 with: name: ${{ github.event.inputs.releaseVersion }} tag_name: ${{ github.event.inputs.releaseVersion }} @@ -128,6 +132,9 @@ jobs: release-docker: name: Docker release runs-on: 'ubuntu-latest' + permissions: + contents: read + packages: write # push to ghcr.io with GITHUB_TOKEN needs: - release-maven steps: @@ -138,7 +145,7 @@ jobs: submodules: recursive - name: Docker meta id: meta - uses: docker/metadata-action@v6 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: images: | ${{ github.repository }} @@ -147,22 +154,22 @@ jobs: type=raw,value=latest type=raw,value=${{ github.event.inputs.releaseVersion }} - name: Set up QEMU - uses: docker/setup-qemu-action@v4 + uses: docker/setup-qemu-action@99012661954931238ded8c8b007157a8430204e1 # v4.4.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 - name: Login to DockerHub - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Login to GHCR - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: registry: ghcr.io username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push image - uses: docker/build-push-action@v7 + uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 continue-on-error: true with: context: . @@ -176,6 +183,9 @@ jobs: release-docker-alpine: name: Docker release runs-on: 'ubuntu-latest' + permissions: + contents: read + packages: write # push to ghcr.io with GITHUB_TOKEN needs: - release-maven steps: @@ -186,7 +196,7 @@ jobs: submodules: recursive - name: Docker meta id: meta - uses: docker/metadata-action@v6 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: images: | ${{ github.repository }} @@ -195,23 +205,23 @@ jobs: type=raw,value=alpine type=raw,value=${{ github.event.inputs.releaseVersion }}-alpine - name: Set up QEMU - uses: docker/setup-qemu-action@v4 + uses: docker/setup-qemu-action@99012661954931238ded8c8b007157a8430204e1 # v4.4.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 - name: Login to DockerHub - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Login to GHCR - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: registry: ghcr.io username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push image continue-on-error: true - uses: docker/build-push-action@v7 + uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 with: context: . file: ./Dockerfile-alpine diff --git a/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc b/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc index 1abff680ce..ec12066718 100644 --- a/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc +++ b/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc @@ -12,7 +12,7 @@ information: "Portions copyright [year] [name of copyright owner]". Copyright 2017 ForgeRock AS. - Portions Copyright 2024 3A Systems LLC. + Portions Copyright 2024-2026 3A Systems LLC. //// :figure-caption!: @@ -123,6 +123,13 @@ 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. ++ +The SMTP server certificate is validated against the JVM trust store. Two optional settings relax that: ++ + +* `trustedHosts`—a list of SMTP host names whose certificate is accepted without validation, for example `"trustedHosts" : [ "smtp.internal.example.com" ]`. + +* `trustAll`—when `true`, accepts any server certificate. This disables protection against man-in-the-middle attacks; use it only in development environments. `from`:: (Optional) Specifies a default `From:` address, that users see when they receive emails from OpenIDM. diff --git a/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java b/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java index 31c02d763f..15765e7e4f 100644 --- a/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java +++ b/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java @@ -20,6 +20,8 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * + * Portions Copyright 2026 3A Systems, LLC. */ package org.forgerock.openidm.external.email.impl; @@ -27,7 +29,12 @@ import com.sun.mail.util.MailSSLSocketFactory; import org.forgerock.json.JsonValue; import org.forgerock.json.resource.BadRequestException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.security.GeneralSecurityException; +import java.util.Collections; +import java.util.List; import java.util.Properties; import javax.mail.Message; import javax.mail.MessagingException; @@ -42,6 +49,8 @@ */ public class EmailClient { + private static final Logger logger = LoggerFactory.getLogger(EmailClient.class); + private static final String DEFAULT_HOST = "localhost"; private static final String DEFAULT_PORT = "25"; private String username = null; @@ -60,6 +69,10 @@ 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"; + /** Opt-in: accept any server certificate over STARTTLS. Never use outside development. */ + public static final String CONFIG_MAIL_SMTP_STARTTLS_TRUST_ALL = "trustAll"; + /** Optional list of SMTP hosts whose certificate is accepted without validation. */ + public static final String CONFIG_MAIL_SMTP_STARTTLS_TRUSTED_HOSTS = "trustedHosts"; public static final String CONFIG_MAIL_FROM = "from"; public static final String CONFIG_MAIL_DEBUG = "debug"; @@ -83,19 +96,39 @@ public EmailClient(JsonValue config) throws RuntimeException { boolean startTLS = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_ENABLE).defaultTo(false).asBoolean(); if (startTLS) { props.put("mail.smtp.starttls.enable", String.valueOf(startTLS)); - // temporary hack to avoid cert check - try { - MailSSLSocketFactory sf = new MailSSLSocketFactory(); - sf.setTrustAllHosts(true); - props.put("mail.smtp.ssl.socketFactory", sf); - } catch (Exception e) { - } + configureStartTlsTrust(starttlsConfig); } fromAddr = config.get(CONFIG_MAIL_FROM).asString(); session = Session.getInstance(props); } + /** + * By default the server certificate is validated against the JVM trust store. A custom + * socket factory is installed only when the configuration explicitly relaxes that, either + * for a list of {@code trustedHosts} or, for development only, for all hosts. + */ + private void configureStartTlsTrust(JsonValue starttlsConfig) { + boolean trustAll = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_TRUST_ALL).defaultTo(false).asBoolean(); + List trustedHosts = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_TRUSTED_HOSTS) + .defaultTo(Collections.emptyList()).asList(String.class); + if (!trustAll && trustedHosts.isEmpty()) { + return; + } + try { + MailSSLSocketFactory sf = new MailSSLSocketFactory(); + if (trustAll) { + logger.warn("SMTP STARTTLS certificate validation is disabled (starttls.trustAll=true)"); + sf.setTrustAllHosts(trustAll); + } else { + sf.setTrustedHosts(trustedHosts.toArray(new String[0])); + } + props.put("mail.smtp.ssl.socketFactory", sf); + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Unable to configure the SMTP STARTTLS socket factory", e); + } + } + /** * Send the email according to the parameters in params: * diff --git a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java new file mode 100644 index 0000000000..7b721a5d74 --- /dev/null +++ b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java @@ -0,0 +1,88 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openidm.external.email.impl; + +import static org.assertj.core.api.Assertions.assertThat; +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.lang.reflect.Field; +import java.util.Properties; + +import javax.mail.Session; + +import com.sun.mail.util.MailSSLSocketFactory; +import org.forgerock.json.JsonValue; +import org.testng.annotations.Test; + +/** + * Tests for the STARTTLS trust settings of {@link EmailClient}. + */ +public class EmailClientTest { + + private static final String SOCKET_FACTORY = "mail.smtp.ssl.socketFactory"; + + @Test + public void startTlsValidatesTheServerCertificateByDefault() throws Exception { + Properties props = sessionProperties(json(object( + field("host", "smtp.example.com"), + field("starttls", object(field("enable", true)))))); + + assertThat(props.get("mail.smtp.starttls.enable")).isEqualTo("true"); + assertThat(props.get(SOCKET_FACTORY)).as("no custom trust: JSSE validation applies").isNull(); + } + + @Test + public void startTlsTrustAllIsOptIn() throws Exception { + Properties props = sessionProperties(json(object( + field("host", "smtp.example.com"), + field("starttls", object(field("enable", true), field("trustAll", true)))))); + + MailSSLSocketFactory sf = (MailSSLSocketFactory) props.get(SOCKET_FACTORY); + assertThat(sf).isNotNull(); + assertThat(sf.isTrustAllHosts()).isTrue(); + } + + @Test + public void startTlsTrustedHostsAreLimitedToTheConfiguredList() throws Exception { + Properties props = sessionProperties(json(object( + field("host", "smtp.example.com"), + field("starttls", object(field("enable", true), + field("trustedHosts", array("smtp.example.com", "mail.internal"))))))); + + MailSSLSocketFactory sf = (MailSSLSocketFactory) props.get(SOCKET_FACTORY); + assertThat(sf).isNotNull(); + assertThat(sf.isTrustAllHosts()).isFalse(); + assertThat(sf.getTrustedHosts()).containsExactly("smtp.example.com", "mail.internal"); + } + + @Test + public void withoutStartTlsNoSocketFactoryIsConfigured() throws Exception { + Properties props = sessionProperties(json(object(field("host", "smtp.example.com")))); + + assertThat(props.get("mail.smtp.starttls.enable")).isNull(); + assertThat(props.get(SOCKET_FACTORY)).isNull(); + } + + private static Properties sessionProperties(JsonValue config) throws Exception { + EmailClient client = new EmailClient(config); + Field session = EmailClient.class.getDeclaredField("session"); + session.setAccessible(true); + return ((Session) session.get(client)).getProperties(); + } +} From 054e14aa9db193ea6b453c224352491609f1581c Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 18:54:41 +0300 Subject: [PATCH 2/5] Add .github/dependabot.yml for the SHA-pinned actions Keeps the commit-hash-pinned third-party actions in .github/workflows up to date: Dependabot bumps the SHA and the trailing version comment together, grouped into one weekly PR. Ported from OpenIdentityPlatform/OpenIG#170. --- .github/dependabot.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000000..98caa0cf0d --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,28 @@ +# The contents of this file are subject to the terms of the Common Development and +# Distribution License (the License). You may not use this file except in compliance with the +# License. +# +# You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the +# specific language governing permission and limitations under the License. +# +# When distributing Covered Software, include this CDDL Header Notice in each file and include +# the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL +# Header, with the fields enclosed by brackets [] replaced by your own identifying +# information: "Portions copyright [year] [name of copyright owner]". +# +# Copyright 2026 3A Systems, LLC. +version: 2 +updates: + # Keeps the SHA-pinned third-party actions in .github/workflows up to date: Dependabot + # bumps the commit hash and the trailing "# vX.Y.Z" version comment together. + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + groups: + # One pull request per week for all action updates instead of one per action. + github-actions: + patterns: ["*"] + labels: + - "ci" + - "dependencies" From 017f57d7d7ee81aed4893b0bb0afae1531d36fec Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 3 Oct 2026 10:21:41 +0300 Subject: [PATCH 3/5] Address review on STARTTLS trust: host check, trustedHosts, Admin UI - Validate the SMTP certificate's host name by default (mail.smtp.ssl.checkserveridentity=true); JavaMail 1.4.7 leaves it off - Exclude jakarta.mail from the email bundle's classpath: compiling against its com.sun.mail.util made the bundle import MailSSLSocketFactory from jakarta.mail, which javax.mail's SocketFetcher does not recognise, so starttls.trustedHosts trusted every host - Warn when starttls.trustedHosts does not contain the SMTP host - Admin UI: keep starttls.trustedHosts/trustAll when saving the Email form - Tests: host check, trustAll precedence, socket factory origin - Docs: host match, exact trustedHosts entry, trustAll precedence --- .../asciidoc/integrators-guide/chap-mail.adoc | 6 ++--- openidm-external-email/pom.xml | 11 +++++++++- .../external/email/impl/EmailClient.java | 19 ++++++++++++---- .../external/email/impl/EmailClientTest.java | 22 +++++++++++++++++++ .../ui/admin/settings/EmailConfigView.js | 5 +++++ 5 files changed, 55 insertions(+), 8 deletions(-) diff --git a/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc b/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc index ec12066718..74338ab200 100644 --- a/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc +++ b/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc @@ -124,12 +124,12 @@ 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. + -The SMTP server certificate is validated against the JVM trust store. Two optional settings relax that: +The SMTP server certificate is validated against the JVM trust store and must be issued for the configured `host`. Two optional settings relax that: + -* `trustedHosts`—a list of SMTP host names whose certificate is accepted without validation, for example `"trustedHosts" : [ "smtp.internal.example.com" ]`. +* `trustedHosts`—a list of SMTP host names whose certificate is accepted without validation, for example `"trustedHosts" : [ "smtp.internal.example.com" ]`. Once the list is set, the configured `host` must appear in it exactly as written (the comparison is case-sensitive); any other host is rejected. -* `trustAll`—when `true`, accepts any server certificate. This disables protection against man-in-the-middle attacks; use it only in development environments. +* `trustAll`—when `true`, accepts any server certificate, and takes precedence over `trustedHosts`. This disables protection against man-in-the-middle attacks; use it only in development environments. `from`:: (Optional) Specifies a default `From:` address, that users see when they receive emails from OpenIDM. diff --git a/openidm-external-email/pom.xml b/openidm-external-email/pom.xml index 4ea5893084..70258a8281 100644 --- a/openidm-external-email/pom.xml +++ b/openidm-external-email/pom.xml @@ -22,7 +22,7 @@ ~ your own identifying information: ~ "Portions Copyrighted [year] [name of copyright owner]" ~ - ~ Portions Copyrighted 2024 3A Systems LLC. + ~ Portions Copyrighted 2024-2026 3A Systems LLC. --> 4.0.0 @@ -42,6 +42,15 @@ org.openidentityplatform.openidm openidm-enhanced-config ${project.version} + + + + com.sun.mail + jakarta.mail + + diff --git a/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java b/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java index 15765e7e4f..090ab6e84f 100644 --- a/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java +++ b/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java @@ -71,7 +71,10 @@ public class EmailClient { public static final String CONFIG_MAIL_SMTP_STARTTLS_ENABLE = "enable"; /** Opt-in: accept any server certificate over STARTTLS. Never use outside development. */ public static final String CONFIG_MAIL_SMTP_STARTTLS_TRUST_ALL = "trustAll"; - /** Optional list of SMTP hosts whose certificate is accepted without validation. */ + /** + * Optional list of SMTP hosts whose certificate is accepted without validation. Once set, only + * a {@code host} listed exactly as configured is accepted; any other host is rejected. + */ public static final String CONFIG_MAIL_SMTP_STARTTLS_TRUSTED_HOSTS = "trustedHosts"; public static final String CONFIG_MAIL_FROM = "from"; public static final String CONFIG_MAIL_DEBUG = "debug"; @@ -104,17 +107,25 @@ public EmailClient(JsonValue config) throws RuntimeException { } /** - * By default the server certificate is validated against the JVM trust store. A custom - * socket factory is installed only when the configuration explicitly relaxes that, either - * for a list of {@code trustedHosts} or, for development only, for all hosts. + * By default the server certificate is validated against the JVM trust store and must be + * issued for the configured host. A custom socket factory is installed only when the + * configuration explicitly relaxes that, either for a list of {@code trustedHosts} or, for + * development only, for all hosts. */ private void configureStartTlsTrust(JsonValue starttlsConfig) { boolean trustAll = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_TRUST_ALL).defaultTo(false).asBoolean(); List trustedHosts = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_TRUSTED_HOSTS) .defaultTo(Collections.emptyList()).asList(String.class); if (!trustAll && trustedHosts.isEmpty()) { + // validate the chain (JSSE default) and that the certificate was issued for the host + props.put("mail.smtp.ssl.checkserveridentity", "true"); return; } + String host = props.getProperty("mail.smtp.host"); + if (!trustAll && !trustedHosts.contains(host)) { + // JavaMail matches the host against the list exactly and rejects any other host + logger.warn("starttls.trustedHosts {} does not contain the SMTP host {}", trustedHosts, host); + } try { MailSSLSocketFactory sf = new MailSSLSocketFactory(); if (trustAll) { diff --git a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java index 7b721a5d74..5cc8a10b3d 100644 --- a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java +++ b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java @@ -36,6 +36,7 @@ public class EmailClientTest { private static final String SOCKET_FACTORY = "mail.smtp.ssl.socketFactory"; + private static final String CHECK_SERVER_IDENTITY = "mail.smtp.ssl.checkserveridentity"; @Test public void startTlsValidatesTheServerCertificateByDefault() throws Exception { @@ -45,6 +46,7 @@ public void startTlsValidatesTheServerCertificateByDefault() throws Exception { assertThat(props.get("mail.smtp.starttls.enable")).isEqualTo("true"); assertThat(props.get(SOCKET_FACTORY)).as("no custom trust: JSSE validation applies").isNull(); + assertThat(props.get(CHECK_SERVER_IDENTITY)).isEqualTo("true"); } @Test @@ -69,6 +71,26 @@ public void startTlsTrustedHostsAreLimitedToTheConfiguredList() throws Exception assertThat(sf).isNotNull(); assertThat(sf.isTrustAllHosts()).isFalse(); assertThat(sf.getTrustedHosts()).containsExactly("smtp.example.com", "mail.internal"); + assertThat(props.get(CHECK_SERVER_IDENTITY)).isNull(); + } + + @Test + public void startTlsTrustAllWinsOverTrustedHosts() throws Exception { + Properties props = sessionProperties(json(object( + field("host", "smtp.example.com"), + field("starttls", object(field("enable", true), field("trustAll", true), + field("trustedHosts", array("mail.internal"))))))); + + MailSSLSocketFactory sf = (MailSSLSocketFactory) props.get(SOCKET_FACTORY); + assertThat(sf.isTrustAllHosts()).isTrue(); + } + + @Test + public void socketFactoryComesFromTheJavaMailInUse() { + // JavaMail checks trustedHosts only for its own MailSSLSocketFactory class; a copy from + // another mail jar (jakarta.mail) is not recognised and then trusts every host + assertThat(MailSSLSocketFactory.class.getProtectionDomain().getCodeSource().getLocation()) + .isEqualTo(Session.class.getProtectionDomain().getCodeSource().getLocation()); } @Test diff --git a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/settings/EmailConfigView.js b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/settings/EmailConfigView.js index e7040710f6..29061ce6fb 100644 --- a/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/settings/EmailConfigView.js +++ b/openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/settings/EmailConfigView.js @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2015-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ define([ @@ -133,6 +134,10 @@ define([ e.preventDefault(); var formData = form2js("emailConfigForm",".", true); + if (_.has(formData, "starttls")) { + // keep the STARTTLS keys the form does not edit (trustedHosts, trustAll) + formData.starttls = _.extend({}, this.data.config.starttls, formData.starttls); + } _.extend(this.data.config, formData); if (!_.has(formData, "starttls") || !_.has(formData.starttls, "enable")) { From eead561bf963b725519cadbc23c7a3bce4468cd4 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sun, 4 Oct 2026 12:02:53 +0300 Subject: [PATCH 4/5] Embed javax.mail in the email bundle and enable current TLS for STARTTLS The com.sun.mail.util [1.4,2) import from the previous commit left openidm-external-email unresolved, so OpenIDM never reached "ready" in CI: javax.mail 1.4.7 imports its own com.sun.mail.* packages with no upper bound, Felix wires them to jakarta.mail 2.0.2 and drops the 1.4.7 exports. The same wiring already broke mail on master, where MimeMessage fails with NoSuchMethodError on com.sun.mail.util.PropUtil. - Embed javax.mail 1.4.7 in the bundle so Session, SMTPTransport, SocketFetcher and MailSSLSocketFactory come from one jar, and point the context class loader at the bundle while JavaMail loads its providers. - Set mail.smtp.ssl.protocols to the JVM defaults when STARTTLS is on. Without it JavaMail 1.4.7 enables only TLSv1, which current JDKs disable, so no STARTTLS handshake could succeed. --- openidm-external-email/pom.xml | 20 +++++++++++-- .../external/email/impl/EmailClient.java | 28 ++++++++++++++++++- .../external/email/impl/EmailClientTest.java | 12 ++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/openidm-external-email/pom.xml b/openidm-external-email/pom.xml index 70258a8281..8992d23e78 100644 --- a/openidm-external-email/pom.xml +++ b/openidm-external-email/pom.xml @@ -42,9 +42,8 @@ org.openidentityplatform.openidm openidm-enhanced-config ${project.version} - + com.sun.mail @@ -122,6 +121,21 @@ org.apache.felix maven-bundle-plugin true + + + + mail;groupId=javax.mail;inline=false + + sun.security.util;resolution:=optional, + javax.security.sasl;resolution:=optional, + * + + + diff --git a/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java b/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java index 090ab6e84f..52e15a9bc2 100644 --- a/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java +++ b/openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java @@ -33,6 +33,7 @@ import org.slf4j.LoggerFactory; import java.security.GeneralSecurityException; +import java.security.NoSuchAlgorithmException; import java.util.Collections; import java.util.List; import java.util.Properties; @@ -43,6 +44,7 @@ import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; import javax.mail.internet.MimeMessage; +import javax.net.ssl.SSLContext; /** * Email client. @@ -99,11 +101,30 @@ public EmailClient(JsonValue config) throws RuntimeException { boolean startTLS = starttlsConfig.get(CONFIG_MAIL_SMTP_STARTTLS_ENABLE).defaultTo(false).asBoolean(); if (startTLS) { props.put("mail.smtp.starttls.enable", String.valueOf(startTLS)); + // without this JavaMail 1.4.7 enables only TLSv1 for STARTTLS, which current JDKs disable + props.put("mail.smtp.ssl.protocols", defaultTlsProtocols()); configureStartTlsTrust(starttlsConfig); } fromAddr = config.get(CONFIG_MAIL_FROM).asString(); - session = Session.getInstance(props); + // JavaMail looks up its providers and resources through the context class loader first; + // point it at this bundle so the embedded javax.mail is used, not another copy + ClassLoader originalContextClassLoader = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread().setContextClassLoader(EmailClient.class.getClassLoader()); + session = Session.getInstance(props); + } finally { + Thread.currentThread().setContextClassLoader(originalContextClassLoader); + } + } + + /** The TLS protocols the JVM enables by default, space-separated as JavaMail expects them. */ + private static String defaultTlsProtocols() { + try { + return String.join(" ", SSLContext.getDefault().getDefaultSSLParameters().getProtocols()); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("Unable to determine the default TLS protocols", e); + } } /** @@ -204,7 +225,10 @@ public void send(JsonValue params) throws BadRequestException { throw new BadRequestException("Bad Bcc: email address"); } + // the transport and the content handlers are loaded through the context class loader (see constructor) + ClassLoader originalContextClassLoader = Thread.currentThread().getContextClassLoader(); try { + Thread.currentThread().setContextClassLoader(EmailClient.class.getClassLoader()); Message message = new MimeMessage(session); message.setFrom(from); message.setRecipients(Message.RecipientType.TO, to); @@ -248,6 +272,8 @@ public void send(JsonValue params) throws BadRequestException { } catch (MessagingException e) { throw new BadRequestException(e); + } finally { + Thread.currentThread().setContextClassLoader(originalContextClassLoader); } } diff --git a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java index 5cc8a10b3d..e0f239d95e 100644 --- a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java +++ b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java @@ -25,6 +25,7 @@ import java.util.Properties; import javax.mail.Session; +import javax.net.ssl.SSLContext; import com.sun.mail.util.MailSSLSocketFactory; import org.forgerock.json.JsonValue; @@ -85,6 +86,17 @@ public void startTlsTrustAllWinsOverTrustedHosts() throws Exception { assertThat(sf.isTrustAllHosts()).isTrue(); } + @Test + public void startTlsUsesTheJvmDefaultProtocols() throws Exception { + Properties props = sessionProperties(json(object( + field("host", "smtp.example.com"), + field("starttls", object(field("enable", true)))))); + + // JavaMail 1.4.7 falls back to TLSv1 alone, which current JDKs disable + assertThat(props.getProperty("mail.smtp.ssl.protocols").split(" ")) + .containsExactly(SSLContext.getDefault().getDefaultSSLParameters().getProtocols()); + } + @Test public void socketFactoryComesFromTheJavaMailInUse() { // JavaMail checks trustedHosts only for its own MailSSLSocketFactory class; a copy from From a997a148de7795de8a574633278831fd3ac84756 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Mon, 5 Oct 2026 11:00:33 +0300 Subject: [PATCH 5/5] Address review round 2 on STARTTLS trust: IP SAN note and tests - chap-mail.adoc: on Java 17+ the host is matched only against DNS SANs (or the CN), so a relay addressed by IP is rejected even with an IP SAN - EmailClientTest: replace withoutStartTlsNoSocketFactoryIsConfigured with trustSettingsApplyOnlyWithStartTls, which fails if the trust settings are applied outside STARTTLS - EmailConfigViewTest: pin that saving Settings > Email keeps starttls.trustedHosts and starttls.trustAll --- .../asciidoc/integrators-guide/chap-mail.adoc | 2 +- .../external/email/impl/EmailClientTest.java | 7 ++- .../ui/admin/settings/EmailConfigViewTest.js | 54 +++++++++++++++++-- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc b/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc index 74338ab200..2df63146d9 100644 --- a/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc +++ b/openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc @@ -124,7 +124,7 @@ 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. + -The SMTP server certificate is validated against the JVM trust store and must be issued for the configured `host`. Two optional settings relax that: +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: + * `trustedHosts`—a list of SMTP host names whose certificate is accepted without validation, for example `"trustedHosts" : [ "smtp.internal.example.com" ]`. Once the list is set, the configured `host` must appear in it exactly as written (the comparison is case-sensitive); any other host is rejected. diff --git a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java index e0f239d95e..628ff368d2 100644 --- a/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java +++ b/openidm-external-email/src/test/java/org/forgerock/openidm/external/email/impl/EmailClientTest.java @@ -106,11 +106,14 @@ public void socketFactoryComesFromTheJavaMailInUse() { } @Test - public void withoutStartTlsNoSocketFactoryIsConfigured() throws Exception { - Properties props = sessionProperties(json(object(field("host", "smtp.example.com")))); + public void trustSettingsApplyOnlyWithStartTls() throws Exception { + Properties props = sessionProperties(json(object( + field("host", "smtp.example.com"), + field("starttls", object(field("enable", false), field("trustAll", true)))))); assertThat(props.get("mail.smtp.starttls.enable")).isNull(); assertThat(props.get(SOCKET_FACTORY)).isNull(); + assertThat(props.get(CHECK_SERVER_IDENTITY)).isNull(); } private static Properties sessionProperties(JsonValue config) throws Exception { diff --git a/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/settings/EmailConfigViewTest.js b/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/settings/EmailConfigViewTest.js index 19d92721b4..cf78712bd5 100644 --- a/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/settings/EmailConfigViewTest.js +++ b/openidm-ui/openidm-ui-admin/src/test/qunit/org/forgerock/openidm/ui/admin/settings/EmailConfigViewTest.js @@ -1,5 +1,53 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Portions Copyright 2026 3A Systems, LLC. + */ + define([ - "org/forgerock/openidm/ui/admin/settings/EmailConfigView" -], function (EmailConfigView) { + "jquery", + "sinon", + "org/forgerock/openidm/ui/admin/settings/EmailConfigView", + "org/forgerock/openidm/ui/common/delegates/ConfigDelegate" +], function ($, sinon, EmailConfigView, ConfigDelegate) { QUnit.module('EmailConfigView Tests'); -}); \ No newline at end of file + + QUnit.test("save keeps the STARTTLS keys the form does not edit", function (assert) { + var saved, + stub = sinon.stub(ConfigDelegate, "updateEntity", function (id, config) { + saved = config; + return $.Deferred(); + }); + + $("#qunit-fixture").html('' + + '
' + + '' + + '' + + '
'); + EmailConfigView.$el = $("#qunit-fixture"); + EmailConfigView.model = { externalEmailExists: true }; + EmailConfigView.data = { + config: { + host: "smtp.example.com", + starttls: { enable: true, trustedHosts: ["smtp.internal"], trustAll: false } + } + }; + + EmailConfigView.save({ preventDefault: $.noop }); + stub.restore(); + + assert.ok(saved.starttls.enable, "the form's STARTTLS flag is saved"); + assert.deepEqual(saved.starttls.trustedHosts, ["smtp.internal"], "trustedHosts is kept"); + assert.strictEqual(saved.starttls.trustAll, false, "trustAll is kept"); + }); +});