Merge ~ahasenack/ubuntu/+source/samba:trusty-samba-include-1583324 into ubuntu/+source/samba:ubuntu/trusty-devel

Proposed by Andreas Hasenack
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: b00d3badfeee22c5d45e174bfeb6e85862986daa
Merge reported by: Andreas Hasenack
Merged at revision: a16ccf562aa30e940585272256052c9cf054ba77
Proposed branch: ~ahasenack/ubuntu/+source/samba:trusty-samba-include-1583324
Merge into: ubuntu/+source/samba:ubuntu/trusty-devel
Diff against target: 95 lines (+73/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/bug_1583324_include_with_macro.patch (+65/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
Canonical Server Pending
Review via email: mp+352300@code.launchpad.net

Description of the change

Fix for https://bugs.launchpad.net/ubuntu/+source/samba/+bug/1583324. Bug has the SRU template and testing instructions.

PPA with test packages: https://launchpad.net/~ahasenack/+archive/ubuntu/samba-include-1583324 (ppa:ahasenack/samba-include-1583324).

TL;DR "samba-tool testparm" chokes when smb.conf has an include file directive that uses %variable substitutions: it tries to load the filename without expanding the variable. The patch makes it ignore such cases.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

If approving this, then also please push the upload tag and sponsor it, as I can't upload samba myself. Thanks.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Formalisms (changelog, version, headers, ...) all LGTM.
Code change itself reads ok from a samba code POV.
Changes are upstream and in newer Ubuntu releases already for a while.

next is testing ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

First of all upgrade and the fix as described worked and you have my +1 on what you fixed.

An extra commend/discussion in the Xenial MP of this.

Revision history for this message
Christian Ehrhardt  (paelzer) :
review: Needs Information
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

+1 after discussion in the related Xenial MP

review: Approve
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

upload/2%4.3.11+dfsg-0ubuntu0.14.04.15 pushed

$ dput ubuntu ../samba_4.3.11+dfsg-0ubuntu0.14.04.15_source.changes
Checking signature on .changes
Checking signature on .dsc
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading samba_4.3.11+dfsg-0ubuntu0.14.04.15.dsc: done.
  Uploading samba_4.3.11+dfsg-0ubuntu0.14.04.15.debian.tar.xz: done.
  Uploading samba_4.3.11+dfsg-0ubuntu0.14.04.15_source.buildinfo: done.
  Uploading samba_4.3.11+dfsg-0ubuntu0.14.04.15_source.changes: done.
Successfully uploaded packages.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Please set to merged when accepted by the SRU Team

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 43819f4..af969c6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
1samba (2:4.3.11+dfsg-0ubuntu0.14.04.17) trusty; urgency=medium
2
3 * d/p/bug_1583324_include_with_macro.patch: don't fail parsing the
4 config file if it has macros in include directives (LP: #1583324)
5
6 -- Andreas Hasenack <andreas@canonical.com> Thu, 02 Aug 2018 18:27:50 -0300
7
1samba (2:4.3.11+dfsg-0ubuntu0.14.04.16) trusty-security; urgency=medium8samba (2:4.3.11+dfsg-0ubuntu0.14.04.16) trusty-security; urgency=medium
29
3 * SECURITY UPDATE: Insufficient input validation on client directory10 * SECURITY UPDATE: Insufficient input validation on client directory
diff --git a/debian/patches/bug_1583324_include_with_macro.patch b/debian/patches/bug_1583324_include_with_macro.patch
4new file mode 10064411new file mode 100644
index 0000000..600070e
--- /dev/null
+++ b/debian/patches/bug_1583324_include_with_macro.patch
@@ -0,0 +1,65 @@
1From 3c6ea3293c6aac67bc442f47185fd494714e4806 Mon Sep 17 00:00:00 2001
2From: Quentin Gibeaux <qgibeaux@iris-tech.fr>
3Date: Thu, 29 Oct 2015 13:48:27 +0100
4Subject: [PATCH] lib/param: handle (ignore) substitution variable in smb.conf
5
6BUG: https://bugzilla.samba.org/show_bug.cgi?id=10722
7
8The function handle_include returns false when trying to include
9files that have a substitution variable in filename (like %U),
10this patch makes handle_include to ignore this case, to make
11samba-tool work when there is such include in samba's configuration.
12
13Error was :
14 root@ubuntu:/usr/local/samba# grep 'include.*%U' etc/smb.conf
15 include = %U.conf
16 root@ubuntu:/usr/local/samba# ./bin/samba-tool user list
17 Can't find include file %U.conf
18 ERROR(runtime): uncaught exception - Unable to load default file
19
20Signed-off-by: Quentin Gibeaux <qgibeaux@iris-tech.fr>
21Reviewed-by: Michael Adam <obnox@samba.org>
22Reviewed-by: Jeremy Allison <jra@samba.org>
23
24Autobuild-User(master): Jeremy Allison <jra@samba.org>
25Autobuild-Date(master): Wed Dec 9 02:05:30 CET 2015 on sn-devel-104
26Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/samba/+bug/1583324
27Last-Update: 2018-10-02
28---
29 lib/param/loadparm.c | 18 ++++++++++++++++++
30 1 file changed, 18 insertions(+)
31
32--- a/lib/param/loadparm.c
33+++ b/lib/param/loadparm.c
34@@ -1109,6 +1109,8 @@
35 const char *pszParmValue, char **ptr)
36 {
37 char *fname;
38+ const char *substitution_variable_substring;
39+ char next_char;
40
41 if (lp_ctx->s3_fns) {
42 return lp_ctx->s3_fns->lp_include(lp_ctx, service, pszParmValue, ptr);
43@@ -1123,6 +1125,22 @@
44 if (file_exist(fname))
45 return pm_process(fname, do_section, lpcfg_do_parameter, lp_ctx);
46
47+ /*
48+ * If the file doesn't exist, we check that it isn't due to variable
49+ * substitution
50+ */
51+ substitution_variable_substring = strchr(fname, '%');
52+
53+ if (substitution_variable_substring != NULL) {
54+ next_char = substitution_variable_substring[1];
55+ if ((next_char >= 'a' && next_char <= 'z')
56+ || (next_char >= 'A' && next_char <= 'Z')) {
57+ DEBUG(2, ("Tried to load %s but variable substitution in "
58+ "filename, ignoring file.\n", fname));
59+ return true;
60+ }
61+ }
62+
63 DEBUG(2, ("Can't find include file %s\n", fname));
64
65 return false;
diff --git a/debian/patches/series b/debian/patches/series
index f2ce2cb..9b1f74a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -103,3 +103,4 @@ CVE-2018-10919-7.patch
103CVE-2018-10919-8.patch103CVE-2018-10919-8.patch
104CVE-2018-10919-9.patch104CVE-2018-10919-9.patch
105CVE-2018-10919-10.patch105CVE-2018-10919-10.patch
106bug_1583324_include_with_macro.patch

Subscribers

People subscribed via source and target branches