diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 4a948e409..24b5ac2cd 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -51,6 +51,7 @@ type Controller struct { RelyingParties map[string]rp.RelyingParty CookieStore *CookieStore HTPasswd *HTPasswd + HTPasswdWatcher *HTPasswdWatcher LDAPClient *LDAPClient taskScheduler *scheduler.Scheduler // runtime params @@ -99,9 +100,17 @@ func NewController(appConfig *config.Config) *Controller { Str("clusterMemberIndex", strconv.Itoa(memberSocketIdx)).Logger() } + htp := NewHTPasswd(logger) + + htw, err := NewHTPasswdWatcher(htp, "") + if err != nil { + logger.Panic().Err(err).Msg("failed to create htpasswd watcher") + } + controller.Config = appConfig controller.Log = logger - controller.HTPasswd = NewHTPasswd(logger) + controller.HTPasswd = htp + controller.HTPasswdWatcher = htw if appConfig.Log.Audit != "" { audit := log.NewAuditLogger(appConfig.Log.Level, appConfig.Log.Audit) @@ -285,6 +294,13 @@ func (c *Controller) Init() error { c.InitCVEInfo() + if c.Config.IsHtpasswdAuthEnabled() { + err := c.HTPasswdWatcher.ChangeFile(c.Config.HTTP.Auth.HTPasswd.Path) + if err != nil { + return err + } + } + return nil } @@ -367,10 +383,9 @@ func (c *Controller) LoadNewConfig(newConfig *config.Config) { c.Config.HTTP.Auth.HTPasswd = newConfig.HTTP.Auth.HTPasswd c.Config.HTTP.Auth.LDAP = newConfig.HTTP.Auth.LDAP - if c.Config.HTTP.Auth.HTPasswd.Path == "" { - c.HTPasswd.Clear() - } else { - _ = c.HTPasswd.Reload(c.Config.HTTP.Auth.HTPasswd.Path) + err := c.HTPasswdWatcher.ChangeFile(c.Config.HTTP.Auth.HTPasswd.Path) + if err != nil { + c.Log.Error().Err(err).Msg("failed to change watched htpasswd file") } if c.LDAPClient != nil { @@ -379,6 +394,8 @@ func (c *Controller) LoadNewConfig(newConfig *config.Config) { c.LDAPClient.BindPassword = newConfig.HTTP.Auth.LDAP.BindPassword() c.LDAPClient.lock.Unlock() } + } else { + _ = c.HTPasswdWatcher.ChangeFile("") } // reload periodical gc config diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index c01fd432c..81063ef25 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -3142,7 +3142,7 @@ func TestBasicAuthWithReloadedCredentials(t *testing.T) { ctlr := api.NewController(conf) ctlrManager := test.NewControllerManager(ctlr) - hotReloader, err := server.NewHotReloader(ctlr, configPath, "", ldapConfigPath) + hotReloader, err := server.NewHotReloader(ctlr, configPath, ldapConfigPath) So(err, ShouldBeNil) hotReloader.Start() diff --git a/pkg/api/htpasswd.go b/pkg/api/htpasswd.go index 9c3136984..0c3032a81 100644 --- a/pkg/api/htpasswd.go +++ b/pkg/api/htpasswd.go @@ -2,15 +2,23 @@ package api import ( "bufio" + "context" + "errors" "os" + "os/signal" "strings" "sync" + "syscall" + "github.com/fsnotify/fsnotify" "golang.org/x/crypto/bcrypt" "zotregistry.dev/zot/pkg/log" ) +// HTPasswd user auth store +// +// Currently supports only bcrypt hashes. type HTPasswd struct { mu sync.RWMutex credMap map[string]string @@ -29,7 +37,7 @@ func (s *HTPasswd) Reload(filePath string) error { credsFile, err := os.Open(filePath) if err != nil { - s.log.Error().Err(err).Str("credsFile", filePath).Msg("failed to reload htpasswd") + s.log.Error().Err(err).Str("htpasswd-file", filePath).Msg("failed to reload htpasswd") return err } @@ -38,15 +46,16 @@ func (s *HTPasswd) Reload(filePath string) error { scanner := bufio.NewScanner(credsFile) for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, ":") { - tokens := strings.Split(scanner.Text(), ":") - credMap[tokens[0]] = tokens[1] + user, hash, ok := strings.Cut(scanner.Text(), ":") + if ok { + credMap[user] = hash } } if len(credMap) == 0 { - s.log.Warn().Str("credsFile", filePath).Msg("loaded htpasswd file appears to have zero users") + s.log.Warn().Str("htpasswd-file", filePath).Msg("loaded htpasswd file appears to have zero users") + } else { + s.log.Info().Str("htpasswd-file", filePath).Int("users", len(credMap)).Msg("loaded htpasswd file") } s.mu.Lock() @@ -81,5 +90,109 @@ func (s *HTPasswd) Authenticate(username, passphrase string) (ok, present bool) err := bcrypt.CompareHashAndPassword([]byte(passphraseHash), []byte(passphrase)) ok = err == nil + if err != nil && !errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { + // Log that user's hash has unsupported format. Better than silently return 401. + s.log.Warn().Err(err).Str("username", username).Msg("htpasswd bcrypt compare failed") + } + return } + +// HTPasswdWatcher helper which triggers htpasswd reload on file change event. +// +// Cannot be restarted. +type HTPasswdWatcher struct { + htp *HTPasswd + filePath string + watcher *fsnotify.Watcher + cancel context.CancelFunc + log log.Logger +} + +// NewHTPasswdWatcher create and start watcher. +func NewHTPasswdWatcher(htp *HTPasswd, filePath string) (*HTPasswdWatcher, error) { + watcher, err := fsnotify.NewWatcher() + if err != nil { + return nil, err + } + + if filePath != "" { + err = watcher.Add(filePath) + if err != nil { + return nil, errors.Join(err, watcher.Close()) + } + } + + // background event processor job context + ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT) + + ret := &HTPasswdWatcher{ + htp: htp, + filePath: filePath, + watcher: watcher, + cancel: cancel, + log: htp.log, + } + + go func() { + defer ret.watcher.Close() //nolint: errcheck + + for { + select { + case ev := <-ret.watcher.Events: + if ev.Op != fsnotify.Write { + continue + } + + ret.log.Info().Str("htpasswd-file", ret.filePath).Msg("htpasswd file changed, trying to reload config") + + err := ret.htp.Reload(ret.filePath) + if err != nil { + ret.log.Warn().Err(err).Str("htpasswd-file", ret.filePath).Msg("failed to reload file") + } + + case err := <-ret.watcher.Errors: + ret.log.Error().Err(err).Str("htpasswd-file", ret.filePath).Msg("failed to fsnotfy, got error while watching file") + + case <-ctx.Done(): + ret.log.Debug().Msg("htpasswd watcher terminating...") + + return + } + } + }() + + return ret, nil +} + +// ChangeFile changes monitored file. Empty string clears store. +func (s *HTPasswdWatcher) ChangeFile(filePath string) error { + if s.filePath != "" { + err := s.watcher.Remove(s.filePath) + if err != nil { + return err + } + } + + if filePath == "" { + s.filePath = filePath + s.htp.Clear() + + return nil + } + + err := s.watcher.Add(filePath) + if err != nil { + return err + } + + s.filePath = filePath + + return s.htp.Reload(filePath) +} + +func (s *HTPasswdWatcher) Close() error { + s.cancel() + + return nil +} diff --git a/pkg/cli/server/config_reloader.go b/pkg/cli/server/config_reloader.go index aac8f20a5..5d2ccbe68 100644 --- a/pkg/cli/server/config_reloader.go +++ b/pkg/cli/server/config_reloader.go @@ -16,12 +16,11 @@ import ( type HotReloader struct { watcher *fsnotify.Watcher configPath string - htpasswdPath string ldapCredentialsPath string ctlr *api.Controller } -func NewHotReloader(ctlr *api.Controller, filePath, htpasswdPath, ldapCredentialsPath string) (*HotReloader, error) { +func NewHotReloader(ctlr *api.Controller, filePath, ldapCredentialsPath string) (*HotReloader, error) { // creates a new file watcher watcher, err := fsnotify.NewWatcher() if err != nil { @@ -31,7 +30,6 @@ func NewHotReloader(ctlr *api.Controller, filePath, htpasswdPath, ldapCredential hotReloader := &HotReloader{ watcher: watcher, configPath: filePath, - htpasswdPath: htpasswdPath, ldapCredentialsPath: ldapCredentialsPath, ctlr: ctlr, } @@ -85,20 +83,6 @@ func (hr *HotReloader) Start() { continue } - if hr.ctlr.Config.HTTP.Auth != nil && - hr.ctlr.Config.HTTP.Auth.HTPasswd.Path != newConfig.HTTP.Auth.HTPasswd.Path { - err = hr.watcher.Remove(hr.ctlr.Config.HTTP.Auth.HTPasswd.Path) - if err != nil && !errors.Is(err, fsnotify.ErrNonExistentWatch) { - log.Error().Err(err).Msg("failed to remove old watch for the htpasswd file") - } - - err = hr.watcher.Add(newConfig.HTTP.Auth.HTPasswd.Path) - if err != nil { - log.Panic().Err(err).Str("htpasswd-file", newConfig.HTTP.Auth.HTPasswd.Path). - Msg("failed to watch htpasswd file") - } - } - if hr.ctlr.Config.HTTP.Auth != nil && hr.ctlr.Config.HTTP.Auth.LDAP != nil && hr.ctlr.Config.HTTP.Auth.LDAP.CredentialsFile != newConfig.HTTP.Auth.LDAP.CredentialsFile { err = hr.watcher.Remove(hr.ctlr.Config.HTTP.Auth.LDAP.CredentialsFile) @@ -133,13 +117,6 @@ func (hr *HotReloader) Start() { log.Panic().Err(err).Str("config", hr.configPath).Msg("failed to add config file to fsnotity watcher") } - if hr.htpasswdPath != "" { - if err := hr.watcher.Add(hr.htpasswdPath); err != nil { - log.Panic().Err(err).Str("htpasswd-file", hr.htpasswdPath). - Msg("failed to add htpasswd to fsnotity watcher") - } - } - if hr.ldapCredentialsPath != "" { if err := hr.watcher.Add(hr.ldapCredentialsPath); err != nil { log.Panic().Err(err).Str("ldap-credentials", hr.ldapCredentialsPath). diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index f225cc6e2..c0db1bb3c 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -57,18 +57,13 @@ func newServeCmd(conf *config.Config) *cobra.Command { ctlr := api.NewController(conf) - htpasswdPath := "" ldapCredentials := "" - if conf.HTTP.Auth != nil { - htpasswdPath = conf.HTTP.Auth.HTPasswd.Path - } - if conf.HTTP.Auth != nil && conf.HTTP.Auth.LDAP != nil { ldapCredentials = conf.HTTP.Auth.LDAP.CredentialsFile } // config reloader - hotReloader, err := NewHotReloader(ctlr, args[0], htpasswdPath, ldapCredentials) + hotReloader, err := NewHotReloader(ctlr, args[0], ldapCredentials) if err != nil { ctlr.Log.Error().Err(err).Msg("failed to create a new hot reloader") diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index ed1b03b32..53abcae11 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -2069,7 +2069,7 @@ func TestConfigReloader(t *testing.T) { _, err = cfgfile.WriteString(content) So(err, ShouldBeNil) - hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "", "") + hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "") So(err, ShouldBeNil) hotReloader.Start() @@ -2219,7 +2219,7 @@ func TestConfigReloader(t *testing.T) { _, err = cfgfile.WriteString(content) So(err, ShouldBeNil) - hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "", "") + hotReloader, err := cli.NewHotReloader(dctlr, cfgfile.Name(), "") So(err, ShouldBeNil) hotReloader.Start()