Self-check patches for 2.2.0 alpha 3

David Collier-Brown davecb at canada.sun.com
Wed Apr 11 13:54:58 GMT 2001


This replaces incoming/178, which was described as:
---
   This is my sanity-check code, which has allowed me to catch 
 numerous silly errors in smb.con files people have posted on
 comp.protocols.smb or mailed to samba at samba.org
   I'd like to see in in 2.2 if that's achievable, but doing so
 has a cost: if the team has changed the meaning of some of
 the closely interrelated options, this may see it as a user error.
 It is therefor necessary to test changes between 2./0.7 and 2.2,
 or scan through the test code by eye, looking for impossibilities.
   As it's larger than the other, it also has a bigger chance
 of introducing regression.
--- 

Warning: this was written for 2.0.7, and ported to 2.2.0: it
will certainly miss some things [e.g., over-long share names]


--dave
-- 
David Collier-Brown,           | Always do right. This will gratify 
Performance & Engineering Team | some people and astonish the rest.
Americas Customer Engineering  |                      -- Mark Twain
(905) 415-2849                 | davecb at canada.sun.com
-------------- next part --------------
--- -	Wed Apr 11 09:30:53 2001
+++ loadparm.c	Tue Apr 10 12:56:52 2001
@@ -532,6 +532,9 @@
 static BOOL handle_vfs_object(char *pszParmValue, char **ptr);
 static BOOL handle_source_env(char *pszParmValue, char **ptr);
 static BOOL handle_netbios_name(char *pszParmValue, char **ptr);
+static BOOL handle_netbios_aliases(char *pszParmValue,char **ptr);
+static BOOL handle_password_server(char *pszParmValue, char **ptr);
+static BOOL validate_netbios_name(char *pszParmValue);
 static BOOL handle_winbind_id(char *pszParmValue, char **ptr);
 static BOOL handle_wins_server_list(char *pszParmValue, char **ptr);
 static BOOL handle_debug_list( char *pszParmValue, char **ptr );
@@ -647,6 +650,8 @@
 #endif
 
 /* note that we do not initialise the defaults union - it is not allowed in ANSI C */
+
+/*        label,      type, class,        ptr,         special, enumlist, flags,  default */
 static struct parm_struct parm_table[] = {
 	{"Base Options", P_SEP, P_SEPARATOR},
 	
@@ -1690,6 +1695,7 @@
 			 service * pserviceDest);
 static void copy_service(service * pserviceDest,
 		      service * pserviceSource, BOOL *pcopymapDest);
+static BOOL globals_ok(void);
 static BOOL service_ok(int iService);
 static BOOL do_parameter(char *pszParmName, char *pszParmValue);
 static BOOL do_section(char *pszSectionName);
@@ -2035,52 +2041,238 @@
 	}
 }
 
