commit 9fc5fa180c3dc0a4fa5ed02efe4b990c40ba641d
author jayaprakash Mutyala <mutyalax.jayaprakash@intel.com> Thu Aug 29 15:14:06 2019 +0000
committer Vernon Mauery <vernon.mauery@linux.intel.com> Sun Nov 10 15:29:46 2019 +0000
tree 7d3dbda7d100345cb416c13076b599d91ba28e0b
parent 5aa2693770ea4bb17eb581ab9d5274e7a1edf862

user_mgmt:password authentication/update by PAM

Allowed password characters will be handled by password management,
instead of set user password command. This makes the checking to be
moved from set user password command to pam layer and accordingly
throw error, when the same is not valid.

Fix: So modified the code to handle with pam module itself.

Tested:
ipmitool user set password 6 0penBmc1\'
Set User Password command successful (user 6)

ipmitool user set password 6 0penBmc1\"
Set User Password command successful (user 6)

ipmitool user set password 6 12345678 //invalid password
IPMI command failed: Invalid data field in request
Set User Password command failed (user 6)

ipmitool user set password 3 asdf1234 //user id does not exit
IPMI command failed: Unspecified error
Set User Password command failed (user 3)

Signed-off-by: jayaprakash Mutyala <mutyalax.jayaprakash@intel.com>
Change-Id: Iba6e2c29a927d53e6ebdb5d32e83ecc7cbbd2fd0

user_channel/user_mgmt.cpp

diff --git a/user_channel/user_mgmt.cpp b/user_channel/user_mgmt.cpp
index aa20c2b..6b9d1d3 100644
--- a/user_channel/user_mgmt.cpp
+++ b/user_channel/user_mgmt.cpp
@@ -682,38 +682,28 @@
  *  @return status
  */
 
-bool pamUpdatePasswd(const char* username, const char* password)
+int pamUpdatePasswd(const char* username, const char* password)
 {
     const struct pam_conv localConversation = {pamFunctionConversation,
                                                const_cast<char*>(password)};
     pam_handle_t* localAuthHandle = NULL; // this gets set by pam_start
 
-    if (pam_start("passwd", username, &localConversation, &localAuthHandle) !=
-        PAM_SUCCESS)
-    {
-        return false;
-    }
-    int retval = pam_chauthtok(localAuthHandle, PAM_SILENT);
+    int retval =
+        pam_start("passwd", username, &localConversation, &localAuthHandle);
 
     if (retval != PAM_SUCCESS)
     {
-        if (retval == PAM_AUTHTOK_ERR)
-        {
-            log<level::DEBUG>("Authentication Failure");
-        }
-        else
-        {
-            log<level::DEBUG>("pam_chauthtok returned failure",
-                              entry("ERROR=%d", retval));
-        }
-        pam_end(localAuthHandle, retval);
-        return false;
+        return retval;
     }
-    if (pam_end(localAuthHandle, PAM_SUCCESS) != PAM_SUCCESS)
+
+    retval = pam_chauthtok(localAuthHandle, PAM_SILENT);
+    if (retval != PAM_SUCCESS)
     {
-        return false;
+        pam_end(localAuthHandle, retval);
+        return retval;
     }
-    return true;
+
+    return pam_end(localAuthHandle, PAM_SUCCESS);
 }
 
 bool pamUserCheckAuthenticate(std::string_view username,
@@ -760,7 +750,7 @@
 ipmi_ret_t UserAccess::setSpecialUserPassword(const std::string& userName,
                                               const std::string& userPassword)
 {
-    if (!pamUpdatePasswd(userName.c_str(), userPassword.c_str()))
+    if (pamUpdatePasswd(userName.c_str(), userPassword.c_str()) != PAM_SUCCESS)
     {
         log<level::DEBUG>("Failed to update password");
         return IPMI_CC_UNSPECIFIED_ERROR;
@@ -772,29 +762,36 @@
                                        const char* userPassword)
 {
     std::string userName;
-    if (ipmiUserGetUserName(userId, userName) != IPMI_CC_OK)
+    if (ipmiUserGetUserName(userId, userName) != ipmi::ccSuccess)
     {
         log<level::DEBUG>("User Name not found",
                           entry("USER-ID=%d", (uint8_t)userId));
-        return IPMI_CC_PARM_OUT_OF_RANGE;
+        return ipmi::ccParmOutOfRange;
     }
     std::string passwd;
     passwd.assign(reinterpret_cast<const char*>(userPassword), 0,
                   maxIpmi20PasswordSize);
-    if (!std::regex_match(passwd.c_str(),
-                          std::regex("[a-zA-z_0-9][a-zA-Z_0-9,?:`!\"]*")))
+
+    int retval = pamUpdatePasswd(userName.c_str(), passwd.c_str());
+
+    switch (retval)
     {
-        log<level::DEBUG>("Invalid password fields",
-                          entry("USER-ID=%d", (uint8_t)userId));
-        return IPMI_CC_INVALID_FIELD_REQUEST;
+        case PAM_SUCCESS:
+        {
+            return ipmi::ccSuccess;
+        }
+        case PAM_AUTHTOK_ERR:
+        {
+            log<level::DEBUG>("Bad authentication token");
+            return ipmi::ccInvalidFieldRequest;
+        }
+        default:
+        {
+            log<level::DEBUG>("Failed to update password",
+                              entry("USER-ID=%d", (uint8_t)userId));
+            return ipmi::ccUnspecifiedError;
+        }
     }
-    if (!pamUpdatePasswd(userName.c_str(), passwd.c_str()))
-    {
-        log<level::DEBUG>("Failed to update password",
-                          entry("USER-ID=%d", (uint8_t)userId));
-        return IPMI_CC_UNSPECIFIED_ERROR;
-    }
-    return IPMI_CC_OK;
 }
 
 ipmi_ret_t UserAccess::setUserEnabledState(const uint8_t userId,

user_channel/user_mgmt.hpp

diff --git a/user_channel/user_mgmt.hpp b/user_channel/user_mgmt.hpp
index 773b18d..0c38374 100644
--- a/user_channel/user_mgmt.hpp
+++ b/user_channel/user_mgmt.hpp
@@ -16,12 +16,11 @@
 #pragma once
 #include "user_layer.hpp"
 
-#include <ipmid/api.h>
-
 #include <boost/interprocess/sync/file_lock.hpp>
 #include <boost/interprocess/sync/named_recursive_mutex.hpp>
 #include <cstdint>
 #include <ctime>
+#include <ipmid/api.hpp>
 #include <sdbusplus/bus.hpp>
 #include <variant>