> I can't apply this patch. Are you using an up-to-date CVS version?

I was diffing from  my last patch (w/-ru options) which was sent. Let me
refresh my CVS copt and issue a new one.
Patch is below

> Are you sure the test is not "if (userCert)" ("!" removed)?
> Why calling X509_free(userCert); _only_ when userCert is NULL?



(smack forehead) you are right! I've been trying to chase down an
intermittent segment violation that only occurs with gdm (and which as
no way to capture STDERR) than when it stopped, I thought I had it
fixed. I guess it's still intermittent. Let me see if it happens again after this 
change.

Here is the diff from the last CVS.

Is there any way I can get a ssh account? our proxy blocks CVS connections, 
and I have to go home to update.



 diff -rubB /local/CVS/muscleplugins/MusclePAM/Makefile MusclePAM/Makefile
--- /local/CVS/muscleplugins/MusclePAM/Makefile 2004-07-03 13:43:05.000000000 -0400
+++ MusclePAM/Makefile  2004-10-08 09:43:06.000000000 -0400
@@ -15,17 +15,17 @@
 all: pam_musclecard.so
 
 clean:
-       rm -f $(OBJ) pam_musclecard.so
+       rm -f $(OBJ) pam_musclecard.so install
 
 test:
        $(CC) -o testpam testpam.c $(OBJ) $(LIBS)
 
-install:
+install: pam_musclecard.so pam-muscle.conf
        install --directory $(DESTDIR)/lib/security/
        install --mode=755 pam_musclecard.so $(DESTDIR)/lib/security/
        install --directory $(DESTDIR)/etc/musclepam
        install --mode=644 pam-muscle.conf $(DESTDIR)/etc/musclepam/
-
+       touch install
 pam_musclecard.so: $(OBJ)
        $(LD) -shared -o $@ $(OBJ) $(LIBS)
 
diff -rubB /local/CVS/muscleplugins/MusclePAM/pam-muscle.conf MusclePAM/pam-muscle.conf
--- /local/CVS/muscleplugins/MusclePAM/pam-muscle.conf  2003-10-03 15:30:31.000000000 
-0400
+++ MusclePAM/pam-muscle.conf   2004-10-08 09:44:49.000000000 -0400
@@ -5,10 +5,14 @@
 #
 
 Debug       = OFF                       # Debug ON or OFF
-CertNumber  = 0                         # Certificate number to use
+# Usually, cert #0 is the private key, and cert# 1 is the public key
+CertNumber  = 1                         # Certificate number to use
 PinNumber   = 1                         # Pin number to verify
-UserPath    = /home/                    # Path to user home directory
 CertName    = user.cert                 # User Certificate in DER format
+# If the following value is defined, then it is used to determine the
+# location of the user certificate - i.e. $UserPath/$USER/.muscle/$CertName
+# Otherwise, the user's home directory is used - i.e. ~user/.muscle/$CertName
+#UserPath    = /home/                    # Path to user home directory
 RootCACert  = /etc/musclepam/root.cert  # Root CA certificate
 LDAPHost    = unsupported               # Web-server with LDAP
 LDAPPath    = unsupported               # Search path in LDAP
