Merge ~ahasenack/ubuntu/+source/samba:samba-kerberos-method-crash-1761737 into ubuntu/+source/samba:ubuntu/devel

Proposed by Andreas Hasenack on 2018-04-19
Status: Merged
Merge reported by: Christian Ehrhardt 
Merged at revision: 940122ee6a50d190e6fc3d9f2da90369a10cb26b
Proposed branch: ~ahasenack/ubuntu/+source/samba:samba-kerberos-method-crash-1761737
Merge into: ubuntu/+source/samba:ubuntu/devel
Diff against target: 67 lines (+45/-0)
3 files modified
debian/changelog (+8/-0)
debian/patches/passdb_dont_return_ok_if_pinfo_not_filled.patch (+36/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  2018-04-19 Approve on 2018-04-24
Canonical Server Core Reviewers 2018-04-19 Pending
Review via email: mp+343614@code.launchpad.net

Commit message

  * debian/patches/passdb_dont_return_ok_if_pinfo_not_filled.patch:
    [PATCH] s3:passdb: Do not return OK if we don't have pinfo filled.
    Thanks to Andreas Schneider <email address hidden>. (LP: #1761737)

Description of the change

This fixes a crash in samba when "kerberos method" is set to "secrets and keytab" and the machine was not joined to an active directory domain with "net ads join".

To reproduce the crash:
sudo apt install samba smbclient
# create this /etc/samba/smb.conf:
[global]
 dns proxy = No
 domain master = No
 kerberos method = secrets and keytab
 local master = No
 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
 security = USER
 server role = standalone server
 server string = %h %a
 syslog = 0
 unix password sync = Yes
 usershare allow guests = Yes
 idmap config * : backend = tdb

# restart samba
sudo systemctl restart smbd nmbd

# try to get a share list. It will fail with an error, and a stack trace will be in the samba logs:
$ smbclient -L localhost -N
protocol negotiation failed: NT_STATUS_CONNECTION_DISCONNECTED

/var/log/samba/log.127.0.0.1 will contain a strack trace

PPA with fixed packages:
sudo add-apt-repository -y -u ppa:ahasenack/samba-kerberos-method-1761737

I haven't checked yet if artful and older are affected, the reporter says no. I also think only 4.7.x is affected, so that means bionic only.

To post a comment you must log in.
Christian Ehrhardt  (paelzer) wrote :

Patch review - ok
Format/Changelog - ok
Upgrading to new version - ok
Test the fixed issue - ok (verified trigger and fix via ppa)

Versions/Bug tracking - Given the timing you might need to change this to be an early SRU.
I checked with the release Team for another change this morning (but the same is true for all packages for a few days already - a.k.a since the MP is up).

TL;DR:
...
it would need to be a targeted fix in order to be accepted
if it's anything that looks non-trivial upon code inspection by a release team member, it'll likely get kicked back; you can always upload it and try, though
...

So we can go,as is today and rewrite it to be an SRU if rejected by the release Team.
A ping after the upload would be good thou.
Speaking of uploads please ping me if you need help sponsoring or want tags pushed.

review: Approve
Christian Ehrhardt  (paelzer) wrote :

After discussion on IRC pushed as-is - will be modified to be an SRU instead if requested by the release Team.

Tag creation seems affected by a new incarnation of bug 1687940.
Sponsored as discussed for now.

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 abc3310..bf75b4e 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+samba (2:4.7.6+dfsg~ubuntu-0ubuntu2) bionic; urgency=medium
7+
8+ * debian/patches/passdb_dont_return_ok_if_pinfo_not_filled.patch:
9+ [PATCH] s3:passdb: Do not return OK if we don't have pinfo filled.
10+ Thanks to Andreas Schneider <asn@samba.org>. (LP: #1761737)
11+
12+ -- Andreas Hasenack <andreas@canonical.com> Wed, 18 Apr 2018 11:49:55 -0300
13+
14 samba (2:4.7.6+dfsg~ubuntu-0ubuntu1) bionic; urgency=medium
15
16 * New upstream version:
17diff --git a/debian/patches/passdb_dont_return_ok_if_pinfo_not_filled.patch b/debian/patches/passdb_dont_return_ok_if_pinfo_not_filled.patch
18new file mode 100644
19index 0000000..d4ac1bf
20--- /dev/null
21+++ b/debian/patches/passdb_dont_return_ok_if_pinfo_not_filled.patch
22@@ -0,0 +1,36 @@
23+From 8fb23665ddad8f65a6461c310ed5680d104fd9bf Mon Sep 17 00:00:00 2001
24+From: Andreas Schneider <asn@samba.org>
25+Date: Tue, 17 Apr 2018 08:55:23 +0200
26+Subject: [PATCH] s3:passdb: Do not return OK if we don't have pinfo set up
27+
28+This prevents a crash in fill_mem_keytab_from_secrets()
29+
30+BUG: https://bugzilla.samba.org/show_bug.cgi?id=13376
31+
32+Signed-off-by: Andreas Schneider <asn@samba.org>
33+Reviewed-by: Stefan Metzmacher <metze@samba.org>
34+
35+(cherry picked from commit 99859479fc6e12b2f74ce2dfa83da56d8b8f3d26)
36+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/samba/+bug/1761737
37+Origin: https://attachments.samba.org/attachment.cgi?id=14159
38+Last-Update: 2018-04-19
39+---
40+ source3/passdb/machine_account_secrets.c | 2 +-
41+ 1 file changed, 1 insertion(+), 1 deletion(-)
42+
43+diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
44+index 75f31cb04e2..d36fa268a4b 100644
45+--- a/source3/passdb/machine_account_secrets.c
46++++ b/source3/passdb/machine_account_secrets.c
47+@@ -1317,7 +1317,7 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
48+
49+ last_set_time = secrets_fetch_pass_last_set_time(domain);
50+ if (last_set_time == 0) {
51+- return NT_STATUS_OK;
52++ return NT_STATUS_CANT_ACCESS_DOMAIN_INFO;
53+ }
54+ unix_to_nt_time(&last_set_nt, last_set_time);
55+
56+--
57+2.16.3
58+
59diff --git a/debian/patches/series b/debian/patches/series
60index 37e914d..1aa159c 100644
61--- a/debian/patches/series
62+++ b/debian/patches/series
63@@ -11,3 +11,4 @@ systemd-syslog.target-is-obsolete.patch
64 Add-documentation-to-systemd-Unit-files.patch
65 fix_kill_path_in_units.patch
66 nmbd-requires-a-working-network.patch
67+passdb_dont_return_ok_if_pinfo_not_filled.patch

Subscribers

People subscribed via source and target branches