From 3d98512a9f58aa9c583fe8887554a9ea35514f6c Mon Sep 17 00:00:00 2001 From: Pablo Rodriguez Nava Date: Mon, 27 Apr 2026 11:25:39 +0200 Subject: [PATCH 1/2] OCPBUGS-84332: Fix ssh and password rollbacks This change fixes the issue in SSH keys and user passwords that made the rollback useless as it tried to apply the new configuration instead of the previous one. Signed-off-by: Pablo Rodriguez Nava --- pkg/daemon/update.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 96243c5ed0..f368bfbaf9 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -1241,7 +1241,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi defer func() { if retErr != nil { - if err := dn.updateSSHKeys(newIgnConfig.Passwd.Users, oldIgnConfig.Passwd.Users); err != nil { + if err := dn.updateSSHKeys(oldIgnConfig.Passwd.Users, newIgnConfig.Passwd.Users); err != nil { errs := kubeErrs.NewAggregate([]error{err, retErr}) retErr = fmt.Errorf("error rolling back SSH keys updates: %w", errs) return @@ -1257,7 +1257,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi defer func() { if retErr != nil { - if err := dn.SetPasswordHash(newIgnConfig.Passwd.Users, oldIgnConfig.Passwd.Users); err != nil { + if err := dn.SetPasswordHash(oldIgnConfig.Passwd.Users, newIgnConfig.Passwd.Users); err != nil { errs := kubeErrs.NewAggregate([]error{err, retErr}) retErr = fmt.Errorf("error rolling back password hash updates: %w", errs) return From 867a618fad502302088c7f6a8aa9d3c8d04d8b06 Mon Sep 17 00:00:00 2001 From: Pablo Rodriguez Nava Date: Tue, 28 Apr 2026 10:14:01 +0200 Subject: [PATCH 2/2] OCPBUGS-83830: Apply password only if changes exist This bugfix ensures that the MCD only runs `usermod` if the password hash has actually changed and not in every update. This aligns the behavior we currently have for SSH passwords. Signed-off-by: Pablo Rodriguez Nava --- pkg/daemon/update.go | 51 ++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index f368bfbaf9..12ff701e8d 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -1248,22 +1248,22 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi } } }() - } - // Set password hash - if err := dn.SetPasswordHash(newIgnConfig.Passwd.Users, oldIgnConfig.Passwd.Users); err != nil { - return err - } + // Set password hash + if err := dn.SetPasswordHash(newIgnConfig.Passwd.Users, oldIgnConfig.Passwd.Users); err != nil { + return err + } - defer func() { - if retErr != nil { - if err := dn.SetPasswordHash(oldIgnConfig.Passwd.Users, newIgnConfig.Passwd.Users); err != nil { - errs := kubeErrs.NewAggregate([]error{err, retErr}) - retErr = fmt.Errorf("error rolling back password hash updates: %w", errs) - return + defer func() { + if retErr != nil { + if err := dn.SetPasswordHash(oldIgnConfig.Passwd.Users, newIgnConfig.Passwd.Users); err != nil { + errs := kubeErrs.NewAggregate([]error{err, retErr}) + retErr = fmt.Errorf("error rolling back password hash updates: %w", errs) + return + } } - } - }() + }() + } if dn.os.IsCoreOSVariant() { coreOSDaemon := CoreOSDaemon{dn} @@ -2439,7 +2439,21 @@ func (dn *Daemon) atomicallyWriteSSHKey(authKeyPath, keys string) error { return nil } -// Set a given PasswdUser's Password Hash +func getUserPasswordHash(user string) (string, error) { + shadowOut, err := exec.Command("getent", "shadow", user).CombinedOutput() + if err != nil { + return "", fmt.Errorf("Failed to check password hash for %s: %w", user, err) + } + shadowSlice := strings.SplitN(strings.TrimSpace(string(shadowOut)), ":", 3) + if len(shadowSlice) >= 2 { + return shadowSlice[1], nil + } + return "", nil + +} + +// SetPasswordHash updates the password for each user in newUsers, skipping +// users whose password already matches the desired configuration. func (dn *Daemon) SetPasswordHash(newUsers, oldUsers []ign3types.PasswdUser) error { // confirm that user exits klog.Info("Checking if absent users need to be disconfigured") @@ -2464,6 +2478,15 @@ func (dn *Daemon) SetPasswordHash(newUsers, oldUsers []ign3types.PasswdUser) err pwhash = *u.PasswordHash } + // Check if hash update is needed. Skip if not. + currentHash, err := getUserPasswordHash(u.Name) + if err != nil { + return err + } + if currentHash == pwhash { + continue + } + if out, err := exec.Command("usermod", "-p", pwhash, u.Name).CombinedOutput(); err != nil { return fmt.Errorf("Failed to reset password for %s: %s:%w", u.Name, out, err) }