Fix isPwdValid. Correct logs. Change history related function names.

[Issue#]        N/A
[Bug]           Wrong value returned by isPwdValid. Some logs showed up incorrectly as error.
                History related functions had misleading names.
[Cause]         Incorrect casting in isPwdValid.
[Solution]      Add condition correcting returned time by isPwdValid. Correct log types. Change
                historySize related functions to maxHistorySize.
[Verification]  Build, run tests. Test tc43 should pass, others should work as earlier. Commit with
                tc43 - http://slp-info.sec.samsung.net/gerrit/#/c/357776/

Change-Id: Ic9ce0423f2ec233f3c1d8703dba8ab92e998632d

diff --git a/src/server/common/protocols.cpp b/src/server/common/protocols.cpp
index 4f03c9f..8118e2e 100644 (file)
--- a/src/server/common/protocols.cpp
+++ b/src/server/common/protocols.cpp
@@ -61,6 +61,7 @@ const unsigned int MAX_PASSWORD_HISTORY = 50;
 const unsigned int PASSWORD_INFINITE_EXPIRATION_DAYS = 0;
 const time_t PASSWORD_INFINITE_EXPIRATION_TIME = 0xFFFFFFFF;
 const unsigned int PASSWORD_INFINITE_ATTEMPT_COUNT = 0;
+const unsigned int PASSWORD_API_NO_EXPIRATION = 0xFFFFFFFF;
 
 const int SECURITY_SERVER_MAX_OBJ_NAME = 30;
 

diff --git a/src/server/common/protocols.h b/src/server/common/protocols.h
index b663972..231f28d 100644 (file)
--- a/src/server/common/protocols.h
+++ b/src/server/common/protocols.h
@@ -80,6 +80,7 @@ extern const unsigned int MAX_PASSWORD_HISTORY;
 extern const unsigned int PASSWORD_INFINITE_EXPIRATION_DAYS;
 extern const time_t PASSWORD_INFINITE_EXPIRATION_TIME;
 extern const unsigned int PASSWORD_INFINITE_ATTEMPT_COUNT;
+extern const unsigned int PASSWORD_API_NO_EXPIRATION;
 
 extern const int SECURITY_SERVER_MAX_OBJ_NAME;
 

diff --git a/src/server/service/password-file.cpp b/src/server/service/password-file.cpp
index bd7254b..87c1f32 100644 (file)
--- a/src/server/service/password-file.cpp
+++ b/src/server/service/password-file.cpp
@@ -68,7 +68,8 @@ namespace SecurityServer
         Serialization::Serialize(stream, m_password);
     }
 
-    PasswordFile::PasswordFile(): m_maxAttempt(PASSWORD_INFINITE_ATTEMPT_COUNT), m_historySize(0),
+    PasswordFile::PasswordFile(): m_maxAttempt(PASSWORD_INFINITE_ATTEMPT_COUNT),
+                                  m_maxHistorySize(0),
                                   m_expireTime(PASSWORD_INFINITE_EXPIRATION_TIME),
                                   m_passwordActive(false), m_attempt(0)
     {
@@ -159,12 +160,13 @@ namespace SecurityServer
     {
         PasswordFileBuffer pwdBuffer;
 
-        LogError("Saving max_att: " << m_maxAttempt << ", history_size: " << m_historySize <<
-                 ", m_expireTime: " << m_expireTime << ", isActive: " << m_passwordActive);
+        LogSecureDebug("Saving max_att: " << m_maxAttempt << ", history_size: " <<
+                       m_maxHistorySize << ", m_expireTime: " << m_expireTime << ", isActive: " <<
+                       m_passwordActive);
 
         //serialize password attributes
         Serialization::Serialize(pwdBuffer, m_maxAttempt);
-        Serialization::Serialize(pwdBuffer, m_historySize);
+        Serialization::Serialize(pwdBuffer, m_maxHistorySize);
         Serialization::Serialize(pwdBuffer, m_expireTime);
         Serialization::Serialize(pwdBuffer, m_passwordActive);
         Serialization::Serialize(pwdBuffer, m_passwords);
@@ -181,13 +183,14 @@ namespace SecurityServer
         m_passwords.clear();
 
         Deserialization::Deserialize(pwdFile, m_maxAttempt);
-        Deserialization::Deserialize(pwdFile, m_historySize);
+        Deserialization::Deserialize(pwdFile, m_maxHistorySize);
         Deserialization::Deserialize(pwdFile, m_expireTime);
         Deserialization::Deserialize(pwdFile, m_passwordActive);
         Deserialization::Deserialize(pwdFile, m_passwords);
 
-        LogError("Received max_att: " << m_maxAttempt << ", history_size: " << m_historySize <<
-                 ", m_expireTime: " << m_expireTime << ", isActive: " << m_passwordActive);
+        LogSecureDebug("Loaded max_att: " << m_maxAttempt << ", history_size: " <<
+                       m_maxHistorySize << ", m_expireTime: " << m_expireTime << ", isActive: " <<
+                       m_passwordActive);
     }
 
     void PasswordFile::writeAttemptToFile() const
@@ -226,19 +229,19 @@ namespace SecurityServer
         return m_passwordActive;
     }
 
-    void PasswordFile::setHistory(unsigned int history)
+    void PasswordFile::setMaxHistorySize(unsigned int history)
     {
         //setting history should be independent from password being set
-        m_historySize = history;
+        m_maxHistorySize = history;
 
         //we want to keep 1 current pwd, plus history amount of passwords.
         if(m_passwords.size() > 1+history)
             m_passwords.resize(1+history);
     }
 
-    unsigned int PasswordFile::getHistorySize() const
+    unsigned int PasswordFile::getMaxHistorySize() const
     {
-        return m_historySize;
+        return m_maxHistorySize;
     }
 
     unsigned int PasswordFile::getAttempt() const
@@ -270,7 +273,8 @@ namespace SecurityServer
     {
         RawHash hashedPwd = hashPassword(password);
 
-        LogSecureDebug("PwdCount: " << m_passwords.size() << ", PwdMaxHistory: " << getHistorySize());
+        LogSecureDebug("Checking if pwd is reused. PwdCount: " << m_passwords.size() <<
+                       ", PwdMaxHistory: " << getMaxHistorySize());
 
         auto history_beginning = (m_passwords.begin())++;
 
@@ -292,7 +296,7 @@ namespace SecurityServer
         m_passwords.push_front(Password(hashedPwd));
 
         //one current password, plus history amount of passwords
-        if(m_passwords.size() > 1+getHistorySize())
+        if(m_passwords.size() > 1+getMaxHistorySize())
             m_passwords.pop_back();
     }
 
@@ -313,17 +317,13 @@ namespace SecurityServer
         }
     }
 
-    time_t PasswordFile::getExpireTime() const
+    unsigned int PasswordFile::getExpireTimeLeft() const
     {
-        return m_expireTime;
-    }
-
-    time_t PasswordFile::getExpireTimeLeft() const
-    {
-        if(m_expireTime != PASSWORD_INFINITE_EXPIRATION_TIME)
-            return (m_expireTime - time(NULL));
-        else
-            return m_expireTime;
+        if(m_expireTime != PASSWORD_INFINITE_EXPIRATION_TIME) {
+            time_t timeLeft = m_expireTime - time(NULL);
+            return (timeLeft < 0) ? 0 : static_cast<unsigned int>(timeLeft);
+        } else
+            return PASSWORD_API_NO_EXPIRATION;
     }
 
     bool PasswordFile::checkExpiration() const
@@ -349,7 +349,7 @@ namespace SecurityServer
 
     bool PasswordFile::isHistoryActive() const
     {
-        return (m_historySize != 0);
+        return (m_maxHistorySize != 0);
     }
 
     //hashPassword is also used in Password struct constructor, that's why it's static. Moreover

diff --git a/src/server/service/password-file.h b/src/server/service/password-file.h
index a122108..0c385f1 100644 (file)
--- a/src/server/service/password-file.h
+++ b/src/server/service/password-file.h
@@ -50,11 +50,10 @@ namespace SecurityServer
         void activatePassword();
         bool isPasswordActive() const;
 
-        void setHistory(unsigned int history);
-        unsigned int getHistorySize() const;
+        void setMaxHistorySize(unsigned int history);
+        unsigned int getMaxHistorySize() const;
 
-        time_t getExpireTime() const;
-        time_t getExpireTimeLeft() const;
+        unsigned int getExpireTimeLeft() const;
         void setExpireTime(int expireTime);
 
         //attempt manipulating functions
@@ -104,7 +103,7 @@ namespace SecurityServer
         //password file data
         PasswordList m_passwords;
         unsigned int m_maxAttempt;
-        unsigned int m_historySize;
+        unsigned int m_maxHistorySize;
         time_t m_expireTime;
         bool m_passwordActive;
 

diff --git a/src/server/service/password-manager.cpp b/src/server/service/password-manager.cpp
index 853513e..cf9c3a1 100644 (file)
--- a/src/server/service/password-manager.cpp
+++ b/src/server/service/password-manager.cpp
@@ -155,7 +155,7 @@ namespace SecurityServer
         //check delivered currentPassword
         //when m_passwordActive flag is true, currentPassword shouldn't be empty
         if (currentPassword.empty() && m_pwdFile.isPasswordActive()) {
-            LogError("Password is already set. History count: " << m_pwdFile.getHistorySize());
+            LogError("Password is already set. Max history: " << m_pwdFile.getMaxHistorySize());
             return SECURITY_SERVER_API_ERROR_PASSWORD_EXIST;
         }
 
@@ -268,7 +268,7 @@ namespace SecurityServer
             return SECURITY_SERVER_API_ERROR_PASSWORD_RETRY_TIMER;
         }
 
-        m_pwdFile.setHistory(history);
+        m_pwdFile.setMaxHistorySize(history);
         m_pwdFile.writeMemoryToFile();
 
         return SECURITY_SERVER_API_SUCCESS;