[gdm-list] Jon - some questions about GDM & PAM

[Brian Cameron <Brian Cameron Sun COM>]

Jon:

I've been more carefully reviewing the D-Bus GDM PAM code and how it 
relates to auditing, and I have some comments and questions about
the new code:

One thing I notice that is quite different in the new GDM is that
in the old GDM, logging in with PAM was handled as one discrete action
in the gdm_verify_user function of daemon/verify-pam.c (also as one
discrete action in gdm_verify_setup_user which is used for autologin).

In the new GDM D-Bus branch, it seems that this is split into two
steps:

- gdm_session_worker_verify_user - manages initializing of PAM
  and the user authentication steps.
- gdm_session_worker_open_user_session - manages the opening
  of the session.

In the old code, we audit login SUCCESS and FAILURE after the
opening of the session.  In the new design, do you think we should
still do this, or do you think we should just audit at the end
of gdm_session_worker_verify_user?  I'd think we would want to
continue doing the audit after the pam_open_session so that if
there are any PAM related errors, this gets logged as a FAILURE
and not a SUCCESS.  No?

I've been trying to understand why we want to split out the
pam_open_session into a separate step.  Looking at the code, I
notice there are two paths to calling pam_open_session in
daemon/gdm-session-worker.c:

- gdm_session_worker_start_program called by on_start_program
  which corresponds to the signal StartProgram.

  This StartProgram signal comes from daemon/gdm-session.c in the
  gdm_session_start_program function.  This seems to be the normal
  case where the session has asked PAM to verify the username
  password via gdm_session_get_username in daemon/gdm-product-slave.c
  and dameon/gdm-simple-slave.c.  This makes sense to me.

- gdm_session_worker_open called by open_idle.  The idle
  handler seems to be setup in on_begin_verification and in
  on_begin_verification_for_user.  These correspond to the
  signals BeginVerification and BeginVerificationForUser

  These signals seem to get generated in gdm-session when the
  opened signal is generated via handle_connection.  How this
  works isn't clear to me.  Could you explain?

  I notice this comment in gdm_session_worker_open, which sounds
  like it relates to this, but even after reading the comment I'm
  still not clear what the code is doing.

        /* Did start_program get called early? if so, process it now,
         * otherwise we'll do it asynchronously later.
         */

I also notice that worker->priv->arguments only gets set when
the StartProgram signal is received.  Since
gdm_session_worker_open_user_session wants to run the command in
worker->priv->arguments, then I'm guessing the code wouldn't work right
if BeginVerification were received before StartProgram.  Is it just
known that this could never happen?

----

Moving along to how to approach integrating the audit code into
the new D-Bus branch of GDM, I have reviewed the existing audit
code and note the following requirements to support both ADH and
libaudit style auditing:

1) for login success we need to have the following information
   - username   (required by both ADH and libaudit)
   - hostname   (required by libaudit)
   - devicename (required by libaudit)
   - a boolean set to FALSE if the user was asked to change
     their password and this failed, or set to TRUE if the
     user was asked to change their password and had success.
     (requred by ADH)

2) for login failure we need the following information
   - username    (required by both ADH and libaudit)
   - hostname    (required by both ADH and libaudit)
   - devicename  (required by both ADH and libaudit)
   - a boolean set to FALSE if the user was asked to change
     their password and this failed, or set to TRUE if the
     user was asked to change their password and had success.
     (required by ADH)
   - the failed PAM return code (required by ADH)

The above two seem pretty straightforward to abstract.  I'm
guessing we can just have two functions that take all the
above arguments and let the libaudit/ADH backends just use
the ones they want.

3) for logout, the ADH interface requires that you pass in a
   data structure that is passed back when you call the login
   success function above.  I'm guessing we'll need to add
   a gpointer to the GdmSessionWorkerPrivate structure to
   store this so when we call gdm_session_worker_uninitialize_pam
   we can call this function?  Since libaudit doesn't need this,
   this gpointer could just hold NULL and the function could be
   a stub.

If we decide that we should handle auditing login success/failure
after the opening of the session, does this mean that we also
need to cache the hostname, devicename, pamerr, and the "did the
user change password" boolean in the GdmSessionWorkerPrivate
structure as well?  At the moment, I notice that the code doesn't
keep track of this information when the BeginVerification signal
is managed.

Brian