Samba won't start when an include statement in smb.conf has a variable substitution

[Mike E.]

[Impact]

Samba in AD mode refuses to start when the smb.conf file contains an include line with a variable substitution like "include = /etc/samba/smb.conf.%U"

This happens because the initscript calls "samba-tool testparm" to obtain a configuration parameter from smb.conf, and this testparm tool fails because it doesn't expand the %U macro and fails to read the literal filename.

Note that "samba-tool testparm" is different from just "testparm". The latter doesn't fail. We could just replace one with the other in the initscript, but later on in the process of provisioning an AD controller this error is encountered again, so it's best to fix it properly.

The patch is straight from an upstream commit, and that code is in place in the samba packages from bionic and cosmic already.

[Test Case]

* install samba:
sudo apt install samba

* create /etc/samba/smb.conf with this content:
[global]
        netbios name = samba
 log file = /var/log/samba/log.%m
 map to guest = Bad User
 max log size = 1000
 obey pam restrictions = Yes
 pam password change = Yes
 panic action = /usr/share/samba/panic-action %d
 passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
 passwd program = /usr/bin/passwd %u
 server string = %h server (Samba, Ubuntu)
 unix password sync = Yes
 usershare allow guests = Yes
 idmap config * : backend = tdb
 include = /etc/samba/smb.conf.%U

* run the command that fails:
ubuntu@trusty-samba-include:~$ sudo samba-tool testparm -d 2
lpcfg_load: refreshing parameters from /etc/samba/smb.conf
Can't find include file /etc/samba/smb.conf.%U
ERROR: Unable to load default file
ubuntu@trusty-samba-include:~$ echo $?
255

Note the debug level 2 message saying the include file couldn't be found.

* install the updated packages and run the same command again. This time it will work, exit with status 0, and show the main config file.
ubuntu@trusty-samba-include:~$ sudo samba-tool testparm -d 2 --suppress-prompt
lpcfg_load: refreshing parameters from /etc/samba/smb.conf
Tried to load /etc/samba/smb.conf.%U but variable substitution in filename, ignoring file.
# Global parameters
[global]
 netbios name = SAMBA
 server string = %h server (Samba, Ubuntu)
 map to guest = Bad User
 obey pam restrictions = Yes
 pam password change = Yes
 passwd program = /usr/bin/passwd %u
 passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
 unix password sync = Yes
 log level = 2
 log file = /var/log/samba/log.%m
 max log size = 1000
 usershare allow guests = Yes
 panic action = /usr/share/samba/panic-action %d
 idmap config * : backend = tdb
 include = /etc/samba/smb.conf.%U
ubuntu@trusty-samba-include:~$ echo $?
0

We can also see the new debugging message saying that the include file was ignored because of the variable substitution.

[Regression Potential]
If you happen to include a filename that has an actual "%" in its name, followed by a letter, and it doesn't exist, this patch will not flag that as an error and just ignore it, whereas before it would be flagged. Having such a filename is asking for trouble, though, because %<letter> is a variable and samba would try to expand it.

[Other Info]
The fix and test procedure is identical for trusty and xenial. It's the same samba version in both releases of ubuntu.

There is still a difference in behaviour between "testparm(1)" and "samba-tool testparm". This fix only affects "samba-tool testparm":
ubuntu@trusty-samba-include:~$ testparm
Load smb config files from /etc/samba/smb.conf
Can't find include file /etc/samba/smb.conf.
(...)
Note the ending dot in the error above, that's where %U would come. testparm doesn't exit non-zero because of that, though.

[Original Description]

Samba refuses to start when the smb.conf file contains an include line with a variable substitution like "include = /etc/samba/smb.conf.%U"

According to the man page for smb.conf, all but a few specific variable substitutions should work.

include (G)

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

           It takes the standard substitutions, except %u, %P and %S.

           The parameter include = registry has a special meaning: It does not include a file named registry from the current working directory, but
           instead reads the global configuration options from the registry. See the section on registry-based configuration for details. Note that
           this option automatically activates registry shares.

           Default: include =

           Example: include = /usr/local/samba/lib/admin_smb.conf

It is probably related to this bug in samba:
https://bugzilla.samba.org/show_bug.cgi?id=10722