diff -rubB /local/CVS/muscleplugins/MusclePAM/pam_smartcard.c MusclePAM/pam_smartcard.c
--- /local/CVS/muscleplugins/MusclePAM/pam_smartcard.c  2004-09-15 03:00:58.000000000 
-0400
+++ MusclePAM/pam_smartcard.c   2004-10-08 06:19:29.000000000 -0400
@@ -42,7 +42,7 @@
 #define DEBUG 1
 
 /**
- * This struct contains secure data and shoul be overwritten
+ * This struct contains secure data and should be overwritten
  * before releasing!
  */
 struct secure_data {
@@ -253,14 +253,34 @@
 
 int readUserPubKey(EVP_PKEY **userCert, MSCTokenConnection pConnection, 
                 struct secure_data *sd) {
-  int rv;
+  int rv = 0;
   EVP_PKEY *tmpCert;
   X509 *tmpCert1;
   unsigned char homeFile[200];
   /* tries to get a public key information from the user's public key certificate */
 
-  snprintf(homeFile, 200, "%s%s/.muscle/%s", pr.userpath, sd->user, pr.certname);  
+  /* Is userpath defined? If so, use it as a way to 'find' someone's
+   home directory (ignoring the value in the password file). 
+   I'm using this because the existence of the
+   specified file overrides the end user's default certificate in their 
+   home directory  - so userpath is defined for debugging only */
 
+  if (strlen(pr.userpath)>0) {
+       snprintf(homeFile, 200, "%s%s/.muscle/%s", pr.userpath, sd->user, 
pr.certname);  
+  } else {
+       /* It's not defined. We have to determine the user's home directory. */
+       struct passwd *pw;
+       if ((pw=getpwnam(sd->user)) == NULL) {
+         syslog(LOG_ERR, "su attempt to non-existing user: %s", sd->user);
+         return -1;
+       } else {
+         snprintf(homeFile, 200, "%s/.muscle/%s", pw->pw_dir, pr.certname);  
+       }
+  }
+  if (util_CheckFile(homeFile,(char *)(sd->user))) {
+       syslog(LOG_ERR, "Unsafe permissions on user certificate, file: %s: user: %s", 
homeFile, sd->user);
+       return -1;
+  }
   rv = getFileCert(homeFile, &tmpCert1);
 
   if (rv == -1) {
@@ -271,24 +291,20 @@
           without an outside authority to sign it. */
 
     rv = getPubKeyFromFile(homeFile, &tmpCert);
+       if (pr.debug) syslog(LOG_INFO, "user certificate successfully read from %s", 
homeFile);
   } else {
-       syslog(LOG_ERR, "user certificate successfully read from %s", homeFile);
        rv = checkCert(tmpCert1);
 
        if (rv == -1) {
          syslog(LOG_ERR, "user certificate expired or revoked");
-         pcsc_release(&pConnection);
-         pam_release_data(sd);
-         return PAM_AUTHINFO_UNAVAIL;
+         return -1;
        }
        rv = getPublicKey(tmpCert1, &tmpCert);
   }    
   
   if (rv == -1) {
     syslog(LOG_ERR, "cannot read certificate from %s", homeFile);
-    pcsc_release(&pConnection);
-    pam_release_data(sd);
-    return PAM_AUTHINFO_UNAVAIL;
+       return -1;
   }
 
 
@@ -311,7 +327,7 @@
   int try_first_pass, use_first_pass;
   unsigned char error[150];
   //unsigned char nameBuf[100];
-  X509 *userCert;
+  X509 *userCert = 0;
   EVP_PKEY *pubkey;
 
   reader = 0;
@@ -336,7 +352,16 @@
   if (rv != MSC_SUCCESS) {
     syslog(LOG_ERR, "musclecard error during pcsc_init: %s", msc_error(rv));
     pam_release_data(sd);
+    /* if we return PAM_AUTHINFO_UNAVAIL, then we cannot use pam "auth
+       sufficient" That is, if the token is unavailable, will you
+       allow a password to be used. If we return PAM_AUTHINFO_UNAVAIL,
+       we cannot fall back on a password. Therefore, return PAM_ERR,
+       allowing password to be sufficient.
+
     return PAM_AUTHINFO_UNAVAIL;
+    */
+    result = PAM_AUTH_ERR; 
+
   }
 
   if (pr.debug) printf("Welcome to pam_musclecard.so verification Module\n");
@@ -439,8 +464,11 @@
 
   if (pr.authmode == ROOTCERT) {
     rv = readRootCert(&userCert, pConnection, sd);
-      if (rv != 0)
+       if (rv != 0) {
+         pcsc_release(&pConnection);
+         pam_release_data(sd);
         return PAM_AUTHINFO_UNAVAIL;
+       }
     rv = getPublicKey(userCert, &pubkey);
   } else {
     rv = readUserPubKey(&pubkey, pConnection, sd);
@@ -453,11 +481,10 @@
     return PAM_AUTHINFO_UNAVAIL;
   }
 
-
-  /* This sectionj can be used to verify the user's certificate */
+  /* This section can be used to verify the user's certificate */
 #ifdef DEBUG_SPECIAL1
   if (pr.debug) {
-       syslog(LOG_ERR, "Public key file read from user certificate");
+       syslog(LOG_INFO, "Public key file read from user certificate");
        printf("public exponent : %s\n", BN_bn2hex(pubkey->pkey.rsa->e));
        printf("public modulus : %s\n", BN_bn2hex(pubkey->pkey.rsa->n));
   }
@@ -514,6 +541,7 @@
        sd2 = (struct secure_data*)malloc(sizeof(struct secure_data));
        if (sd2 == NULL) {
          syslog(LOG_CRIT, "not enough free memory to initialize sd2");
+         pam_release_data(sd);
          return PAM_AUTHINFO_UNAVAIL;
        }
        memcpy(sd2->rand,sd->cipher,RAND_SIZE);
@@ -527,7 +555,7 @@
        printf("Undecoded value using key #%d  = ", cryptInit.keyNum);
          for (i=0; i < RAND_SIZE; i++) {
                rv = (unsigned char)sd2->cipher[i];
-               printf("%02x", rv);
+               printf("%02X", rv);
          }     
          printf("\n\n");
   }
@@ -561,8 +589,10 @@
          printf("Challenge was Successfully met\n");
        result = PAM_SUCCESS;
   } else {
-       if (pr.debug)
+       if (pr.debug) {
          syslog(LOG_ERR, "musclecard challenge failed for user %s", sd->user);
+         printf("Challenge failed\n");
+       }
 
        result = PAM_AUTH_ERR; 
   }  
