From d0cfe429c4db8abcf8d5de82e37960dc3fa5a1a7 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Wed, 8 May 2024 14:04:53 +0200 Subject: [PATCH] confd: fix regression, loss of admin account after upgrade to v24.04 Fix regression introduced in 3f87945, which tries to map a user to an existing home directory. The root cause for the problem is basically an invalid check of the /home/admin UID being in use by any system account or not. This patch uses the same check used by the BusyBox adduser command. In addition to the uid-already-in-use fix, several other checks have been added to ensure we do not end up with an invalid system state: - If reusing an existing /home/admin fails in the 'adduser admin' command, wipe out /home/admin and retry adding the user cleanly - Always delete any group with the same name before trying to add the user, regardless if /home/$user exists or not, this prevents issues with stale group records if we end up failing to load the startup-config and need to load failure-config - For the same reason (failure-config), make sure to always clean up any stale user in the retry step. The same step is used when there is a uid:gid mismapping (the original bug) Fixes #428 Signed-off-by: Joachim Wiberg --- src/confd/src/ietf-system.c | 160 ++++++++++++++++++++++-------------- src/libsrx/src/common.h | 7 +- 2 files changed, 103 insertions(+), 64 deletions(-) diff --git a/src/confd/src/ietf-system.c b/src/confd/src/ietf-system.c index ffbfbe83..c09cb190 100644 --- a/src/confd/src/ietf-system.c +++ b/src/confd/src/ietf-system.c @@ -4,6 +4,8 @@ #include #include #include +#include + #include #include #include @@ -670,36 +672,12 @@ static char *sys_find_usable_shell(sr_session_ctx_t *sess, char *name) return shell; } -static uid_t sys_home_exists(char *user) -{ - char path[strlen(user) + 10]; - struct stat st; - - snprintf(path, sizeof(path), "/home/%s", user); - if (stat(path, &st)) - return 0; - - return st.st_uid; -} - -static int sys_uid_busy(char *user, uid_t uid) -{ - struct passwd *pw; - int rc = 0; - - setpwent(); - while ((pw = getpwent())) { - if (pw->pw_uid != uid) - continue; - if (strcmp(pw->pw_name, user)) - rc = -1; - } - endpwent(); - - return rc; -} - -static int sys_del_user(char *user) +/* + * Used both when deleting a user from the system configuration, and + * when cleaning up stale users while adding a new user to the system. + * In the latter case we don't need to log failure/success. + */ +static int sys_del_user(char *user, bool silent) { char *args[] = { "deluser", "--remove-home", user, NULL @@ -709,49 +687,71 @@ static int sys_del_user(char *user) erasef("/var/run/sshd/%s.keys", user); err = systemv_silent(args); if (err) { - ERROR("Error deleting user \"%s\"", user); + if (!silent) + ERROR("Error deleting user \"%s\"", user); + + /* Ensure $HOME is removed at least. */ + systemf("rm -rf /home/%s", user); + return SR_ERR_SYS; } - NOTE("User \"%s\" deleted", user); + + if (!silent) + NOTE("User \"%s\" deleted", user); return SR_ERR_OK; } -static int sys_add_new_user(sr_session_ctx_t *sess, char *name) +/* + * Helper for sys_add_user(), notice how uid:gid must never be 0:0, it + * is reserved for the system root user which is never created/deleted. + * + * This function balances both /etc/passwd and /etc/group, so to make + * things a little bit easier, we simplify the world and reserve the + * same group name as the username. This means we can always remove + * any stale group entries first. + * + * When uid:gid is non-zero we create the user and re-attach them with + * their $HOME. When uid:gid *is* zero, however, we must make sure to + * clean up any stale user + $HOME before calling adduser to allocate + * a new uid:gid pair for the new user. + * + * If this function fails, it is up to the callee to retry the operation + * with a zero uid:gid pair, which is the most iron clad form. + */ +static int sys_call_adduser(sr_session_ctx_t *sess, char *name, uid_t uid, gid_t gid) { char *shell = sys_find_usable_shell(sess, name); - char uid_str[10]; char *eargs[] = { - "adduser", "-d", "-s", shell, "-u", NULL, "-H", name, NULL + "adduser", "-d", "-s", shell, "-u", NULL, "-G", NULL, "-H", name, NULL }; char *nargs[] = { "adduser", "-d", "-s", shell, name, NULL }; - bool do_chown = false; + char uid_str[10], grp_str[strlen(name) + 1]; char **args; - uid_t uid; int err; - uid = sys_home_exists(name); - if (uid) { - if (sys_uid_busy(name, uid)) { - /* Exists but owned by someone else. */ - ERROR("Creating user %s, /home/%s existed but was owned by another uid (%d)", - name, name, uid); - ERROR("Cleaning up stale /home/%s", name); - systemf("rm -rf /home/%s", name); - args = nargs; - } else { - /* Not in passwd or owned by us already */ - do_chown = true; + DEBUG("Adding new user %s, cleaning up any stale group.", name); + systemf("delgroup %s 2>/dev/null", name); + + /* reusing existing uid:gid from $HOME */ + if (uid && gid) { + /* recreate group first */ + systemf("addgroup -g %d %s", gid, name); + + snprintf(uid_str, sizeof(uid_str), "%d", uid); + snprintf(grp_str, sizeof(grp_str), "%s", name); + eargs[5] = uid_str; + eargs[7] = grp_str; + + args = eargs; + } else { + DEBUG("Cleaning up any stale user %s and /home/%s", name, name); + sys_del_user(name, true); - NOTE("Reusing uid %d and /home/%s for new user %s", uid, name, name); - snprintf(uid_str, sizeof(uid_str), "%d", uid); - eargs[5] = uid_str; - args = eargs; - } - } else args = nargs; + } /** * The Busybox implementation of 'adduser -d' sets the password @@ -760,14 +760,51 @@ static int sys_add_new_user(sr_session_ctx_t *sess, char *name) */ err = systemv_silent(args); free(shell); + + return err; +} + +/* + * Create a new (locked) user, if they have a matching $HOME we use the + * uid:gid from that, if they are free. In all cases of conflict with + * any (type of) user we remove the existing $HOME and start with a new + * uid:gid pair. + */ +static int sys_add_user(sr_session_ctx_t *sess, char *name) +{ + char home[strlen(name) + 10]; + bool reused = false; + struct stat st; + int err; + + /* Map users to their existing $HOME */ + snprintf(home, sizeof(home), "/home/%s", name); + if (!stat(home, &st)) { + /* Verify IDs aren't already used, like BusyBox adduser */ + if (getpwuid(st.st_uid) || getgrgid(st.st_uid) || getgrgid(st.st_gid)) { + /* Exists but owned by someone else. */ + ERROR("Failed mapping user %s to /home/%s, uid:gid (%d:%d) already exists.", + name, name, st.st_uid, st.st_gid); + err = sys_call_adduser(sess, name, 0, 0); + } else { + DEBUG("Reusing uid:gid %d:%d and /home/%s for new user %s", + st.st_uid, st.st_gid, name, name); + err = sys_call_adduser(sess, name, st.st_uid, st.st_gid); + if (err) { + ERROR("Failed reusing uid:gid from /home/%s, retrying create user ...", name); + err = sys_call_adduser(sess, name, 0, 0); + } else + reused = true; + } + } else + err = sys_call_adduser(sess, name, 0, 0); + if (err) { - ERROR("Failed creating new user \"%s\"\n", name); + ERROR("Failed creating new user \"%s\"", name); return SR_ERR_SYS; } - NOTE("New user \"%s\" created\n", name); - if (do_chown) - systemf("chown -R %s:%s /home/%s", name, name, name); + NOTE("User \"%s\" created%s.", name, reused ? ", mapped to existing home directory" : ""); /* * OpenSSH in Infix has been set up to use /var/run/sshd/%s.keys @@ -775,6 +812,7 @@ static int sys_add_new_user(sr_session_ctx_t *sess, char *name) * /home/%s/.ssh/authorized_keys file. This creates a both the * directory and the symlink owned by root to prevent tampering. */ + DEBUG("Adding secure /home/%s/.ssh directory.", name); fmkpath(0750, "/home/%s/.ssh", name); systemf("ln -sf /var/run/sshd/%s.keys /home/%s/.ssh/authorized_keys", name, name); @@ -932,7 +970,7 @@ static sr_error_t handle_sr_user_update(augeas *aug, sr_session_ctx_t *sess, str assert(change->new); name = sr_xpath_key_value(change->new->xpath, "user", "name", &state); - err = sys_add_new_user(sess, name); + err = sys_add_user(sess, name); if (err) { sr_xpath_recover(&state); return err; @@ -943,7 +981,7 @@ static sr_error_t handle_sr_user_update(augeas *aug, sr_session_ctx_t *sess, str assert(change->old); name = sr_xpath_key_value(change->old->xpath, "user", "name", &state); - err = sys_del_user(name); + err = sys_del_user(name, false); if (err) { sr_xpath_recover(&state); return err; diff --git a/src/libsrx/src/common.h b/src/libsrx/src/common.h index 549d08a8..cda9a5b9 100644 --- a/src/libsrx/src/common.h +++ b/src/libsrx/src/common.h @@ -20,12 +20,13 @@ int asprintf(char **strp, const char *fmt, ...); /* * Only NOTICE and above are logged by default, see setlogmask() for details. + * Use `set DEBUG=1` at top of package/confd/confd.conf for logmask = debug */ #define DEBUG(fmt, ...) do { if (debug) syslog(LOG_DEBUG, fmt, ##__VA_ARGS__); } while (0) #define INFO(fmt, ...) syslog(LOG_INFO, fmt, ##__VA_ARGS__) #define NOTE(fmt, ...) syslog(LOG_NOTICE, fmt, ##__VA_ARGS__) -#define WARN(fmt, ...) syslog(LOG_WARNING, "%s: " fmt, __func__, ##__VA_ARGS__) -#define ERROR(fmt, ...) syslog(LOG_ERR, "%s: " fmt, __func__, ##__VA_ARGS__) -#define ERRNO(fmt, ...) syslog(LOG_ERR, "%s: " fmt ": %s", __func__, ##__VA_ARGS__, strerror(errno)) +#define WARN(fmt, ...) syslog(LOG_WARNING, fmt, ##__VA_ARGS__) +#define ERROR(fmt, ...) syslog(LOG_ERR, fmt, ##__VA_ARGS__) +#define ERRNO(fmt, ...) syslog(LOG_ERR, fmt ": %s", ##__VA_ARGS__, strerror(errno)) #endif /* CONFD_COMMON_H_ */