smbclient ignores included config files

[Sebastian Unger]

Hi, since upgrading to Ubuntu 20.04 I cannot gio mount or otherwise connect to my companies' samba shares due to them requiring the NT1 protocol. Adding client min protocol = NT1 to ~/.smb/smb.conf works, but I need to deploy this to a largish number of managed machines. The file /etc/samba/smb.conf on these machines looks like this:

include = /usr/share/samba/smb.conf
include = /usr/share/tait-config/ttde-smb.conf
include = /etc/samba/user.conf

where /usr/share/samba/smb.conf is the config template shipped with samba itself, /usr/share/tait-config/ttde-smb.conf contains config overrides that are deployed via a locally built and hosted config package (the file itself is not a dpkg config file so will get silently rewritten when we update the package) and finally, individual machines' users can customise the config in /etc/samba/user.conf.

Unfortunately adding client min protocol = NT1 only works in the first included file but not the second where we really need this to go. It seems that there is a bug in the included config file parser somewhere.

ubuntu-bug requested me to include this information though I'm not sure how relevant it is:

seb@eragon:~/Documents/work/Terminals[0]$ smbclient -L localhost
Enter WORKGROUP\seb's password:

 Sharename Type Comment
 --------- ---- -------
 print$ Disk Printer Drivers
 Home Disk /home on eragon
 IPC$ IPC IPC Service (eragon server (Samba, Ubuntu))
Reconnecting with SMB1 for workgroup listing.
smbXcli_negprot_smb1_done: No compatible protocol selected by server.
protocol negotiation failed: NT_STATUS_INVALID_NETWORK_RESPONSE
Unable to connect with SMB1 -- no workgroup available

ProblemType: Bug
DistroRelease: Ubuntu 20.04
Package: smbclient 2:4.11.6+dfsg-0ubuntu1.4
ProcVersionSignature: Ubuntu 5.4.0-42.46-generic 5.4.44
Uname: Linux 5.4.0-42-generic x86_64
ApportVersion: 2.20.11-0ubuntu27.8
Architecture: amd64
CasperMD5CheckResult: skip
CurrentDesktop: ubuntu:GNOME
Date: Wed Sep 2 19:24:03 2020
EcryptfsInUse: Yes
ProcEnviron:
 TERM=xterm-256color
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_NZ.UTF-8
 SHELL=/bin/bash
SambaClientRegression: Yes
SourcePackage: samba
UpgradeStatus: No upgrade log present (probably fresh install)

[Paride Legovini (paride)]

Hi Sebastien and thanks for this bug report. The smb.conf(5) manpage says about "include" lines:

  This allows you to include one config file inside another.
  The file is included literally, as though typed in place.

so it would be interesting to see replacing those includes with the actual, literal file contents causes any change in behavior. If it does it's a good indicator of a bug in the include mechanism, and we should probably follow-up upstream. If it does not, then we'll need to dig in a different direction.

Is it only the "client min protocol = NT1" setting that gets ignored in the second included file, or is it all of it?

I'm marking this report to Incomplete for the moment, please change its status back to New after commenting back with your findings, and we'll look at it again. Thanks!

[Paride Legovini (paride)]

Also: samba may not like "reopening" config section, or maybe the [global] section specifically, which I think is what you are doing according to LP: #1880305.

[Sebastian Unger (sebunger44)]

I have tried including

[global]
client min protocol = NT1

in the top-level file both before and after all the includes. In all cases it worked. This seems to indicate that a section can be re-opened and the problem is likely in the include mechanism itself.

[Christian Ehrhardt  (paelzer)]

It somewhat reminds me of bug 1876320 where also the new include mechanism was broken.
Anyway, thanks Sebastian for confirming the thoughts of Paride.

[Christian Ehrhardt  (paelzer)]

I have marked the bug to be clear that someone has to start to debug this.
But I wanted to ask you Sebastian (as you already have the test setup) if you could easily (e.g. in a VM or such) give the 5.12.5 version in groovy a try. If it turns out that works it simply becomes the search for an existing fix which would be much faster.

[Paride Legovini (paride)]

@Sebastian: just to be sure, does *any* of the second included file get included/parsed? In other words, is the problem specific to

[global]
client min protocol = NT1

or is it the whole file that gets ignored, like if samba doesn't go back to finish parsing smb.conf after encountering the first include?

[Sebastian Unger (sebunger44)]

@Paride: Good catch. The second file's contents are:

# Global ttde samba config. Do not edit.
[global]
workgroup = TEL
include=/etc/samba/dhcp.conf
encrypt passwords = true
# ICT don't like us to rebroadcast printers all over the shop
load printers=no
show add printer wizard=no
disable spools=yes
client min protocol = NT1

