From eb83f7ce842944518bac726c19eb071257a2ed56 Mon Sep 17 00:00:00 2001 From: Pascal Mathis Date: Sat, 12 May 2018 20:19:49 +0200 Subject: lib: Improved warnings for 'no (enable) password' When the user executes one of the commands 'no password' or 'no enable password', a warning message gets shown to inform the user of the security implications. While the current implementation works, a warning message gets printed once for each daemon, which can lead to seeing the same message many times. This does not affect functionality, but looks like an error to the user as it can be seen within issue #1432. This commit only prints the warning message inside lib when vtysh dispatch is not being used. Additionally, the warning message was copied into the vtysh command handlers, so that they get printed exactly once. Signed-off-by: Pascal Mathis --- lib/command.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) (limited to 'lib/command.c') diff --git a/lib/command.c b/lib/command.c index 69e301fcfa..3761f444bc 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1960,19 +1960,23 @@ DEFUN (no_config_password, bool warned = false; if (host.password) { - vty_out(vty, - "Please be aware that removing the password is a security risk and " - "you should think twice about this command\n"); - warned = true; + if (!vty_shell_serv(vty)) { + vty_out(vty, + "Please be aware that removing the password is " + "a security risk and you should think twice " + "about this command\n"); + warned = true; + } XFREE(MTYPE_HOST, host.password); } host.password = NULL; if (host.password_encrypt) { - if (!warned) + if (!warned && !vty_shell_serv(vty)) vty_out(vty, - "Please be aware that removing the password is a security risk " - "and you should think twice about this command\n"); + "Please be aware that removing the password is " + "a security risk and you should think twice " + "about this command\n"); XFREE(MTYPE_HOST, host.password_encrypt); } host.password_encrypt = NULL; @@ -2044,19 +2048,23 @@ DEFUN (no_config_enable_password, bool warned = false; if (host.enable) { - vty_out(vty, - "Please be aware that removing the password is a security risk and " - "you should think twice about this command\n"); - warned = true; + if (!vty_shell_serv(vty)) { + vty_out(vty, + "Please be aware that removing the password is " + "a security risk and you should think twice " + "about this command\n"); + warned = true; + } XFREE(MTYPE_HOST, host.enable); } host.enable = NULL; if (host.enable_encrypt) { - if (!warned) + if (!warned && !vty_shell_serv(vty)) vty_out(vty, - "Please be aware that removing the password is a security risk " - "and you should think twice about this command\n"); + "Please be aware that removing the password is " + "a security risk and you should think twice " + "about this command\n"); XFREE(MTYPE_HOST, host.enable_encrypt); } host.enable_encrypt = NULL; -- cgit v1.2.3 From 4911ca9cab5d75b5031edb83b52423ed47798324 Mon Sep 17 00:00:00 2001 From: Pascal Mathis Date: Sun, 13 May 2018 19:11:43 +0200 Subject: lib: Moved no-password warnings into header file The warning string which appears when the users executes 'no (enable) password' was moved into command.h and declared as a constant named 'NO_PASSWD_CMD_WARNING'. This avoids duplicate code and makes it easy to change the warning message in all places at once. Signed-off-by: Pascal Mathis --- lib/command.c | 20 ++++---------------- lib/command.h | 4 ++++ vtysh/vtysh.c | 8 ++------ 3 files changed, 10 insertions(+), 22 deletions(-) (limited to 'lib/command.c') diff --git a/lib/command.c b/lib/command.c index 3761f444bc..0fa6bde334 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1961,10 +1961,7 @@ DEFUN (no_config_password, if (host.password) { if (!vty_shell_serv(vty)) { - vty_out(vty, - "Please be aware that removing the password is " - "a security risk and you should think twice " - "about this command\n"); + vty_out(vty, NO_PASSWD_CMD_WARNING); warned = true; } XFREE(MTYPE_HOST, host.password); @@ -1973,10 +1970,7 @@ DEFUN (no_config_password, if (host.password_encrypt) { if (!warned && !vty_shell_serv(vty)) - vty_out(vty, - "Please be aware that removing the password is " - "a security risk and you should think twice " - "about this command\n"); + vty_out(vty, NO_PASSWD_CMD_WARNING); XFREE(MTYPE_HOST, host.password_encrypt); } host.password_encrypt = NULL; @@ -2049,10 +2043,7 @@ DEFUN (no_config_enable_password, if (host.enable) { if (!vty_shell_serv(vty)) { - vty_out(vty, - "Please be aware that removing the password is " - "a security risk and you should think twice " - "about this command\n"); + vty_out(vty, NO_PASSWD_CMD_WARNING); warned = true; } XFREE(MTYPE_HOST, host.enable); @@ -2061,10 +2052,7 @@ DEFUN (no_config_enable_password, if (host.enable_encrypt) { if (!warned && !vty_shell_serv(vty)) - vty_out(vty, - "Please be aware that removing the password is " - "a security risk and you should think twice " - "about this command\n"); + vty_out(vty, NO_PASSWD_CMD_WARNING); XFREE(MTYPE_HOST, host.enable_encrypt); } host.enable_encrypt = NULL; diff --git a/lib/command.h b/lib/command.h index 9ba53e0907..8d9c39b0ea 100644 --- a/lib/command.h +++ b/lib/command.h @@ -376,6 +376,10 @@ struct cmd_node { #define CONF_BACKUP_EXT ".sav" +/* Command warnings. */ +#define NO_PASSWD_CMD_WARNING \ + "Please be aware that removing the password is a security risk and you should think twice about this command.\n" + /* IPv4 only machine should not accept IPv6 address for peer's IP address. So we replace VTY command string like below. */ #define NEIGHBOR_ADDR_STR "Neighbor address\nIPv6 address\n" diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 8403912ea3..9fff2ee58c 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2372,9 +2372,7 @@ DEFUNSH(VTYSH_ALL, no_vtysh_config_password, no_vtysh_password_cmd, "no password", NO_STR "Modify the terminal connection password\n") { - vty_out(vty, - "Please be aware that removing the password is a security risk " - "and you should think twice about this command\n"); + vty_out(vty, NO_PASSWD_CMD_WARNING); return CMD_SUCCESS; } @@ -2394,9 +2392,7 @@ DEFUNSH(VTYSH_ALL, no_vtysh_config_enable_password, "Modify enable password parameters\n" "Assign the privileged level password\n") { - vty_out(vty, - "Please be aware that removing the password is a security risk " - "and you should think twice about this command\n"); + vty_out(vty, NO_PASSWD_CMD_WARNING); return CMD_SUCCESS; } -- cgit v1.2.3