jwk: Forces JWK to have a unique ID

Previously, JSON Web Keys did not have to specify a unique id. JWKs
generated by ORY Hydra typically only used `public` or `private`
as KeyID. This patch changes that and appends a unique id if no
KeyID was given. To be able to separate between public and private key
pairs in resource name, the public/private convention was kept.

This change targets specifically the OpenID Connect ID Token and HTTP
TLS keys. The ID Token key was previously "hydra.openid.id-token:public"
and "hydra.openid.id-token:private" which now changed to something like
"hydra.openid.id-token:public:9a458aa3-65a0-4982-835f-343eec45183c" and
"hydra.openid.id-token:private:fa353995-d77d-420a-b967-63bf0721271b"
with the UUID part being random for every installation.

This change will help greatly with key rotation in the future.

Closes #589

UPGRADE.md

@@ -54,6 +54,38 @@ before finalizing the upgrade process.
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
+## 1.0.0-prerelease
+
+This section summarizes important changes introduced in 1.0.0.
+
+### Minor breaking changes
+
+Minor breaking changes do not require any special upgrade paths, unless you explicitly rely on those features in some way.
+
+#### jwk: Forces JWK to have a unique ID
+
+Previously, JSON Web Keys did not have to specify a unique id. JWKs
+generated by ORY Hydra typically only used `public` or `private`
+as KeyID. This patch changes that and appends a unique id if no
+KeyID was given. To be able to separate between public and private key
+pairs in resource name, the public/private convention was kept.
+
+This change targets specifically the OpenID Connect ID Token and HTTP
+TLS keys. The ID Token key was previously "hydra.openid.id-token:public"
+and "hydra.openid.id-token:private" which now changed to something like
+"hydra.openid.id-token:public:9a458aa3-65a0-4982-835f-343eec45183c" and
+"hydra.openid.id-token:private:fa353995-d77d-420a-b967-63bf0721271b"
+with the UUID part being random for every installation.
+
+This change will help greatly with key rotation in the future.
+
+If you rely on these keys in your applications and if they are hardcoded in some way, you may want to use the `./well-known/openid-configuration`
+or `./well-known/jwks.json` endpoints instead. Libraries, which handle these standards appropriately, exist for almost any
+programming language.
+
+These keys will be generated automatically if they do not exist yet in the database. No further steps for upgrading are
+required.
+
 ## 0.11.3
 
 The experimental endpoint `/health/metrics` has been removed as it caused various issues such as increased memory usage,

cmd/server/handler.go

