Re: PAM authentication patch - v2

[Brian Murphy]

Mark D. Baushke wrote:

> A possible replacement for the awkard parts of the paragraph might be
> the following:
>
>  
Sounded fine to me and now included in the attached patch.

> Note: Somewhere along the line, it may be desirable to mark the PAM
> feature as an experimental feature that may not make it into a fully
> released version of cvs and suggest positive feedback of the feature be
> sent to the either the bug-cvs or info-cvs e-mail addresses.
>
>  
Also done.

Also I fixed a stupid configure error which meant the default PAM 
service name
was "".

I hope this makes everyone happy ;-).

/Brian

? contrib/pam
Index: config.h.in
===================================================================
RCS file: /cvs/ccvs/config.h.in,v
retrieving revision 1.74
diff -u -r1.74 config.h.in
--- config.h.in 19 Mar 2003 21:13:29 -0000      1.74
+++ config.h.in 27 Apr 2003 20:00:22 -0000
@@ -206,6 +206,12 @@
 /* Define to 1 if you have the <ndir.h> header file, and it defines `DIR'. */
 #undef HAVE_NDIR_H
 
+/* Defined to 1 if you use PAM system authentication instead of getpwnam */
+#undef HAVE_PAM
+
+/* Define if you want a hardcoded PAM service name */
+#undef PAM_SERVICE_NAME
+
 /* Define to 1 if you have the `putenv' function. */
 #undef HAVE_PUTENV
 
Index: configure.in
===================================================================
RCS file: /cvs/ccvs/configure.in,v
retrieving revision 1.188
diff -u -r1.188 configure.in
--- configure.in        11 Apr 2003 17:36:00 -0000      1.188
+++ configure.in        27 Apr 2003 20:00:23 -0000
@@ -544,6 +544,24 @@
 dnl
 
 
+
+dnl
+dnl --with-hardcoded-pam-service-name
+dnl
+AC_ARG_WITH(
+  [hardcoded-pam-service-name],
+  AC_HELP_STRING(
+    [--with-hardcoded-pam-service-name],
+    [use this to hard code a service name for PAM cvs authentication 
+    (defaults to the name cvs is invoked as)]))
+
+if test -n "$with_hardcoded_pam_service_name"; then
+  AC_DEFINE_UNQUOTED(PAM_SERVICE_NAME, "$with_hardcoded_pam_service_name",
+    [Define to hardcode a service name for PAM])
+fi
+
+
+
 dnl
 dnl Find a temporary directory
 dnl
@@ -739,6 +757,37 @@
        the CVS client disabled (--disable-client)])
   fi
 fi
+
+
+
+dnl
+dnl Check if PAM authentication is enabled
+dnl
+AC_ARG_ENABLE(
+  [pam],
+  AC_HELP_STRING(
+    [--enable-pam],
+    [Use to enable system authentication with PAM instead of using the 
+    simple getpwnam interface.  This allows authentication (in theory) 
+    with any PAM module, e.g. on systems with shadow passwords or via LDAP]), ,
+  [enable_pam=yes]
+  )
+
+if test yes = $enable_pam; then
+  AC_CHECK_HEADER(security/pam_appl.h, 
+    AC_DEFINE(HAVE_PAM, 1, 
+    [Define to enable system authentication with PAM instead of using the 
+    simple getpwnam interface.  This allows authentication (in theory) 
+    with any PAM module, e.g. on systems with shadow passwords or via LDAP])
+    AC_CHECK_LIB(pam, pam_start, [LIBS="${LIBS} -lpam"],
+      AC_MSG_ERROR([Could not find PAM libraries but the headers exist.
+      Give the --disable-pam option to compile without PAM support (or fix
+      your broken configuration)])
+    ),
+    AC_MSG_WARN([PAM authentication disabled - could not find PAM headers])
+  )
+fi
+
 
 
 dnl
