diff --git a/config-appview.example.yaml b/config-appview.example.yaml index eb319bb..ce56aed 100644 --- a/config-appview.example.yaml +++ b/config-appview.example.yaml @@ -76,6 +76,8 @@ jetstream: auth: # X.509 certificate matching the JWT signing key (auto-generated on each boot from the JWT key in the database). cert_path: /var/lib/atcr/auth/private-key.crt + # Allow unauthenticated Docker pulls from public holds. Per-hold privacy (captain.Public) still applies. Default true. + allow_anonymous_pull: true # Credential helper download settings. credential_helper: # Tangled repository URL for credential helper downloads. diff --git a/deploy/upcloud/configs/appview.yaml.tmpl b/deploy/upcloud/configs/appview.yaml.tmpl index e48b4ce..c0cf917 100644 --- a/deploy/upcloud/configs/appview.yaml.tmpl +++ b/deploy/upcloud/configs/appview.yaml.tmpl @@ -41,6 +41,9 @@ jetstream: - https://relay1.us-west.bsky.network auth: cert_path: "{{.BasePath}}/auth/private-key.crt" + # Allow unauthenticated Docker pulls from public holds. Per-hold privacy + # (captain.Public) still applies, and push always requires credentials. + allow_anonymous_pull: true legal: company_name: Seamark jurisdiction: State of Texas, United States diff --git a/pkg/appview/config.go b/pkg/appview/config.go index 41fe578..6442d81 100644 --- a/pkg/appview/config.go +++ b/pkg/appview/config.go @@ -126,6 +126,10 @@ type AuthConfig struct { // X.509 certificate matching the JWT signing key. CertPath string `yaml:"cert_path" comment:"X.509 certificate matching the JWT signing key (auto-generated on each boot from the JWT key in the database)."` + // AllowAnonymousPull permits credential-less Docker pulls. Per-hold privacy + // (captain.Public) still applies — a private hold rejects anonymous reads. + AllowAnonymousPull bool `yaml:"allow_anonymous_pull" comment:"Allow unauthenticated Docker pulls from public holds. Per-hold privacy (captain.Public) still applies. Default true."` + // TokenExpiration is the JWT expiration duration (5 minutes, not configurable) TokenExpiration time.Duration `yaml:"-"` @@ -235,6 +239,7 @@ func setDefaults(v *viper.Viper) { // Auth defaults v.SetDefault("auth.cert_path", "/var/lib/atcr/auth/private-key.crt") + v.SetDefault("auth.allow_anonymous_pull", true) // Log shipper defaults v.SetDefault("log_shipper.batch_size", 100) diff --git a/pkg/appview/middleware/bearer_challenge.go b/pkg/appview/middleware/bearer_challenge.go new file mode 100644 index 0000000..19ca7c1 --- /dev/null +++ b/pkg/appview/middleware/bearer_challenge.go @@ -0,0 +1,75 @@ +package middleware + +import ( + "fmt" + "net/http" + + "atcr.io/pkg/auth/token" +) + +// bearerChallengeResponseWriter wraps http.ResponseWriter and, on the first +// WriteHeader call, injects a Bearer WWW-Authenticate challenge when the status +// is 401 and no challenge was already set. This covers 401s produced deep in the +// handler stack (e.g. the blob proxy denying an anonymous read of a private +// hold) that are served via errcode.ServeJSON and would otherwise carry no +// WWW-Authenticate header, leaving Docker clients with no hint to re-authenticate. +type bearerChallengeResponseWriter struct { + http.ResponseWriter + challenge string + wroteHeader bool +} + +func (w *bearerChallengeResponseWriter) WriteHeader(code int) { + if !w.wroteHeader { + w.wroteHeader = true + if code == http.StatusUnauthorized && w.Header().Get("WWW-Authenticate") == "" { + w.Header().Set("WWW-Authenticate", w.challenge) + } + } + w.ResponseWriter.WriteHeader(code) +} + +func (w *bearerChallengeResponseWriter) Write(b []byte) (int, error) { + if !w.wroteHeader { + // Implicit 200 — still fire WriteHeader so the flag flips. + w.WriteHeader(http.StatusOK) + } + return w.ResponseWriter.Write(b) +} + +// BearerChallenge returns middleware that guarantees every 401 in the registry +// subtree carries a Bearer WWW-Authenticate challenge. The distribution +// library's own challenges (which include a scope) are left untouched; only +// bare 401s get the fallback header. realm is the token endpoint URL. +// +// services is every registry domain this AppView fronts, in priority order, the +// same list the atcr-token access controller is built from. The challenge names +// the domain the request arrived on, because that controller demands its own +// domain's audience: advertising a different service would send the client back +// with a token that host then rejects. A request on a host that is not a +// configured domain falls back to services[0], matching the controller's own +// fallback. A single-element list makes every challenge identical. +func BearerChallenge(realm string, services []string) func(http.Handler) http.Handler { + byHost := make(map[string]string, len(services)) + for _, svc := range services { + byHost[token.NormalizeService(svc)] = fmt.Sprintf(`Bearer realm=%q,service=%q`, realm, svc) + } + + var fallback string + if len(services) > 0 { + fallback = byHost[token.NormalizeService(services[0])] + } else { + fallback = fmt.Sprintf(`Bearer realm=%q`, realm) + } + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + challenge, ok := byHost[token.NormalizeService(r.Host)] + if !ok { + challenge = fallback + } + wrapped := &bearerChallengeResponseWriter{ResponseWriter: w, challenge: challenge} + next.ServeHTTP(wrapped, r) + }) + } +} diff --git a/pkg/appview/middleware/bearer_challenge_test.go b/pkg/appview/middleware/bearer_challenge_test.go new file mode 100644 index 0000000..b5cc1c1 --- /dev/null +++ b/pkg/appview/middleware/bearer_challenge_test.go @@ -0,0 +1,125 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestBearerChallenge(t *testing.T) { + const realm = "https://atcr.io/auth/token" + services := []string{"atcr.io", "buoy.cr"} + wantChallenge := `Bearer realm="https://atcr.io/auth/token",service="atcr.io"` + + tests := []struct { + name string + handler http.HandlerFunc + wantCode int + wantChallenge string + }{ + { + name: "bare 401 gets a challenge injected", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }, + wantCode: http.StatusUnauthorized, + wantChallenge: wantChallenge, + }, + { + name: "401 with existing challenge is left untouched", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer realm="x",service="y",scope="repository:a/b:pull"`) + w.WriteHeader(http.StatusUnauthorized) + }, + wantCode: http.StatusUnauthorized, + wantChallenge: `Bearer realm="x",service="y",scope="repository:a/b:pull"`, + }, + { + name: "200 is not given a challenge", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }, + wantCode: http.StatusOK, + wantChallenge: "", + }, + { + name: "implicit 200 via Write is not given a challenge", + handler: func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("ok")) + }, + wantCode: http.StatusOK, + wantChallenge: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wrapped := BearerChallenge(realm, services)(tt.handler) + req := httptest.NewRequest(http.MethodGet, "/v2/alice/app/blobs/sha256:abc", nil) + req.Host = "atcr.io" + w := httptest.NewRecorder() + + wrapped.ServeHTTP(w, req) + + if w.Code != tt.wantCode { + t.Errorf("status = %d, want %d", w.Code, tt.wantCode) + } + if got := w.Header().Get("WWW-Authenticate"); got != tt.wantChallenge { + t.Errorf("WWW-Authenticate = %q, want %q", got, tt.wantChallenge) + } + }) + } +} + +// The challenge has to name the domain the request arrived on: the atcr-token +// access controller demands that domain's own audience, so advertising another +// would send the client back with a token this host rejects. +func TestBearerChallengePerDomain(t *testing.T) { + const realm = "https://seamark.dev/auth/token" + services := []string{"atcr.io", "buoy.cr"} + + tests := []struct { + name string + host string + want string + }{ + { + name: "primary domain", + host: "atcr.io", + want: `Bearer realm="https://seamark.dev/auth/token",service="atcr.io"`, + }, + { + name: "secondary domain gets its own service", + host: "buoy.cr", + want: `Bearer realm="https://seamark.dev/auth/token",service="buoy.cr"`, + }, + { + name: "host with port still matches", + host: "buoy.cr:443", + want: `Bearer realm="https://seamark.dev/auth/token",service="buoy.cr"`, + }, + { + name: "unconfigured host falls back to the primary", + host: "elsewhere.example", + want: `Bearer realm="https://seamark.dev/auth/token",service="atcr.io"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wrapped := BearerChallenge(realm, services)(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + req := httptest.NewRequest(http.MethodGet, "/v2/alice/app/blobs/sha256:abc", nil) + req.Host = tt.host + w := httptest.NewRecorder() + + wrapped.ServeHTTP(w, req) + + if got := w.Header().Get("WWW-Authenticate"); got != tt.want { + t.Errorf("WWW-Authenticate = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index ec140e2..0aea1a5 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -11,6 +11,7 @@ import ( "github.com/distribution/distribution/v3" "github.com/distribution/distribution/v3/registry/api/errcode" + v2 "github.com/distribution/distribution/v3/registry/api/v2" registrymw "github.com/distribution/distribution/v3/registry/middleware/registry" "github.com/distribution/distribution/v3/registry/storage/driver" "github.com/distribution/reference" @@ -317,10 +318,22 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name identityStr = decoded } - // Resolve identity to DID, handle, and PDS endpoint + // Resolve identity to DID, handle, and PDS endpoint. + // + // A bare error here reaches distribution as ErrorCodeUnknown and becomes a + // 500. That is wrong on its own terms — an unresolvable name is a bad request, + // not a server fault — and anonymous pull makes it reachable by any + // unauthenticated caller, so `/v2//x/manifests/y` would answer 500 to + // the internet and send clients that retry 5xx into a retry loop. NAME_UNKNOWN + // is the OCI-correct answer and terminates the client immediately. did, handle, pdsEndpoint, err := atproto.ResolveIdentity(ctx, identityStr) if err != nil { - return nil, err + slog.Debug("Identity resolution failed", + "component", "registry/middleware", "identity", identityStr, "error", err) + return nil, errcode.Error{ + Code: v2.ErrorCodeNameUnknown, + Message: fmt.Sprintf("repository name not known to registry: %s", identityStr), + } } slog.Debug("Resolved identity", "component", "registry/middleware", "did", did, "pds", pdsEndpoint, "handle", handle) @@ -525,6 +538,7 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name PullerDID: pullerDID, // Authenticated user making the request PullerPDSEndpoint: pullerPDSEndpoint, // Puller's PDS for service token refresh HasPushScope: hasPushScope, // Whether JWT has push scope (for pull stats filtering) + Anonymous: pullerDID == "", // No puller identity: hold decides via captain.Public AutoRemoveUntagged: sailorProfile != nil && sailorProfile.AutoRemoveUntagged, Database: nr.database, Authorizer: nr.authorizer, diff --git a/pkg/appview/server.go b/pkg/appview/server.go index cc6ca6a..7028242 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -521,7 +521,10 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, // Wrap with auth method extraction middleware, then with the Retry-After // emitter so it can read the carrier installed before deeper handlers run. - wrappedApp := middleware.RetryAfterMiddleware(middleware.ExtractAuthMethod(app)) + // Outermost is the Bearer challenge guard, so any 401 from deep in the stack + // (e.g. anonymous read of a private hold) still carries WWW-Authenticate. + wrappedApp := middleware.BearerChallenge(cfg.Server.BaseURL+"/auth/token", cfg.Auth.Services)( + middleware.RetryAfterMiddleware(middleware.ExtractAuthMethod(app))) // Mount registry at /v2/ mainRouter.Handle("/v2/*", wrappedApp) @@ -590,6 +593,11 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, // the audience names the front door actually used. tokenHandler.SetServices(cfg.Auth.Services) + // Anonymous pull: when enabled, credential-less pull-only requests get a + // token with an empty subject; the destination hold still enforces + // captain.Public. Push always requires credentials. + tokenHandler.SetAllowAnonymousPull(cfg.Auth.AllowAnonymousPull) + tokenHandler.SetOAuthSessionValidator(s.Refresher) // Auth-phase gate: crew reconciliation for any token request, plus diff --git a/pkg/appview/storage/context.go b/pkg/appview/storage/context.go index 4f9d4ba..7cc40cb 100644 --- a/pkg/appview/storage/context.go +++ b/pkg/appview/storage/context.go @@ -73,6 +73,7 @@ type RegistryContext struct { PullerDID string // Puller's DID - who is making the request (from JWT Subject) PullerPDSEndpoint string // Puller's PDS endpoint URL HasPushScope bool // Whether the JWT token has push scope (used to filter pull stats) + Anonymous bool // Request carries no puller identity (anonymous pull); hold decides via captain.Public // Per-request user preferences AutoRemoveUntagged bool // Whether to auto-delete untagged manifests on tag overwrite diff --git a/pkg/appview/storage/proxy_blob_store.go b/pkg/appview/storage/proxy_blob_store.go index e0844bf..4b79774 100644 --- a/pkg/appview/storage/proxy_blob_store.go +++ b/pkg/appview/storage/proxy_blob_store.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -61,20 +62,18 @@ func NewProxyBlobStore(ctx *RegistryContext) *ProxyBlobStore { } } -// doAuthenticatedRequest performs an HTTP request with service token authentication -// Uses the service token from middleware to authenticate requests to the hold service +// doAuthenticatedRequest performs an HTTP request to the hold service, attaching +// the service token when one is present. An empty service token means an +// anonymous pull: the request is sent without an Authorization header and the +// hold authorizes it per captain.Public. Write call sites (multipart upload) are +// push-only and always carry a service token, so they never go out unauthenticated. func (p *ProxyBlobStore) doAuthenticatedRequest(ctx context.Context, req *http.Request) (*http.Response, error) { - // Use service token that middleware already validated and cached - // Middleware fails fast with HTTP 401 if OAuth session is invalid - if p.ctx.ServiceToken == "" { - // Should never happen - middleware validates OAuth before handlers run - slog.Error("No service token in context", "component", "proxy_blob_store", "did", p.ctx.DID) - return nil, fmt.Errorf("no service token available (middleware should have validated)") + // Service token was validated and cached by middleware (which fails fast with + // HTTP 401 if the OAuth session is invalid). Anonymous reads have none. + if p.ctx.ServiceToken != "" { + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", p.ctx.ServiceToken)) } - // Add Bearer token to Authorization header - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", p.ctx.ServiceToken)) - return p.httpClient.Do(req) } @@ -88,7 +87,14 @@ func (p *ProxyBlobStore) checkReadAccess(ctx context.Context) error { return fmt.Errorf("authorization check failed: %w", err) } if !allowed { - // Return 403 Forbidden instead of masquerading as missing blob + if p.ctx.Anonymous { + // Anonymous request to a private hold: surface a 401 so the Docker + // client prompts for credentials rather than treating it as a hard + // 403. The BearerChallenge middleware attaches WWW-Authenticate. + return errcode.ErrorCodeUnauthorized.WithMessage("authentication required") + } + // Authenticated but unauthorized: 403 Forbidden instead of masquerading + // as a missing blob, and without bouncing the user back to re-auth. return errcode.ErrorCodeDenied.WithMessage("read access denied") } return nil @@ -105,6 +111,15 @@ func (p *ProxyBlobStore) Stat(ctx context.Context, dgst digest.Digest) (distribu url, err := p.getPresignedURL(ctx, method, dgst) if err != nil { + // Preserve an authorization verdict. distribution calls Stat before + // ServeBlob on both GET and HEAD, so flattening everything to + // ErrBlobUnknown here turns the hold's "private, authenticate first" into + // a 404 and leaves BearerChallenge with no 401 to annotate — the client + // is told the blob doesn't exist instead of being asked for credentials. + var ecErr errcode.Error + if errors.As(err, &ecErr) { + return distribution.Descriptor{}, err + } return distribution.Descriptor{}, distribution.ErrBlobUnknown } @@ -362,6 +377,12 @@ func (p *ProxyBlobStore) getPresignedURL(ctx context.Context, operation string, } defer resp.Body.Close() + if resp.StatusCode == http.StatusForbidden && p.ctx.Anonymous { + // Stale local captain cache let an anonymous request through, but the + // hold says private. Surface a 401 so the client re-authenticates. + return "", errcode.ErrorCodeUnauthorized.WithMessage("authentication required") + } + if resp.StatusCode != http.StatusOK { bodyBytes, _ := io.ReadAll(resp.Body) return "", fmt.Errorf("hold service returned error: status %d, body: %s", resp.StatusCode, string(bodyBytes)) diff --git a/pkg/appview/storage/proxy_blob_store_test.go b/pkg/appview/storage/proxy_blob_store_test.go index fe321db..fb59a18 100644 --- a/pkg/appview/storage/proxy_blob_store_test.go +++ b/pkg/appview/storage/proxy_blob_store_test.go @@ -18,9 +18,74 @@ import ( "atcr.io/pkg/atproto" "atcr.io/pkg/auth" "github.com/distribution/distribution/v3" + "github.com/distribution/distribution/v3/registry/api/errcode" "github.com/opencontainers/go-digest" ) +// readAccessStub is a HoldAuthorizer whose read decision is configurable, for +// exercising checkReadAccess. All other methods are inert. +type readAccessStub struct{ allow bool } + +func (s readAccessStub) GetCaptainRecord(context.Context, string) (*atproto.CaptainRecord, error) { + return nil, nil +} +func (s readAccessStub) CheckReadAccess(context.Context, string, string) (bool, error) { + return s.allow, nil +} +func (s readAccessStub) CheckWriteAccess(context.Context, string, string) (bool, error) { + return false, nil +} +func (s readAccessStub) IsCrewMember(context.Context, string, string) (bool, error) { + return false, nil +} +func (s readAccessStub) ClearCrewDenial(context.Context, string, string) error { return nil } +func (s readAccessStub) IsCachedCrewMember(context.Context, string, string) (bool, error) { + return false, nil +} +func (s readAccessStub) RecordCrewApproval(context.Context, string, string) error { return nil } + +var _ auth.HoldAuthorizer = readAccessStub{} + +func TestCheckReadAccess_AnonymousVsAuthenticatedDenial(t *testing.T) { + tests := []struct { + name string + anonymous bool + allow bool + wantErr bool + wantCode errcode.ErrorCode + }{ + {name: "public allows anyone", anonymous: true, allow: true, wantErr: false}, + {name: "anonymous denied -> 401", anonymous: true, allow: false, wantErr: true, wantCode: errcode.ErrorCodeUnauthorized}, + {name: "authenticated denied -> 403", anonymous: false, allow: false, wantErr: true, wantCode: errcode.ErrorCodeDenied}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := NewProxyBlobStore(&RegistryContext{ + DID: "did:plc:owner", + HoldDID: "did:web:hold.example.com", + Anonymous: tt.anonymous, + Authorizer: readAccessStub{allow: tt.allow}, + }) + + err := store.checkReadAccess(context.Background()) + if tt.wantErr != (err != nil) { + t.Fatalf("checkReadAccess err = %v, wantErr = %v", err, tt.wantErr) + } + if !tt.wantErr { + return + } + ec, ok := err.(errcode.Error) + if !ok { + t.Fatalf("expected errcode.Error, got %T: %v", err, err) + } + if ec.Code != tt.wantCode { + t.Errorf("error code = %v, want %v", ec.Code, tt.wantCode) + } + }) + } +} + // TestGetServiceToken_CachingLogic tests the token caching mechanism func TestGetServiceToken_CachingLogic(t *testing.T) { userDID := "did:plc:test" @@ -149,52 +214,94 @@ func TestDoAuthenticatedRequest_BearerTokenInjection(t *testing.T) { } } -// TestDoAuthenticatedRequest_ErrorWhenTokenUnavailable tests that authentication failures return proper errors -func TestDoAuthenticatedRequest_ErrorWhenTokenUnavailable(t *testing.T) { - // Create test server (should not be called since auth fails first) - called := false +// TestDoAuthenticatedRequest_AnonymousWhenNoToken verifies that an empty service +// token produces an anonymous request (no Authorization header) rather than an +// error. The destination hold then authorizes per captain.Public. +func TestDoAuthenticatedRequest_AnonymousWhenNoToken(t *testing.T) { + var receivedAuthHeader string + var called bool testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true + receivedAuthHeader = r.Header.Get("Authorization") w.WriteHeader(http.StatusOK) })) defer testServer.Close() - // Create ProxyBlobStore without service token (middleware didn't set it) + // Create ProxyBlobStore without service token (anonymous pull) ctx := &RegistryContext{ - DID: "did:plc:fallback", + DID: "did:plc:anon", HoldDID: "did:web:hold.example.com", PDSEndpoint: "https://pds.example.com", Repository: "test-repo", - ServiceToken: "", // No service token - Refresher: nil, + ServiceToken: "", // No service token => anonymous + Anonymous: true, } store := NewProxyBlobStore(ctx) - // Create request req, err := http.NewRequest(http.MethodGet, testServer.URL+"/test", nil) if err != nil { t.Fatalf("Failed to create request: %v", err) } - // Do authenticated request - should fail when no service token resp, err := store.doAuthenticatedRequest(context.Background(), req) - if err == nil { - t.Fatal("Expected doAuthenticatedRequest to fail when no service token is available") - } - if resp != nil { - resp.Body.Close() + if err != nil { + t.Fatalf("doAuthenticatedRequest failed: %v", err) } + defer resp.Body.Close() - // Verify error indicates authentication/authorization issue - errStr := err.Error() - if !strings.Contains(errStr, "service token") && !strings.Contains(errStr, "UNAUTHORIZED") { - t.Errorf("Expected service token or unauthorized error, got: %v", err) + if !called { + t.Error("Expected the anonymous request to reach the hold") } + if receivedAuthHeader != "" { + t.Errorf("Expected no Authorization header for anonymous request, got %q", receivedAuthHeader) + } +} - if called { - t.Error("Expected request to NOT be made when authentication fails") - } +// TestGetPresignedURL_HoldForbiddenMapsByAnonymity verifies a hold 403 becomes a +// client 401 for anonymous requests (stale captain cache) but stays a generic +// error for authenticated ones. +func TestGetPresignedURL_HoldForbiddenMapsByAnonymity(t *testing.T) { + holdServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer holdServer.Close() + + dgst := digest.FromString("forbidden-blob") + + t.Run("anonymous -> 401", func(t *testing.T) { + store := NewProxyBlobStore(&RegistryContext{ + DID: "did:plc:owner", + HoldDID: "did:web:hold.example.com", + HoldURL: holdServer.URL, + Anonymous: true, + }) + _, err := store.getPresignedURL(context.Background(), "GET", dgst) + ec, ok := err.(errcode.Error) + if !ok { + t.Fatalf("expected errcode.Error, got %T: %v", err, err) + } + if ec.Code != errcode.ErrorCodeUnauthorized { + t.Errorf("expected UNAUTHORIZED, got %v", ec.Code) + } + }) + + t.Run("authenticated -> generic error", func(t *testing.T) { + store := NewProxyBlobStore(&RegistryContext{ + DID: "did:plc:owner", + HoldDID: "did:web:hold.example.com", + HoldURL: holdServer.URL, + ServiceToken: "tok", + Anonymous: false, + }) + _, err := store.getPresignedURL(context.Background(), "GET", dgst) + if err == nil { + t.Fatal("expected an error for authenticated 403") + } + if _, ok := err.(errcode.Error); ok { + t.Errorf("expected a generic (non-errcode) error for authenticated 403, got errcode: %v", err) + } + }) } // TestResolveHoldURL tests URL passthrough (no network needed) diff --git a/pkg/auth/token/claims.go b/pkg/auth/token/claims.go index ee26424..7b21dec 100644 --- a/pkg/auth/token/claims.go +++ b/pkg/auth/token/claims.go @@ -13,6 +13,10 @@ import ( const ( AuthMethodOAuth = "oauth" AuthMethodAppPassword = "app_password" + // AuthMethodAnonymous marks a token issued without credentials for a + // pull-only scope. Such tokens carry an empty Subject (no puller DID); the + // destination hold decides whether anonymous reads are allowed (captain.Public). + AuthMethodAnonymous = "anonymous" ) // Claims represents the JWT claims for registry authentication @@ -87,6 +91,28 @@ func HasPushScope(access []auth.AccessEntry) bool { return false } +// IsPullOnlyScope reports whether the requested access is safe to grant without +// credentials. This is the gate for anonymous token issuance, so it allowlists: +// every requested action must be exactly "pull". Empty access (the /v2/ ping) +// and an entry with no actions are both fine. +// +// It must not be written as a denylist of "push"/"delete". Distribution's +// actionSet.contains returns true for ANY action when the set holds "*" +// (registry/auth/token/util.go), so a scope of `repository:victim/img:*` names +// neither string yet authorizes push and delete. Denylisting would hand an +// unauthenticated caller a token that clears the whole appview authorization +// layer — the authgate is deliberately skipped for anonymous tokens. +func IsPullOnlyScope(access []auth.AccessEntry) bool { + for _, entry := range access { + for _, action := range entry.Actions { + if action != "pull" { + return false + } + } + } + return true +} + // ExtractSubject parses a JWT token string and extracts the Subject claim (the user's DID) // Returns the subject or empty string if not found or token is invalid // This does NOT validate the token - it only parses it to extract the claim diff --git a/pkg/auth/token/claims_test.go b/pkg/auth/token/claims_test.go index 477bae6..27c8428 100644 --- a/pkg/auth/token/claims_test.go +++ b/pkg/auth/token/claims_test.go @@ -75,3 +75,43 @@ func TestNewClaims_EmptyAccess(t *testing.T) { t.Error("Expected Access to be nil when not provided") } } + +func TestIsPullOnlyScope(t *testing.T) { + repo := func(name string, actions ...string) auth.AccessEntry { + return auth.AccessEntry{Type: "repository", Name: name, Actions: actions} + } + + tests := []struct { + name string + access []auth.AccessEntry + want bool + }{ + {"nil access (ping)", nil, true}, + {"empty access", []auth.AccessEntry{}, true}, + {"pull only", []auth.AccessEntry{repo("alice/app", "pull")}, true}, + {"wildcard pull", []auth.AccessEntry{repo("*", "pull")}, true}, + {"push", []auth.AccessEntry{repo("alice/app", "push")}, false}, + {"delete", []auth.AccessEntry{repo("alice/app", "delete")}, false}, + {"pull and push", []auth.AccessEntry{repo("alice/app", "pull", "push")}, false}, + {"mixed entries with one push", []auth.AccessEntry{repo("alice/app", "pull"), repo("alice/other", "push")}, false}, + + // Wildcard ACTION means "any action" to distribution's actionSet.contains, + // so it must never be treated as pull-only: an anonymous token carrying it + // would authorize push and delete on someone else's repository. (Wildcard + // NAME with a pull action, above, is fine — that is scope, not action.) + {"wildcard action", []auth.AccessEntry{repo("alice/app", "*")}, false}, + {"wildcard action on wildcard name", []auth.AccessEntry{repo("*", "*")}, false}, + {"pull plus wildcard action", []auth.AccessEntry{repo("alice/app", "pull", "*")}, false}, + {"catalog wildcard", []auth.AccessEntry{{Type: "registry", Name: "catalog", Actions: []string{"*"}}}, false}, + {"unknown action", []auth.AccessEntry{repo("alice/app", "frobnicate")}, false}, + {"entry with no actions", []auth.AccessEntry{repo("alice/app")}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsPullOnlyScope(tt.access); got != tt.want { + t.Errorf("IsPullOnlyScope(%+v) = %v, want %v", tt.access, got, tt.want) + } + }) + } +} diff --git a/pkg/auth/token/handler.go b/pkg/auth/token/handler.go index e42d4bd..f742db7 100644 --- a/pkg/auth/token/handler.go +++ b/pkg/auth/token/handler.go @@ -78,6 +78,8 @@ type Handler struct { oauthSessionValidator OAuthSessionValidator authorizer Authorizer serviceAuthFetcher ServiceAuthFetcher + allowAnonymousPull bool // issue credential-less tokens for pull-only scopes + // services is the set of registry domains this AppView fronts, keyed by // normalized hostname. Nil means single-domain: the lookups in // resolveService miss and every token gets the issuer's own service. @@ -87,9 +89,10 @@ type Handler struct { // NewHandler creates a new token handler func NewHandler(issuer *Issuer, deviceStore *db.DeviceStore) *Handler { return &Handler{ - issuer: issuer, - validator: auth.NewSessionValidator(), - deviceStore: deviceStore, + issuer: issuer, + validator: auth.NewSessionValidator(), + deviceStore: deviceStore, + allowAnonymousPull: true, } } @@ -120,6 +123,14 @@ func (h *Handler) SetServiceAuthFetcher(fetcher ServiceAuthFetcher) { h.serviceAuthFetcher = fetcher } +// SetAllowAnonymousPull toggles credential-less token issuance for pull-only +// scopes. When false, requests without credentials are always challenged, so +// the handler behaves exactly as it did before anonymous pull existed. Per-hold +// privacy (captain.Public) still applies regardless of this setting. +func (h *Handler) SetAllowAnonymousPull(allow bool) { + h.allowAnonymousPull = allow +} + // SetServices declares the registry domains this AppView fronts, e.g. // ["buoy.cr", "seamark.cr", "atcr.io"]. Each issued JWT is stamped with // whichever of these the client is authenticating against, so the audience @@ -262,13 +273,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: - var ok bool - username, password, ok = r.BasicAuth() - if !ok { - slog.Debug("No Basic auth credentials provided") - sendAuthError(w, r, "authentication required") - return - } + // Missing credentials are not fatal on this form: an anonymous pull-only + // request is served without them, and whether that applies depends on the + // scope, which is parsed below. An empty username is the signal. + username, password, _ = r.BasicAuth() scopeParam = r.URL.Query().Get("scope") requestedService = r.URL.Query().Get("service") @@ -336,6 +344,22 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + // No credentials. Only reachable on the GET form — the POST form rejects an + // empty username above, and a client that lands there without one is sent to + // the GET form by that 401. Issue an anonymous, pull-only token if enabled + // and the requested scope carries no write actions; the destination hold then + // decides whether anonymous reads are allowed (captain.Public). Anything + // requesting push/delete still gets the standard auth challenge. + if username == "" { + if h.allowAnonymousPull && IsPullOnlyScope(access) { + h.issueAnonymousToken(w, r, access, service) + return + } + slog.Debug("No Basic auth credentials provided") + sendAuthError(w, r, "authentication required") + return + } + var did string var handle string var accessToken string @@ -513,7 +537,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if errors.Is(res.err, auth.ErrAppPasswordInsufficientScope) { slog.Info("service-auth pre-mint denied: app-password lacks scope", "did", did, "error", res.err) _ = errcode.ServeJSON(w, errcode.ErrorCodeDenied.WithMessage( - "your app password lacks the permissions ATCR needs. A read-only app password cannot mint the service token used to authenticate with your storage hold. Use a standard (full-access) app password.")) + "your app password lacks the permissions ATCR needs. A read-only app password cannot mint the service token used to authenticate with your storage hold. Use a standard (full-access) app password, or pull public images without logging in.")) return } slog.Warn("service-auth pre-mint failed", "did", did, "error", res.err) @@ -564,6 +588,44 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, resp) } +// issueAnonymousToken mints a credential-less registry JWT for a pull-only +// scope. The token carries an empty Subject (no puller DID) and the anonymous +// auth method. It deliberately skips the authorizer gate and service-auth +// pre-mint: there is no identity to reconcile and no AppView↔hold service-auth +// to bind, since anonymous reads never carry a service token to the hold. +// +// The service is still stamped: an anonymous pull against a secondary registry +// domain has to satisfy that domain's access controller, which demands its own +// audience, so the issuer's default would be rejected there. +func (h *Handler) issueAnonymousToken(w http.ResponseWriter, r *http.Request, access []auth.AccessEntry, service string) { + // Defensive: ValidateAccess only restricts push/delete to the owner, so a + // pull-only scope (the only thing routed here) always passes. Empty DID is + // fine — anonymous tokens have no owner. + if err := auth.ValidateAccess("", "", access); err != nil { + slog.Debug("Anonymous access validation failed", "error", err) + http.Error(w, fmt.Sprintf("access denied: %v", err), http.StatusForbidden) + return + } + + tokenString, err := h.issuer.IssueWithExpiration("", access, AuthMethodAnonymous, h.issuer.expiration, service) + if err != nil { + slog.Error("Failed to issue anonymous token", "error", err) + http.Error(w, fmt.Sprintf("failed to issue token: %v", err), http.StatusInternalServerError) + return + } + + slog.Debug("Issued anonymous pull token", "tokenLength", len(tokenString)) + + now := time.Now() + resp := TokenResponse{ + Token: tokenString, + AccessToken: tokenString, + ExpiresIn: int(h.issuer.expiration.Seconds()), + IssuedAt: now.Format(time.RFC3339), + } + render.JSON(w, r, resp) +} + // parseBasicAuthDID fixes DID usernames that are mangled by HTTP Basic Auth. // // This handles two cases: diff --git a/pkg/auth/token/handler_test.go b/pkg/auth/token/handler_test.go index f42f389..06f0dbb 100644 --- a/pkg/auth/token/handler_test.go +++ b/pkg/auth/token/handler_test.go @@ -141,6 +141,8 @@ func TestHandler_ServeHTTP_NoAuth(t *testing.T) { } handler := NewHandler(issuer, nil) + // With anonymous pull disabled, a credential-less request is challenged. + handler.SetAllowAnonymousPull(false) req := httptest.NewRequest(http.MethodGet, "/auth/token?service=registry", nil) w := httptest.NewRecorder() @@ -157,6 +159,128 @@ func TestHandler_ServeHTTP_NoAuth(t *testing.T) { } } +func TestHandler_ServeHTTP_AnonymousPull(t *testing.T) { + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + handler := NewHandler(issuer, nil) // allowAnonymousPull defaults true + + // No credentials, pull-only scope for someone else's repo. + req := httptest.NewRequest(http.MethodGet, "/auth/token?service=registry&scope=repository:bob.bsky.social/myapp:pull", nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("Expected status %d for anonymous pull, got %d. Body: %s", http.StatusOK, w.Code, w.Body.String()) + } + + var resp TokenResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + if resp.Token == "" { + t.Fatal("Expected non-empty anonymous token") + } + + // The anonymous token carries no subject and the anonymous auth method. + if sub := ExtractSubject(resp.Token); sub != "" { + t.Errorf("Expected empty subject for anonymous token, got %q", sub) + } + if am := ExtractAuthMethod(resp.Token); am != AuthMethodAnonymous { + t.Errorf("Expected auth method %q, got %q", AuthMethodAnonymous, am) + } + access := ExtractAccess(resp.Token) + if len(access) != 1 || access[0].Name != "bob.bsky.social/myapp" { + t.Errorf("Expected pull access for bob.bsky.social/myapp, got %+v", access) + } +} + +func TestHandler_ServeHTTP_AnonymousPing(t *testing.T) { + // The /v2/ ping requests a token with no scope (empty access). Anonymous + // issuance must grant it so the ping succeeds without credentials. + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + handler := NewHandler(issuer, nil) + + req := httptest.NewRequest(http.MethodGet, "/auth/token?service=registry", nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("Expected status %d for anonymous ping, got %d. Body: %s", http.StatusOK, w.Code, w.Body.String()) + } + + var resp TokenResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + if resp.Token == "" { + t.Error("Expected non-empty token for anonymous ping") + } + if len(ExtractAccess(resp.Token)) != 0 { + t.Errorf("Expected empty access for ping token, got %+v", ExtractAccess(resp.Token)) + } +} + +func TestHandler_ServeHTTP_AnonymousPushChallenged(t *testing.T) { + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + handler := NewHandler(issuer, nil) + + for _, action := range []string{"push", "delete", "pull,push"} { + t.Run(action, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/auth/token?service=registry&scope=repository:bob.bsky.social/myapp:"+action, nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d for anonymous %s, got %d. Body: %s", http.StatusUnauthorized, action, w.Code, w.Body.String()) + } + if w.Header().Get("WWW-Authenticate") == "" { + t.Error("Expected WWW-Authenticate header on anonymous write challenge") + } + }) + } +} + +func TestHandler_ServeHTTP_AnonymousDisabledChallengesPull(t *testing.T) { + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + handler := NewHandler(issuer, nil) + handler.SetAllowAnonymousPull(false) + + req := httptest.NewRequest(http.MethodGet, "/auth/token?service=registry&scope=repository:bob.bsky.social/myapp:pull", nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d when anonymous pull disabled, got %d. Body: %s", http.StatusUnauthorized, w.Code, w.Body.String()) + } +} + func TestHandler_ServeHTTP_WrongMethod(t *testing.T) { keyPath := getSharedTestKey(t) diff --git a/test/integration/auth_matrix_test.go b/test/integration/auth_matrix_test.go index 43c75c6..ff600ec 100644 --- a/test/integration/auth_matrix_test.go +++ b/test/integration/auth_matrix_test.go @@ -127,23 +127,42 @@ func TestAuthMatrix(t *testing.T) { repoFn: func(_ string) string { return seedRef.String() }, }, { - name: "anonymous_push_denied", - creds: h.AnonCreds(), - op: "push", - repoFn: func(c string) string { return fmt.Sprintf("%s/anonymous/own-%s:tag", h.AppViewHostPort(), c) }, - // /auth/token requires Basic auth — no creds → 401. crane - // surfaces the response body ("authentication required"); - // oras-go drops the body and surfaces "Unauthorized". + name: "anonymous_push_denied", + creds: h.AnonCreds(), + op: "push", + // Target a real identity's namespace. A push scope carries a write + // action, so it is never pull-only and /auth/token challenges it even + // with anonymous pull enabled. crane surfaces the response body + // ("authentication required"); oras-go drops the body and surfaces + // "Unauthorized". + repoFn: func(c string) string { + return fmt.Sprintf("%s/%s/anon-push-%s:tag", h.AppViewHostPort(), captain.Handle(), c) + }, wantErr: true, errContains: []string{"authentication required", "Unauthorized", "unauthorized"}, }, { - name: "anonymous_pull_denied", - creds: h.AnonCreds(), - op: "pull", - repoFn: func(_ string) string { return seedRef.String() }, + name: "anonymous_pull", + // The harness hold is public (captain.Public), so a credential-less + // pull is allowed: /auth/token issues a pull-only token with an empty + // subject and the hold serves the blobs. Privacy is still the hold's + // call — a private hold rejects the same request. + creds: h.AnonCreds(), + op: "pull", + repoFn: func(_ string) string { return seedRef.String() }, + }, + { + name: "anonymous_pull_unknown_identity_denied", + // An unresolvable identity must be a clean NAME_UNKNOWN, not a 500. + // Anonymous pull makes this path reachable without credentials, and a + // 5xx would send clients that retry server errors into a retry loop. + creds: h.AnonCreds(), + op: "pull", + repoFn: func(c string) string { + return fmt.Sprintf("%s/not-a-real-handle/%s:tag", h.AppViewHostPort(), c) + }, wantErr: true, - errContains: []string{"authentication required", "Unauthorized", "unauthorized"}, + errContains: []string{"not known to registry", "NAME_UNKNOWN", "not found", "404"}, }, }