Two patches to add self-checks, part 1 of 7

[David Collier-Brown -- Customer Engineering]

Andrew Bartlett wrote:
> I almost applied it, but I'm not sure about some of the assertions being
> made.  In particular, I think some of the configurations that are
> restricted actually make sense:


	Ok, my sugegstions follow...


>>+	if (*lp_winbind_separator() == '+') {
>>+		/* Self-test 9 */
>>+		DEBUG(0,("WARNING: winbind separator = + may cause "
>>+			 "problems with group membership.\n"));
>>+	}
> 
> 
> We need to be able to 'warn' without filling logs.  Perhaps make level 1
> or 2 for warnings?  Or some way that testparm can log these at level 0,
> but others at 1/2?

	Hmmn: if testparm defaulted to debug level = 2
	then we could use 1 for warnings and 2 for notices.
	It's a simple enough program you shouldn't get
	too many exteraneous messages at levels 1 and 2...
	and you don't: you get only one extra message,
	INFO: Debug class all level = 2   (pid 10202 from pid 10202)

	Therefor we can do that.


> 
> 
>>+ 
>>+    /* Password server should be a netbios name or IP address. */
>>+    if (lp_passwordserver != NULL) {
>>+ 	if (strchr(lp_passwordserver(),'.') != NULL &&
>>+ 	    !isdigit(*lp_passwordserver())) {
>>+		/* Self-test 10. */
>>+ 		DEBUG(0,("ERROR: password server \"%s\" is not a legal "
>>+ 			 "NetBIOS name or IP address, logons will fail.\n",
>>+ 			 lp_passwordserver()));
>>+ 		bRetval = False;
> 
> 
> DO logons really fail?  To my mind, these would work fine, as long as
> the host we are pointing too is in DNS.  Given we are moving to AD, this
> is quite a reasonable possibility.

	If this is something we wish to acheive, then say:

#define DNS_PASSWD_SERVER_UNSUPPORTED 1
#ifdef DNS_PASSWD_SERVER_UNSUPPORTED
  /* Password server should be a netbios name or IP address. */
     if (lp_passwordserver != NULL) {
  	if (strchr(lp_passwordserver(),'.') != NULL &&
  	    !isdigit(*lp_passwordserver())) {
		/* Self-test 10. */
  		DEBUG(0,("ERROR: password server \"%s\" is not a legal "
  			 "NetBIOS name or IP address, logons will fail",
  			 lp_passwordserver()));
  		bRetval = False;
     }
#endif
	as a remonder to ourselves to ass the capability.



>>+    /* Be sure update encrypted is done with NON-encrypted passwords. */
>>+    if (lp_update_encrypted() && lp_encrypted_passwords()) {
>>+       /* Self-test 12. */
>>+       DEBUG(0,("WARNING: update encrypted = yes requires encrypt "
>>+          "passwords = yes.\n"));
> 
> 
> Requires 'encrypt passwords = no'.

	OOPS!  Typo on my part.


>>+ 
>>+    /* 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) {
>>+	/* Self-test 25. */
>>+       DEBUG(0,( "NOTICE: Service [%s] is unbrowsable, but browse "
>>+            "lists are being served.\n", s->szService));
>>+    }
> 
> 
> I'm not quite sure on this one - isn't the idea here just to hide a
> couple of shares from the normally visible list?

	yes, it arguably should be at a low debug level, as well
	as being just a notice.

>>+    /* A "*" by itself means search for Primary or Backup Domain controllers */
>>+    if (lp_security() == SEC_DOMAIN && *pszParmValue == '*') {
>>+        pstrcpy(buf,pszParmValue); 
> 
> 
> This is valid (if stupid - even easier to spoof) for security=server
> too.

	Ok, the test shouold be for
if ((lp_security() == SEC_DOMAIN || lp_security() == SEC_SERVER)
    && *pszParmValue == '*')

--dave
-- 
David Collier-Brown,           | Always do right. This will gratify
Sun Microsystems DCMO          | some people and astonish the rest.
Toronto, Ontario               |
(905) 415-2849 or x52849       | davecb at canada.sun.com