Index: doc/cvs.texinfo
===================================================================
RCS file: /cvs/ccvs/doc/cvs.texinfo,v
retrieving revision 1.565
diff -u -r1.565 cvs.texinfo
--- doc/cvs.texinfo     31 Mar 2003 19:29:48 -0000      1.565
+++ doc/cvs.texinfo     27 Apr 2003 20:00:32 -0000
@@ -2489,13 +2489,104 @@
 the username and password using the operating system's
 user-lookup routines (this "fallback" behavior can be
 disabled by setting @code{SystemAuth=no} in the
-@sc{cvs} @file{config} file, @pxref{config}).  Be
-aware, however, that falling back to system
+@sc{cvs} @file{config} file, @pxref{config}).
+
+The default fallback behaviour is to look in 
+@file{/etc/passwd} for this system password but if your
+system has PAM - Pluggable Authentication Modules - 
+and @sc{cvs} is configured to use it at compile time 
+then it will be used instead. This means that with a 
+global configuration file usually @file{/etc/pam.conf}
+or possibly @file{/etc/pam.d/cvs}
+you can tell cvs to use LDAP or normal UNIX passwd 
+authentication or many other possibilities - see your
+PAM documentation for details. 
+
+Note that PAM is an experimental feature so feedback is encouraged. 
+Please send a mail to one of the @sc{cvs} mailing lists
+@code{bugs-cvs@@gnu.org} or @code{info-cvs@@gnu.org} if you use the 
+@sc{cvs} PAM support.
+
+Using PAM gives the system administrator much more 
+flexibility in how cvs users are authenticated but 
+no more security than other methods, see below. 
+
+CVS needs an "auth" and "account" module in the 
+PAM configuration file. A typical PAM configuration 
+would therefore have the following lines 
+in @file{/etc/pam.conf} to emulate the standard @sc{cvs} 
+system @file{/etc/passwd} authentication:
+
+@example
+cvs    auth        required    pam_unix.so
+cvs    account     required    pam_unix.so
+@end example
+
+The the equivalent @file{/etc/pam.d/cvs} would contain
+
+@example
+auth       required    pam_unix.so
+account            required    pam_unix.so
+@end example
+
+Some systems require a full path to the module so that
+@file{pam_unix.so} (Linux) would become something like 
+@file{/usr/lib/security/$ISA/pam_unix.so.1} (Sun Solaris).
+See the @file{contrib/pam} subdirectory of the @sc{cvs}
+source distribution for further example configurations.
+
+The PAM service name given above as "cvs" is just
+the service name in the default configuration. 
+The PAM service name is in fact the same as the name 
+the @sc{cvs} binary is invoked as. This means that you
+can have several different authentication configurations. 
+You can also chose at compile time to remove this 
+flexibility and hard code a PAM service name into the 
+binary by configuring using the hardcoded-pam-service-name
+option thus:
+
+@example
+./configure --with-hardcoded-pam-service-name="cvs"
+@end example
+
+substituting "cvs" for whatever you wish the service name
+to be. No matter how the binary is now invoked it will always
+use the same service name, "cvs" in this case.
+
+Be aware, however, that falling back to system
 authentication might be a security risk: @sc{cvs}
 operations would then be authenticated with that user's
 regular login password, and the password flies across
 the network in plaintext.  See @ref{Password
 authentication security} for more on this.
+This may be more of a problem with PAM authentication
+because it is likely that the source of the system 
+password is some central authentication service like
+LDAP which is also used to authenticate other services.
+
+On the other hand, PAM makes it very easy to change your password
+regularly. If they are given the option of a one-password system for
+all of their activities, users are often more willing to change their
+password on a regular basis.
+
+In the non-PAM configuration where the password is stored in the
+@file{CVSROOT/passwd} file, it is difficult to change passwords on a
+regular basis since only administrative users (or in some cases
+processes that act as an administrative user) are typicaly given
+access to modify this file. So, either there needs to be some
+hand-crafted web page or set-uid program to update the file, or the
+update needs to be done by submitting a request to an administrator do
+perform the duty by hand. In the first case, having to remember to
+update a separate password on a periodic basis can be difficult. In
+the second case, the manual nature of the change will typically mean
+that the password will not be changed unless it is absolutely
+necessary.
+
+Note that PAM administrators should probably avoid configuring
+one-time-passwords (OTP) for @sc{cvs} authentication/authorization. If
+OTPs are desired, the administrator may wish to encourage the use of
+one of the other Client/Server access methods. See the section on
+@pxref{Remote repositories} for a list of other methods.
 
 Right now, the only way to put a password in the
 @sc{cvs} @file{passwd} file is to paste it there from