+
+/***************************************************************************
+Check globals for consistency. Return False if something fatal will happen.
+The messages use ERROR WARNING and NOTICE the same way service_ok does.
+This replaces do_global_checks in utils/testparm.c, and should be called at
+least once, and possibly each time the smb.conf file is reread.
+***************************************************************************/
+static BOOL globals_ok(void)
+{
+   BOOL bRetval = True;
+   SMB_STRUCT_STAT st;
+   char *wins;
+   static BOOL filesChecked = False;
+
+   /* Look for inconstancies in roughly decreasing order of severity, */
+
+   /* Check for both wins server and wins support, make sure server wins. */
+   /* DCB: The data structure changes in 2.0.7, requiring this check. */
+   wins = Globals.szWINSserver;
+   if (Globals.bWINSsupport == True && wins != NULL
+     && ! (wins[0] == '\0' || strwicmp(wins, "127.0.0.1") == 0)) {
+       DEBUG(0,( "ERROR: both wins server = yes and wins support == yes, "
+         "using wins server \"%s\".\n", wins));
+         Globals.bWINSsupport = False;
+   }
+
+   /* Make sure we have a lock directory, but only once. */
+   if (filesChecked == False) {
+      if (!directory_exist(Globals.szLockDir, &st)) {
+         DEBUG(0,("ERROR: lock directory \"%s\" does not exist\n",
+                 Globals.szLockDir));
+         bRetval = False;
+      }
+      else if ((st.st_mode & 0777) != 0755) {
+         DEBUG(0,("WARNING: lock directory \"%s\" should have "
+	    "permissions 0755 for browsing and locks to work\n", 
+	    Globals.szLockDir));
+      }
+      filesChecked = True;
+   }
+
+   /* Security = (server or domain) requires password server to be set.  */
+   if ((Globals.szPasswordServer == NULL || Globals.szPasswordServer[0] == '\0' )
+     && (Globals.security == SEC_SERVER || Globals.security == SEC_DOMAIN)) {
+      DEBUG(0,("ERROR: security = server and security = domain "
+         "both require a password server.\n"));
+      bRetval = False;
+   }
+
+   /* Password server should be a netbios name. */
+   if (Globals.szPasswordServer != NULL 
+     && (strchr(Globals.szPasswordServer,'.') != NULL 
+       || strlen(Globals.szPasswordServer) >= 15)) {
+       DEBUG(0,("ERROR: password server \"%s\" is not a legal "
+                "NetBIOS name, logons will fail.\n",
+                Globals.szPasswordServer));
+       bRetval = False;
+   }
+
+   /* Check unix password sync prerequisites. */
+   if (Globals.bUnixPasswdSync) {
+      if (Globals.security != SEC_USER) {
+         DEBUG(0,("WARNING: unix password sync = yes requires "
+	    "security = user.\n"));
+      }
+      if (Globals.bEncryptPasswords == False) {
+        DEBUG(0,("WARNING: unix password sync = yes requires "
+	    "encrypt passwords = yes.\n"));
+      }
+   }
+
+   /* Be sure update encrypted is done with NON-encrypted passwords. */
+   if (Globals.bUpdateEncrypt && Globals.bEncryptPasswords == False) {
+      DEBUG(0,("WARNING: update encrypted = yes requires encrypt "
+         "passwords = no.\n"));
+   }
+
+   /* Er, should unix password sync be automatic if smb password file exists? */
+
+   /* If unix password sync, check the prerequisites.*/
+   if (Globals.bUnixPasswdSync) {
+      /* See if the program is executable. */
+      {  pstring passwd_prog;
+               char *p;
+         pstrcpy(passwd_prog, Globals.szPasswdProgram);
+        if ((p = strchr(passwd_prog, ' ')) != NULL) {
+            *p = '\0';
+        }
+         if (access(passwd_prog, X_OK) == -1) {
+            DEBUG(0,("WARNING: the unix password sync program \"%s\" "
+	       "can't be executed, errno was %d (\"%s\").\n",
+	       Globals.szPasswdProgram,
+               errno, strerror(errno)));
+        }
+      }
+
+      /* And the passwd chat isn't expecting the old password. */
+      if (strstr(Globals.szPasswdChat, "%o") != 0) {
+         DEBUG(0,("WARNING: the 'passwd chat' script [%s] depends on "
+	    "getting the old password via the %%o substitution. With "
+	    "encrypted passwords this is not possible.\n",
+            Globals.szPasswdChat));
+      }
+
+   }
+
+   /* Check for announcing as something other than NT, which can */
+   /* interefere with serving browse lists. */
+   if (Globals.announce_as != ANNOUNCE_AS_NT_SERVER) {
+      DEBUG(0,( "WARNING: announce as not set to \"NT\", this may "
+         "interfere with browsing.\n"));
+   }
+   return True;
+} 
+
+
 /***************************************************************************
 Check a service for consistency. Return False if the service is in any way
 incomplete or faulty, else True.
 ***************************************************************************/
 static BOOL service_ok(int iService)
 {
-	BOOL bRetval;
-
-	bRetval = True;
-	if (iSERVICE(iService).szService[0] == '\0')
-	{
-		DEBUG(0,
-		      ("The following message indicates an internal error:\n"));
-		DEBUG(0, ("No service name in service entry.\n"));
-		bRetval = False;
-	}
-
-	/* The [printers] entry MUST be printable. I'm all for flexibility, but */
-	/* I can't see why you'd want a non-printable printer service...        */
-	if (strwicmp(iSERVICE(iService).szService, PRINTERS_NAME) == 0) {
-		if (!iSERVICE(iService).bPrint_ok) {
-			DEBUG(0,
-			      ("WARNING: [%s] service MUST be printable!\n",
-			       iSERVICE(iService).szService));
-			iSERVICE(iService).bPrint_ok = True;
-		}
-		/* [printers] service must also be non-browsable. */
-		if (iSERVICE(iService).bBrowseable)
-			iSERVICE(iService).bBrowseable = False;
-	}
-
-	if (iSERVICE(iService).szPath[0] == '\0' &&
-	    strwicmp(iSERVICE(iService).szService, HOMES_NAME) != 0)
-	{
-		DEBUG(0,
-		      ("No path in service %s - using %s\n",
-		       iSERVICE(iService).szService, tmpdir()));
-		string_set(&iSERVICE(iService).szPath, tmpdir());
-	}
-
-	/* If a service is flagged unavailable, log the fact at level 0. */
-	if (!iSERVICE(iService).bAvailable)
-		DEBUG(1, ("NOTE: Service %s is flagged unavailable.\n",
-			  iSERVICE(iService).szService));
-
-	return (bRetval);
+   BOOL bRetval = True;
+   struct stat buf;
+   service *s = pSERVICE(iService);
+
+   /* Look for inconstancies in roughly decreasing order of severity, */
+   /* Test for a missing service name. */
+   if (s->szService == NULL || s->szService[0] == '\0') {
+        DEBUG(0,( "The following message indicates an internal error:\n"));
+        DEBUGADD(0,( "ERROR: No service name in service entry.\n"));
+        bRetval = False;
+     }
+  
+   /* Test for path not pointing to a dir (other tests follow). */
+   if (s->szPath != NULL && s->szPath[0] != '\0') {
+      if (stat(s->szPath,&buf) == -1) {
+         DEBUG(0,("ERROR: can't stat path %s in service [%s].\n",
+               s->szPath, s->szService));
+         DEBUGADD(0,("errno = %d (%s).\n", errno, strerror(errno)));
+      }
+      else if ((buf.st_mode & S_IFDIR) != S_IFDIR) {
+         DEBUG(0,("ERROR: Path %s in service [%s] isn't a directory.\n",
+               s->szPath, s->szService));
+      }
+   }
+
+   /* Contradictions in terms: */
+   /* The [printers] entry MUST be printable. I'm all for flexibility, but */
+   /* I can't see why you'd want a non-printable printer  service...        */
+   if (strwicmp(s->szService,PRINTERS_NAME) == 0) {
+      if (!s->bPrint_ok) {
+         DEBUG(0,( "ERROR: printer service [%s] MUST be printable!\n",
+               s->szService));
+        s->bPrint_ok = True;
+        }
+   }
+  
+   /* Things that are inherently dangerous: WARNINGs. */
+   
+   /* Look for map archive forced to fail by bad create mask. */
+   /* Ditto map system, map hidden. */
+   if (s->bMap_archive == True && (s->iCreate_mask & 0100) == 0) {
+       DEBUG(0,( "WARNING: map archive = yes in share [%s], but create "
+                "mask doesn't allow setting archive bit (100 octal).\n",
+                s->szService));
+     }
+  
+    if (s->bMap_system == True && (s->iCreate_mask & 0010) == 0) {
+       DEBUG(0,( "WARNING: map system = yes in share [%s], but create "
+                "mask doesn't allow setting system bit (010 octal).\n",
+                s->szService));
+    }
+
+   if (s->bMap_hidden == True && (s->iCreate_mask & 0001) == 0) {
+       DEBUG(0,( "WARNING: map hidden = yes in share [%s], but create "
+                "mask doesn't allow setting hidden bit (001 octal).\n",
+                s->szService));
+   }
+
+   /* And see if force create mode sets any of the same three bits */
+   if ((s->iCreate_force_mode & 0100) != 0) {
+       DEBUG(0,( "WARNING: force create mode forces archive bit on "
+                "on all files in share [%s].\n", 
+                s->szService));
+   }
+   if ((s->iCreate_force_mode & 0010) != 0) {
+       DEBUG(0,( "WARNING: force create mode forces system bit on "
+                "on all files in share [%s].\n", 
+                s->szService));
+   }
+   if ((s->iCreate_force_mode & 0001) != 0) {
+       DEBUG(0,( "WARNING: force create mode forces hidden bit on "
+                "on all file in share [%s].\n",
+                s->szService));
+   }
+   /* Implausible overrides and missing prerequisites: WARNINGs. */
+   /* If a service is flagged unavailable, log the fact at level 0. */
+   if (!s->bAvailable) {
+      DEBUG(1,( "NOTICE: Service [%s] is marked available = no.\n",
+            s->szService));
+   }
+
+   /* If it's unbrowsable but we're serving browse lists, log that too. */
+   if (s->bBrowseable == False && Globals.bBrowseList == True
+     && strwicmp(s->szService,HOMES_NAME) != 0) {
+      DEBUG(0,( "NOTICE: Service [%s] is unbrowsable, but browse "
+           "lists are being served.\n", s->szService));
+   }
+
+   /* If we're syncing always, we need strict sync on too. */
+   if (s->bSyncAlways == True && s->bStrictSync == False) {
+      DEBUG(0,("NOTICE: In service [%s], sync always = yes, which requires "
+        "strict sync = yes.\n", s->szService));
+   }
+
+   /* We ned oplocks for level2 oplocks. */
+   if (s->bLevel2OpLocks == True && s->bOpLocks == False) {
+      DEBUG(0,("NOTICE: In service [%s], level 2 oplocks = yes, which "
+         "requires oplocks = yes as well.\n", s->szService));
+   }
+
+   /* Depending on defaults for proper execution: NOTICEs.*/
+   /* Test for missing path, substitute the default if unset. */
+   if (s->szPath != NULL && s->szPath[0] == '\0' 
+     && strwicmp(s->szService,HOMES_NAME) != 0) {
+      DEBUG(0,("NOTICE: No path in service [%s], using %s\n",
+              s->szService,tmpdir()));
+      string_set(&s->szPath,tmpdir());      
+  }
+  
+     return (bRetval);
 }
 
 static struct file_lists