[Home]
comment = /home on %h
path = /home
writeable = yes

However, connecting with smbclient -L localhost asks for WORKGROUP\seb's password not TEL\seb's. So it's definitely not just the client min protocol. However, the above command *does* list the home share. So its also not the entire file that is being ignored.

I'd also like to point out that I'm not sure this is a new issue! I only noticed it when I tried to put in the client min protocol which is needed since 20.04.

@Christian: I'll try, but that will take a moment longer since I have not yet set our management system up for Groovy since we typically stay with the LTSs and I haven't even managed to get us onto 20.04.

[Sebastian Unger (sebunger44)]

Ok, after a few starting hiccups, I managed to get our management system to support groovy and install a VM. I have only tested the behaviour of the workgroup setting with smbclient -L localhost since it does not rely on any external servers etc.

I can confirm that the behaviour is unchanged in groovy (samba version 2:4.12.5+dfsg-3ubuntu3). I.e. the workgroup setting is ignored in the included file but works in the top-level.

[Christian Ehrhardt  (paelzer)]

To help this case going forward I ensured there is an easy repro for anyone to try.
The following works well on a clean Ubuntu (and most likely any) install.

$ sudo apt install smbclient samba
$ mkdir /tmp/testshare
$ sudo chmod 0777 /tmp/testshare/
$ cat << EOF | sudo tee -a /etc/samba/smb.conf
[testshare]
   comment = Test bug 1893906
   path = /tmp/testshare
   browseable = yes
   read only = no
   create mask = 0700
   directory mask = 0700
   valid users = ubuntu
   guest ok = yes
include = /etc/samba/test1.conf
include = /etc/samba/test2.conf
include = /etc/samba/test3.conf
include = /etc/samba/gtest1.conf
include = /etc/samba/gtest2.conf
include = /etc/samba/gtest3.conf
EOF
$ cat << EOF | sudo tee -a /etc/samba/test1.conf
   comment = Test bug 1893906 - 1
include = /etc/samba/test1b.conf
EOF
$ cat << EOF | sudo tee -a /etc/samba/test1b.conf
   comment = Test bug 1893906 - 1b
EOF
$ cat << EOF | sudo tee -a /etc/samba/test2.conf
   comment = Test bug 1893906 - 2
EOF
$ cat << EOF | sudo tee -a /etc/samba/test3.conf
   comment = Test bug 1893906 - 3
EOF

The overall structure we now created is like:
smb.conf
 \- test1.conf
    \- test1b.conf
 \- test2.conf
 \- test3.conf

$ sudo systemctl restart smbd
$ smbclient -L 127.0.0.1 -U ubuntu%ubuntu | grep testshare
 testshare Disk Test bug 1893906 - 3

We see (and can experiment with) the comment that is just nicely set in each case.

[Christian Ehrhardt  (paelzer)]

Get it to read modified [gobal] sections:

1. drop the "comment=" of the testshare in smb.conf
2. in smb.conf set in the global directive
   comment = Test bug 1893906 - smb.conf
3. Modify the test includes that they look like:
[global]
   comment = Test bug 1893906 - g1
include = /etc/samba/test1b.conf

I found that in this case the re-definition of [global] needs happen before a section that uses it. (No reverse propagation of configuration). Due to that the includes have to move up.

4. move the includes up above the [testshare] section

$ smbclient -L 127.0.0.1 -U ubuntu%ubuntu | grep testshare
 testshare Disk Test bug 1893906 - smb.conf

So we can see that it ignored the new default-comment in that case.

From here moving up the includes for the global just before [testshare] makes it work
$ smbclient -L 127.0.0.1 -U ubuntu%ubuntu | grep testshare
 testshare Disk Test bug 1893906 - g3

But with that we surely know that /etc/samba/test3.conf is read (for the comment).
So let us set the "client min protocol" there - and indeed it's value is not considered as if it would be in the initial [global] section.

It is parsed thou, if I set an invalid value for client min protocol in test3.conf it fails to restart the server.

[Christian Ehrhardt  (paelzer)]

Seeing the above I got the idea that maybe the server properly does include parsing (e.g. for comment of a share) but the clients might not.
"client min protocol" is a client value, let us try "server max protocol" instead.

/etc/samba/test3.conf
[global]
   server max protocol = NT1
   comment = Test bug 1893906 - g3

And yeah, the server side config "server max protocol" has an effect even if in the last config file of the chain. Could it really be that the server includes everything, but the client doesn't?

Most of the configurations are for the server, I picked another one that is more client-like.
   client signing = mandatory
If added to [global] in smb.conf I can clearly see it's effects.
If added to any (or all) of the include files it does nothing.

With that in mind let us focus on that for a minute:
$ strace -rTt -f -o smbclient.strace smbclient -L 127.0.0.1 -U ubuntu%ubuntu