@@ -567,9 +597,10 @@
        result = PAM_AUTH_ERR; 
   }  
   
-
   /* Release certificates */
+  if (userCert) {
   X509_free(userCert);
+  }
 
   /* release pcsc */
   pcsc_release(&pConnection);
diff -rubB /local/CVS/muscleplugins/MusclePAM/preferences.c MusclePAM/preferences.c
--- /local/CVS/muscleplugins/MusclePAM/preferences.c    2003-10-03 15:30:31.000000000 
-0400
+++ MusclePAM/preferences.c     2004-10-06 16:42:12.000000000 -0400
@@ -7,6 +7,7 @@
  * Authors:                                                         *
  *   Chris Osgod          <[EMAIL PROTECTED]>                           *
  *   Eirik A. Herskedal   <[EMAIL PROTECTED]>                  *
+ *   Bruce Barnett        <[EMAIL PROTECTED]>
  *                                                                  *
  ********************************************************************
  *                                                                  *
@@ -19,14 +20,21 @@
 #include <stdio.h>
 #include <syslog.h>
 #include <string.h>
-
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <pwd.h>
+#include <sys/types.h>
 #include "preferences.h"
 
+/* The third parameter was /home, but this overrides the default value
+   of the home directory specified by the passwd file */
+
 struct preferences pr = {
   DEBUGOFF,
   0,
   1,
-  "/home/",
+  "",   /* was "/home/" */
   "user.cert",
   "/etc/musclepam/root.cert",
   "",
@@ -194,15 +202,138 @@
   char rcfilename[] = "/etc/musclepam/pam-muscle.conf";
   char buf[1024];
   
+  if (pr.debug) syslog(LOG_INFO, "Reading the preferences from file '%s'", 
rcfilename);
+
+  if (util_CheckFile(rcfilename,"root")) {
+       return -1;
+  }
 
   fp = fopen(rcfilename, "rb");
   if (fp)
     {
       while (fgets(buf, sizeof(buf), fp))
        util_ParsePreference(buf, sizeof(buf));
-      
       fclose(fp);
     }
   return rv;
 }
 
