bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PAM support lacks pam_setcred() call


From: Derek Robert Price
Subject: Re: PAM support lacks pam_setcred() call
Date: Tue, 21 Oct 2003 15:29:32 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Steve McIntyre wrote:

|On Tue, Oct 21, 2003 at 02:13:43PM +0100, Steve McIntyre wrote:
|
|>Attached. I've added a couple of new config options (PamAuth and
|
|
|Sorry. Really attached this time.


Thanks!  This particular patch looks pretty good.  I'd like to see it
against the trunk since that should merge more easily with the changes
Brian already made.  I've made a few other notes below.

|diff -ur cvs-1.12.1/config.h.in cvs-1.12.1.new/config.h.in
|--- cvs-1.12.1/config.h.in 2003-06-02 00:07:49.000000000 +0100
|+++ cvs-1.12.1.new/config.h.in 2003-05-25 15:40:03.000000000 +0100


Not needed - automatically generated.

|diff -ur cvs-1.12.1/configure cvs-1.12.1.new/configure
|--- cvs-1.12.1/configure    2003-01-16 20:16:56.000000000 +0000
|+++ cvs-1.12.1.new/configure 2003-01-16 20:16:56.000000000 +0000


Ditto.

|diff -ur cvs-1.12.1/configure.in cvs-1.12.1.new/configure.in
|--- cvs-1.12.1/configure.in    2003-01-16 20:16:56.000000000 +0000
|+++ cvs-1.12.1.new/configure.in    2003-01-30 01:32:00.000000000 +0000
|@@ -258,6 +258,12 @@
| AC_SEARCH_LIBS(getspnam, sec gen, AC_DEFINE(HAVE_GETSPNAM, 1,
| [Define if you have the getspnam function.]))
|
|+dnl
|+dnl Check for pam support.
|+dnl
|+AC_SEARCH_LIBS(pam_open_session, pam, AC_DEFINE(HAVE_PAM, 1,
|+[Define if you have the pam_open_session function.]))
|+
| AC_FUNC_UTIME_NULL
| AC_SYS_LONG_FILE_NAMES
|


I like this in general, but for now, while this is an experimental
feature, I prefer the user-enabled version already commited based on
Brian's patch.  If Brian's isn't also checking that PAM is present, it
might not hurt to merge the two and issue a warning when the user
specifies that PAM should be used and the library couldn't be found.

|diff -ur cvs-1.12.1/src/cvs.h cvs-1.12.1.new/src/cvs.h
|--- cvs-1.12.1/src/cvs.h    2002-12-28 18:01:30.000000000 +0000
|+++ cvs-1.12.1.new/src/cvs.h    2003-01-30 01:32:17.000000000 +0000
|@@ -457,6 +457,7 @@
| void root_allow_add PROTO ((char *));
| void root_allow_free PROTO ((void));
| int root_allow_ok PROTO ((char *));
|+void set_default_pam_user PROTO ((char *));
|
| char *gca PROTO((const char *rev1, const char *rev2));
| extern void check_numeric PROTO ((const char *, int, char **));
|diff -ur cvs-1.12.1/src/main.c cvs-1.12.1.new/src/main.c
|--- cvs-1.12.1/src/main.c    2002-10-24 19:38:37.000000000 +0100
|+++ cvs-1.12.1.new/src/main.c    2003-01-30 01:32:56.000000000 +0000
|@@ -416,6 +416,9 @@
|     {"help-synonyms", 0, NULL, 2},
|     {"help-options", 0, NULL, 4},
|     {"allow-root", required_argument, NULL, 3},
|+#ifdef HAVE_PAM
|+        {"default-pam-user", required_argument, NULL, 5},
|+#endif
|         {0, 0, 0, 0}
|     };
|     /* `getopt_long' stores the option index here, but right now we
|@@ -520,6 +523,12 @@
|         /* --allow-root */
|         root_allow_add (optarg);
|         break;
|+#ifdef HAVE_PAM
|+        case 5:
|+            /* --default-pam-user */
|+            set_default_pam_user (optarg);
|+            break;
|+#endif
|         case 'Q':
|         really_quiet = 1;
|         /* FALL THROUGH */


