search

i3 / i3lock

improved screen locker
https://i3wm.org/i3lock
BSD 3-Clause "New" or "Revised" License
921 stars 404 forks source link

Add support for bsd_auth(3) as authentication backend #123

Closed jasperla closed 7 years ago

jasperla commented 7 years ago

This PR adds support for bsd_auth(3) as an authentication backend to i3lock. To keep things simple it's a compile-time option and enabled by default on OpenBSD-only.

In the process of keeping the actual addition of bsd_auth(3) as small as possible I renamed a lot of non-PAM specific variables/fields that had PAM/pam in them to AUTH/auth. I think this will also make it easier for future backends to be added.

Airblader commented 7 years ago

@stapelberg What do you think about this?

@jasperla Generally speaking we don't want compile-time flags in i3lock & family. It should just be platform-specific like the other things, though of course this leaves the pam dependency in OpenBSD. Besides, is there any reason why i3lock on OpenBSD should no longer support pam?

jasperla commented 7 years ago

@Airblader it's not so much that i3lock on OpenBSD should no longer support PAM; rather that OpenBSD never had PAM. There is an openpam port but it's merely a stub library than anything else. I can see why you generally wouldn't want any compile time options, but for such fundamental things as the authentication backend that is system-specific (bsd_auth and more or less PAM too) I think it would be justified (I may be biased ;-)).

Airblader commented 7 years ago

Thanks for the explanation – you can tell I'm not knowledgable on BSDs. :-) I'd rather leave this decision up to @stapelberg since it's pretty fundamental, as you pointed out yourself.

stapelberg commented 7 years ago

I don’t see why this needs to be optional. If OpenBSD doesn’t provide a working PAM stack, i3lock should always use bsd_auth(3) on OpenBSD.

Can you update the PR to use ifdefs please?

jasperla commented 7 years ago

The code is already using #ifdef to conditionally compile either codepath.

stapelberg commented 7 years ago

Yes, but the ifdefs aren’t consistent with the way we’re using them in i3status/i3: you’re currently declaring a custom definition (USE_BSDAUTH and USE_PAM) whereas the rest of the code directly checks under which platform it is compiled.

jasperla commented 7 years ago

Oh right, so #ifdef __OpenBSD__, #else, #endif ?

stapelberg commented 7 years ago

Yes.

jasperla commented 7 years ago

That should do it.

stapelberg commented 7 years ago

Thanks!

jasperla commented 7 years ago

Great, thanks a lot for the merge!