After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 651554 - implement OpenBSD authentication
implement OpenBSD authentication
Status: RESOLVED FIXED
Product: gnome-screensaver
Classification: Deprecated
Component: general
3.0.x
Other OpenBSD
: Normal normal
: ---
Assigned To: gnome-screensaver maintainers
gnome-screensaver maintainers
Depends on:
Blocks:
 
 
Reported: 2011-05-31 14:25 UTC by Antoine Jacoutot
Modified: 2011-12-01 07:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
implement OpenBSD authentication using bsd_auth (8.05 KB, patch)
2011-05-31 14:25 UTC, Antoine Jacoutot
reviewed Details | Review
implement OpenBSD authentication using bsd_auth (8.98 KB, patch)
2011-08-30 12:16 UTC, Antoine Jacoutot
none Details | Review
implement OpenBSD authentication using bsd_auth (9.19 KB, patch)
2011-11-30 11:10 UTC, Antoine Jacoutot
none Details | Review

Description Antoine Jacoutot 2011-05-31 14:25:13 UTC
Created attachment 188942 [details] [review]
implement OpenBSD authentication using bsd_auth

Hi.

OpenBSD does not use PAM for authentication but bsd_auth(3). It has a subset of the similar functionalities (i.e. authenticate against different password backends).
This patch adds a bsd_auth authentication scheme so that locking works on OpenBSD.
Note that this is a first proposal, because I don't know where gnome-screensaver is heading wrt to other authenticators, so I used what was available in the code
already (like the setuid stuff) to make the patch as small as possible (especially the autoconf bits) so it's easier to review.

Thoughts?
Comment 1 Antoine Jacoutot 2011-06-14 22:22:24 UTC
No one?
Comment 2 Antoine Jacoutot 2011-06-25 15:31:43 UTC
Hi.

Is there any way I could make people interested in this?
Thanks.
Comment 3 Antoine Jacoutot 2011-08-21 10:39:57 UTC
Folks seriously, I know no one gives a fuck about !linux in the GNOME community but I find it really rude not even having someone acknowledge and comment this...

I'm doing my best trying to support another platform (since most GNOME people usually advertise the fact that if we want our OS supported we have to do the job), but being ignored is really frustrating :-(
Comment 4 Colin Walters 2011-08-25 13:52:15 UTC
Review of attachment 188942 [details] [review]:

::: configure.ac
@@ +468,3 @@
+case "$host" in                                                                                       
+  *-openbsd*)                                                                                         
+    have_bsdauth=yes

Is this really specific to openbsd?  Or just since you h

@@ +474,3 @@
+      NEED_SETUID=yes
+    fi
+    AC_SUBST(NEED_SETUID)

Weird...it looks like NEED_SETUID at one point was set by gnome-screensaver, then it stopped.

Anyways, you should take the AC_SUBST outside if the case statement.

@@ +552,2 @@
 else
+	if test "x$have_bsdauth" = "xno" ; then

Really we should only check for PAM if we haven't found bsdauth.  Or the other way around.

::: src/gs-auth-pam.c
@@ +25,3 @@
 #include "config.h"
 
+#ifndef HAVE_BSDAUTH

If the entire gs-auth-pam.c file isn't needed, rather than ifdef'ing it out, you should make it conditionally compiled in Makefile.am.
Comment 5 Antoine Jacoutot 2011-08-25 14:05:03 UTC
(In reply to comment #4)
> Review of attachment 188942 [details] [review]:
> 
> ::: configure.ac
> @@ +468,3 @@
> +case "$host" in                                                                
> +  *-openbsd*)                                                                  
> +    have_bsdauth=yes
> 
> Is this really specific to openbsd?  Or just since you h

Yes. BSD authentication was also used by BSDi (BSD/OS) in the past but this OS is no more.

> +      NEED_SETUID=yes
> +    fi
> +    AC_SUBST(NEED_SETUID)
> 
> Weird...it looks like NEED_SETUID at one point was set by gnome-screensaver,
> then it stopped.

Indeed. It was needed for shadow authentication which got removed. It is now needed only for BSD authentication at run time, then it will drop privileges to setgid auth (bsd auth helpers are only accessible by the auth group).

<snip>

I will take care of the other things you mentioned and update the patch asap, thanks for the review :-)
Comment 6 Antoine Jacoutot 2011-08-30 12:16:54 UTC
Created attachment 195187 [details] [review]
implement OpenBSD authentication using bsd_auth

Hi Colin.

Would something like this do?
Comment 7 Antoine Jacoutot 2011-10-06 09:54:40 UTC
Hi.

I still haven't had any clear answer on that bug report. Now that 3.2 is out, I think it'll be a good time to push this to master so if anyone could have a look, I'd appreciate.
Comment 8 Jasper Lievisse Adriaanse 2011-11-14 09:34:02 UTC
Considering the fact you incorporated Colin's suggestions and that I've been using this too; I for one would be happy to see this being pushed to master.
Comment 9 Colin Walters 2011-11-29 19:48:14 UTC
I only myself look at GNOME/Linux integration issues in general.  The official maintainer is in MAINTAINERS file, but I doubt Jon is going to review this.

It looks pretty simple and unlikely to regress the PAM case, so I guess I'd tentatively say it's OK.

The only other thing I noticed is the type definition in the middle of the function will require C99, but generally in GNOME we stick to C89.
Comment 10 Matthias Clasen 2011-11-29 22:59:02 UTC
I concur; the patch looks ok to me too. Should just add a { } around the content of the ifdef to address the C99 concern.
Comment 11 Antoine Jacoutot 2011-11-30 11:09:46 UTC
Thanks guys.
Ok new patch with the brackets and some simplification in the bsdauth handling code.
If I hear nothing within the next 24h, I'll commit that, fair enough I hope :)
Comment 12 Antoine Jacoutot 2011-11-30 11:10:33 UTC
Created attachment 202432 [details] [review]
implement OpenBSD authentication using bsd_auth
Comment 13 Matthias Clasen 2011-11-30 12:45:00 UTC
Yes, please go ahead
Comment 14 Antoine Jacoutot 2011-12-01 07:59:51 UTC
It's in, d7ab99c00a01bea870cf51d855ecf320e0d23bac