Add password hash authentication by spacefrogg · Pull Request #573 · hyprwm/hyprlock

spacefrogg · 2024-12-02T16:47:55Z

As an alternative to PAM authentication, password hash authentication only relies on the availability of libgcrypt (and some program like OpenSSL or sha256sum to create the hash).

Supports salted hashes and all hash algorithms that are available in the actual libgcrypt installation.

This is practical in more restricted environments. As an example: I work on a Ubuntu machine w/o root privileges and use Hyprland via Nix Home Manager. Ubuntu uses a patched PAM library to support their own @include primitive which is incompatible with standard PAM. Additionally dlopen across PAM library version does not work well from 1.6 to 1.5.

I am aware that the documentation likely has to move from the README to the Wiki, but I put it there for easy initial review.

vaxerski

what does libgcrypt offer over libcrypto?
is there not a C++ library?

spacefrogg · 2024-12-03T08:55:05Z

what does libgcrypt offer over libcrypto?

Hyprlock currently uses neither and I have just picked one of the two. Libgcrypt is much smaller and simpler than libcrypto as a whole. Although, mere one-off digest creation and checking looks quite similar in both. I knrew how to do it in libgcrypt. So I used it.

is there not a C++ library?

I don't know of any famous and battle-proven library. Do you? So I stayed on the safe side. As the task is a purely functional "take this data, compute the digest and compare it to this other digest", there is no inherent value in a library with an object model. There is no state-transfer into libgcrypt which could collide with the C++ object model.

vaxerski · 2024-12-04T16:32:57Z

I just am not a fan of #define GCRYPT_NO_DEPRECATED and C-style code in general, but fair, if there is no better alternative

src/core/BAuth.hpp

vaxerski · 2024-12-04T16:33:46Z

src/core/PwAuth.cpp

+#define NEED_LIBGCRYPT_VERSION nullptr
+#include <gcrypt.h>
+
+using namespace std::chrono_literals;


Again, I have no strong feelings about this, but what is to be had against readable literals? Is it just a too un-C++ thing to do? I will gladly change it back, if you want me to.

src/core/PwAuth.cpp

vaxerski · 2024-12-04T16:34:35Z

src/core/PwAuth.cpp

+using namespace std::chrono_literals;
+
+static auto hex2Bytes(const std::string& hex) -> std::unique_ptr<unsigned char[]> {
+    auto bytes = std::make_unique<unsigned char[]>(hex.length() / 2u);


what the actual fuck? std::vector??

I'm sorry? I just need a unique_ptr to some fixed buffer, here (for automatic deletion after the hash generation). How would you recommend doing that with std::vector (or, rather std::array)?

src/core/PwAuth.cpp

src/core/PwAuth.hpp

vaxerski · 2024-12-04T16:38:46Z

src/core/PwAuth.hpp

+    bool                             m_bBlockInput    = true;
+    bool                             m_bAuthenticated = false;
+    bool                             m_bLibFailed     = false;
+    std::unique_ptr<unsigned char[]> m_sHash;


PointerDilemma · 2024-12-05T09:02:08Z

I think pam is fundamentally broken, so I am not entirely opposed to a custom Auth method.

Two security relevant things.

hashing is too fast. we need an artificial delay (could be configurable). Otherwise brute forcing trivial passwords is too easy.
I think we should store password hash + salt in a separate hyprlang config (like "secret.conf") and make sure it's permissions are -rw-r---- .

PointerDilemma · 2024-12-05T09:07:26Z

src/core/PwAuth.cpp

+        }
+    } else {
+        Debug::log(CRIT, "libgcrypt could not be initialized");
+        m_bLibFailed = true;


Why not just crash? Maybe with a RASSERT or something. Without a crash you won't be able to log in, so I think that would make more sense.

This happens during initialisation. You want the locker to lock up your system, because of an error that is outside of the user's realm of influence? Was thinking that the locker should rather not lock the system if it knows beforehand that it cannot reliably unlock afterwards.

In the end this is your decision to make. I just wanted to bring this up as a counter argument, as I have thought about this as well and came to the opposite conclusion.

If we don't crash we lock up the system, but silently.
If we crash we lock up the system in a more obvious way.

So the decision is to make it crash :)

~~The way I programmed it now is to not lock up the system at all but immediately return. Hyprlock has not yet told the Compositor to lock the system.~~

EDIT: In line 161 in auth() you can see that it immediately returns authenticated (and subsequently unlocks) if m_bLibFailed is true.

I make it crash if you want, but your statement about the current silent lock is not correct.

From the perspective of the application, every case of m_bLibFailed = true looks identical. So, I would crash the application in all cases, then, not just when the library fails to initialise.

spacefrogg · 2024-12-06T10:45:27Z

