fix(authn): make hashing/encryption keys used to secure cookies (#2536)

fix(authn): configurable hashing/encryption keys used to secure cookies

If they are not configured zot will generate a random hashing key at startup,
invalidating all cookies if zot is restarted. closes: #2526

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>

pkg/api/authn.go

@@ -22,7 +22,6 @@ import (
 	"github.com/google/go-github/v52/github"
 	"github.com/google/uuid"
 	"github.com/gorilla/mux"
-	"github.com/gorilla/securecookie"
 	"github.com/gorilla/sessions"
 	godigest "github.com/opencontainers/go-digest"
 	"github.com/zitadel/oidc/v3/pkg/client/rp"
@@ -334,10 +333,12 @@ func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun
 
 		for provider := range ctlr.Config.HTTP.Auth.OpenID.Providers {
 			if config.IsOpenIDSupported(provider) {
-				rp := NewRelyingPartyOIDC(context.TODO(), ctlr.Config, provider, ctlr.Log)
+				rp := NewRelyingPartyOIDC(context.TODO(), ctlr.Config, provider, ctlr.Config.HTTP.Auth.SessionHashKey,
+					ctlr.Config.HTTP.Auth.SessionEncryptKey, ctlr.Log)
 				ctlr.RelyingParties[provider] = rp
 			} else if config.IsOauth2Supported(provider) {
-				rp := NewRelyingPartyGithub(ctlr.Config, provider, ctlr.Log)
+				rp := NewRelyingPartyGithub(ctlr.Config, provider, ctlr.Config.HTTP.Auth.SessionHashKey,
+					ctlr.Config.HTTP.Auth.SessionEncryptKey, ctlr.Log)
 				ctlr.RelyingParties[provider] = rp
 			}
 		}
@@ -610,20 +611,25 @@ func (rh *RouteHandler) AuthURLHandler() http.HandlerFunc {
 	}
 }
 
-func NewRelyingPartyOIDC(ctx context.Context, config *config.Config, provider string, log log.Logger) rp.RelyingParty {
-	issuer, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config, provider, log)
+func NewRelyingPartyOIDC(ctx context.Context, config *config.Config, provider string,
+	hashKey, encryptKey []byte, log log.Logger,
+) rp.RelyingParty {
+	issuer, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config,
+		provider, hashKey, encryptKey, log)
 
 	relyingParty, err := rp.NewRelyingPartyOIDC(ctx, issuer, clientID, clientSecret, redirectURI, scopes, options...)
 	if err != nil {
 		log.Panic().Err(err).Str("issuer", issuer).Str("redirectURI", redirectURI).Strs("scopes", scopes).
-			Msg("failed to get new relying party oicd")
+			Msg("failed to initialize new relying party oidc")
 	}
 
 	return relyingParty
 }
 
