diff --git a/README.md b/README.md index 50f465f..0ef0c78 100644 --- a/README.md +++ b/README.md @@ -780,6 +780,15 @@ Then add an Apple provider that accepts the following parameters: Subsequent logins to your app using Sign In with Apple with the same account do not share any user info and will only return a user identifier in IDToken claims. This behaves correctly until a user delete sign in for you service with Apple ID in own Apple account profile (security section). It is recommend that you securely cache the at first login containing the user info for bind it with a user UID at next login. + +**Token validation:** + +The provider verifies the Apple `id_token` signature against Apple's published JWK, then enforces: + +* `iss == "https://appleid.apple.com"` -- per Apple's Sign-in-with-Apple [REST API spec](https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user) +* `aud == ClientID` (your Service ID or App ID) + +A token signed by Apple but issued for a different `aud` is rejected with `403 invalid id_token`. The `aud` check (not the `iss` check) is what closes the confused-deputy attack: a sibling service in the same Apple developer team (or any other Sign-in-with-Apple client) holds tokens with the real Apple `iss` but their own `aud`, and could replay one of those tokens against this service if `aud` were not enforced. Provider always get user `UID` (`sub` claim) in `IDToken`. * Apple doesn't have an API for fetch avatar and user info. diff --git a/provider/apple.go b/provider/apple.go index 86ade33..05a7d36 100644 --- a/provider/apple.go +++ b/provider/apple.go @@ -13,6 +13,7 @@ import ( "crypto/x509" "encoding/json" "encoding/pem" + "errors" "fmt" "io" "net/http" @@ -354,6 +355,12 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) { return } + if err = validateAppleIDClaims(tokenClaims, ah.conf.ClientID); err != nil { + ah.Logf("[WARN] apple id_token rejected: %s", err.Error()) + rest.SendErrorJSON(w, r, ah.L, http.StatusForbidden, nil, "invalid id_token") + return + } + u := ah.mapUser(tokenClaims) u, err = setAvatar(ah.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second}) @@ -568,3 +575,21 @@ func presence(s string) string { } return "present" } + +// appleIDTokenIssuer is the issuer Apple sets on every id_token issued by Sign in with Apple. +// see https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user +const appleIDTokenIssuer = "https://appleid.apple.com" // #nosec G101 -- public Apple issuer URL, not a credential + +// validateAppleIDClaims checks that the id_token claims have the expected Apple +// issuer and that the audience matches the relying party's client ID. The signature +// must already have been verified by the caller; this only catches confused-deputy +// cases where another Apple-signed token is replayed against the wrong audience. +func validateAppleIDClaims(claims jwt.MapClaims, expectedAud string) error { + if !claims.VerifyIssuer(appleIDTokenIssuer, true) { + return errors.New("invalid id_token issuer") + } + if !claims.VerifyAudience(expectedAud, true) { + return errors.New("invalid id_token audience") + } + return nil +} diff --git a/provider/apple_test.go b/provider/apple_test.go index 351b018..c8341f7 100644 --- a/provider/apple_test.go +++ b/provider/apple_test.go @@ -177,6 +177,51 @@ func TestAppleHandlerCreateClientSecret(t *testing.T) { assert.Equal(t, "auth.example.com", testClaims["sub"]) } +func TestValidateAppleIDClaims(t *testing.T) { + const clientID = "auth.example.com" + tests := []struct { + name string + claims jwt.MapClaims + wantErr string + }{ + { + name: "valid iss and aud accepted", + claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": clientID, "sub": "u1"}, + }, + { + name: "wrong issuer rejected", + claims: jwt.MapClaims{"iss": "https://attacker.example.com", "aud": clientID, "sub": "u1"}, + wantErr: "invalid id_token issuer", + }, + { + name: "missing issuer rejected", + claims: jwt.MapClaims{"aud": clientID, "sub": "u1"}, + wantErr: "invalid id_token issuer", + }, + { + name: "wrong audience rejected", + claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": "other.example.com", "sub": "u1"}, + wantErr: "invalid id_token audience", + }, + { + name: "missing audience rejected", + claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "sub": "u1"}, + wantErr: "invalid id_token audience", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateAppleIDClaims(tt.claims, clientID) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} + func TestAppleParseUserData(t *testing.T) { ah := AppleHandler{Params: Params{L: logger.NoOp}} @@ -327,14 +372,54 @@ func TestAppleHandler_LoginHandler(t *testing.T) { } +// TestAppleHandler_LoginHandler_RejectsWrongIssuer is the regression-style +// integration test: drives the full LoginHandler exchange flow with a token +// signed by the test JWK but carrying iss other than https://appleid.apple.com. +func TestAppleHandler_LoginHandler_RejectsWrongIssuer(t *testing.T) { + override := testIDTokenOverride{iss: "https://attacker.example.com"} + teardown := prepareAppleOauthTest(t, 8983, 8984, nil, override) + defer teardown() + + jar, err := cookiejar.New(nil) + require.NoError(t, err) + client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + + resp, err := client.Get("http://localhost:8983/login?site=remark") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong issuer must be rejected at the handler boundary") + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "invalid id_token") +} + +// TestAppleHandler_LoginHandler_RejectsWrongAudience is the symmetric +// regression test to TestAppleHandler_LoginHandler_RejectsWrongIssuer. +func TestAppleHandler_LoginHandler_RejectsWrongAudience(t *testing.T) { + override := testIDTokenOverride{aud: "other.example.com"} + teardown := prepareAppleOauthTest(t, 8985, 8986, nil, override) + defer teardown() + + jar, err := cookiejar.New(nil) + require.NoError(t, err) + client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + + resp, err := client.Get("http://localhost:8985/login?site=remark") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong audience must be rejected at the handler boundary") + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "invalid id_token") +} + // TestAppleHandler_LoginHandlerFromRejectsExternalHost is the regression -// test for the redirect validator on the apple login path: with an allowlist -// policy enabled, /login?from=https://evil.example.com must NOT 302 to evil. +// test for the redirect validator on the apple login path. func TestAppleHandler_LoginHandlerFromRejectsExternalHost(t *testing.T) { enablePolicy := func(p *Params) { p.AllowedRedirectHosts = token.AllowedHostsFunc(func() ([]string, error) { return nil, nil }) } - teardown := prepareAppleOauthTest(t, 8987, 8988, nil, enablePolicy) + teardown := prepareAppleOauthTest(t, 8987, 8988, nil, paramOpts(enablePolicy)) defer teardown() jar, err := cookiejar.New(nil) @@ -488,7 +573,25 @@ func prepareAppleHandlerTest(responseMode string, scopes []string) (*AppleHandle return NewApple(p, aCfg, cl) } -func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, paramOpts ...func(*Params)) func() { +// paramOpts wraps a Params modifier so it can be passed alongside a +// testIDTokenOverride through prepareAppleOauthTest's variadic any list. +type paramOpts func(*Params) + +func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, opts ...any) func() { + var override testIDTokenOverride + var paramMods []func(*Params) + for _, o := range opts { + switch v := o.(type) { + case testIDTokenOverride: + override = v + case paramOpts: + paramMods = append(paramMods, v) + case func(*Params): + paramMods = append(paramMods, v) + default: + t.Fatalf("prepareAppleOauthTest: unsupported option type %T", o) + } + } signKey, testJWK := createTestSignKeyPairs(t) provider, err := prepareAppleHandlerTest("", []string{}) assert.NoError(t, err) @@ -511,7 +614,7 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str require.NoError(t, err) // create self-signed JWT - testResponseToken, err := createTestResponseToken(signKey) + testResponseToken, err := createTestResponseTokenWith(signKey, override.issOrDefault(), override.audOrDefault()) require.NoError(t, err) require.NotEmpty(t, testResponseToken) if testToken != nil { @@ -535,7 +638,7 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str params := Params{URL: "url", Cid: "cid", Csecret: "csecret", JwtService: jwtService, Issuer: "go-pkgz/auth", L: logger.Std} - for _, opt := range paramOpts { + for _, opt := range paramMods { opt(¶ms) } provider.Params = params @@ -647,12 +750,35 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str } } -func createTestResponseToken(privKey any) (string, error) { +// testIDTokenOverride lets a test inject a token with non-default iss/aud +// through prepareAppleOauthTest. The default (zero value) produces a token +// with the canonical Apple issuer and the test ClientID, exercising the +// happy path through validateAppleIDClaims. +type testIDTokenOverride struct { + iss string + aud string +} + +func (o testIDTokenOverride) issOrDefault() string { + if o.iss == "" { + return appleIDTokenIssuer + } + return o.iss +} + +func (o testIDTokenOverride) audOrDefault() string { + if o.aud == "" { + return "auth.example.com" + } + return o.aud +} + +func createTestResponseTokenWith(privKey any, iss, aud string) (string, error) { claims := &jwt.MapClaims{ - "iss": "http://go.localhost.test", + "iss": iss, "iat": time.Now().Unix(), "exp": time.Now().Add(time.Second * 30).Unix(), - "aud": "go-pkgz/auth", + "aud": aud, "sub": "userid1", "email": "test@example.go", } diff --git a/v2/provider/apple.go b/v2/provider/apple.go index d4b6521..aaee874 100644 --- a/v2/provider/apple.go +++ b/v2/provider/apple.go @@ -13,6 +13,7 @@ import ( "crypto/x509" "encoding/json" "encoding/pem" + "errors" "fmt" "io" "net/http" @@ -345,10 +346,22 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) { return } - // get token claims for extract uid (and email or name if they exist in scope) + // get token claims for extract uid (and email or name if they exist in scope). + // jwt v5 parser options enforce iss == https://appleid.apple.com and + // aud == ClientID inline so we don't need a separate validate pass. tokenClaims := jwt.MapClaims{} - _, err = jwt.ParseWithClaims(resp.IDToken, tokenClaims, keySet.keyFunc) + _, err = jwt.ParseWithClaims(resp.IDToken, tokenClaims, keySet.keyFunc, + jwt.WithIssuer(appleIDTokenIssuer), + jwt.WithAudience(ah.conf.ClientID)) if err != nil { + // distinguish a confused-deputy reject (iss/aud) from a server-side + // parse/sig failure so the handler returns the same 403 + body as + // before for the security-relevant case. + if errors.Is(err, jwt.ErrTokenInvalidIssuer) || errors.Is(err, jwt.ErrTokenInvalidAudience) { + ah.Logf("[WARN] apple id_token rejected: %s", err.Error()) + rest.SendErrorJSON(w, r, ah.L, http.StatusForbidden, nil, "invalid id_token") + return + } ah.Logf("[ERROR] failed to get claims: " + err.Error()) rest.SendErrorJSON(w, r, ah.L, http.StatusInternalServerError, nil, fmt.Sprintf("failed to token validation, key is invalid: %s", resp.Error)) return @@ -568,3 +581,7 @@ func presence(s string) string { } return "present" } + +// appleIDTokenIssuer is the issuer Apple sets on every id_token issued by Sign in with Apple. +// see https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user +const appleIDTokenIssuer = "https://appleid.apple.com" // #nosec G101 -- public Apple issuer URL, not a credential diff --git a/v2/provider/apple_test.go b/v2/provider/apple_test.go index 029df04..ba220e2 100644 --- a/v2/provider/apple_test.go +++ b/v2/provider/apple_test.go @@ -285,7 +285,7 @@ func TestAppleHandlerMakeRedirURL(t *testing.T) { func TestAppleHandler_LoginHandler(t *testing.T) { - teardown := prepareAppleOauthTest(t, 8981, 8982, nil) + teardown := prepareAppleOauthTest(t, 8981, 8982, nil, testIDTokenOverride{}) defer teardown() jar, err := cookiejar.New(nil) @@ -331,7 +331,7 @@ func TestAppleHandler_LoginHandlerFromRejectsExternalHost(t *testing.T) { enablePolicy := func(p *Params) { p.AllowedRedirectHosts = token.AllowedHostsFunc(func() ([]string, error) { return nil, nil }) } - teardown := prepareAppleOauthTest(t, 8987, 8988, nil, enablePolicy) + teardown := prepareAppleOauthTest(t, 8987, 8988, nil, testIDTokenOverride{}, enablePolicy) defer teardown() jar, err := cookiejar.New(nil) @@ -355,9 +355,59 @@ func TestAppleHandler_LoginHandlerFromRejectsExternalHost(t *testing.T) { assert.NotContains(t, string(body), "evil.example.com") } +// TestAppleHandler_LoginHandler_RejectsWrongIssuer is the regression-style +// integration test: drives the full LoginHandler exchange flow with a token +// signed by the test JWK but carrying iss other than https://appleid.apple.com. +// With the jwt.WithIssuer parser option in place the handler returns 403; if +// the option is dropped the foreign-iss token authenticates and the handler +// returns 200 with a JWT cookie -- this test then fails on the status-code +// assertion. +func TestAppleHandler_LoginHandler_RejectsWrongIssuer(t *testing.T) { + override := testIDTokenOverride{iss: "https://attacker.example.com"} + teardown := prepareAppleOauthTest(t, 8983, 8984, nil, override) + defer teardown() + + jar, err := cookiejar.New(nil) + require.NoError(t, err) + client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + + resp, err := client.Get("http://localhost:8983/login?site=remark") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong issuer must be rejected at the handler boundary") + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "invalid id_token") +} + +// TestAppleHandler_LoginHandler_RejectsWrongAudience is the symmetric +// regression test to TestAppleHandler_LoginHandler_RejectsWrongIssuer: +// drives the LoginHandler exchange flow with a token whose iss is the +// real Apple issuer but whose aud belongs to a different relying party. +// Without the jwt.WithAudience parser option this is the confused-deputy +// path -- a sibling Sign-in-with-Apple client in the same Apple developer +// team can mint a token for itself and replay it here. +func TestAppleHandler_LoginHandler_RejectsWrongAudience(t *testing.T) { + override := testIDTokenOverride{aud: "other.example.com"} + teardown := prepareAppleOauthTest(t, 8985, 8986, nil, override) + defer teardown() + + jar, err := cookiejar.New(nil) + require.NoError(t, err) + client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + + resp, err := client.Get("http://localhost:8985/login?site=remark") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong audience must be rejected at the handler boundary") + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "invalid id_token") +} + func TestAppleHandler_LogoutHandler(t *testing.T) { - teardown := prepareAppleOauthTest(t, 8691, 8692, nil) + teardown := prepareAppleOauthTest(t, 8691, 8692, nil, testIDTokenOverride{}) defer teardown() jar, err := cookiejar.New(nil) @@ -389,7 +439,7 @@ func TestAppleHandler_LogoutHandler(t *testing.T) { func TestAppleHandler_Exchange(t *testing.T) { var testResponseToken string - teardown := prepareAppleOauthTest(t, 8981, 8982, &testResponseToken) + teardown := prepareAppleOauthTest(t, 8981, 8982, &testResponseToken, testIDTokenOverride{}) defer teardown() ah, err := prepareAppleHandlerTest("", []string{}) @@ -485,7 +535,7 @@ func prepareAppleHandlerTest(responseMode string, scopes []string) (*AppleHandle return NewApple(p, aCfg, cl) } -func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, paramOpts ...func(*Params)) func() { +func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, override testIDTokenOverride, paramOpts ...func(*Params)) func() { signKey, testJWK := createTestSignKeyPairs(t) provider, err := prepareAppleHandlerTest("", []string{}) assert.NoError(t, err) @@ -508,7 +558,7 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str require.NoError(t, err) // create self-signed JWT - testResponseToken, err := createTestResponseToken(signKey) + testResponseToken, err := createTestResponseTokenWith(signKey, override.issOrDefault(), override.audOrDefault()) require.NoError(t, err) require.NotEmpty(t, testResponseToken) if testToken != nil { @@ -644,12 +694,35 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str } } -func createTestResponseToken(privKey any) (string, error) { +// testIDTokenOverride lets a test inject a token with non-default iss/aud +// through prepareAppleOauthTest. The default (zero value) produces a token +// with the canonical Apple issuer and the test ClientID, exercising the +// happy path through the jwt.WithIssuer / jwt.WithAudience parser options. +type testIDTokenOverride struct { + iss string + aud string +} + +func (o testIDTokenOverride) issOrDefault() string { + if o.iss == "" { + return appleIDTokenIssuer + } + return o.iss +} + +func (o testIDTokenOverride) audOrDefault() string { + if o.aud == "" { + return "auth.example.com" + } + return o.aud +} + +func createTestResponseTokenWith(privKey any, iss, aud string) (string, error) { claims := &jwt.MapClaims{ - "iss": "http://go.localhost.test", + "iss": iss, "iat": time.Now().Unix(), "exp": time.Now().Add(time.Second * 30).Unix(), - "aud": "go-pkgz/auth", + "aud": aud, "sub": "userid1", "email": "test@example.go", }