CVE-2019-3870 tests: Extend smbd tests to check for umask being overwritten

The smbd changes the umask - if the code fails to restore the umask to
what it was, then this is very bad. Add an extra check to every
smbd-related test that the umask at the end of the test is the same as
what it was at the beginning (i.e. if the smbd code changed the umask
then it correctly restored the value afterwards).

As the selftest sets the umask for all tests to zero, it makes it hard
to detect this problem, so the test setUp() needs to set it to something
else first.

This extra checking is added to the setUp()/tearDown() so that it
applies to all test-cases. However, any failure that occur with this
approach will not be able to be known-failed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13834

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/python/samba/tests/ntacls_backup.py b/python/samba/tests/ntacls_backup.py
index 03ee821e595e9c5830afe1bebfb453982649bffb..b6689dcd515586dc2454b31d3090541b619c9d14 100644 (file)
--- a/python/samba/tests/ntacls_backup.py
+++ b/python/samba/tests/ntacls_backup.py
@@ -26,10 +26,11 @@ from samba import ntacls
 
 from samba.auth import system_session
 from samba.dcerpc import security
-from samba.tests import TestCaseInTempDir, env_loadparm
+from samba.tests import env_loadparm
+from samba.tests.smbd_base import SmbdBaseTests
 
 
-class NtaclsBackupRestoreTests(TestCaseInTempDir):
+class NtaclsBackupRestoreTests(SmbdBaseTests):
     """
     Tests for NTACLs backup and restore.
     """

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 65ca2c846f5c89c53030dcb828f36d8ea3b573a3..7e1fb3ec55e19fb754a48c8db744e135a089ee35 100644 (file)
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -20,7 +20,7 @@
 
 from samba.ntacls import setntacl, getntacl, checkset_backend
 from samba.dcerpc import security, smb_acl, idmap
-from samba.tests import TestCaseInTempDir
+from samba.tests.smbd_base import SmbdBaseTests
 from samba import provision
 import os
 from samba.samba3 import smbd, passdb
@@ -32,7 +32,7 @@ DOM_SID = "S-1-5-21-2212615479-2695158682-2101375467"
 ACL = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
 
 
-class PosixAclMappingTests(TestCaseInTempDir):
+class PosixAclMappingTests(SmbdBaseTests):
 
     def setUp(self):
         super(PosixAclMappingTests, self).setUp()

diff --git a/python/samba/tests/smbd_base.py b/python/samba/tests/smbd_base.py
new file mode 100644 (file)
index 0000000..4e5c364
--- /dev/null
+++ b/python/samba/tests/smbd_base.py
@@ -0,0 +1,48 @@
+# Unix SMB/CIFS implementation. Common code for smbd python bindings tests
+# Copyright (C) Catalyst.Net Ltd 2019
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+from samba.tests import TestCaseInTempDir
+import os
+
+TEST_UMASK = 0o022
+
+class SmbdBaseTests(TestCaseInTempDir):
+
+    def get_umask(self):
+        # we can only get the umask by setting it to something
+        curr_umask = os.umask(0)
+        # restore the old setting
+        os.umask(curr_umask)
+        return curr_umask
+
+    def setUp(self):
+        super(SmbdBaseTests, self).setUp()
+        self.orig_umask = self.get_umask()
+
+        # set an arbitrary umask - the underlying smbd code should override
+        # this, but it allows us to check if umask is left unset
+        os.umask(TEST_UMASK)
+
+    def tearDown(self):
+        # the current umask should be what we set it to earlier - if it's not,
+        # it indicates the code has changed it and not restored it
+        self.assertEqual(self.get_umask(), TEST_UMASK,
+                         "umask unexpectedly overridden by test")
+
+        # restore the original umask value (before we interferred with it)
+        os.umask(self.orig_umask)
+
+        super(SmbdBaseTests, self).tearDown()

diff --git a/selftest/knownfail.d/umask-leak b/selftest/knownfail.d/umask-leak
new file mode 100644 (file)
index 0000000..5580beb
--- /dev/null
+++ b/selftest/knownfail.d/umask-leak
@@ -0,0 +1,3 @@
+^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_smbd_create_file
+^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_backup_online
+^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_backup_offline