This is nice.  Perhaps an #else that prints some message such as "PAM
not enabled in this executable" would be appropriate.

|diff -ur cvs-1.12.1/src/parseinfo.c cvs-1.12.1.new/src/parseinfo.c
|--- cvs-1.12.1/src/parseinfo.c    2002-12-06 19:09:26.000000000 +0000
|+++ cvs-1.12.1.new/src/parseinfo.c    2003-01-30 01:33:22.000000000 +0000
|@@ -398,6 +398,27 @@
|         else if (strcmp (p, "stat") == 0)
|           RereadLogAfterVerify = LOGMSG_REREAD_STAT;
|     }
|+    else if (strcmp (line, "DefaultPamUser") == 0)
|+    {
|+#ifdef HAVE_PAM
|+        set_default_pam_user(p);
|+#endif
|+    } /* Don't complain if we don't have PAM here... */
|+    else if (strcmp (line, "PamAuth") == 0)
|+    {
|+        if (strcmp (p, "no") == 0)
|+#ifdef HAVE_PAM
|+            pam_auth = 0;
|+#else
|+            ;
|+#endif
|+        else if (strcmp (p, "yes") == 0)
|+#ifdef HAVE_PAM
|+            pam_auth = 1;
|+#else
|+            ;
|+#endif
|+    } /* Don't complain if we don't have PAM here... */
|     else
|     {
|         /* We may be dealing with a keyword which was added in a
|diff -ur cvs-1.12.1/src/server.c cvs-1.12.1.new/src/server.c
|--- cvs-1.12.1/src/server.c    2003-06-01 23:41:37.000000000 +0100
|+++ cvs-1.12.1.new/src/server..c    2003-10-13 00:39:24.000000000 +0100
|@@ -16,6 +16,13 @@
| #include "getline.h"
| #include "buffer.h"
|
|+#ifdef HAVE_PAM
|+#include <security/pam_misc.h>
|+#include <security/pam_appl.h>
|+static char *default_pam_username = NULL;
|+int pam_auth = 1;
|+#endif
|+
| #if defined(SERVER_SUPPORT) || defined(CLIENT_SUPPORT)
|
| # ifdef HAVE_GSSAPI
|@@ -117,7 +117,7 @@
|
| /* Should we check for system usernames/passwords?  Can be changed by
|    CVSROOT/config.  */
|-int system_auth = 1;
|+int system_auth = 0;


I think I would prefer defaults of pam_auth=0 and system_auth=1, at
least for now.  It's never nice to change defaults on users unless we
can't help it.  If PAM ever ceases to be an experimental feature, then
we can consider it, but system_auth should certainly never default to 0
when !HAVE_PAM.

|
| # endif /* AUTH_SERVER_SUPPORT */
|
|@@ -5431,6 +5438,142 @@
|     return retval;
| }
|
|+#ifdef HAVE_PAM
|+/* The callback function that the pam modules will use to talk to
|+   us. Modelled closely on the misc_conv module of Linux-PAM. This
|+   blatantly subverts one of the principles of PAM - PAM is meant to
|+   handle all the password work. But this does the job and means I can
|+   transition to LDAP right now. SAM 2001/12/23 */
|+int cvs_conv(int num_msg, const struct pam_message **msgm,
|+             struct pam_response **response, void *appdata_ptr)
|+{
|+    int count=0;
|+    struct pam_response *reply;
|+
|+    if (num_msg <= 0)
|+        return PAM_CONV_ERR;
|+ |+ reply = (struct pam_response *) calloc(num_msg, sizeof(struct
pam_response));
|+    if (reply == NULL)
|+        return PAM_CONV_ERR;
|+ |+ for (count=0; count < num_msg; ++count)
|+    {
|+        char *string=NULL;
|+
|+        switch (msgm[count]->msg_style)
|+        {
|+            case PAM_PROMPT_ECHO_OFF:
|+            case PAM_PROMPT_ECHO_ON:
|+                string = (char *)appdata_ptr;
|+                break;
|+            default:
|+                break;
|+        }
|+
|+        if (string) /* must add to reply array */
|+ { |+ /* add string to list of responses */ |+ reply[count].resp_retcode = 0;
|+            reply[count].resp = string;
|+            string = NULL;
|+        }
|+    }
|+
|+    *response = reply;
|+    reply = NULL;
|+
|+    return PAM_SUCCESS;
|+}
|+
|+static struct pam_conv conv = {
|+    cvs_conv,
|+    NULL
|+};
|+
|+/* Modelled very closely on the example code in "The Linux-PAM
|+   Application Developers' Guide" by Andrew G. Morgan. */
|+/* Return a hosting username if password matches, else NULL. */
|+static char *
|+check_pam_password (username, password, repository)
|+    char *username, *password, *repository;
|+{
|+    pam_handle_t *pamh = NULL;
|+    struct passwd *pw = NULL;
|+    int retval;
|+    int rc = 0;
|+    char *host_user = NULL;
|+
|+    conv.appdata_ptr = xstrdup(password);
|+
|+    retval = pam_start("cvs", username, &conv, &pamh);
|+    if (retval != PAM_SUCCESS)
|+        return NULL;
|+
|+    if (retval == PAM_SUCCESS)
|+        retval = pam_authenticate(pamh, 0);    /* is user really user? */
|+
|+    if (retval == PAM_SUCCESS)
|+        retval = pam_acct_mgmt(pamh, 0);       /* permitted access? */
|+
|+    /* This is where we have been authorized or not. */
|+
|+    switch(retval)
|+    {
|+        case PAM_SUCCESS:
|+            host_user = xstrdup(username);
|+            rc = 1;
|+            break;
|+        case PAM_AUTH_ERR:
|+            host_user = NULL;
|+            rc = 2;
|+            break;
|+        default:
|+            host_user = NULL;
|+            rc = 0;
|+            break;
|+    }
|+
|+    /* now close PAM */
|+    if ( (rc != 0) && (pam_end(pamh,retval) != PAM_SUCCESS) )
|+ { |+ pamh = NULL;
|+        error(0, 0, "pam: failed to release authenticator\n");
|+    }
|+
|+    /* An issue with using pam is that the host may well not have a
|+       local user entry to match the authenticated user. Check with
|+       getpwnam; if that fails, then we can optionally fall back to a
|+       specified local username */
|+    if(1 == rc)
|+    {
|+        pw = getpwnam (host_user);
|+        if (pw == NULL)
|+        {
|+            if(NULL != default_pam_username)
|+            {
|+                free(host_user);
|+                host_user = xstrdup(default_pam_username);
|+                /* And don't check existence again - switch_to_user()
|+                   will do it for us later */
|+            }
|+        }
|+    }
|+
|+    return host_user;
|+}
|+
|+/* Set the default user to use for a remote pam user for whom
|+   getpwnam() will fail */
|+void
|+set_default_pam_user (username)
|+    char *username;
|+{
|+    if( (username != NULL) && (strlen(username) > 0))
|+        default_pam_username = xstrdup(username);
|+}
|+
|+#endif /* HAVE_PAM */
|
| /* Return a hosting username if password matches, else NULL. */
| static char *
|@@ -5460,7 +5603,11 @@
|
|     assert (rc == 0);
|
|+#ifdef HAVE_PAM
|+    if (!pam_auth && !system_auth)
|+#else
|     if (!system_auth)
|+#endif /* HAVE_PAM */
|     {
|     /* Note that the message _does_ distinguish between the case in
|        which we check for a system password and the case in which
|@@ -5581,7 +5728,7 @@
|     char *password = NULL;
|     size_t password_allocated = 0;
|
|-    char *host_user;
|+    char *host_user = NULL;
|     char *descrambled_password;
| #endif /* AUTH_SERVER_SUPPORT */
|     int verify_and_exit = 0;
|@@ -5715,7 +5862,15 @@
|
|     /* We need the real cleartext before we hash it. */
|     descrambled_password = descramble (password);
|-    host_user = check_password (username, descrambled_password,
repository);
|+
|+    if(NULL == host_user)
|+        host_user = check_password (username, descrambled_password,
repository);
|+
|+#ifdef HAVE_PAM
|+    if (NULL == host_user && pam_auth)
|+        host_user = check_pam_password (username,
descrambled_password, repository);
|+#endif /* HAVE_PAM */
|+
|     if (host_user == NULL)
|     {
| #ifdef HAVE_SYSLOG_H
|diff -ur cvs-1.12.1/src/server.h cvs-1.12.1.new/src/server.h
|--- cvs-1.12.1/src/server.h    2003-05-19 18:57:50.000000000 +0100
|+++ cvs-1.12.1.new/src/server.h    2003-10-13 00:12:18.000000000 +0100
|@@ -142,6 +142,9 @@
| #ifdef AUTH_SERVER_SUPPORT
| extern char *CVS_Username;
| extern int system_auth;
|+#ifdef HAVE_PAM
|+extern int pam_auth;
|+#endif /* HAVE_PAM */
| #endif /* AUTH_SERVER_SUPPORT */
|
| #endif /* SERVER_SUPPORT */
|diff -ur cvs-1.12.1/doc/cvs.texinfo cvs-1.12.1.new/doc/cvs.texinfo
|--- cvs-1.12.1/doc/cvs.texinfo    2003-05-27 00:22:02.000000000 +0100
|+++ cvs-1.12.1.new/doc/cvs.texinfo 00:51:00.000000000 +0100
|@@ -2465,23 +2465,40 @@
| colon after the @sc{cvs} username is always necessary,
| even if the password is empty.
|
|-@sc{cvs} can also fall back to use system authentication.
|-When authenticating a password, the server first checks
|-for the user in the @file{$CVSROOT/CVSROOT/passwd}
|-file.  If it finds the user, it will use that entry for
|-authentication as described above.  But if it does not
|-find the user, or if the @sc{cvs} @file{passwd} file
|-does not exist, then the server can try to authenticate
|-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
|-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.
|+@sc{cvs} can also fall back to use system
|+authentication.  When authenticating a password, the
|+server first checks for the user in the
|+@file{$CVSROOT/CVSROOT/passwd} file. If it finds the
|+user, it will use that entry for authentication as
|+described above.  But if it does not find the user, or
|+if the @sc{cvs} @file{passwd} file does not exist, then
|+the server can try to authenticate the username and
|+password using the operating system's user-lookup
|+routines. There are two sets of user-lookup routines
|+available, first PAM (Pluggable Authentication Modules)
|+and then the normal Unix-style passwd file. The
|+"fallback" behaviour can be controlled using the two
|+variables @code{PamAuth} and @code{SystemAuth}. On a
|+Debian system, @code{PamAuth} defaults to @code{yes}
|+and @code{SystemAuth} to @code{no} - after all, PAM can
|+supports passwd file lookups itself. Changing these is
|+possible by setting @code{PamAuth=no} and
|+@code{SystemAuth=yes} in the @sc{cvs} @file{config}
|+file, @pxref{config}). 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.
|+
|+If you wish to use PAM for authentication, and details
|+of your users are not available using getpwnam(), you
|+can set a default PAM username on the server, either by
|+setting @code{DefaultPamUser=user} in the @sc{cvs}
|+@file{config} file, @pxref{config}, or by adding a
|+command-line option @sc{--default-pam-user user} on the
|+server command line.
|
| Right now, the only way to put a password in the
| @sc{cvs} @file{passwd} file is to paste it there from


Derek

- --
~                *8^)

Email: derek@ximbiot.com

Get CVS support at <http://ximbiot.com>!
- --
"May the forces of evil become confused on the way to your house."

           -- George Carlin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)
Comment: Using GnuPG with Netscape - http://enigmail.mozdev.org

iD8DBQE/lYkaLD1OTBfyMaQRAsyMAKCqTG7OyfOZrDhpFM4TUkFYLDf5YACgpYyK
YkPJNF35WfIPFP9Jt+0rKjw=
=KuDt
-----END PGP SIGNATURE-----






reply via email to

[Prev in Thread] Current Thread [Next in Thread]