6:06 a.m.
On Čt, 2014-09-18 at 13:02 +0400, Dmitry V. Levin wrote:
On Thu, Sep 18, 2014 at 10:44:57AM +0200, Tomas Mraz wrote:
> On Čt, 2014-09-18 at 02:17 +0400, Dmitry V. Levin wrote:
> > On Fri, Sep 05, 2014 at 07:24:31AM +0000, Tomáš Mráz wrote:
> > [...]
> >
> > I've discovered an inconsistency in the way how grantor is initialized:
> >
> > > --- a/libpam/pam_dispatch.c
> > > +++ b/libpam/pam_dispatch.c
> > > @@ -217,8 +217,14 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int
flags, struct handler *h,
> > > status = retval;
> > > }
> > > }
> > > - if ( impression == _PAM_POSITIVE && action ==
_PAM_ACTION_DONE ) {
> > > - goto decision_made;
> > > + if ( impression == _PAM_POSITIVE ) {
> > > + if ( retval == PAM_SUCCESS ) {
> > > + h->grantor = 1;
> > > + }
> > > +
> > > + if ( action == _PAM_ACTION_DONE ) {
> > > + goto decision_made;
> > > + }
> > > }
> > > break;
> > >
> >
> > Here grantor is being set every time retval is PAM_SUCCESS and
> > impression is _PAM_POSITIVE, ...
> >
> > > @@ -262,6 +268,9 @@ static int _pam_dispatch_aux(pam_handle_t *pamh, int
flags, struct handler *h,
> > > || (impression == _PAM_POSITIVE
> > > && status == PAM_SUCCESS) ) {
> > > if ( retval != PAM_IGNORE || cached_retval == retval ) {
> > > + if ( impression == _PAM_UNDEF && retval == PAM_SUCCESS )
{
> > > + h->grantor = 1;
> > > + }
> > > impression = _PAM_POSITIVE;
> > > status = retval;
> >
> > while here grantor is set only if retval is PAM_SUCCESS and
> > impression is not yet _PAM_POSITIVE, so if impression is already
> > _PAM_POSITIVE, grantor will not be set.
>
> I did that on purpose. In this case (jump in the second chain), the
> module is really grantor only when the impression is not set yet. In
> other cases it is just a jump and impression is kept as _PAM_POSITIVE.
Sorry, but "impression = _PAM_POSITIVE" and "status = retval"
assignments are present both in case of a jump and in case of
_PAM_ACTION_OK/_PAM_ACTION_DONE, and they mean exactly the same.
Taking this into consideration, why do you suppose that a module is not
a "really grantor" in case of a jump but still a "really grantor" in
case
of _PAM_ACTION_OK and _PAM_ACTION_DONE?
Hmm, I really wonder whether the bug is not actually in the fact that
for the cached chain the status=retval assignment is done for jump even
if the impression is already _PAM_POSITIVE. This actually does not look
right to me, because it means that in the cached chain the jumps affect
the return value of the libpam call much more than for the initial
chain.
This behavior is in libpam for a long time - since version 0.75. But
that does not mean that it is completely right. Even in the commit
message I do not see any argument why jumps in cached chain should
behave this way.
I am not sure whether Andrew G. Morgan can still be reached and would be
able to tell us what was the reason for this particular handling of
jumps in the cached chain.
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)