From 39e49353691a35c8acbc2943c1afd8e5187151ea Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sat, 6 Jul 2024 13:18:21 +0200 Subject: [PATCH] confd: refactor user hash validation Consolidate empty password and $factory$ hash extraction to a common function called before set_pasword(). The purpose is to ensure all error handling and log messages are in the same place, except for missing factory-password-hash, which is now treated as a PANIC and logged to /dev/console. Signed-off-by: Joachim Wiberg --- board/common/rootfs/etc/syslog.d/console.conf | 2 + src/confd/src/ietf-system.c | 83 ++++++++++++------- src/libsrx/src/common.h | 1 + 3 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 board/common/rootfs/etc/syslog.d/console.conf diff --git a/board/common/rootfs/etc/syslog.d/console.conf b/board/common/rootfs/etc/syslog.d/console.conf new file mode 100644 index 00000000..54af29ec --- /dev/null +++ b/board/common/rootfs/etc/syslog.d/console.conf @@ -0,0 +1,2 @@ +# Log critical and above to console, system likely unusable. +*.=crit;*.=alert;*.=emerg /dev/console diff --git a/src/confd/src/ietf-system.c b/src/confd/src/ietf-system.c index 99fe0042..69807a6b 100644 --- a/src/confd/src/ietf-system.c +++ b/src/confd/src/ietf-system.c @@ -958,29 +958,12 @@ fail: return -1; } -static int set_password(struct confd *confd, const char *user, const char *hash, bool lock) +static int set_password(const char *user, const char *hash, bool lock) { - const char *factory = "$factory$"; struct spwd *sp; FILE *fp = NULL; int fd = -1; - if (!strncmp(hash, factory, strlen(factory))) { - struct json_t *pwd; - - pwd = json_object_get(confd->root, "factory-password-hash"); - if (!json_is_string(pwd)) { - /* - * Do not fail, lock account instead. This way developers can - * enable root account login at build-time to diagnose the system. - */ - ERROR("%s: cannot find factory-default password hash!", user); - lock = true; - } else { - hash = json_string_value(pwd); - } - } - if (lckpwdf()) goto exit; @@ -1028,11 +1011,46 @@ exit: return -1; } +/* + * The iana-crypt-hash yang model is used to validate password hashes. + * This function traps empty passwords and the $factory$ keyword. + * + * Empty passwords are not allowed, but instead of locking ("!") the + * account, we disable password login with "*", allowing the user to + * log in with SSH keys. + * + * The $factory$ keyword hash is converted to device's default password + * hash, or NULL if it's not available, in which case the account will + * be locked. + */ +static const char *is_valid_hash(struct confd *confd, const char *user, const char *hash) +{ + const char *factory = "$factory$"; + + if (!hash || !strlen(hash)) + return "*"; + + if (!strncmp(hash, factory, strlen(factory))) { + struct json_t *pwd; + + pwd = json_object_get(confd->root, "factory-password-hash"); + if (!json_is_string(pwd)) { + EMERG("Cannot find factory-default password hash for user %s!", user); + return NULL; + } + + hash = json_string_value(pwd); + } + + return hash; +} + static sr_error_t handle_sr_passwd_update(sr_session_ctx_t *, struct confd *confd, struct sr_change *change) { sr_error_t err = SR_ERR_OK; const char *hash; char *user; + bool lock; user = change_get_user(change); if (!user) @@ -1048,25 +1066,28 @@ static sr_error_t handle_sr_passwd_update(sr_session_ctx_t *, struct confd *conf err = SR_ERR_INTERNAL; break; } - hash = change->new->data.string_val; - /* - * The iana-crypt-hash yang model is used to validate password - * hash sanity. This check traps empty passwords, which we do - * not allow. But instead of disabling ("!") the user, we set - * it as "*", meaning the user can log in with SSH keys. - */ - if (!hash || !strlen(hash)) { - ERROR("Empty passwords are not allowed, disabling password login."); - hash = "*"; - } - if (set_password(confd, user, hash, false)) + hash = is_valid_hash(confd, user, change->new->data.string_val); + if (!hash) { + /* + * Do not fail, lock account instead. This way developers can + * enable root account login at build-time to diagnose the system. + */ + lock = true; + } else + lock = false; + + if (set_password(user, hash, lock)) err = SR_ERR_SYS; + else if (lock) + NOTE("User account %s locked.", user); + else if (!strcmp(hash, "*")) + NOTE("Password login disabled for user %s", user); else NOTE("Password updated for user %s", user); break; case SR_OP_DELETED: - if (set_password(confd, user, "*", true)) + if (set_password(user, "*", true)) err = SR_ERR_SYS; else NOTE("Password deleted for user %s", user); diff --git a/src/libsrx/src/common.h b/src/libsrx/src/common.h index 426b7ab6..6505305f 100644 --- a/src/libsrx/src/common.h +++ b/src/libsrx/src/common.h @@ -26,6 +26,7 @@ int asprintf(char **strp, const char *fmt, ...); #define INFO(fmt, ...) syslog(LOG_INFO, fmt, ##__VA_ARGS__) #define NOTE(fmt, ...) syslog(LOG_NOTICE, fmt, ##__VA_ARGS__) #define WARN(fmt, ...) syslog(LOG_WARNING, fmt, ##__VA_ARGS__) +#define EMERG(fmt, ...) syslog(LOG_EMERG, fmt, ##__VA_ARGS__) #define ERROR(fmt, ...) syslog(LOG_ERR, fmt, ##__VA_ARGS__) #define ERRNO(fmt, ...) syslog(LOG_ERR, fmt ": %s", ##__VA_ARGS__, strerror(errno))