I think pam is fundamentally broken, so I am not entirely opposed to a custom Auth method.

Two security relevant things.
* hashing is too fast. we need an artificial delay (could be configurable). Otherwise brute forcing trivial passwords is too easy.

Is that not true for PAM as well? So, it should be some general artificial delay between login tries, not just for this method.

* I think we should store password hash + salt in a separate hyprlang config (like "secret.conf") and make sure it's permissions are -rw-r---- .

Well, I don't believe in security by obscurity. The strength of the hash is usually as good as any public key. If you entrust public keys to the public, you can as well with the hash. I know, though, that other people think different about this and leave this for you to decide.

spacefrogg · 2024-12-06T11:57:41Z

What I just wrote is, of course, not equally true, for hashes that are used without salt.

PointerDilemma · 2024-12-06T12:23:22Z

Is that not true for PAM as well? So, it should be some general artificial delay between login tries, not just for this method.

All PAM login configurations i came across have such a delay.
Edit: it is just the crypto inducing this delay i think

Well, I don't believe in security by obscurity. The strength of the hash is usually as good as any public key. If you entrust public keys to the public, you can as well with the hash. I know, though, that other people think different about this and leave this for you to decide.

No!! Password hashes need to be treated with care. Especially if the salt is also stored in the same file. You are right that having the password hash leaked to another user does not straight up mean they can login. But with enough time you might be able to find the password (for example, if the password is in rockyou.txt and you know the salt, it is game over).

Finding a private key for a corresponding public key is MUCH harder than finding the plaintext for a certain hash. For that your security is only the plaintext length.
Edit: if you consider the whole input range, then yes it can be considered very hard to find the plaintext of a certain hash. but by making certain assumptions about a password (charset and length), weak passwords are trivial to guess, because sha256 is fast. This is why you normally use PBKDF for password hashes.

spacefrogg · 2024-12-06T19:39:30Z

Is that not true for PAM as well? So, it should be some general artificial delay between login tries, not just for this method.

All PAM login configurations i came across have such a delay. Edit: it is just the crypto inducing this delay i think

Yeah, so I would just implement that near hyprlock.cpp: onPasswordCheckTimer() for all authentication interfaces.

Finding a private key for a corresponding public key is MUCH harder than finding the plaintext for a certain hash. For that your security is only the plaintext length. Edit: if you consider the whole input range, then yes it can be considered very hard to find the plaintext of a certain hash. but by making certain assumptions about a password (charset and length), weak passwords are trivial to guess, because sha256 is fast. This is why you normally use PBKDF for password hashes.

You are right! So, it must be a read-restricted file.

As an alternative to PAM authentication, password hash authentication only relies on the availability of libgcrypt (and some program like OpenSSL or sha256sum to create the hash). Supports salted hashes and all hash algorithms that are available in the actual libgcrypt installation.

PointerDilemma · 2024-12-12T09:43:54Z

Since my last comment I

Created a PR for an authentication interface, that makes it easier to integrate a new authentication method into hyprlock. (auth: add an interface for different authentication methods #578)
Implemeneted a POC for hash based authentication using libsodium. See POC: Custom hash based auth using libsodium PointerDilemma/hyprlock#2
I think this provides a better starting point. We just need to think about how users are supposed to generate the hash, or if hyprlock even does it for you.

vaxerski reviewed Dec 2, 2024

View reviewed changes

vaxerski reviewed Dec 4, 2024

View reviewed changes

PointerDilemma reviewed Dec 5, 2024

View reviewed changes

spacefrogg force-pushed the pwhash branch 2 times, most recently from 963382e to 2ee663b Compare December 6, 2024 19:16

spacefrogg force-pushed the pwhash branch from 2ee663b to cdef659 Compare December 10, 2024 17:45

PointerDilemma mentioned this pull request Dec 16, 2024

auth: add libsodium based hash authentication #585

Closed

spacefrogg mentioned this pull request Dec 5, 2025

Add password hash authentication method #924

Open

Uh oh!

Conversation

spacefrogg commented Dec 2, 2024

Uh oh!

vaxerski left a comment

Choose a reason for hiding this comment

Uh oh!

spacefrogg commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vaxerski commented Dec 4, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PointerDilemma commented Dec 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacefrogg Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacefrogg commented Dec 6, 2024

Uh oh!

spacefrogg commented Dec 6, 2024

Uh oh!

PointerDilemma commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spacefrogg commented Dec 6, 2024

Uh oh!

PointerDilemma commented Dec 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

spacefrogg commented Dec 3, 2024 •

edited

Loading

spacefrogg Dec 6, 2024 •

edited

Loading

PointerDilemma commented Dec 6, 2024 •

edited

Loading