]> granicus.if.org Git - icinga2/commitdiff
Remove ApiUser password_hash functionality 6387/head
authorMichael Friedrich <michael.friedrich@icinga.com>
Mon, 18 Jun 2018 09:05:56 +0000 (11:05 +0200)
committerMichael Friedrich <michael.friedrich@icinga.com>
Tue, 19 Jun 2018 09:32:03 +0000 (11:32 +0200)
This affects and fixes

- Windows reload
- Config validation
- RHEL 7.5 OpenSSL memory corruption
- Hash algorithm, requested changes

refs #6378
refs #6279
refs #6278

14 files changed:
doc/09-object-types.md
doc/11-cli-commands.md
doc/12-icinga2-api.md
doc/16-upgrading-icinga-2.md
lib/base/tlsutility.cpp
lib/base/tlsutility.hpp
lib/cli/CMakeLists.txt
lib/cli/apiusercommand.cpp [deleted file]
lib/cli/apiusercommand.hpp [deleted file]
lib/remote/apiuser.cpp
lib/remote/apiuser.hpp
lib/remote/apiuser.ti
test/CMakeLists.txt
test/remote-user.cpp [deleted file]

index 1c6436739a0be2c693d1c9e8c0430fe4be33a93a..2d81d3ee3f61f091be3f7e5f378c7b4673614ebc 100644 (file)
@@ -112,7 +112,6 @@ Configuration Attributes:
   Name                      | Type                  | Description
   --------------------------|-----------------------|----------------------------------
   password                  | String                | **Optional.** Password string. Note: This attribute is hidden in API responses.
-  password\_hash            | String                | **Optional.** A hashed password string in the form of /etc/shadow. Note: This attribute is hidden in API responses.
   client\_cn                | String                | **Optional.** Client Common Name (CN).
   permissions               | Array                 | **Required.** Array of permissions. Either as string or dictionary with the keys `permission` and `filter`. The latter must be specified as function.
 
index d288c64f502e1835d3655a17a5a5580665b3e5c4..1b50116838fd0c2bb69eee9b9d9c949e22e256ce 100644 (file)
@@ -20,7 +20,6 @@ Usage:
 
 Supported commands:
   * api setup (setup for API)
-  * api user (API user creation helper)
   * ca list (lists all certificate signing requests)
   * ca sign (signs an outstanding certificate request)
   * console (Icinga console)
index c029aee0e05b78142201ef3b330ab2af3011e754..722b88a2bca9f807957e9e682cc15eaffe7c2077 100644 (file)
@@ -21,25 +21,6 @@ If you prefer to set up the API manually, you will have to perform the following
 
 The next chapter provides a quick overview of how you can use the API.
 
-### Creating ApiUsers <a id="icinga2-api-creating-users"></a>
-
-The CLI command `icinga2 api user` allows you to create an ApiUser object with a hashed password string, ready to be
-added to your configuration. Example:
-
-```
-$ icinga2 api user --user icingaweb2 --password icinga
-object ApiUser "icingaweb2" {
-  password_hash ="$5$d5f1a17ea308acb6$9e9fd5d24a9373a16e8811765cc5a5939687faf9ef8ed496db6e7f1d0ae9b2a9"
-  // client_cn = ""
-
-  permissions = [ "*" ]
-}
-```
-
-Optionally a salt can be provided with `--salt`, otherwise a random value will be used. When ApiUsers are stored this
-way, even somebody able to read the configuration files won't be able to authenticate using this information. There is
-no way to recover your password should you forget it, you'd need to create it anew.
-
 ## Introduction <a id="icinga2-api-introduction"></a>
 
 The Icinga 2 API allows you to manage configuration objects
index 2b8d486bf73424f3033f33c087048521d56449c5..cf509cf33ffc5223629ed5c37853e8fc5488dfe3 100644 (file)
@@ -18,6 +18,11 @@ The CORS attributes `access_control_allow_credentials`, `access_control_allow_he
 
 The `node setup` parameter `--master_host` was deprecated and replaced with `--parent_host`. This parameter is now optional to allow connection-less client setups similar to the `node wizard` CLI command. The `parent_zone` parameter has been added to modify the parent zone name e.g. for client-to-satellite setups.
 
+The `api user` command which was released in v2.8.2 turned out to cause huge problems with
+configuration validation, windows restarts and OpenSSL versions. It is therefore removed in 2.9,
+the `password_hash` attribute for the ApiUser object stays intact but has no effect. This is to ensure
+that clients don't break on upgrade. We will revise this feature in future development iterations.
+
 ## Upgrading to v2.8.2 <a id="upgrading-to-2-8-2"></a>
 
 With version 2.8.2 the location of settings formerly found in `/etc/icinga2/init.conf` has changed. They are now
index 9b3c33fb2a89bd7af6000f3ab0f6fe0260d752b0..c3fe07bd9b57f00c22d91911e0302aebe310a10e 100644 (file)
@@ -784,34 +784,4 @@ std::string to_string(const errinfo_openssl_error& e)
        return "[errinfo_openssl_error]" + tmp.str() + "\n";
 }
 
-bool ComparePassword(const String& hash, const String& password, const String& 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)
-               c |= p1[i] ^ p2[i];
-
-       return (c == 0);
-}
-
-/* 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)
-{
-       // 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));
-}
-
 }
index 31bb4e46683ea67c3a788c6cc9652ffdee72a11d..38df7e58756c53e4b10f6dfb389a8cde2ec41cda 100644 (file)
@@ -56,8 +56,6 @@ String SHA1(const String& s, bool binary = false);
 String SHA256(const String& s);
 String RandomString(int length);
 bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::shared_ptr<X509>& certificate);
-bool ComparePassword(const String& hash, const String& password, const String& Salt);
-String CreateHashedPasswordString(const String& password, const String& salt, int algorithm = 5);
 
 class openssl_error : virtual public std::exception, virtual public boost::exception { };
 
index d92c22c4517110eeaf3ac70a1c2ba6f721cd42e2..258597dec02dc3d9bbed5d47b7b792560e8e891a 100644 (file)
@@ -18,7 +18,6 @@
 set(cli_SOURCES
   i2-cli.hpp
   apisetupcommand.cpp apisetupcommand.hpp
-  apiusercommand.cpp apiusercommand.hpp
   apisetuputility.cpp apisetuputility.hpp
   calistcommand.cpp calistcommand.hpp
   casigncommand.cpp casigncommand.hpp
diff --git a/lib/cli/apiusercommand.cpp b/lib/cli/apiusercommand.cpp
deleted file mode 100644 (file)
index 5bd77ab..0000000
+++ /dev/null
@@ -1,104 +0,0 @@
-/******************************************************************************
- * Icinga 2                                                                   *
- * Copyright (C) 2012-2017 Icinga Development Team (https://www.icinga.com/)  *
- *                                                                            *
- * This program is free software; you can redistribute it and/or              *
- * modify it under the terms of the GNU General Public License                *
- * as published by the Free Software Foundation; either version 2             *
- * of the License, or (at your option) any later version.                     *
- *                                                                            *
- * This program is distributed in the hope that it will be useful,            *
- * but WITHOUT ANY WARRANTY; without even the implied warranty of             *
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the              *
- * GNU General Public License for more details.                               *
- *                                                                            *
- * You should have received a copy of the GNU General Public License          *
- * along with this program; if not, write to the Free Software Foundation     *
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.             *
- ******************************************************************************/
-
-#include "cli/apiusercommand.hpp"
-#include "base/logger.hpp"
-#include "base/tlsutility.hpp"
-#include "base/configwriter.hpp"
-#include "remote/apiuser.hpp"
-#include <iostream>
-
-using namespace icinga;
-namespace po = boost::program_options;
-
-REGISTER_CLICOMMAND("api/user", ApiUserCommand);
-
-String ApiUserCommand::GetDescription(void) const
-{
-       return "Create a hashed user and password string for the Icinga 2 API";
-}
-
-String ApiUserCommand::GetShortDescription(void) const
-{
-       return "API user creation helper";
-}
-
-void ApiUserCommand::InitParameters(boost::program_options::options_description& visibleDesc,
-    boost::program_options::options_description& hiddenDesc) const
-{
-       visibleDesc.add_options()
-               ("user", po::value<std::string>(), "API username")
-               ("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");
-}
-
-/**
- * The entry point for the "api user" CLI command.
- *
- * @returns An exit status.
- */
-int ApiUserCommand::Run(const boost::program_options::variables_map& vm, const std::vector<std::string>& ap) const
-{
-       String passwd, salt;
-       if (!vm.count("user") && !vm.count("oneline")) {
-               Log(LogCritical, "cli", "Username (--user) must be specified.");
-               return 1;
-       }
-
-       if (!vm.count("password")) {
-               Log(LogCritical, "cli", "Password (--password) must be specified.");
-               return 1;
-       }
-
-       passwd = vm["password"].as<std::string>();
-       salt = vm.count("salt") ? String(vm["salt"].as<std::string>()) : RandomString(8);
-
-       if (salt.FindFirstOf('$') != String::NPos) {
-               Log(LogCritical, "cli", "Salt (--salt) may not contain '$'");
-               return 1;
-       }
-
-       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 << std::endl;
-       else {
-               std::cout << "object ApiUser ";
-
-               ConfigWriter::EmitString(std::cout, vm["user"].as<std::string>());
-
-               std::cout << "{\n"
-                       << "  password_hash = ";
-
-               ConfigWriter::EmitString(std::cout, hashedPassword);
-
-               std::cout << "\n"
-                       << "  // client_cn = \"\"\n"
-                       << "\n"
-                       << "  permissions = [ \"*\" ]\n"
-                       << "}\n";
-       }
-
-       return 0;
-}
diff --git a/lib/cli/apiusercommand.hpp b/lib/cli/apiusercommand.hpp
deleted file mode 100644 (file)
index 4a4bfb2..0000000
+++ /dev/null
@@ -1,47 +0,0 @@
-/******************************************************************************
- * Icinga 2                                                                   *
- * Copyright (C) 2012-2017 Icinga Development Team (https://www.icinga.com/)  *
- *                                                                            *
- * This program is free software; you can redistribute it and/or              *
- * modify it under the terms of the GNU General Public License                *
- * as published by the Free Software Foundation; either version 2             *
- * of the License, or (at your option) any later version.                     *
- *                                                                            *
- * This program is distributed in the hope that it will be useful,            *
- * but WITHOUT ANY WARRANTY; without even the implied warranty of             *
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the              *
- * GNU General Public License for more details.                               *
- *                                                                            *
- * You should have received a copy of the GNU General Public License          *
- * along with this program; if not, write to the Free Software Foundation     *
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.             *
- ******************************************************************************/
-
-#ifndef APIUSERCOMMAND_H
-#define APIUSERCOMMAND_H
-
-#include "cli/clicommand.hpp"
-
-namespace icinga
-{
-
-/**
- * The "api user" command.
- *
- * @ingroup cli
- */
-class ApiUserCommand : public CLICommand
-{
-public:
-       DECLARE_PTR_TYPEDEFS(ApiUserCommand);
-
-       virtual String GetDescription(void) const override;
-       virtual String GetShortDescription(void) const override;
-       virtual void InitParameters(boost::program_options::options_description& visibleDesc,
-               boost::program_options::options_description& hiddenDesc) const override;
-       virtual int Run(const boost::program_options::variables_map& vm, const std::vector<std::string>& ap) const override;
-};
-
-}
-
-#endif /* APIUSERCOMMAND_H */
index 02a6efc07a3ee790d6b1facc229984e027feda29..d7b40d13e926ed989bd960d65b2a36979e93f8ed 100644 (file)
@@ -27,18 +27,6 @@ using namespace icinga;
 
 REGISTER_TYPE(ApiUser);
 
-void ApiUser::OnConfigLoaded(void)
-{
-       ObjectImpl<ApiUser>::OnConfigLoaded();
-
-       if (GetPasswordHash().IsEmpty()) {
-               String hashedPassword = CreateHashedPasswordString(GetPassword(), RandomString(8), 5);
-               VERIFY(hashedPassword != String());
-               SetPasswordHash(hashedPassword);
-               SetPassword("********");
-       }
-}
-
 ApiUser::Ptr ApiUser::GetByClientCN(const String& cn)
 {
        for (const ApiUser::Ptr& user : ConfigType::GetObjectsByType<ApiUser>()) {
@@ -75,31 +63,9 @@ ApiUser::Ptr ApiUser::GetByAuthHeader(const String& auth_header)
         */
        if (!user || password.IsEmpty())
                return nullptr;
-       else if (user && user->GetPassword() != password) {
-               Dictionary::Ptr passwordDict = user->GetPasswordDict();
-               if (!passwordDict || !ComparePassword(passwordDict->Get("password"), password, passwordDict->Get("salt")))
-                       return nullptr;
-       }
+       else if (user && user->GetPassword() != password)
+               return nullptr;
 
        return user;
 }
 
-Dictionary::Ptr ApiUser::GetPasswordDict(void) const
-{
-       String password = GetPasswordHash();
-       if (password.IsEmpty() || password[0] != '$')
-               return nullptr;
-
-       String::SizeType saltBegin = password.FindFirstOf('$', 1);
-       String::SizeType passwordBegin = password.FindFirstOf('$', saltBegin+1);
-
-       if (saltBegin == String::NPos || saltBegin == 1 || passwordBegin == String::NPos)
-               return nullptr;
-
-       Dictionary::Ptr passwordDict = new Dictionary();
-       passwordDict->Set("algorithm", password.SubStr(1, saltBegin - 1));
-       passwordDict->Set("salt", password.SubStr(saltBegin + 1, passwordBegin - saltBegin - 1));
-       passwordDict->Set("password", password.SubStr(passwordBegin + 1));
-
-       return passwordDict;
-}
index 44cb84df7f89d284586a6e0c86485cb09cd3fca8..15b1c41e371b7a9ea28dc5a4cc19867c6d4d872f 100644 (file)
@@ -35,12 +35,8 @@ public:
        DECLARE_OBJECT(ApiUser);
        DECLARE_OBJECTNAME(ApiUser);
 
-       virtual void OnConfigLoaded(void) override;
-
        static ApiUser::Ptr GetByClientCN(const String& cn);
        static ApiUser::Ptr GetByAuthHeader(const String& auth_header);
-
-       Dictionary::Ptr GetPasswordDict(void) const;
 };
 
 }
index a767f833a09dd96b0f02bceb7d6dd44f01f3ae1f..36ec9870460fe51f08726c753b8242dc6f7a1b8a 100644 (file)
@@ -29,7 +29,7 @@ class ApiUser : ConfigObject
 {
        /* No show config */
        [no_user_view, no_user_modify] String password;
-       [config, no_user_view] String password_hash;
+       [deprecated, config, no_user_view] String password_hash;
        [config] String client_cn (ClientCN);
        [config] array(Value) permissions;
 };
index 29a16646f8f79dc28c5306c0ca1719ff1f4838cb..78955005291caf32b6b39228f5d9167a2b1b15a4 100644 (file)
@@ -43,7 +43,6 @@ set(base_test_SOURCES
   icinga-notification.cpp
   icinga-perfdata.cpp
   remote-url.cpp
-  remote-user.cpp
   ${base_OBJS}
   $<TARGET_OBJECTS:config>
   $<TARGET_OBJECTS:remote>
@@ -142,7 +141,6 @@ add_boost_test(base
     remote_url/get_and_set
     remote_url/format
     remote_url/illegal_legal_strings
-    api_user/password
 )
 
 if(ICINGA2_WITH_LIVESTATUS)
diff --git a/test/remote-user.cpp b/test/remote-user.cpp
deleted file mode 100644 (file)
index f43816a..0000000
+++ /dev/null
@@ -1,50 +0,0 @@
-/******************************************************************************
- * Icinga 2                                                                   *
- * Copyright (C) 2012-2017 Icinga Development Team (https://www.icinga.com/)  *
- *                                                                            *
- * This program is free software; you can redistribute it and/or              *
- * modify it under the terms of the GNU General Public License                *
- * as published by the Free Software Foundation; either version 2             *
- * of the License, or (at your option) any later version.                     *
- *                                                                            *
- * This program is distributed in the hope that it will be useful,            *
- * but WITHOUT ANY WARRANTY; without even the implied warranty of             *
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the              *
- * GNU General Public License for more details.                               *
- *                                                                            *
- * You should have received a copy of the GNU General Public License          *
- * along with this program; if not, write to the Free Software Foundation     *
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.             *
- ******************************************************************************/
-
-#include "remote/apiuser.hpp"
-#include "base/tlsutility.hpp"
-#include <BoostTestTargetConfig.h>
-
-#include <iostream>
-
-using namespace icinga;
-
-BOOST_AUTO_TEST_SUITE(api_user)
-
-BOOST_AUTO_TEST_CASE(password)
-{
-       ApiUser::Ptr user = new ApiUser();
-       String passwd = RandomString(16);
-       String salt = RandomString(8);
-       user->SetPasswordHash(CreateHashedPasswordString(passwd, salt));
-       user->OnConfigLoaded();
-       user->OnAllConfigLoaded();
-       user->Start();
-
-       BOOST_CHECK(user->GetPasswordHash() != passwd);
-
-       Dictionary::Ptr passwdd = user->GetPasswordDict();
-
-       BOOST_CHECK(passwdd);
-       BOOST_CHECK(passwdd->Get("salt") == salt);
-       BOOST_CHECK(ComparePassword(passwdd->Get("password"), passwd, salt));
-       BOOST_CHECK(!ComparePassword(passwdd->Get("password"), "wrong password uwu!", salt));
-}
-
-BOOST_AUTO_TEST_SUITE_END()