mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 12:41:30 +00:00
fix(handler): SEC-021 — wrap BCL provider re-fetch via SafeOIDCContext
Acquisition-audit Sprint 1 follow-up to SEC-001 (2026-05-16). Companion
to SEC-020 (prior commit). Closes the second of the two adjacent OIDC
call sites the original SEC-001 sweep missed: the per-request discovery
re-fetch in DefaultBCLVerifier.Verify.
Pre-fix:
func (v *DefaultBCLVerifier) Verify(ctx, logoutToken) {
...
provider, perr := gooidc.NewProvider(ctx, matched.IssuerURL)
...
}
Same shape as service.go::fetchUserinfoGroups (closed in the prior
commit) and service.go:1084 (closed by SEC-001 itself). go-oidc's
NewProvider derives its http.Client from ctx; bare ctx falls through
to http.DefaultClient at the discovery-doc + JWKS-fetch dial. An IdP
whose registered IssuerURL resolves to a reserved address (or is
rebinding to one at logout time) would trigger an unguarded HTTPS
egress on every back-channel-logout request.
Post-fix:
provider, perr := gooidc.NewProvider(
oidcsvc.SafeOIDCContext(ctx), matched.IssuerURL)
The 'oidcsvc' alias for github.com/certctl-io/certctl/internal/auth/oidc
is added to the import block (matches the canonical alias used in
cmd/server/main.go:29). SafeOIDCContext routes the dial through
validation.SafeHTTPDialContext, which re-resolves the issuer host at
dial time and refuses reserved-address answers (loopback /
link-local / 169.254.169.254 cloud-metadata).
Files touched:
internal/api/handler/auth_session_oidc_bcl.go — add oidcsvc import +
wrap ctx at the NewProvider call site
internal/api/handler/auth_session_oidc_bcl_test.go — NEW FILE.
TestDefaultBCLVerifier_SSRF_BlocksReservedAddress constructs a
stubProviderRepo with IssuerURL='http://127.0.0.1:1' (literal
loopback — the IP-literal class that SafeHTTPDialContext.
isReservedIPForDial refuses up-front, before any DNS resolution).
Hand-rolls a 3-segment JWT whose payload base64url-decodes to
{"iss":"<loopback url>"} so peekIssuer extracts the matching
issuer and provs.List() returns the seeded provider. Calls Verify
and asserts the error wraps the dial-time reserved-address
rejection (substring match on 'refusing to dial' / 'reserved
address') AND that it's wrapped through the 'provider discovery:'
prefix that distinguishes a discovery-time dial failure from a
signature-verification failure.
docs/operator/auth-threat-model.md — NEW subsection 'Userinfo + BCL
SSRF parity (post-SEC-001 follow-up)' under '### Back-channel
logout'. Documents both SEC-020 and SEC-021 closures, the
context-key shape (why a single SafeOIDCContext wrap covers both
go-oidc and oauth2 legs), and the out-of-scope RFC 1918 carve-out
(covered separately by acquisition-audit Sprint 5 RED-005). Cross-
references the two pinning tests by name so future audits can
locate the load-bearing enforcement.
Verified:
gofmt -l internal/ docs/ (clean)
go vet ./... (clean)
go test -race -short ./internal/api/handler/... (all green)
TestDefaultBCLVerifier_SSRF_BlocksReservedAddress (new; green)
All 4 cited CI guards pass.
Acceptance grep on the BCL handler:
internal/api/handler/auth_session_oidc_bcl.go:132:
provider, perr := gooidc.NewProvider(oidcsvc.SafeOIDCContext(ctx), matched.IssuerURL)
No bare-ctx NewProvider remains in the BCL verifier. Combined with the
SEC-020 commit, every gooidc.NewProvider + Provider.UserInfo call site
in the production OIDC + BCL surface now routes through
SafeOIDCContext.
Closes acquisition-audit SEC-021. Sprint 1 ACQ is complete (2/2
findings). The single sprint shipped as two operator-authored commits
(per-finding, mirrors the project's commit cadence for closures).
This commit is contained in:
@@ -300,6 +300,64 @@ constant, router-level no-rbacGate-wraps-protocol-paths).
|
|||||||
attacks where an attacker captures a logout JWT and replays it.
|
attacks where an attacker captures a logout JWT and replays it.
|
||||||
- **Cache-Control: no-store** on the response per spec §2.5.
|
- **Cache-Control: no-store** on the response per spec §2.5.
|
||||||
|
|
||||||
|
### Userinfo + BCL SSRF parity (post-SEC-001 follow-up)
|
||||||
|
|
||||||
|
The original SEC-001 closure (Sprint 1, 2026-05-16) routed two OIDC
|
||||||
|
discovery legs — `test_discovery.go` dry-run and `service.go` runtime
|
||||||
|
provider load — through `validation.SafeHTTPDialContext` via the
|
||||||
|
`SafeOIDCContext(ctx)` helper at
|
||||||
|
[`internal/auth/oidc/safehttp.go`](../../internal/auth/oidc/safehttp.go).
|
||||||
|
The acquisition-audit follow-up (2026-05-16) flagged two adjacent
|
||||||
|
call sites the sweep missed; both are now wrapped identically.
|
||||||
|
|
||||||
|
- **SEC-020 — Userinfo fallback (`fetchUserinfoGroups`).**
|
||||||
|
`internal/auth/oidc/service.go` previously called
|
||||||
|
`entry.provider.UserInfo(ctx, ts)` with the bare request context
|
||||||
|
on the userinfo-fallback leg (operator opt-in when an IdP doesn't
|
||||||
|
surface groups in the ID token). go-oidc/v3's `Provider.UserInfo`
|
||||||
|
derives its `http.Client` from `ctx` via `getClient(ctx)`
|
||||||
|
(`oidc.go:61-65`); without an override the internal `doRequest`
|
||||||
|
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, cloud-metadata `169.254.169.254`, RFC 1918) would
|
||||||
|
trigger an unguarded egress at userinfo-fetch time. Fixed by
|
||||||
|
wrapping `ctx` via `SafeOIDCContext(ctx)` before both
|
||||||
|
`oauthConfig.TokenSource` and `provider.UserInfo`. Pinned by
|
||||||
|
`TestFetchUserinfoGroups_SSRF_BlocksReservedAddress`.
|
||||||
|
|
||||||
|
- **SEC-021 — Back-channel logout discovery re-fetch.**
|
||||||
|
`internal/api/handler/auth_session_oidc_bcl.go::Verify` performs
|
||||||
|
a per-request `gooidc.NewProvider(ctx, matched.IssuerURL)` to
|
||||||
|
fetch the JWKS for verifying the BCL token's signature. Same
|
||||||
|
bare-ctx shape — an IdP whose registered `IssuerURL` resolves to
|
||||||
|
a reserved address (or that is rebinding to one at logout time)
|
||||||
|
would dial an unguarded HTTPS egress. Fixed by wrapping via
|
||||||
|
`oidcsvc.SafeOIDCContext(ctx)` before `NewProvider`. Pinned by
|
||||||
|
`TestDefaultBCLVerifier_SSRF_BlocksReservedAddress`.
|
||||||
|
|
||||||
|
- **Context-key shape (why a single wrap covers both legs).**
|
||||||
|
`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 (Provider.UserInfo, NewProvider
|
||||||
|
discovery, Verifier JWKS) AND oauth2-driven HTTP
|
||||||
|
(Config.TokenSource refresh, Config.Exchange). No additional
|
||||||
|
`context.WithValue(ctx, oauth2.HTTPClient, ...)` is required.
|
||||||
|
|
||||||
|
- **Out-of-scope: RFC 1918.** Per the `IsReservedIP` policy
|
||||||
|
documented at [`internal/validation/ssrf.go:15-32`](../../internal/validation/ssrf.go),
|
||||||
|
RFC 1918 ranges are NOT treated as reserved by the SSRF guard.
|
||||||
|
certctl is designed to manage certificates inside private
|
||||||
|
networks; filtering 10/8 + 172.16/12 + 192.168/16 would break
|
||||||
|
the primary use case. Operators on hosted IaaS who want
|
||||||
|
RFC 1918 treated as reserved can opt in via the future
|
||||||
|
`CERTCTL_BLOCK_RFC1918_OUTBOUND` toggle (see acquisition-audit
|
||||||
|
Sprint 5 RED-005). The Sprint 1 SSRF parity fix above closes
|
||||||
|
the loopback / link-local / cloud-metadata leg only.
|
||||||
|
|
||||||
### OIDC first-admin bootstrap
|
### OIDC first-admin bootstrap
|
||||||
|
|
||||||
- **Coexists with the env-var-token bootstrap path.** Both can be
|
- **Coexists with the env-var-token bootstrap path.** Both can be
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import (
|
|||||||
|
|
||||||
gooidc "github.com/coreos/go-oidc/v3/oidc"
|
gooidc "github.com/coreos/go-oidc/v3/oidc"
|
||||||
|
|
||||||
|
oidcsvc "github.com/certctl-io/certctl/internal/auth/oidc"
|
||||||
oidcdomain "github.com/certctl-io/certctl/internal/auth/oidc/domain"
|
oidcdomain "github.com/certctl-io/certctl/internal/auth/oidc/domain"
|
||||||
"github.com/certctl-io/certctl/internal/repository"
|
"github.com/certctl-io/certctl/internal/repository"
|
||||||
)
|
)
|
||||||
@@ -122,7 +123,13 @@ func (v *DefaultBCLVerifier) Verify(ctx context.Context, logoutToken string) (is
|
|||||||
if v.verifyOverride != nil {
|
if v.verifyOverride != nil {
|
||||||
idToken, err = v.verifyOverride(ctx, matched.IssuerURL, logoutToken)
|
idToken, err = v.verifyOverride(ctx, matched.IssuerURL, logoutToken)
|
||||||
} else {
|
} else {
|
||||||
provider, perr := gooidc.NewProvider(ctx, matched.IssuerURL)
|
// Acquisition-audit SEC-021 closure (Sprint 1 follow-up to SEC-001,
|
||||||
|
// 2026-05-16). Per-request discovery re-fetch threaded through
|
||||||
|
// SafeOIDCContext so the dial-time SSRF guard
|
||||||
|
// (validation.SafeHTTPDialContext) re-resolves the issuer host and
|
||||||
|
// refuses reserved-address answers — matching the SEC-001 sweep
|
||||||
|
// over the runtime + dry-run discovery legs in internal/auth/oidc.
|
||||||
|
provider, perr := gooidc.NewProvider(oidcsvc.SafeOIDCContext(ctx), matched.IssuerURL)
|
||||||
if perr != nil {
|
if perr != nil {
|
||||||
return "", "", "", "", 0, fmt.Errorf("provider discovery: %w", perr)
|
return "", "", "", "", 0, fmt.Errorf("provider discovery: %w", perr)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
// Copyright 2026 certctl LLC. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: BUSL-1.1
|
||||||
|
|
||||||
|
package handler
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"encoding/base64"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
oidcdomain "github.com/certctl-io/certctl/internal/auth/oidc/domain"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Acquisition-audit SEC-021 closure (Sprint 1 follow-up to SEC-001,
|
||||||
|
// 2026-05-16). DefaultBCLVerifier.Verify performs a per-request
|
||||||
|
// discovery re-fetch via gooidc.NewProvider(ctx, matched.IssuerURL).
|
||||||
|
// Pre-fix, the bare ctx fell through to http.DefaultClient at the dial
|
||||||
|
// layer — no SSRF guard, no DNS-rebinding re-resolve. The fix wraps
|
||||||
|
// ctx via oidcsvc.SafeOIDCContext so the dial-time
|
||||||
|
// validation.SafeHTTPDialContext refuses reserved-address answers
|
||||||
|
// (loopback / link-local / cloud-metadata).
|
||||||
|
//
|
||||||
|
// This test pins the wrap end-to-end:
|
||||||
|
//
|
||||||
|
// 1. Construct a stubProviderRepo with one provider whose IssuerURL is
|
||||||
|
// a literal-loopback http:// URL (the literal-IP class that
|
||||||
|
// SafeHTTPDialContext.isReservedIPForDial refuses up-front, before
|
||||||
|
// any DNS resolution attempt).
|
||||||
|
// 2. Hand-roll a 3-segment JWT whose payload base64url-decodes to
|
||||||
|
// {"iss":"<loopback url>"} so peekIssuer extracts the matching
|
||||||
|
// issuer and provs.List() returns the seeded provider.
|
||||||
|
// 3. Call Verify. The discovery NewProvider call now routes through
|
||||||
|
// SafeOIDCContext; SafeHTTPDialContext sees the literal 127.0.0.1
|
||||||
|
// and refuses with "refusing to dial reserved address <ip>".
|
||||||
|
// 4. Assert the returned error wraps that rejection (substring match
|
||||||
|
// on "refusing to dial" / "reserved address") rather than a
|
||||||
|
// generic connect-refused or "did not respond" wrap.
|
||||||
|
//
|
||||||
|
// Companion to TestFetchUserinfoGroups_SSRF_BlocksReservedAddress in
|
||||||
|
// internal/auth/oidc/service_test.go which exercises the same wrap on
|
||||||
|
// the userinfo-fallback leg. Together they pin the post-SEC-001 sweep.
|
||||||
|
func TestDefaultBCLVerifier_SSRF_BlocksReservedAddress(t *testing.T) {
|
||||||
|
// Literal-loopback issuer URL. Port :1 keeps the URL syntactically
|
||||||
|
// valid; SafeHTTPDialContext refuses on the literal-IP check before
|
||||||
|
// the dial-time TCP connect, so the port choice is moot.
|
||||||
|
const reservedIssuer = "http://127.0.0.1:1"
|
||||||
|
|
||||||
|
provs := &stubProviderRepo{
|
||||||
|
provs: []*oidcdomain.OIDCProvider{
|
||||||
|
{ID: "op-loopback", IssuerURL: reservedIssuer, ClientID: "test-client"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
v := NewDefaultBCLVerifier(provs, "t-default", nil)
|
||||||
|
|
||||||
|
// Hand-roll the JWT. peekIssuer (see auth_session_oidc_bcl.go) parses
|
||||||
|
// only the iss claim from the 2nd segment (payload), so the header +
|
||||||
|
// signature segments only need to be syntactically present.
|
||||||
|
header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256"}`))
|
||||||
|
payload := base64.RawURLEncoding.EncodeToString([]byte(`{"iss":"` + reservedIssuer + `"}`))
|
||||||
|
logoutToken := header + "." + payload + ".sig"
|
||||||
|
|
||||||
|
_, _, _, _, _, err := v.Verify(context.Background(), logoutToken)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("Verify against literal-loopback issuer URL: expected SSRF reject; got nil")
|
||||||
|
}
|
||||||
|
msg := err.Error()
|
||||||
|
if !strings.Contains(msg, "refusing to dial") && !strings.Contains(msg, "reserved address") {
|
||||||
|
t.Errorf("Verify err = %q; want SafeHTTPDialContext reserved-address rejection", msg)
|
||||||
|
}
|
||||||
|
// Also confirm the error is wrapped through the Verify "provider
|
||||||
|
// discovery:" prefix so callers can distinguish a discovery-time
|
||||||
|
// dial failure from a signature-verification failure.
|
||||||
|
if !strings.Contains(msg, "provider discovery") {
|
||||||
|
t.Errorf("Verify err = %q; want \"provider discovery:\" wrap", msg)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user