@@ -2174,6 +2366,9 @@
 {
 	pstring netbios_name;
 
+	if (validate_netbios_name(pszParmValue) == False)
+	        return False;
+
 	pstrcpy(netbios_name, pszParmValue);
 
 	standard_sub_basic(netbios_name);
@@ -2194,6 +2389,107 @@
 }
 
 /***************************************************************************
+handle_netbios_aliases -- validate and insert multiple netbios-name
+parameters. It's a pstring global, with the DOS_STRING attribute.
+***************************************************************************/
+static BOOL handle_netbios_aliases(char *pszParmValue, char **parm_ptr) {
+   char *p;
+   pstring buf;
+
+   *buf = '\0';
+   for (p=strtok(pszParmValue, " \t"); p != NULL; p=strtok(NULL," \t")) {
+      if (validate_netbios_name(p) == False)
+         return False;
+      pstrcat(buf,p);
+      pstrcat(buf," ");
+   }
+   buf[MIN(strlen(buf)-1,sizeof(buf))] = '\0';
+
+  /* I've treated it here as an uppercase pstring. P_USTRING --davecb */
+  string_set(parm_ptr,buf);
+  unix_to_dos(*(char **)parm_ptr, True);
+  strupper(*(char **)parm_ptr);
+  return True;
+}
+
+/***************************************************************************
+handle_password_server -- validate and insert multiple netbios-name
+parameters. It's a pstring global.
+***************************************************************************/
+static BOOL handle_password_server(char *pszParmValue, char **parm_ptr) {
+   char *p;
+   pstring buf;
+ 
+   *buf = '\0';
+   if (Globals.security != SEC_SERVER && Globals.security != SEC_DOMAIN) {
+       DEBUG(0,("WARNING: password server set to \"%s\", ",pszParmValue));
+       DEBUGADD(0,("but security is neither server nor domain.\n"
+        "password server value ignored\n"));
+       return True;
+   }
+
+   /* a "*" by itself means search for Primary or Backup Domain controllers */
+   if (Globals.security == SEC_DOMAIN && *pszParmValue == '*') {
+       pstrcpy(buf,pszParmValue); 
+   }
+   else {
+       for (p=strtok(pszParmValue, " \t"); p != NULL; p=strtok(NULL," \t")) {
+          if (validate_netbios_name(p) == False)
+             return False;
+          pstrcat(buf,p);
+          pstrcat(buf," ");
+       }
+       buf[MIN(strlen(buf)-1,sizeof(buf))] = '\0';
+   }
+
+   /* I've treated it here as an uppercase pstring. P_USTRING --davecb */
+   string_set(parm_ptr,buf);
+   unix_to_dos(*(char **)parm_ptr, True);
+   strupper(*(char **)parm_ptr);
+   return True;
+}
+
+/**************************************************************************
+validate a single netbios-name
+**************************************************************************/
+static char *legalNetbiosChars = "abcdefghijklmnopqrstuvwxyz"
+       "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-";
+
+static BOOL validate_netbios_name(char *pszParmValue) {
+   int len, i;
+   char *p;
+
+   /* Alternative 1a: manage FQDN by removing domain */
+   if ((p = strchr(pszParmValue, '.')) != NULL) {
+      DEBUG(0,("WARNING: netbios name \"%s\" contained a dot,\n",
+	       pszParmValue));
+      *p = '\0';
+      DEBUGADD(0,("which is only legal in DNS domain names.\n"));
+      DEBUGADD(0,("Name has been truncated to \"%s\".\n",
+		pszParmValue));
+   }
+
+   /* Alternative 1b: manage over-long name by truncating it */
+   len = strlen(pszParmValue);
+   if (len > 15) {
+      pszParmValue[15] = '\0';
+      len = 15;
+      DEBUG(0,("netbios name is longer than 15 characters, "
+              "and has been truncated to \"%s\".\n", pszParmValue));
+   }
+
+   /* Alternative 2: fail non-alphanumerics. */
+   if ((i = strspn(pszParmValue,legalNetbiosChars)) < len) {
+      DEBUG(0,("netbios name \"%s\" contains non-alphanumeric character "
+              "\"%c\", and cannot be set.\n", pszParmValue, 
+              pszParmValue[i]));
+      return False;
+   }
+   return True;
+}
+
+
+/***************************************************************************
  Do the work of sourcing in environment variable/value pairs.
 ***************************************************************************/
 
