diff --git a/internal/handlers/anonymous_credentials_test.go b/internal/handlers/anonymous_credentials_test.go new file mode 100644 index 0000000..4d4f2fd --- /dev/null +++ b/internal/handlers/anonymous_credentials_test.go @@ -0,0 +1,218 @@ +package handlers + +import ( + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/dependabot/proxy/internal/config" +) + +// Anonymous registries are declared without any credential so that their host +// lands in the per-job egress allowlist. The credential must therefore still be +// present in the job config, which previously caused each handler to attach an +// empty Authorization header ("Bearer " with no value, or Basic base64(":")). +// Nexus and Artifactory reject that outright rather than serving the request +// anonymously, so the request must instead be forwarded untouched. + +// TestCredentialIsAnonymous pins which credential shapes count as anonymous. +// The two empty-secret shapes arrive by different routes: an org-level registry +// is serialized with no token key at all, while a repo-level one is serialized +// by dependabot-api as token ":" when neither username nor password is set. +func TestCredentialIsAnonymous(t *testing.T) { + testCases := []struct { + name string + cred config.Credential + expected bool + }{ + { + name: "url only (org-level anonymous)", + cred: config.Credential{"type": "npm_registry", "url": "https://nexus.example.net"}, + expected: true, + }, + { + name: "colon token (repo-level anonymous)", + cred: config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "token": ":"}, + expected: true, + }, + { + name: "empty token", + cred: config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "token": ""}, + expected: true, + }, + { + name: "token", + cred: config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "token": "secret"}, + expected: false, + }, + { + name: "username and password", + cred: config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "username": "u", "password": "p"}, + expected: false, + }, + { + // A username with no password is a misconfiguration, not an anonymous + // registry. Treating it as anonymous would silently drop an intended + // credential, so it stays registered and keeps its existing behaviour. + name: "username only is not anonymous", + cred: config.Credential{"type": "npm_registry", "url": "https://nexus.example.net", "username": "u"}, + expected: false, + }, + { + name: "auth-key", + cred: config.Credential{"type": "hex_repository", "url": "https://hex.example.net", "auth-key": "k"}, + expected: false, + }, + { + name: "key", + cred: config.Credential{"type": "hex_organization", "organization": "o", "key": "k"}, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, credentialIsAnonymous(tc.cred)) + }) + } +} + +// TestAnonymousCredentialsAreNotAuthenticated covers every handler that did not +// already skip credential-free entries. Each asserts no Authorization header is +// added at all -- an empty "Bearer " or Basic base64(":") would fail this. +func TestAnonymousCredentialsAreNotAuthenticated(t *testing.T) { + testCases := []struct { + name string + cred config.Credential + target string + handlerFactory func(config.Credentials) oidcHandler + }{ + { + name: "npm_registry", + cred: config.Credential{"type": "npm_registry", "registry": "https://nexus.example.net/repository/npm-all"}, + target: "https://nexus.example.net/repository/npm-all/some-package", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewNPMRegistryHandler(creds, testOIDCClient) + }, + }, + { + name: "composer_repository", + cred: config.Credential{"type": "composer_repository", "registry": "https://nexus.example.net/repository/composer"}, + target: "https://nexus.example.net/repository/composer/packages.json", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewComposerHandler(creds, testOIDCClient) + }, + }, + { + name: "helm_registry", + cred: config.Credential{"type": "helm_registry", "registry": "https://nexus.example.net/repository/helm"}, + target: "https://nexus.example.net/repository/helm/index.yaml", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewHelmRegistryHandler(creds, testOIDCClient) + }, + }, + { + name: "python_index", + cred: config.Credential{"type": "python_index", "index-url": "https://nexus.example.net/repository/pypi/simple"}, + target: "https://nexus.example.net/repository/pypi/simple/requests/", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewPythonIndexHandler(creds, testOIDCClient) + }, + }, + { + name: "rubygems_server", + cred: config.Credential{"type": "rubygems_server", "url": "https://nexus.example.net/repository/gems"}, + target: "https://nexus.example.net/repository/gems/specs.4.8.gz", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewRubyGemsServerHandler(creds, testOIDCClient) + }, + }, + { + name: "docker_registry", + cred: config.Credential{"type": "docker_registry", "registry": "nexus.example.net"}, + target: "https://nexus.example.net/v2/library/alpine/manifests/latest", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewDockerRegistryHandler(creds, testOIDCClient, nil) + }, + }, + { + name: "nuget_feed", + cred: config.Credential{"type": "nuget_feed", "url": "https://nexus.example.net/repository/nuget/index.json"}, + target: "https://nexus.example.net/repository/nuget/index.json", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewNugetFeedHandler(creds, testOIDCClient) + }, + }, + { + name: "cargo_registry", + cred: config.Credential{"type": "cargo_registry", "url": "https://nexus.example.net/repository/cargo"}, + target: "https://nexus.example.net/repository/cargo/api/v1/crates/serde", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewCargoRegistryHandler(creds, testOIDCClient) + }, + }, + { + name: "terraform_registry", + cred: config.Credential{"type": "terraform_registry", "url": "https://nexus.example.net/terraform"}, + target: "https://nexus.example.net/terraform/v1/modules/foo/bar/aws/versions", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewTerraformRegistryHandler(creds, testOIDCClient) + }, + }, + { + name: "pub_repository", + cred: config.Credential{"type": "pub_repository", "url": "https://nexus.example.net/pub"}, + target: "https://nexus.example.net/pub/api/packages/http", + handlerFactory: func(creds config.Credentials) oidcHandler { + return NewPubRepositoryHandler(creds, testOIDCClient) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + handler := tc.handlerFactory(config.Credentials{tc.cred}) + req := httptest.NewRequestWithContext(t.Context(), "GET", tc.target, nil) + req = handleRequestAndClose(handler, req, nil) + assertUnauthenticated(t, req, "anonymous "+tc.name+" request") + }) + } + + // The repo-level anonymous shape arrives as token ":" rather than as a + // missing key. Several handlers guard only on an empty token, so this shape + // reaches the request path and becomes Basic base64(":"), "Bearer :", or a + // raw "Authorization: :" depending on the handler. + t.Run("colon token", func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cred := config.Credential{"token": ":"} + for k, v := range tc.cred { + cred[k] = v + } + + handler := tc.handlerFactory(config.Credentials{cred}) + req := httptest.NewRequestWithContext(t.Context(), "GET", tc.target, nil) + req = handleRequestAndClose(handler, req, nil) + assertUnauthenticated(t, req, "colon-token "+tc.name+" request") + }) + } + }) +} + +// TestAnonymousCredentialsStillAuthenticateWhenSecretPresent guards against the +// anonymous skip being too eager: the same handlers must keep authenticating a +// credential that does carry a secret. +func TestAnonymousCredentialsStillAuthenticateWhenSecretPresent(t *testing.T) { + handler := NewNPMRegistryHandler(config.Credentials{ + config.Credential{ + "type": "npm_registry", + "registry": "https://nexus.example.net/repository/npm-all", + "token": "s3cret", + }, + }, testOIDCClient) + + req := httptest.NewRequestWithContext(t.Context(), "GET", "https://nexus.example.net/repository/npm-all/some-package", nil) + req = handleRequestAndClose(handler, req, nil) + assertHasTokenAuth(t, req, "Bearer", "s3cret", "credentialed npm request") +} diff --git a/internal/handlers/cargo_registry.go b/internal/handlers/cargo_registry.go index 7109e34..5133960 100644 --- a/internal/handlers/cargo_registry.go +++ b/internal/handlers/cargo_registry.go @@ -103,6 +103,12 @@ func NewCargoRegistryHandler(credentials config.Credentials, client *http.Client continue } + // A ":" token passes the guard above but carries no credentials, and + // SetRawAuthorization would send it verbatim as "Authorization: :". + if credentialIsAnonymous(credential) { + continue + } + handler.credentials = append(handler.credentials, cargoCred) } return &handler diff --git a/internal/handlers/composer.go b/internal/handlers/composer.go index 8533908..5bcfe5f 100644 --- a/internal/handlers/composer.go +++ b/internal/handlers/composer.go @@ -45,6 +45,12 @@ func NewComposerHandler(creds config.Credentials, client *http.Client) *Composer continue } + // An anonymous registry is allowlisted by its url/registry field but must + // not have an empty Authorization header attached to its requests. + if credentialIsAnonymous(cred) { + continue + } + composerCred := composerCredentials{ registry: registry, url: url, diff --git a/internal/handlers/credentials.go b/internal/handlers/credentials.go index 8c6762b..0d23a22 100644 --- a/internal/handlers/credentials.go +++ b/internal/handlers/credentials.go @@ -4,6 +4,8 @@ import ( "encoding/base64" "net/http" "strings" + + "github.com/dependabot/proxy/internal/config" ) const authorizationPlaceholder = "******" @@ -37,3 +39,42 @@ func hasUsableAuthorization(req *http.Request) bool { func proxyOnlyCredentialRequestAllowed(req *http.Request) bool { return req.URL.Scheme == "https" && (req.URL.Port() == "" || req.URL.Port() == "443") } + +// credentialSecretKeys are the credential fields that can carry secret material. +// "username" is included because a username-only credential still represents an +// intent to authenticate, and must not be mistaken for an anonymous one. +var credentialSecretKeys = []string{ + "token", + "password", + "auth-key", + "key", + "username", +} + +// credentialIsAnonymous reports whether a credential names a registry but +// carries nothing to authenticate with. +// +// An anonymous registry must still appear in the job's credentials, because the +// egress allowlist derives its per-job hosts from exactly that list (see +// credentialHosts in egress_dynamic_hosts.go). Its presence would otherwise +// cause a handler to attach an empty Authorization header -- "Bearer " with no +// value, or Basic base64(":") -- which Nexus and Artifactory reject outright +// instead of falling back to anonymous access. Skipping registration lets the +// request through unauthenticated, which is what such a registry expects, while +// leaving the host allowlisted. +// +// A token of ":" is treated as absent: it is what dependabot-api emits for a +// registry configured with neither a username nor a password, and it decodes to +// an empty username and password. +// +// Callers must apply this only after OIDC registration has been attempted, so +// that a credential which mints its token at request time is never mistaken for +// an anonymous one. +func credentialIsAnonymous(cred config.Credential) bool { + for _, key := range credentialSecretKeys { + if value := cred.GetString(key); value != "" && value != ":" { + return false + } + } + return true +} diff --git a/internal/handlers/docker_registry.go b/internal/handlers/docker_registry.go index 22dece5..26cc62b 100644 --- a/internal/handlers/docker_registry.go +++ b/internal/handlers/docker_registry.go @@ -76,6 +76,12 @@ func NewDockerRegistryHandler(creds config.Credentials, client *http.Client, get if proxyOnly && cred.GetString("username") == "" && cred.GetString("password") == "" { continue } + // An anonymous registry is allowlisted by its registry field but must not + // have an empty Authorization header attached to its requests. ECR + // credentials carry an AWS key pair, so they are never anonymous here. + if credentialIsAnonymous(cred) { + continue + } parsedRegistry, err := helpers.ParseURLLax(registryLocation) if err != nil || parsedRegistry.Host == "" { diff --git a/internal/handlers/egress_allowlist_test.go b/internal/handlers/egress_allowlist_test.go index a00e3fc..fb87892 100644 --- a/internal/handlers/egress_allowlist_test.go +++ b/internal/handlers/egress_allowlist_test.go @@ -1447,3 +1447,79 @@ func TestEgressAllowlist_ChangelogHostsThirdWaveAllowed(t *testing.T) { } } } + +// TestEgressAllowlist_AnonymousCredentialAllowlistsHost pins the behaviour the +// org-level Anonymous registry option depends on: a credential carrying only a +// registry location, with no secret of any kind, must still contribute its host +// to the per-job allowlist. This is why an anonymous registry has to appear in +// the job credentials at all, and therefore why the handlers must skip +// authenticating it rather than the config omitting it. +func TestEgressAllowlist_AnonymousCredentialAllowlistsHost(t *testing.T) { + for _, tc := range []struct { + name string + cred config.Credential + host string + }{ + { + name: "npm_registry via registry", + cred: config.Credential{"type": "npm_registry", "registry": "nexus.example.net/repository/npm-all"}, + host: "nexus.example.net", + }, + { + name: "docker_registry via registry", + cred: config.Credential{"type": "docker_registry", "registry": "docker.example.net"}, + host: "docker.example.net", + }, + { + name: "python_index via index-url", + cred: config.Credential{"type": "python_index", "index-url": "https://pypi.example.net/simple"}, + host: "pypi.example.net", + }, + { + name: "maven_repository via url", + cred: config.Credential{"type": "maven_repository", "url": "https://maven.example.net/releases"}, + host: "maven.example.net", + }, + { + name: "nuget_feed via url", + cred: config.Credential{"type": "nuget_feed", "url": "https://nuget.example.net/v3/index.json"}, + host: "nuget.example.net", + }, + { + name: "colon token is still anonymous", + cred: config.Credential{"type": "npm_registry", "registry": "colon.example.net", "token": ":"}, + host: "colon.example.net", + }, + } { + t.Run(tc.name, func(t *testing.T) { + h := newEgressHandlerWithCreds(config.Credentials{tc.cred}) + assert.Nil(t, egressResult(t, h, "https://"+tc.host+"/some/path"), + "anonymous credential must allowlist its host: "+tc.host) + }) + } +} + +// TestEgressAllowlist_AnonymousCredentialDoesNotWidenBeyondExactHost holds the +// dynamic-host exact-match invariant for credential-free entries specifically. +// Dynamic hosts are matched with AreHostnamesEqual only, so declaring an +// anonymous registry must authorize that one host and nothing beneath or beside +// it. These probes fail the moment dynamic matching is relaxed to a suffix. +func TestEgressAllowlist_AnonymousCredentialDoesNotWidenBeyondExactHost(t *testing.T) { + h := newEgressHandlerWithCreds(config.Credentials{ + config.Credential{"type": "npm_registry", "registry": "nexus.example.net/repository/npm-all"}, + }) + + for _, blocked := range []string{ + // Child probe: fails if the dynamic entry becomes a leading-dot suffix. + "https://evil.nexus.example.net/repository/npm-all", + // Sibling probe: fails if the entry is widened to its parent namespace. + "https://attacker.example.net/repository/npm-all", + // Suffix-appending lookalike. + "https://nexus.example.net.attacker.com/repository/npm-all", + } { + resp := egressResult(t, h, blocked) + if assert.NotNil(t, resp, "must stay blocked: "+blocked) { + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + } + } +} diff --git a/internal/handlers/helm_registry.go b/internal/handlers/helm_registry.go index 639ced0..847fe7d 100644 --- a/internal/handlers/helm_registry.go +++ b/internal/handlers/helm_registry.go @@ -45,6 +45,12 @@ func NewHelmRegistryHandler(creds config.Credentials, client *http.Client) *Helm continue } + // An anonymous registry is allowlisted by its registry field but must not + // have an empty Authorization header attached to its requests. + if credentialIsAnonymous(cred) { + continue + } + helmCred := helmRegistryCredentials{ registry: registry, username: cred.GetString("username"), diff --git a/internal/handlers/npm_registry.go b/internal/handlers/npm_registry.go index c4607c4..d1e6220 100644 --- a/internal/handlers/npm_registry.go +++ b/internal/handlers/npm_registry.go @@ -55,6 +55,11 @@ func NewNPMRegistryHandler(creds config.Credentials, client *http.Client) *NPMRe if proxyOnly && cred.GetString("token") == "" && cred.GetString("password") == "" { continue } + // An anonymous registry is allowlisted by its url/registry field but must + // not have an empty Authorization header attached to its requests. + if credentialIsAnonymous(cred) { + continue + } npmCred := npmRegistryCredentials{ registry: registry, diff --git a/internal/handlers/nuget_feed.go b/internal/handlers/nuget_feed.go index e4b929b..979840a 100644 --- a/internal/handlers/nuget_feed.go +++ b/internal/handlers/nuget_feed.go @@ -108,6 +108,16 @@ func NewNugetFeedHandler(creds config.Credentials, client *http.Client) *NugetFe } } + // An anonymous feed is allowlisted by its url field but must not have an + // empty Authorization header attached to its requests. This runs after + // OIDC registration so a token minted at request time is never treated as + // anonymous. The guard below only tests for an empty token, so a ":" token + // would otherwise reach authenticateNugetRequest and become Basic + // base64(":"). + if credentialIsAnonymous(cred) { + continue + } + feedCred := nugetFeedCredentials{ url: url, host: host, diff --git a/internal/handlers/pub_repository.go b/internal/handlers/pub_repository.go index 3086174..c1b8102 100644 --- a/internal/handlers/pub_repository.go +++ b/internal/handlers/pub_repository.go @@ -61,6 +61,12 @@ func NewPubRepositoryHandler(credentials config.Credentials, client *http.Client logrus.Warnf("missing token for hosted url (%s)", pubCred.url) continue } + + // A ":" token passes the guard above but carries no credentials, and + // would be sent as "Bearer :". + if credentialIsAnonymous(credential) { + continue + } handler.credentials = append(handler.credentials, pubCred) } return &handler diff --git a/internal/handlers/python_index.go b/internal/handlers/python_index.go index cb36ab7..c0b8416 100644 --- a/internal/handlers/python_index.go +++ b/internal/handlers/python_index.go @@ -64,6 +64,12 @@ func NewPythonIndexHandler(creds config.Credentials, client *http.Client) *Pytho continue } + // An anonymous index is allowlisted by its index-url/url field but must not + // have an empty Authorization header attached to its requests. + if credentialIsAnonymous(cred) { + continue + } + indexCred := pythonIndexCredentials{ indexURL: indexURL, token: cred.GetString("token"), diff --git a/internal/handlers/rubygems_server.go b/internal/handlers/rubygems_server.go index 7b8c85f..3eaad18 100644 --- a/internal/handlers/rubygems_server.go +++ b/internal/handlers/rubygems_server.go @@ -50,6 +50,11 @@ func NewRubyGemsServerHandler(creds config.Credentials, client *http.Client) *Ru if proxyOnly && cred.GetString("token") == "" { continue } + // An anonymous server is allowlisted by its url field but must not have an + // empty Authorization header attached to its requests. + if credentialIsAnonymous(cred) { + continue + } serverCred := rubyGemsServerCredentials{ host: host, diff --git a/internal/handlers/terraform_registry.go b/internal/handlers/terraform_registry.go index 5e2d7f8..08af8b3 100644 --- a/internal/handlers/terraform_registry.go +++ b/internal/handlers/terraform_registry.go @@ -49,6 +49,12 @@ func NewTerraformRegistryHandler(credentials config.Credentials, client *http.Cl continue } + // A ":" token passes the guard above but carries no credentials, and + // would be sent as "Bearer :". + if credentialIsAnonymous(credential) { + continue + } + terraformCred := terraformRegistryCredentials{ url: url, token: token,