+/******************************************************************************
+** Function: util_CheckFile
+**
+** Checks to make sure the file is not modifyable by someone other
+** than the owner This utility is given the complete name for the file
+** and recursively walks up the direwctory tree until it reaches the
+** root directory.
+**
+**
+** Parameters:
+**  file (string)
+**   user (string)
+**
+** Returns:
+**  zero if no problem
+**  -1 if there is a security risk
+**  -2 if the file does not exist
+**
+
+******************************************************************************/
+
+int util_CheckFile (char *file, char *user) {
+       char path[1024];
+       int i;
+       char *l;
+
+       if (strstr(file,"..")) {
+         /* This would probably never happen, and the owner of the
+                /etc/musclepam/pam-muscle.conf file has to put it
+                there. . But it's better to be paranoid and safe than
+                sorry */
+         syslog(LOG_ERR, "File '%s' contains the string '..'  - unsafe place to put 
configuration file", file);
+         return -1;
+       }
+
+       if (file[0] != '/') {
+         syslog(LOG_ERR, "File '%s' is relative. Must use absolute path  - unsafe 
place to put configuration file", file);
+         return -1;
+       }                       
+
+       /* Now - check the entire file */
+       if ((i=util_CheckFileComponent(file,user))<0) return i;
+       /* looks okay - now check all of the paths */
+       (void)strncpy(path,file,1024);
+       while ((l=strrchr(path,'/')) != NULL) {
+         /* chop off all characters after the '/' */
+         *l=0;
+         if (strlen(path)) {
+               if ((i=util_CheckFileComponent(path,user)) <0) return i;
+         }     
+       }
+          
+       if (pr.debug) 
+         syslog(LOG_INFO, "File '%s' and user %s look okay", file, user);
+       return 0;
+}
+
+/******************************************************************************
+** Function: util_CheckFileComponent
+**
+** Checks to make sure the file component (either a single file or
+** directory) is not modifyable by someone other than the owner
+**
+** Parameters:
+**  file (string)
+**   user (string)
+**
+** Returns:
+**  zero if no problem
+**  -1 if there is a security risk
+**  -2 if the file or directory does not exist
+**
+
+******************************************************************************/
+
+int util_CheckFileComponent (char *file, char *user) {
+    struct stat buf;
+       struct passwd *pw;
+       uid_t uid;
+
+       /* Check if the file is a symbolic link */
+       if (lstat(file,&buf)) {
+         /* if (pr.debug) printf("File does not exist %s - done\n", file); */
+         return -2; /* doesn't exist */
+       }
+
+
+       if (S_ISLNK(buf.st_mode)) {
+         syslog(LOG_ERR, "File '%s' is a symbolic link - unsafe place to put 
configuration file", file);
+         return -1;
+       } 
+  
+       /* Check the real file */
+       if (stat(file,&buf)) {
+         /* if (pr.debug) printf("File does not exist (2) %s - done\n", file); */
+         return -2; /* I can't check it, so it's okay */
+       };
+  
+       /* Find the user's UID number */
+       if ((pw=getpwnam(user)) == NULL) {
+         syslog(LOG_ERR, "User '%s' does not exist", user);
+         return -1;
+       }
+ 
+       uid=pw->pw_uid;
+
+       if ((buf.st_uid != uid) && (buf.st_uid != 0)) {
+         syslog(LOG_ERR, "File '%s' is  owned by UID %d, and should be owned by %d 
(%s) - unsafe operation", file, buf.st_uid, uid, user);
+         return -1;
+       }
+
+       if ((buf.st_mode & 0022) != 0) {
+         /* if (pr.debug) printf("File mode is BAD,  %o vs. %o\n", buf.st_mode, 
(buf.st_mode&022)); */
+         syslog(LOG_ERR, "File '%s' is group or world writable - may be unsafe 
operation", file);
+         return -1;
+       }
+       return 0;
+}
+
diff -rubB /local/CVS/muscleplugins/MusclePAM/preferences.h MusclePAM/preferences.h
--- /local/CVS/muscleplugins/MusclePAM/preferences.h    2002-07-11 17:10:37.000000000 
-0400
+++ MusclePAM/preferences.h     2004-10-06 16:42:13.000000000 -0400
@@ -40,4 +40,9 @@
 /* Parse the configuration file */
 int util_ReadPreferences();
 
+/* Check the permissions of the file */
+int util_CheckFile (char *file, char *user);
+
+/* Check the permissions of the file Component*/
+int util_CheckFileComponent (char *file, char *user);
 
Only in MusclePAM: preferences.h.orig
Only in MusclePAM: preferences.o
diff -rubB /local/CVS/muscleplugins/MusclePAM/README MusclePAM/README
--- /local/CVS/muscleplugins/MusclePAM/README   2004-09-15 03:00:58.000000000 -0400
+++ MusclePAM/README    2004-10-08 06:31:23.000000000 -0400
@@ -42,7 +42,11 @@
 have two values:
 
 1. UserCert -   the module will look in ~/.muscle/user.cert for the 
-                certificate. 
+               certificate. If the file contains a value for UserPath, then
+               this is used instead of the user's home directory. This allows
+               the admin to create a directory containing certificates that
+               are not modify able by the user. Of it can be used for testing.
+
 2. RootCert -   the module will retreive the certificate from the smartcard 
                 and validate the signature by looking at the RootCA's
                 certificate in /etc/root.cert. It will also check that the
@@ -60,11 +64,13 @@
         [snip]
         -----END CERTIFICATE-----
     3) PEM - PUBKEY
+               This is also in base64 format, and has the form
         -----BEGIN PUBLIC KEY-----
         MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDhs17bA92YPg8XrR9s34qoK2Fc
         [snip]
         -----END PUBLIC KEY-----
 
+       
     The third form can be obtained by using musacletool to create a
     keypair, export it, and then using b2fs to convert it into a
     pubkey format
@@ -101,24 +107,130 @@
     % b2fs cert.blob >~/.muscle/user.cert
 
 
-Note that the user can do this without any outside authority. Make sure
-the home directory and certificate is not writable. (pam_musclecard.so
-should check to make sure this is not possible. However it doesn't, so
-this is a potential security problem).
-
-Once you have the certificate in place, I have been able to get the su
-and login modules to work with PAM.
-
-Modify
-    /etc/pam.d/su
-to contain
-   auth       required     /lib/security/pam_musclecard.so service=system-auth
-
- and /etc/pam.d/login to contain
-   auth       required /lib/security/pam_musclecard.so service=system-auth
-
-and when you try to log into the console, or to do an su, it first
-checks if a smartcard is plugged in. If so, it asks for a PIN value.
-Then it procees to verify it with the user's ~user/.muscle/user.cert
-file.
+       Also not that you have to change the certificate number from 0
+       (which is the private key) to 1, which would in this case be the
+       public key.
+
+Note that the user can do this without any outside authority. 
+
+The pam module makes sure that only the user and root has write access
+to the certificate.  The certificate, and all of the directories
+above, cannot be group or world writable, and have to be owned by the
+user or root. Otherwise, the user is not allowed to log in, and a log
+message is written to the log file.
+
+Once you have the certificate in place, you can use PAM to add
+smartcard authentication to the following modules [I have verified
+that these work on Fedora Core 1. Other applications might work as
+well, but I have not tested them.  -Bruce]
+
+       login
+       su
+       xscreensaver
+       gdm
+       gdm-autologin
+
+
+The modifications to the /etc/pam.d/* files only change the auth
+values (lines whose first column is "auth")
+
+Gdm can be configured in several ways.
+
+Variation #1: If the reader is present, and the token inserted
+               only ask for the PIN, and not the password
+               If the reader or token is not present, ask for the password
+------------
+auth       sufficient  pam_musclecard.so service=system-auth debug
+auth       required    pam_stack.so service=system-auth
+-------------
+
+Variation #2: Only use the smartcard, never the password
+------------
+auth       required    pam_musclecard.so service=system-auth debug
+#auth       required   pam_stack.so service=system-auth
+-------------
+ 
+
+Variation #3: Require a password (first) and a pin (second)
+------------
+auth       required    pam_stack.so service=system-auth
+auth       required    pam_musclecard.so service=system-auth debug
+-------------
+
+Note that you can optionally allow root to log in without a smartcard by adding
+------------
+auth       required    pam_nologin.so
+------------
+
+xscreeensaver only requires a PIN value, but prompts for a password.
+This seems to be a bug in xscreensaver.
+
+
+::::::::::::::
+gdm
+::::::::::::::
+#%PAM-1.0
+auth       required    pam_env.so
+# If musclecard is "sufficient", then if this fails, fall back on the password
+auth       sufficient  pam_musclecard.so service=system-auth debug
+auth       required    pam_stack.so service=system-auth
+# If you are sure, you may want to remove the next line
+# It allows root to log in without a smartcard
+auth       required    pam_nologin.so
+account    required    pam_stack.so service=system-auth
+password   required    pam_stack.so service=system-auth
+session    required    pam_stack.so service=system-auth
+session    optional     pam_console.so
+::::::::::::::
+gdm-autologin
+::::::::::::::
+#%PAM-1.0
+auth       required    pam_env.so
+auth       required    pam_nologin.so
+auth       required    pam_permit.so
+auth       required    pam_musclecard.so service=system-auth
+account    required    pam_stack.so service=system-auth
+password   required    pam_stack.so service=system-auth
+session    required    pam_stack.so service=system-auth
+session    optional     pam_console.so
+::::::::::::::
+login
+::::::::::::::
+#%PAM-1.0
+#auth       required   pam_securetty.so
+auth       required    /lib/security/pam_musclecard.so service=system-auth
+#auth       required   pam_stack.so service=system-auth
+#auth       required   pam_nologin.so
+account    required    pam_stack.so service=system-auth
+password   required    pam_stack.so service=system-auth
+session    required    pam_stack.so service=system-auth
+session    optional    pam_console.so
+::::::::::::::
+su
+::::::::::::::
+#%PAM-1.0
+# If you remove the next line, then root will require a smartcard to do a 'su
+# This might break things (cronjobs, admin scripts, etc.)
+auth       sufficient   /lib/security/$ISA/pam_rootok.so
+# Uncomment the following line to implicitly trust users in the "wheel" group.
+#auth       sufficient   /lib/security/$ISA/pam_wheel.so trust use_uid
+# Uncomment the following line to require a user to be in the "wheel" group.
+#auth       required     /lib/security/$ISA/pam_wheel.so use_uid
+#was
+#  Normally we use the pam_stack, but in this case we use musclecard
+#auth       required   /lib/security/$ISA/pam_stack.so service=system-auth
+auth       required    /lib/security/pam_musclecard.so service=system-auth 
+account    required    /lib/security/$ISA/pam_stack.so service=system-auth
+password   required    /lib/security/$ISA/pam_stack.so service=system-auth
+session    required    /lib/security/$ISA/pam_stack.so service=system-auth
+session    optional    /lib/security/$ISA/pam_xauth.so
+::::::::::::::
+xscreensaver
+::::::::::::::
+#%PAM-1.0
+
+# Red Hat says this is right for them, as of 7.3:
+auth       required    pam_stack.so service=system-auth
+auth       required    /lib/security/pam_musclecard.so service=system-auth  debug
+# auth       required  pam_pwdb.so shadow nullok
 
_______________________________________________
Muscle mailing list
[EMAIL PROTECTED]
http://lists.drizzle.com/mailman/listinfo/muscle

Reply via email to