Allow to hash/encrypt credentials or use an external storage

[dnsmichi]

Targets

Any credentials stored in the configuration files.

• ApiUser password
• User defined custom attributes used for command arguments
• User defined constants applied to object attributes

Ideas

• Hash passwords and only store the hashes
• Encryption, something like Puppet's EYAML: https://puppet.com/blog/encrypt-your-data-using-hiera-eyaml
• External auth providers, such as Vault

Impact Analysis

• Would this slow/halt config compilation/validation?
• Evaluated at runtime only or pre-generated on the node which has access to external sources?
• Simplified user interface possible? E.g. use a function, specific DSL variable, another extension
• Security, disallowing to exploit this via DSL or at runtime
• Distributed setups: Who's the authority of the credential value
• REST API: How to set values at runtime, and via Director

Tasks

• [ ] Common hash algorithm (#6278)
• [ ] Creation/Encryption via CLI command
• [ ] Hide credentials on the CLI and API (needs a flag in .ti)
• [ ] Default handling of hashes or plain text (don't do this in OnConfigLoaded with OpenSSL functions)
• [ ] Discuss this idea as password store with other feature requests

[Crunsher]

The algorithm used was PBKDF2 following https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet

This is still the best algorithm to use as it's implemented by openssl 1.0.2.

[Al2Klimov]

There was 6504606e23c9ce5f748925dc7445c0053d0f31e8 in v2.8.x which just has not been included in v2.9.0. What was the problem with that change?

[dnsmichi]

#6387 and #6278. Apart from that, this feature was introduced in a bug fix release (2.8.2) and was not tested well enough.

[justinmueller05082000]

I found a tool that has a similar output to crypt. It is reasonable that a Linux user to install the tool.

jmueller@nb-jmueller:~$ openssl passwd -5 -salt saltvalue password
$5$saltvalue$rtc73UxmvUVY.aTgGqMbSvxXR7GSMnKoYEg6i3RRP0.
jmueller@nb-jmueller:~$ 

@lippserd Should we hash our passwords like that?

[lippserd]

Yeah, looks good. It is ok for me that users have to generate password hashes on their own. We should document example commands with sane args though. We just have to support all key derivation functions supported by crypt (and openssl passwd).

[Al2Klimov]

@justinmueller05082000 Write a C++ program which one calls like this:

./theprogramfile password '$5$saltvalue$rtc73UxmvUVY.aTgGqMbSvxXR7GSMnKoYEg6i3RRP0.'

... and it exits with 0 – i.e. an immediately subsequent echo $? prints 0.

./theprogramfile wrongpassword '$5$saltvalue$rtc73UxmvUVY.aTgGqMbSvxXR7GSMnKoYEg6i3RRP0.'

... shall exit with 1.

Paste the source code as comment here.

[justinmueller05082000]

#include <crypt.h>
#include <cstring>

int main(int argc, char *argv[]) {
    if (strcmp(argv[2], crypt(argv[1], argv[2])) == 0) {
        return 0;
    } else {
        return 1;
    }
}

The program takes two command line parameters and check if the password ( Argument 1) in plain text matches the hashed password ( Argument 2). To do that it converts the plain text into a hashed one and compare both with each other. If both passwords are equal the program returns 0 otherwise 1.

[Al2Klimov]

• [x] Fork this repo
• [x] In your fork create a new branch (same scheme as in your Go sort, also refs and fixes)
• On this branch:
  • [x] Add a new attribute "password_hash" to the ApiUser class
  • [x] Make use of this attribute (if set) instead of the plaintext attribute "password"
• [x] Create a PR and let me review

[JGalego]

+1

[Al2Klimov]

@lippserd Is the following reasonable for a user?

$ htpasswd -bnBC 10 '' 123456 |tr -d ':\n'
$2y$10$Se/Zy0gzbMuuS4792CAdYuStiKMf9T3A3BfYs9SixginUlOmOV1u6
$

[lippserd]

Why should we do that? 😆

[Al2Klimov]

To hash a password via bcrypt (available to C++ on *nix and WinNT!) w/o having Icinga 2 installed locally.

[lippserd]

Just use the openssl example. We really should not break our head with other commands.

[Al2Klimov]

Unfortunately the generic crypt(3) isn't easily available on WinNT. bcrypt is available, secure and the CLI hasher htpasswd is also used for Apache and nginx. The only keed-to-know variable in my example is the password (123456).

[Al2Klimov]

💡 Just answered my own question: If it's reasonable enough for both Apache and nginx, it's also reasonable enough for us.

[MarcusCaepio]

Hi all, what about this issue? As icinga2 follows the principle "monitoring as code" it is not unlikely to handle this code in git and deploy it with CI/CD mechanics. As saving passwords in git is a bad idea, there are workarounds like constants out of the /etc/icinga2 directory ( see https://community.icinga.com/t/hide-credentials-from-icinga2-config-files/6514 ). But handling these constant files in an environment with +1000 hosts is not enjoyable. Using the directory is not an option as long as the director does not delivers the same functions as the core ( apply for, host dictionaries and so on... ).

[Al2Klimov]

You can combine Director delivering hosts with static config providing service apply for rules.

[davekempe]

We have an absolutely massive customer requesting this feature. Particularly api-users.conf need to be able to have passwords hashed on disk somehow. Is there a short term work around I can supply? We also need the icingadb redis and other passwords in icingaweb2/resources.ini hashed if at all possible. I mean if we use no password at all, then the credentials aren't on disk I suppose :)

[slalomsk8er]

We also need the icingadb redis and other passwords in icingaweb2/resources.ini hashed if at all possible. I mean if we use no password at all, then the credentials aren't on disk I suppose :)

It's impossible to hash a password on the client. You can only hash it on the server/target as the server can compare the valid password from the client with the stored hash to figure out if it's valid with a good enough probability. For the secrets in the resources.ini this would not work as Icinga2 needs to send the password to the DB as the DB has to compare it to the hash it stored of the password. The only way out would be to encrypt the secrets in resources.ini while they are at rest but then you would need to decrypt them on config load and how would you get the credential to do that injected?

So the credentials icinga2 needs to connect can't be hashed only the credentials that are needed to connect to icinga2. IMHO, for enterprise grade config encryption, the secret needed to decrypt resources.ini at config load time, would need to be stored in the server's TPM or in a HW-dongle like a YubiKey that's plugged into the server.

[Al2Klimov]

@julianbrost According to public info* (which I may not confirm publicly :) due to NDA) the author of https://github.com/Icinga/icinga2/issues/6404#issuecomment-2091971343 is an Icinga partner. IMAO we shall finally make a decision between:

• #7942
• #5715
  • I.e. revert #6387 and fix locking
• something entirely new

* Navigate to that comment, its author, his organisation, their website and dig a bit around.

[slalomsk8er]

We have an absolutely massive customer requesting this feature. Particularly api-users.conf need to be able to have passwords hashed on disk somehow.

As a workaround, certificate based API authentication could be an option. https://icinga.com/docs/icinga-2/latest/doc/12-icinga2-api/#authentication

[julianbrost]

IMAO we shall finally make a decision between:

• #7942
• #5715
  • I.e. revert #6387 and fix locking
• something entirely new

What were the problems with that implementation so that it was reverted completely? We're linking to OpenSSL anyways, so shouldn't it be possible to use that? Looks like I'm just repeating https://github.com/Icinga/icinga2/pull/7942#issuecomment-1076219503 now.

[Al2Klimov]

• According to #6387
  • Misleading format #6278
  • Missing synchronisation #6279

[julianbrost]

You've created a PR that does not use OpenSSL but bundles a Blowfish implementation. Why did you conclude that this is necessary?

I mean the OpenSSL command line tool allows to hash passwords:

$ openssl passwd -6 asdf
$6$pFwdybmmryTgBXnB$o7oglIUL5Yv1vylI4LHNwVRBbNgTJfz451144SkOSaJOHvEUKHmSM/1m0qQHaXjcJVj2JXm0eW.7h2sH48j6t.

These look perfectly compatible with crypt(3), at least they can be checked with Python as well:

>>> from crypt import crypt
>>> crypt('asdf', '$6$pFwdybmmryTgBXnB$o7oglIUL5Yv1vylI4LHNwVRBbNgTJfz451144SkOSaJOHvEUKHmSM/1m0qQHaXjcJVj2JXm0eW.7h2sH48j6t.') == '$6$pFwdybmmryTgBXnB$o7oglIUL5Yv1vylI4LHNwVRBbNgTJfz451144SkOSaJOHvEUKHmSM/1m0qQHaXjcJVj2JXm0eW.7h2sH48j6t.'
True

Missing synchronization sounds strange as well. What's the problem there? None of the operations should have to operate on any shared state.

[Al2Klimov]

You've created a PR that does not use OpenSSL but bundles a Blowfish implementation. Why did you conclude that this is necessary?

PHP password_hash() uses it as well and I'm not that arrogant to think I know it better. Also, "bcrypt is stronger than PBKDF2 for most types of passwords". Modern monitoring deserves modern password hashing.

I mean the OpenSSL command line tool allows to hash passwords:

$ openssl passwd -6 asdf
$6$pFwdybmmryTgBXnB$o7oglIUL5Yv1vylI4LHNwVRBbNgTJfz451144SkOSaJOHvEUKHmSM/1m0qQHaXjcJVj2JXm0eW.7h2sH48j6t.

These look perfectly compatible with crypt(3), at least they can be checked with Python as well:

>>> from crypt import crypt
>>> crypt('asdf', '$6$pFwdybmmryTgBXnB$o7oglIUL5Yv1vylI4LHNwVRBbNgTJfz451144SkOSaJOHvEUKHmSM/1m0qQHaXjcJVj2JXm0eW.7h2sH48j6t.') == '$6$pFwdybmmryTgBXnB$o7oglIUL5Yv1vylI4LHNwVRBbNgTJfz451144SkOSaJOHvEUKHmSM/1m0qQHaXjcJVj2JXm0eW.7h2sH48j6t.'
True

Very nice to know we could use OpenSSL CLI instead of https://github.com/Icinga/icinga2/issues/6404#issuecomment-603953265 and benefit from a prompt (openssl passwd -6) AND crypt(3) compatibility. Ex. on Mac where your example seems not to work – @yhabteab, please cross-check.

But we're discussing problems with the reverted implementation in Icinga 2 anyway, so I've built v2.8.2. E.g. icinga2 api user --user lolcat --password 123456 printed password_hash = "$5$a2d6a1cc5d1c2f9b$801d451dae23da49d2cf702610c245703138099e87b0954e167576a0d940a10e" which I can perfectly paste in Python (on Linux):

>>> from crypt import crypt
<stdin>:1: DeprecationWarning: 'crypt' is deprecated and slated for removal in Python 3.13
>>> password_hash = "$5$a2d6a1cc5d1c2f9b$801d451dae23da49d2cf702610c245703138099e87b0954e167576a0d940a10e"
>>> crypt('123456', password_hash) == password_hash
False

The old implementation advertised itself as a $5$ one. But openssl passwd -5 prints much shorter hashes. Admittedly I was wrong: we can't just re-incarnate it. Not like this. So now it's bcrypt vs. something entirely new (i.e. openssl passwd -6). And you seem to prefer the latter which I can live with.

Missing synchronization sounds strange as well. What's the problem there? None of the operations should have to operate on any shared state.

Full ack, they SHOULD not. But the crashes speak for themselves: https://github.com/Icinga/icinga2/issues/6279#issuecomment-385957892 I'd try to add some locks if that implementation would still matter given my above arguments.

How to build v2.8.2

diff --git a/icinga-app/icinga.cpp b/icinga-app/icinga.cpp
index a8caaabea..4dcf6b3ab 100644
--- a/icinga-app/icinga.cpp
+++ b/icinga-app/icinga.cpp
@@ -175,7 +175,6 @@ int Main(void)
                } catch (const std::invalid_argument& ex) {
                        std::cout
                                << "Error while parsing \"ICINGA2_RLIMIT_FILES\" from sysconfig: " << ex.what() << '\n';
-                       return EXIT_FAILURE;
                }
        }
 #endif /* RLIMIT_NOFILE */
@@ -190,7 +189,6 @@ int Main(void)
                } catch (const std::invalid_argument& ex) {
                        std::cout
                                << "Error while parsing \"ICINGA2_RLIMIT_PROCESSES\" from sysconfig: " << ex.what() << '\n';
-                       return EXIT_FAILURE;
                }
        }
 #endif /* RLIMIT_NPROC */
@@ -205,7 +203,6 @@ int Main(void)
                } catch (const std::invalid_argument& ex) {
                        std::cout
                                << "Error while parsing \"ICINGA2_RLIMIT_STACK\" from sysconfig: " << ex.what() << '\n';
-                       return EXIT_FAILURE;
                }
        }
 #endif /* RLIMIT_STACK */
@@ -501,6 +498,7 @@ int Main(void)
        } else if (command) {
                Logger::DisableTimestamp(true);
 #ifndef _WIN32
+               goto Skip;
                if (command->GetImpersonationLevel() == ImpersonateRoot) {
                        if (getuid() != 0) {
                                Log(LogCritical, "cli", "This command must be run as root.");
@@ -575,6 +573,7 @@ int Main(void)
                                }
                        }
                }
+       Skip:

                Process::InitializeSpawnHelper();
 #endif /* _WIN32 */
diff --git a/lib/cli/apiusercommand.cpp b/lib/cli/apiusercommand.cpp
index 188691ae0..60a72a374 100644
--- a/lib/cli/apiusercommand.cpp
+++ b/lib/cli/apiusercommand.cpp
@@ -68,7 +68,7 @@ int ApiUserCommand::Run(const boost::program_options::variable
s_map& vm, const s
                return 1;
        }

-       passwd = vm["passwd"].as<std::string>();
+       passwd = vm["password"].as<std::string>();
        salt = vm.count("salt") ? String(vm["salt"].as<std::string>()) : RandomS
tring(8);

        if (salt.FindFirstOf('$') != String::NPos) {

[yhabteab]

Very nice to know we could use OpenSSL CLI instead of #6404 (comment) and benefit from a prompt (openssl passwd -6) AND crypt(3) compatibility. Ex. on Mac where your example seems not to work – @yhabteab, please cross-check.

Yep!

~/Workspace/docker-icinga2 (master ✔) openssl passwd -6 icinga
$6$/5TEMR45BMxgBV6Z$BRLhSQwn8zinFdElU7SFMUfQDn1rdCqnQTofQvJtyijk0eppM542vezw8uvzC.O5vH9tHj2QJwT9WdOPBuPg5/

>>> from crypt import crypt
>>> crypt('icinga', '$6$/5TEMR45BMxgBV6Z$BRLhSQwn8zinFdElU7SFMUfQDn1rdCqnQTofQvJtyijk0eppM542vezw8uvzC.O5vH9tHj2QJwT9WdOPBuPg5/') == '$6$/5TEMR45BMxgBV6Z$BRLhSQwn8zinFdElU7SFMUfQDn1rdCqnQTofQvJtyijk0eppM542vezw8uvzC.O5vH9tHj2QJwT9WdOPBuPg5/'
False

When using PHP, it does seem to work though!

php > var_dump(password_hash('icinga', CRYPT_SHA512));
string(60) "$2y$10$ybyNfSJc6F14.4fUFTrSueWm/EdAkcRLOGP0ekAhNWmNz9MtttzCC"
php > var_dump(password_verify('icinga', '$2y$10$ybyNfSJc6F14.4fUFTrSueWm/EdAkcRLOGP0ekAhNWmNz9MtttzCC'));
bool(true)

// OpenSSL password hash
php > var_dump(password_verify('icinga', '$6$/5TEMR45BMxgBV6Z$BRLhSQwn8zinFdElU7SFMUfQDn1rdCqnQTofQvJtyijk0eppM542vezw8uvzC.O5vH9tHj2QJwT9WdOPBuPg5/'));
bool(true)

[Al2Klimov]

@LordHepipud Do people use the API against Windows agents? Because my next idea is to support not a particular algorithm, but whatever crypt(3) provides on your platform. The joke is there's no crypt(3) on Windows.