mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 22:21:30 +00:00
shankar0123
5d7bc86451
Acquisition-audit Sprint 1 follow-up to SEC-001 (2026-05-16). The
original SEC-001 sweep routed two OIDC discovery legs (test_discovery.go
dry-run + service.go runtime provider load) through
validation.SafeHTTPDialContext via the SafeOIDCContext(ctx) helper.
This commit closes one of the two adjacent call sites the sweep missed:
the userinfo-fallback path at service.go::fetchUserinfoGroups.
Pre-fix:
func (s *Service) fetchUserinfoGroups(ctx, entry, token, path) {
...
ts := entry.oauthConfig.TokenSource(ctx, token)
uinfo, err := entry.provider.UserInfo(ctx, ts)
...
}
go-oidc/v3 Provider.UserInfo (oidc.go:351-374) derives its
http.Client from ctx via getClient(ctx) (oidc.go:61-65). Without an
override, the internal doRequest (oidc.go:87-92) falls through to
http.DefaultClient — no SSRF guard, no DNS-rebinding re-resolve at
dial time. An IdP whose discovery doc advertises a userinfo_endpoint
pointing at a reserved address (loopback / link-local /
169.254.169.254 cloud-metadata) would trigger an unguarded HTTPS
egress at userinfo-fetch time. Operator opt-in to fetch_userinfo=true
turns the gap on; the leg fires whenever the ID token doesn't surface
the configured groups claim.
Post-fix:
safeCtx := SafeOIDCContext(ctx)
ts := entry.oauthConfig.TokenSource(safeCtx, token)
uinfo, err := entry.provider.UserInfo(safeCtx, ts)
Context-key shape: gooidc.ClientContext is implemented as
context.WithValue(ctx, oauth2.HTTPClient, client) (go-oidc v3.18.0
oidc.go:57-59). Both go-oidc's getClient AND golang.org/x/oauth2's
internal.ContextClient read the same oauth2.HTTPClient key, so the
SINGLE SafeOIDCContext wrap covers go-oidc-driven HTTP calls
(Provider.UserInfo / Verifier JWKS) AND oauth2-driven HTTP calls
(Config.TokenSource refresh / Exchange). No additional
context.WithValue(ctx, oauth2.HTTPClient, ...) is required.
Files touched:
internal/auth/oidc/service.go — wrap ctx in fetchUserinfoGroups
internal/auth/oidc/safehttp.go — extend SEC-001 header comment block
to enumerate the two newly-patched sites (SEC-020 here +
SEC-021 in the next commit) and the oauth2.HTTPClient key-sharing
rationale, so future audits don't re-flag the design as confused
internal/auth/oidc/service_test.go — new test
TestFetchUserinfoGroups_SSRF_BlocksReservedAddress that
stands up a loopback discovery server whose discovery doc
advertises userinfo_endpoint = http://169.254.169.254/userinfo,
constructs *gooidc.Provider via the test-bypassed
oidcDiscoveryClient (setup_test.go's init() pattern), then
RESTORES the production SafeHTTPDialContext-backed client just
before the fetchUserinfoGroups call. Asserts the error wraps
SafeHTTPDialContext's 'refusing to dial reserved address'
rejection rather than a generic connect-refused. Companion to
the TestDefaultBCLVerifier_SSRF_BlocksReservedAddress that
SEC-021 (next commit) adds.
Verified:
gofmt -l internal/ docs/ (clean)
go vet ./... (clean)
go test -race -short ./internal/auth/oidc/... (all green)
TestFetchUserinfoGroups_SSRF_BlocksReservedAddress (new; green)
All 4 cited CI guards pass (openapi-handler-parity,
openapi-codegen-drift, no-sh-c-in-connectors, skip-inventory-drift)
Acceptance grep:
internal/auth/oidc/service.go:963: uinfo, err := entry.provider.UserInfo(safeCtx, ts)
internal/auth/oidc/service.go:1084: provider, err := gooidc.NewProvider(SafeOIDCContext(ctx), cfgRow.IssuerURL)
No bare-ctx UserInfo / NewProvider remains in service.go.
Closes acquisition-audit SEC-020. SEC-021 (BCL discovery re-fetch)
lands in the next commit.