openssh bugs - May 2006 - [Bug 1188] keyboard-interactive should not allow retry after pam_acct_mgmt fails

http://bugzilla.mindrot.org/show_bug.cgi?id=1188

           Summary: keyboard-interactive should not allow retry after
                    pam_acct_mgmt fails
           Product: Portable OpenSSH
           Version: -current
          Platform: Other
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: PAM support
        AssignedTo: bitbucket at mindrot.org
        ReportedBy: dtucker at zip.com.au
OtherBugsDependingO 1155
             nThis:


Because each keyboard-interactive attempt is effectively
self-contained, when the PAM account check fails, the user is
reprompted, even though they can never possible succeed (since
do_pam_account() caches the result).  Eg:

$ ssh localhost
Password:
Your account has expired; please contact your system administrator

Password:

sshd should prevent further keyboard-interactive attempts if the PAM
account check fails.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188





------- Comment #1 from dtucker at zip.com.au  2006-05-03 12:45 -------
Created an attachment (id=1130)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1130&action=view)
prevent retry of keyboard-interactive if PAM account check fails.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188


djm at mindrot.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1130|                            |ok+
               Flag|                            |




------- Comment #2 from djm at mindrot.org  2006-05-03 12:51 -------
(From update of attachment 1130)
Looks OK to me as far as PAM goes, but you should update the copyright
year while you are in there.

Would it make sense to use a global flag? so a PAM authorization
failure blocks all other methods...




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED




------- Comment #3 from dtucker at zip.com.au  2006-05-03 13:06 -------
(In reply to comment #2)
> Would it make sense to use a global flag? so a PAM authorization
> failure blocks all other methods...
I don't think that's necessary.  do_pam_account() caches its result and
it gets called in auth1.c or auth2.c.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188





------- Comment #4 from fcusack at fcusack.com  2006-05-03 15:35 -------
PAM acct mgmt can fail for reasons other than password expiry.  The
patch looks like you assume this is the reason.  Also, if the account
IS expired, the user should be given a chance to update their password.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188





------- Comment #5 from dtucker at zip.com.au  2006-05-03 15:57 -------
(In reply to comment #4)
> PAM acct mgmt can fail for reasons other than password expiry. The
> patch looks like you assume this is the reason.
The patch is about *account* expiry not *password* expiry.  Actually,
it's about any failures of pam_acct_mgmt that aren't password expiry.

do_pam_account() sets force_pwchange and returns success if
pam_account_mgmt returns PAM_NEW_AUTHTOK_REQD (but the code already
checks for that) or returns a failure for any other non-success code.
> Also, if the account IS expired, the user should be given a chance
> to update their password.
If pam_acct_mgmt failed for any reason other than PAM_NEW_AUTHTOK_REQD
then no, they shouldn't.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188





------- Comment #6 from fcusack at fcusack.com  2006-05-04 12:49
-------
> do_pam_account() sets force_pwchange and returns success if
> pam_account_mgmt returns PAM_NEW_AUTHTOK_REQD (but the code already
> checks for that) or returns a failure for any other non-success code.
I hadn't looked at do_pam_acct(), I only looked at the patch.  So
without
enough context I mistook the effects of the patch.  I did at least say
"looks like".

Thanks for the additional info, it sounds like the patch DTRT.
>> Also, if the account IS expired, the user should be given a chance
>> to update their password.
>
> If pam_acct_mgmt failed for any reason other than PAM_NEW_AUTHTOK_REQD
> then no, they shouldn't.
That's what I just said.  Since the patch doesn't have the effect I
thought it
did, you can obviously ignore this comment.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188





------- Comment #7 from dtucker at zip.com.au  2006-05-04 12:57 -------
(In reply to comment #6)
[...]
> it sounds like the patch DTRT.
Thanks for reviewing it.
> >> Also, if the account IS expired, the user should be given a chance
> >> to update their password.
> >
> > If pam_acct_mgmt failed for any reason other than
> > PAM_NEW_AUTHTOK_REQD then no, they shouldn't.
> 
> That's what I just said.  Since the patch doesn't have the effect I
> thought it did, you can obviously ignore this comment.
Actually you said "if the *account* is expired.  Since you apparently
meant "if the password is expired" then we're in agreement and you
can
ignore my comment too :-)




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1188


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED




------- Comment #8 from dtucker at zip.com.au  2006-05-15 17:23 -------
Patch applied, thanks.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.