Description: Ubuntu 16.04 LTS
Release: 16.04

samba:
  Installed: 2:4.3.9+dfsg-0ubuntu0.16.04.1
  Candidate: 2:4.3.9+dfsg-0ubuntu0.16.04.1
  Version table:
 *** 2:4.3.9+dfsg-0ubuntu0.16.04.1 500
        500 http://us.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages
        500 http://security.ubuntu.com/ubuntu xenial-security/main amd64 Packages
        100 /var/lib/dpkg/status
     2:4.3.8+dfsg-0ubuntu1 500
        500 http://us.archive.ubuntu.com/ubuntu xenial/main amd64 Packages

[Andreas Hasenack (ahasenack)]

This seems to be working in 4.3.11 that is in xenial:
root@xenial-samba-dynamic-include:~# testparm -s
Load smb config files from /etc/samba/smb.conf
rlimit_max: increasing rlimit_max (1024) to minimum Windows limit (16384)
WARNING: The "syslog" option is deprecated
Can't find include file /etc/samba/smb.conf.
Loaded services file OK.
WARNING: The 'netbios name' is too long (max. 15 chars).

Server role: ROLE_STANDALONE

# Global parameters
[global]
 server string = %h server (Samba, Ubuntu)
 server role = standalone server
 map to guest = Bad User
 obey pam restrictions = Yes
 pam password change = Yes
 passwd program = /usr/bin/passwd %u
 passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
 unix password sync = Yes
 syslog = 0
 log file = /var/log/samba/log.%m
 max log size = 1000
 dns proxy = No
 usershare allow guests = Yes
 panic action = /usr/share/samba/panic-action %d
 idmap config * : backend = tdb
 include = /etc/samba/smb.conf.
root@xenial-samba-dynamic-include:~# echo $?
0

Notice how it didn't expand %U, and the include file was /etc/samba/smb.conf. (ending dot). Restart works:
root@xenial-samba-dynamic-include:~# systemctl restart smbd nmbd
root@xenial-samba-dynamic-include:~# systemctl status smbd nmbd
● smbd.service - LSB: start Samba SMB/CIFS daemon (smbd)
   Loaded: loaded (/etc/init.d/smbd; bad; vendor preset: enabled)
   Active: active (running) since Tue 2018-07-17 19:12:51 UTC; 3s ago
     Docs: man:systemd-sysv-generator(8)
  Process: 2983 ExecStop=/etc/init.d/smbd stop (code=exited, status=0/SUCCESS)
  Process: 3012 ExecStart=/etc/init.d/smbd start (code=exited, status=0/SUCCESS)
    Tasks: 3
   Memory: 7.1M
      CPU: 127ms
   CGroup: /system.slice/smbd.service
           ├─3023 /usr/sbin/smbd -D
           ├─3024 /usr/sbin/smbd -D
           └─3026 /usr/sbin/smbd -D

Jul 17 19:12:51 xenial-samba-dynamic-include systemd[1]: Starting LSB: start Samba SMB/CIFS daemon (smbd)...
Jul 17 19:12:51 xenial-samba-dynamic-include smbd[3012]: * Starting SMB/CIFS daemon smbd
Jul 17 19:12:51 xenial-samba-dynamic-include smbd[3012]: ...done.
Jul 17 19:12:51 xenial-samba-dynamic-include systemd[1]: Started LSB: start Samba SMB/CIFS daemon (smbd).

● nmbd.service - LSB: start Samba NetBIOS nameserver (nmbd)
   Loaded: loaded (/etc/init.d/nmbd; bad; vendor preset: enabled)
   Active: active (running) since Tue 2018-07-17 19:12:51 UTC; 3s ago
     Docs: man:systemd-sysv-generator(8)
  Process: 2990 ExecStop=/etc/init.d/nmbd stop (code=exited, status=0/SUCCESS)
  Process: 2998 ExecStart=/etc/init.d/nmbd start (code=exited, status=0/SUCCESS)
    Tasks: 1
   Memory: 3.7M
      CPU: 127ms
   CGroup: /system.slice/nmbd.service
           └─3011 /usr/sbin/nmbd -D

