On Mon, 2006 Jul 31 11:50:04 +0200, Thorsten Kukuk wrote:
>
> > * Instead of directly exec'ing a named program, the module invokes a
> > /bin/sh command string, for greater flexibility.
>
> Which flexibility should this give?
You can use redirections, shell variables, environment assignments, even
pipes in the command string...
LANG=C make -C /var/yp >/var/log/yp.log 2>&1
beep -f $PW_UID
who | grep "^$PW_USER\$" | awk '{print $2}' | ...
...without needing to wrap them in a small, separate shell script. I was
originally using execv(), and building up the array of arguments for it,
but the benefits of the /bin/sh approach seemed much more compelling.
Oh, and it avoids some annoying parse issues, like if you have a quoted
string with spaces:
make CFLAGS="-O3 -g" foo
Here, PAM will give you an argument vector of
make
CFLAGS="-O3
-g"
foo
which doesn't work so well in execv() :-)
> The pam_exec line is from a pam_exec implementation calling openlog/closelog.
> This is very bad. You loose the information, which application called
> pam_exec, you destroy the syslog options of the main applications and you
> will get problems with threaded applications.
>
> The pam_unix one looks more correct. You should always use pam_syslog.
> We will not accpet self written logging functions again in Linux-PAM,
> we are happy that all are replaced with pam_*() functions now.
Hmm. I had refrained from using Linux-PAM-specific functions, bearing in
mind compatibility with OpenPAM et al., but I suppose the log() function
can be #ifdef'ed appropriately. I'll make this change.
> > * The module will accept the following options, but I'm not sure what
> > exactly it should do (if anything) with them:
> >
> > expose_account
> > try_first_pass
> > use_first_pass
> > use_mapped_pass
>
> We don't need them at all.
Okay. Should the module accept-but-ignore them?
(I see the "4. Generic optional arguments" section in the manual states
"Here we list the generic arguments that all modules can expect to be
passed," which implies that this ought to be the case.)
> > * I'm a bit unclear on how pam_fail_delay() is supposed to work. Is the
> > manner in which I handled it---and described it in the documentation---at
> > all incorrect?
>
> You should not call pam_fail_delay() from a module. The application has to
> set it. Else you will overwrite defaults which the sysadmin set.
Erm... the "2.2 Other functions provided by libpam" section states "The
pam_fail_delay() function provides the mechanism by which an application or
module can suggest a minimum delay (of micro_sec micro-seconds). Linux-PAM
keeps a record of the longest time requested with this function."
Am I missing something, or is the manual out of date?
> I would prefer to handle such large changes like the kernel developers
> do: A lot of small patches for every new functionality, so that it
> is possible to review and discuss every change and not one very big
> patch.
I'm not quite sure how to approach that. The original code was fairly small
and simple to begin with, and was refactored anyhow in the course of this
work. Would you figure something of a reverse progression, starting from
this code and removing some major feature at each turn?
Alternately... I did aim for good function delegation; all the routines
(with the exception of the main do_exec() function) serve a well-defined
purpose. Would a function-by-function review be workable?
This new implementation is not complicated in concept---for all the talk of
"features," there really isn't that much new machinery here. The main bit
of novelty is the code dealing with file descriptors and IPC.
--Daniel
--
NAME = Daniel Richard G. ## Remember, skunks _\|/_ meef?
EMAIL1 = skunk@xxxxxxxxxx ## don't smell bad--- (/o|o\) /
EMAIL2 = skunk@xxxxxxxxxxxx ## it's the people who < (^),>
WWW = http://www.******.org/ ## annoy them that do! / \
--
(****** = site not yet online)
_______________________________________________
Pam-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/pam-list
