Skip to content

Close the cheap CodeQL findings: pinned actions, token scopes, TLS identity, private temp files - #1134

Open
vharseko wants to merge 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:codeql-medium-quick-fixes
Open

vharseko wants to merge 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:codeql-medium-quick-fixes

Conversation

@vharseko

@vharseko vharseko commented Sep 18, 2026

Copy link
Copy Markdown
Member

The mechanical part of the CodeQL medium triage: findings whose fix is a few lines and carries no behaviour risk, grouped in one PR.

Workflows (actions/unpinned-tag ×11, actions/missing-workflow-permissions ×5)

  • docker/metadata-action, docker/setup-qemu-action, docker/setup-buildx-action, docker/login-action, docker/build-push-action and softprops/action-gh-release are pinned to the commit their major tag resolves to today, with the precise release in a comment (v6.2.0, v4.4.0, v4.4.1, v4.6.0, v7.4.0, v3.0.3). actions/* stay on tags: first-party, and not what the query flags.
  • .github/dependabot.yml (new) enables the github-actions ecosystem, weekly and grouped into one pull request: a SHA pin is frozen by definition, so without it the actions would never move again. Dependabot updates the SHA and the version comment together, and the grouped PR carries the ci/dependencies labels, matching Harden GitHub Actions workflows: token permissions, SHA pinning, latest actions OpenIG#170.
  • Every job declares the GITHUB_TOKEN scopes it uses: build-maven / build-docker contents: read (the image goes to a local registry); deploy-maven contents: write (docs are pushed to the wiki with github.token); release-maven contents: write (release:prepare pushes the tag, the GitHub release is created, the wiki is pushed); release-docker contents: read + packages: write (GHCR login with GITHUB_TOKEN). The Docker Hub and doc-repo pushes use their own secrets and need nothing from the token.

TLS identity (java/insecure-smtp-ssl ×2, java/unsafe-hostname-verification 1 of 3)

  • AMSendMail.postMail(…, ssl=true) sets mail.smtp.ssl.checkserveridentity=true: the SMTP server's certificate has to be the host's, not merely one the JVM trusts. Pinned by shouldCheckTheServerIdentityOnAnSslConnection (was null).
  • AMSetupUtils.openConnection no longer installs a trust-all HostnameVerifier. getRemoteServerInfo posts the admin password to the remote server over that connection, so a certificate that is valid for some other host must not do. A remote OpenAM whose certificate does not name the URL's host now fails the setup step with the JDK's message instead of silently accepting it; openConnection is package-private for shouldKeepTheDefaultHostNameVerifierForARemoteServerOverHttps.
  • Not changed, proposed for dismissal instead: ClusterStateService (a site health probe whose answer is a boolean; strict verification would break clusters whose certificates do not name the internal host names) and SoapSTSConsumer.handleSTSServerCertCNDNSMismatch (a test helper whose javadoc already says it must not be relied upon in production).

Private temporary files (java/local-temp-file-or-directory-information-disclosure ×4)

File.createTempFile creates the file with the process umask — rw-r--r-- on a typical server — in the shared temporary directory. java.nio.file.Files.createTempFile creates it rw-------.

  • ApiDocsService: the generated .asciidoc and .html go through privateTempFile (owner-only, deleted on exit).
  • NT: the file that carries the user name and password to the Samba helper goes through createCredentialsFile; its name no longer embeds the user name (which also failed for names shorter than three characters, the prefix minimum).
  • Embedded Cassandra Server: the storage directory keeps its fixed default under java.io.tmpdir — a random name would not survive a restart of the development store — but is created, and if it already exists set, to rwx------ where the file system is POSIX; elsewhere it is created as before.

Each test was watched failing on the previous code: the permission ones with [OWNER_READ, OWNER_WRITE, GROUP_READ, OTHERS_READ] (files) and 0755 / inherited (directory).

Verification

openam-core 2132, openam-core-rest 441, openam-auth-nt 7, openam-cassandra-embedded 3 (under JDK 11 — the module's tests are skipped on JDK ≥ 15 by the jdk-15-cassandra profile, as in CI — including ServerTest, which starts the embedded Cassandra through the changed run()), 0 failures. Workflow YAML parsed and the job permissions read back. NTCredentialsFileTest is a separate plain class because NTTest runs under PowerMock's class loader, which cannot instrument sun.nio.fs on JDK 26.

The CodeQL bot's SSRF flag on AMSetupUtils.openConnection (alert #511) is triaged and dismissed as won't fix — a re-fingerprint of #145.

…entity, 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.
@vharseko vharseko added security Security fix or hardening (CVE, GHSA, XSS/CSRF/SSRF) java Pull requests that update java code tests Test suite: coverage, fixtures, or test infrastructure ci CI, GitHub Actions, or build pipeline labels Sep 18, 2026
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.
@vharseko vharseko added the dependencies Pull requests that update a dependency file label Sep 18, 2026
Comment thread openam-core/src/main/java/com/sun/identity/setup/AMSetupUtils.java Dismissed
Add "ci"/"dependencies" labels and the grouping comment to
.github/dependabot.yml, matching OpenIdentityPlatform/OpenIG#170.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI, GitHub Actions, or build pipeline dependencies Pull requests that update a dependency file java Pull requests that update java code security Security fix or hardening (CVE, GHSA, XSS/CSRF/SSRF) tests Test suite: coverage, fixtures, or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants