Skip to content

Introduce ApiUser#hashed_password - #7942

Open
Al2Klimov wants to merge 2 commits into
masterfrom
feature/hash-credentials-6404
Open

Introduce ApiUser#hashed_password#7942
Al2Klimov wants to merge 2 commits into
masterfrom
feature/hash-credentials-6404

Conversation

@Al2Klimov

Copy link
Copy Markdown
Member

fixes #6404

@Al2Klimov Al2Klimov added this to the 2.13.0 milestone Mar 25, 2020
@Al2Klimov Al2Klimov self-assigned this Mar 25, 2020
@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from f093994 to 3fcf7d0 Compare March 26, 2020 09:11
@Al2Klimov

Copy link
Copy Markdown
Member Author
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu none: https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu none:123456 https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu none:123457 https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu plain: https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu plain:123456 https://127.0.0.1:5665/v1/stats
{"error":404.0,"status":"The requested path 'v1/stats' could not be found or the request method is not valid for this path."}
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu plain:123457 https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu hash: https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu hash:123456 https://127.0.0.1:5665/v1/stats
{"error":404.0,"status":"The requested path 'v1/stats' could not be found or the request method is not valid for this path."}
➜  icinga2 git:(feature/hash-credentials-6404) curl -skSu hash:123457 https://127.0.0.1:5665/v1/stats
<h1>Unauthorized. Please check your user credentials.</h1>
➜  icinga2 git:(feature/hash-credentials-6404)

@Al2Klimov Al2Klimov changed the title WIP Introduce ApiUser#hashed_password Mar 26, 2020
@Al2Klimov

Copy link
Copy Markdown
Member Author
➜  icinga2 git:(feature/hash-credentials-6404) cat prefix/etc/icinga2/conf.d/api-users.conf
/**
 * The ApiUser objects are used for authentication against the API.
 */
object ApiUser "root" {
  password = "e509e644a1cc8570"
  // client_cn = ""

  permissions = [ "*" ]
}

object ApiUser "hash" {
	hashed_password = "$2y$10$uJUA0HVV3lHVVE0LsxPYZ.ioK2b7Lt82h6vwpgVBQSPqYjL7Jke7q"
}

object ApiUser "plain" {
	password = "123456"
}

object ApiUser "none" {
}
➜  icinga2 git:(feature/hash-credentials-6404)

@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from 466d5b5 to 4433f81 Compare March 26, 2020 12:00
@Al2Klimov Al2Klimov removed their assignment Mar 26, 2020
@Al2Klimov
Al2Klimov marked this pull request as ready for review March 26, 2020 12:02
@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from 4433f81 to 0cd3773 Compare December 14, 2020 17:14
@julianbrost julianbrost modified the milestones: 2.13.0, 2.14.0 May 31, 2021
@Al2Klimov

Copy link
Copy Markdown
Member Author

@cla-bot check

@cla-bot cla-bot Bot added the cla/signed label Aug 4, 2021
@Al2Klimov Al2Klimov added the enhancement New feature or request label Aug 10, 2021
@Al2Klimov

Copy link
Copy Markdown
Member Author

AFAIK the concept review is already positive: #6404 (comment)

@Crunsher

Copy link
Copy Markdown
Contributor

Hey @Al2Klimov,

I have some history to this topic that might help:
A similar feature was introduced some years ago as password_hash (see apiuser.ti) but suffered greatly from the format. I see you are also planning on using this flawed approach and would recommend not following this path again.

Also why add another third party dependency and do the dangerous thing of doing it by hand when openssl can do the job for you lol

Kind regards,
Diana

@Al2Klimov

Copy link
Copy Markdown
Member Author

suffered greatly from the format

I don't quite understand. No one actually reads that, people just C&P that from $TOOL output.

Also why add another third party dependency

Have a look at Icinga DB's deps' amount :P

openssl can do the job for you

OpenSSL can $2y$? OK, now I'm all ears...

@Crunsher

Copy link
Copy Markdown
Contributor

Of course it can't use the most bleeding edge hashing algorithms available, but who wants that? But you do you and I do not know the current requirements for this feature.

The format is the kicker. Feedback was it's a surprise to the user to find this format in the config when there are established best practices for hashes of this kind.

@Al2Klimov

Copy link
Copy Markdown
Member Author

Your implementation wasn’t crypt(3) compatible. Mine is.

And it's the hot shit. <-- change my mind :)

@Al2Klimov

Copy link
Copy Markdown
Member Author

who wants that

Wait!
I guess... everyone? (CC @julianbrost)

@julianbrost

Copy link
Copy Markdown
Member

Well you have to balance things here. What would be the best thing that OpenSSL would offer at the moment (must also be available on the ancient CentOS 7 version)? Is it really that bad that we have to vendor a bunch of code? I mean almost anything would be a huge improvement over the status quo.

@julianbrost julianbrost removed this from the 2.14.0 milestone Jan 23, 2023
@Al2Klimov

Copy link
Copy Markdown
Member Author

If you don’t care about crypt(3) compatibility and accept something less secure than the best available, shall we (I) just reincarnate the old implementation using OpenSSL?

@Al2Klimov

Copy link
Copy Markdown
Member Author

Tbh.: yes, we could cover the $2y$ -or at least $2a$- availability via unit tests and I even know how to make test Icinga 2 on OpenBSD and FreeBSD. But Windows has no crypt(3).

@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from 0cd3773 to 10281c0 Compare January 31, 2025 16:00
@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from 10281c0 to f4478e3 Compare April 30, 2025 15:33
@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from f4478e3 to e1754a3 Compare May 28, 2025 15:35
@Al2Klimov
Al2Klimov force-pushed the feature/hash-credentials-6404 branch from e1754a3 to cccd4c1 Compare August 28, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow to hash/encrypt credentials or use an external storage

3 participants