$ grep -e 'samba\/smb\.conf' -e 'test.*\.conf' smbclient.strace
4815 09:52:37 (+ 0.000102) stat("/etc/samba/smb.conf", {st_mode=S_IFREG|0644, st_size=9245, ...}) = 0 <0.000011>
4815 09:52:37 (+ 0.000046) openat(AT_FDCWD, "/etc/samba/smb.conf", O_RDONLY) = 3 <0.000011>

No reading of any of the extra include config files by the client -- hmm suspicious at least

[Sebastian Unger (sebunger44)]

LOL, I would call that a bit more than "suspicious". More like a smoking gun?!

Thanks for putting time into this Christian. Does this bring us any closer to fixing it? Does it need to be raised upstream?

[Christian Ehrhardt  (paelzer)]

Some debug symbols later and with
(gdb) break open if strcmp($rdi,"/etc/samba/smb.conf") == 0

We see that code is mostly from ../../source3/param/loadparm.c:4076

lp_load_client
  -> lp_load_global
     -> lp_load
        -> lp_load_ex
           -> pm_process (lib/util/params.c:94)
              -> tini_parse (lib/util/tini.c)

Real backtrace:
#5 0x00007ffff7808e74 in pm_process (filename=filename@entry=0x5555555c2810 "/etc/samba/smb.conf", sfunc=sfunc@entry=0x7ffff7a709f0 <lp_do_section>,
    pfunc=pfunc@entry=0x7ffff7a678f0 <do_parameter>, private_data=private_data@entry=0x5555555b73b0) at ../../lib/util/params.c:94
#6 0x00007ffff7a7273d in lp_load_ex (pszFname=0x7ffff7810caf "/etc/samba/smb.conf", global_only=global_only@entry=true, save_defaults=save_defaults@entry=false,
    add_ipc=add_ipc@entry=false, reinit_globals=reinit_globals@entry=true, allow_include_registry=allow_include_registry@entry=true, load_all_shares=<optimized out>)
    at ../../source3/param/loadparm.c:3911
#7 0x00007ffff7a73024 in lp_load (reinit_globals=true, add_ipc=false, save_defaults=false, global_only=true, pszFname=<optimized out>) at ../../source3/param/loadparm.c:4048
#8 lp_load_global (file_name=<optimized out>) at ../../source3/param/loadparm.c:4048
#9 0x00007ffff7a73040 in lp_load_client (file_name=<optimized out>) at ../../source3/param/loadparm.c:4076
#10 0x00007ffff7c916c0 in popt_common_credentials_callback (data=<optimized out>, arg=<optimized out>, opt=<optimized out>, reason=<optimized out>, con=0x5555555b6950)
    at ../../source3/lib/popt_common_cmdline.c:107

Compare that to the smbd service:
$ strace -rtT -f -o smbd.strace /usr/sbin/smbd --foreground --no-process-group
$ grep -Hrn -e 'open.*smb.conf' -e "open.*test" smbd.strace
smbd.strace:1747:12014 10:25:26 (+ 0.000049) openat(AT_FDCWD, "/etc/samba/smb.conf", O_RDONLY) = 3 <0.000012>
smbd.strace:1833:12014 10:25:26 (+ 0.000041) openat(AT_FDCWD, "/etc/samba/smb.conf", O_RDONLY) = 5 <0.000010>
smbd.strace:1840:12014 10:25:26 (+ 0.000037) openat(AT_FDCWD, "/etc/samba/test1.conf", O_RDONLY) = 10 <0.000010>
smbd.strace:1845:12014 10:25:26 (+ 0.000040) openat(AT_FDCWD, "/etc/samba/test1b.conf", O_RDONLY) = 11 <0.000009>
smbd.strace:1854:12014 10:25:26 (+ 0.000037) openat(AT_FDCWD, "/etc/samba/test2.conf", O_RDONLY) = 10 <0.000009>
smbd.strace:1861:12014 10:25:26 (+ 0.000037) openat(AT_FDCWD, "/etc/samba/test3.conf", O_RDONLY) = 10 <0.000009>

(gdb) break open if strcmp($rdi,"/etc/samba/smb.conf") == 0
(gdb) break open if strcmp($rdi,"/etc/samba/test1.conf") == 0

Breakpoint 1, __libc_open64 (file=0x5555555a2e40 "/etc/samba/smb.conf", oflag=0) at ../sysdeps/unix/sysv/linux/open64.c:37
37 ../sysdeps/unix/sysv/linux/open64.c: No such file or directory.
(gdb) bt
#0 __libc_open64 (file=0x5555555a2e40 "/etc/samba/smb.conf", oflag=0) at ../sysdeps/unix/sysv/linux/open64.c:37
#1 0x00007ffff72ac196 in __GI__IO_file_open (fp=fp@entry=0x555555590c40, filename=<optimized out>, posix_mode=<optimized out>, prot=prot@entry=438, read_write=8, is32not64=<optimized out>)
    at fileops.c:189
