OBO tokens: load the encryption key from a keystore and harden the crypto

[beanuwave]

Description

Category: Bug fix, Enhancement

The OBO key path was audited end to end. The encryption rewrite is the FIPS trigger; the rest are weaknesses found alongside it.

Key changes

• Encryption rewrite. EncryptionDecryptionUtil built its cipher with a bare Cipher.getInstance("AES") and a key forced to 16 bytes via Arrays.copyOf (silently truncating or zero-padding). Now AES-256-GCM (random 12-byte IV, 128-bit tag) with the key derived via HKDF-SHA256. A bare "AES" transformation resolves to the provider's default mode — ECB — which for data confidentiality violates NIST SP 800-38A^[2] (ECB is approved only for key wrapping, SP 800-38F^[3]). The mode was never spelled out or configurable, and BC FIPS permits the ECB primitive at runtime, so this had to be caught by review rather than by the provider.
• Entropy floor (SP 800-133r2^[5]). HKDF cannot create entropy, so a short encryption_key would yield a nominal AES-256 key with sub-112-bit strength. A FIPS guard rejects input keying material < 32 bytes and zeroes the IKM after derivation.
• Signing-key length check. Validates the decoded key bytes; measuring the Base64 string would over-count by ~4/3 and let a 384-bit key pass a 512-bit gate.
• Lazy, fail-closed init. OnBehalfOfAuthenticator initialises lazily and atomically: the constructor is inert, the first token request triggers init, and a failure logs once and declines. A bad key can no longer throw out of the constructor, propagate through config reload and retry forever. Mirrors ApiTokenAuthenticator.
• Keystore support. Signing and encryption keys can be loaded from a keystore (KeyUtils.loadKeyFromKeystore → PemKeyReader.loadSecretKeyFromKeystore) instead of inline config, keeping secrets out of cluster state. Relative *_keystore_path values resolve against the node config dir, consistent with other security file settings. This is SP 800-57^[6] key-at-rest hardening, not a FIPS 140-3^[1] requirement.
• Secret redaction (CWE-532). OnBehalfOfSettings.toString() redacts signing_key / encryption_key so they no longer leak into logs.

Reviewer call-outs

1. OBO token wire-format break. GCM-encrypted tokens are not interchangeable with old AES-ECB tokens across the upgrade boundary. Benign in practice (OBO TTL
   < 10 min, self-heals shortly after rollout), but worth a release note for
   hot/rolling deployments.
2. JKS cannot hold SecretKey entries (engineSetKeyEntry requires a PrivateKey), which is why the JWT-signing-key path needs BCFKS — the only FIPS-approved store that holds secret keys.

Testing

./gradlew test integrationTest

Manual test: OnBehalfOf (OBO) token, non-FIPS

Exercises OBO token issuance and verification against an already-running non-FIPS cluster, in three modes: keys inline in the dynamic config (Scenario A), held in a PKCS#12 keystore out of cluster state (Scenario B), or in a BCFKS keystore (Scenario C — works on a non-FIPS node too, because the plugin still registers the BC FIPS provider at load on this branch). No restart needed — -t config pushes only the dynamic on_behalf_of block, picked up live. Pick ONE scenario, edit config.yml, then run the apply / issue / use steps. All paths are relative to $OPENSEARCH_HOME.

cd $OPENSEARCH_HOME
export ADMIN_AUTH="admin:<admin-password>"

# === Apply / issue / use (the verify loop - run after editing config.yml) ====
# apply: push the dynamic config (-t config = on_behalf_of block only, live reload).
sh ./plugins/opensearch-security/tools/securityadmin.sh \
  -f ./config/opensearch-security/config.yml \
  -t config \
  -icl \
  -nhnv \
  -cacert config/root-ca.pem \
  -cert config/kirk.pem \
  -key config/kirk-key.pem \
  -h localhost \
  -p 9200

# issue: generate a token.
export OBO_TOKEN=$(curl -sk \
  -u "$ADMIN_AUTH" \
  -X POST \
  -H 'Content-Type: application/json' \
  https://localhost:9200/_plugins/_security/api/generateonbehalfoftoken \
  -d '{
        "description": "obo test",
        "service": "test-service",
        "durationSeconds": "300"
      }' | jq -r '.authenticationToken')
echo "$OBO_TOKEN"

# use: a populated user_name / roles proves the verify side loaded the key,
#      checked the signature, and decrypted the roles (AES-GCM round trip).
curl -sk \
  -H "Authorization: Bearer $OBO_TOKEN" \
  https://localhost:9200/_plugins/_security/authinfo?pretty

# === Scenario A - inline keys ================================================
# signing_key >= 512 bits (64 bytes) for HS512.
export OBO_SIGNING_KEY=$(openssl rand 64 | base64 -w0)
export OBO_ENCRYPTION_KEY=$(openssl rand 32 | base64 -w0)

# Put under config.dynamic.on_behalf_of in config/opensearch-security/config.yml:
#   on_behalf_of:
#     enabled: true
#     signing_key: "<value of $OBO_SIGNING_KEY>"
#     encryption_key: "<value of $OBO_ENCRYPTION_KEY>"

# === Scenario B - PKCS#12 keystore ===========================================
# PKCS#12 has no per-entry passwords, so use one value for -storepass and -keypass.
keytool \
  -genseckey \
  -alias obo-signing \
  -keyalg HmacSHA512 \
  -keysize 512 \
  -storetype PKCS12 \
  -keystore config/obo.p12 \
  -storepass kspass \
  -keypass kspass

keytool \
  -genseckey \
  -alias obo-enc \
  -keyalg AES \
  -keysize 256 \
  -storetype PKCS12 \
  -keystore config/obo.p12 \
  -storepass kspass \
  -keypass kspass

# Reference the keystore in config.yml (no inline keys). <key>_keystore_path is
# config-dir-relative; the loader falls back to the keystore password when
# _keystore_key_password is absent.
#   on_behalf_of:
#     enabled: true
#     signing_key_keystore_path: "obo.p12"
#     signing_key_keystore_type: "PKCS12"
#     signing_key_keystore_alias: "obo-signing"
#     signing_key_keystore_password: "kspass"
#     encryption_key_keystore_path: "obo.p12"
#     encryption_key_keystore_type: "PKCS12"
#     encryption_key_keystore_alias: "obo-enc"
#     encryption_key_keystore_password: "kspass"
#
# Same result as Scenario A, but no key material in the config index. GET
# /_plugins/_security/api/securityconfig shows only keystore references.

# === Scenario C - BCFKS keystore =============================================
keytool \
  -genseckey \
  -alias obo-signing \
  -keyalg HmacSHA512 \
  -keysize 512 \
  -storetype BCFKS \
  -providername BCFIPS \
  -keystore config/obo.bcfks \
  -storepass kspass \
  -keypass keypass \
  -providerClass org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider \
  -providerPath lib/bc-fips-2.1.2.jar

keytool \
  -genseckey \
  -alias obo-enc \
  -keyalg AES \
  -keysize 256 \
  -storetype BCFKS \
  -providername BCFIPS \
  -keystore config/obo.bcfks \
  -storepass kspass \
  -keypass keypass \
  -providerClass org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider \
  -providerPath lib/bc-fips-2.1.2.jar

# Reference it as in Scenario B, with per-entry key passwords:
#   on_behalf_of:
#     enabled: true
#     signing_key_keystore_path: "obo.bcfks"
#     signing_key_keystore_type: "BCFKS"
#     signing_key_keystore_alias: "obo-signing"
#     signing_key_keystore_password: "kspass"
#     signing_key_keystore_key_password: "keypass"
#     encryption_key_keystore_path: "obo.bcfks"
#     encryption_key_keystore_type: "BCFKS"
#     encryption_key_keystore_alias: "obo-enc"
#     encryption_key_keystore_password: "kspass"
#     encryption_key_keystore_key_password: "keypass"

Negative checks (each one, then restore a working config):

• Tampered token. Change a character in the token's last segment — the use step returns no credentials.
• Lazy, fail-closed init. Scenario B/C with a wrong alias (signing_key_keystore_alias: "nope"). apply still succeeds and the cluster stays healthy (_cluster/health); on the first OBO request the node logs On-behalf-of authentication is misconfigured; OBO tokens will be rejected: ... No key found at alias 'nope' in keystore once, and OBO requests are declined.
• Signing-key strength on decoded bytes. Scenario A with a 48-byte key (openssl rand 48 | base64 -w0 — 64 Base64 chars, 384 bits) is rejected with Signing key size was 384 bits, which is not secure enough.
• No FIPS-only restriction leaks into non-FIPS. Scenario A with a short encryption_key (e.g. ZW5jcnlwdGlvbktleQ==, 13 bytes) is accepted here; the 32-byte floor applies only in FIPS mode (tested in PR 5).

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit dec2ca3.

⛔ Hard block: Issues at High severity or above will block this PR from merging.

Path	Line	Severity	Description
build.gradle	609	high	New dependency org.bouncycastle:bctls-fips added in three scopes (integrationTestImplementation line 609, compileOnly line 716, testImplementation line 821). Per mandatory rule, all dependency additions must be flagged regardless of apparent legitimacy — maintainers must verify the artifact's authenticity and provenance.
build.gradle	77	high	A new Gradle build script (gradle/fips.gradle) is applied via 'apply from:'. This script adds JVM arguments to every Test task in the build (including configuring custom security providers, truststore paths, and system properties), making it a build plugin change that must be reviewed per mandatory flagging rules.
src/test/resources/fips-jvm-truststore.bcfks	1	medium	A binary BCFKS keystore file is committed to the repository. Its contents cannot be verified from the diff — it may contain unexpected certificates or keys beyond its stated purpose as a FIPS test truststore. Binary credential/keystore files warrant inspection of their actual contents.
src/main/java/org/opensearch/security/support/FipsMode.java	19	low	The security-critical FIPS mode check relies on a package-private mutable static field (envSupplier). Any class in the same package can substitute the supplier at runtime, bypassing the environment-variable-based check. While clearly intended for testability, it weakens the integrity of the FIPS enforcement boundary.
src/main/java/org/opensearch/security/auth/ldap2/SNISettingTLSSocketFactory.java	44	low	A static ThreadLocal holding the SNI hostname (hostnameThreadLocal) is accessible via package-private static methods (getHostname, clearContext, configure). Any code in the same package can manipulate the hostname used for TLS SNI and hostname verification in LDAP connections on the current thread.
src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java	468	low	OnBehalfOfSettings now accepts arbitrary unknown JSON fields via @JsonAnySetter and re-emits them via @JsonAnyGetter. Any unrecognized key/value pair from the security configuration YAML will be silently absorbed and round-tripped, potentially masking typos in sensitive keystore settings or allowing unexpected configuration injection.

The table above displays the top 10 most important findings.

Total: 6 | Critical: 0 | High: 2 | Medium: 1 | Low: 3

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.