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

[eusebiu-constantin-petu-dbk]

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

What type of PR is this?
bug

Which issue does this PR fix:
restarting zot invalidates currently active sessions.

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:
#2526

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?
no

Does this PR introduce any user-facing change?:

modified configuration file, client can specify hashing and encrypt keys to secure web session cookies.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[AndersBennedsgaard]

I tried to build and deploy Zot with these changes, and I still get the error explained in #2526 with these changes, however, it only happens when I run multiple replicas of Zot. I tried to enable some extra logging by changing

https://github.com/peusebiu/zot/blob/c6688abf0bf7edc59db02fca3bea8994b0383dba/pkg/api/authn.go#L851

to

stateCookie, err := ctlr.CookieStore.Get(r, "statecookie")                                                          
if err != nil {                                                                                                     
  ctlr.Log.Error().Err(err).Str("component", "openID").Msg("failed to get 'statecookie' from request")            
}

and I started to get

{"level":"error","error":"securecookie: the value is not valid","component":"openID","goroutine":296,"caller":"zotregistry.dev/zot/pkg/api/authn.go:856","time":"2024-07-12T09:34:55.128562134Z","message":"failed to get 'statecookie' from request"}

I guess that Zot can't handle cookies from different running instances, as it saves some things in memory which the other instance does not have access to?

[eusebiu-constantin-petu-dbk]

Hello @AndersBennedsgaard I have pushed a new commit, can you try your test again please?

thank you!

[rchincha]

@peusebiu pls fix the linter issues and rebase also.

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 9 lines in your changes missing coverage. Please review.

Project coverage is 92.73%. Comparing base (2dea22f) to head (2c9765c).

Files	Patch %	Lines
pkg/cli/server/root.go	74.19%	5 Missing and 3 partials ⚠️
pkg/api/authn.go	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2536      +/-   ##
==========================================
- Coverage   92.74%   92.73%   -0.01%     
==========================================
  Files         169      169              
  Lines       22471    22482      +11     
==========================================
+ Hits        20840    20848       +8     
- Misses       1015     1017       +2     
- Partials      616      617       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rchincha]

@AndersBennedsgaard does this PR work for you?

[rchincha]

Instead of a env var, can we read this from a file? If deployed in kubernetes, this can be made a secret and mounted as a file.

[eusebiu-constantin-petu-dbk]

@rchincha updated, now it's using a separate file for keys.

[AndersBennedsgaard]

@rchincha and @peusebiu sorry for not getting back to you before (I was on vacation) but I have now tested these changes and it seems to work perfectly with multiple replicas 🥳

My only issue I've found is that while encryptKey is supposed to be optional, Zot fails to start up if you don't set it. I guess you would need a mapstructure:",omitempty" on the EncryptKey key?

[eusebiu-constantin-petu-dbk]

@rchincha and @peusebiu sorry for not getting back to you before (I was on vacation) but I have now tested these changes and it seems to work perfectly with multiple replicas 🥳

My only issue I've found is that while encryptKey is supposed to be optional, Zot fails to start up if you don't set it. I guess you would need a mapstructure:",omitempty" on the EncryptKey key?

Fixed it, sorry for that, can you test again? :D

[andaaron]

lgtm

[AndersBennedsgaard]

It works perfectly now, thanks @peusebiu 🥳