@@ -162,7 +162,7 @@ func (h *Handler) registerRoutes(router *httprouter.Router) {
 	injectConsentManager(c)
 	clientsManager := newClientManager(c)
 	injectFositeStore(c, clientsManager)
-	oauth2Provider := newOAuth2Provider(c, ctx.KeyManager)
+	oauth2Provider, idTokenKeyID := newOAuth2Provider(c)
 
 	// set up warden
 	ctx.Warden = &warden.LocalWarden{
@@ -181,7 +181,7 @@ func (h *Handler) registerRoutes(router *httprouter.Router) {
 	h.Keys = newJWKHandler(c, router)
 	h.Policy = newPolicyHandler(c, router)
 	h.Consent = newConsentHanlder(c, router)
-	h.OAuth2 = newOAuth2Handler(c, router, ctx.ConsentManager, oauth2Provider)
+	h.OAuth2 = newOAuth2Handler(c, router, ctx.ConsentManager, oauth2Provider, idTokenKeyID)
 	h.Warden = warden.NewHandler(c, router)
 	h.Groups = &group.Handler{
 		H:              herodot.NewJSONWriter(c.GetLogger()),

cmd/server/handler_oauth2_factory.go

@@ -18,8 +18,6 @@ import (
 	"fmt"
 	"net/url"
 
-	"os"
-
 	"github.com/gorilla/sessions"
 	"github.com/julienschmidt/httprouter"
 	"github.com/ory/fosite"
@@ -31,7 +29,6 @@ import (
 	"github.com/ory/hydra/oauth2"
 	"github.com/ory/hydra/pkg"
 	"github.com/ory/hydra/warden"
-	"github.com/pkg/errors"
 )
 
 func injectFositeStore(c *config.Config, clients client.Manager) {
@@ -68,26 +65,20 @@ func injectFositeStore(c *config.Config, clients client.Manager) {
 	ctx.FositeStore = store
 }
 
-func newOAuth2Provider(c *config.Config, km jwk.Manager) fosite.OAuth2Provider {
+func newOAuth2Provider(c *config.Config) (fosite.OAuth2Provider, string) {
 	var ctx = c.Context()
 	var store = ctx.FositeStore
 
-	createRS256KeysIfNotExist(c, oauth2.OpenIDConnectKeyName, "private", "sig")
-	keys, err := km.GetKey(oauth2.OpenIDConnectKeyName, "private")
-	if errors.Cause(err) == pkg.ErrNotFound {
-		c.GetLogger().Warnln("Could not find OpenID Connect signing keys. Generating a new keypair...")
-		keys, err = new(jwk.RS256Generator).Generate("")
+	privateKey, err := createOrGetJWK(c, oauth2.OpenIDConnectKeyName, "private")
+	if err != nil {
+		c.GetLogger().WithError(err).Fatalf(`Could not fetch private signing key for OpenID Connect - did you forget to run "hydra migrate sql" or forget to set the SYSTEM_SECRET?`)
+	}
 
-		pkg.Must(err, "Could not generate signing key for OpenID Connect")
-		km.AddKeySet(oauth2.OpenIDConnectKeyName, keys)
-		c.GetLogger().Infoln("Keypair generated.")
-		c.GetLogger().Warnln("WARNING: Automated key creation causes low entropy. Replace the keys as soon as possible.")
-	} else if err != nil {
-		fmt.Fprintf(os.Stderr, `Could not fetch signing key for OpenID Connect - did you forget to run "hydra migrate sql" or forget to set the SYSTEM_SECRET? Got error: %s`+"\n", err.Error())
-		os.Exit(1)
+	publicKey, err := createOrGetJWK(c, oauth2.OpenIDConnectKeyName, "public")
+	if err != nil {
+		c.GetLogger().WithError(err).Fatalf(`Could not fetch public signing key for OpenID Connect - did you forget to run "hydra migrate sql" or forget to set the SYSTEM_SECRET?`)
 	}
 
-	rsaKey := jwk.MustRSAPrivate(jwk.First(keys.Keys))
 	fc := &compose.Config{
 		AccessTokenLifespan:        c.GetAccessTokenLifespan(),
 		AuthorizeCodeLifespan:      c.GetAuthCodeLifespan(),
@@ -96,12 +87,13 @@ func newOAuth2Provider(c *config.Config, km jwk.Manager) fosite.OAuth2Provider {
 		ScopeStrategy:              c.GetScopeStrategy(),
 		SendDebugMessagesToClients: c.SendOAuth2DebugMessagesToClients,
 	}
+
 	return compose.Compose(
 		fc,
 		store,
 		&compose.CommonStrategy{
 			CoreStrategy:               compose.NewOAuth2HMACStrategy(fc, c.GetSystemSecret()),
-			OpenIDConnectTokenStrategy: compose.NewOpenIDConnectStrategy(rsaKey),
+			OpenIDConnectTokenStrategy: compose.NewOpenIDConnectStrategy(jwk.MustRSAPrivate(privateKey)),
 		},
 		nil,
 		compose.OAuth2AuthorizeExplicitFactory,
@@ -113,10 +105,10 @@ func newOAuth2Provider(c *config.Config, km jwk.Manager) fosite.OAuth2Provider {
 		compose.OpenIDConnectImplicitFactory,
 		compose.OAuth2TokenRevocationFactory,
 		warden.OAuth2TokenIntrospectionFactory,
-	)
+	), publicKey.KeyID
 }
 
-func newOAuth2Handler(c *config.Config, router *httprouter.Router, cm oauth2.ConsentRequestManager, o fosite.OAuth2Provider) *oauth2.Handler {
+func newOAuth2Handler(c *config.Config, router *httprouter.Router, cm oauth2.ConsentRequestManager, o fosite.OAuth2Provider, idTokenKeyID string) *oauth2.Handler {
 	if c.ConsentURL == "" {
 		proto := "https"
 		if c.ForceHTTP {
@@ -144,6 +136,7 @@ func newOAuth2Handler(c *config.Config, router *httprouter.Router, cm oauth2.Con
 			ConsentManager:           c.Context().ConsentManager,
 			DefaultChallengeLifespan: c.GetChallengeTokenLifespan(),
 			DefaultIDTokenLifespan:   c.GetIDTokenLifespan(),
+			KeyID: idTokenKeyID,
 		},
 		ConsentURL:          *consentURL,
 		H:                   herodot.NewJSONWriter(c.GetLogger()),

cmd/server/helper_cert.go

@@ -30,7 +30,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
-	"github.com/square/go-jose"
 )
 
 const (
@@ -86,42 +85,36 @@ func getOrCreateTLSCertificate(cmd *cobra.Command, c *config.Config) tls.Certifi
 	}
 
 	ctx := c.Context()
-	keys, err := ctx.KeyManager.GetKey(tlsKeyName, "private")
-	if errors.Cause(err) == pkg.ErrNotFound {
-		c.GetLogger().Warn("No TLS Key / Certificate for HTTPS found. Generating self-signed certificate.")
-
-		keys, err = new(jwk.ECDSA256Generator).Generate("")
-		pkg.Must(err, "Could not generate key: %s", err)
-
-		cert, err := createSelfSignedCertificate(jwk.First(keys.Key("private")).Key)
-		pkg.Must(err, "Could not create X509 PEM Key Pair: %s", err)
-
-		private := jwk.First(keys.Key("private"))
-		private.Certificates = []*x509.Certificate{cert}
-		keys = &jose.JSONWebKeySet{
-			Keys: []jose.JSONWebKey{
-				*private,
-				*jwk.First(keys.Key("public")),
-			},
+	privateKey, err := createOrGetJWK(c, tlsKeyName, "private")
+	if err != nil {
+		c.GetLogger().WithError(err).Fatalf(`Could not fetch TLS keys - did you forget to run "hydra migrate sql" or forget to set the SYSTEM_SECRET?`)
+	}
+
+	if len(privateKey.Certificates) == 0 {
+		cert, err := createSelfSignedCertificate(privateKey.Key)
+		if err != nil {
+			c.GetLogger().WithError(err).Fatalf(`Could not generate a self signed TLS certificate.`)
 		}
 
-		err = ctx.KeyManager.AddKeySet(tlsKeyName, keys)
-		pkg.Must(err, "Could not persist key: %s", err)
-	} else {
-		pkg.Must(err, "Could not retrieve key: %s", err)
+		privateKey.Certificates = []*x509.Certificate{cert}
+		if err := ctx.KeyManager.DeleteKey(tlsKeyName, privateKey.KeyID); err != nil {
+			c.GetLogger().WithError(err).Fatalf(`Could not update (delete) the self signed TLS certificate.`)
+		}
+		if err := ctx.KeyManager.AddKey(tlsKeyName, privateKey); err != nil {
+			c.GetLogger().WithError(err).Fatalf(`Could not update (add) the self signed TLS certificate.`)
+		}
 	}
 
-	private := jwk.First(keys.Key("private"))
-	block, err := jwk.PEMBlockForKey(private.Key)
+	block, err := jwk.PEMBlockForKey(privateKey.Key)
 	if err != nil {
 		pkg.Must(err, "Could not encode key to PEM: %s", err)
 	}
 
-	if len(private.Certificates) == 0 {
+	if len(privateKey.Certificates) == 0 {
 		c.GetLogger().Fatal("TLS certificate chain can not be empty")
 	}
 
-	pemCert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: private.Certificates[0].Raw})
+	pemCert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: privateKey.Certificates[0].Raw})
 	pemKey := pem.EncodeToMemory(block)
 	cert, err := tls.X509KeyPair(pemCert, pemKey)
 	pkg.Must(err, "Could not decode certificate: %s", err)

cmd/server/helper_keys.go

@@ -22,25 +22,54 @@ import (
 	"github.com/ory/hydra/jwk"
 	"github.com/ory/hydra/pkg"
 	"github.com/pkg/errors"
+	"github.com/square/go-jose"
 )
 
-func createRS256KeysIfNotExist(c *config.Config, set, kid, use string) {
+func createOrGetJWK(c *config.Config, set string, prefix string) (key *jose.JSONWebKey, err error) {
 	ctx := c.Context()
-	generator := jwk.RS256Generator{}
 
-	if _, err := ctx.KeyManager.GetKey(set, kid); errors.Cause(err) == pkg.ErrNotFound {
-		c.GetLogger().Infof("Key pair for signing %s is missing. Creating new one.", set)
+	keys, err := ctx.KeyManager.GetKeySet(set)
+	if errors.Cause(err) == pkg.ErrNotFound || len(keys.Keys) == 0 {
+		c.GetLogger().Infof("JSON Web Key Set %s does not exist yet, generating new key pair...", set)
+		keys, err = createJWKS(ctx, set)
+		if err != nil {
+			return nil, err
+		}
+	} else if err != nil {
+		return nil, err
+	}
 
-		keys, err := generator.Generate("")
-		pkg.Must(err, "Could not generate %s key: %s", set, err)
+	key, err = jwk.FindKeyByPrefix(keys, prefix)
+	if err != nil {
+		c.GetLogger().Infof("JSON Web Key with prefix %s not found in JSON Web Key Set %s, generating new key pair...", prefix, set)
 
-		for i, k := range keys.Keys {
-			k.Use = use
-			keys.Keys[i] = k
+		keys, err = createJWKS(ctx, set)
+		if err != nil {
+			return nil, err
+		}
+
+		key, err = jwk.FindKeyByPrefix(keys, prefix)
+		if err != nil {
+			return nil, err
 		}
-		err = ctx.KeyManager.AddKeySet(set, keys)
-		pkg.Must(err, "Could not persist %s key: %s", set, err)
 	}
+
+	return key, nil
+}
+
+func createJWKS(ctx *config.Context, set string) (*jose.JSONWebKeySet, error) {
+	generator := jwk.RS256Generator{}
+	keys, err := generator.Generate("")
+	if err != nil {
+		return nil, errors.Wrapf(err, "Could not generate %s key", set)
+	}
+
+	err = ctx.KeyManager.AddKeySet(set, keys)
+	if err != nil {
+		return nil, errors.Wrapf(err, "Could not persist %s key", set)
+	}
+
+	return keys, nil
 }
 
 func publicKey(key interface{}) interface{} {

jwk/generator_hs256.go

@@ -19,6 +19,7 @@ import (
 	"crypto/x509"
 	"io"
 
+	"github.com/pborman/uuid"
 	"github.com/pkg/errors"
 	"github.com/square/go-jose"
 )
@@ -33,6 +34,10 @@ func (g *HS256Generator) Generate(id string) (*jose.JSONWebKeySet, error) {
 		return nil, errors.WithStack(err)
 	}
 
+	if id == "" {
+		id = uuid.New()
+	}
+
 	var sliceKey = key[:]
 
 	return &jose.JSONWebKeySet{

jwk/generator_hs512.go

@@ -19,6 +19,7 @@ import (
 	"crypto/x509"
 	"io"
 
+	"github.com/pborman/uuid"
 	"github.com/pkg/errors"
 	"github.com/square/go-jose"
 )
@@ -33,6 +34,10 @@ func (g *HS512Generator) Generate(id string) (*jose.JSONWebKeySet, error) {
 		return nil, errors.WithStack(err)
 	}
 
+	if id == "" {
+		id = uuid.New()
+	}
+
 	var sliceKey = key[:]
 
 	return &jose.JSONWebKeySet{

jwk/generator_rs256.go

@@ -18,7 +18,6 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
-	"fmt"
 
 	"github.com/pkg/errors"
 	"github.com/square/go-jose"
@@ -59,10 +58,3 @@ func (g *RS256Generator) Generate(id string) (*jose.JSONWebKeySet, error) {
 		},
 	}, nil
 }
-
-func ider(typ, id string) string {
-	if id != "" {
-		return fmt.Sprintf("%s:%s", typ, id)
-	}
-	return typ
-}

jwk/handler.go

@@ -128,28 +128,49 @@ type joseWebKeySetRequest struct {
 //       500: genericError
 func (h *Handler) WellKnown(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	var ctx = context.Background()
-	if _, err := h.W.TokenAllowed(ctx, h.W.TokenFromRequest(r), &firewall.TokenAccessRequest{
-		Resource: h.PrefixResource("keys:" + IDTokenKeyName + ":public"),
-		Action:   "get",
-	}, "hydra.keys.get"); err != nil {
-		if err := h.W.IsAllowed(ctx, &firewall.AccessRequest{
-			Subject:  "",
-			Resource: h.PrefixResource("keys:" + IDTokenKeyName + ":public"),
+
+	var fw = func(id string) error {
+		if _, err := h.W.TokenAllowed(ctx, h.W.TokenFromRequest(r), &firewall.TokenAccessRequest{
+			Resource: h.PrefixResource("keys:" + IDTokenKeyName + ":" + id),
 			Action:   "get",
-		}); err != nil {
-			h.H.WriteError(w, r, err)
+		}, "hydra.keys.get"); err != nil {
+			if err := h.W.IsAllowed(ctx, &firewall.AccessRequest{
+				Subject:  "",
+				Resource: h.PrefixResource("keys:" + IDTokenKeyName + ":" + id),
+				Action:   "get",
+			}); err != nil {
+				h.H.WriteError(w, r, err)
+				return err
+			} else {
+				// Allow unauthorized requests to access this resource if it is enabled by policies
+				return nil
+			}
+		}
+		return nil
+	}
+
+	keys, err := h.Manager.GetKeySet(IDTokenKeyName)
+	if err != nil {
+		if err := fw("public:"); err != nil {
 			return
-		} else {
-			// Allow unauthorized requests to access this resource if it is enabled by policies
 		}
+
+		h.H.WriteError(w, r, err)
+		return
 	}
 
-	keys, err := h.Manager.GetKey(IDTokenKeyName, "public")
+	keys, err = FindKeysByPrefix(keys, "public")
 	if err != nil {
 		h.H.WriteError(w, r, err)
 		return
 	}
 
+	for _, key := range keys.Keys {
+		if err := fw(key.KeyID); err != nil {
+			return
+		}
+	}
+
 	h.H.Write(w, r, keys)
 }