Patch (was Re: disable "fake" samba authentication error messages)

Charlie Brady cbrady at ind.tansu.com.au
Mon Aug 10 09:47:51 GMT 1998


Attached is a patch to 1.9.18p8 to kill the "authentication failure"
messages when samba is permuting upper/lower cases. This follows the
strategy suggested by Andrew Morgan.

The patch isn't really as big as it looks, I've moved some code around so
that there is less repetition.

I note that samba always tries the password lowercased unless it receives
the password in mixed case. I would have thought that we knew which
clients might have uppercased it, and should only do the case permutation
for known dumb clients. Otherwise we reduce password security somewhat on
well behaved clients. 

There's also the issue of PAM adding a pause on every authentication
failure (PAM_FAIL_DELAY). At the moment this can be disabled on a per
service basis in the PAM config file, but I think that samba should turn
it off in the code, then turn it back on for the last authentication
attempt.

I sent this patch to Andrew and John Terpstra back in July, but it seems
to have been lost in the noise, so I'll repost it here in case it is
useful to anyone.

I can't claim to have tested this exhaustively, but it was doing the right
thing for me.

Charlie Brady - Telstra  |internet: cbrady at ind.tansu.com.au
Network Products         |Snail    : Locked Bag 6581, GPO Sydney 2001 Australia
Platform Technologies    |Physical : Lvl 2, 175 Liverpool St, Sydney 2000
 IN-Sub Unit - Sydney    | Phone: +61 2 9206 3470 Fax: +61 2 9281 1301

---------- Forwarded message ----------
Date: 18 Jul 1998 13:43:59 -0000
From: charlieb at charlieb.nlc.net.au
To: cbrady at ind.tansu.com.au
Subject: samba.patch

--- password.c.orig	Mon May 11 18:37:47 1998
+++ password.c	Sat Jul 18 23:37:11 1998
@@ -449,10 +449,17 @@
     NULL
 };
 
+static  pam_handle_t *pamh;
+
+static BOOL samba_pam_checkpass(char *password)
+{
+  PAM_password = password;
+
+  return (PAM_SUCCESS == pam_authenticate(pamh, 0));
+}
 
 static BOOL pam_auth(char *this_user,char *password)
 {
-  pam_handle_t *pamh;
   int pam_error;
 
   /* Now use PAM to do authentication.  For now, we won't worry about
@@ -462,27 +469,34 @@
    * verbose as would otherwise make sense.
    * Query: should we be using PAM_SILENT to shut PAM up?
    */
-  #define PAM_BAIL if (pam_error != PAM_SUCCESS) { \
-     pam_end(pamh, 0); return False; \
-   }
   PAM_password = password;
   PAM_username = this_user;
-  pam_error = pam_start("samba", this_user, &PAM_conversation, &pamh);
-  PAM_BAIL;
+
+  if (PAM_SUCCESS != pam_start("samba", this_user, &PAM_conversation, &pamh))
+  {
+    pam_end(pamh, 0); return False;
+  }
+
 /* Setting PAM_SILENT stops generation of error messages to syslog
  * to enable debugging on Red Hat Linux set:
  * /etc/pam.d/samba:
  *	auth required /lib/security/pam_pwdb.so nullok shadow audit
  * _OR_ change PAM_SILENT to 0 to force detailed reporting (logging)
  */
-  pam_error = pam_authenticate(pamh, PAM_SILENT);
-  PAM_BAIL;
+
+  if (!string_combinations(password, samba_pam_checkpass, lp_passwordlevel()))
+  {
+    pam_end(pamh, 0); return False;
+  }
+
   /* It is not clear to me that account management is the right thing
    * to do, but it is not clear that it isn't, either.  This can be
    * removed if no account management should be done.  Alternately,
    * put a pam_allow.so entry in /etc/pam.conf for account handling. */
-  pam_error = pam_acct_mgmt(pamh, PAM_SILENT);
-  PAM_BAIL;
+  if (PAM_SUCCESS != pam_acct_mgmt(pamh, PAM_SILENT))
+  {
+    pam_end(pamh, 0); return False;
+  }
   pam_end(pamh, PAM_SUCCESS);
   /* If this point is reached, the user has been authenticated. */
   return(True);
@@ -756,6 +770,7 @@
 offset is the first char to try and change (start with 0)
 it assumes the string starts lowercased
 ****************************************************************************/
+static BOOL string_combinations(char *s,BOOL (*fn)(char *),int N);
 static BOOL string_combinations2(char *s,int offset,BOOL (*fn)(char *),int N)
 {
   int len = strlen(s);
@@ -790,8 +805,29 @@
 static BOOL string_combinations(char *s,BOOL (*fn)(char *),int N)
 {
   int n;
+  pstring pass2;
+
+  if (fn(s))
+    return(True);
+
+  /* if the password was given to us with mixed case then we don't
+     need to proceed as we know it hasn't been case modified by the
+     client */
+
+  if (strhasupper(s) && strhaslower(s))
+    return(False);
+
+  /* make a copy of it */
+  StrnCpy(pass2,s,sizeof(pstring)-1);
+  
+  /* try all lowercase */
+  strlower(pass2);
+  if (fn(pass2))
+    return(True);
+
   for (n=1;n<=N;n++)
-    if (string_combinations2(s,0,fn,n)) return(True);
+    if (string_combinations2(pass2,0,fn,n))
+      return(True);
   return(False);
 }
 
@@ -804,15 +840,6 @@
 {
 
 #ifdef USE_PAM
-/* This falls through if the password check fails
-	- if NO_CRYPT is defined this causes an error msg
-		saying Warning - no crypt available
-	- if NO_CRYPT is NOT defined this is a potential security hole
-		as it may authenticate via the crypt call when PAM
-		settings say it should fail.
-  if (pam_auth(this_user,password)) return(True);
-Hence we make a direct return to avoid a second chance!!!
-*/
   return (pam_auth(this_user,password));
 #endif
 
@@ -1112,49 +1139,11 @@
 #endif    
   }
 
-  /* try it as it came to us */
-  if (password_check(password))
-    {
-      update_protected_database(user,True);
-      if (pass && update_encrypted)
-        update_smbpassword_file(pass,password);
-      return(True);
-    }
-
-  /* if the password was given to us with mixed case then we don't
-     need to proceed as we know it hasn't been case modified by the
-     client */
-  if (strhasupper(password) && strhaslower(password))
-    return(False);
-
-  /* make a copy of it */
-  StrnCpy(pass2,password,sizeof(pstring)-1);
-  
-  /* try all lowercase */
-  strlower(password);
+#ifdef USE_PAM
   if (password_check(password))
-    {
-      update_protected_database(user,True);
-      if (pass && update_encrypted)
-        update_smbpassword_file(pass,password);
-      return(True);
-    }
-
-  /* give up? */
-  if (level < 1)
-    {
-      update_protected_database(user,False);
-
-      /* restore it */
-      strcpy(password,pass2);
-
-      return(False);
-    }
-
-  /* last chance - all combinations of up to level chars upper! */
-  strlower(password);
-
+#else
   if (string_combinations(password,password_check,level))
+#endif
     {
       update_protected_database(user,True);
       if (pass && update_encrypted)
@@ -1163,10 +1152,6 @@
     }
 
   update_protected_database(user,False);
-  
-  /* restore it */
-  strcpy(password,pass2);
-  
   return(False);
 }
 



More information about the samba-technical mailing list