Index: src/server.c
===================================================================
RCS file: /cvs/ccvs/src/server.c,v
retrieving revision 1.290
diff -u -r1.290 server.c
--- src/server.c        17 Mar 2003 06:32:11 -0000      1.290
+++ src/server.c        27 Apr 2003 20:00:35 -0000
@@ -5548,66 +5548,118 @@
     return retval;
 }
 
+#ifdef HAVE_PAM
 
-/* Return a hosting username if password matches, else NULL. */
-static char *
-check_password (username, password, repository)
-    char *username, *password, *repository;
-{
-    int rc;
-    char *host_user = NULL;
-    char *found_passwd = NULL;
-    struct passwd *pw;
+#include <security/pam_appl.h>
 
-    /* First we see if this user has a password in the CVS-specific
-       password file.  If so, that's enough to authenticate with.  If
-       not, we'll check /etc/passwd. */
+#ifndef PAM_SERVICE_NAME
+#define PAM_SERVICE_NAME program_name
+#endif
 
-    rc = check_repository_password (username, password, repository,
-                                   &host_user);
+struct cvs_pam_userinfo {
+    char *username;
+    char *password;
+};
+
+static int
+cvs_pam_conv(num_msg, msg, resp, appdata_ptr)
+    int num_msg; 
+    const struct pam_message **msg;
+    struct pam_response **resp; 
+    void *appdata_ptr;
+{
+    int i;
+    struct pam_response *response;
+    struct cvs_pam_userinfo *ui = (struct cvs_pam_userinfo *)appdata_ptr;
 
-    if (rc == 2)
-       return NULL;
+    assert (ui && ui->username && ui->password && msg && resp);
 
-    if (rc == 1)
+    response = xmalloc(num_msg * sizeof(struct pam_response));
+    memset(response, 0, num_msg * sizeof(struct pam_response));
+
+    for (i = 0; i < num_msg; i++)
     {
-       /* host_user already set by reference, so just return. */
-       goto handle_return;
+       switch(msg[i]->msg_style) 
+       {
+           /* PAM wants a username */
+           case PAM_PROMPT_ECHO_ON:
+               response[i].resp = xstrdup(ui->username);
+               break;
+           /* PAM wants a password */
+           case PAM_PROMPT_ECHO_OFF:
+               response[i].resp = xstrdup(ui->password);
+               break;
+           case PAM_ERROR_MSG:
+           case PAM_TEXT_INFO:
+               printf("E %s\n",msg[i]->msg);
+               break;
+           /* PAM wants something we don't understand - bail out */
+           default:
+               goto cleanup;
+       }
     }
 
-    assert (rc == 0);
+    *resp = response;
+    return PAM_SUCCESS;
 
-    if (!system_auth)
+cleanup:
+    for (i = 0; i < num_msg; i++)
     {
-       /* Note that the message _does_ distinguish between the case in
-          which we check for a system password and the case in which
-          we do not.  It is a real pain to track down why it isn't
-          letting you in if it won't say why, and I am not convinced
-          that the potential information disclosure to an attacker
-          outweighs this.  */
-       printf ("error 0 no such user %s in CVSROOT/passwd\n", username);
+       if (response[i].resp)
+       {
+           free(response[i].resp);
+           response[i].resp = 0;
+       }
+    }
+    free(response);
+    return PAM_CONV_ERR;
+}
+
+static int
+check_system_password (username, password)
+    char *username, *password;
+{
+    pam_handle_t *pamh = NULL;
+    int retval;
+    struct cvs_pam_userinfo ui = { username, password };
+    struct pam_conv conv = { cvs_pam_conv, (void *)&ui };
 
+    retval = pam_start(PAM_SERVICE_NAME, username, &conv, &pamh);
+
+    if (retval == PAM_SUCCESS)
+       retval = pam_authenticate(pamh, 0);
+
+    if (retval == PAM_SUCCESS)
+       retval = pam_acct_mgmt(pamh, 0);
+
+    if (pam_end(pamh,retval) != PAM_SUCCESS)
+    {
+       printf("E Fatal error, aborting.\n
+               pam failed to release authenticator\n");
        error_exit ();
     }
 
-    /* No cvs password found, so try /etc/passwd. */
-
+    return (retval == PAM_SUCCESS);       /* indicate success */
+}
+#else
+static int
+check_system_password (username, password)
+    char *username, *password;
+{
+    char *found_passwd = NULL;
+    struct passwd *pw;
 #ifdef HAVE_GETSPNAM
     {
        struct spwd *spw;
 
        spw = getspnam (username);
        if (spw != NULL)
-       {
            found_passwd = spw->sp_pwdp;
-       }
     }
 #endif
 
     if (found_passwd == NULL && (pw = getpwnam (username)) != NULL)
-    {
        found_passwd = pw->pw_passwd;
-    }
 
     if (found_passwd == NULL)
     {
@@ -5636,34 +5688,74 @@
     {
        /* user exists and has a password */
        if (strcmp (found_passwd, crypt (password, found_passwd)) == 0)
-       {
-           host_user = xstrdup (username);
-       }
+           return 1;
        else
        {
-           host_user = NULL;
 #ifdef LOG_AUTHPRIV
            syslog (LOG_AUTHPRIV | LOG_NOTICE,
                    "password mismatch for %s: %s vs. %s", username,
                    crypt(password, found_passwd), found_passwd);
 #endif
+           return 0;
        }
-       goto handle_return;
     }
 
-    if (password && *password)
-    {
-       /* user exists and has no system password, but we got
-          one as parameter */
-       host_user = xstrdup (username);
+#ifdef LOG_AUTHPRIV
+    syslog (LOG_AUTHPRIV | LOG_NOTICE,
+           "user %s authenticated because of blank system password",
+           username);
+#endif
+    return 1;
+}
+#endif
+
+/* Return a hosting username if password matches, else NULL. */
+static char *
+check_password (username, password, repository)
+    char *username, *password, *repository;
+{
+    int rc;
+    char *host_user = NULL;
+
+    /* First we see if this user has a password in the CVS-specific
+       password file.  If so, that's enough to authenticate with.  If
+       not, we'll check /etc/passwd. */
+
+    rc = check_repository_password (username, password, repository,
+                                   &host_user);
+
+    if (rc == 2)
+       return NULL;
+
+    if (rc == 1)
+       /* host_user already set by reference, so just return. */
        goto handle_return;
+
+    assert (rc == 0);
+
+    if (!system_auth)
+    {
+       /* Note that the message _does_ distinguish between the case in
+          which we check for a system password and the case in which
+          we do not.  It is a real pain to track down why it isn't
+          letting you in if it won't say why, and I am not convinced
+          that the potential information disclosure to an attacker
+          outweighs this.  */
+       printf ("error 0 no such user %s in CVSROOT/passwd\n", username);
+
+       error_exit ();
     }
 
-    /* user exists but has no password at all */
-    host_user = NULL;
+    /* No cvs password found, so try /etc/passwd. */
+    if ( check_system_password(username, password) )
+       host_user = xstrdup (username);
+    else
+       host_user = NULL;
+
 #ifdef LOG_AUTHPRIV
-    syslog (LOG_AUTHPRIV | LOG_NOTICE,
-           "login refused for %s: user has no password", username);
+    if (!host_user)
+       syslog (LOG_AUTHPRIV | LOG_NOTICE,
+               "login refused for %s: user has no password", username);
 #endif
 
 handle_return: