[
https://issues.apache.org/jira/browse/KNOX-3414?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104533#comment-18104533
]
ASF subversion and git services commented on KNOX-3414:
-------------------------------------------------------
Commit dcd1e01779b95bfc3d39214d820ba35b8d950b44 in knox's branch
refs/heads/knox_idf from Sandor Molnar
[ https://gitbox.apache.org/repos/asf?p=knox.git;h=dcd1e0177 ]
KNOX-3414: harden secrets-at-rest, auth-code single-use, and persistence (1.5,
2.4, 2.11)
Three KnoxIDF security/correctness hardening findings from the OIDC-provider
review, landed together.
1.5 - Secrets at rest
- Federated OP client_secret is now resolvable via AliasService. New optional
topology param federated.op.<name>.clientSecret.alias
(FederatedOpConfiguration). AuthorizeResource.resolveClientSecret prefers the
alias (getPasswordFromAliasForCluster, cluster from GATEWAY_CLUSTER_ATTRIBUTE
with NO_CLUSTER_NAME fallback); fails closed if a configured alias is
unresolvable rather than leaking the plaintext param. Plaintext clientSecret
retained only as a fallback when no alias is set.
- Federated OP access token is no longer persisted at rest:
decorateAuthCodeToken
stores only the FEDERATED_IDENTITY_ID pointer. Removed the now-dead
split/join/chunk token helpers and FEDERATED_*_TOKEN_PREFIX constants.
2.4 - Single-use authorization codes (close the replay window)
- SPI TokenStateService: new default consumeToken(String) - atomic single-use
consume; exactly one concurrent caller receives true, an absent token is false
(never throws).
- DefaultTokenStateService: overrides consumeToken with a true atomic claim
(tokenExpirations.remove(id) != null), evicting the remaining per-token state.
- JDBCTokenStateService: overrides consumeToken using the primary-key DELETE as
the atomic arbiter, evicts the in-memory cache, and fails closed on
SQLException
(the inherited removeToken swallows it and would falsely report a win). Derby
inherits this.
- idf TokenResource: validateAuthCode now returns the metadata it already reads;
handleAuthorizationCodeFlow validates, then atomically consumes the code
BEFORE
issuing any token, stashing the metadata in a per-request attribute consumed
by
the issuance steps (getAuthCodeMetadata). The revoke-in-finally is removed. A
code that fails validation is deliberately NOT consumed, so replaying with bad
params cannot burn a victim's still-valid code.
2.11 - Persistence hardening
- Consent key reshaped to fit KNOX_TOKEN_METADATA.md_name VARCHAR(32):
AuthorizeResource.consentMetadataKey(subject) = "consent_" +
first-20-hex(SHA-256(subject)) (28 chars), used by both hasConsent and
markConsentAccepted so read and write agree. No schema change to md_name.
- Derby DDL parity: added the missing NOT NULL constraints to the federated
identity and attribute tables (the UNIQUE index was already present).
- TOCTOU: JdbcFederatedIdentityService.addFederatedIdentity now inserts and
catches instead of check-then-insert; a unique-constraint violation
(SQLIntegrityConstraintViolationException or SQLState class 23, walked up the
cause chain) is treated as a benign already-exists. The unique index is the
arbiter.
- Transaction boundary: FederatedIdentityDatabase.addFederatedIdentity writes
the
core row and attribute rows on a single connection with autocommit off, then
commits (rollback on failure) so an identity is never persisted without its
attributes.
- Double-checked locking: added the missing inner recheck in
JdbcFederatedIdentityService.init; corrected its misleading exception message.
- SELECT * replaced with the explicit column list in the
by-provider/issuer/subject
query; KnoxDatabase resolves DDL via getClass().getClassLoader() so each
subclass
loads its own create*.sql.
Tests
- DefaultTokenStateServiceTest: +3 consumeToken tests (single-use, state
removed,
unknown id).
- New TokenResourceAuthCodeReplayTest: a losing (already-consumed) redemption
yields invalid_grant with no issuance; a winning redemption issues exactly
once.
- New ConsentMetadataKeyTest: key fits VARCHAR(32) for realistic subjects,
deterministic, distinct subjects -> distinct keys.
- FederatedOpConfigurationTest: +2 (clientSecret alias read / absent by
default).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
> KnoxIDF (OIDC Provider): security hardening, correctness fixes, and
> documentation
> ---------------------------------------------------------------------------------
>
> Key: KNOX-3414
> URL: https://issues.apache.org/jira/browse/KNOX-3414
> Project: Apache Knox
> Issue Type: Improvement
> Components: KnoxIDF
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Critical
> Fix For: 3.1.0
>
>
> Follow-up work on the KnoxIDF OAuth2/OIDC Authorization Server. Items to fix:
> Client authentication & flow security
> - Authenticate the client on the refresh_token grant.
> - Authenticate the client on the authorization_code grant (PKCE S256 or
> constant-time client_secret).
> - Reject PKCE plain; accept only S256, and advertise only S256 in discovery.
> - Require and validate state/nonce; bind the federated id_token to the
> request via nonce.
> - Atomically consume authorization codes and refresh tokens before issuance
> (close replay windows).
> Federation
> - Fully validate the federated OP id_token (signature/JWKS, issuer, audience,
> exp/nbf, sub) before trusting any claim.
> - Reject a federated id_token missing sub with a 4xx, not a 500.
> - Fail closed when a federated-OP client-secret alias is unresolvable.
> - Stop persisting federated OP access tokens at rest; store only
> ID-token-derived identity data.
> - Activate federated-identity persistence (was silently disabled) with
> self-provisioning embedded Derby default.
> Redirect-URI / registration
> - Normalize redirect_uri path before wildcard matching; match on parsed URI
> components (fix open redirect).
> - Build a well-formed success redirect when redirect_uri already carries a
> query.
> - Require HTTPS for registered redirect URIs (loopback HTTP excepted, RFC
> 8252).
> - Make anonymous dynamic client registration an explicit, secure-by-default
> opt-in.
> Correctness / robustness
> - Fix SCOPE_ATTRIBUTE always resolving to null (double getClaim).
> - Fix iat=1970 regression on non-KnoxIDF tokens (default issue time to now).
> - /userinfo returns 401 invalid_token, not 500, for a bad bearer token.
> - Fix broken double-checked locking in service init paths.
> - Escape LDAP username input; remove hardcoded LDAP system-password fallback
> (fail fast).
> - Per-request claim map (fix cross-user claim leakage in the shared
> singleton).
> Persistence
> - Fix TOCTOU on federated-identity insert (unique constraint as arbiter).
> - Write identity + attributes in a single transaction.
> - Align Derby schema (NOT NULL / unique index) with other dialects.
> Key management & auditing
> - Publish multiple JWKs and select the verification key by kid (enables
> zero-downtime signing-key rotation).
> - Emit structured audit records across the OAuth2/OIDC endpoints.
> OIDC discovery / docs
> - Complete OIDC discovery metadata for OAuth/MCP client integration.
> - Add opt-in Keycloak federation E2E test; extend existing E2E suite to cover
> the fixes above.
> - Add the Identity Federation (OIDC Provider) documentation book.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
