Bug 12276 - Group substitution for connected user fails
Summary: Group substitution for connected user fails
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.4.5
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-19 11:59 UTC by Andreas Schneider
Modified: 2020-12-11 13:27 UTC (History)
6 users (show)

See Also:


Attachments
Proposed patch for master (1.07 KB, patch)
2016-09-21 07:23 UTC, Andreas Schneider
no flags Details
patch for 4.5 (2.00 KB, patch)
2016-10-10 10:44 UTC, Andreas Schneider
slow: review+
Details
patch for 4.4 (2.00 KB, patch)
2016-10-10 10:45 UTC, Andreas Schneider
slow: review+
Details
Additional patch for 4.5, cherry-picked from master (1.27 KB, patch)
2017-04-25 14:04 UTC, Ralph Böhme
slow: review? (jra)
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2016-09-19 11:59:29 UTC
smbclient -UZELTRUST+Administrator%secret -c quit //127.0.0.1/zelshare/
Domain=[ZELTRUST] OS=[Windows 6.1] Server=[Samba 4.4.4]
tree connect failed: NT_STATUS_BAD_NETWORK_NAME

Error in log.smbd:
++++++
[2016/09/19 05:03:58.549831,  0, pid=24686, effective(0, 0), real(0, 0)] ../source3/smbd/service.c:808(make_connection_snum) canonicalize_connect_path failed for service zelshare, path /home/DZELTRUST/uZELTRUST+administrator/Uadministrator/gZELTRUST+domain users/G%G
++++++

config:

Server role: ROLE_DOMAIN_MEMBER

# Global parameters
[global]
        realm = ZELTRUST.ZEL
        workgroup = ZELTRUST
        security = ADS
        server signing = if_required
        create krb5 conf = No
        template homedir = /home/%D/%G/%U
        template shell = /bin/bash
        winbind request timeout = 120
        winbind separator = +
        wins server = 10.34.36.20
        idmap config * : range = 10000-20000
        idmap config * : backend = tdb
                     
[zelshare]                                                                      
        path = /home/D%D/u%u/U%U/g%g/G%G
        read only = No


Patch will follow.
Comment 1 Christian Ambach 2016-09-20 21:02:31 UTC
Dupe of Bug 10286?
Comment 2 Andreas Schneider 2016-09-21 07:22:48 UTC
No, bug #10286 introduced the issue.

diff --git a/source3/lib/substitute.c b/source3/lib/substitute.c
index ce4fbba..4e2ce9b 100644
--- a/source3/lib/substitute.c
+++ b/source3/lib/substitute.c
@@ -500,7 +500,9 @@ char *talloc_sub_basic(TALLOC_CTX *mem_ctx,
 		case 'G' : {
 			struct passwd *pass;
 
-			if (domain_name != NULL && domain_name[0] != '\0') {
+			if (domain_name != NULL && domain_name[0] != '\0' &&
+			    !strequal(domain_name, my_sam_name()))
+			{
 				r = talloc_asprintf(tmp_ctx,
 						    "%s%c%s",
 						    domain_name,

We are 'security = ADS'

domain_name is MYDOMAIN
lp_workgroup() is MYDOMAIN which is returned by my_sam_name().

So we do a:
    getpwnam(Administrator)

this will fail as there is no such user on our system unless you have 'winbind use default domain' set! In the case of a member server we need to add the domain name!
Comment 3 Andreas Schneider 2016-09-21 07:23:24 UTC
Created attachment 12500 [details]
Proposed patch for master
Comment 4 Andreas Schneider 2016-10-10 10:44:15 UTC
Created attachment 12558 [details]
patch for 4.5
Comment 5 Andreas Schneider 2016-10-10 10:45:01 UTC
Created attachment 12559 [details]
patch for 4.4
Comment 6 Ralph Böhme 2016-10-10 11:37:26 UTC
Reassigning to Karolin for inclusion in 4.4 and 4.5.
Comment 7 Karolin Seeger 2016-10-19 07:49:02 UTC
(In reply to Ralph Böhme from comment #6)
Pushed to autobuild-v4-{5,4}-test.
Comment 8 Karolin Seeger 2016-10-25 07:42:10 UTC
(In reply to Karolin Seeger from comment #7)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 9 Ralph Böhme 2017-04-25 13:32:36 UTC
We need probably need backports of 6ec81ca3c196f3c4659a4e1c473759b393708d12 for 4.5 and 4.6.

On a 4.5.6 system without 6ec81ca3c196f3c4659a4e1c473759b393708d12 I saw this crash:

...
#4  0x00007f1291ac9703 in fault_report (sig=11) at ../lib/util/fault.c:83
#5  sig_fault (sig=11) at ../lib/util/fault.c:94
#6  <signal handler called>
#7  0x00007f128ddc45d4 in __strchr_sse42 () from /lib64/libc.so.6
#8  0x00007f1291aba529 in strchr_m (src=0x1 <Address 0x1 out of bounds>, c=37 '%') at ../lib/util/charset/util_str.c:338
#9  0x00007f128f1cc8f2 in talloc_sub_basic (mem_ctx=<value optimized out>, smb_name=0x7f128f404fa0 "froh", domain_name=0x7f128f4051a0 "ITS-AD", str=<value optimized out>) at ../source3/lib/substitute.c:487
#10 0x00007f128f1d7ed2 in lp_string (ctx=0x7f1293fbdfc0, s=0x7f1293f97be0 "/etc/samba/buildprofildir %U %G") at ../source3/param/loadparm.c:982
#11 0x00007f128f1d8861 in lp_root_preexec (ctx=<value optimized out>, i=<value optimized out>) at default/lib/param/param_functions.c:178
#12 0x00007f12916a84a5 in make_connection_snum (xconn=0x7f1293fc4360, conn=0x7f1293fc20c0, snum=6, vuser=0x7f1293fc6ad0, pdev=<value optimized out>) at ../source3/smbd/service.c:672
...

(gdb) f 9
#9  0x00007f128f1cc8f2 in talloc_sub_basic (mem_ctx=<value optimized out>, smb_name=0x7f128f404fa0 "froh", domain_name=0x7f128f4051a0 "ITS-AD", str=<value optimized out>) at ../source3/lib/substitute.c:487
487             for (b = s = a_string; (p = strchr_m(s, '%')); s = a_string + (p - b)) {

This looks very much like it's caused by p being overwritten in the for loop.
Comment 10 Ralph Böhme 2017-04-25 14:04:26 UTC
Created attachment 13173 [details]
Additional patch for 4.5, cherry-picked from master

This is only needed in 4.5, 4.6 already has it.
Comment 11 Ralph Böhme 2017-04-25 15:20:21 UTC
Re-reassigning to Karolin for inclusion in 4.5.
Comment 12 Karolin Seeger 2017-04-28 06:59:40 UTC
(In reply to Ralph Böhme from comment #11)
Pushed additional patch to autobuild-v4-5-test.
Comment 13 Karolin Seeger 2017-05-02 07:28:00 UTC
(In reply to Karolin Seeger from comment #12)
Pushed to v4-5-test.
Closing out bug report.

Thanks!