Jul 17 19:12:51 xenial-samba-dynamic-include systemd[1]: Starting LSB: start Samba NetBIOS nameserver (nmbd)...
Jul 17 19:12:51 xenial-samba-dynamic-include nmbd[2998]: * Starting NetBIOS name server nmbd
Jul 17 19:12:51 xenial-samba-dynamic-include nmbd[2998]: ...done.
Jul 17 19:12:51 xenial-samba-dynamic-include systemd[1]: Started LSB: start Samba NetBIOS nameserver (nmbd).

Are you also seeing this behavior n...

[Andreas Hasenack (ahasenack)]

The upstream linked bug is still open, though, so maybe I'm not hitting the same exact error condition. In that case, please attach an smb.conf that reproduces the error.

Thanks!

[Mike E. (melkevizth)]

With the latest version (4.3.11+dfsg-0ubuntu0.16.04.13) I still get failures when I try to startup samba with an include that contains %U.

I think the difference is that I'm using samba as an AD DC. So the service is started with systemctl (re)start samba-ad-dc instead of the smbd and nmbd services.

The watered down smb.conf is:

[global]
    workgroup = WORKGROUP
    realm = AD.WORKGROUP.COM
    netbios name = SERVER1
    server role = active directory domain controller
    server services = s3fs, rpc, nbt, wrepl, ldap, cldap, kdc, drepl, winbindd, ntp_signd, kcc, dnsupdate
    idmap_ldb:use rfc2307 = yes
    include = /etc/samba/smb.conf.%U

[netlogon]
    path = /var/lib/samba/sysvol/ad.workgroup.com/scripts
    read only = No

[sysvol]
    path = /var/lib/samba/sysvol
    read only = No

Samba will refuse to start with the above smb.conf unless the include line is removed or changed to a static file.

[Andreas Hasenack (ahasenack)]

Ah, looks like a different behaviour between testparm and samba-tool testparm:

root@xenial-samba-include:~# testparm -s &>/dev/null; echo $?
0
root@xenial-samba-include:~# samba-tool testparm; echo $?
ERROR: Unable to load default file
255

[Andreas Hasenack (ahasenack)]

As a workaround, if you replace the indicated line in /etc/init.d/samba-ad-dc, does it work?
--- /etc/init.d/samba-ad-dc.orig 2018-07-20 13:33:22.418002978 +0000
+++ /etc/init.d/samba-ad-dc 2018-07-20 13:33:34.966042631 +0000
@@ -27,7 +27,7 @@

 case "$1" in
  start)
- SERVER_ROLE=`samba-tool testparm --parameter-name="server role" 2>/dev/null | tail -1`
+ SERVER_ROLE=`testparm -s --parameter-name="server role" 2>/dev/null | tail -1`
   if [ "$SERVER_ROLE" != "active directory domain controller" ]; then
       exit 0
   fi

i.e., replace "samba-tool testparm" with "testparm -s", and then issue

sudo systemctl daemon-reload

But I suspect you added the %U include directive after provisioning the domain, as the provision tool complains about it if you start off with such a smb.conf, so this needs a better fix.

[Mike E. (melkevizth)]

I confirmed that this change does allow the server to start with a %U in the include line.

[Andreas Hasenack (ahasenack)]

Commit closing the upstream bug: https://github.com/samba-team/samba/commit/3c6ea3293c6aac67bc442f47185fd494714e4806

[Andreas Hasenack (ahasenack)]

I applied the upstream patch and looks like it solves the problem. Here is a PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/samba-include-1583324/+packages

I did a quick upgrade test (was broken before, worked after), and also checked samba-tool testparm, and it's not complaining anymore about the include line.

Monday I'll refine this testing a bit, but it looks good so far.

[Andreas Hasenack (ahasenack)]

Marking as fix released in the main task because it's fixed in bionic and later.

[C de-Avillez (hggdh2)]

accepted for Xenial and Trusty updates

[Robie Basak (racb)]

Andreas' sponsored uploads were trumped by security. He has rebased them, and I'm "sponsoring" his rebased versions for Xenial and Trusty on the basis that I've checked that the changes he's introducing are identical to what was sponsored previously.

[Robie Basak (racb)]

Hello Mike, or anyone else affected,

Accepted samba into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/samba/2:4.3.11+dfsg-0ubuntu0.16.04.16 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Robie Basak (racb)]

