Bug 25155

There was a patch for shadow support posted on the mailing list last month[1]. Has there been any progress in getting that into the tree? Gentoo is also interested in this[2] since we give the option of using shadow instead of pam for our users.

1. http://lists.freedesktop.org/archives/polkit-devel/2010-January/000288.html
2. http://bugs.gentoo.org/show_bug.cgi?id=291116

The patch is now in the Slackware tree, and it seems fine, but there's still not review from David et al in upstream.

http://lists.freedesktop.org/archives/polkit-devel/2010-June/000310.html

Created attachment 36614 [details] [review]
patch to support shadow in polkit-1

David,

Attached is a corrected version of the polkit-1 shadow patch.  It was tested on Slackware 13.1 and Fedora 13 installs. All suggested corrections were implemented except:

-- Moving the $(NULL) in the modified Makefile.am.  There were was a block of lines below that particular part that selected a particular backend to compile in.  As far as I can tell, it was correct as it was (I am no expert on automake, though).

-- Using "#define _XOPEN_SOURCE" to use crypt(3), as it resulted in compiler warnings related to usleep(3) and clearenv(3).  After a little fiddling, I discovered that "#define _GNU_SOURCE" worked, and produced no compiler warnings for either backend.  It is defined in polkitagenthelperprivate.h.

I also formatted the patch using git-format-patch(1), which I believe what you wanted me to use.

If anything else needs to be changed, please tell me, and I'll take a look at it as soon as I can.

~Andrew

Comment on attachment 36614 [details] [review]
patch to support shadow in polkit-1

This patch has trailing whitespace.

The patch also, for some reason, fails to apply with 'git am' - please test that as well (it applies fine with just using patch -p1).

>Robby Workman <rw@rlworkman.net> changed:
>         Priority|medium                      |high

Really? First of all, the Priority field is useless; second, no, it's not going to make things go faster if you set it to high; third, it's a sure-fire way to piss off most maintainers; fourth, I think the Priority field was reserved for triagers/maintainers, not as a way for people to whine "my bug is more important than other bugs!".

(In reply to comment #5)
> >Robby Workman <rw@rlworkman.net> changed:
> >         Priority|medium                      |high
> 
> Really? First of all, the Priority field is useless; second, no, it's not going
> to make things go faster if you set it to high; third, it's a sure-fire way to
> piss off most maintainers; fourth, I think the Priority field was reserved for
> triagers/maintainers, not as a way for people to whine "my bug is more
> important than other bugs!".

Of course if this was a honest mistake, no worries. I just get really agitated when people play around with the Priority. Anyway, water under the bridge, let's focus on getting this patch committed.

Created attachment 36648 [details] [review]
patch to support shadow in polkit-1

Here's round two.

This one fixes the whitespace errors that Robby mentioned.  In order to apply it with 'git am', I had to 'rm -rf .git/rebase-apply' first.  Once done, the patching should go without incident.

(In reply to comment #7)
> Created an attachment (id=36648) [details]
> patch to support shadow in polkit-1
> 
> Here's round two.
> 
> This one fixes the whitespace errors that Robby mentioned.  In order to apply
> it with 'git am', I had to 'rm -rf .git/rebase-apply' first.  Once done, the
> patching should go without incident.

Committed with a patch like the one in [1] on top and then squashed into one, see

http://cgit.freedesktop.org/PolicyKit/commit/?id=a2edcef54d2ab1a92f729e34dfa0c183b2533c61

[1] :

--- a/src/polkitagent/Makefile.am
+++ b/src/polkitagent/Makefile.am
@@ -68,7 +68,8 @@ libpolkit_agent_1_la_LDFLAGS = -export-symbols-regex '(^polkit_.*)'
 libexec_PROGRAMS = polkit-agent-helper-1
 
 polkit_agent_helper_1_SOURCES = 					\
-	polkitagenthelperprivate.c polkitagenthelperprivate.h
+	polkitagenthelperprivate.c polkitagenthelperprivate.h		\
+	$(NULL)
 
 if POLKIT_AUTHFW_PAM
 polkit_agent_helper_1_SOURCES += polkitagenthelper-pam.c
@@ -76,7 +77,6 @@ endif
 if POLKIT_AUTHFW_SHADOW
 polkit_agent_helper_1_SOURCES += polkitagenthelper-shadow.c
 endif
-polkit_agent_helper_1_SOURCES += $(NULL)
 
--- a/src/polkitagent/polkitagenthelper-pam.c
+++ b/src/polkitagent/polkitagenthelper-pam.c
@@ -49,7 +49,7 @@ main (int argc, char *argv[])
   pam_h = NULL;
 
   /* clear the entire environment to avoid attacks using with libraries honoring environment variables */
-  if (clearenv () != 0)
+  if (_polkit_clearenv () != 0)
     goto error;

--- a/src/polkitagent/polkitagenthelper-shadow.c
+++ b/src/polkitagent/polkitagenthelper-shadow.c
@@ -51,7 +51,7 @@ main (int argc, char *argv[])
 
   /* clear the entire environment to avoid attacks with
      libraries honoring environment variables */
-  if (clearenv () != 0)
+  if (_polkit_clearenv () != 0)
     goto error;

--- a/src/polkitagent/polkitagenthelperprivate.c
+++ b/src/polkitagent/polkitagenthelperprivate.c
@@ -29,13 +29,19 @@
 #ifndef HAVE_CLEARENV
 extern char **environ;
 
-static int
-clearenv (void)
+int
+_polkit_clearenv (void)
 {
   if (environ != NULL)
     environ[0] = NULL;
   return 0;
 }
+#else
+int
+_polkit_clearenv (void)
+{
+  return clearenv ();
+}
 #endif

--- a/src/polkitagent/polkitagenthelperprivate.h
+++ b/src/polkitagent/polkitagenthelperprivate.h
@@ -36,6 +36,8 @@
 #  define LOG_AUTHPRIV    (10<<3)
 #endif
 
+int _polkit_clearenv (void);