bug-cvs
[Top][All Lists]
Advanced

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

Re: PAM authentication patch - v2


From: Matt Doar
Subject: Re: PAM authentication patch - v2
Date: 30 Jun 2003 14:01:58 -0700

Some feedback on this patch: I installed it last week against the head
of tree. Everything was fine during build and it worked as expected,
except that for NIS I had to login with users who are only defined in
NIS and *not* locally in /etc/passwd (seems fair enough). And some
documentation on debugging PAM authorization failures would be helpful.

I am hoping to use this in production sometime this summer.

~Matt

On Mon, 2003-06-30 at 12:17, Brian Murphy wrote:
> Derek Robert Price wrote:
> 
> > Hey Brian,
> >
> > I've been putting you off for a long time, haven't I?
> 
> Indeed.
> 
> > Sorry about that. Anyway, would you mind forwarding me the most recent 
> > version of your patch?  I've been looking through my email, but it was 
> > rather messy and I want to make sure I got the right patch.
> >
> See attachment for patch and changelog entries.
> 
> /Brian
> 
> ______________________________________________________________________
> 
> Index: configure.in
> ===================================================================
> RCS file: /cvs/ccvs/configure.in,v
> retrieving revision 1.207
> diff -u -r1.207 configure.in
> --- configure.in      23 Jun 2003 16:52:05 -0000      1.207
> +++ configure.in      26 Jun 2003 20:48:41 -0000
> @@ -566,6 +566,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
> @@ -761,6 +779,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=no]
> +  )
> +
> +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_ERROR([Could not find PAM headers])
> +  )
> +fi
> +
>  
> 
>  dnl
> Index: doc/cvs.texinfo
> ===================================================================
> RCS file: /cvs/ccvs/doc/cvs.texinfo,v
> retrieving revision 1.573
> diff -u -r1.573 cvs.texinfo
> --- doc/cvs.texinfo   16 Jun 2003 17:11:13 -0000      1.573
> +++ doc/cvs.texinfo   26 Jun 2003 20:48:50 -0000
> @@ -2478,13 +2478,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.298
> diff -u -r1.298 server.c
> --- src/server.c      11 Jun 2003 18:24:57 -0000      1.298
> +++ src/server.c      26 Jun 2003 20:48:53 -0000
> @@ -5425,66 +5425,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)
>      {
> @@ -5513,34 +5565,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:
> --- /dev/null 2003-04-09 22:42:25.000000000 +0200
> +++ contrib/pam/cvs.linux     2003-05-06 21:03:28.000000000 +0200
> @@ -0,0 +1,8 @@
> +# This is a sample PAM configuration for Linux
> +# which does standard unix authentication against
> +# the password in /etc/passwd or /etc/shadow.
> +# The contents of this file should be copied to
> +# /etc/pam.d/cvs
> +
> +auth      required    pam_unix.so
> +account   required    pam_unix.so
> --- /dev/null 2003-04-09 22:42:25.000000000 +0200
> +++ contrib/pam/cvs.solaris   2003-05-06 21:03:28.000000000 +0200
> @@ -0,0 +1,8 @@
> +# This is a sample PAM configuration for Sun/Solaris
> +# which does standard unix authentication against
> +# the password in /etc/passwd or /etc/shadow.
> +# The contents of this file should be inserted
> +# at an appropriate position in /etc/pam.conf
> +
> +cvs     auth        required    /usr/lib/security/$ISA/pam_unix.so.1
> +cvs     account     required    /usr/lib/security/$ISA/pam_unix.so.1
> 
> ______________________________________________________________________
> 
> ChangeLog for PAM changes from Brian Murphy <brian@murphy.dk>
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/ccvs/ChangeLog,v
> retrieving revision 1.723
> diff -r1.723 ChangeLog
> 0a1,2
> >     * configure.in: added options necessary to support PAM.
> > 
> Index: doc/ChangeLog
> ===================================================================
> RCS file: /cvs/ccvs/doc/ChangeLog,v
> retrieving revision 1.753
> diff -r1.753 ChangeLog
> 0a1,3
> >     * cvs.texinfo (Setting up the server for password authentication): 
> >     added documentation for PAM support.
> > 
> Index: src/ChangeLog
> ===================================================================
> RCS file: /cvs/ccvs/src/ChangeLog,v
> retrieving revision 1.2430
> diff -r1.2430 ChangeLog
> 0a1,4
> >     * server.c (cvs_pam_conv, check_system_password): modifications 
> >     to allow PAM authentication instead of the standard system password 
> >     lookup.
> > 
> Index: contrib/ChangeLog
> ===================================================================
> RCS file: /cvs/ccvs/contrib/ChangeLog,v
> retrieving revision 1.103
> diff -r1.103 ChangeLog
> 0a1,3
> >     * pam: added directory and contents with some example
> >     PAM configurations - currently for Solaris and Linux
> > 
> 
> ______________________________________________________________________
> 
> _______________________________________________
> Bug-cvs mailing list
> Bug-cvs@gnu.org
> http://mail.gnu.org/mailman/listinfo/bug-cvs





reply via email to

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