From f357439485e4f2d57dda055b99a3496f6fd6a160 Mon Sep 17 00:00:00 2001
From: Jean Flach <jean-marcel.flach@icinga.com>
Date: Fri, 16 Feb 2018 10:31:00 +0100
Subject: [PATCH] Code style

---
 lib/base/tlsutility.cpp    | 24 +++++++++++++++---------
 lib/base/tlsutility.hpp    |  4 ++--
 lib/cli/apiusercommand.cpp | 12 ++++++++----
 lib/remote/apiuser.cpp     |  8 ++++++--
 4 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp
index 04d8ea49f..c28f8e44f 100644
--- a/lib/base/tlsutility.cpp
+++ b/lib/base/tlsutility.cpp
@@ -761,28 +761,34 @@ bool VerifyCertificate(const boost::shared_ptr<X509>& caCertificate, const boost
 	return rc == 1;
 }
 
-bool ComparePassword(const String hash, const String password, const String salt)
+bool ComparePassword(const String& hash, const String& password, const String& salt)
 {
-	String otherHash = HashPassword(password, salt);
+	String otherHash = PBKDF2_SHA256(password, salt, 1000);
+	VERIFY(otherHash.GetLength() == 64 && hash.GetLength() == 64);
 
 	const char *p1 = otherHash.CStr();
 	const char *p2 = hash.CStr();
 
+	/* By Novelocrat, https://stackoverflow.com/a/25374036 */
 	volatile char c = 0;
 
-	for (size_t i=0; i<64; ++i)
+	for (size_t i = 0; i < 64; ++i)
 		c |= p1[i] ^ p2[i];
 
 	return (c == 0);
 }
 
-String HashPassword(const String& password, const String& salt, const bool shadow)
+/* Returns a String in the format $algorithm$salt$hash or returns an empty string in case of an error */
+String CreateHashedPasswordString(const String& password, const String& salt, int algorithm)
 {
-	if (shadow)
-		//Using /etc/shadow password format. The 5 means SHA256 is being used
-		return String("$5$" + salt + "$" + PBKDF2_SHA256(password, salt, 1000));
-	else
-		return PBKDF2_SHA256(password, salt, 1000);
+	// We currently only support SHA256
+	if (algorithm != 5)
+		return String();
+
+	if (salt.FindFirstOf('$') != String::NPos)
+		return String();
+
+	return String("$5$" + salt + "$" + PBKDF2_SHA256(password, salt, 1000));
 }
 
 }
diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp
index 96bdc63b0..6e5c68d29 100644
--- a/lib/base/tlsutility.hpp
+++ b/lib/base/tlsutility.hpp
@@ -57,8 +57,8 @@ String I2_BASE_API SHA1(const String& s, bool binary = false);
 String I2_BASE_API SHA256(const String& s);
 String I2_BASE_API RandomString(int length);
 bool I2_BASE_API VerifyCertificate(const boost::shared_ptr<X509>& caCertificate, const boost::shared_ptr<X509>& certificate);
-bool I2_BASE_API ComparePassword(const String hash, const String password, const String Salt);
-String I2_BASE_API HashPassword(const String& password, const String& salt, const bool shadow = false);
+bool ComparePassword(const String& hash, const String& password, const String& Salt);
+String CreateHashedPasswordString(const String& password, const String& salt, int algorithm = 5);
 
 class I2_BASE_API openssl_error : virtual public std::exception, virtual public boost::exception { };
 
diff --git a/lib/cli/apiusercommand.cpp b/lib/cli/apiusercommand.cpp
index 6eb5c3bf3..188691ae0 100644
--- a/lib/cli/apiusercommand.cpp
+++ b/lib/cli/apiusercommand.cpp
@@ -44,7 +44,7 @@ void ApiUserCommand::InitParameters(boost::program_options::options_description&
 {
 	visibleDesc.add_options()
 		("user", po::value<std::string>(), "API username")
-		("passwd", po::value<std::string>(), "Password in clear text")
+		("password", po::value<std::string>(), "Password in clear text")
 		("salt", po::value<std::string>(), "Optional salt (default: 8 random chars)")
 		("oneline", "Print only the password hash");
 }
@@ -63,8 +63,8 @@ int ApiUserCommand::Run(const boost::program_options::variables_map& vm, const s
 	} else
 		user = vm["user"].as<std::string>();
 
-	if (!vm.count("passwd")) {
-		Log(LogCritical, "cli", "Password (--passwd) must be specified.");
+	if (!vm.count("password")) {
+		Log(LogCritical, "cli", "Password (--password) must be specified.");
 		return 1;
 	}
 
@@ -76,7 +76,11 @@ int ApiUserCommand::Run(const boost::program_options::variables_map& vm, const s
 		return 1;
 	}
 
-	String hashedPassword = HashPassword(passwd, salt, true);
+	String hashedPassword = CreateHashedPasswordString(passwd, salt, 5);
+	if (hashedPassword == String()) {
+		Log(LogCritical, "cli") << "Failed to hash password \"" << passwd << "\" with salt \"" << salt << "\"";
+		return 1;
+	}
 
 	if (vm.count("oneline"))
 		std::cout << '"' << hashedPassword << "\"\n";
diff --git a/lib/remote/apiuser.cpp b/lib/remote/apiuser.cpp
index 606e66dcc..f3e2215ef 100644
--- a/lib/remote/apiuser.cpp
+++ b/lib/remote/apiuser.cpp
@@ -31,8 +31,12 @@ void ApiUser::OnConfigLoaded(void)
 {
 	ObjectImpl<ApiUser>::OnConfigLoaded();
 
-	if (this->GetPasswordHash().IsEmpty())
-		SetPasswordHash(HashPassword(GetPassword(), RandomString(8), true));
+	if (GetPasswordHash().IsEmpty()) {
+		String hashedPassword =	CreateHashedPasswordString(GetPassword(), RandomString(8), 5);
+		VERIFY(hashedPassword != String());
+		SetPasswordHash(hashedPassword);
+		SetPassword("********");
+	}
 }
 
 ApiUser::Ptr ApiUser::GetByClientCN(const String& cn)
-- 
2.40.0