From cb50e655ba3af38297b0c8f7d771d5532fddf7e6 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 13:18:26 +0300 Subject: [PATCH 1/5] Close the cheap CodeQL findings: pinned actions, token scopes, TLS identity, private temp files - Pin the third-party actions in build.yml and release.yml to commit SHAs and declare the GITHUB_TOKEN permissions each job needs. - AMSendMail asks Jakarta Mail to check the SMTP server identity on SSL. - AMSetupUtils.getRemoteServerInfo keeps the JDK host name verification: the admin password is posted over that connection. - The API docs, the NT credentials file and the embedded Cassandra storage directory are created readable by the server's own account only. --- .github/workflows/build.yml | 10 +-- .github/workflows/deploy.yml | 3 + .github/workflows/release.yml | 22 ++++-- .../authentication/modules/nt/NT.java | 17 ++++- .../modules/nt/NTCredentialsFileTest.java | 53 ++++++++++++++ .../openam/cassandra/embedded/Server.java | 25 ++++++- .../test/java/ServerStorageDirectoryTest.java | 71 +++++++++++++++++++ .../core/rest/docs/api/ApiDocsService.java | 19 +++-- .../rest/docs/api/ApiDocsServiceTest.java | 49 +++++++++++++ .../java/com/iplanet/am/util/AMSendMail.java | 2 + .../com/sun/identity/setup/AMSetupUtils.java | 25 +++---- .../com/iplanet/am/util/AMSendMailTest.java | 9 +++ .../sun/identity/setup/AMSetupUtilsTest.java | 14 +++- 13 files changed, 282 insertions(+), 37 deletions(-) create mode 100644 openam-authentication/openam-auth-nt/src/test/java/com/sun/identity/authentication/modules/nt/NTCredentialsFileTest.java create mode 100644 openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java create mode 100644 openam-core-rest/src/test/java/org/forgerock/openam/core/rest/docs/api/ApiDocsServiceTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6be720a8d3..8ca96748ff 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,6 +22,8 @@ concurrency: cancel-in-progress: true jobs: build-maven: + permissions: + contents: read runs-on: ${{ matrix.os }} strategy: matrix: @@ -97,7 +99,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 }} @@ -105,16 +107,16 @@ jobs: type=raw,value=latest 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: Prepare Dockerfile shell: bash run: sed -i -E '/^#COPY openam-(server|distribution)\//s/^#//' ./openam-distribution/openam-distribution-docker/Dockerfile - name: Build image - uses: docker/build-push-action@v7 + uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 continue-on-error: true with: context: . diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index eb0945b04d..040e1d1fd7 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -12,6 +12,9 @@ concurrency: jobs: deploy-maven: if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event=='push'}} + # contents: write - the docs are pushed to the wiki with github.token + permissions: + contents: write runs-on: 'ubuntu-latest' steps: - name: Print github context diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ddeb2a1870..a5d0519599 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,6 +16,10 @@ concurrency: cancel-in-progress: false jobs: release-maven: + # contents: write - release:prepare pushes the tag, the GitHub release is created + # and the docs are pushed to the wiki, all with github.token + permissions: + contents: write runs-on: 'ubuntu-latest' steps: - name: Print github context @@ -56,7 +60,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 clean 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 }} @@ -113,6 +117,10 @@ jobs: git push --quiet --force origin ${TAG_NAME} release-docker: + # packages: write - the image is pushed to GHCR with GITHUB_TOKEN + permissions: + contents: read + packages: write runs-on: 'ubuntu-latest' needs: - release-maven @@ -124,7 +132,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 }} @@ -133,22 +141,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: ./openam-distribution/openam-distribution-docker diff --git a/openam-authentication/openam-auth-nt/src/main/java/com/sun/identity/authentication/modules/nt/NT.java b/openam-authentication/openam-auth-nt/src/main/java/com/sun/identity/authentication/modules/nt/NT.java index ce68cabc36..d178880ebe 100644 --- a/openam-authentication/openam-auth-nt/src/main/java/com/sun/identity/authentication/modules/nt/NT.java +++ b/openam-authentication/openam-auth-nt/src/main/java/com/sun/identity/authentication/modules/nt/NT.java @@ -28,6 +28,7 @@ /* * Portions Copyrighted [2011] [ForgeRock AS] + * Portions Copyrighted 2026 3A Systems LLC. */ package com.sun.identity.authentication.modules.nt; @@ -50,7 +51,9 @@ import java.io.FileOutputStream; import java.io.InputStreamReader; import java.io.OutputStreamWriter; +import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.nio.file.Files; import java.security.Principal; import java.util.Map; import java.util.ResourceBundle; @@ -235,7 +238,7 @@ public int process(Callback[] callbacks, int state) File tmpFile = null; try { // Create the tmpFile - tmpFile = File.createTempFile(userName,"pwd"); + tmpFile = createCredentialsFile(); FileOutputStream fw = new FileOutputStream(tmpFile); OutputStreamWriter dos = new OutputStreamWriter(fw, "UTF-8"); dos.write("username = " + userName + "\n"); @@ -388,6 +391,16 @@ public void nullifyUsedVars() { host = null; domain = null; userName = null; - smbConfFileName = null; + smbConfFileName = null; + } + + /** + * The file the user name and password are handed to the authentication helper in. It sits in + * the shared temporary directory, so it is created readable by the server's own account only + * ({@code java.nio.file.Files} does that, unlike {@link File#createTempFile}); the caller + * deletes it once the helper has run. + */ + static File createCredentialsFile() throws IOException { + return Files.createTempFile("ntauth", ".pwd").toFile(); } } diff --git a/openam-authentication/openam-auth-nt/src/test/java/com/sun/identity/authentication/modules/nt/NTCredentialsFileTest.java b/openam-authentication/openam-auth-nt/src/test/java/com/sun/identity/authentication/modules/nt/NTCredentialsFileTest.java new file mode 100644 index 0000000000..3bae11c54e --- /dev/null +++ b/openam-authentication/openam-auth-nt/src/test/java/com/sun/identity/authentication/modules/nt/NTCredentialsFileTest.java @@ -0,0 +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]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.authentication.modules.nt; + +import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; +import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; +import static org.testng.Assert.assertEquals; + +import java.io.File; +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.util.EnumSet; + +import org.testng.SkipException; +import org.testng.annotations.Test; + +/** + * Kept apart from {@link NTTest}: that class runs under PowerMock's class loader, which cannot + * instrument the file system classes this test touches. + */ +public class NTCredentialsFileTest { + + /** + * The file handed to the authentication helper carries the user's password; in the shared + * temporary directory it must be readable by the server's account alone. + */ + @Test + public void credentialsFileIsReadableByTheOwnerOnly() throws IOException { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + throw new SkipException("POSIX permissions are not available on this file system"); + } + File credentials = NT.createCredentialsFile(); + try { + assertEquals(Files.getPosixFilePermissions(credentials.toPath()), EnumSet.of(OWNER_READ, OWNER_WRITE)); + } finally { + credentials.delete(); + } + } +} diff --git a/openam-cassandra/openam-cassandra-embedded/src/main/java/org/openidentityplatform/openam/cassandra/embedded/Server.java b/openam-cassandra/openam-cassandra-embedded/src/main/java/org/openidentityplatform/openam/cassandra/embedded/Server.java index 732a68521f..2caaf6e159 100644 --- a/openam-cassandra/openam-cassandra-embedded/src/main/java/org/openidentityplatform/openam/cassandra/embedded/Server.java +++ b/openam-cassandra/openam-cassandra-embedded/src/main/java/org/openidentityplatform/openam/cassandra/embedded/Server.java @@ -12,6 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2019 Open Identity Platform Community. + * Portions Copyright 2026 3A Systems, LLC. */ package org.openidentityplatform.openam.cassandra.embedded; @@ -19,9 +20,14 @@ import java.io.Closeable; import java.io.File; import java.io.InputStream; +import java.nio.file.FileSystems; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; import java.util.Arrays; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; @@ -45,6 +51,23 @@ public class Server implements Runnable, Closeable { final private ExecutorService executor = Executors.newSingleThreadExecutor(); private CassandraDaemon cassandraDaemon; + /** + * The storage directory, created if it does not exist. It defaults to a fixed name under the + * shared temporary directory so that the embedded store survives a restart, which is why it + * is closed to every account but the server's own where the file system can express that. + */ + public static Path privateDirectory(Path path) throws java.io.IOException { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + return Files.createDirectories(path); + } + final EnumSet ownerOnly = EnumSet.of(PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE); + Files.createDirectories(path, PosixFilePermissions.asFileAttribute(ownerOnly)); + // createDirectories applies the attribute only to what it creates. + Files.setPosixFilePermissions(path, ownerOnly); + return path; + } + public void run() { try { //check for external cassandra settings @@ -54,7 +77,7 @@ public void run() { //config final File path=new File(System.getProperty("cassandra.storagedir",System.getProperty("java.io.tmpdir")+File.separator+"embeddedCassandra")); - path.mkdirs(); + privateDirectory(path.toPath()); System.setProperty("cassandra-foreground", "true"); System.setProperty("cassandra.storagedir", path.getPath()); //prepare default keystore diff --git a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java new file mode 100644 index 0000000000..0275db9b4c --- /dev/null +++ b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java @@ -0,0 +1,71 @@ +/* + * 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. + */ +import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE; +import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; +import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.EnumSet; + +import org.junit.Before; +import org.junit.Test; +import org.openidentityplatform.openam.cassandra.embedded.Server; + +/** + * The embedded store keeps its data under the shared temporary directory by default; + * the directory has to be the server account's alone. + */ +public class ServerStorageDirectoryTest { + + private Path base; + + @Before + public void posixOnly() throws IOException { + assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + base = Files.createTempDirectory("embedded-cassandra-test"); + } + + @Test + public void storageDirectoryIsCreatedForTheOwnerOnly() throws IOException { + Path storage = base.resolve("embeddedCassandra"); + + Server.privateDirectory(storage); + + assertTrue(Files.isDirectory(storage)); + assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE), Files.getPosixFilePermissions(storage)); + } + + @Test + public void existingStorageDirectoryKeepsItsDataAndIsClosedToOthers() throws IOException { + Path storage = Files.createDirectory(base.resolve("embeddedCassandra")); + Files.write(storage.resolve("data"), "kept".getBytes(StandardCharsets.UTF_8)); + Files.setPosixFilePermissions(storage, EnumSet.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE, + java.nio.file.attribute.PosixFilePermission.OTHERS_READ, + java.nio.file.attribute.PosixFilePermission.OTHERS_EXECUTE)); + + Server.privateDirectory(storage); + + assertEquals("kept", new String(Files.readAllBytes(storage.resolve("data")), StandardCharsets.UTF_8)); + assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE), Files.getPosixFilePermissions(storage)); + } +} diff --git a/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/docs/api/ApiDocsService.java b/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/docs/api/ApiDocsService.java index c24a4e807d..eeff156760 100644 --- a/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/docs/api/ApiDocsService.java +++ b/openam-core-rest/src/main/java/org/forgerock/openam/core/rest/docs/api/ApiDocsService.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2016 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package org.forgerock.openam.core.rest.docs.api; @@ -156,8 +156,7 @@ public Response handle(@Contextual Request request) { } private File getDocs(File asciidoc) throws IOException { - File docs = File.createTempFile("openam-api.", ".html"); - docs.deleteOnExit(); + File docs = privateTempFile("openam-api.", ".html"); try (Reader reader = new FileReader(asciidoc); Writer writer = new FileWriter(docs)) { asciidoctor.convert( reader, @@ -177,12 +176,22 @@ private File getDocs(File asciidoc) throws IOException { private File getAsciiDoc(ApiDescription description) throws IOException { String asciiDocMarkup = ApiDocGenerator.execute("OpenAM API", description, null); - File asciidoc = File.createTempFile("openam-api.", ".asciidoc"); - asciidoc.deleteOnExit(); + File asciidoc = privateTempFile("openam-api.", ".asciidoc"); Files.write(asciiDocMarkup, asciidoc, StandardCharsets.UTF_8); return asciidoc; } + /** + * A temporary file in the shared temporary directory that only the server's own account can + * read ({@code java.nio.file.Files} creates it with owner-only permissions, unlike + * {@link File#createTempFile}), deleted when the JVM exits. + */ + static File privateTempFile(String prefix, String suffix) throws IOException { + File file = java.nio.file.Files.createTempFile(prefix, suffix).toFile(); + file.deleteOnExit(); + return file; + } + private ApiDescription getDescription() { return rootRouter.handleApiRequest(new RootContext(), newApiRequest(ResourcePath.empty())); } diff --git a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/docs/api/ApiDocsServiceTest.java b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/docs/api/ApiDocsServiceTest.java new file mode 100644 index 0000000000..714bdbbcbd --- /dev/null +++ b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/docs/api/ApiDocsServiceTest.java @@ -0,0 +1,49 @@ +/* + * 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.openam.core.rest.docs.api; + +import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; +import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; +import static org.testng.Assert.assertEquals; + +import java.io.File; +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.util.EnumSet; + +import org.testng.SkipException; +import org.testng.annotations.Test; + +public class ApiDocsServiceTest { + + /** + * The generated documentation sits in the shared temporary directory until it is served; + * nothing but the server's own account may read it there. + */ + @Test + public void temporaryDocsAreReadableByTheOwnerOnly() throws IOException { + if (!FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + throw new SkipException("POSIX permissions are not available on this file system"); + } + File docs = ApiDocsService.privateTempFile("openam-api.", ".html"); + try { + assertEquals(Files.getPosixFilePermissions(docs.toPath()), EnumSet.of(OWNER_READ, OWNER_WRITE)); + } finally { + docs.delete(); + } + } +} diff --git a/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java b/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java index 384b9beab7..a8dcedadd1 100644 --- a/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java +++ b/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java @@ -231,6 +231,8 @@ public void postMail(String recipients[], String subject, String message, moduleProps.put("mail.smtp.socketFactory.port", port); if (ssl) { moduleProps.put("mail.smtp.ssl.enable", "true"); + // The certificate has to be the SMTP host's, not merely one the JVM trusts. + moduleProps.put("mail.smtp.ssl.checkserveridentity", "true"); } moduleProps.put("mail.smtp.socketFactory.fallback", "false"); diff --git a/openam-core/src/main/java/com/sun/identity/setup/AMSetupUtils.java b/openam-core/src/main/java/com/sun/identity/setup/AMSetupUtils.java index 0a0db8a53f..08818dc1f3 100644 --- a/openam-core/src/main/java/com/sun/identity/setup/AMSetupUtils.java +++ b/openam-core/src/main/java/com/sun/identity/setup/AMSetupUtils.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2015 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package com.sun.identity.setup; @@ -20,10 +20,7 @@ import static org.forgerock.openam.utils.IOUtils.closeIfNotNull; import static org.forgerock.openam.utils.IOUtils.readStream; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLHandshakeException; -import javax.net.ssl.SSLSession; import jakarta.servlet.ServletContext; import java.io.FileNotFoundException; import java.io.IOException; @@ -54,7 +51,6 @@ public final class AMSetupUtils { private static final Debug debug = Debug.getInstance(SetupConstants.DEBUG_NAME); - private static final String HTTPS = "https"; private static final String RANDOM_STRING_ALGORITHM = "SHA1PRNG"; private AMSetupUtils() { @@ -228,18 +224,13 @@ public static Map getRemoteServerInfo(String serverUrl, String u } } - private static HttpURLConnection openConnection(String urlString) throws IOException { - URL url = new URL(urlString); - HttpURLConnection connection = (HttpURLConnection) url.openConnection(); - if (url.getProtocol().equals(HTTPS)) { - HttpsURLConnection sslConnection = (HttpsURLConnection) connection; - sslConnection.setHostnameVerifier(new HostnameVerifier() { - public boolean verify(String hostname, SSLSession session) { - return true; - } - }); - } - return connection; + /** + * Opens a connection to the remote server. The admin password is posted over it, so an + * HTTPS connection keeps the JDK's host name verification: the certificate has to be the + * host's, not merely one the JVM trusts. + */ + static HttpURLConnection openConnection(String urlString) throws IOException { + return (HttpURLConnection) new URL(urlString).openConnection(); } private static void writeToConnection(URLConnection connection, String data) throws IOException { diff --git a/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java b/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java index 9f8eeec981..437b60ba84 100644 --- a/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java +++ b/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java @@ -206,6 +206,15 @@ public void shouldNotLetTheConfiguredSubjectIntroduceAHeader() throws Exception assertThat(serialized).contains("attacker@example.org"); } + /** An SSL connection to the SMTP host has to check that the certificate is the host's, not just any valid one. */ + @Test + public void shouldCheckTheServerIdentityOnAnSslConnection() throws Exception { + new AMSendMail().postMail(new String[] {TO}, "Password Reset", BODY, FROM, "text/plain", + "UTF-8", "smtp.example.com", "465", "openam", "secret", true); + + assertThat(sentMessage().getSession().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true"); + } + /** * The recipient is the one field of a self service registration the anonymous caller still fills in himself. * A break outside the quotes is rejected by the address parser, but one in the quoted display name goes diff --git a/openam-core/src/test/java/com/sun/identity/setup/AMSetupUtilsTest.java b/openam-core/src/test/java/com/sun/identity/setup/AMSetupUtilsTest.java index 6c27242db0..3c0c491d45 100644 --- a/openam-core/src/test/java/com/sun/identity/setup/AMSetupUtilsTest.java +++ b/openam-core/src/test/java/com/sun/identity/setup/AMSetupUtilsTest.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2015 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package com.sun.identity.setup; @@ -27,6 +27,8 @@ import java.io.IOException; import java.io.InputStream; +import java.net.HttpURLConnection; +import javax.net.ssl.HttpsURLConnection; import org.forgerock.openam.utils.IOUtils; import org.testng.annotations.Test; @@ -132,4 +134,14 @@ public void shouldGetFirstUnusedPort() { //Then assertThat(unusedPort).isBetween(10, 65535); } + + @Test + public void shouldKeepTheDefaultHostNameVerifierForARemoteServerOverHttps() throws IOException { + // getRemoteServerInfo posts the admin password to that server, so its certificate has to be + // the host's, not merely one the JVM trusts. + HttpURLConnection connection = openConnection("https://openam.example.com/openam/getServerInfo.jsp"); + + assertThat(((HttpsURLConnection) connection).getHostnameVerifier()) + .isSameAs(HttpsURLConnection.getDefaultHostnameVerifier()); + } } From d74590bc797f4e6c24391ec36f7f8b687a7a6868 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 13:56:27 +0300 Subject: [PATCH 2/5] Let Dependabot move the pinned action SHAs The third-party actions are pinned to commit SHAs, so nothing would update them; a github-actions Dependabot configuration does, keeping the version comment in step, one grouped pull request a week. --- .github/dependabot.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000000..fd9eaea4c8 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,25 @@ +# 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: + # The third-party actions in the workflows are pinned to commit SHAs; Dependabot is what + # moves a pin (and its version comment) forward when the action releases. + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + groups: + github-actions: + patterns: + - "*" From 03ddd2c929222bb3678e881a26bea6d3a7319fb5 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 19:06:29 +0300 Subject: [PATCH 3/5] Label the dependabot GitHub Actions PRs like OpenIG does Add "ci"/"dependencies" labels and the grouping comment to .github/dependabot.yml, matching OpenIdentityPlatform/OpenIG#170. --- .github/dependabot.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index fd9eaea4c8..9e70c213a4 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -20,6 +20,10 @@ updates: 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 59890a19d007a0e476c34fbffe292d7e46a7bfbb Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 29 Sep 2026 12:00:07 +0300 Subject: [PATCH 4/5] Let deployments turn the SMTP server identity check off; pin the temp-file call sites - org.openidentityplatform.openam.smtp.checkServerIdentity (default true) turns mail.smtp.ssl.checkserveridentity off again for an SMTP host its certificate does not name; registered in validserverconfig and described in the Configuration Reference. - ServerTest checks the storage directory run() leaves behind; ServerStorageDirectoryTest pins the creation attribute on missing parents and the refusal of a directory owned by another account, and cleans up after itself. --- .../test/java/ServerStorageDirectoryTest.java | 37 ++++++++++++++++++- .../src/test/java/ServerTest.java | 15 +++++++- .../java/com/iplanet/am/util/AMSendMail.java | 6 ++- .../com/iplanet/am/util/AMSendMailTest.java | 20 +++++++++- .../asciidoc/reference/chap-config-ref.adoc | 18 +++++++++ .../config/validserverconfig.properties | 2 + .../com/sun/identity/shared/Constants.java | 8 ++++ 7 files changed, 101 insertions(+), 5 deletions(-) diff --git a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java index 0275db9b4c..ca562084ac 100644 --- a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java +++ b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerStorageDirectoryTest.java @@ -18,17 +18,22 @@ import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystemException; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.EnumSet; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.openidentityplatform.openam.cassandra.embedded.Server; /** @@ -37,12 +42,15 @@ */ public class ServerStorageDirectoryTest { + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + private Path base; @Before public void posixOnly() throws IOException { assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); - base = Files.createTempDirectory("embedded-cassandra-test"); + base = tmp.newFolder("embedded-cassandra-test").toPath(); } @Test @@ -55,6 +63,19 @@ public void storageDirectoryIsCreatedForTheOwnerOnly() throws IOException { assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE), Files.getPosixFilePermissions(storage)); } + /** + * The storage directory itself is set owner-only afterwards as well; a missing parent gets its + * permissions from the attribute it is created with and nothing else. + */ + @Test + public void missingParentsAreCreatedForTheOwnerOnly() throws IOException { + Path parent = base.resolve("parent"); + + Server.privateDirectory(parent.resolve("embeddedCassandra")); + + assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE), Files.getPosixFilePermissions(parent)); + } + @Test public void existingStorageDirectoryKeepsItsDataAndIsClosedToOthers() throws IOException { Path storage = Files.createDirectory(base.resolve("embeddedCassandra")); @@ -68,4 +89,18 @@ public void existingStorageDirectoryKeepsItsDataAndIsClosedToOthers() throws IOE assertEquals("kept", new String(Files.readAllBytes(storage.resolve("data")), StandardCharsets.UTF_8)); assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE), Files.getPosixFilePermissions(storage)); } + + /** + * A storage directory another account created first cannot be closed to that account, so the + * store must not start in it. The root directory stands in for it; the test is skipped where the + * account could write to it, since root would then really change its permissions. + */ + @Test(expected = FileSystemException.class) + public void aDirectoryOwnedByAnotherAccountIsRefused() throws IOException { + Path root = Paths.get("/"); + assumeFalse("root".equals(System.getProperty("user.name"))); + assumeFalse(Files.isWritable(root)); + + Server.privateDirectory(root); + } } diff --git a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java index 628e3caa36..3755643670 100644 --- a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java +++ b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java @@ -16,6 +16,15 @@ */ +import static org.junit.Assert.assertEquals; +import static org.junit.Assume.assumeTrue; + +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; + import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -41,9 +50,13 @@ public static void destory() throws IdRepoException{ } @Test - public void start_test() throws IdRepoException{ + public void start_test() throws IdRepoException, IOException{ cassandra=new Server(); cassandra.run(); + // run() is what closes the storage directory to other accounts, not only privateDirectory. + assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + assertEquals(PosixFilePermissions.fromString("rwx------"), + Files.getPosixFilePermissions(Paths.get(System.getProperty("cassandra.storagedir")))); } } diff --git a/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java b/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java index a8dcedadd1..8192fbe048 100644 --- a/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java +++ b/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java @@ -231,8 +231,10 @@ public void postMail(String recipients[], String subject, String message, moduleProps.put("mail.smtp.socketFactory.port", port); if (ssl) { moduleProps.put("mail.smtp.ssl.enable", "true"); - // The certificate has to be the SMTP host's, not merely one the JVM trusts. - moduleProps.put("mail.smtp.ssl.checkserveridentity", "true"); + // The certificate has to be the SMTP host's, not merely one the JVM trusts, unless the + // deployment turns the check off for a host its certificate does not name. + moduleProps.put("mail.smtp.ssl.checkserveridentity", String.valueOf( + SystemProperties.getAsBoolean(Constants.AM_SMTP_CHECK_SERVER_IDENTITY, true))); } moduleProps.put("mail.smtp.socketFactory.fallback", "false"); diff --git a/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java b/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java index 437b60ba84..107fae138f 100644 --- a/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java +++ b/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java @@ -19,6 +19,8 @@ import java.io.ByteArrayOutputStream; +import com.sun.identity.shared.Constants; + import jakarta.mail.Message; import jakarta.mail.MessagingException; import jakarta.mail.Transport; @@ -42,7 +44,7 @@ * The tests drive the two {@code postMail} overloads that build a message, against a mocked transport, so that they * cover the sanitising the methods themselves do rather than a model of it. */ -@PrepareForTest({ Transport.class, AMSendMail.class, BrowserEncoding.class }) +@PrepareForTest({ Transport.class, AMSendMail.class, BrowserEncoding.class, SystemProperties.class }) // G11NSettings, which the charset mapping of BrowserEncoding is built on, needs a running server. @SuppressStaticInitializationFor("com.iplanet.am.util.BrowserEncoding") public class AMSendMailTest extends PowerMockTestCase { @@ -215,6 +217,22 @@ public void shouldCheckTheServerIdentityOnAnSslConnection() throws Exception { assertThat(sentMessage().getSession().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true"); } + /** + * A deployment whose SMTP host is not named in its certificate - an IP address, a short name, a relay alias - + * can turn the check off again rather than lose all its mail. + */ + @Test + public void shouldLeaveTheServerIdentityUncheckedWhenTheDeploymentTurnsItOff() throws Exception { + PowerMockito.spy(SystemProperties.class); + PowerMockito.doReturn(false).when(SystemProperties.class, "getAsBoolean", + Constants.AM_SMTP_CHECK_SERVER_IDENTITY, true); + + new AMSendMail().postMail(new String[] {TO}, "Password Reset", BODY, FROM, "text/plain", + "UTF-8", "10.0.0.25", "465", "openam", "secret", true); + + assertThat(sentMessage().getSession().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("false"); + } + /** * The recipient is the one field of a self service registration the anonymous caller still fills in himself. * A break outside the quotes is rejected by the address parser, but one in the quoted display name goes diff --git a/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc b/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc index aca1cc5ff4..314a68b71a 100644 --- a/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc +++ b/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc @@ -1903,6 +1903,9 @@ Specifies whether to connect to the SMTP mail server using SSL. + Default: use SSL (`true`) ++ +Over SSL, the server certificate has to name the mail server host; see `org.openidentityplatform.openam.smtp.checkServerIdentity` in xref:#servers-advanced-configuration["Advanced"]. + + `ssoadm` attribute: `forgerockEmailServiceSMTPSSLEnabled` @@ -6117,6 +6120,21 @@ Specify a semicolon-separated list of URL patterns, using `*` as a wildcard, for + Default: empty (only relative and same-origin redirect URLs are allowed) +`org.openidentityplatform.openam.smtp.checkServerIdentity`:: +Controls whether OpenAM checks, when it connects to an SMTP mail server over SSL, that the server certificate names the configured mail server host. + ++ +By default (`true`), a certificate that the JVM trusts but that is issued for another host is rejected, and sending mail fails with `Can't verify identity of server`. This affects every mail OpenAM sends over SSL: password reset, account lockout notification, the Email Service, and one-time passwords sent by email. Earlier releases did not perform this check. + ++ +To pass the check, configure the mail server host as a name the certificate carries in its subject alternative names or common name. A host given as an IP address matches an IP address subject alternative name only when the JVM is started with `--add-exports java.base/sun.security.util=ALL-UNNAMED`, as the OpenAM Docker image is; otherwise use a host name. + ++ +Set this property to `false` to accept any certificate the JVM trusts, as earlier releases did, for example while a mail relay's certificate is being reissued. + ++ +Default: `true` + `securidHelper.ports`:: Port on which SecurID daemon listens. diff --git a/openam-server-only/src/main/resources/config/validserverconfig.properties b/openam-server-only/src/main/resources/config/validserverconfig.properties index d3ae6aca25..74acd0f652 100644 --- a/openam-server-only/src/main/resources/config/validserverconfig.properties +++ b/openam-server-only/src/main/resources/config/validserverconfig.properties @@ -25,6 +25,7 @@ # $Id: validserverconfig.properties,v 1.25 2010/01/26 19:27:38 exu Exp $ # # Portions Copyrighted 2010-2016 ForgeRock AS. +# Portions Copyrighted 2026 3A Systems, LLC. com.sun.identity.saml.xmlsig.certalias= am.encryption.pwd= @@ -92,6 +93,7 @@ com.iplanet.am.session.invalidsessionmaxtime=integer com.iplanet.am.session.protectedPropertiesList= com.iplanet.am.smtphost= com.iplanet.am.smtpport=integer +org.openidentityplatform.openam.smtp.checkServerIdentity=true,false com.iplanet.am.stats.interval=integer com.iplanet.am.util.xml.validating=on,off com.iplanet.am.version= diff --git a/openam-shared/src/main/java/com/sun/identity/shared/Constants.java b/openam-shared/src/main/java/com/sun/identity/shared/Constants.java index ee54cff93b..657a995885 100644 --- a/openam-shared/src/main/java/com/sun/identity/shared/Constants.java +++ b/openam-shared/src/main/java/com/sun/identity/shared/Constants.java @@ -393,6 +393,14 @@ public interface Constants { */ static final String SM_SMTP_PORT = "com.iplanet.am.smtpport"; + /** + * Property string controlling whether an SSL connection to the SMTP host checks that the server + * certificate names that host. Defaults to {@code true}; {@code false} accepts any certificate the + * JVM trusts, as earlier releases did. + */ + static final String AM_SMTP_CHECK_SERVER_IDENTITY = + "org.openidentityplatform.openam.smtp.checkServerIdentity"; + /** * Property string for CDSSO cookie domain. */ From 587492f492ee7fabefc1eaa89d173f4e6a7fdbfa Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 29 Sep 2026 14:44:06 +0300 Subject: [PATCH 5/5] Keep the SMTP identity check unless it is explicitly off; pin ServerTest on a fresh storage directory - AMSendMail turns mail.smtp.ssl.checkserveridentity off only for an explicit "false": the server property validator lets an empty value through, and Boolean.parseBoolean("") would have turned the check off silently. shouldKeepTheServerIdentityCheckForAnEmptyValue pins it. - The Configuration Reference says an IP-address mail host matches an IP SAN on Java 11 as is, and needs --add-exports only on Java 16 and later. - ServerTest points cassandra.storagedir at a new directory under target, so an embeddedCassandra an earlier run left in java.io.tmpdir cannot hide a revert of the run() call site. --- .../src/test/java/ServerTest.java | 7 ++++++- .../java/com/iplanet/am/util/AMSendMail.java | 7 ++++--- .../com/iplanet/am/util/AMSendMailTest.java | 21 +++++++++++++++++-- .../asciidoc/reference/chap-config-ref.adoc | 4 ++-- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java index 3755643670..197e4a29c7 100644 --- a/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java +++ b/openam-cassandra/openam-cassandra-embedded/src/test/java/ServerTest.java @@ -38,7 +38,12 @@ public class ServerTest { static Server cassandra; @BeforeClass - public static void init() throws IdRepoException{ + public static void init() throws IdRepoException, IOException{ + // A storage directory that does not exist yet, so the assertion sees what run() creates rather than what + // an earlier run left behind; under target, so that the data the daemon writes goes with mvn clean. + Files.createDirectories(Paths.get("target")); + System.setProperty("cassandra.storagedir", Files.createTempDirectory(Paths.get("target"), "server-test") + .resolve("embeddedCassandra").toAbsolutePath().toString()); System.setProperty("datastax-java-driver.advanced.auth-provider.class","PlainTextAuthProvider"); System.setProperty("datastax-java-driver.advanced.auth-provider.username","cassandra"); System.setProperty("datastax-java-driver.advanced.auth-provider.password","cassandra"); diff --git a/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java b/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java index 8192fbe048..fdb7f7b8ce 100644 --- a/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java +++ b/openam-core/src/main/java/com/iplanet/am/util/AMSendMail.java @@ -232,9 +232,10 @@ public void postMail(String recipients[], String subject, String message, if (ssl) { moduleProps.put("mail.smtp.ssl.enable", "true"); // The certificate has to be the SMTP host's, not merely one the JVM trusts, unless the - // deployment turns the check off for a host its certificate does not name. - moduleProps.put("mail.smtp.ssl.checkserveridentity", String.valueOf( - SystemProperties.getAsBoolean(Constants.AM_SMTP_CHECK_SERVER_IDENTITY, true))); + // deployment turns the check off for a host its certificate does not name. Only an + // explicit "false" does: an empty or unrecognised value keeps the check. + moduleProps.put("mail.smtp.ssl.checkserveridentity", String.valueOf(!"false".equalsIgnoreCase( + SystemProperties.get(Constants.AM_SMTP_CHECK_SERVER_IDENTITY, "true").trim()))); } moduleProps.put("mail.smtp.socketFactory.fallback", "false"); diff --git a/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java b/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java index 107fae138f..e43a2c71cb 100644 --- a/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java +++ b/openam-core/src/test/java/com/iplanet/am/util/AMSendMailTest.java @@ -224,8 +224,8 @@ public void shouldCheckTheServerIdentityOnAnSslConnection() throws Exception { @Test public void shouldLeaveTheServerIdentityUncheckedWhenTheDeploymentTurnsItOff() throws Exception { PowerMockito.spy(SystemProperties.class); - PowerMockito.doReturn(false).when(SystemProperties.class, "getAsBoolean", - Constants.AM_SMTP_CHECK_SERVER_IDENTITY, true); + PowerMockito.doReturn("false").when(SystemProperties.class, "get", + Constants.AM_SMTP_CHECK_SERVER_IDENTITY); new AMSendMail().postMail(new String[] {TO}, "Password Reset", BODY, FROM, "text/plain", "UTF-8", "10.0.0.25", "465", "openam", "secret", true); @@ -233,6 +233,23 @@ public void shouldLeaveTheServerIdentityUncheckedWhenTheDeploymentTurnsItOff() t assertThat(sentMessage().getSession().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("false"); } + /** + * The server property validator accepts an empty value, so an Advanced tab entry left blank reaches the check as + * "". Only an explicit "false" turns it off: anything else keeps it, rather than quietly accepting any certificate + * the JVM trusts while mail keeps flowing. + */ + @Test + public void shouldKeepTheServerIdentityCheckForAnEmptyValue() throws Exception { + PowerMockito.spy(SystemProperties.class); + PowerMockito.doReturn("").when(SystemProperties.class, "get", + Constants.AM_SMTP_CHECK_SERVER_IDENTITY); + + new AMSendMail().postMail(new String[] {TO}, "Password Reset", BODY, FROM, "text/plain", + "UTF-8", "smtp.example.com", "465", "openam", "secret", true); + + assertThat(sentMessage().getSession().getProperty("mail.smtp.ssl.checkserveridentity")).isEqualTo("true"); + } + /** * The recipient is the one field of a self service registration the anonymous caller still fills in himself. * A break outside the quotes is rejected by the address parser, but one in the quoted display name goes diff --git a/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc b/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc index 314a68b71a..7270569126 100644 --- a/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc +++ b/openam-documentation/openam-doc-source/src/main/asciidoc/reference/chap-config-ref.adoc @@ -6127,10 +6127,10 @@ Controls whether OpenAM checks, when it connects to an SMTP mail server over SSL By default (`true`), a certificate that the JVM trusts but that is issued for another host is rejected, and sending mail fails with `Can't verify identity of server`. This affects every mail OpenAM sends over SSL: password reset, account lockout notification, the Email Service, and one-time passwords sent by email. Earlier releases did not perform this check. + -To pass the check, configure the mail server host as a name the certificate carries in its subject alternative names or common name. A host given as an IP address matches an IP address subject alternative name only when the JVM is started with `--add-exports java.base/sun.security.util=ALL-UNNAMED`, as the OpenAM Docker image is; otherwise use a host name. +To pass the check, configure the mail server host as a name the certificate carries in its subject alternative names or common name. A host given as an IP address matches an IP address subject alternative name on Java 11 as is, and on Java 16 and later only when the JVM is started with `--add-exports java.base/sun.security.util=ALL-UNNAMED`, as the OpenAM Docker image is; otherwise use a host name. + -Set this property to `false` to accept any certificate the JVM trusts, as earlier releases did, for example while a mail relay's certificate is being reissued. +Set this property to `false` to accept any certificate the JVM trusts, as earlier releases did, for example while a mail relay's certificate is being reissued. Only `false` turns the check off; an empty or any other value keeps it. + Default: `true`