Debian Bug report logs - #496924
pam modifcations related to the GSoC project PamNssInstaller

Display info messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Juan Luis Belmonte <jlbelmonte@gmail.com>

To: submit@bugs.debian.org

Subject: pam modifcations related to the GSoC project PamNssInstaller

Date: Thu, 28 Aug 2008 19:18:43 +0200

[Message part 1 (text/plain, inline)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Package: pam
Version: 1.0.1-3
Severity: wishlist
Tags: patch

As part of the GSoC project PamNssInstaller
(http://wiki.debian.org/SummerOfCode2008/PamNssDebianInstaller).
 [libpam-runtime]
    - update-pam tool added, tool to manage pam.d/ files
    - added lines in debian/rules and debian/libpam-runtime.install for the
      installation fo the tool

 [libpam-doc]
    -update-pam manpage
    -added lines in doc/man/Makefile.in for the manpage installation

- -- System Information:
Debian Release: lenny/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: i386 (i686)

Kernel: Linux 2.6.25-2-686 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAki23ewACgkQ2bQWnnlfiVSA/QCaA9qUEpjq9nIy74UBGp6iHNcM
3VYAoMci4FsQWlMhkptcrHQbaeHOv5LW
=6+3h
-----END PGP SIGNATURE-----

[pam-1.0.1-3.patch (text/x-patch, inline)]

diff -Naur pam-1.0.1/debian/libpam-runtime.install pam-1.0.1MOD/debian/libpam-runtime.install
--- pam-1.0.1/debian/libpam-runtime.install	2008-08-27 05:38:24.000000000 +0200
+++ pam-1.0.1MOD/debian/libpam-runtime.install	2008-08-27 03:10:25.000000000 +0200
@@ -3,3 +3,4 @@
 debian/tmp/usr/share/pam	usr/share
 debian/tmp/usr/sbin/pam_getenv	usr/sbin
 debian/tmp/usr/share/locale	usr/share
+debian/tmp/sbin/update-pam      sbin
diff -Naur pam-1.0.1/debian/rules pam-1.0.1MOD/debian/rules
--- pam-1.0.1/debian/rules	2008-08-27 05:38:24.000000000 +0200
+++ pam-1.0.1MOD/debian/rules	2008-08-27 03:12:24.000000000 +0200
@@ -86,8 +86,11 @@
 
 	mkdir -p debian/tmp/etc/pam.d
 	mkdir -p debian/tmp/usr/share/pam
+	mkdir -p debian/tmp/sbin
 	$(i) $(dl)/pam.conf $(d)/tmp/etc
 	-mkdir -p $(d)/tmp/usr/sbin $(d)/tmp/usr/share/man/man8
+	-mkdir -p $(d)/tmp/usr/sbin $(d)/tmp/usr/share/man/man8 $(d)/tmp/usr/share/man/man1
+	$(ie) $(d)/updaters/* $(d)/tmp/sbin
 	$(ie) $(dl)/pam_getenv $(d)/tmp/usr/sbin
 	$(i) $(dl)/other $(d)/tmp/etc/pam.d
 	$(i) $(dl)/common-* $(d)/tmp/usr/share/pam/
@@ -119,7 +122,7 @@
 	rm -f $(d)/libpam-modules/usr/share/man/man8/pam.8
 	rm -f $(d)/libpam-modules/usr/share/man/man5/pam.conf.5
 	rm -f $(d)/libpam-modules/usr/share/man/man5/pam.d.5
-
+	rm -f $(d)/libpam-modules/usr/share/man/man1/update-pam.1
 	dh_installdebconf -a
 	dh_installdocs -a $(BUILD_TREE)/README
 	dh_installexamples -a
diff -Naur pam-1.0.1/debian/updaters/update-pam pam-1.0.1MOD/debian/updaters/update-pam
--- pam-1.0.1/debian/updaters/update-pam	1970-01-01 01:00:00.000000000 +0100
+++ pam-1.0.1MOD/debian/updaters/update-pam	2008-08-27 03:13:22.000000000 +0200
@@ -0,0 +1,171 @@
+#!/usr/bin/perl -w
+#
+# update-pam
+#
+# Copyright (C) 2004 Fabio Tranchitella <kobold@kobold.it>
+# Copyright (C) 2004 Giuseppe Sacco <eppesuig@debian.org>
+#
+# update-pam is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published
+# by the Free Software Foundation; either version 2 of the License,
+# or (at your option) any later version.
+# Developer Juan Luis Belmonte <jlbelmonte@gmail.com>
+
+use strict;
+use Getopt::Long;
+use File::Basename qw(basename);
+use File::Temp qw(tempfile);
+
+my $CONF_DIR = "/etc/pam.d";
+my $VERSION = '0.01';
+
+sub main {
+	my ($m_add, $m_remove, $m_enable, $m_disable, $m_version, $m_help, $before, $after, $service, $control, $actionitem, $type, $attribute);
+    $m_add = $m_remove = $m_enable = $m_disable = 0;
+    $after = $before = $service = $control = $actionitem = $attribute  = $type = "";
+
+	GetOptions(
+		'add|a'      	=> \$m_add,
+		'remove|r'	=> \$m_remove,
+		'enable|e'	=> \$m_enable,
+		'disable|d'	=> \$m_disable,
+		'service=s'   	=> \$service,
+		'actionitem=s' 	=> \$actionitem,
+		'attribute=s'	=> \$attribute,
+		'control=s'   	=> \$control,
+		'type=s'      	=> \$type,
+		'before=s'	=> \$before,
+		'after=s'	=> \$after,
+		'version|v'	=> \$m_version,
+		'help|h'      	=> \$m_help
+	);
+
+	version() if ($m_version);
+	usage() if ($m_help or ($m_add + $m_remove + $m_enable + $m_disable) != 1 or 
+	            (not ($control or $actionitem) and $m_add) or not $type or not @ARGV or ($before ne "" and $after ne ""));
+
+	$service = "common-" . $type if $service eq "";
+	$actionitem=" [".$actionitem."]" if $actionitem ne"";
+    	my $entry = shift @ARGV if @ARGV;
+	
+	if ($m_remove ne""){
+	} elsif ($control =~ /required|requisite|sufficient|optional/){
+	    # here we gain some little  fault tolerance if someone use requiredi acsufficient... is changed to the match, required, sufficient...
+	    $control=$&;
+	    }else{
+		   die "unknown control options";
+	}
+	
+	if ($m_add) {
+       return apply("add", $entry, $service, $control,$actionitem, $attribute, $type, $before, $after);
+	} elsif ($m_remove) {
+       return apply("remove", $entry, $service,$control, $actionitem,$attribute, $type);
+	} elsif ($m_enable) {
+       return apply("enable", $entry, $service,$control, $actionitem,$attribute,$type);
+	} elsif ($m_disable) {
+       return apply("disable", $entry, $service,$control, $actionitem,$attribute,$type);
+	}
+
+}
+
+sub apply {
+	my ($action, $entry, $service, $control, $actionitem,$attribute,$type, $before, $after) = @_;
+    my $output = "";
+
+	open(IN, "$CONF_DIR/$service") or die "Couldn't open $CONF_DIR/$service $!\n";
+	while (<IN>) {
+	    if ($_ =~ /^#/ and $_ !~ /^#<off>#/ or $_ =~ /^[\s\t]+/) {
+			$output .= $_;
+		} elsif ($action eq "add" and $_ =~ /^(\#<off>\#)?(session|account|auth|password)[\s\t]+
+		                                     (requisite|required|sufficient|optional|\[[^\]]+\])
+						                     [\s\t]+(\S+)\s?(.*)/x and $2 eq $type and $entry ne "") {
+			if($entry eq $4){
+				     die " The entry you tryed ($entry) to configure, is already configured in  line $. \n\t $_";
+			
+			}					     
+
+			if ($before eq "$4") {
+				$output .= $type . "\t" .$control. $actionitem. "\t" . $entry . "    $attribute \n" . $_;
+                $entry = "";
+			} elsif ($after eq "$4") {
+				$output .= $_ . $type . "\t" .$control. $actionitem. "\t" . $entry . "    $attribute \n";
+                $entry = "";
+			} else {
+				$output .= $_;
+			}
+
+		} elsif ($action eq "remove" and $_ =~ /^$type[\s\t]+(requisite|required|sufficient|optional|\[[^\]]+\])[\s\t]+$entry(\s(.*)$|$)/x) {
+                        $entry="";
+		} elsif ($action eq "disable" and $_ =~ /^$type[\s\t]+(requisite|required|sufficient|optional|\[[^\]]+\])[\s\t]+$entry(\s(.*)$|$)/x) {
+			$output .= ("#<off>#" . $_);
+                        $entry="";
+		} elsif ($action eq "enable" and $_ =~ /\#<off>\#$type[\s\t]+(requisite|required|sufficient|optional|\[[^\]]+\])[\s\t]+$entry(\s(.*)$|$)/x) {
+			$_ =~ s/^#<off>#//;
+			$output .= $_;
+			$entry="";
+		} else {
+			$output .= $_;
+		}
+	}
+	#if no after before or both given, acts by default.
+		if (($action eq "add") and (($before eq "" and $after eq "") or($before ne "" and $after ne "") or( (($after ne"") or ($before ne ""))and $entry ne""))  ){
+		print STDERR "Not a valid after/before combination was given. Default action was done, the new line is at the bottom \n";
+		$output .= $type . "\t" . $control . $actionitem . "\t" . $entry . "    $attribute \n";
+		$entry="";
+	   }
+
+	close IN;
+
+	if ($entry eq "") {
+		my ($tmp, $filetmp) = tempfile('pam.tmpXXXX') or die ("Couldn't open a temporary file: $!\n");
+		print $tmp $output;
+		close($tmp);
+		rename("$filetmp", "$CONF_DIR/$service") or die "Couldn't rename ".$filetmp.": $!\n";
+
+		return 0;
+	}
+	return 1;
+}
+
+sub usage {
+	print <<EOT;
+Usage: update-pam [OPTIONS] ACTION
+
+Options:
+  --version             output version information and exit
+  --help                display this help and exit
+  --service=SERVICE 	x
+  --type=TYPE           x
+  --control=CONTROL     x
+  --actiointem=SECUENCE x write it without [] and quoted to prevent errors
+  --attribute= STRING	x attributes to write at the end of the line. Write it quted to prevent errors
+  --before=MODULE       x
+  --after=MODULE        x
+
+Actions:
+  --add ENTRY           add a new source
+  --remove ENTRY        remove an existing source 
+  --enable ENTRY        x
+  --disable ENTRY       x
+
+Examples:
+
+  update-pam --actionitem "success=1 default=ignore" --before pam_ldap.so --type account  --add pam_unix.so
+  update-pam --control sufficient --before pam_unix.so --type session --add pam_ldap.so
+
+EOT
+	exit 0;
+}
+
+sub version {
+	print <<EOT;
+update-pam $VERSION
+EOT
+	exit 0;
+}
+
+######################################
+## MAIN CALL, PLEASE DON'T CHANGE IT #
+######################################
+
+	exit main();
diff -Naur pam-1.0.1/doc/man/Makefile.in pam-1.0.1MOD/doc/man/Makefile.in
--- pam-1.0.1/doc/man/Makefile.in	2008-04-16 10:59:04.000000000 +0200
+++ pam-1.0.1MOD/doc/man/Makefile.in	2008-08-27 03:20:45.000000000 +0200
@@ -55,8 +55,9 @@
 SOURCES =
 DIST_SOURCES =
 man3dir = $(mandir)/man3
+man1dir = $(mandir)/man1
 am__installdirs = "$(DESTDIR)$(man3dir)" "$(DESTDIR)$(man5dir)" \
-	"$(DESTDIR)$(man8dir)"
+	"$(DESTDIR)$(man8dir)" "$(DESTDIR)$(man1dir)"
 man5dir = $(mandir)/man5
 man8dir = $(mandir)/man8
 NROFF = nroff
@@ -240,7 +241,7 @@
 	pam_sm_chauthtok.3 pam_start.3 pam_strerror.3 \
 	pam_verror.3 pam_vinfo.3 pam_vprompt.3 pam_vsyslog.3 \
 	misc_conv.3 pam_misc_paste_env.3 pam_misc_drop_env.3 \
-	pam_misc_setenv.3
+	pam_misc_setenv.3 update-pam.1
 
 XMLS = pam.3.xml pam.8.xml \
 	pam_acct_mgmt.3.xml pam_authenticate.3.xml \
@@ -300,6 +301,55 @@
 
 clean-libtool:
 	-rm -rf .libs _libs
+
+install-man1: $(man1_MANS) $(man_MANS)
+	@$(NORMAL_INSTALL)
+	test -z "$(man1dir)" || $(MKDIR_P) "$(DESTDIR)$(man1dir)"
+	@list='$(man1_MANS) $(dist_man1_MANS) $(nodist_man1_MANS)'; \
+       l2='$(man_MANS) $(dist_man_MANS) $(nodist_man_MANS)'; \
+       for i in $$l2; do \
+         case "$$i" in \
+           *.1*) list="$$list $$i" ;; \
+         esac; \
+       done; \
+       for i in $$list; do \
+         if test -f $(srcdir)/$$i; then file=$(srcdir)/$$i; \
+         else file=$$i; fi; \
+         ext=`echo $$i | sed -e 's/^.*\\.//'`; \
+         case "$$ext" in \
+           1*) ;; \
+           *) ext='1' ;; \
+         esac; \
+         inst=`echo $$i | sed -e 's/\\.[0-9a-z]*$$//'`; \
+         inst=`echo $$inst | sed -e 's/^.*\///'`; \
+         inst=`echo $$inst | sed '$(transform)'`.$$ext; \
+         echo " $(INSTALL_DATA) '$$file' '$(DESTDIR)$(man1dir)/$$inst'"; \
+         $(INSTALL_DATA) "$$file" "$(DESTDIR)$(man1dir)/$$inst"; \
+       done
+
+uninstall-man1:
+	@$(NORMAL_UNINSTALL)
+	@list='$(man1_MANS) $(dist_man1_MANS) $(nodist_man1_MANS)'; \
+       l2='$(man_MANS) $(dist_man_MANS) $(nodist_man_MANS)'; \
+       for i in $$l2; do \
+         case "$$i" in \
+           *.1*) list="$$list $$i" ;; \
+         esac; \
+       done; \
+       for i in $$list; do \
+         ext=`echo $$i | sed -e 's/^.*\\.//'`; \
+         case "$$ext" in \
+           1*) ;; \
+           *) ext='1' ;; \
+         esac; \
+         inst=`echo $$i | sed -e 's/\\.[0-9a-z]*$$//'`; \
+         inst=`echo $$inst | sed -e 's/^.*\///'`; \
+         inst=`echo $$inst | sed '$(transform)'`.$$ext; \
+         echo " rm -f '$(DESTDIR)$(man1dir)/$$inst'"; \
+         rm -f "$(DESTDIR)$(man1dir)/$$inst"; \
+       done
+
+
 install-man3: $(man3_MANS) $(man_MANS)
 	@$(NORMAL_INSTALL)
 	test -z "$(man3dir)" || $(MKDIR_P) "$(DESTDIR)$(man3dir)"
diff -Naur pam-1.0.1/doc/man/update-pam.1 pam-1.0.1MOD/doc/man/update-pam.1
--- pam-1.0.1/doc/man/update-pam.1	1970-01-01 01:00:00.000000000 +0100
+++ pam-1.0.1MOD/doc/man/update-pam.1	2008-08-27 03:23:44.000000000 +0200
@@ -0,0 +1,81 @@
+.TH "update-pam" 1
+.SH NAME 
+update-pam \- Modifies /etc/pam.d/ files parameters and options
+.SH SYNOPSIS
+.B update-pam 
+[service|type] [options] [attributes]
+.I \-\-add 
+| 
+.I \-\-remove
+]  
+.SH DESCRIPTION
+The main idea of 
+.I update-pam
+is to provide a polite way to modify, set up  and update the pam conffiles under /etc/pam.d/ 
+.SH OPTIONS
+.B
+--type
+  Selects wich file to modify. account|password|session|auth
+  common-account, common-password, common-session, common-auth respectively.
+
+--services
+  Instead of using --type, the services option is used to define the name  the file to modify if is not one of the common-*.
+
+--control              
+  Defines the module requeriment level. required|requisite|sufficient|optional
+
+--before | --after MODULE      
+   This option defines the position of the module which is being inserted, relative to the module given as parameter.
+   The default option inserts it at the bottom of the file.
+
+--actionitem
+  This option defines an actionitem for the module. It should be written without [] and quoted to prevent errors.
+
+--attribute
+  Some configurations need attributes or actions after the module. For example "nullok_secure".
+  Write them quoted to prevent errors.
+
+--version
+  Prints the version.
+
+--help
+  Prints a simple usage help.
+
+--add   --remove
+  Execute the action of adding or removing the module we are working with.
+
+--disable
+  Comments and marks as disables the line of the module given.
+ 
+--enable
+  Enables a module wich have been disabled before.
+.TP
+.SH EXAMPLES
+.B
+
+Inserting in common-account  pam_module.so with an actionitem, before pam_unix.so.
+
+update-pam.pl --actionitem "success=1 default=ignore" --before pam_unix.so --type account  --add pam_module.so
+
+Inserting in common-session  pam_module.so after  pam_unix.so.
+
+update-pam --control sufficient --after pam_unix.so --type session --add pam_module.so
+
+Adding pam_module.so with attributes 
+
+update-pam --control required  --type auth --after pam_othermodule.so  --add pam_module.so  --attribute "nullok_secure try_first_pass"
+
+
+Removing a module in a file
+ update-pam  --type account  --remove pam_module.so
+
+.TP
+.SH FILES
+.B
+.I /etc/pam.d 
+files 
+For more information  see pam.d(5) 
+.TP
+.SH AUTHOR
+Juan Luis Belmonte <jlbelmonte@gmail.com>
+

Message #10 received at 496924@bugs.debian.org (full text, mbox, reply):

From: Steve Langasek <vorlon@debian.org>

To: Juan Luis Belmonte <jlbelmonte@gmail.com>, 496924@bugs.debian.org

Subject: Re: Bug#496924: pam modifcations related to the GSoC project PamNssInstaller

Date: Fri, 29 Aug 2008 02:10:42 -0700

tags 496924 wontfix
thanks

On Thu, Aug 28, 2008 at 07:18:43PM +0200, Juan Luis Belmonte wrote:
> As part of the GSoC project PamNssInstaller
> (http://wiki.debian.org/SummerOfCode2008/PamNssDebianInstaller).
>  [libpam-runtime]
>     - update-pam tool added, tool to manage pam.d/ files
>     - added lines in debian/rules and debian/libpam-runtime.install for the
>       installation fo the tool

>  [libpam-doc]
>     -update-pam manpage
>     -added lines in doc/man/Makefile.in for the manpage installation

Unfortunately, as we discussed by IRC/mail, this tool does not sufficiently
cover the relevant use cases for me to be willing to accept this patch.

- The syntax requires one to specify the ordering of modules when invoking
  the tool, which means that the caller has to a) know what other modules
  are enabled, and b) know where the requested module should be ordered with
  respect to each.  This is too low-level, and not scalable.

- The tool has to be invoked separately for each of auth, account, password,
  and session, implying that these are always separable.  They are not;
  there are cases where adding a module to, e.g., account but not auth will
  result in a broken stack, which means that adding configs of the various
  types needs to be treated as atomic from the point of view of the caller.

- There is insufficient information stored by this tool to support
  idempotent operation (e.g., removing a line a second time due to
  maintainer script failure after the line has already been removed), or to
  recognize that an admin has made local changes overriding the tool and
  that these should not be overwritten without first asking the admin for
  consent.

- There is no interface for interacting with the user, to allow the user to
  select which installed modules should be enabled.

- The tool permits editing of other config files under /etc/pam.d/, besides
  /etc/pam.d/common-*.  These files are conffiles belonging to the
  individual services, not to PAM; and editing them in this manner is a
  violation of two policy requirements.

It's most regrettable that your GSoC mentor didn't talk to me before you
began coding, because this issue is one that I've been working on
intermittently since last November, and actively since June.  I would have
been happy to provide you with guidance so that you were working towards an
acceptable design.  Since this wasn't done, I have myself been working in
the meantime on implementing the required tool, which is now in beta testing
in Ubuntu, and available as a bzr branch at
<nosmart+http://bzr.debian.org/pkg-pam/debian/features/config-framework/>.
I plan to upload this tool to Debian as soon as lenny is released, at which
point I will close this bug.

There are other minor issues that I see with the patch that could be easily
overcome, if not for the design flaws.  Here's a list of some of these, in
the hopes that you find this feedback useful:

- Your patch installs this tool in /sbin.  Since this is not needed for
  system recovery, it does not belong in /sbin; it's an administration tool,
  and should be shipped in /usr/sbin.

- use File::Basename qw(basename);
  use File::Temp qw(tempfile);
  These modules are included in the 'perl-modules' package, not in
  perl-base; using them would require a dependency on perl-modules, which is
  ill-advised because libpam-runtime is part of the Essential closure (login
  Pre-Depends: libpam-runtime), and perl-modules is explicitly not.

- The manpage handling is done as a patch to upstream build rules.  Since I
  don't think this tool would ever be submitted to Linux-PAM upstream (it
  encodes logic specific to decisions taken by the Debian PAM packages),
  this doesn't seem appropriate.

- [\s\t]+
  This is bad regex style; "\s" means "any whitespace character", which
  implies \t :)

- The indentation of the script is very inconsistent, which makes it
  difficult to follow.  You might want to use something like perltidy on
  your scripts, so that blocks are indented consistently as this improves
  legibility and maintainability.

Cheers,
-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
slangasek@ubuntu.com                                     vorlon@debian.org

Message #17 received at 496924-done@bugs.debian.org (full text, mbox, reply):

From: Steve Langasek <vorlon@debian.org>

To: 496924-done@bugs.debian.org

Subject: Re: pam modifcations related to the GSoC project PamNssInstaller

Date: Sat, 28 Feb 2009 20:58:53 -0800

> I plan to upload this tool to Debian as soon as lenny is released, at
> which point I will close this bug.

This is uploaded now; closing the bug as indicated.

-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
slangasek@ubuntu.com                                     vorlon@debian.org

Send a report that this bug log contains spam.