openssh 3.6.1_p2 problem with pam (fwd)

[Frank Cusack]

OK, here is my take on how PAM flow should go, in pseudocode, with #ifdef
commentary on where openssh-3.6.1p2 strays.  I've just submitted a patch

http://bugzilla.mindrot.org/show_bug.cgi?id=559
http://bugzilla.mindrot.org/attachment.cgi?id=289&action=view

which adresses some of the problems below.  The description below isn't as
readable as I had hoped, but at least it does show PAM behavior as a
single flow of control; whereas in openssh you'll have to look into a
number of files.

#ifdef OPENSSH
pw = getpwbynam(user);
pamh = pam_start(service, pw ? user : "NOUSER", ...);
#else
pamh = pam_start(service, user, ...);
#endif

if (options.permit_empty_password)
    pam_auth_flags = 0;
else
    pam_auth_flags = PAM_DISALLOW_NULL_AUTHTOK;

if (protocol == 1) {
    if (password_auth_enabled()) {
	/* kludge */
	set_conv(non_interactive_conv_func);

#ifdef OPENSSH
	{
#else
	if (options.permit_empty_password) {
#endif
	    /*
	     * Need to do this because of a password_auth deficiency:  the
	     * client prompts for the password before the server asks for it.
	     * This breaks "don't prompt on null password" expected behavior.
	     *
	     * Results in a spurious pam log for accounts with password. :-(
	     * And is just plain broken, for the general case--consider
	     * a PAM module that only allows x attempts in y secs.
	     */
	    set_password("");
#ifdef OPENSSH
	    /*
	     * NB: pam_auth_flags might be PAM_DISALLOW_NULL_AUTHTOK,
	     * in which case this call to pam_authenticate() is pointless.
	     * See above, where we test options.permit_empty_password first.
	     */
#endif
	    pam_authenticate(pamh, pam_auth_flags);
	    if (authenticated)
		goto pam_account;
	}
	set_password(get_password());
	pam_authenticate(pamh, pam_auth_flags);
    }
}

if ("none" authentication) {
    /* protocol 2 does this first */
#ifdef OPENSSH
    {
#else
    if (password_auth_enabled() && options.permit_empty_password) {
	/* kbdint handles this itself */
#endif
	set_conv(non_interactive_conv_func);
	set_password("");
#ifdef OPENSSH
	/* see comment about pam_auth_flags, above */
#endif
	pam_authenticate(pamh, pam_auth_flags);
	if (authenticated)
	    goto pam_account;
    }
}


if (is_password_auth || is_kbd_int_auth) {
    if (is_password_auth)
	set_conv(non_interactive_conv_func);
    else
	set_conv(interactive_conv_function);
#ifdef OPENSSH
    /* NB: Ignores setting of options.permit_empty_password */
    if (protocol == 2)
	pam_auth_flags = 0;
#endif
    pam_authenticate(pamh, pam_auth_flags);
    if (!authenticated)
	return failure;
}

pam_account:
r = pam_acct_mgmt(pamh, pam_auth_flags);
if (r == PAM_SUCCESS)
    return success;

#ifndef OPENSSH
if (r != PAM_NEW_AUTHTOK_REQD)
#endif
    /* NB: OPENSSH notes below don't matter since we fail here */
    return failure;

/* new password required */
if (is_password_auth) {
    /* password_auth deficiency, argh */
    flag_password_change_needed();
    return success;
}

if (is_kbd_int_auth) {
#ifdef OPENSSH
    /* password_auth deficiency, not a kbd_int deficiency, argh! */
    flag_password_change_needed();
    return success;
#else
    /* PAM_CHANGE_EXPIRED_AUTHTOK is iffy */
    return ((pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK) == 0)
	    ? success
	    : failure);
#endif
}

/* non password, non kbd-int, but password expired ... a tough call */
return success;

In addition to the parts noted above, additional bugs:

- #if 0'd out code for pam_chauthtok && privsep, even though it works
  correctly for non-privsep, AFAIK (auth-pam.c:251; auth-pam.c:347)
- extraneous failure message for 'none' auth (noise)
- PAM_TEXT_INFO and PAM_ERROR_MSG messages are aggregrated rather than passed
  to the client in order; if the conversation is *only* PAM_TEXT_INFO and
  PAM_ERROR_MSG messages the client doesn't see them at all, for kbdint
- do_pam_conversation_kbd_int() leaks char *text
- protocol 2 assumes client will do 'none' authentication
- kbdint authentication cannot be abandoned
- user:style is not supported by the ietf-drafts

/fc