Harden GitHub workflows and SMTP STARTTLS trust

[vharseko]

Summary

Closes 30 of the 31 open medium CodeQL alerts: the 29 GitHub Actions findings and java/insecure-smtp-ssl #22. The remaining one (#4, ResourceServlet redirect) touches a file that #202 is already changing and will follow once that PR lands.

GitHub Actions

actions/missing-workflow-permissions (#711–#716, #920, #921) — every workflow now starts from permissions: contents: read; only the jobs that actually use the token get more, each with an inline comment saying why:

Job	Permissions	Reason
build.yml (all jobs)	contents: read	artifacts and the local registry:2 service need nothing else
deploy.yml / deploy-maven	contents: write	pushes the generated docs to the repository wiki with github.token (the doc-site push uses a PAT)
release.yml / release-maven	contents: write	release:prepare pushes the release tag, action-gh-release creates the release, docs go to the wiki
release.yml / release-docker*	contents: read, packages: write	GHCR login with GITHUB_TOKEN

actions/unpinned-tag (#718–#738) — the six third-party actions are pinned to commit SHAs, with the resolved version kept as a comment:

docker/metadata-action v6.2.0 · docker/setup-qemu-action v4.4.0 · docker/setup-buildx-action v4.4.1 · docker/build-push-action v7.4.0 · docker/login-action v4.6.0 · softprops/action-gh-release v3.0.3

actions/* and github/* are not covered by the rule and stay on major tags.

Second commit adds .github/dependabot.yml (ported from OpenIdentityPlatform/OpenIG#170) with the github-actions ecosystem, grouped into one weekly PR, so the pinned SHAs stay current. All six pins above were verified against the actions' latest releases at the time of writing and are already up to date.

#22 — EmailClient trusted every SMTP certificate over STARTTLS

The "temporary hack to avoid cert check" installed a MailSSLSocketFactory with setTrustAllHosts(true) whenever starttls.enable was set, so the TLS upgrade gave no protection against an on-path attacker. By default the certificate must now chain to the JVM trust store and be issued for the configured host (mail.smtp.ssl.checkserveridentity=true, which JavaMail 1.4.7 leaves off). Two optional settings relax it (documented in the integrator's guide):

"starttls" : {
    "enable" : true,
    "trustedHosts" : [ "smtp.internal.example.com" ],   // accepted without validation; `host` must be listed exactly
    "trustAll" : false                                   // dev only; logs a warning when true; wins over trustedHosts
}

Once trustedHosts is set, JavaMail accepts only a host found in the list (case-sensitive) and rejects any other, so EmailClient logs a warning when the configured host is missing from it.

javax.mail is embedded in the bundle (review rounds 1–2). bundle/ ships both javax.mail:mail:1.4.7 and com.sun.mail:jakarta.mail:2.0.2, and both contain com.sun.mail.*. The javax.mail bundle imports its own com.sun.mail.* packages with version="1.4" and no upper bound, so Felix wires them to jakarta.mail 2.0.2 and drops the 1.4.7 exports. This already breaks mail on master: MimeMessage fails with NoSuchMethodError on com.sun.mail.util.PropUtil, and external/email?_action=send returns 500. Before this fix, MailSSLSocketFactory came from jakarta.mail, so javax.mail's SocketFetcher did not recognise it, and a non-empty trustedHosts would have trusted every host. Now openidm-external-email embeds javax.mail 1.4.7 (Embed-Dependency) and imports neither javax.mail nor com.sun.mail, so Session, SMTPTransport, SocketFetcher and MailSSLSocketFactory all come from one jar. While JavaMail looks up providers, EmailClient points the context class loader at the bundle. jakarta.mail is excluded from this module's classpath and still ships in bundle/ through the other modules.

STARTTLS protocols: with no mail.smtp.ssl.protocols set, JavaMail 1.4.7 enables only TLSv1 on the STARTTLS socket, and current JDKs disable it, so no handshake could succeed. EmailClient now sets the property to the JVM's default protocols.

Admin UI: saving Settings > Email now keeps starttls.trustedHosts / starttls.trustAll, which the form does not edit. Before, the shallow merge replaced the stored starttls object with {enable}.

Behaviour change: deployments that use STARTTLS against an SMTP server with a self-signed or otherwise untrusted certificate, or with a certificate that does not match host (for example a relay addressed by IP), will fail to send mail until they either fix the certificate / trust store or set trustedHosts / trustAll.

Test plan

• New EmailClientTest (7): default → no custom socket factory and host check on; trustAll → trust-all factory; trustedHosts → limited factory, no host check; trustAll wins over trustedHosts; STARTTLS uses the JVM's default TLS protocols; MailSSLSocketFactory comes from the same jar as javax.mail.Session; no STARTTLS → nothing configured. The host-check, protocols and socket-factory-origin cases fail without the fixes
• openidm-external-email suite green (12/12)
• Built bundle embeds mail-1.4.7.jar (Bundle-ClassPath: .,mail-1.4.7.jar) and imports neither javax.mail nor com.sun.mail
• Runtime on OpenIDM (OSGi), against a local STARTTLS SMTP server with a self-signed certificate for localhost: startup reaches "ready" with a clean log; trustedHosts: ["localhost"] delivers over TLS; a host missing from trustedHosts is rejected ("Server is not trusted") and the warning is logged; default settings reject the certificate. The same install with the master bundle returns 500 (NoSuchMethodError)
• Same javax.mail jar outside OSGi: trustAll delivers; a case-mismatched trustedHosts entry is rejected; with the test CA trusted, localhost delivers and 127.0.0.1 is rejected by the host name check
• EmailConfigView.save() merge logic checked in isolation; the Admin UI was not built locally, so lint is left to CI
• Workflow YAML parses; permission layout verified per job
• CodeQL on this PR closes #711–#716, #718–#738, #920, #921, [#10 #14] FIX openidm (object) is not defined (rhino export global context) #22
• Next release run confirms contents: write / packages: write are sufficient (the token had full default permissions before)

[maximthomas]

praise: The trust-all default is gone, and the GitHub token is narrowed job by job.

• EmailClient.configureStartTlsTrust installs no socket factory by default, and a GeneralSecurityException now surfaces as IllegalStateException instead of being swallowed.
• Every workflow starts from permissions: contents: read. In release.yml only the jobs that push get contents: write / packages: write, each with a comment saying why.
• Third-party actions are pinned to commit SHAs with the version as a comment, and .github/dependabot.yml keeps them current.

---

issue (blocking): With STARTTLS on and no relaxation set, the server certificate's chain is validated but its host name is not, so CodeQL #22 stays open.

openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java:115-117, :126

javax.mail 1.4.7 reads mail.smtp.ssl.checkserveridentity with default false and sets no endpoint identification algorithm, so JSSE checks only the chain, and nothing in the tree sets the flag. A man-in-the-middle with any publicly trusted certificate for a name they control is accepted for host and receives the SMTP AUTH credentials and the mail. The CodeQL predicate (Mail.qll, isInsecureMailPropertyConfig) matches any constant %.socketFactory% key on props, and line 126 still puts one. Only a constant mail.smtp.ssl.checkserveridentity = "true" on the same variable clears it. The predicate is flow-insensitive, so the alert moves from line 96 to line 103 instead of closing. If the check was left off on purpose (relays addressed by IP or localhost), trustedHosts already covers that case.

        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;
        }

Pin: in startTlsValidatesTheServerCertificateByDefault, assertThat(props.get("mail.smtp.ssl.checkserveridentity")).isEqualTo("true"); is red at this head. Add a sentence to chap-mail.adoc saying that the certificate must also match host.

---

issue (blocking): Saving Settings > Email in the Admin UI deletes starttls.trustedHosts and starttls.trustAll from the stored config.

openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/settings/EmailConfigView.js:134-140, openidm-ui/openidm-ui-admin/src/main/resources/templates/admin/settings/EmailConfigTemplate.html:57

save() merges the form into the config with the shallow _.extend(this.data.config, formData). The form's only STARTTLS field is starttls.enable, so formData.starttls is {enable: true} and replaces the whole stored object. Only auth.password is restored by hand. An operator who follows the behaviour-change note and adds trustedHosts for a self-signed relay loses it on the next UI save, a port change for example. After that every send fails the handshake, and nothing points at the UI.

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

---

suggestion (non-blocking): Document that trustedHosts must contain the configured host exactly as written. A host missing from the list is not validated normally: JavaMail rejects it.

openidm-external-email/src/main/java/org/forgerock/openidm/external/email/impl/EmailClient.java:124, openidm-doc/src/main/asciidoc/integrators-guide/chap-mail.adoc:130

Once the list is set, MailSSLSocketFactory skips the trust-store check for every host. SocketFetcher then accepts only a host found in the list (case-sensitive match) and otherwise throws "Server is not trusted: ". The doc ("accepted without validation") and the constant's Javadoc imply that unlisted hosts get normal validation. With host: "mail.example.com" and trustedHosts: ["MAIL.example.com"] (or an IP), sending fails even though the certificate is valid. One caveat, not run: this bundle imports com.sun.mail.util from jakarta.mail 2.0.2. If OSGi wires javax.mail 1.4.7's SocketFetcher to its own copy of that package, its instanceof check fails and an unlisted host is accepted with no check at all.

        String host = props.getProperty("mail.smtp.host");
        if (!trustAll && !trustedHosts.contains(host)) {
            logger.warn("starttls.trustedHosts {} does not contain the SMTP host {}", trustedHosts, host);
        }

---

suggestion (non-blocking): No case sets trustAll and trustedHosts together, so no test covers which one takes precedence (EmailClient.java:120-125).

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

If the branches are swapped so that trustedHosts is tested first, all four cases stay green. Yet {trustAll: true, trustedHosts: [...]} then no longer accepts every certificate, which contradicts chap-mail.adoc ("trustAll—when true, accepts any server certificate"). I traced this mutant against the four cases by reading; it was not executed.

    @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();
    }

Pin: this case goes red when the branches are swapped, because isTrustAllHosts() is false there.

[vharseko]

@maximthomas all four points are addressed in 1fe86c6 (the branch is also rebased onto the current master).

1. Host name check. With STARTTLS on and no relaxation set, EmailClient now puts mail.smtp.ssl.checkserveridentity = "true", so the certificate must be issued for host as well as chain to the JVM trust store. startTlsValidatesTheServerCertificateByDefault pins it, and it fails on the previous head. The trustedHosts path deliberately leaves the check off, because a self-signed relay certificate with a different CN would not pass it. chap-mail.adoc now says that the certificate must match host.

2. Admin UI. EmailConfigView.save() merges formData.starttls over the stored starttls object before the shallow _.extend, so trustedHosts and trustAll survive a save. Unchecking STARTTLS still drops the whole object, as before.

3. trustedHosts / OSGi. Your caveat holds, and it was worse than an unlisted host slipping through:

• com.sun.mail:jakarta.mail:2.0.2 reached this module's compile classpath ahead of javax.mail:mail:1.4.7, via openidm-enhanced-config → openidm-crypto → openidm-util → json-resource-http. The built bundle therefore imported com.sun.mail.util;version="[2.0,3)", so EmailClient got MailSSLSocketFactory from jakarta.mail.
• javax.mail 1.4.7's SMTPTransport calls new MailLogger(Class, String, javax.mail.Session), which only its own com.sun.mail.util provides. So whenever sending works, its SocketFetcher is the 1.4.7 copy, sf instanceof MailSSLSocketFactory is false, and isServerTrusted never runs. Since the trust manager skips chain validation once a list is set, any non-empty trustedHosts trusted every host. I derived this from the jars and the manifests; I have not inspected the wiring on a running instance.
• jakarta.mail is now excluded from that dependency in openidm-external-email/pom.xml. The bundle imports com.sun.mail.util;version="[1.4,2)", and socketFactoryComesFromTheJavaMailInUse pins that the factory comes from the same jar as javax.mail.Session (it fails without the exclusion). jakarta.mail still ships in bundle/ through the other modules.
• I also added your warning when host is not in trustedHosts, and the Javadoc and chap-mail.adoc now say that host must appear in the list exactly as written, case-sensitively, and that any other host is rejected.

4. Precedence. I added startTlsTrustAllWinsOverTrustedHosts as you proposed, and the doc now states that trustAll takes precedence.

openidm-external-email passes 11/11. I could not build the Admin UI offline, so for EmailConfigView.js I ran only a syntax check and the merge logic in isolation. CI will cover the rest.

[vharseko]

@maximthomas a correction to point 3 of my previous reply, in 68c17fb.

What went wrong. The com.sun.mail.util;version="[1.4,2)" import left openidm-external-email unresolved, and OpenIDM never reached "ready" in any build-maven job. The reason is in the javax.mail 1.4.7 manifest: besides exporting com.sun.mail.*;version="1.4.7", the bundle imports the same packages with version="1.4", which has no upper bound. Felix wires those imports to jakarta.mail 2.0.2 and drops the 1.4.7 exports, so nothing exports com.sun.mail.util 1.4.x. My statement that "whenever sending works, its SocketFetcher is the 1.4.7 copy" was wrong. I had derived it from the manifests without running an instance.

The same wiring already breaks mail on master. I ran a current master build: POST /openidm/external/email?_action=send returns 500 because MimeMessage fails with NoSuchMethodError: com.sun.mail.util.PropUtil.getBooleanSessionProperty(javax.mail.Session, …), since it reaches jakarta.mail's PropUtil.

Fix.

• javax.mail 1.4.7 is now embedded in the bundle (Embed-Dependency), so Session, SMTPTransport, SocketFetcher and MailSSLSocketFactory always come from one jar. The manifest no longer imports javax.mail or com.sun.mail. While JavaMail looks up providers, EmailClient points the context class loader at the bundle, because JavaMail tries that loader first.
• I found a second problem while testing at runtime. With no mail.smtp.ssl.protocols set, JavaMail 1.4.7 enables only TLSv1 on the STARTTLS socket (SocketFetcher.configureSSLSocket), and current JDKs disable TLSv1. So no STARTTLS handshake could succeed (No appropriate protocol). EmailClient now sets the property to the JVM's default protocols, and startTlsUsesTheJvmDefaultProtocols covers it.

Verified at runtime. I ran a local SMTP server with STARTTLS and a self-signed certificate for localhost. In OpenIDM (OSGi, with this bundle):

• trustedHosts: ["localhost"]: the mail is delivered over TLS.
• trustedHosts: ["mail.other"]: the handshake completes, then JavaMail rejects the host ("Server is not trusted") and the warning is logged. This is the check from your point 3, now applied at runtime.
• Default settings: rejected on the certificate (certificate_unknown).

Startup log is clean against the CI pattern. Outside OSGi, on the same javax.mail jar, I also checked trustAll (delivered), LOCALHOST in the list (rejected, the match is case-sensitive), and with the test CA trusted, localhost (delivered) vs 127.0.0.1 (rejected, "Can't verify identity of server"). openidm-external-email passes 12/12.