Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cc_user_groups incorrectly assumes "useradd" never locks password field #5355

189 changes: 176 additions & 13 deletions cloudinit/distros/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
doas_fn = "/etc/doas.conf"
ci_sudoers_fn = "/etc/sudoers.d/90-cloud-init-users"
hostname_conf_fn = "/etc/hostname"
shadow_fn = "/etc/shadow"
shadow_extrausers_fn = "/var/lib/extrausers/shadow"
# /etc/shadow match patterns indicating empty passwords
shadow_empty_locked_passwd_patterns = ["^{username}::", "^{username}:!:"]
tz_zone_dir = "/usr/share/zoneinfo"
default_owner = "root:root"
init_cmd = ["service"] # systemctl, service etc
Expand Down Expand Up @@ -655,19 +659,21 @@ def preferred_ntp_clients(self):
def get_default_user(self):
return self.get_option("default_user")

def add_user(self, name, **kwargs):
def add_user(self, name, **kwargs) -> bool:
"""
Add a user to the system using standard GNU tools

This should be overridden on distros where useradd is not desirable or
not available.

Returns False if user already exists, otherwise True.
"""
# XXX need to make add_user idempotent somehow as we
# still want to add groups or modify SSH keys on pre-existing
# users in the image.
if util.is_user(name):
LOG.info("User %s already exists, skipping.", name)
return
return False

if "create_groups" in kwargs:
create_groups = kwargs.pop("create_groups")
Expand Down Expand Up @@ -771,6 +777,9 @@ def add_user(self, name, **kwargs):
util.logexc(LOG, "Failed to create user %s", name)
raise e

# Indicate that a new user was created
return True

def add_snap_user(self, name, **kwargs):
"""
Add a snappy user to the system using snappy tools
Expand Down Expand Up @@ -798,6 +807,40 @@ def add_snap_user(self, name, **kwargs):

return username

def _shadow_file_has_empty_user_password(self, username) -> bool:
"""
Check whether username exists in shadow files with empty password.

Support reading /var/lib/extrausers/shadow on snappy systems.
"""
if util.system_is_snappy():
shadow_files = [self.shadow_extrausers_fn, self.shadow_fn]
else:
shadow_files = [self.shadow_fn]
shadow_empty_passwd_re = "|".join(
[
pattern.format(username=username)
for pattern in self.shadow_empty_locked_passwd_patterns
]
)
for shadow_file in shadow_files:
if not os.path.exists(shadow_file):
continue
shadow_content = util.load_text_file(shadow_file)
if not re.findall(rf"^{username}:", shadow_content, re.MULTILINE):
LOG.debug("User %s not found in %s", username, shadow_file)
continue
LOG.debug(
"User %s found in %s. Checking for empty password",
username,
shadow_file,
)
if re.findall(
shadow_empty_passwd_re, shadow_content, re.MULTILINE
):
return True
return False

def create_user(self, name, **kwargs):
"""
Creates or partially updates the ``name`` user in the system.
Expand All @@ -824,20 +867,93 @@ def create_user(self, name, **kwargs):
return self.add_snap_user(name, **kwargs)

# Add the user
self.add_user(name, **kwargs)

# Set password if plain-text password provided and non-empty
if "plain_text_passwd" in kwargs and kwargs["plain_text_passwd"]:
self.set_passwd(name, kwargs["plain_text_passwd"])

# Set password if hashed password is provided and non-empty
if "hashed_passwd" in kwargs and kwargs["hashed_passwd"]:
self.set_passwd(name, kwargs["hashed_passwd"], hashed=True)
pre_existing_user = not self.add_user(name, **kwargs)

has_existing_password = False
ud_blank_password_specified = False
ud_password_specified = False
password_key = None

if "plain_text_passwd" in kwargs:
ud_password_specified = True
password_key = "plain_text_passwd"
if kwargs["plain_text_passwd"]:
# Set password if plain-text password provided and non-empty
self.set_passwd(name, kwargs["plain_text_passwd"])
else:
ud_blank_password_specified = True

if "hashed_passwd" in kwargs:
ud_password_specified = True
password_key = "hashed_passwd"
if kwargs["hashed_passwd"]:
# Set password if hashed password is provided and non-empty
self.set_passwd(name, kwargs["hashed_passwd"], hashed=True)
else:
ud_blank_password_specified = True

if pre_existing_user:
if not ud_password_specified:
if "passwd" in kwargs:
password_key = "passwd"
# Only "plain_text_passwd" and "hashed_passwd"
# are valid for an existing user.
LOG.warning(
"'passwd' in user-data is ignored for existing "
"user %s",
name,
)

