openssh vs pam

[Darren Tucker]

Frank Cusack wrote:
 > Darren (et al.),
 > Thanks for taking so much time here!  I wish I had more time to
 > devote to it.

That's OK.

I had a goal about a year ago to stop sshd's PAM interface being a 
source of complaint.  It's maybe halfway there, but to be honest I'm 
running out of enthusiasm for the job.

I tried to like PAM, really I did, but every time I started to like it 
just a little bit it would shaft me in one of the many ways I enumerated 
earlier.

Anyway, on with the show...

[about sshd's PAM interface]
[...]
>> It's never going to be perfect. Given the SSHv1 and v2 protocol
>> specs and PAM as it stands I don't think it's possible.
> 
> Disagree. It is quite possible for ssh v2 to work properly with PAM.
> I have it working. Sun has it working.

I specified SSHv1 and SSHv2, but sticking to just SSHv2, could you 
please explain how you do the following within the existing SSHv2 
protocol and PAM interfaces:

a) Perform the "none" auth test specified by SSHv2 via PAM without 
causing either unnecessary delays or spurious failure messages from PAM 
in the logs.  Currently sshd doesn't try PAM for "none" if 
PermitEmptyPasswords=no but strictly speaking that's a hack.

b) Perform vanilla "password" USERAUTH_REQUEST via PAM in a way that 
works for the general case.

Now a) you can configure around, and I imagine your response to b) will 
be "use kbdint, that's what it's for!" :-)

My point is there are parts of the spec that you can't implement 
perfectly via PAM, given the current protocol and PAM implementations. 
It's not that OpenSSH can't improve (I've already said that it can).

Originally I had another thing in the list, which I now think is 
possible but tricky, and wanted to know if you implemented it and if so, 
how:

c) Handle the case where pam_acct_mgmt() wants to interact after a 
public-key authentication.

>> 6) The multiple-messages part of the PAM conversation protocol (ie 
>> allowing more than one message per call to the conversation
>> function) is more complex than necessary and this has been a source
>> of bugs.
> 
> A big no here.
[...]
> Multiple messages are required because a) authentication exchanges
> are not limited to single request/reply type of messages, and b)
> clients cannot derive any semantic meaning from server-generated
> messages.

It's not that there are multiple messages in the authentication 
exchange, but that there's multiple authentication messages *per API 
call*, in a format that can be silently gotten wrong in a way that's a 
security problem (and, historically, has been).

Exhibit A: LinuxPAM got it wrong and nobody noticed until it was to late 
to fix it.  It's still broken to this day.

(For anyone following the thread that doesn't realise: the PAM spec 
(XSSO, pp 89) specifies that the message parameter passed to the 
conversation function is "a pointer to an array", but LinuxPAM treats it 
as an array of pointers.  These are equivalent for the case where 
there's exactly one message but will blow up horribly for more.)

Exhibit B: des at freebsd, who wrote one of the PAM implementations 
(OpenPAM) from scratch, got it wrong in the FreeBSD sshd PAM code:
http://mail-index.netbsd.org/current-users/2003/10/02/0005.html

So the authors of two of the three PAM implementations I'm aware of have 
got it wrong at one time or another, in at least two cases undetected 
long enough to make it to released code.

(and I'm not trying to pick on either des or amorgan here, but with a 
history of misuse of the interface even by folks that know it well, you 
have to ask if maybe the interface is at fault here)

A separate but related issue to the fact this complex interface is 
supplied via a callback.  It's the callback that's the primary problem 
in sshd's case, though.  Give me a reentrant equivalent to the 
conversation function and I can live with the rest.  It could probably 
even fit withing the existing API with a few new flags.

> This is a great example of both not understanding PAM and not wanting
> to. One, you're wrong, two, you can't imagine that the PAM interface
> might be the way it is for a reason, rather you insist that it's
> broken and refuse to accommodate it.

You seem to be confusing understanding it with having a high opinion of 
it.  Do you believe that PAM is so fundamentally clean, elegant and 
beautiful that if someone understood it they could not fail to like it?

As to why it is the way it is: it seemed me to be that way to make it 
easier to do X-based dialogs (the callback is a strong hint in that 
direction).  Whatever the reason, it doesn't alter the fact that it's a 
pain for me to deal with.

[...]
> Gobbledygook, I know, but we need only look at password expiry, 
> something you are quite familiar, dare I say, expert, in for a
> concrete example.
> 
> pam_authenticate():
>  info_request:  0: [enter password], type prompt
>  info_response: 0: 1234
> pam_acct_mgmt(): PAM_NEW_AUTHTOK_REQD
> pam_chauthtok():
>  info_request:  0: [your password has expired], type message
>                 1: [old password], type prompt
>                 2: [new password], type prompt
>                 3: [verify], type prompt
>  info_response: ...
> 
> 1) Look at how difficult it is to do this (in ssh) without PAM, then get 
> back to me about how PAM is broken.

That's more a function of whether or not the underlying OS has any kind 
of change-password API at all.  I could implement an eqivalent on AIX 
using chpass(), for example.

> 2) Openssh tends to assume a CLI client, and even then handles this 
> exchange incorrectly.

If you're referring to the accumulation of messages then see below.  If 
you're referring to something else then please elaborate.

> 3) With a GUI client, openssh is especially broken here (multiple 
> dialogs instead of one dialog).

OK, now that might be easily fixable (for Protocol 2, anyway).  I'll 
look at that when I have time.

> 4) This is a simple example.
> 
> I'm picking on password expiry because it is probably the most common
> issue, also since that is/was one of your focuses (foci?) I figure
> you can appreciate the problem. But the same problem exists purely in
> the pam_authenticate() phase.
> 
> I threw in the comment about semantic meaning to head off any
> arguments that the client might possibly understand that this is a
> password expiry and somehow coalesce messages. The server can
> communicate in any language, not just ASCII english. The client
> cannot be expected to understand what is going on, nor should it. Nor
> are multiple message exchanges limited to password expiry. So, PAM
> clients need to handle the general case.
> 
> Now, previously, in one sense I did overstate the case when I said
> "it's easy". The issue just described is a subtle one, and is
> probably not even understood by many of the current PAM stack
> implementers and maintainers. Even the authors of PAM don't describe 
> this in the PAM specification, and in fact they get a few things
> wrong! (viz, they claim kerberos and smartcard support, which isn't
> possible strictly within the PAM framework as specified.)

The authors of the standard and some of the implementations don't 
understand the interface?  Does this not indicate that the interface is 
too complex and thus prone to misuse?

> OTOH, I
> give an explicit example of the need for multiple messages in a
> single exchange in the kbdint draft, so this shouldn't be some new or
> novel requirement for ssh implementers. (I will say, though, that I 
> took heat from some WG folks for including such examples, but I
> insist on keeping them for what may now be obvious reasons.)
> 
> So, sure, understanding the PAM API isn't exactly "easy", but it's 
> certainly not "hard".
> 
> From an implementation perspective, it IS easy though.

Yet witness the continuing lack of "easy" patches demonstrating it.

> Perhaps not palatable, but still easy.

The only ways the conversation (callback) interface can be used by sshd 
are all ugly.  To me that means it's a broken API for this purpose 
regardless of the historic reasons for it.  But then, I tend to reject 
arguments for current brokenness based on historic brokenness :-)

> You can either use a nested event loop, as Nico shows, or read
> incoming data from an internal queue rather than the kernel packet
> buffer, as I have done (and I'm sure I've submitted patches for).
> Getting new ssh messages from a managed queue instead of from the
> kernel read() interface allows you to "putback" messages, thus you
> can escape the conversation function without nesting the event loop.
> I didn't actually implement a queue, I just buffered one message and
> added a hack to go back one message, because the pam conversation is
> the only place it was necessary, so a general purpose interface
> seemed unnecessarily complex.

The closest thing I could find to that description is this:
http://marc.theaimsgroup.com/?l=openssh-unix-dev&m=101073742305758
which seems to be only for abandoning a keyboard-interactive attempt, it 
doesn't seem to help the general case of handling conversation callbacks.

> The Sun patches are readily available.

Really?  Where?  If you're referring to OpenSolaris the when I looked, 
ssh was one of the components that is not part of the release.  (Nico: 
any particular reason for this, and is it expected to change?)

> Mine are not, and I'm sorry but I don't have time to generate them,
> but it's easy enough to implement from scratch.
[...]
> TEXT_INFO and ERROR_MSG buffering is broken (see password expiry
> example above for one specific case).

If you're referring to sshpam_query(), then remember that it is used for 
TIS C/R in SSHv1 as well as kbdint.

For the SSHv2/kbdint case I guess it could pass the messages through to 
the client directly rather than accumulating which should be an 
improvement, right?

For the other cases (eg password, or where sshpam_store_conv is used to 
accumulate messages from, eg, pam_acct_mgmt) there's no way to send the 
messages to the client before the session starts in SSHv1 (other than a 
disconnect message), and in SSHv2 sshd already copies the messages to 
the privsep slave and sends them the the client as SSH2 userauth banner 
messages.

> pam_start() with "NOUSER" is broken (described verbosely on the list in 
> the past).

Sure, but sshd hasn't done that for over two years.  See auth1.c rev 
1.84 and auth2.c rev 1.116.

> pam_getenvlist() is broken (you already know this).

sshd's use of pam_getenvlist is in the same category as threads: a 
necessary evil right now but to be removed when possible.

> I think there's something specifically wrong with privsep and PAM but
> I'll be damned if I can think of it now. (I've never used privsep
> because it was historically broken for PAM, nowadays some issues are
> fixed but there's no privsep value for me.)

I'll stipulate that privsep makes dealing with PAM harder.

That said, I thought I had a nice way of dealing with one of the 
outstanding problems relating to privsep and ran afoul of one of the 
many differences between PAM implementations (which is a regular occurance):
http://bugzilla.mindrot.org/show_bug.cgi?id=926

[...]
> OK, add that pam_set_data() and pam_get_data() is broken. Although, I
> don't get it since these should work.   Maybe the problem is that you
> fork a child (in addition to the privsep fork) to do PAM; that's
> broken unless you start PAM in the forked child.

The pam_set_data() thing is a symptom of the separate process thing.

This is an example of my "other of its characteristics make PAM as a 
whole harder" statement earlier: the combination of the blocking API 
calls combined with the fact that pam_set_data does such a good job of 
hiding the data is harder to deal with that either alone.

> Just guessing; I'm
> not familiar with the current implementation. I remember when it was
> added though, and I felt that the previous implementation was
> superior (for reasons that I've now forgotten).

You also said this about the new code:
http://marc.theaimsgroup.com/?l=openssh-unix-dev&m=104627748509519

"The code as a whole /is/ far cleaner than what exists currently, so 
that is a big plus."

Anyway, I do appreciate your time (and Nico's) even when I'm not 
agreeing with you.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
     Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.