diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 00130e00b8..04d1be9eaf 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -1408,29 +1408,29 @@ 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 } } }() - } - // 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(newIgnConfig.Passwd.Users, oldIgnConfig.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} @@ -2489,7 +2489,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") @@ -2514,6 +2528,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) } diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index d3abbbd365..bde61a2f9a 100644 --- a/test/e2e/mcd_test.go +++ b/test/e2e/mcd_test.go @@ -533,8 +533,11 @@ func TestNoReboot(t *testing.T) { currentEtcShadowContents := helpers.ExecCmdOnNode(t, cs, infraNode, "grep", "^core:", "/rootfs/etc/shadow") - if currentEtcShadowContents == initialEtcShadowContents { - t.Fatalf("updated password hash not found in etc/shadow, got %s", currentEtcShadowContents) + initialPasswordHash := extractPasswordHashFromShadowLine(initialEtcShadowContents) + currentPasswordHash := extractPasswordHashFromShadowLine(currentEtcShadowContents) + + if currentPasswordHash == initialPasswordHash { + t.Fatalf("updated password hash not found in etc/shadow") } t.Logf("Node %s has Password Hash", infraNode.Name) @@ -614,7 +617,9 @@ func TestNoReboot(t *testing.T) { require.Nil(t, err) rollbackEtcShadowContents := helpers.ExecCmdOnNode(t, cs, infraNode, "grep", "^core:", "/rootfs/etc/shadow") - assert.Equal(t, initialEtcShadowContents, rollbackEtcShadowContents) + + assert.Equal(t, extractPasswordHashFromShadowLine(initialEtcShadowContents), extractPasswordHashFromShadowLine(rollbackEtcShadowContents), + "password hash should match after rollback") } func TestPoolDegradedOnFailToRender(t *testing.T) { @@ -976,6 +981,17 @@ func assertExpectedPerms(t *testing.T, cs *framework.ClientSet, node corev1.Node assert.Equal(t, expectedPerms, actualPerms, "expected %s to have perms %v, got: %v", path, expectedPerms, actualPerms) } +// extractPasswordHashFromShadowLine extracts the password hash field from an /etc/shadow line. +// The shadow file format is: username:password:lastchg:min:max:warn:inactive:expire:reserved +// This function returns the password hash (field index 1). +func extractPasswordHashFromShadowLine(shadowLine string) string { + fields := strings.Split(strings.TrimSpace(shadowLine), ":") + if len(fields) < 2 { + return "" + } + return fields[1] +} + // Tests that changes to the internal image registry pull secret has the // desired effect on both the ControllerConfig and the individual nodes // filesystems.