From c0f89011973ee980d33184806bc2688d6f815934 Mon Sep 17 00:00:00 2001 From: "Adam T. Williams" Date: Mon, 28 Oct 2024 14:48:13 -0600 Subject: [PATCH 1/4] refactor(jwk): aquire read lock unless generating --- jwk/helper.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/jwk/helper.go b/jwk/helper.go index 50f3a28b2d2..600edebd4f0 100644 --- a/jwk/helper.go +++ b/jwk/helper.go @@ -44,12 +44,16 @@ func EnsureAsymmetricKeypairExists(ctx context.Context, r InternalRegistry, alg, } func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, kid, alg string) (private *jose.JSONWebKey, err error) { - getLock(set).Lock() - defer getLock(set).Unlock() - + getLock(set).RLock() keys, err := m.GetKeySet(ctx, set) + getLock(set).RUnlock() + if errors.Is(err, x.ErrNotFound) || keys != nil && len(keys.Keys) == 0 { r.Logger().Warnf("JSON Web Key Set \"%s\" does not exist yet, generating new key pair...", set) + + getLock(set).Lock() + defer getLock(set).Unlock() + keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { return nil, err @@ -64,6 +68,9 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, } else { r.Logger().WithField("jwks", set).Warnf("JSON Web Key not found in JSON Web Key Set %s, generating new key pair...", set) + getLock(set).Lock() + defer getLock(set).Unlock() + keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { return nil, err From d1101c7981c45a905fd81e81365b1378be6548bb Mon Sep 17 00:00:00 2001 From: "Adam T. Williams" Date: Mon, 28 Oct 2024 15:59:41 -0600 Subject: [PATCH 2/4] rm lock --- jwk/helper.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/jwk/helper.go b/jwk/helper.go index 600edebd4f0..465abb1ab3d 100644 --- a/jwk/helper.go +++ b/jwk/helper.go @@ -12,7 +12,6 @@ import ( "crypto/x509" "encoding/json" "encoding/pem" - "sync" hydra "github.com/ory/hydra-client-go/v2" @@ -26,34 +25,16 @@ import ( "github.com/pkg/errors" ) -var mapLock sync.RWMutex -var locks = map[string]*sync.RWMutex{} - -func getLock(set string) *sync.RWMutex { - mapLock.Lock() - defer mapLock.Unlock() - if _, ok := locks[set]; !ok { - locks[set] = new(sync.RWMutex) - } - return locks[set] -} - func EnsureAsymmetricKeypairExists(ctx context.Context, r InternalRegistry, alg, set string) error { _, err := GetOrGenerateKeys(ctx, r, r.KeyManager(), set, set, alg) return err } func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, kid, alg string) (private *jose.JSONWebKey, err error) { - getLock(set).RLock() keys, err := m.GetKeySet(ctx, set) - getLock(set).RUnlock() if errors.Is(err, x.ErrNotFound) || keys != nil && len(keys.Keys) == 0 { r.Logger().Warnf("JSON Web Key Set \"%s\" does not exist yet, generating new key pair...", set) - - getLock(set).Lock() - defer getLock(set).Unlock() - keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { return nil, err @@ -67,10 +48,6 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, return privKey, nil } else { r.Logger().WithField("jwks", set).Warnf("JSON Web Key not found in JSON Web Key Set %s, generating new key pair...", set) - - getLock(set).Lock() - defer getLock(set).Unlock() - keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { return nil, err From a353c382ceafd620cb074e72d8c1e50e0c5f92fd Mon Sep 17 00:00:00 2001 From: "Adam T. Williams" Date: Mon, 28 Oct 2024 16:08:39 -0600 Subject: [PATCH 3/4] lock for persisting --- jwk/helper.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/jwk/helper.go b/jwk/helper.go index 465abb1ab3d..8d7ae9bf8cc 100644 --- a/jwk/helper.go +++ b/jwk/helper.go @@ -12,6 +12,7 @@ import ( "crypto/x509" "encoding/json" "encoding/pem" + "sync" hydra "github.com/ory/hydra-client-go/v2" @@ -25,6 +26,15 @@ import ( "github.com/pkg/errors" ) +var locks = map[string]*sync.Mutex{} + +func getLock(set string) *sync.Mutex { + if _, ok := locks[set]; !ok { + locks[set] = new(sync.Mutex) + } + return locks[set] +} + func EnsureAsymmetricKeypairExists(ctx context.Context, r InternalRegistry, alg, set string) error { _, err := GetOrGenerateKeys(ctx, r, r.KeyManager(), set, set, alg) return err @@ -35,6 +45,10 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, if errors.Is(err, x.ErrNotFound) || keys != nil && len(keys.Keys) == 0 { r.Logger().Warnf("JSON Web Key Set \"%s\" does not exist yet, generating new key pair...", set) + + getLock(set).Lock() + defer getLock(set).Unlock() + keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { return nil, err @@ -48,6 +62,10 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, return privKey, nil } else { r.Logger().WithField("jwks", set).Warnf("JSON Web Key not found in JSON Web Key Set %s, generating new key pair...", set) + + getLock(set).Lock() + defer getLock(set).Unlock() + keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { return nil, err From ee7a2ced4c033eb1bd0848e76aba407bda196b1e Mon Sep 17 00:00:00 2001 From: "Adam T. Williams" Date: Mon, 28 Oct 2024 18:15:52 -0600 Subject: [PATCH 4/4] restore map lock but acquire once --- jwk/helper.go | 11 +++++++---- .../TestHandlerWellKnown-hsm_enabled=false.json | 3 ++- .../TestHandlerWellKnown-hsm_enabled=true.json | 3 ++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/jwk/helper.go b/jwk/helper.go index 8d7ae9bf8cc..32574754925 100644 --- a/jwk/helper.go +++ b/jwk/helper.go @@ -26,9 +26,12 @@ import ( "github.com/pkg/errors" ) +var mapLock sync.Mutex var locks = map[string]*sync.Mutex{} func getLock(set string) *sync.Mutex { + mapLock.Lock() + defer mapLock.Unlock() if _, ok := locks[set]; !ok { locks[set] = new(sync.Mutex) } @@ -46,8 +49,8 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, if errors.Is(err, x.ErrNotFound) || keys != nil && len(keys.Keys) == 0 { r.Logger().Warnf("JSON Web Key Set \"%s\" does not exist yet, generating new key pair...", set) - getLock(set).Lock() - defer getLock(set).Unlock() + l := getLock(set) + defer l.Lock() keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { @@ -63,8 +66,8 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, } else { r.Logger().WithField("jwks", set).Warnf("JSON Web Key not found in JSON Web Key Set %s, generating new key pair...", set) - getLock(set).Lock() - defer getLock(set).Unlock() + l := getLock(set) + defer l.Lock() keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig") if err != nil { diff --git a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json index 215fa018214..5bc92ec79a5 100644 --- a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json +++ b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json @@ -63,7 +63,8 @@ "require_request_uri_registration": true, "response_modes_supported": [ "query", - "fragment" + "fragment", + "form_post" ], "response_types_supported": [ "code", diff --git a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json index 215fa018214..5bc92ec79a5 100644 --- a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json +++ b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json @@ -63,7 +63,8 @@ "require_request_uri_registration": true, "response_modes_supported": [ "query", - "fragment" + "fragment", + "form_post" ], "response_types_supported": [ "code",