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

Proposed by Andreas Hasenack on 2018-08-03
Status: Merged
Approved by: Christian Ehrhardt  on 2018-08-07
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  2018-08-03 Approve on 2018-08-07
Canonical Server Team 2018-08-03 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.
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.

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 ...

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.

review: Needs Information
Christian Ehrhardt  (paelzer) wrote :

+1 after discussion in the related Xenial MP

review: Approve
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.

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

Subscribers

People subscribed via source and target branches