# Default locking down the account. 'lock_passwd' defaults to True.
# lock account unless lock_password is False.
# As no password specified for the existing user in user-data
# then check if the existing user's hashed password value is
# empty (whether locked or not).
has_existing_password = not (
self._shadow_file_has_empty_user_password(name)
)
else:
if "passwd" in kwargs:
ud_password_specified = True
password_key = "passwd"
if not kwargs["passwd"]:
ud_blank_password_specified = True

# Default locking down the account. 'lock_passwd' defaults to True.
# Lock account unless lock_password is False in which case unlock
# account as long as a password (blank or otherwise) was specified.
if kwargs.get("lock_passwd", True):
self.lock_passwd(name)
elif has_existing_password or ud_password_specified:
# 'lock_passwd: False' and either existing account already with
# non-blank password or else existing/new account with password
# explicitly set in user-data.
if ud_blank_password_specified:
LOG.debug(
"Allowing unlocking empty password for %s based on empty"
" '%s' in user-data",
name,
password_key,
)

# Unlock the existing/new account
self.unlock_passwd(name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unlock is still being called even if blank_password and pre_existing_user. I think we still don't want to unlock the password in this case.

I'm working on a couple of unittests for these paths so we can be more certain we aren't adding exposure to unlocking insecure accounts by default.

Copy link
Contributor Author

@dermotbradley dermotbradley Aug 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm renaming some of the variables to make it clear what their purpose is, so "blank_password" becomes "ud_blank_password_specified" to indicate this is where user-data contains a passwd/plain_text_passwd/hashed_passwd value of "" rather than where an existing user account has no password.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dermotbradley thanks on this. If we get a chance to get this updated with any changes/refactors please ping. I'd like to get this into 24.3 which is scheduled for the week of Aug 19th.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blackboxsw Ok, I've pushed further changes.

Still needed are tests for snappy and for the various BSDs - neither of those are currently tested at all in test_create_users.py.

I'd also like to get this in 24.3 if possible.

elif pre_existing_user:
# Pre-existing user with no existing password and none
# explicitly set in user-data.
LOG.warning(
"Not unlocking blank password for existing user %s."
" 'lock_passwd: false' present in user-data but no existing"
" password set and no 'plain_text_passwd'/'hashed_passwd'"
" provided in user-data",
name,
)
else:
# No password (whether blank or otherwise) explicitly set
LOG.warning(
"Not unlocking password for user %s. 'lock_passwd: false'"
" present in user-data but no 'passwd'/'plain_text_passwd'/"
"'hashed_passwd' provided in user-data",
name,
)

# Configure doas access
if "doas" in kwargs:
Expand Down Expand Up @@ -914,6 +1030,50 @@ def lock_passwd(self, name):
util.logexc(LOG, "Failed to disable password for user %s", name)
raise e

def unlock_passwd(self, name: str):
"""
Unlock the password of a user, i.e., enable password logins
"""
# passwd must use short '-u' due to SLES11 lacking long form '--unlock'
unlock_tools = (["passwd", "-u", name], ["usermod", "--unlock", name])
try:
cmd = next(tool for tool in unlock_tools if subp.which(tool[0]))
except StopIteration as e:
raise RuntimeError(
"Unable to unlock user account '%s'. No tools available. "
" Tried: %s." % (name, [c[0] for c in unlock_tools])
) from e
try:
_, err = subp.subp(cmd, rcs=[0, 3])
dermotbradley marked this conversation as resolved.
Show resolved Hide resolved
except Exception as e:
util.logexc(LOG, "Failed to enable password for user %s", name)
raise e
if err:
# if "passwd" or "usermod" are unable to unlock an account with
# an empty password then they display a message on stdout. In
# that case then instead set a blank password.
passwd_set_tools = (
["passwd", "-d", name],
["usermod", "--password", "''", name],
)
dermotbradley marked this conversation as resolved.
Show resolved Hide resolved
try:
cmd = next(
tool for tool in passwd_set_tools if subp.which(tool[0])
)
except StopIteration as e:
raise RuntimeError(
"Unable to set blank password for user account '%s'. "
"No tools available. "
" Tried: %s." % (name, [c[0] for c in unlock_tools])
) from e
try:
subp.subp(cmd)
except Exception as e:
util.logexc(
LOG, "Failed to set blank password for user %s", name
)
raise e

def expire_passwd(self, user):
try:
subp.subp(["passwd", "--expire", user])
Expand Down Expand Up @@ -948,6 +1108,9 @@ def chpasswd(self, plist_in: list, hashed: bool):
)
+ "\n"
)
# Need to use the short option name '-e' instead of '--encrypted'
# (which would be more descriptive) since Busybox and SLES 11
# chpasswd don't know about long names.
cmd = ["chpasswd"] + (["-e"] if hashed else [])
subp.subp(cmd, data=payload)