Hello Mike, or anyone else affected,

Accepted samba into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/samba/2:4.3.11+dfsg-0ubuntu0.14.04.17 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-trusty to verification-done-trusty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-trusty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Mike E. (melkevizth)]

I confirmed that the version of samba in the xenial-proposed repository (version 2:4.3.11+dfsg-0ubuntu0.16.04.16) does allow the server to start with a %U in the include line while the server role is set to active directory domain controller. I don't have access to a trusty install, so I won't be able to confirm that.

[Andreas Hasenack (ahasenack)]

Trusty verification

Starting with the packages that show the bug:
ubuntu@trusty-samba-include-1583324:~$ apt-cache policy samba
samba:
  Installed: 2:4.3.11+dfsg-0ubuntu0.14.04.16
  Candidate: 2:4.3.11+dfsg-0ubuntu0.14.04.16
  Version table:
 *** 2:4.3.11+dfsg-0ubuntu0.14.04.16 0
        500 http://archive.ubuntu.com/ubuntu/ trusty-updates/main amd64 Packages
        500 http://security.ubuntu.com/ubuntu/ trusty-security/main amd64 Packages
        100 /var/lib/dpkg/status

ubuntu@trusty-samba-include-1583324:~$ sudo samba-tool testparm -d 2
lpcfg_load: refreshing parameters from /etc/samba/smb.conf
Can't find include file /etc/samba/smb.conf.%U
ERROR: Unable to load default file
ubuntu@trusty-samba-include-1583324:~$ echo $?
255

Upgrading to the packages from proposed:
ubuntu@trusty-samba-include-1583324:~$ apt-cache policy samba
samba:
  Installed: 2:4.3.11+dfsg-0ubuntu0.14.04.17
  Candidate: 2:4.3.11+dfsg-0ubuntu0.14.04.17
  Version table:
 *** 2:4.3.11+dfsg-0ubuntu0.14.04.17 0
        500 http://archive.ubuntu.com/ubuntu/ trusty-proposed/main amd64 Packages
        100 /var/lib/dpkg/status

The testparm command now works. It shows the config file, remarks that the "%U" file was ignored, and exits with status 0:
ubuntu@trusty-samba-include-1583324:~$ sudo samba-tool testparm -d 2
lpcfg_load: refreshing parameters from /etc/samba/smb.conf
Tried to load /etc/samba/smb.conf.%U but variable substitution in filename, ignoring file.
Press enter to see a dump of your service definitions

# Global parameters
[global]
 netbios name = SAMBA
 server string = %h server (Samba, Ubuntu)
 map to guest = Bad User
 obey pam restrictions = Yes
 pam password change = Yes
 passwd program = /usr/bin/passwd %u
 passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
 unix password sync = Yes
 log level = 2
 log file = /var/log/samba/log.%m
 max log size = 1000
 usershare allow guests = Yes
 panic action = /usr/share/samba/panic-action %d
 idmap config * : backend = tdb
 include = /etc/samba/smb.conf.%U
ubuntu@trusty-samba-include-1583324:~$ echo $?
0

Trusty verification complete.

[Łukasz Zemczak (sil2100)]

The gvfs autopkgtest regressions are broken in xenial-updates already. Hinting in.

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for samba has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package samba - 2:4.3.11+dfsg-0ubuntu0.16.04.16

---------------
samba (2:4.3.11+dfsg-0ubuntu0.16.04.16) xenial; urgency=medium

  * d/p/bug_1583324_include_with_macro.patch: don't fail parsing the
    config file if it has macros in include directives (LP: #1583324)

 -- Andreas Hasenack <email address hidden> Thu, 02 Aug 2018 18:30:26 -0300

[Launchpad Janitor (janitor)]

This bug was fixed in the package samba - 2:4.3.11+dfsg-0ubuntu0.14.04.17

---------------
samba (2:4.3.11+dfsg-0ubuntu0.14.04.17) trusty; urgency=medium

  * d/p/bug_1583324_include_with_macro.patch: don't fail parsing the
    config file if it has macros in include directives (LP: #1583324)

 -- Andreas Hasenack <email address hidden> Thu, 02 Aug 2018 18:27:50 -0300