Winbind issue after upgrading from 4.7.x to 4.8.x

[miguel.sanders.external at arcelormittal.com]

Hi guys

We recently upgraded our Samba clusters from 4.7.x to 4.8.x and noticed 
a difference in behavior when AD users have a (local) Linux user equivalent.
Assume we have a local Linux user XYZ (UID 519) as well as a AD user 
object XYZ (UID 30001).

4.7.x
# id XYZ
uid=519(XYZ) gid=1(bin) groups=1(bin)

4.8.x
# id XYZ
uid=519(XYZ) gid=1(bin) groups=1(bin),30004(DOMAIN+domain users)

The problem originates from the parse_domain_user() function in 
winbindd_util.c
In 4.7.x, if there is no winbind separator and no '@' in domuser, the 
function simply returns false.

bool parse_domain_user(const char *domuser, fstring domain, fstring user)
{
         char *p = strchr(domuser,*lp_winbind_separator());

         if ( !p ) {
                 fstrcpy(user, domuser);
                 p = strchr(domuser, '@');

                 if ( assume_domain(lp_workgroup()) && p == NULL) {
                         fstrcpy(domain, lp_workgroup());
                 } else if (p != NULL) {
                         fstrcpy(domain, p + 1);
                         user[PTR_DIFF(p, domuser)] = 0;
                 } else {
*return False;*
                 }
         } else {
                 fstrcpy(user, p+1);
                 fstrcpy(domain, domuser);
                 domain[PTR_DIFF(p, domuser)] = 0;
         }

         return strupper_m(domain);
}


In 4.8.x, we have reversed logic but the function returns true even 
though there is no winbind separator and no '@' in domuser.

bool parse_domain_user(const char *domuser,
                        fstring namespace,
                        fstring domain,
                        fstring user)
{
         char *p = NULL;

         if (strlen(domuser) == 0) {
                 return false;
         }

         p = strchr(domuser, *lp_winbind_separator());
         if (p != NULL) {
                 fstrcpy(user, p + 1);
                 fstrcpy(domain, domuser);
                 domain[PTR_DIFF(p, domuser)] = '\0';
                 fstrcpy(namespace, domain);
         } else {
                 fstrcpy(user, domuser);

                 domain[0] = '\0';
                 namespace[0] = '\0';
                 p = strchr(domuser, '@');
                 if (p != NULL) {
                         /* upn */
                         fstrcpy(namespace, p + 1);
                 } else if (assume_domain(lp_workgroup())) {
                         fstrcpy(domain, lp_workgroup());
                         fstrcpy(namespace, domain);
                 }
         }

         return strupper_m(domain);
}


We are simply missing the else block which should return false (similar 
to the 4.7.x code).
Attached a simply patch which solves the issue.

Review is appreciated

Many thanks

-- 
Met vriendelijke groeten
Best regards

*Miguel Sanders*
ArcelorMittal Europe – Flat Products – Business Division North

External collaborator | Midrange UNIX
John Kennedylaan 51 B-9042 Gent
*T* +32 9 347 52 78
*E* gen-sid-ism-cbi-sig at arcelormittal.com
*E* miguel.sanders.external at arcelormittal.com
-------------- next part --------------
--- /tmp/winbindd_util.c	2018-06-26 16:42:46.000000000 +0200
+++ winbindd_util.c	2018-08-09 14:43:56.386223278 +0200
@@ -1605,6 +1605,8 @@
 		} else if (assume_domain(lp_workgroup())) {
 			fstrcpy(domain, lp_workgroup());
 			fstrcpy(namespace, domain);
+		} else {
+			return false;
 		}
 	}