CP-25960: allowlist discovery-derived hosts before authenticating against them - #829
Open
roeezis wants to merge 3 commits into
Open
CP-25960: allowlist discovery-derived hosts before authenticating against them#829roeezis wants to merge 3 commits into
roeezis wants to merge 3 commits into
Conversation
added 2 commits
September 3, 2026 17:55
CP-25960 (AG1, Medium): service discovery's identity/discoverycontext/ secrets_manager API hosts were trusted verbatim from the response body and handed straight to the Conjur/Identity clients, which then POST the agent's SA token (or username/password) to that host with no allowlist — an SSRF-shaped hole if the discovery response is ever tampered with. Mirrors finding R2 already fixed on the discoverycontext-regional-resources side (token.py's iss-host allowlist). Constrain each discovered host to a known CyberArk root domain, or to the same host we already made the successful, TLS-authenticated discovery call to (covers the ARK_DISCOVERY_API dev/CI override and same-origin test mocks without a per-env map). A dropped host is treated the same as one absent from the response. CP-25964 (A1, Low): the authn-jwt exchange error included up to 4KiB of Conjur's response body, which propagates to a Kubernetes Pod Event (pkg/agent/run.go's PushingErr notification) — readable by anyone with `get events` in the namespace. The body can contain Conjur policy structure, service IDs and host identities. Log it at V(2) instead; the returned error (and therefore the Event) now carries only the status code and the existing troubleshooting hint. SCR: https://ca-il-confluence.il.cyber-ark.com/pages/viewpage.action?pageId=710861530
The allowlist added in the previous commit was copied from the identity authorizer Lambda's local commercial-only clone of everest_env_utils' ROOT_DOMAIN map, not the real package. The real map (everest_env_utils_cyberark, v2.0.117) has 8 more entries for the GOV_* environments (*-cyberarkgov.com/.cloud) that the clone omits. Without them, any gov-cloud tenant's agent would have every discovery-derived host rejected, breaking identity discovery entirely (fatal, since it's required unconditionally).
Author
|
Follow-up commit: the allowlist added above was copied from the identity authorizer Lambda's local commercial-only clone of `everest_env_utils`'s `ROOT_DOMAIN` map. Checked the real package (`everest_env_utils_cyberark` v2.0.117, installed in `discoverycontext-file-ingestor-service`'s venv) and found 8 more entries for the GOV_* environments that the Lambda's local clone omits. Added them — without this, any gov-cloud tenant's agent would have every discovery-derived host rejected (fatal for |
CI's verify job failed: gci requires third-party imports (k8s.io/klog) grouped separately from and before this module's own imports, per .golangci.yaml's [standard, default, localmodule] section order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two agent-side hardening findings from the Conjur Agent Authentication Secure Code Review (CP-24572):
identity/discoverycontext/secrets_managerAPI hosts were trusted verbatim from the response body and handed straight to the Conjur/Identity clients, which then POST the agent's SA token (or username/password) to that host — no allowlist. Mirrors finding R2, already fixed on thediscoverycontext-regional-resourcesside. Now constrains each discovered host to a known CyberArk root domain, or to the same host the discovery call itself succeeded against (coversARK_DISCOVERY_APIdev/CI overrides and same-origin test mocks with no per-env map needed). A dropped host is treated the same as one absent from the response.get eventsin the namespace. Now logged atV(2)instead; the error/Event carries only the status code and the existing troubleshooting hint.Test plan
go vet ./...make test-unit— 431 tests, 4 skipped, 2 pre-existing unrelated failures (KUBEBUILDER_ASSETS-gated test not run viamake's own env in this invocation path is unaffected; thejson.RawMessage/jsontext.Valuemessage-drift failure inpkg/clientis pre-existing onmaster, unrelated to this change)servicediscoveryhost-allowlist accept/reject cases;conjurerror-body-omitted-from-returned-error case