diff --git a/modules/wristband/oidc_security_test.go b/modules/wristband/oidc_security_test.go new file mode 100644 index 00000000..bcc46714 --- /dev/null +++ b/modules/wristband/oidc_security_test.go @@ -0,0 +1,153 @@ +package wristband + +import ( + "bytes" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + "time" +) + +// TestOIDCCodeBindingSmoke pins T-19.4-02: a code is bound to the client +// and the redirect URI it was issued for, a failed redemption burns it, the +// authorize endpoint accepts only an S256 challenge of 43 to 128 base64url +// characters, the token endpoint only a verifier of 43 to 128 unreserved +// characters, and a code lives 60 seconds by default. +func TestOIDCCodeBindingSmoke(t *testing.T) { + f := newOIDCFixture(t, testOIDCIssuer, nil) + other := f.backend.addClient(OIDCClientRecord{ + ClientID: "oas_other", Kind: ClientKindOIDC, RedirectURIs: []string{testOIDCRedirect}, + Scopes: []string{"openid"}, IsActive: true, + }, testOIDCSecret) + + wantInvalidGrant := func(what string, rec *httptest.ResponseRecorder) { + t.Helper() + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), `"error":"invalid_grant"`) { + t.Fatalf("%s = %d %s, want 400 invalid_grant", what, rec.Code, rec.Body.String()) + } + } + + // A code issued to one client is refused for another and is burned. + code := f.obtainCode(t, f.client, 7, "openid") + wantInvalidGrant("code of another client", f.exchange(other, testOIDCSecret, code, oidcTestVerifier)) + wantInvalidGrant("code after a client mismatch", f.exchange(f.client, f.clientSecret, code, oidcTestVerifier)) + + // A code presented with another redirect URI is refused and burned. + code = f.obtainCode(t, f.client, 7, "openid") + wantInvalidGrant("redirect mismatch", f.tokenCall(url.Values{ + "grant_type": {"authorization_code"}, "code": {code}, + "redirect_uri": {testOIDCRedirect + "/elsewhere"}, "code_verifier": {oidcTestVerifier}, + }, f.client.ClientID, f.clientSecret)) + wantInvalidGrant("code after a redirect mismatch", f.exchange(f.client, f.clientSecret, code, oidcTestVerifier)) + + // Authorize refuses the plain method and a short challenge. + for _, tc := range []struct { + name string + mutate func(url.Values) + desc string + }{ + {"plain method", func(q url.Values) { + q.Set("code_challenge_method", "plain") + q.Set("code_challenge", oidcTestVerifier) + }, "The+code+challenge+method+is+required+and+must+be+S256."}, + {"short challenge", func(q url.Values) { q.Set("code_challenge", strings.Repeat("a", 42)) }, + "A+PKCE+code+challenge+is+required+and+must+be+a+base64url-encoded+SHA-256+digest."}, + {"challenge outside base64url", func(q url.Values) { q.Set("code_challenge", strings.Repeat("a", 42)+"+") }, + "A+PKCE+code+challenge+is+required+and+must+be+a+base64url-encoded+SHA-256+digest."}, + } { + rec := f.get("/oauth/authorize?" + oidcAuthorizeParams(f, tc.mutate).Encode()) + if rec.Code != http.StatusFound || !strings.Contains(rec.Header().Get("Location"), "error=invalid_request&error_description="+tc.desc) { + t.Fatalf("%s: %d %q", tc.name, rec.Code, rec.Header().Get("Location")) + } + } + + // The verifier rules: S256 only, 43 to 128 unreserved characters. + short := strings.Repeat("v", 42) + long := strings.Repeat("v", 129) + odd := strings.Repeat("v", 42) + "!" + for _, tc := range []struct { + verifier, challenge, method string + want bool + }{ + {oidcTestVerifier, s256Challenge(oidcTestVerifier), "S256", true}, + {oidcTestVerifier, oidcTestVerifier, "plain", false}, + {oidcTestVerifier, s256Challenge(oidcTestVerifier), "plain", false}, + {short, s256Challenge(short), "S256", false}, + {long, s256Challenge(long), "S256", false}, + {odd, s256Challenge(odd), "S256", false}, + {oidcTestVerifier, "", "S256", false}, + } { + if got := verifyOIDCPkce(tc.verifier, tc.challenge, tc.method); got != tc.want { + t.Fatalf("verifyOIDCPkce(%d chars, %s) = %v, want %v", len(tc.verifier), tc.method, got, tc.want) + } + } + + if ttl := DefaultOIDCOptions().CodeTTL; ttl != 60*time.Second { + t.Fatalf("default CodeTTL = %s, want 60s", ttl) + } +} + +// TestOIDCLogsCarryNoCredentials pins T-19.4-09: a rejected token exchange +// logs the client id, the client IP and a fixed reason, never the secret, +// the code or the verifier; key parsing errors never quote the key. +func TestOIDCLogsCarryNoCredentials(t *testing.T) { + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil))) + t.Cleanup(func() { slog.SetDefault(prev) }) + + f := newOIDCFixture(t, testOIDCIssuer, nil) + const wrongSecret = "oas_cs_wrong-guess-0123456789abcdefghijklmnopqrstuv" + const wrongVerifier = "wrong-verifier-0123456789abcdefghijklmnopqrstuvwxyz" + + // Wrong secret in Basic and in the form. + f.tokenCall(url.Values{"grant_type": {"authorization_code"}}, f.client.ClientID, wrongSecret) + f.tokenCall(url.Values{ + "grant_type": {"authorization_code"}, "client_id": {f.client.ClientID}, "client_secret": {wrongSecret}, + }, "", "") + // A wrong verifier burns the code, the replay revokes. + code := f.obtainCode(t, f.client, 7, "openid") + f.exchange(f.client, f.clientSecret, code, wrongVerifier) + f.exchange(f.client, f.clientSecret, code, oidcTestVerifier) + // A code that never existed. + f.exchange(f.client, f.clientSecret, "oas_ac_never-issued-0123456789abcdefghijklmnop", oidcTestVerifier) + + logs := buf.String() + for _, want := range []string{"reason=invalid_client", "reason=invalid_grant:pkce", "reason=invalid_grant:replay", "client_id=" + f.client.ClientID} { + if !strings.Contains(logs, want) { + t.Fatalf("log lacks %q:\n%s", want, logs) + } + } + for _, secret := range []string{wrongSecret, testOIDCSecret, code, wrongVerifier, oidcTestVerifier, "oas_ac_", "oas_cs_", "oas_at_"} { + if strings.Contains(logs, secret) { + t.Fatalf("log carries a credential (%q):\n%s", secret, logs) + } + } + + // Key parsing errors are fixed texts. + const marker = "U0VDUkVUTUFSS0VS" + if _, err := ParsePrivateKeyPEM([]byte("-----BEGIN PRIVATE KEY-----\n" + marker + "\n-----END PRIVATE KEY-----\n")); err == nil || strings.Contains(err.Error(), marker) { + t.Fatalf("private key error = %v", err) + } + if _, err := ParsePublicKeyPEM([]byte("-----BEGIN PUBLIC KEY-----\n" + marker + "\n-----END PUBLIC KEY-----\n")); err == nil || strings.Contains(err.Error(), marker) { + t.Fatalf("public key error = %v", err) + } +} + +// uuidV4 is the shape of a random subject. +var uuidV4 = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`) + +// TestOIDCSubjectIsRandomSmoke pins T-19.4-13: a public sub is a random +// UUIDv4, never the user id, the email or the name. +func TestOIDCSubjectIsRandomSmoke(t *testing.T) { + f := newOIDCFixture(t, testOIDCIssuer, nil) + tok := decodeToken(t, f.exchange(f.client, f.clientSecret, f.obtainCode(t, f.client, 7, "openid"), oidcTestVerifier)) + sub, _ := f.parseIDToken(t, tok.IDToken)["sub"].(string) + if !uuidV4.MatchString(sub) || sub == "7" || strings.Contains(sub, "ada") { + t.Fatalf("sub = %q, want a random UUIDv4", sub) + } +}