Expand Down
39 changes: 37 additions & 2 deletions cloudinit/distros/alpine.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,18 @@ def preferred_ntp_clients(self):

return self._preferred_ntp_clients

def add_user(self, name, **kwargs):
def add_user(self, name, **kwargs) -> bool:
"""
Add a user to the system using standard tools

On Alpine this may use either 'useradd' or 'adduser' depending
on whether the 'shadow' package is installed.

Returns False if user already exists, otherwise True.
"""
if util.is_user(name):
LOG.info("User %s already exists, skipping.", name)
return
return False

if "selinux_user" in kwargs:
LOG.warning("Ignoring selinux_user parameter for Alpine Linux")
Expand Down Expand Up @@ -418,6 +420,9 @@ def add_user(self, name, **kwargs):
LOG, "Failed to update %s for user %s", shadow_file, name
)

# Indicate that a new user was created
return True

def lock_passwd(self, name):
"""
Lock the password of a user, i.e., disable password logins
Expand Down Expand Up @@ -446,6 +451,36 @@ def lock_passwd(self, name):
util.logexc(LOG, "Failed to disable password for user %s", name)
raise e

def unlock_passwd(self, name: str):
"""
Unlock the password of a user, i.e., enable password logins
"""

# Check whether Shadow's or Busybox's version of 'passwd'.
# If Shadow's 'passwd' is available then use the generic
# lock_passwd function from __init__.py instead.
if not os.path.islink(
"/usr/bin/passwd"
) or "bbsuid" not in os.readlink("/usr/bin/passwd"):
return super().unlock_passwd(name)

cmd = ["passwd", "-u", name]
# Busybox's 'passwd', unlike Shadow's 'passwd', errors
# if password is already unlocked:
#
# "passwd: password for user2 is already unlocked"
#
# with exit code 1
#
# and also does *not* error if no password is set.
try:
_, err = subp.subp(cmd, rcs=[0, 1])
if re.search(r"is already unlocked", err):
return True
except subp.ProcessExecutionError as e:
util.logexc(LOG, "Failed to unlock password for user %s", name)
raise e

def expire_passwd(self, user):
# Check whether Shadow's or Busybox's version of 'passwd'.
# If Shadow's 'passwd' is available then use the generic
Expand Down
1 change: 1 addition & 0 deletions cloudinit/distros/bsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class BSD(distros.Distro):
networking_cls = BSDNetworking
hostname_conf_fn = "/etc/rc.conf"
rc_conf_fn = "/etc/rc.conf"
shadow_fn = "/etc/master.passwd"
default_owner = "root:wheel"

# This differs from the parent Distro class, which has -P for
Expand Down
28 changes: 27 additions & 1 deletion cloudinit/distros/freebsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ class Distro(cloudinit.distros.bsd.BSD):
dhclient_lease_directory = "/var/db"
dhclient_lease_file_regex = r"dhclient.leases.\w+"

# /etc/shadow match patterns indicating empty passwords
# For FreeBSD (from https://man.freebsd.org/cgi/man.cgi?passwd(5)) a
# password field of "" indicates no password, and a password
# field value of either "*" or "*LOCKED*" indicate differing forms of
# "locked" but with no password defined.
shadow_empty_locked_passwd_patterns = [
r"^{username}::",
r"^{username}:\*:",
r"^{username}:\*LOCKED\*:",
]

@classmethod
def reload_init(cls, rcs=None):
"""
Expand Down Expand Up @@ -86,7 +97,12 @@ def manage_service(
def _get_add_member_to_group_cmd(self, member_name, group_name):
return ["pw", "usermod", "-n", member_name, "-G", group_name]

def add_user(self, name, **kwargs):
def add_user(self, name, **kwargs) -> bool:
"""
Add a user to the system using standard tools

Returns False if user already exists, otherwise True.
"""
if util.is_user(name):
LOG.info("User %s already exists, skipping.", name)
return False
Expand Down Expand Up @@ -140,6 +156,9 @@ def add_user(self, name, **kwargs):
if passwd_val is not None:
self.set_passwd(name, passwd_val, hashed=True)

# Indicate that a new user was created
return True

def expire_passwd(self, user):
try:
subp.subp(["pw", "usermod", user, "-p", "01-Jan-1970"])
Expand Down Expand Up @@ -170,6 +189,13 @@ def lock_passwd(self, name):
util.logexc(LOG, "Failed to lock password login for user %s", name)
raise

def unlock_passwd(self, name):
LOG.debug(
"Dragonfly BSD/FreeBSD password lock is not reversible, "
"ignoring unlock for user %s",
name,
)

def apply_locale(self, locale, out_fn=None):
# Adjust the locales value to the new value
newconf = StringIO()
Expand Down
Loading
Loading