Repository navigation
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-4762.docs.buildwithfern.com/openshell |
Compatibility review (advisory)New or updated breaking changes; see the full report for all findings. 1. api-behavior — No explicit risk acceptance foundThe baseline emitted a
Review details
Advisory review; publication gates are unchanged. |
|
Acknowledging the compatibility finding and accepting the risk. The baseline for this review is #4761 (855e0e3), which has merged to main but not shipped in a release. The success-then-failure sequence this PR removes was introduced there, so no released gateway has emitted it: v0.1.5 emits only authentication failures. No SIEM rule can depend on it yet. The change is intended. A compute-driver credential on The remaining risk is a release cut before this PR merges, which would ship the interim sequence for one release. We'll merge this PR before the next release so it lands alongside #4761. |
b4aabdb to
6c87ace
Compare
TaylorMutch
left a comment
There was a problem hiding this comment.
Reviewed at 6c87ace. No actionable findings. The deferred logon checks, credential redaction, and audit event coverage look sound.
Some credentials are checked by handlers rather than by the gateway's authenticators, so their rejections only reached the caller and plain logs. They now emit the same OCSF events as the router's checks: - SSH session tokens. A ForwardTcp SSH connection whose token is missing, unknown, revoked, minted for another sandbox, or expired emits an Authentication failure with protocol ssh_session_token and a reason category. An accepted token logs on once, like other credentials. The router has already authenticated the caller, so these events name that caller. - IssueSandboxToken. A compute-driver identity that no longer matches the sandbox's recorded runtime emits an Authentication failure (stale_identity); a sandbox with no record, or no usable recorded identity, emits unknown_identity. A sandbox calling it with a gateway JWT is refused with method_not_permitted and raises the sandbox boundary finding. - RefreshSandboxToken. A caller that is not a gateway-JWT sandbox is refused with method_not_permitted. A request for an extension service the sandbox policy does not select is refused with the new extension_service_not_selected reason, without a finding, because a policy update can race the supervisor's request. - Sandbox logons on IssueSandboxToken and RefreshSandboxToken. The router logged a compute-driver credential on IssueSandboxToken, or a gateway JWT on RefreshSandboxToken, before the handler finished checking the recorded runtime or the token lineage, so a credential that failed that check recorded a success and then a failure. Those handlers now record the logon once their check passes. Other credentials on these methods are fully verified by the router and still log on there, even when authorization then refuses them. - GetCurrentUser. A non-user principal is refused with method_not_permitted. The router already blocks such callers, so this is defense in depth. Callers receive the same status codes and messages as before, and the events never include the tokens. Refs #2912 Signed-off-by: Kris Hicks <khicks@nvidia.com>
6c87ace to
7892e4e
Compare
TaylorMutch
left a comment
There was a problem hiding this comment.
Reviewed the full diff and credential validation paths at 7892e4e. No actionable findings. Deferred logons occur after runtime or lineage validation, SSH session failures retain their existing caller statuses, and structured events avoid presented tokens. Tests cover rejection reasons, request correlation, deduplication, and boundary findings. Required CI gates pass; validation relied on code review and CI, with no local test run.
Summary
The gateway now records OCSF security events for the credentials that request handlers check themselves, so they appear in the same audit stream as the router's authentication and authorization decisions:
ForwardTcpSSH connections emit an Authentication failure when rejected and a logon when first accepted, with protocolssh_session_token.IssueSandboxToken's runtime-identity check emits an Authentication failure.IssueSandboxToken,RefreshSandboxToken, andGetCurrentUser, and the extension-service check inRefreshSandboxToken, emit authorization denials.Callers receive the same status codes and messages as before.
Related Issue
Closes #2912
This is the last PR for #2912. It covers the remaining authentication paths, which meets the criteria for reason categories across the gateway's supported authentication paths and for SSH bearer redaction. The operator setting for success events has been dropped from the issue for now: logons record each credential once an hour per replica, so they stay on.
Changes
SSH session tokens (
acquire_forward_connection_guard):missing_credentialinvalid_credentialaudience_or_issuer_mismatchexpired_credentialidentity_provider_unavailableThe router has already authenticated the caller's bearer token, so these events name that caller in
user. An accepted token logs on once per replica through the shared logon record from feat(auth): emit OCSF logon events for accepted gateway credentials #4761, keyed on the caller and a digest of the token.IssueSandboxToken: a compute-driver identity that doesn't match the sandbox's recorded runtime emitsstale_identity. A sandbox with no record, or no usable recorded identity, emitsunknown_identity; both use protocolcompute_driver, and an unreadable store emitsidentity_provider_unavailable. A sandbox holding a gateway JWT is refused withmethod_not_permitted, which also raises thesandbox-authorization-boundaryfinding, since supervisors refresh instead.RefreshSandboxToken: callers other than a gateway-JWT sandbox are refused withmethod_not_permitted. A request for an extension service the sandbox policy doesn't select is refused with the newextension_service_not_selectedreason, naming the service as anextension_serviceresource. It raises no finding, because a policy update can race the supervisor's request.Logons that wait for the handler. feat(auth): emit OCSF logon events for accepted gateway credentials #4761's router logs a credential on before the handler runs, but
IssueSandboxTokenandRefreshSandboxTokenfinish verifying the credential in the handler: the recorded runtime binding, and the token lineage. A stale runtime or a replayed, already-consumed bearer therefore recorded "succeeded" and then "failed" for one request. The router now skips the logon only for those deferred sandbox credentials (a compute-driver credential onIssueSandboxToken, a gateway JWT onRefreshSandboxToken), and each handler records it once its check passes. Users, operators, and gateway peers on these methods are fully verified by the router, so they still log on there before authorization refuses them. Moving the checks into the authenticators instead would have changed which error a misconfigured gateway returns.GetCurrentUser: non-user principals are refused withmethod_not_permitted. The router already blocks these callers; this keeps the handler's own check audited.auth::audit:emit_credential_failureandemit_ssh_session_logontake the request path and origin from the handler's request scope, asemit_authorization_denialdoes.Docs:
ocsf-json-export.mdxdescribes handler-checked credentials and when those two methods log on, adds thessh_session_tokenprotocol and theextension_service_not_selectedreason, and notes that SSH session tokens log on even on gateways without authentication, since the token itself is verified.Events never include presented tokens.
Testing
cargo nextest run -p openshell-server -p openshell-ocsf: all 2,613 tests pass.cargo clippy -p openshell-server --all-targets -- -D warningsstale_identity, with protocolcompute_driver. A missing sandbox, or one with no recorded identity, emitsunknown_identity.stale_identity.IssueSandboxToken, and a compute-driver principal callingRefreshSandboxToken, emit a denial and a finding.RefreshSandboxTokenemits a denial without a finding.extension_service_not_selectedwithout a finding; a successful refresh emits nothing.GetCurrentUseremits a denial and a finding.Checklist