-func NewRelyingPartyGithub(config *config.Config, provider string, log log.Logger) rp.RelyingParty {
-	_, clientID, clientSecret, redirectURI, scopes, options := getRelyingPartyArgs(config, provider, log)
+func NewRelyingPartyGithub(config *config.Config, provider string, hashKey, encryptKey []byte, log log.Logger,
+) rp.RelyingParty {
+	_, clientID, clientSecret, redirectURI, scopes,
+		options := getRelyingPartyArgs(config, provider, hashKey, encryptKey, log)
 
 	rpConfig := &oauth2.Config{
 		ClientID:     clientID,
@@ -636,13 +642,13 @@ func NewRelyingPartyGithub(config *config.Config, provider string, log log.Logge
 	relyingParty, err := rp.NewRelyingPartyOAuth(rpConfig, options...)
 	if err != nil {
 		log.Panic().Err(err).Str("redirectURI", redirectURI).Strs("scopes", scopes).
-			Msg("failed to get new relying party oauth")
+			Msg("failed to initialize new relying party oauth")
 	}
 
 	return relyingParty
 }
 
-func getRelyingPartyArgs(cfg *config.Config, provider string, log log.Logger) (
+func getRelyingPartyArgs(cfg *config.Config, provider string, hashKey, encryptKey []byte, log log.Logger) (
 	string, string, string, string, []string, []rp.Option,
 ) {
 	if _, ok := cfg.HTTP.Auth.OpenID.Providers[provider]; !ok {
@@ -683,9 +689,8 @@ func getRelyingPartyArgs(cfg *config.Config, provider string, log log.Logger) (
 		rp.WithVerifierOpts(rp.WithIssuedAtOffset(issuedAtOffset)),
 	}
 
-	key := securecookie.GenerateRandomKey(32) //nolint:mnd
+	cookieHandler := httphelper.NewCookieHandler(hashKey, encryptKey, httphelper.WithMaxAge(relyingPartyCookieMaxAge))
 
-	cookieHandler := httphelper.NewCookieHandler(key, key, httphelper.WithMaxAge(relyingPartyCookieMaxAge))
 	options = append(options, rp.WithCookieHandler(cookieHandler))
 
 	if clientSecret == "" {
@@ -839,14 +844,14 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st
 	stateOrigin, ok := stateCookie.Values["state"].(string)
 	if !ok {
 		ctlr.Log.Error().Err(zerr.ErrInvalidStateCookie).Str("component", "openID").
-			Msg(": failed to get 'state' cookie from request")
+			Msg("failed to get 'state' cookie from request")
 
 		return "", zerr.ErrInvalidStateCookie
 	}
 
 	if stateOrigin != state {
 		ctlr.Log.Error().Err(zerr.ErrInvalidStateCookie).Str("component", "openID").
-			Msg(": 'state' cookie differs from the actual one")
+			Msg("'state' cookie differs from the actual one")
 
 		return "", zerr.ErrInvalidStateCookie
 	}

pkg/api/authn_test.go

@@ -960,7 +960,7 @@ func TestCookiestoreCleanup(t *testing.T) {
 			DefaultStore: imgStore,
 		}
 
-		cookieStore, err := api.NewCookieStore(storeController)
+		cookieStore, err := api.NewCookieStore(storeController, nil, nil)
 		So(err, ShouldBeNil)
 
 		cookieStore.RunSessionCleaner(taskScheduler)
@@ -995,7 +995,7 @@ func TestCookiestoreCleanup(t *testing.T) {
 			DefaultStore: imgStore,
 		}
 
-		cookieStore, err := api.NewCookieStore(storeController)
+		cookieStore, err := api.NewCookieStore(storeController, []byte("secret"), nil)
 		So(err, ShouldBeNil)
 
 		err = os.Chmod(rootDir, 0o000)

pkg/api/config/config.go

@@ -67,12 +67,15 @@ type AuthHTPasswd struct {
 }
 
 type AuthConfig struct {
-	FailDelay int
-	HTPasswd  AuthHTPasswd
-	LDAP      *LDAPConfig
-	Bearer    *BearerConfig
-	OpenID    *OpenIDConfig
-	APIKey    bool
+	FailDelay         int
+	HTPasswd          AuthHTPasswd
+	LDAP              *LDAPConfig
+	Bearer            *BearerConfig
+	OpenID            *OpenIDConfig
+	APIKey            bool
+	SessionKeysFile   string
+	SessionHashKey    []byte `json:"-"`
+	SessionEncryptKey []byte `json:"-"`
 }
 
 type BearerConfig struct {
@@ -81,6 +84,11 @@ type BearerConfig struct {
 	Cert    string
 }
 
+type SessionKeys struct {
+	HashKey    string
+	EncryptKey string `mapstructure:",omitempty"`
+}
+
 type OpenIDConfig struct {
 	Providers map[string]OpenIDProviderConfig
 }

pkg/api/controller.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/gorilla/mux"
+	"github.com/gorilla/securecookie"
 	"github.com/zitadel/oidc/v3/pkg/client/rp"
 
 	"zotregistry.dev/zot/errors"
@@ -308,7 +309,14 @@ func (c *Controller) InitImageStore() error {
 func (c *Controller) initCookieStore() error {
 	// setup sessions cookie store used to preserve logged in user in web sessions
 	if c.Config.IsBasicAuthnEnabled() {
-		cookieStore, err := NewCookieStore(c.StoreController)
+		if c.Config.HTTP.Auth.SessionHashKey == nil {
+			c.Log.Warn().Msg("hashKey is not set in config, generating a random one")
+
+			c.Config.HTTP.Auth.SessionHashKey = securecookie.GenerateRandomKey(64) //nolint: gomnd
+		}
+
+		cookieStore, err := NewCookieStore(c.StoreController, c.Config.HTTP.Auth.SessionHashKey,
+			c.Config.HTTP.Auth.SessionEncryptKey)
 		if err != nil {
 			return err
 		}

pkg/api/controller_test.go

@@ -2272,7 +2272,7 @@ func TestAuthnErrors(t *testing.T) {
 		}
 
 		So(func() {
-			api.NewRelyingPartyGithub(conf, "prov", log.NewLogger("debug", ""))
+			api.NewRelyingPartyGithub(conf, "prov", nil, nil, log.NewLogger("debug", ""))
 		}, ShouldPanic)
 
 		err = os.Chmod(tmpFile, 0o644)
@@ -4099,7 +4099,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
 		}
 
 		Convey("provider not found in config", func() {
-			So(func() { _ = api.NewRelyingPartyOIDC(ctx, conf, "notDex", log.NewLogger("debug", "")) }, ShouldPanic)
+			So(func() { _ = api.NewRelyingPartyOIDC(ctx, conf, "notDex", nil, nil, log.NewLogger("debug", "")) }, ShouldPanic)
 		})
 
 		Convey("key path not found on disk", func() {
@@ -4107,7 +4107,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
 			oidcProviderCfg.KeyPath = "path/to/file"
 			conf.HTTP.Auth.OpenID.Providers["oidc"] = oidcProviderCfg
 
-			So(func() { _ = api.NewRelyingPartyOIDC(ctx, conf, "oidc", log.NewLogger("debug", "")) }, ShouldPanic)
+			So(func() { _ = api.NewRelyingPartyOIDC(ctx, conf, "oidc", nil, nil, log.NewLogger("debug", "")) }, ShouldPanic)
 		})
 
 		Convey("https callback", func() {
@@ -4116,7 +4116,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
 				Key:  ServerKey,
 			}
 
-			rp := api.NewRelyingPartyOIDC(ctx, conf, "oidc", log.NewLogger("debug", ""))
+			rp := api.NewRelyingPartyOIDC(ctx, conf, "oidc", nil, nil, log.NewLogger("debug", ""))
 			So(rp, ShouldNotBeNil)
 		})
 
@@ -4125,7 +4125,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
 			oidcProvider.ClientSecret = ""
 			conf.HTTP.Auth.OpenID.Providers["oidc"] = oidcProvider
 
-			rp := api.NewRelyingPartyOIDC(ctx, conf, "oidc", log.NewLogger("debug", ""))
+			rp := api.NewRelyingPartyOIDC(ctx, conf, "oidc", nil, nil, log.NewLogger("debug", ""))
 			So(rp, ShouldNotBeNil)
 		})
 
@@ -4134,7 +4134,7 @@ func TestNewRelyingPartyOIDC(t *testing.T) {
 			oidcProvider.Issuer = ""
 			conf.HTTP.Auth.OpenID.Providers["oidc"] = oidcProvider
 
-			So(func() { _ = api.NewRelyingPartyOIDC(ctx, conf, "oidc", log.NewLogger("debug", "")) }, ShouldPanic)
+			So(func() { _ = api.NewRelyingPartyOIDC(ctx, conf, "oidc", nil, nil, log.NewLogger("debug", "")) }, ShouldPanic)
 		})
 	})
 }
@@ -4148,9 +4148,10 @@ func TestOpenIDMiddleware(t *testing.T) {
 	conf.HTTP.Port = port
 
 	testCases := []struct {
-		testCaseName string
-		address      string
-		externalURL  string
+		testCaseName   string
+		address        string
+		externalURL    string
+		useSessionKeys bool
 	}{
 		{
 			address:      "0.0.0.0",
@@ -4162,6 +4163,12 @@ func TestOpenIDMiddleware(t *testing.T) {
 			externalURL:  "",
 			testCaseName: "without ExternalURL provided in config",
 		},
+		{
+			address:        "127.0.0.1",
+			externalURL:    "",
+			testCaseName:   "without ExternalURL provided in config and session keys for cookies",
+			useSessionKeys: true,
+		},
 	}
 
 	// need a username different than ldap one, to test both logic
@@ -4246,6 +4253,11 @@ func TestOpenIDMiddleware(t *testing.T) {
 
 	for _, testcase := range testCases {
 		t.Run(testcase.testCaseName, func(t *testing.T) {
+			if testcase.useSessionKeys {
+				ctlr.Config.HTTP.Auth.SessionHashKey = []byte("3lrioGLGO2RfG9Y7HQGgWa3ayBjMLw2auMXqEWcSXjQKc9SoQ3fKTIbO+toPYa7e")
+				ctlr.Config.HTTP.Auth.SessionEncryptKey = []byte("KOzt01JrDz2uC//UBC5ZikxQw4owfmI8")
+			}
+
 			Convey("make controller", t, func() {
 				dir := t.TempDir()
 

pkg/api/cookiestore.go

@@ -11,10 +11,8 @@ import (
 	"strings"
 	"time"
 
-	"github.com/gorilla/securecookie"
 	"github.com/gorilla/sessions"
 
-	zerr "zotregistry.dev/zot/errors"
 	"zotregistry.dev/zot/pkg/scheduler"
 	"zotregistry.dev/zot/pkg/storage"
 	storageConstants "zotregistry.dev/zot/pkg/storage/constants"
@@ -38,16 +36,11 @@ func (c *CookieStore) RunSessionCleaner(sch *scheduler.Scheduler) {
 	}
 }
 
-func NewCookieStore(storeController storage.StoreController) (*CookieStore, error) {
+func NewCookieStore(storeController storage.StoreController, hashKey, encryptKey []byte) (*CookieStore, error) {
 	// To store custom types in our cookies
 	// we must first register them using gob.Register
 	gob.Register(map[string]interface{}{})
 
-	hashKey, err := getHashKey()
-	if err != nil {
-		return &CookieStore{}, err
-	}
-
 	var store sessions.Store
 
 	var sessionsDir string
@@ -60,14 +53,14 @@ func NewCookieStore(storeController storage.StoreController) (*CookieStore, erro
 			return &CookieStore{}, err
 		}
 
-		localStore := sessions.NewFilesystemStore(sessionsDir, hashKey)
+		localStore := sessions.NewFilesystemStore(sessionsDir, hashKey, encryptKey)
 
 		localStore.MaxAge(cookiesMaxAge)
 
 		store = localStore
 		needsCleanup = true
 	} else {
-		memStore := sessions.NewCookieStore(hashKey)
+		memStore := sessions.NewCookieStore(hashKey, encryptKey)
 
 		memStore.MaxAge(cookiesMaxAge)
 
@@ -81,15 +74,6 @@ func NewCookieStore(storeController storage.StoreController) (*CookieStore, erro
 	}, nil
 }
 
-func getHashKey() ([]byte, error) {
-	hashKey := securecookie.GenerateRandomKey(64)
-	if hashKey == nil {
-		return nil, zerr.ErrHashKeyNotCreated
-	}
-
-	return hashKey, nil
-}
-
 func IsExpiredSession(dirEntry fs.DirEntry) bool {
 	fileInfo, err := dirEntry.Info()
 	if err != nil { // may have been deleted in the meantime