Adding Access-Challenge support for rlm_pam

[Alan DeKok]

Kenny Root wrote:
> Instead I've worked a bit to enable Access-Challenge support in rlm_pam.
> This is done in terms of PAM_CONV_AGAIN which allows rlm_pam to send
> Access-Challenge and keep state in a structure similar to how
> rlm_securid works.

  OK.  That's probably not the best idea, tho.  The SecurID code works,
but it's not the best example to use.

> This is the first time I've looked at the source code for FreeRADIUS, so
> I'd welcome any review of my code as it currently stands. It doesn't
> currently behave like the existing rlm_pam. For instance, it doesn't try
> to submit the first password it sees as the reponse to the first
> pam_conv message. However, I should be able to fix that soon.

  OK.  That could be configurable.

  The code mostly looks OK.  I have minor formatting concerns (as always).

  I'm unsure why the module needs to track multiple challenges.  If the
client re-transmits an Access-Request packet, the server will
re-transmit a cached response without bothering the module.  So the
module really only needs to track one outstanding challenge for a State
attribute.

  And the pam_session_cmp() function is completely wrong.  The sessions
need to be tracked by State variable, not by client IP address.  The
current code means that there can only be one user doing challenges at
any one NAS.

  See rlm_eap for a better handling of session && state.  The idea is this:

- access-request without state:

  - new session = allocate session, create 16-byte random state,
  insert session into tree, keyed by state

  - send challenge + state to the user

- access-request with state:

  - look up session in tree, keyed by state

  - barf if it doesn't exist

  - process session, and update session data


  Given that this is a common pattern, there should really be an API in
the server core, used by multiple modules.  That would let all of the
modules requiring State to use it.  It would also mean less duplicated
code, and more *correct* code.

  Alan DeKok.