From d82c6e0051084764383ca260445c2879a07a67bb Mon Sep 17 00:00:00 2001 From: G Dutton <35460590+gdttn@users.noreply.github.com> Date: Mon, 2 Sep 2019 14:26:30 +0100 Subject: [PATCH 1/4] user: Alter "set password" usage information This changes the usage string to match reality and note that [<16|20>] is an optional argument to set password. --- lib/ipmi_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ipmi_user.c b/lib/ipmi_user.c index 3c70e8f8..4eddecb2 100644 --- a/lib/ipmi_user.c +++ b/lib/ipmi_user.c @@ -434,7 +434,7 @@ print_user_usage(void) lprintf(LOG_NOTICE, " set name "); lprintf(LOG_NOTICE, -" set password [ <16|20>]"); +" set password [ [<16|20>]]"); lprintf(LOG_NOTICE, " disable "); lprintf(LOG_NOTICE, From 541f49d953621d170439f188798784423a1854e0 Mon Sep 17 00:00:00 2001 From: G Dutton <35460590+gdttn@users.noreply.github.com> Date: Mon, 2 Sep 2019 21:51:11 +0100 Subject: [PATCH 2/4] user: Improve password length handling No longer truncate passwords (16 < p <= 20) silently, instead attempt to set a 20-char password when such a password is given. Fail if an explicit length is exceeded, and any time the upper limit is exceeded. --- lib/ipmi_user.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/ipmi_user.c b/lib/ipmi_user.c index 4eddecb2..408a647d 100644 --- a/lib/ipmi_user.c +++ b/lib/ipmi_user.c @@ -657,24 +657,25 @@ ipmi_user_password(struct ipmi_intf *intf, int argc, char **argv) } } else { password = argv[3]; - if (argc > 4) { - if ((str2uchar(argv[4], &password_type) != 0) - || (password_type != 16 && password_type != 20)) { - lprintf(LOG_ERR, "Invalid password length '%s'", argv[4]); - return (-1); - } - } else { - password_type = 16; - } - } - - if (!password) { - lprintf(LOG_ERR, "Unable to parse password argument."); - return (-1); - } else if (strlen(password) > 20) { - lprintf(LOG_ERR, "Password is too long (> 20 bytes)"); - return (-1); } + + if (argc > 4) { + if ((str2uchar(argv[4], &password_type) != 0) + || (password_type != 16 && password_type != 20)) { + lprintf(LOG_ERR, "Invalid password length '%s'", argv[4]); + return (-1); + } + } else if (strlen(password) > 16) { + password_type = 20; + } + + if (!password) { + lprintf(LOG_ERR, "Unable to parse password argument."); + return (-1); + } else if (strlen(password) > password_type) { + lprintf(LOG_ERR, "Password is too long (> %d bytes)", password_type); + return (-1); + } ccode = _ipmi_set_user_password(intf, user_id, IPMI_PASSWORD_SET_PASSWORD, password, From e55a7a4ddc2fdc27549a24137b09d3b44637424e Mon Sep 17 00:00:00 2001 From: Alexander Amelkin Date: Tue, 3 Sep 2019 14:11:49 +0300 Subject: [PATCH 3/4] user: Cleanup/refactor ipmi_user_password() Get rid of magic numbers, fix some formatting, drop unneeded checks. Signed-off-by: Alexander Amelkin --- lib/ipmi_user.c | 67 ++++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/lib/ipmi_user.c b/lib/ipmi_user.c index 408a647d..d35f23ac 100644 --- a/lib/ipmi_user.c +++ b/lib/ipmi_user.c @@ -626,12 +626,17 @@ ipmi_user_mod(struct ipmi_intf *intf, int argc, char **argv) return 0; } +#define USER_PW_IPMI15_LEN 16 /* IPMI 1.5 only allowed for 16 bytes */ +#define USER_PW_IPMI20_LEN 20 /* IPMI 2.0 allows for 20 bytes */ +#define USER_PW_MAX_LEN USER_PW_IPMI20_LEN + int ipmi_user_password(struct ipmi_intf *intf, int argc, char **argv) { char *password = NULL; int ccode = 0; - uint8_t password_type = 16; + uint8_t password_type = USER_PW_IPMI15_LEN; + size_t password_len; uint8_t user_id = 0; if (is_ipmi_user_id(argv[2], &user_id)) { return (-1); @@ -640,53 +645,63 @@ ipmi_user_password(struct ipmi_intf *intf, int argc, char **argv) if (argc == 3) { /* We need to prompt for a password */ char *tmp; + size_t tmplen; password = ask_password(user_id); if (!password) { lprintf(LOG_ERR, "ipmitool: malloc failure"); return (-1); } tmp = ask_password(user_id); + tmplen = strnlen(tmp, USER_PW_MAX_LEN + 1); if (!tmp) { lprintf(LOG_ERR, "ipmitool: malloc failure"); return (-1); } - if (strlen(password) != strlen(tmp) - || strncmp(password, tmp, strlen(tmp))) { - lprintf(LOG_ERR, "Passwords do not match."); + if (strncmp(password, tmp, tmplen)) { + lprintf(LOG_ERR, "Passwords do not match or are " + "longer than %d", USER_PW_MAX_LEN); return (-1); } } else { password = argv[3]; } - - if (argc > 4) { - if ((str2uchar(argv[4], &password_type) != 0) - || (password_type != 16 && password_type != 20)) { - lprintf(LOG_ERR, "Invalid password length '%s'", argv[4]); - return (-1); - } - } else if (strlen(password) > 16) { - password_type = 20; - } - - if (!password) { - lprintf(LOG_ERR, "Unable to parse password argument."); - return (-1); - } else if (strlen(password) > password_type) { - lprintf(LOG_ERR, "Password is too long (> %d bytes)", password_type); - return (-1); - } + + if (!password) { + lprintf(LOG_ERR, "Unable to parse password argument."); + return (-1); + } + + password_len = strnlen(password, USER_PW_MAX_LEN + 1); + + if (argc > 4) { + if ((str2uchar(argv[4], &password_type) != 0) + || (password_type != USER_PW_IPMI15_LEN + && password_type != USER_PW_IPMI20_LEN)) + { + lprintf(LOG_ERR, "Invalid password length '%s'", + argv[4]); + return (-1); + } + } else if (password_len > USER_PW_IPMI15_LEN) { + password_type = USER_PW_IPMI20_LEN; + } + + if (password_len > password_type) { + lprintf(LOG_ERR, "Password is too long (> %d bytes)", + password_type); + return (-1); + } ccode = _ipmi_set_user_password(intf, user_id, - IPMI_PASSWORD_SET_PASSWORD, password, - password_type > 16); + IPMI_PASSWORD_SET_PASSWORD, password, + password_type > USER_PW_IPMI15_LEN); if (eval_ccode(ccode) != 0) { lprintf(LOG_ERR, "Set User Password command failed (user %d)", - user_id); + user_id); return (-1); } else { printf("Set User Password command successful (user %d)\n", - user_id); + user_id); return 0; } } From 728efe3920a60e5f29838417b35277ca623fa40d Mon Sep 17 00:00:00 2001 From: G Dutton <35460590+gdttn@users.noreply.github.com> Date: Mon, 2 Sep 2019 14:22:55 +0100 Subject: [PATCH 4/4] doc: Update man page regarding `user set password` Document password length argument to "set password", as per usage info. Since we don't check whether we're using IPMI1.5/2.0 when setting the password, ipmitool can't reject 20-char passwords being sent to 16-char interfaces, so the behaviour is somewhat undefined. For 20-chars, it's now clearer and long passwords will be rejected. Man page changed to reflect the above. --- doc/ipmitool.1.in | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/doc/ipmitool.1.in b/doc/ipmitool.1.in index cac71322..8f54a59d 100644 --- a/doc/ipmitool.1.in +++ b/doc/ipmitool.1.in @@ -259,11 +259,15 @@ system. It is thus recommended that IPMI password management only be done over IPMIv2.0 \fIlanplus\fP interface or the system interface on the local station. -For IPMI v1.5, the maximum password length is 16 characters. -Passwords longer than 16 characters will be truncated. +For IPMI v1.5, the maximum password length is 16 characters; longer +passwords might be truncated or rejected by the server, or rejected +by +.BR ipmitool . + +For IPMI v2.0, the maximum password length is 20 characters; longer +passwords will be rejected by +.BR ipmitool . -For IPMI v2.0, the maximum password length is 20 characters; -longer passwords are truncated. .SH "COMMANDS" .TP \fIhelp\fP @@ -3526,12 +3530,13 @@ Displays a list of user information for all defined userids. Sets the username associated with the given userid. .TP -\fIpassword\fP <\fBuserid\fR> [<\fBpassword\fR>] +\fIpassword\fP <\fBuserid\fR> [<\fBpassword\fR> [<\fB16|20\fR>]] .br Sets the password for the given userid. If no password is given, the password is cleared (set to the NULL password). Be careful when -removing passwords from administrator\-level accounts. +removing passwords from administrator\-level accounts. If specified, +16 or 20 determines the maximum password length. .RE .TP \fIdisable\fP <\fBuserid\fR>