diff --git a/.env.appview.example b/.env.appview.example index 0a11c5b..b86cab7 100644 --- a/.env.appview.example +++ b/.env.appview.example @@ -49,6 +49,16 @@ ATCR_DEFAULT_HOLD_DID=did:web:127.0.0.1:8080 # JWT token expiration in seconds (default: 300 = 5 minutes) # ATCR_TOKEN_EXPIRATION=300 +# Path to OAuth client P-256 signing key (auto-generated on first run) +# Used for confidential OAuth client authentication (production only) +# Localhost deployments always use public OAuth clients (no key needed) +# Default: /var/lib/atcr/oauth/client.key +# ATCR_OAUTH_KEY_PATH=/var/lib/atcr/oauth/client.key + +# OAuth client display name (shown in authorization screens) +# Default: AT Container Registry +# ATCR_CLIENT_NAME=AT Container Registry + # ============================================================================== # UI Configuration # ============================================================================== diff --git a/CLAUDE.md b/CLAUDE.md index c87c11b..f8a4d2b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -221,31 +221,48 @@ ATCR implements the full ATProto OAuth specification with mandatory security fea **Key Components** (`pkg/auth/oauth/`): 1. **Client** (`client.go`) - Core OAuth client with encapsulated configuration - - Constructor: `NewClient(baseURL)` - accepts base URL, derives client ID/redirect URI - - `NewClientWithKey(baseURL, dpopKey)` - for token refresh with stored DPoP key - - `ClientID()` - computes localhost vs production client ID dynamically + - Uses indigo's `NewLocalhostConfig()` for localhost (public client) + - Uses `NewPublicConfig()` for production base (upgraded to confidential if key provided) - `RedirectURI()` - returns `baseURL + "/auth/oauth/callback"` - `GetDefaultScopes()` - returns ATCR registry scopes + - `GetConfigRef()` - returns mutable config for `SetClientSecret()` calls - All OAuth flows (authorization, token exchange, refresh) in one place -2. **Token Storage** (`store.go`) - Persists OAuth sessions for AppView - - File-based storage in `/var/lib/atcr/refresh-tokens.json` (AppView) +2. **Keys** (`keys.go`) - P-256 key management for confidential clients + - `GenerateOrLoadClientKey()` - generates or loads P-256 key from disk + - Follows hold service pattern: auto-generation, 0600 permissions, /var/lib/atcr/oauth/ + - `GenerateKeyID()` - derives key ID from public key hash + - `PrivateKeyToMultibase()` - converts key for `SetClientSecret()` API + - **Key type:** P-256 (ES256) for OAuth standard compatibility (not K-256 like PDS keys) + +3. **Token Storage** (`store.go`) - Persists OAuth sessions for AppView + - SQLite-backed storage in UI database (not file-based) - Client uses `~/.atcr/oauth-token.json` (credential helper) -3. **Refresher** (`refresher.go`) - Token refresh manager for AppView +4. **Refresher** (`refresher.go`) - Token refresh manager for AppView - Caches OAuth sessions with automatic token refresh (handled by indigo library) - Per-DID locking prevents concurrent refresh races - Uses Client methods for consistency -4. **Server** (`server.go`) - OAuth authorization endpoints for AppView +5. **Server** (`server.go`) - OAuth authorization endpoints for AppView - `GET /auth/oauth/authorize` - starts OAuth flow - `GET /auth/oauth/callback` - handles OAuth callback - Uses Client methods for authorization and token exchange -5. **Interactive Flow** (`interactive.go`) - Reusable OAuth flow for CLI tools +6. **Interactive Flow** (`interactive.go`) - Reusable OAuth flow for CLI tools - Used by credential helper and hold service registration - Two-phase callback setup ensures PAR metadata availability +**Client Configuration:** +- **Localhost:** Always public client (no client authentication) + - Client ID: `http://localhost?redirect_uri=...&scope=...` (query-based) + - No P-256 key generation +- **Production:** Confidential client with P-256 private key (if key exists) + - Client ID: `{baseURL}/client-metadata.json` (metadata endpoint) + - Key path: `/var/lib/atcr/oauth/client.key` (auto-generated on first run) + - Key algorithm: ES256 (P-256, not K-256) + - Upgraded via `config.SetClientSecret(key, keyID)` + **Authentication Flow:** ``` 1. User configures Docker to use the credential helper (adds to config.json) @@ -280,6 +297,11 @@ Later (subsequent docker push): - No trust in client-provided identity information - DPoP binds tokens to specific client key - 15-minute token expiry for registry JWTs +- **Confidential clients** (production): Client authentication via P-256 private key JWT assertion + - Prevents client impersonation attacks + - Key stored in `/var/lib/atcr/oauth/client.key` with 0600 permissions + - Automatically generated on first run +- **Public clients** (localhost): No client authentication (development only) ### Key Components diff --git a/cmd/appview/serve.go b/cmd/appview/serve.go index f894a83..e983109 100644 --- a/cmd/appview/serve.go +++ b/cmd/appview/serve.go @@ -119,8 +119,8 @@ func serveRegistry(cmd *cobra.Command, args []string) error { slog.Info("TEST_MODE enabled - will use HTTP for local DID resolution and transition:generic scope") } - // Create OAuth app (indigo client) - oauthApp, err := oauth.NewApp(baseURL, oauthStore, defaultHoldDID, testMode) + // Create OAuth app (automatically configures confidential client for production) + oauthApp, err := oauth.NewApp(baseURL, oauthStore, defaultHoldDID, cfg.Server.OAuthKeyPath, cfg.Server.ClientName) if err != nil { return fmt.Errorf("failed to create OAuth app: %w", err) } @@ -132,7 +132,7 @@ func serveRegistry(cmd *cobra.Command, args []string) error { // Invalidate sessions with mismatched scopes on startup // This ensures all users have the latest required scopes after deployment - desiredScopes := oauth.GetDefaultScopes(defaultHoldDID, testMode) + desiredScopes := oauth.GetDefaultScopes(defaultHoldDID) invalidatedCount, err := oauthStore.InvalidateSessionsWithMismatchedScopes(context.Background(), desiredScopes) if err != nil { slog.Warn("Failed to invalidate sessions with mismatched scopes", "error", err) @@ -385,9 +385,27 @@ func serveRegistry(cmd *cobra.Command, args []string) error { config := oauthApp.GetConfig() metadata := config.ClientMetadata() + // Convert indigo's metadata to map so we can add custom fields + metadataBytes, err := json.Marshal(metadata) + if err != nil { + http.Error(w, "Failed to marshal metadata", http.StatusInternalServerError) + return + } + + var metadataMap map[string]interface{} + if err := json.Unmarshal(metadataBytes, &metadataMap); err != nil { + http.Error(w, "Failed to unmarshal metadata", http.StatusInternalServerError) + return + } + + // Add custom fields + metadataMap["client_name"] = cfg.Server.ClientName + metadataMap["client_uri"] = cfg.Server.BaseURL + metadataMap["logo_uri"] = cfg.Server.BaseURL + "/web-app-manifest-192x192.png" + w.Header().Set("Content-Type", "application/json") w.Header().Set("Access-Control-Allow-Origin", "*") - if err := json.NewEncoder(w).Encode(metadata); err != nil { + if err := json.NewEncoder(w).Encode(metadataMap); err != nil { http.Error(w, "Failed to encode metadata", http.StatusInternalServerError) } }) diff --git a/deploy/.env.prod.template b/deploy/.env.prod.template index 81a68a2..9873160 100644 --- a/deploy/.env.prod.template +++ b/deploy/.env.prod.template @@ -151,6 +151,10 @@ S3_ENDPOINT=https://6vmss.upcloudobjects.com # Default: 300 (5 minutes) ATCR_TOKEN_EXPIRATION=300 +# OAuth client display name (shown in authorization screens) +# Default: AT Container Registry +# ATCR_CLIENT_NAME=AT Container Registry + # Enable web UI # Default: true ATCR_UI_ENABLED=true diff --git a/deploy/init-upcloud.sh b/deploy/init-upcloud.sh index f1c8b38..b2755ab 100644 --- a/deploy/init-upcloud.sh +++ b/deploy/init-upcloud.sh @@ -130,11 +130,13 @@ if [ -f "deploy/.env.prod.template" ] && [ ! -f "$ATCR_DIR/.env" ]; then log_warn "IMPORTANT: Edit $ATCR_DIR/.env with your configuration!" fi -# Create systemd service -log_info "Creating systemd service..." -cat > /etc/systemd/system/atcr.service <<'EOF' +# Create systemd services (caddy, appview, hold) +log_info "Creating systemd services..." + +# Caddy service (reverse proxy for both appview and hold) +cat > /etc/systemd/system/atcr-caddy.service <<'EOF' [Unit] -Description=ATCR Container Registry +Description=ATCR Caddy Reverse Proxy Requires=docker.service After=docker.service network-online.target Wants=network-online.target @@ -145,14 +147,76 @@ RemainAfterExit=yes WorkingDirectory=/opt/atcr EnvironmentFile=/opt/atcr/.env -# Start containers -ExecStart=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml up -d +# Start caddy container +ExecStart=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml up -d caddy -# Stop containers -ExecStop=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml down +# Stop caddy container +ExecStop=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml stop caddy -# Restart containers -ExecReload=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml restart +# Restart caddy container +ExecReload=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml restart caddy + +# Always restart on failure +Restart=on-failure +RestartSec=10 + +[Install] +WantedBy=multi-user.target +EOF + +# AppView service (registry + web UI) +cat > /etc/systemd/system/atcr-appview.service <<'EOF' +[Unit] +Description=ATCR AppView (Registry + Web UI) +Requires=docker.service atcr-caddy.service +After=docker.service network-online.target atcr-caddy.service +Wants=network-online.target + +[Service] +Type=oneshot +RemainAfterExit=yes +WorkingDirectory=/opt/atcr +EnvironmentFile=/opt/atcr/.env + +# Start appview container +ExecStart=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml up -d atcr-appview + +# Stop appview container +ExecStop=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml stop atcr-appview + +# Restart appview container +ExecReload=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml restart atcr-appview + +# Always restart on failure +Restart=on-failure +RestartSec=10 + +[Install] +WantedBy=multi-user.target +EOF + +# Hold service (storage backend) +cat > /etc/systemd/system/atcr-hold.service <<'EOF' +[Unit] +Description=ATCR Hold (Storage Service) +Requires=docker.service atcr-caddy.service +After=docker.service network-online.target atcr-caddy.service +Wants=network-online.target + +[Service] +Type=oneshot +RemainAfterExit=yes +WorkingDirectory=/opt/atcr +EnvironmentFile=/opt/atcr/.env + +# Start hold container +ExecStart=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml up -d atcr-hold + +# Stop hold container +ExecStop=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml stop atcr-hold + +# Restart hold container +ExecReload=/usr/bin/docker compose -f /opt/atcr/deploy/docker-compose.prod.yml restart atcr-hold # Always restart on failure Restart=on-failure @@ -166,10 +230,12 @@ EOF log_info "Reloading systemd daemon..." systemctl daemon-reload -# Enable service (but don't start yet - user needs to configure .env) -systemctl enable atcr.service +# Enable all services (but don't start yet - user needs to configure .env) +systemctl enable atcr-caddy.service +systemctl enable atcr-appview.service +systemctl enable atcr-hold.service -log_info "Systemd service created and enabled" +log_info "Systemd services created and enabled" # Create helper scripts log_info "Creating helper scripts..." @@ -193,14 +259,6 @@ docker compose -f deploy/docker-compose.prod.yml logs -f "$@" EOF chmod +x "$ATCR_DIR/logs.sh" -# Script to get hold OAuth URL -cat > "$ATCR_DIR/get-hold-oauth.sh" <<'EOF' -#!/bin/bash -echo "Checking atcr-hold logs for OAuth registration URL..." -docker logs atcr-hold 2>&1 | grep -i "oauth\|authorization\|visit\|http" | tail -20 -EOF -chmod +x "$ATCR_DIR/get-hold-oauth.sh" - log_info "Helper scripts created in $ATCR_DIR" # Print completion message @@ -241,29 +299,35 @@ echo " CNAME blobs.atcr.io → atcr.us-chi1.upcloudobjects.com (gray cloud cat <<'EOF' -4. Start ATCR: - systemctl start atcr +4. Start ATCR services: + systemctl start atcr-caddy atcr-appview atcr-hold -5. Complete Hold OAuth registration: - /opt/atcr/get-hold-oauth.sh - - Visit the OAuth URL in your browser to authorize the hold service. - -6. Check status: - systemctl status atcr +5. Check status: + systemctl status atcr-caddy + systemctl status atcr-appview + systemctl status atcr-hold docker ps /opt/atcr/logs.sh Helper Scripts: /opt/atcr/rebuild.sh - Rebuild and restart containers /opt/atcr/logs.sh [service] - View logs (e.g., logs.sh atcr-hold) - /opt/atcr/get-hold-oauth.sh - Get hold OAuth URL Service Management: - systemctl start atcr - Start ATCR - systemctl stop atcr - Stop ATCR - systemctl restart atcr - Restart ATCR - systemctl status atcr - Check status + systemctl start atcr-caddy - Start Caddy reverse proxy + systemctl start atcr-appview - Start AppView (registry + UI) + systemctl start atcr-hold - Start Hold (storage service) + + systemctl stop atcr-appview - Stop AppView only + systemctl stop atcr-hold - Stop Hold only + systemctl stop atcr-caddy - Stop all (stops reverse proxy) + + systemctl restart atcr-appview - Restart AppView + systemctl restart atcr-hold - Restart Hold + + systemctl status atcr-caddy - Check Caddy status + systemctl status atcr-appview - Check AppView status + systemctl status atcr-hold - Check Hold status Documentation: https://tangled.org/@evan.jarrett.net/at-container-registry diff --git a/docs/OAUTH.md b/docs/OAUTH.md new file mode 100644 index 0000000..7113a6a --- /dev/null +++ b/docs/OAUTH.md @@ -0,0 +1,399 @@ +# OAuth Implementation in ATCR + +This document describes ATCR's OAuth implementation, which uses the ATProto OAuth specification with DPoP (Demonstrating Proof of Possession) for secure authentication. + +## Overview + +ATCR implements a full OAuth 2.0 + DPoP flow following the ATProto specification. The implementation uses the [indigo OAuth library](https://github.com/bluesky-social/indigo) and extends it with ATCR-specific configuration for registry operations. + +### Key Features + +- **DPoP (RFC 9449)**: Cryptographic proof-of-possession binds tokens to specific client keys +- **PAR (RFC 9126)**: Pushed Authorization Requests for secure server-to-server parameter exchange +- **PKCE (RFC 7636)**: Proof Key for Code Exchange prevents authorization code interception +- **Confidential Clients**: Production deployments use P-256 private keys for client authentication +- **Public Clients**: Development (localhost) uses simpler public client configuration + +## Client Types + +ATCR supports two OAuth client types depending on the deployment environment: + +### Public Clients (Development) + +**When:** `baseURL` contains `localhost` or `127.0.0.1` + +**Configuration:** +- Client ID: `http://localhost?redirect_uri=...&scope=...` (query-based) +- No client authentication +- Uses indigo's `NewLocalhostConfig()` helper +- DPoP still required for token requests + +**Example:** +```go +// Automatically uses public client for localhost +config := oauth.NewClientConfigWithScopes("http://127.0.0.1:5000", scopes) +``` + +### Confidential Clients (Production) + +**When:** `baseURL` is a public domain (not localhost) + +**Configuration:** +- Client ID: `{baseURL}/client-metadata.json` (metadata endpoint) +- Client authentication: P-256 (ES256) private key JWT assertion +- Private key stored at `/var/lib/atcr/oauth/client.key` +- Auto-generated on first run with 0600 permissions +- Upgraded via `config.SetClientSecret(privateKey, keyID)` + +**Example:** +```go +// 1. Create base config (public) +config := oauth.NewClientConfigWithScopes("https://atcr.io", scopes) + +// 2. Load or generate P-256 key +privateKey, err := oauth.GenerateOrLoadClientKey("/var/lib/atcr/oauth/client.key") + +// 3. Generate key ID +keyID, err := oauth.GenerateKeyID(privateKey) + +// 4. Upgrade to confidential +err = config.SetClientSecret(privateKey, keyID) +``` + +## Key Management + +### P-256 Key Generation + +ATCR uses **P-256 (NIST P-256, ES256)** keys for OAuth client authentication. This differs from the K-256 keys used for ATProto PDS signing. + +**Why P-256?** +- Standard OAuth/OIDC key algorithm +- Widely supported by authorization servers +- Compatible with indigo's `SetClientSecret()` API + +**Key Storage:** +- Default path: `/var/lib/atcr/oauth/client.key` +- Configurable via: `ATCR_OAUTH_KEY_PATH` environment variable +- File permissions: `0600` (owner read/write only) +- Directory permissions: `0700` (owner access only) +- Format: Raw binary bytes (not PEM) + +**Key Lifecycle:** +1. On first production startup, AppView checks for key at configured path +2. If missing, generates new P-256 key using `atcrypto.GeneratePrivateKeyP256()` +3. Saves raw key bytes to disk with restrictive permissions +4. Logs generation event: `"Generated new P-256 OAuth client key"` +5. On subsequent startups, loads existing key +6. Logs load event: `"Loaded existing P-256 OAuth client key"` + +**Key Rotation:** +To rotate the OAuth client key: +1. Stop the AppView service +2. Delete or rename the existing key file +3. Restart AppView (new key will be generated automatically) +4. Note: Active OAuth sessions may need re-authentication + +### Key ID Generation + +The key ID is derived from the public key for stable identification: + +```go +func GenerateKeyID(privateKey *atcrypto.PrivateKeyP256) (string, error) { + pubKey, _ := privateKey.PublicKey() + pubKeyBytes := pubKey.Bytes() + hash := sha256.Sum256(pubKeyBytes) + return hex.EncodeToString(hash[:])[:8], nil +} +``` + +This generates an 8-character hex ID from the SHA-256 hash of the public key. + +## Authentication Flow + +### AppView OAuth Flow + +```mermaid +sequenceDiagram + participant User + participant Browser + participant AppView + participant PDS + + User->>Browser: docker push atcr.io/alice/myapp + Browser->>AppView: Credential helper redirects + AppView->>PDS: Resolve handle → DID + AppView->>PDS: Discover OAuth metadata + AppView->>PDS: PAR request (with DPoP) + PDS-->>AppView: request_uri + AppView->>Browser: Redirect to authorization page + Browser->>PDS: User authorizes + PDS->>AppView: Authorization code + AppView->>PDS: Token exchange (with DPoP) + PDS-->>AppView: OAuth tokens + DPoP binding + AppView->>User: Issue registry JWT +``` + +### Key Steps + +1. **Identity Resolution** + - AppView resolves handle to DID via `.well-known/atproto-did` + - Resolves DID to PDS endpoint via DID document + +2. **OAuth Discovery** + - Fetches `/.well-known/oauth-authorization-server` from PDS + - Extracts `authorization_endpoint`, `token_endpoint`, etc. + +3. **Pushed Authorization Request (PAR)** + - AppView sends authorization parameters to PDS token endpoint + - Includes DPoP header with proof JWT + - Receives `request_uri` for authorization + +4. **User Authorization** + - User is redirected to PDS authorization page + - User approves application access + - PDS redirects back with authorization code + +5. **Token Exchange** + - AppView exchanges code for tokens at PDS token endpoint + - Includes DPoP header with proof JWT + - Receives access token, refresh token (both DPoP-bound) + +6. **Token Storage** + - AppView stores OAuth session in SQLite database + - Indigo library manages token refresh automatically + - DPoP key stored with session for future requests + +7. **Registry JWT Issuance** + - AppView validates OAuth session + - Issues short-lived registry JWT (15 minutes) + - JWT contains validated DID from PDS session + +## DPoP Implementation + +### What is DPoP? + +DPoP (Demonstrating Proof of Possession) binds OAuth tokens to a specific client key, preventing token theft and replay attacks. + +**How it works:** +1. Client generates ephemeral key pair (or uses persistent key) +2. Client includes DPoP proof JWT in Authorization header +3. Proof JWT contains hash of HTTP request details +4. Authorization server validates proof and issues DPoP-bound token +5. Token can only be used with the same client key + +### DPoP Headers + +Every request to the PDS token endpoint includes a DPoP header: + +```http +POST /oauth/token HTTP/1.1 +Host: pds.example.com +Content-Type: application/x-www-form-urlencoded +DPoP: eyJhbGciOiJFUzI1NiIsInR5cCI6ImRwb3Arand0IiwiandrIjp7Imt0eSI6Ik... + +grant_type=authorization_code&code=...&redirect_uri=... +``` + +The DPoP header is a signed JWT containing: +- `htm`: HTTP method (e.g., "POST") +- `htu`: HTTP URI (e.g., "https://pds.example.com/oauth/token") +- `jti`: Unique request identifier +- `iat`: Timestamp +- `jwk`: Public key (JWK format) + +### Indigo DPoP Management + +ATCR uses indigo's built-in DPoP management: + +```go +// Indigo automatically handles DPoP +clientApp := oauth.NewClientApp(&config, store) + +// All token requests include DPoP automatically +tokens, err := clientApp.ProcessCallback(ctx, params) + +// Refresh automatically includes DPoP +session, err := clientApp.ResumeSession(ctx, did, sessionID) +``` + +Indigo manages: +- DPoP key generation and storage +- DPoP proof JWT creation +- DPoP header inclusion in token requests +- Token binding to DPoP keys + +## Client Configuration + +### Environment Variables + +**ATCR_OAUTH_KEY_PATH** +- Path to OAuth client P-256 signing key +- Default: `/var/lib/atcr/oauth/client.key` +- Auto-generated on first run (production only) +- Format: Raw binary P-256 private key + +**ATCR_BASE_URL** +- Public URL of AppView service +- Required for OAuth redirect URIs +- Example: `https://atcr.io` +- Determines client type (public vs confidential) + +**ATCR_UI_DATABASE_PATH** +- Path to SQLite database (includes OAuth session storage) +- Default: `/var/lib/atcr/ui.db` + +### Client Metadata Endpoint + +Production deployments serve OAuth client metadata at `{baseURL}/client-metadata.json`: + +```json +{ + "client_id": "https://atcr.io/client-metadata.json", + "client_name": "ATCR Registry", + "client_uri": "https://atcr.io", + "redirect_uris": ["https://atcr.io/auth/oauth/callback"], + "scope": "atproto blob:... repo:...", + "grant_types": ["authorization_code", "refresh_token"], + "response_types": ["code"], + "token_endpoint_auth_method": "private_key_jwt", + "token_endpoint_auth_signing_alg": "ES256", + "jwks": { + "keys": [ + { + "kty": "EC", + "crv": "P-256", + "x": "...", + "y": "...", + "kid": "abc12345" + } + ] + } +} +``` + +For localhost, the client ID is query-based and no metadata endpoint is used. + +## Scope Management + +ATCR requests the following OAuth scopes: + +**Base scopes:** +- `atproto`: Basic ATProto access + +**Blob scopes (for layer/manifest media types):** +- `blob:application/vnd.oci.image.manifest.v1+json` +- `blob:application/vnd.docker.distribution.manifest.v2+json` +- `blob:application/vnd.oci.image.index.v1+json` +- `blob:application/vnd.docker.distribution.manifest.list.v2+json` +- `blob:application/vnd.cncf.oras.artifact.manifest.v1+json` + +**Repo scopes (for ATProto collections):** +- `repo:io.atcr.manifest`: Manifest records +- `repo:io.atcr.tag`: Tag records +- `repo:io.atcr.star`: Star records +- `repo:io.atcr.sailor.profile`: User profile records + +**RPC scope:** +- `rpc:com.atproto.repo.getRecord?aud=*`: Read access to any user's records + +Scopes are automatically invalidated on startup if they change, forcing users to re-authenticate. + +## Security Considerations + +### Token Security + +**OAuth Tokens (managed by AppView):** +- Stored in SQLite database +- DPoP-bound (cannot be used without client key) +- Automatically refreshed by indigo library +- Used for PDS API requests (manifests, service tokens) + +**Registry JWTs (issued to Docker clients):** +- Short-lived (15 minutes) +- Signed by AppView's JWT signing key +- Contain validated DID from OAuth session +- Used for OCI Distribution API requests + +### Attack Prevention + +**Token Theft:** +- DPoP prevents stolen tokens from being used +- Tokens are bound to specific client key +- Attacker would need both token AND private key + +**Client Impersonation:** +- Confidential clients use private key JWT assertion +- Prevents attackers from impersonating AppView +- Public keys published in client metadata JWKS + +**Man-in-the-Middle:** +- All OAuth flows use HTTPS in production +- DPoP includes HTTP method and URI in proof +- Prevents replay attacks on different endpoints + +**Authorization Code Interception:** +- PKCE prevents code interception attacks +- Code verifier required to exchange code for token +- Protects against malicious redirect URI attacks + +## Troubleshooting + +### Common Issues + +**"Failed to initialize OAuth client key"** +- Check that `/var/lib/atcr/oauth/` directory exists and is writable +- Verify directory permissions are 0700 +- Check disk space + +**"OAuth session not found"** +- User needs to re-authenticate (session expired or invalidated) +- Check that UI database is accessible +- Verify OAuth session storage is working + +**"Invalid DPoP proof"** +- Clock skew between AppView and PDS +- DPoP key mismatch (token was issued with different key) +- Check that indigo library is managing DPoP correctly + +**"Client authentication failed"** +- Confidential client key may be corrupted +- Key ID may not match public key +- Try rotating the client key (delete and regenerate) + +### Debugging + +Enable debug logging to see OAuth flow details: + +```bash +export ATCR_LOG_LEVEL=debug +./bin/atcr-appview serve +``` + +Look for log messages: +- `"Generated new P-256 OAuth client key"` - Key was auto-generated +- `"Loaded existing P-256 OAuth client key"` - Key was loaded from disk +- `"Configured confidential OAuth client"` - Production confidential client active +- `"Localhost detected - using public OAuth client"` - Development public client active + +### Testing OAuth Flow + +Test OAuth flow manually: + +```bash +# 1. Start AppView in debug mode +ATCR_LOG_LEVEL=debug ./bin/atcr-appview serve + +# 2. Try docker login +docker login atcr.io + +# 3. Check logs for OAuth flow details +# Look for: PAR request, token exchange, DPoP headers, etc. +``` + +## References + +- [ATProto OAuth Specification](https://atproto.com/specs/oauth) +- [RFC 9449: OAuth 2.0 Demonstrating Proof of Possession (DPoP)](https://datatracker.ietf.org/doc/html/rfc9449) +- [RFC 9126: OAuth 2.0 Pushed Authorization Requests (PAR)](https://datatracker.ietf.org/doc/html/rfc9126) +- [RFC 7636: Proof Key for Code Exchange (PKCE)](https://datatracker.ietf.org/doc/html/rfc7636) +- [Indigo OAuth Library](https://github.com/bluesky-social/indigo/tree/main/atproto/auth/oauth) diff --git a/pkg/appview/config.go b/pkg/appview/config.go index ae33d3b..e474033 100644 --- a/pkg/appview/config.go +++ b/pkg/appview/config.go @@ -48,6 +48,14 @@ type ServerConfig struct { // DebugAddr is the debug/pprof HTTP listen address (from env: ATCR_DEBUG_ADDR, default: ":5001") DebugAddr string `yaml:"debug_addr"` + + // OAuthKeyPath is the path to the OAuth client P-256 signing key (from env: ATCR_OAUTH_KEY_PATH, default: "/var/lib/atcr/oauth/client.key") + // Auto-generated on first run for production (non-localhost) deployments + OAuthKeyPath string `yaml:"oauth_key_path"` + + // ClientName is the OAuth client display name (from env: ATCR_CLIENT_NAME, default: "AT Container Registry") + // Shown in OAuth authorization screens + ClientName string `yaml:"client_name"` } // UIConfig defines web UI settings @@ -123,6 +131,8 @@ func LoadConfigFromEnv() (*Config, error) { return nil, fmt.Errorf("ATCR_DEFAULT_HOLD_DID is required") } cfg.Server.TestMode = os.Getenv("TEST_MODE") == "true" + cfg.Server.OAuthKeyPath = getEnvOrDefault("ATCR_OAUTH_KEY_PATH", "/var/lib/atcr/oauth/client.key") + cfg.Server.ClientName = getEnvOrDefault("ATCR_CLIENT_NAME", "AT Container Registry") // Auto-detect base URL if not explicitly set cfg.Server.BaseURL = os.Getenv("ATCR_BASE_URL") diff --git a/pkg/appview/static/site.webmanifest b/pkg/appview/static/site.webmanifest index ccf313a..f16fc39 100644 --- a/pkg/appview/static/site.webmanifest +++ b/pkg/appview/static/site.webmanifest @@ -1,6 +1,6 @@ { - "name": "MyWebSite", - "short_name": "MySite", + "name": "At Container Registry", + "short_name": "ATCR.io", "icons": [ { "src": "/web-app-manifest-192x192.png", diff --git a/pkg/auth/oauth/client.go b/pkg/auth/oauth/client.go index caf6925..b0f4dea 100644 --- a/pkg/auth/oauth/client.go +++ b/pkg/auth/oauth/client.go @@ -7,6 +7,7 @@ package oauth import ( "context" "fmt" + "log/slog" "net/url" "strings" @@ -23,13 +24,58 @@ type App struct { } // NewApp creates a new OAuth app for ATCR with default scopes -func NewApp(baseURL string, store oauth.ClientAuthStore, holdDid string, testMode bool) (*App, error) { - return NewAppWithScopes(baseURL, store, GetDefaultScopes(holdDid, testMode)) +func NewApp(baseURL string, store oauth.ClientAuthStore, holdDid string, keyPath string, clientName string) (*App, error) { + return NewAppWithScopes(baseURL, store, GetDefaultScopes(holdDid), keyPath, clientName) } // NewAppWithScopes creates a new OAuth app for ATCR with custom scopes -func NewAppWithScopes(baseURL string, store oauth.ClientAuthStore, scopes []string) (*App, error) { - config := NewClientConfigWithScopes(baseURL, scopes) +// Automatically configures confidential client for production deployments +// keyPath specifies where to store/load the OAuth client P-256 key (ignored for localhost) +// clientName is added to OAuth client metadata +func NewAppWithScopes(baseURL string, store oauth.ClientAuthStore, scopes []string, keyPath string, clientName string) (*App, error) { + var config oauth.ClientConfig + redirectURI := RedirectURI(baseURL) + + // If production (not localhost), automatically set up confidential client + if !isLocalhost(baseURL) { + clientID := baseURL + "/client-metadata.json" + config = oauth.NewPublicConfig(clientID, redirectURI, scopes) + + // Generate or load P-256 key + privateKey, err := GenerateOrLoadClientKey(keyPath) + if err != nil { + return nil, fmt.Errorf("failed to load OAuth client key: %w", err) + } + + // Generate key ID from public key + keyID, err := GenerateKeyID(privateKey) + if err != nil { + return nil, fmt.Errorf("failed to generate key ID: %w", err) + } + + // Upgrade to confidential client + if err := config.SetClientSecret(privateKey, keyID); err != nil { + return nil, fmt.Errorf("failed to configure confidential client: %w", err) + } + + slog.Info("Configured confidential OAuth client", "key_id", keyID, "key_path", keyPath) + } else { + config = oauth.NewLocalhostConfig(redirectURI, scopes) + + // Append client_name to localhost client ID query string + if clientName != "" { + u, err := url.Parse(config.ClientID) + if err == nil { + q := u.Query() + q.Set("client_name", clientName) + u.RawQuery = q.Encode() + config.ClientID = u.String() + } + } + + slog.Info("Using public OAuth client (localhost development)") + } + clientApp := oauth.NewClientApp(&config, store) clientApp.Dir = atproto.GetDirectory() @@ -39,23 +85,8 @@ func NewAppWithScopes(baseURL string, store oauth.ClientAuthStore, scopes []stri }, nil } -// NewClientConfigWithScopes creates an OAuth client configuration with custom scopes -func NewClientConfigWithScopes(baseURL string, scopes []string) oauth.ClientConfig { - clientID := ClientIDWithScopes(baseURL, scopes) - redirectURI := RedirectURI(baseURL) - - // Check if this is localhost (public client) or production (confidential client) - if strings.Contains(baseURL, "127.0.0.1") || strings.Contains(baseURL, "localhost") { - return oauth.NewPublicConfig(clientID, redirectURI, scopes) - } - - // Production: confidential client - // Note: Client secrets would be configured separately if needed - return oauth.NewPublicConfig(clientID, redirectURI, scopes) -} - -func (a *App) GetConfig() oauth.ClientConfig { - return *a.clientApp.Config +func (a *App) GetConfig() *oauth.ClientConfig { + return a.clientApp.Config } // StartAuthFlow initiates an OAuth authorization flow for a given handle @@ -104,19 +135,6 @@ func (a *App) Directory() identity.Directory { return a.clientApp.Dir } -// ClientIDWithScopes generates a client ID with custom scopes -func ClientIDWithScopes(baseURL string, scopes []string) string { - scopeStr := strings.Join(scopes, " ") - if strings.Contains(baseURL, "127.0.0.1") || strings.Contains(baseURL, "localhost") { - // Localhost: use query-based client ID - return fmt.Sprintf("http://localhost?redirect_uri=%s&scope=%s", - url.QueryEscape(RedirectURI(baseURL)), - url.QueryEscape(scopeStr)) - } - // Production: use metadata URL - return baseURL + "/client-metadata.json" -} - // RedirectURI returns the OAuth redirect URI for ATCR func RedirectURI(baseURL string) string { return baseURL + "/auth/oauth/callback" @@ -124,7 +142,7 @@ func RedirectURI(baseURL string) string { // GetDefaultScopes returns the default OAuth scopes for ATCR registry operations // testMode determines whether to use transition:generic (test) or rpc scopes (production) -func GetDefaultScopes(did string, testMode bool) []string { +func GetDefaultScopes(did string) []string { scopes := []string{ "atproto", // Image manifest types (single-arch) @@ -135,10 +153,10 @@ func GetDefaultScopes(did string, testMode bool) []string { "blob:application/vnd.docker.distribution.manifest.list.v2+json", // OCI artifact manifests (for cosign signatures, SBOMs, attestations) "blob:application/vnd.cncf.oras.artifact.manifest.v1+json", + // Used for service token validation on holds + "rpc:com.atproto.repo.getRecord?aud=*", } - scopes = append(scopes, fmt.Sprintf("rpc:com.atproto.repo.getRecord?aud=%s", "*")) - // Add repo scopes scopes = append(scopes, fmt.Sprintf("repo:%s", atproto.ManifestCollection), @@ -176,3 +194,8 @@ func ScopesMatch(stored, desired []string) bool { return true } + +// isLocalhost checks if a base URL is a localhost address +func isLocalhost(baseURL string) bool { + return strings.Contains(baseURL, "127.0.0.1") || strings.Contains(baseURL, "localhost") +} diff --git a/pkg/auth/oauth/client_test.go b/pkg/auth/oauth/client_test.go index 99a9b54..7ba92df 100644 --- a/pkg/auth/oauth/client_test.go +++ b/pkg/auth/oauth/client_test.go @@ -7,6 +7,7 @@ import ( func TestNewApp(t *testing.T) { tmpDir := t.TempDir() storePath := tmpDir + "/oauth-test.json" + keyPath := tmpDir + "/oauth-key.bin" store, err := NewFileStore(storePath) if err != nil { @@ -16,7 +17,7 @@ func TestNewApp(t *testing.T) { baseURL := "http://localhost:5000" holdDID := "did:web:hold.example.com" - app, err := NewApp(baseURL, store, holdDID, false) + app, err := NewApp(baseURL, store, holdDID, keyPath, "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -33,6 +34,7 @@ func TestNewApp(t *testing.T) { func TestNewAppWithScopes(t *testing.T) { tmpDir := t.TempDir() storePath := tmpDir + "/oauth-test.json" + keyPath := tmpDir + "/oauth-key.bin" store, err := NewFileStore(storePath) if err != nil { @@ -42,7 +44,7 @@ func TestNewAppWithScopes(t *testing.T) { baseURL := "http://localhost:5000" scopes := []string{"atproto", "custom:scope"} - app, err := NewAppWithScopes(baseURL, store, scopes) + app, err := NewAppWithScopes(baseURL, store, scopes, keyPath, "AT Container Registry") if err != nil { t.Fatalf("NewAppWithScopes() error = %v", err) } diff --git a/pkg/auth/oauth/interactive.go b/pkg/auth/oauth/interactive.go index 4d6af75..eaa2354 100644 --- a/pkg/auth/oauth/interactive.go +++ b/pkg/auth/oauth/interactive.go @@ -35,11 +35,13 @@ func InteractiveFlowWithCallback( // Create OAuth app with custom scopes (or defaults if nil) // Interactive flows are typically for production use (credential helper, etc.) // so we default to testMode=false + // For CLI tools, we use an empty keyPath since they're typically localhost (public client) + // or ephemeral sessions var app *App if scopes != nil { - app, err = NewAppWithScopes(baseURL, store, scopes) + app, err = NewAppWithScopes(baseURL, store, scopes, "", "AT Container Registry") } else { - app, err = NewApp(baseURL, store, "*", false) + app, err = NewApp(baseURL, store, "*", "", "AT Container Registry") } if err != nil { return nil, fmt.Errorf("failed to create OAuth app: %w", err) diff --git a/pkg/auth/oauth/keys.go b/pkg/auth/oauth/keys.go new file mode 100644 index 0000000..ff98c43 --- /dev/null +++ b/pkg/auth/oauth/keys.go @@ -0,0 +1,193 @@ +package oauth + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "log/slog" + "os" + "path/filepath" + + "github.com/bluesky-social/indigo/atproto/atcrypto" +) + +// KeyType represents the elliptic curve algorithm for key generation +type KeyType int + +const ( + // KeyTypeP256 uses NIST P-256 (ES256) - standard for OAuth/OIDC + KeyTypeP256 KeyType = iota + // KeyTypeK256 uses secp256k1 (ES256K) - used for ATProto PDS signing + KeyTypeK256 +) + +// GenerateOrLoadKey generates a new key pair or loads an existing one +// Supports both P-256 (OAuth) and K-256 (ATProto PDS) key types +func GenerateOrLoadKey(keyPath string, keyType KeyType) (atcrypto.PrivateKey, error) { + // Ensure directory exists + dir := filepath.Dir(keyPath) + if err := os.MkdirAll(dir, 0700); err != nil { + return nil, fmt.Errorf("failed to create key directory: %w", err) + } + + // Check if key already exists + if _, err := os.Stat(keyPath); err == nil { + // Key exists, load it + return loadKey(keyPath, keyType) + } + + // Key doesn't exist, generate new one + return generateKey(keyPath, keyType) +} + +// generateKey creates a new key pair of the specified type +func generateKey(keyPath string, keyType KeyType) (atcrypto.PrivateKey, error) { + var privateKey atcrypto.PrivateKey + var keyName string + var err error + + switch keyType { + case KeyTypeP256: + privateKey, err = atcrypto.GeneratePrivateKeyP256() + keyName = "P-256" + case KeyTypeK256: + privateKey, err = atcrypto.GeneratePrivateKeyK256() + keyName = "K-256" + default: + return nil, fmt.Errorf("unsupported key type: %d", keyType) + } + + if err != nil { + return nil, fmt.Errorf("failed to generate %s key: %w", keyName, err) + } + + // Serialize key to bytes + exportableKey, ok := privateKey.(atcrypto.PrivateKeyExportable) + if !ok { + return nil, fmt.Errorf("key does not support export") + } + keyBytes := exportableKey.Bytes() + + // Write to file with restrictive permissions + if err := os.WriteFile(keyPath, keyBytes, 0600); err != nil { + return nil, fmt.Errorf("failed to write key file: %w", err) + } + + slog.Info("Generated new signing key", "type", keyName, "path", keyPath) + return privateKey, nil +} + +// loadKey loads an existing private key from disk +func loadKey(keyPath string, keyType KeyType) (atcrypto.PrivateKey, error) { + // Read key bytes + keyBytes, err := os.ReadFile(keyPath) + if err != nil { + return nil, fmt.Errorf("failed to read key file: %w", err) + } + + var privateKey atcrypto.PrivateKey + var keyName string + + switch keyType { + case KeyTypeP256: + privateKey, err = atcrypto.ParsePrivateBytesP256(keyBytes) + keyName = "P-256" + case KeyTypeK256: + // Check for old PEM format (migration path) + if IsPEMFormat(keyBytes) { + slog.Warn("Detected old P-256 PEM key, replacing with K-256") + return generateKey(keyPath, keyType) + } + privateKey, err = atcrypto.ParsePrivateBytesK256(keyBytes) + keyName = "K-256" + default: + return nil, fmt.Errorf("unsupported key type: %d", keyType) + } + + if err != nil { + return nil, fmt.Errorf("failed to parse %s private key: %w", keyName, err) + } + + slog.Info("Loaded existing signing key", "type", keyName, "path", keyPath) + return privateKey, nil +} + +// IsPEMFormat checks if bytes are in PEM format (for migration detection) +// Exported for testing and migration utilities +func IsPEMFormat(data []byte) bool { + return len(data) > 10 && string(data[:5]) == "-----" +} + +// GenerateOrLoadClientKey generates a new P256 key pair or loads an existing one +// This is a convenience wrapper for OAuth client keys +func GenerateOrLoadClientKey(keyPath string) (*atcrypto.PrivateKeyP256, error) { + key, err := GenerateOrLoadKey(keyPath, KeyTypeP256) + if err != nil { + return nil, err + } + + p256Key, ok := key.(*atcrypto.PrivateKeyP256) + if !ok { + return nil, fmt.Errorf("expected P-256 key, got different type") + } + + return p256Key, nil +} + +// GenerateOrLoadPDSKey generates a new K256 key pair or loads an existing one +// This is a convenience wrapper for ATProto PDS signing keys +func GenerateOrLoadPDSKey(keyPath string) (*atcrypto.PrivateKeyK256, error) { + key, err := GenerateOrLoadKey(keyPath, KeyTypeK256) + if err != nil { + return nil, err + } + + k256Key, ok := key.(*atcrypto.PrivateKeyK256) + if !ok { + return nil, fmt.Errorf("expected K-256 key, got different type") + } + + return k256Key, nil +} + +// GenerateKeyID generates a stable key ID from a P256 public key +// Uses the first 8 characters of the hex-encoded SHA256 hash of the public key bytes +func GenerateKeyID(privateKey *atcrypto.PrivateKeyP256) (string, error) { + // Get public key + pubKey, err := privateKey.PublicKey() + if err != nil { + return "", fmt.Errorf("failed to get public key: %w", err) + } + + // Get public key bytes + pubKeyBytes := pubKey.Bytes() + + // Hash public key bytes + hash := sha256.Sum256(pubKeyBytes) + + // Return first 8 characters of hex-encoded hash + return hex.EncodeToString(hash[:])[:8], nil +} + +// PrivateKeyToMultibase converts a P256 private key to multibase format +// Required by indigo's SetClientSecret() API +func PrivateKeyToMultibase(key *atcrypto.PrivateKeyP256) string { + return key.Multibase() +} + +// MultibaseToPrivateKey parses a multibase-encoded P256 private key +func MultibaseToPrivateKey(encoded string) (*atcrypto.PrivateKeyP256, error) { + // ParsePrivateMultibase returns PrivateKeyExportable interface + key, err := atcrypto.ParsePrivateMultibase(encoded) + if err != nil { + return nil, fmt.Errorf("failed to parse multibase key: %w", err) + } + + // Type assert to P256 key + p256Key, ok := key.(*atcrypto.PrivateKeyP256) + if !ok { + return nil, fmt.Errorf("expected P-256 key, got different key type") + } + + return p256Key, nil +} diff --git a/pkg/auth/oauth/refresher_test.go b/pkg/auth/oauth/refresher_test.go index b87bf89..bd619a7 100644 --- a/pkg/auth/oauth/refresher_test.go +++ b/pkg/auth/oauth/refresher_test.go @@ -13,7 +13,7 @@ func TestNewRefresher(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -45,7 +45,7 @@ func TestRefresher_SetUISessionStore(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } diff --git a/pkg/auth/oauth/server_test.go b/pkg/auth/oauth/server_test.go index 2f56ac2..095e5e1 100644 --- a/pkg/auth/oauth/server_test.go +++ b/pkg/auth/oauth/server_test.go @@ -19,7 +19,7 @@ func TestNewServer(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -43,7 +43,7 @@ func TestServer_SetRefresher(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -66,7 +66,7 @@ func TestServer_SetPostAuthCallback(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -92,7 +92,7 @@ func TestServer_SetUISessionStore(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -155,7 +155,7 @@ func TestServer_ServeAuthorize_MissingHandle(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -182,7 +182,7 @@ func TestServer_ServeAuthorize_InvalidMethod(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -211,7 +211,7 @@ func TestServer_ServeCallback_InvalidMethod(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -238,7 +238,7 @@ func TestServer_ServeCallback_OAuthError(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -270,7 +270,7 @@ func TestServer_ServeCallback_WithPostAuthCallback(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -314,7 +314,7 @@ func TestServer_ServeCallback_UIFlow_SessionCreationLogic(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -343,7 +343,7 @@ func TestServer_RenderError(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } @@ -377,7 +377,7 @@ func TestServer_RenderRedirectToSettings(t *testing.T) { t.Fatalf("NewFileStore() error = %v", err) } - app, err := NewApp("http://localhost:5000", store, "*", false) + app, err := NewApp("http://localhost:5000", store, "*", "", "AT Container Registry") if err != nil { t.Fatalf("NewApp() error = %v", err) } diff --git a/pkg/hold/oci/xrpc_test.go b/pkg/hold/oci/xrpc_test.go index 62fe078..50b86fe 100644 --- a/pkg/hold/oci/xrpc_test.go +++ b/pkg/hold/oci/xrpc_test.go @@ -14,6 +14,7 @@ import ( "testing" "atcr.io/pkg/atproto" + "atcr.io/pkg/auth/oauth" "atcr.io/pkg/hold/pds" "atcr.io/pkg/s3" "github.com/distribution/distribution/v3/registry/storage/driver/factory" @@ -37,7 +38,7 @@ func TestMain(m *testing.M) { // Generate one signing key to be reused across all tests sharedTestKeyPath = filepath.Join(tmpDir, "shared-signing-key") - privateKey, err := pds.GenerateOrLoadKey(sharedTestKeyPath) + privateKey, err := oauth.GenerateOrLoadPDSKey(sharedTestKeyPath) if err != nil { panic(fmt.Sprintf("Failed to generate shared signing key: %v", err)) } diff --git a/pkg/hold/pds/keys.go b/pkg/hold/pds/keys.go deleted file mode 100644 index 81df124..0000000 --- a/pkg/hold/pds/keys.go +++ /dev/null @@ -1,78 +0,0 @@ -package pds - -import ( - "fmt" - "log/slog" - "os" - "path/filepath" - - "github.com/bluesky-social/indigo/atproto/atcrypto" -) - -// GenerateOrLoadKey generates a new K256 key pair or loads an existing one -func GenerateOrLoadKey(keyPath string) (*atcrypto.PrivateKeyK256, error) { - // Ensure directory exists - dir := filepath.Dir(keyPath) - if err := os.MkdirAll(dir, 0700); err != nil { - return nil, fmt.Errorf("failed to create key directory: %w", err) - } - - // Check if key already exists - if _, err := os.Stat(keyPath); err == nil { - // Key exists, load it - return loadKey(keyPath) - } - - // Key doesn't exist, generate new one - return generateKey(keyPath) -} - -// generateKey creates a new K256 (secp256k1) key pair using indigo's atcrypto -func generateKey(keyPath string) (*atcrypto.PrivateKeyK256, error) { - // Generate K256 key (secp256k1) using indigo - privateKey, err := atcrypto.GeneratePrivateKeyK256() - if err != nil { - return nil, fmt.Errorf("failed to generate key: %w", err) - } - - // Serialize key to bytes - keyBytes := privateKey.Bytes() - - // Write to file with restrictive permissions - if err := os.WriteFile(keyPath, keyBytes, 0600); err != nil { - return nil, fmt.Errorf("failed to write key file: %w", err) - } - - slog.Info("Generated new K-256 signing key", "path", keyPath) - return privateKey, nil -} - -// loadKey loads an existing private key from disk -func loadKey(keyPath string) (*atcrypto.PrivateKeyK256, error) { - // Read key bytes - keyBytes, err := os.ReadFile(keyPath) - if err != nil { - return nil, fmt.Errorf("failed to read key file: %w", err) - } - - // Try to parse as K256 private key - privateKey, err := atcrypto.ParsePrivateBytesK256(keyBytes) - if err != nil { - // Check if this is an old P-256 PEM key (migration) - if isPEMFormat(keyBytes) { - slog.Warn("Detected old P-256 key, replacing with K-256") - // Generate new K-256 key (overwrites old P-256) - return generateKey(keyPath) - } - - return nil, fmt.Errorf("failed to parse private key: %w", err) - } - - slog.Info("Loaded existing K-256 signing key", "path", keyPath) - return privateKey, nil -} - -// isPEMFormat checks if bytes are in PEM format (old P-256 keys) -func isPEMFormat(data []byte) bool { - return len(data) > 10 && string(data[:5]) == "-----" -} diff --git a/pkg/hold/pds/keys_test.go b/pkg/hold/pds/keys_test.go deleted file mode 100644 index 9c5458a..0000000 --- a/pkg/hold/pds/keys_test.go +++ /dev/null @@ -1,437 +0,0 @@ -package pds - -import ( - "bytes" - "os" - "path/filepath" - "testing" - - "github.com/bluesky-social/indigo/atproto/atcrypto" -) - -// TestGenerateOrLoadKey_Generate tests generating a new key -func TestGenerateOrLoadKey_Generate(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "test-key") - - // Verify key doesn't exist yet - if _, err := os.Stat(keyPath); !os.IsNotExist(err) { - t.Fatal("Expected key file to not exist") - } - - // Generate key - key, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed: %v", err) - } - - if key == nil { - t.Fatal("Expected non-nil key") - } - - // Verify key file was created - if _, err := os.Stat(keyPath); os.IsNotExist(err) { - t.Error("Expected key file to be created") - } - - // Verify key file has restrictive permissions (0600) - fileInfo, err := os.Stat(keyPath) - if err != nil { - t.Fatalf("Failed to stat key file: %v", err) - } - - perm := fileInfo.Mode().Perm() - expectedPerm := os.FileMode(0600) - if perm != expectedPerm { - t.Errorf("Expected key file permissions %o, got %o", expectedPerm, perm) - } - - // Verify key can sign data - testData := []byte("test data") - signature, err := key.HashAndSign(testData) - if err != nil { - t.Fatalf("Failed to sign with generated key: %v", err) - } - - if len(signature) == 0 { - t.Error("Expected non-empty signature") - } - - // Verify signature - pubKey, err := key.PublicKey() - if err != nil { - t.Fatalf("Failed to get public key: %v", err) - } - - err = pubKey.HashAndVerify(testData, signature) - if err != nil { - t.Fatalf("Failed to verify signature: %v", err) - } -} - -// TestGenerateOrLoadKey_Load tests loading an existing key -func TestGenerateOrLoadKey_Load(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "test-key") - - // Generate initial key - key1, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed on first call: %v", err) - } - - // Get key bytes for comparison - key1Bytes := key1.Bytes() - - // Load the same key again - key2, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed on second call: %v", err) - } - - // Get key2 bytes - key2Bytes := key2.Bytes() - - // Verify keys are identical - if len(key1Bytes) != len(key2Bytes) { - t.Fatalf("Key byte length mismatch: %d vs %d", len(key1Bytes), len(key2Bytes)) - } - - for i := range key1Bytes { - if key1Bytes[i] != key2Bytes[i] { - t.Errorf("Key byte mismatch at position %d: %x vs %x", i, key1Bytes[i], key2Bytes[i]) - } - } - - // Verify both keys produce same signature for same data - testData := []byte("consistent test data") - - sig1, err := key1.HashAndSign(testData) - if err != nil { - t.Fatalf("Failed to sign with key1: %v", err) - } - - // Verify sig1 with key2's public key - pubKey2, err := key2.PublicKey() - if err != nil { - t.Fatalf("Failed to get public key from key2: %v", err) - } - - err = pubKey2.HashAndVerify(testData, sig1) - if err != nil { - t.Error("Signature from key1 should verify with key2 (they're the same key)") - } -} - -// TestGenerateOrLoadKey_P256Migration tests migrating from old P-256 keys -func TestGenerateOrLoadKey_P256Migration(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "old-pem-key") - - // Create a fake PEM file (old P-256 format) - pemContent := []byte(`-----BEGIN EC PRIVATE KEY----- -MHcCAQEEIFakeKeyDataHereThisIsNotARealKeyButHasPEMFormat ------END EC PRIVATE KEY-----`) - - err := os.WriteFile(keyPath, pemContent, 0600) - if err != nil { - t.Fatalf("Failed to write fake PEM key: %v", err) - } - - // Verify file exists and is in PEM format - data, err := os.ReadFile(keyPath) - if err != nil { - t.Fatalf("Failed to read key file: %v", err) - } - - if !isPEMFormat(data) { - t.Fatal("Expected key file to be in PEM format") - } - - // Call GenerateOrLoadKey - should detect PEM and generate new K-256 key - key, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed during P-256 migration: %v", err) - } - - if key == nil { - t.Fatal("Expected non-nil key after migration") - } - - // Verify key file was replaced (no longer PEM) - newData, err := os.ReadFile(keyPath) - if err != nil { - t.Fatalf("Failed to read new key file: %v", err) - } - - if isPEMFormat(newData) { - t.Error("Expected key file to no longer be in PEM format after migration") - } - - // Verify new key is K-256 and works - testData := []byte("test after migration") - signature, err := key.HashAndSign(testData) - if err != nil { - t.Fatalf("Failed to sign with migrated key: %v", err) - } - - pubKey, err := key.PublicKey() - if err != nil { - t.Fatalf("Failed to get public key: %v", err) - } - - err = pubKey.HashAndVerify(testData, signature) - if err != nil { - t.Fatalf("Failed to verify signature from migrated key: %v", err) - } -} - -// TestKeyPersistence tests that key bytes survive save/load cycle -func TestKeyPersistence(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "persist-key") - - // Generate key - originalKey, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed: %v", err) - } - - // Get original key bytes - originalBytes := originalKey.Bytes() - - // Read key file directly - fileBytes, err := os.ReadFile(keyPath) - if err != nil { - t.Fatalf("Failed to read key file: %v", err) - } - - // Verify file bytes match key bytes - if len(fileBytes) != len(originalBytes) { - t.Fatalf("File byte length mismatch: %d vs %d", len(fileBytes), len(originalBytes)) - } - - for i := range originalBytes { - if fileBytes[i] != originalBytes[i] { - t.Errorf("File byte mismatch at position %d: %x vs %x", i, fileBytes[i], originalBytes[i]) - } - } - - // Parse key directly from file bytes - parsedKey, err := atcrypto.ParsePrivateBytesK256(fileBytes) - if err != nil { - t.Fatalf("Failed to parse key from file bytes: %v", err) - } - - // Verify parsed key matches original - parsedBytes := parsedKey.Bytes() - if len(parsedBytes) != len(originalBytes) { - t.Fatalf("Parsed key byte length mismatch: %d vs %d", len(parsedBytes), len(originalBytes)) - } - - for i := range originalBytes { - if parsedBytes[i] != originalBytes[i] { - t.Errorf("Parsed key byte mismatch at position %d: %x vs %x", i, parsedBytes[i], originalBytes[i]) - } - } -} - -// TestGenerateOrLoadKey_DirectoryCreation tests that parent directory is created -func TestGenerateOrLoadKey_DirectoryCreation(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "nested", "dir", "test-key") - - // Verify nested directories don't exist - nestedDir := filepath.Join(tmpDir, "nested", "dir") - if _, err := os.Stat(nestedDir); !os.IsNotExist(err) { - t.Fatal("Expected nested directory to not exist") - } - - // Generate key (should create directories) - key, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed: %v", err) - } - - if key == nil { - t.Fatal("Expected non-nil key") - } - - // Verify directories were created - if _, err := os.Stat(nestedDir); os.IsNotExist(err) { - t.Error("Expected nested directory to be created") - } - - // Verify key file exists - if _, err := os.Stat(keyPath); os.IsNotExist(err) { - t.Error("Expected key file to be created") - } - - // Verify directory has restrictive permissions (0700) - dirInfo, err := os.Stat(nestedDir) - if err != nil { - t.Fatalf("Failed to stat directory: %v", err) - } - - dirPerm := dirInfo.Mode().Perm() - expectedDirPerm := os.FileMode(0700) - if dirPerm != expectedDirPerm { - t.Errorf("Expected directory permissions %o, got %o", expectedDirPerm, dirPerm) - } -} - -// TestIsPEMFormat tests the PEM format detection -func TestIsPEMFormat(t *testing.T) { - tests := []struct { - name string - data []byte - expected bool - }{ - { - name: "Valid PEM", - data: []byte("-----BEGIN EC PRIVATE KEY-----\ndata\n-----END EC PRIVATE KEY-----"), - expected: true, - }, - { - name: "Valid PEM (RSA)", - data: []byte("-----BEGIN RSA PRIVATE KEY-----\ndata\n-----END RSA PRIVATE KEY-----"), - expected: true, - }, - { - name: "Binary data", - data: []byte{0x01, 0x02, 0x03, 0x04, 0x05}, - expected: false, - }, - { - name: "Empty data", - data: []byte{}, - expected: false, - }, - { - name: "Short data", - data: []byte("----"), - expected: false, - }, - { - name: "Almost PEM (missing dashes)", - data: []byte("----BEGIN KEY-----"), - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := isPEMFormat(tt.data) - if result != tt.expected { - t.Errorf("Expected isPEMFormat=%v, got %v", tt.expected, result) - } - }) - } -} - -// TestGenerateKey_UniqueKeys tests that each generated key is unique -func TestGenerateKey_UniqueKeys(t *testing.T) { - tmpDir := t.TempDir() - - // Generate multiple keys - var keyBytes [][]byte - for i := 0; i < 5; i++ { - keyPath := filepath.Join(tmpDir, "key-"+string(rune('a'+i))) - key, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed for key %d: %v", i, err) - } - keyBytes = append(keyBytes, key.Bytes()) - } - - // Verify all keys are different - for i := 0; i < len(keyBytes); i++ { - for j := i + 1; j < len(keyBytes); j++ { - // Keys should be different - identical := true - if len(keyBytes[i]) != len(keyBytes[j]) { - identical = false - } else { - for k := range keyBytes[i] { - if keyBytes[i][k] != keyBytes[j][k] { - identical = false - break - } - } - } - - if identical { - t.Errorf("Keys %d and %d are identical (expected unique keys)", i, j) - } - } - } -} - -// TestLoadKey_InvalidFormat tests loading key with invalid format -func TestLoadKey_InvalidFormat(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "invalid-key") - - // Write invalid data (not a valid K-256 key and not PEM) - invalidData := []byte("This is not a valid key format at all") - err := os.WriteFile(keyPath, invalidData, 0600) - if err != nil { - t.Fatalf("Failed to write invalid key: %v", err) - } - - // Try to load (should fail with parse error, then try to generate new key) - // Since it's not PEM, it will try to parse as K-256 and fail, - // then NOT migrate (migration only happens for PEM), so it should error - _, err = GenerateOrLoadKey(keyPath) - if err == nil { - t.Fatal("Expected error when loading invalid key format") - } - - // Error should mention parsing failure - if err != nil && err.Error() == "" { - t.Error("Expected non-empty error message") - } -} - -// TestGenerateOrLoadKey_CorruptedKey tests behavior with corrupted key file -func TestGenerateOrLoadKey_CorruptedKey(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "corrupted-key") - - // Generate valid key first - key1, err := GenerateOrLoadKey(keyPath) - if err != nil { - t.Fatalf("GenerateOrLoadKey failed: %v", err) - } - - originalBytes := key1.Bytes() - - // Corrupt the key file (flip some bits in the middle) - corruptedBytes := make([]byte, len(originalBytes)) - copy(corruptedBytes, originalBytes) - if len(corruptedBytes) > 10 { - corruptedBytes[5] ^= 0xFF - corruptedBytes[10] ^= 0xFF - } - - err = os.WriteFile(keyPath, corruptedBytes, 0600) - if err != nil { - t.Fatalf("Failed to write corrupted key: %v", err) - } - - // Try to load corrupted key - // Note: K256 keys are flexible, so slightly corrupted keys might still be valid - // We just verify that it either loads successfully or returns an error - key2, err := GenerateOrLoadKey(keyPath) - if err != nil { - // Expected - corrupted key failed to load - t.Logf("Corrupted key failed to load as expected: %v", err) - return - } - - // If it loaded, verify it's different from the original - key2Bytes := key2.Bytes() - if bytes.Equal(originalBytes, key2Bytes) { - t.Error("Corrupted key loaded with same bytes as original (unexpected)") - } -} diff --git a/pkg/hold/pds/server.go b/pkg/hold/pds/server.go index 8ac991b..a5c187f 100644 --- a/pkg/hold/pds/server.go +++ b/pkg/hold/pds/server.go @@ -9,6 +9,7 @@ import ( "strings" "atcr.io/pkg/atproto" + "atcr.io/pkg/auth/oauth" "github.com/bluesky-social/indigo/atproto/atcrypto" "github.com/bluesky-social/indigo/carstore" lexutil "github.com/bluesky-social/indigo/lex/util" @@ -44,7 +45,7 @@ type HoldPDS struct { // NewHoldPDS creates or opens a hold PDS with SQLite carstore func NewHoldPDS(ctx context.Context, did, publicURL, dbPath, keyPath string, enableBlueskyPosts bool) (*HoldPDS, error) { // Generate or load signing key - signingKey, err := GenerateOrLoadKey(keyPath) + signingKey, err := oauth.GenerateOrLoadPDSKey(keyPath) if err != nil { return nil, fmt.Errorf("failed to initialize signing key: %w", err) } diff --git a/pkg/hold/pds/status_test.go b/pkg/hold/pds/status_test.go index 9f094c6..cee6e56 100644 --- a/pkg/hold/pds/status_test.go +++ b/pkg/hold/pds/status_test.go @@ -12,6 +12,7 @@ import ( "time" "atcr.io/pkg/atproto" + "atcr.io/pkg/auth/oauth" "atcr.io/pkg/s3" bsky "github.com/bluesky-social/indigo/api/bsky" ) @@ -265,7 +266,7 @@ func TestMain(m *testing.M) { // Generate one signing key to be reused across all tests in the package sharedTestKeyPath = filepath.Join(tmpDir, "shared-signing-key") - privateKey, err := GenerateOrLoadKey(sharedTestKeyPath) + privateKey, err := oauth.GenerateOrLoadPDSKey(sharedTestKeyPath) if err != nil { panic(fmt.Sprintf("Failed to generate shared signing key: %v", err)) }