@@ -3293,6 +3589,9 @@
 		string_set(&Globals.szWINSserver, "127.0.0.1");
 
 	}
+
+	/* If we get here, check globals. */
+	bRetval = globals_ok();
 
 	return (bRetval);
 }
-------------- next part --------------
--- -	Wed Apr 11 09:53:28 2001
+++ testparm.c	Tue Apr 10 12:46:30 2001
@@ -41,104 +41,12 @@
 
 /***********************************************
  Here we do a set of 'hard coded' checks for bad
- configuration settings.
+configuration settings. Replaced by dynamic checks.
 ************************************************/
 
-static int do_global_checks(void)
-{
-	int ret = 0;
-	SMB_STRUCT_STAT st;
-
-	if (lp_security() == SEC_DOMAIN && !lp_encrypted_passwords()) {
-		printf("ERROR: in 'security=domain' mode the 'encrypt passwords' parameter must also be set to 'true'.\n");
-		ret = 1;
-	}
-
-	if (lp_wins_support() && *lp_wins_server()) {
-		printf("ERROR: both 'wins support = true' and 'wins server = <server>' \
-cannot be set in the smb.conf file. nmbd will abort with this setting.\n");
-		ret = 1;
-	}
-
-	if (!directory_exist(lp_lockdir(), &st)) {
-		printf("ERROR: lock directory %s does not exist\n",
-		       lp_lockdir());
-		ret = 1;
-	} else if ((st.st_mode & 0777) != 0755) {
-		printf("WARNING: lock directory %s should have permissions 0755 for browsing to work\n",
-		       lp_lockdir());
-		ret = 1;
-	}
-
-	/*
-	 * Password server sanity checks.
-	 */
-
-	if((lp_security() == SEC_SERVER || lp_security() == SEC_DOMAIN) && !lp_passwordserver()) {
-		pstring sec_setting;
-		if(lp_security() == SEC_SERVER)
-			pstrcpy(sec_setting, "server");
-		else if(lp_security() == SEC_DOMAIN)
-			pstrcpy(sec_setting, "domain");
-
-		printf("ERROR: The setting 'security=%s' requires the 'password server' parameter be set \
-to a valid password server.\n", sec_setting );
-		ret = 1;
-	}
-
-	/*
-	 * Password chat sanity checks.
-	 */
-
-	if(lp_security() == SEC_USER && lp_unix_password_sync()) {
-
-		/*
-		 * Check that we have a valid lp_passwd_program().
-		 */
-
-		if(lp_passwd_program() == NULL) {
-			printf("ERROR: the 'unix password sync' parameter is set and there is no valid 'passwd program' \
-parameter.\n" );
-			ret = 1;
-		} else {
-			pstring passwd_prog;
-			pstring truncated_prog;
-			char *p;
-
-			pstrcpy( passwd_prog, lp_passwd_program());
-			p = passwd_prog;
-			*truncated_prog = '\0';
-			next_token(&p, truncated_prog, NULL, sizeof(pstring));
-
-			if(access(truncated_prog, F_OK) == -1) {
-				printf("ERROR: the 'unix password sync' parameter is set and the 'passwd program' (%s) \
-cannot be executed (error was %s).\n", truncated_prog, strerror(errno) );
-				ret = 1;
-			}
-		}
-
-		if(lp_passwd_chat() == NULL) {
-			printf("ERROR: the 'unix password sync' parameter is set and there is no valid 'passwd chat' \
-parameter.\n");
-			ret = 1;
-		}
-
-		/*
-		 * Check that we have a valid script and that it hasn't
-		 * been written to expect the old password.
-		 */
-
-		if(lp_encrypted_passwords()) {
-			if(strstr( lp_passwd_chat(), "%o")!=NULL) {
-				printf("ERROR: the 'passwd chat' script [%s] expects to use the old plaintext password \
-via the %%o substitution. With encrypted passwords this is not possible.\n", lp_passwd_chat() );
-				ret = 1;
-			}
-		}
-	}
-
-	return ret;
-}   
+static int do_global_checks(void) {
+  return 0;
+}
 
 static void usage(char *pname)
 {


More information about the samba-technical mailing list