From f2545780c8234241fbb71e8fbdcd11355bad88ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Sat, 17 Aug 2024 16:35:30 +0300 Subject: [PATCH 1/8] fix(op): initialize http Headers in response objects --- pkg/op/server.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/op/server.go b/pkg/op/server.go index 6faee87..b500e43 100644 --- a/pkg/op/server.go +++ b/pkg/op/server.go @@ -218,7 +218,8 @@ type Response struct { // without custom headers. func NewResponse(data any) *Response { return &Response{ - Data: data, + Header: make(http.Header), + Data: data, } } @@ -242,7 +243,10 @@ type Redirect struct { } func NewRedirect(url string) *Redirect { - return &Redirect{URL: url} + return &Redirect{ + Header: make(http.Header), + URL: url, + } } func (red *Redirect) writeOut(w http.ResponseWriter, r *http.Request) { From fa73f36780989f6fe20c95e3e9bbfd888c3dc33f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Sun, 18 Aug 2024 13:14:01 +0300 Subject: [PATCH 2/8] fix test --- pkg/op/error_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/op/error_test.go b/pkg/op/error_test.go index 170039c..107f9d0 100644 --- a/pkg/op/error_test.go +++ b/pkg/op/error_test.go @@ -428,7 +428,8 @@ func TestTryErrorRedirect(t *testing.T) { parent: oidc.ErrInteractionRequired().WithDescription("sign in"), }, want: &Redirect{ - URL: "http://example.com/callback?error=interaction_required&error_description=sign+in&state=state1", + Header: make(http.Header), + URL: "http://example.com/callback?error=interaction_required&error_description=sign+in&state=state1", }, wantLog: `{ "level":"WARN", From 7e1846e6e2c7a85eaa2715146af44010924ea818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Mon, 19 Aug 2024 20:57:07 +0300 Subject: [PATCH 3/8] feat(crypto): hash algorithm for EdDSA --- pkg/crypto/hash.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/crypto/hash.go b/pkg/crypto/hash.go index ab9f8c1..552d769 100644 --- a/pkg/crypto/hash.go +++ b/pkg/crypto/hash.go @@ -21,6 +21,16 @@ func GetHashAlgorithm(sigAlgorithm jose.SignatureAlgorithm) (hash.Hash, error) { return sha512.New384(), nil case jose.RS512, jose.ES512, jose.PS512: return sha512.New(), nil + + // There is no published spec for this yet. + // There is consensus here: https://bitbucket.org/openid/connect/issues/1125/_hash-algorithm-for-eddsa-id-tokens + // Currently go-jose only supports the ed25519 curve key for EdDSA, so we can safely assume sha512 here. + // + // TODO: When go-jose ever decides to support ed448, we need to know the "crv" parameter and use shake256 for ed448. + // The "crv" value is currently not exposed by go-jose.JSONWebKey and is currently only hard-coded to be set during marshalling. + case jose.EdDSA: + return sha512.New(), nil + default: return nil, fmt.Errorf("%w: %q", ErrUnsupportedAlgorithm, sigAlgorithm) } From eb249c4c70340ebd7b0add8f129865721d070a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Mon, 19 Aug 2024 21:04:21 +0300 Subject: [PATCH 4/8] chore(example): add supported signing algorithms to RP --- example/client/app/app.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/example/client/app/app.go b/example/client/app/app.go index 448c530..a870e00 100644 --- a/example/client/app/app.go +++ b/example/client/app/app.go @@ -11,6 +11,7 @@ import ( "sync/atomic" "time" + "github.com/go-jose/go-jose/v4" "github.com/google/uuid" "github.com/sirupsen/logrus" @@ -53,7 +54,23 @@ func main() { options := []rp.Option{ rp.WithCookieHandler(cookieHandler), - rp.WithVerifierOpts(rp.WithIssuedAtOffset(5 * time.Second)), + rp.WithVerifierOpts( + rp.WithIssuedAtOffset(5*time.Second), + // When the OP uses other signing algorithms then RS256, + // We need to tell the RP to accept them. + // The actual handshake is done with the "kid" and "alg" header claims. + // However, [jose.ParseSigned] needs a list of algorithms we are willing to accept. + // This example sets all the algorithms the ZITADEL product supports. + rp.WithSupportedSigningAlgorithms( + string(jose.EdDSA), + string(jose.RS256), + string(jose.RS384), + string(jose.RS512), + string(jose.ES256), + string(jose.ES384), + string(jose.ES512), + ), + ), rp.WithHTTPClient(client), rp.WithLogger(logger), } From 2a3e87afff95f127e17de602879881d17cc7e67c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 20 Aug 2024 10:44:45 +0300 Subject: [PATCH 5/8] update code comment --- pkg/crypto/hash.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/crypto/hash.go b/pkg/crypto/hash.go index 552d769..14acdee 100644 --- a/pkg/crypto/hash.go +++ b/pkg/crypto/hash.go @@ -22,12 +22,10 @@ func GetHashAlgorithm(sigAlgorithm jose.SignatureAlgorithm) (hash.Hash, error) { case jose.RS512, jose.ES512, jose.PS512: return sha512.New(), nil - // There is no published spec for this yet. + // There is no published spec for this yet, but we have confirmation it will get published. // There is consensus here: https://bitbucket.org/openid/connect/issues/1125/_hash-algorithm-for-eddsa-id-tokens - // Currently go-jose only supports the ed25519 curve key for EdDSA, so we can safely assume sha512 here. - // - // TODO: When go-jose ever decides to support ed448, we need to know the "crv" parameter and use shake256 for ed448. - // The "crv" value is currently not exposed by go-jose.JSONWebKey and is currently only hard-coded to be set during marshalling. + // Currently Go and go-jose only supports the ed25519 curve key for EdDSA, so we can safely assume sha512 here. + // It is unlikely ed448 will ever be supported: https://github.com/golang/go/issues/29390 case jose.EdDSA: return sha512.New(), nil From 248df8c1f11f4c099252dab5a1d35f7a26a65fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 20 Aug 2024 11:44:09 +0300 Subject: [PATCH 6/8] rp: modify keytype check to support EdDSA --- pkg/oidc/keyset.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/oidc/keyset.go b/pkg/oidc/keyset.go index 833878d..a8b89b0 100644 --- a/pkg/oidc/keyset.go +++ b/pkg/oidc/keyset.go @@ -6,6 +6,7 @@ import ( "crypto/ed25519" "crypto/rsa" "errors" + "strings" jose "github.com/go-jose/go-jose/v4" ) @@ -92,17 +93,17 @@ func FindMatchingKey(keyID, use, expectedAlg string, keys ...jose.JSONWebKey) (k } func algToKeyType(key any, alg string) bool { - switch alg[0] { - case 'R', 'P': + if strings.HasPrefix(alg, "RS") || strings.HasPrefix(alg, "PS") { _, ok := key.(*rsa.PublicKey) return ok - case 'E': + } + if strings.HasPrefix(alg, "ES") { _, ok := key.(*ecdsa.PublicKey) return ok - case 'O': - _, ok := key.(*ed25519.PublicKey) - return ok - default: - return false } + if alg == string(jose.EdDSA) { + _, ok := key.(ed25519.PublicKey) + return ok + } + return false } From 0df1caff1b501fdd04738de4d1ba0a7deccb0671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 20 Aug 2024 16:03:44 +0300 Subject: [PATCH 7/8] Revert "chore(example): add supported signing algorithms to RP" This reverts commit eb249c4c70340ebd7b0add8f129865721d070a2a. --- example/client/app/app.go | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/example/client/app/app.go b/example/client/app/app.go index a870e00..448c530 100644 --- a/example/client/app/app.go +++ b/example/client/app/app.go @@ -11,7 +11,6 @@ import ( "sync/atomic" "time" - "github.com/go-jose/go-jose/v4" "github.com/google/uuid" "github.com/sirupsen/logrus" @@ -54,23 +53,7 @@ func main() { options := []rp.Option{ rp.WithCookieHandler(cookieHandler), - rp.WithVerifierOpts( - rp.WithIssuedAtOffset(5*time.Second), - // When the OP uses other signing algorithms then RS256, - // We need to tell the RP to accept them. - // The actual handshake is done with the "kid" and "alg" header claims. - // However, [jose.ParseSigned] needs a list of algorithms we are willing to accept. - // This example sets all the algorithms the ZITADEL product supports. - rp.WithSupportedSigningAlgorithms( - string(jose.EdDSA), - string(jose.RS256), - string(jose.RS384), - string(jose.RS512), - string(jose.ES256), - string(jose.ES384), - string(jose.ES512), - ), - ), + rp.WithVerifierOpts(rp.WithIssuedAtOffset(5 * time.Second)), rp.WithHTTPClient(client), rp.WithLogger(logger), } From 07d4268b897f9c3880c3f1c737236719c6d27546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 20 Aug 2024 16:07:20 +0300 Subject: [PATCH 8/8] example: signing algs from discovery --- example/client/app/app.go | 1 + 1 file changed, 1 insertion(+) diff --git a/example/client/app/app.go b/example/client/app/app.go index 448c530..0b9b19d 100644 --- a/example/client/app/app.go +++ b/example/client/app/app.go @@ -56,6 +56,7 @@ func main() { rp.WithVerifierOpts(rp.WithIssuedAtOffset(5 * time.Second)), rp.WithHTTPClient(client), rp.WithLogger(logger), + rp.WithSigningAlgsFromDiscovery(), } if clientSecret == "" { options = append(options, rp.WithPKCE(cookieHandler))