#2 0x00007ffff72ac45a in _IO_new_file_fopen (fp=fp@entry=0x555555590c40, filename=filenam...

[Christian Ehrhardt  (paelzer)]

smbclient config parsing in debug:

$ smbclient -d 4 -L 127.0.0.1 -U ubuntu%ubuntu
...
Processing section "[global]"
doing parameter workgroup = WORKGROUP
doing parameter comment = Test bug 1893906 - smb1.conf
doing parameter server string = %h server (Samba, Ubuntu)
doing parameter log file = /var/log/samba/log.%m
doing parameter max log size = 1000
doing parameter logging = file
doing parameter panic action = /usr/share/samba/panic-action %d
doing parameter server role = standalone server
doing parameter obey pam restrictions = yes
doing parameter unix password sync = yes
doing parameter passwd program = /usr/bin/passwd %u
doing parameter passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
doing parameter pam password change = yes
doing parameter map to guest = bad user
doing parameter usershare allow guests = yes
pm_process() returned Yes
...

server
$ /usr/sbin/smbd -S -d 4 --foreground --no-process-group
...
Processing section "[global]"
doing parameter workgroup = WORKGROUP
doing parameter comment = Test bug 1893906 - smb1.conf
doing parameter server string = %h server (Samba, Ubuntu)
doing parameter log file = /var/log/samba/log.%m
doing parameter max log size = 1000
doing parameter logging = file
doing parameter panic action = /usr/share/samba/panic-action %d
doing parameter server role = standalone server
doing parameter obey pam restrictions = yes
doing parameter unix password sync = yes
doing parameter passwd program = /usr/bin/passwd %u
doing parameter passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
doing parameter pam password change = yes
doing parameter map to guest = bad user
doing parameter usershare allow guests = yes
pm_process() returned Yes
get_current_groups: user is in 1 groups: 0
Registered MSG_REQ_POOL_USAGE
Registered MSG_REQ_DMALLOC_MARK and LOG_CHANGED
lp_load_ex: refreshing parameters
Initialising global parameters
rlimit_max: increasing rlimit_max (1024) to minimum Windows limit (16384)
Processing section "[global]"
doing parameter workgroup = WORKGROUP
doing parameter comment = Test bug 1893906 - smb1.conf
doing parameter server string = %h server (Samba, Ubuntu)
doing parameter log file = /var/log/samba/log.%m
doing parameter max log size = 1000
doing parameter logging = file
doing parameter panic action = /usr/share/samba/panic-action %d
doing parameter server role = standalone server
doing parameter obey pam restrictions = yes
doing parameter unix password sync = yes
doing parameter passwd program = /usr/bin/passwd %u
doing parameter passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
doing parameter pam password change = yes
doing parameter map to guest = bad user
doing parameter usershare allow guests = yes
Processing section "[printers]"
doing parameter comment = All Printers
doing parameter browseable = no
doing parameter path = /var/spool/samba
doing parameter printable = yes
doing parameter guest ok = no
doing parameter read only = yes
doing parameter create mask = 0700
Processing section "[print$]"
doing paramet...

[Christian Ehrhardt  (paelzer)]

I checked, even adding [global] before the includes (when keeping them later in the config) will make them work. As then the include is "bInGlobalSection" and followed.

The workaround for current users would be to place includes into the global section if it is intended that the includes shall be able to set global config (like the one used for clients).

IMHO that is an upstream bug - includes should always be processed (as they could add the new sections), but that is a report to be done after this is confirmed here.

@Sebastian - could you check if your "include" statements are themselves in the [global] section. If not the clients will not have a chance to reach them. In that case move the includes there OR re-open the [global] section before the include.
Does that get your "client min protocol" set as you'd expect then?

[Sebastian Unger (sebunger44)]

They were not. Adding a [global] before each include (since sections opened in an include file stay open in the parent) makes them work as expected!

Thanks so much Christian for your work on this!

[Christian Ehrhardt  (paelzer)]

Ok, I'm glad we are not facing two odd behaviors then and to be able to provide a workaround to affected users that way.

[Christian Ehrhardt  (paelzer)]

Reported upstream in https://bugzilla.samba.org/show_bug.cgi?id=14481

[Christian Ehrhardt  (paelzer)]

I have put some suggestions on the upstream bug, but we'd rely on them to resolve it.
Until then placing the includes in the right place (or prepending [global]) will fix this as a workaround.