From b708ce3f1ac863dea8051b51b717bced2a433546 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Fri, 15 Mar 2019 15:20:21 +1300
Subject: [PATCH 1/6] 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>

(This backport to Samba 4.9 by Andrew Bartlett was not a pure
cherry-pick due to merge conflicts)
---
 python/samba/tests/ntacls_backup.py |  4 +--
 python/samba/tests/posixacl.py      |  4 +--
 python/samba/tests/smbd_base.py     | 48 +++++++++++++++++++++++++++++
 selftest/knownfail.d/umask-leak     |  3 ++
 4 files changed, 55 insertions(+), 4 deletions(-)
 create mode 100644 python/samba/tests/smbd_base.py
 create mode 100644 selftest/knownfail.d/umask-leak

diff --git a/python/samba/tests/ntacls_backup.py b/python/samba/tests/ntacls_backup.py
index 9ab264a27fd..763804fd63f 100644
--- a/python/samba/tests/ntacls_backup.py
+++ b/python/samba/tests/ntacls_backup.py
@@ -27,10 +27,10 @@ from samba import ntacls
 from samba.auth import system_session
 from samba.param import LoadParm
 from samba.dcerpc import security
-from samba.tests import TestCaseInTempDir
+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 8b48825fc6f..2005f4eef59 100644
--- 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
index 00000000000..4e5c3641e2c
--- /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
index 00000000000..5580beb4b68
--- /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
-- 
2.17.1


From 83cc536a42003bf2df0a5a121b07df33c1ffd96a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Fri, 15 Mar 2019 13:52:50 +1300
Subject: [PATCH 2/6] CVE-2019-3870 tests: Add test to check file-permissions
 are correct after provision

This provisions a new DC and checks there are no world-writable
files in the new DC's private directory.

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>
---
 selftest/knownfail.d/provision_fileperms   |  1 +
 source4/selftest/tests.py                  |  1 +
 source4/setup/tests/provision_fileperms.sh | 71 ++++++++++++++++++++++
 3 files changed, 73 insertions(+)
 create mode 100644 selftest/knownfail.d/provision_fileperms
 create mode 100755 source4/setup/tests/provision_fileperms.sh

diff --git a/selftest/knownfail.d/provision_fileperms b/selftest/knownfail.d/provision_fileperms
new file mode 100644
index 00000000000..88b1585fd19
--- /dev/null
+++ b/selftest/knownfail.d/provision_fileperms
@@ -0,0 +1 @@
+samba4.blackbox.provision_fileperms.provision-fileperms\(none\)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 18b2c1162b0..d6fb388dc33 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -904,6 +904,7 @@ plantestsuite_loadlist("samba4.deletetest.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [
 plantestsuite("samba4.blackbox.samba3dump", "none", [os.path.join(samba4srcdir, "selftest/test_samba3dump.sh")])
 plantestsuite("samba4.blackbox.upgrade", "none", ["PYTHON=%s" % python, os.path.join(samba4srcdir, "setup/tests/blackbox_s3upgrade.sh"), '$PREFIX/provision'])
 plantestsuite("samba4.blackbox.provision.py", "none", ["PYTHON=%s" % python, os.path.join(samba4srcdir, "setup/tests/blackbox_provision.sh"), '$PREFIX/provision'])
+plantestsuite("samba4.blackbox.provision_fileperms", "none", ["PYTHON=%s" % python, os.path.join(samba4srcdir, "setup/tests/provision_fileperms.sh"), '$PREFIX/provision'])
 plantestsuite("samba4.blackbox.supported_features", "none",
               ["PYTHON=%s" % python,
                os.path.join(samba4srcdir,
diff --git a/source4/setup/tests/provision_fileperms.sh b/source4/setup/tests/provision_fileperms.sh
new file mode 100755
index 00000000000..0b3ef0321fb
--- /dev/null
+++ b/source4/setup/tests/provision_fileperms.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+if [ $# -lt 1 ]; then
+cat <<EOF
+Usage: $0 PREFIX
+EOF
+exit 1;
+fi
+
+PREFIX="$1"
+shift 1
+
+. `dirname $0`/../../../testprogs/blackbox/subunit.sh
+
+# selftest sets the umask to zero. Explicitly set it to 022 here,
+# which should mean files should never be writable for anyone else
+ORIG_UMASK=`umask`
+umask 0022
+
+# checks that the files in the 'private' directory created are not
+# world-writable
+check_private_file_perms()
+{
+    target_dir="$1/private"
+    result=0
+
+    for file in `ls $target_dir/`
+    do
+        filepath="$target_dir/$file"
+
+        # skip directories/sockets for now
+        if [ ! -f $filepath ] ; then
+            continue;
+        fi
+
+        # use stat to get the file permissions, i.e. -rw-------
+        file_perm=`stat -c "%A" $filepath`
+
+        # then use cut to drop the first 4 chars containing the file type
+        # and owner permissions. What's left is the group and other users
+        global_perm=`echo $file_perm | cut -c4-`
+
+        # check the remainder doesn't have write permissions set
+        if [ -z "${global_perm##*w*}" ] ; then
+            echo "Error: $file has $file_perm permissions"
+            result=1
+        fi
+    done
+    return $result
+}
+
+TARGET_DIR=$PREFIX/basic-dc
+rm -rf $TARGET_DIR
+
+# create a dummy smb.conf - we need to use fake ACLs for the file system here
+# (but passing --option args with spaces in it proved too difficult in bash)
+SMB_CONF=$TARGET_DIR/tmp/smb.conf
+mkdir -p `dirname $SMB_CONF`
+echo "vfs objects = fake_acls xattr_tdb" > $SMB_CONF
+
+# provision a basic DC
+testit "basic-provision" $PYTHON $BINDIR/samba-tool domain provision --server-role="dc" --domain=FOO --realm=foo.example.com --targetdir=$TARGET_DIR --configfile=$SMB_CONF
+
+# check the file permissions in the 'private' directory really are private
+testit "provision-fileperms" check_private_file_perms $TARGET_DIR
+
+rm -rf $TARGET_DIR
+
+umask $ORIG_UMASK
+
+exit $failed
-- 
2.17.1


From 65a175aac08bc69eaaf6b4e011eb59b262e3417b Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 21 Mar 2019 17:21:58 +1300
Subject: [PATCH 3/6] CVE-2019-3870 pysmbd: Include tests to show the outside
 umask has no impact

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---
 python/samba/tests/ntacls_backup.py | 13 +++++++++++++
 python/samba/tests/smbd_base.py     |  2 +-
 selftest/knownfail.d/pymkdir-umask  |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 selftest/knownfail.d/pymkdir-umask

diff --git a/python/samba/tests/ntacls_backup.py b/python/samba/tests/ntacls_backup.py
index 763804fd63f..b7defd35903 100644
--- a/python/samba/tests/ntacls_backup.py
+++ b/python/samba/tests/ntacls_backup.py
@@ -112,6 +112,12 @@ class NtaclsBackupRestoreTests(SmbdBaseTests):
 
         dirpath = os.path.join(self.service_root, 'a-dir')
         smbd.mkdir(dirpath, self.service)
+        mode = os.stat(dirpath).st_mode
+
+        # This works in conjunction with the TEST_UMASK in smbd_base
+        # to ensure that permissions are not related to the umask
+        # but instead the smb.conf settings
+        self.assertEquals(mode & 0o777, 0o755)
         self.assertTrue(os.path.isdir(dirpath))
 
     def test_smbd_create_file(self):
@@ -123,6 +129,13 @@ class NtaclsBackupRestoreTests(SmbdBaseTests):
         smbd.create_file(filepath, self.service)
         self.assertTrue(os.path.isfile(filepath))
 
+        mode = os.stat(filepath).st_mode
+
+        # This works in conjunction with the TEST_UMASK in smbd_base
+        # to ensure that permissions are not related to the umask
+        # but instead the smb.conf settings
+        self.assertEquals(mode & 0o777, 0o644)
+
         # As well as checking that unlink works, this removes the
         # fake xattrs from the dev/inode based DB
         smbd.unlink(filepath, self.service)
diff --git a/python/samba/tests/smbd_base.py b/python/samba/tests/smbd_base.py
index 4e5c3641e2c..b49bcc0828f 100644
--- a/python/samba/tests/smbd_base.py
+++ b/python/samba/tests/smbd_base.py
@@ -17,7 +17,7 @@
 from samba.tests import TestCaseInTempDir
 import os
 
-TEST_UMASK = 0o022
+TEST_UMASK = 0o042
 
 class SmbdBaseTests(TestCaseInTempDir):
 
diff --git a/selftest/knownfail.d/pymkdir-umask b/selftest/knownfail.d/pymkdir-umask
new file mode 100644
index 00000000000..5af01be44e3
--- /dev/null
+++ b/selftest/knownfail.d/pymkdir-umask
@@ -0,0 +1 @@
+^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_smbd_mkdir
\ No newline at end of file
-- 
2.17.1


From 30db48655f7aae97586d9143b0c0e00308392115 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 14 Mar 2019 18:20:06 +1300
Subject: [PATCH 4/6] CVE-2019-3870 pysmbd: Move umask manipuations as close as
 possible to users

Umask manipulation was added to pysmbd with e146fe5ef96c1522175a8e81db15d1e8879e5652 in 2012
and init_files_struct was split out in 747c3f1fb379bb68cc7479501b85741493c05812 in 2018 for
Samba 4.9. (It was added to assist the smbd.create_file() routine used in the backup and
restore tools, which needed to write files with full metadata).

This in turn avoids leaving init_files_struct() without resetting the umask to
the original, saved, value.

Per umask(2) this is required before open() and mkdir() system calls (along
side other file-like things such as those for Unix domain socks and FIFOs etc).

Therefore for safety and clarify the additional 'belt and braces' umask
manipuations elsewhere are removed.

mkdir() will be protected by a umask() bracket, for correctness, in the next patch.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>

(This backport to Samba 4.9 by Andrew Bartlett is not a pure
cherry-pick due to merge conflicts)
---
 selftest/knownfail.d/provision_fileperms |  1 -
 selftest/knownfail.d/umask-leak          |  3 ---
 source3/smbd/pysmbd.c                    | 34 +++++++-----------------
 3 files changed, 10 insertions(+), 28 deletions(-)
 delete mode 100644 selftest/knownfail.d/provision_fileperms
 delete mode 100644 selftest/knownfail.d/umask-leak

diff --git a/selftest/knownfail.d/provision_fileperms b/selftest/knownfail.d/provision_fileperms
deleted file mode 100644
index 88b1585fd19..00000000000
--- a/selftest/knownfail.d/provision_fileperms
+++ /dev/null
@@ -1 +0,0 @@
-samba4.blackbox.provision_fileperms.provision-fileperms\(none\)
diff --git a/selftest/knownfail.d/umask-leak b/selftest/knownfail.d/umask-leak
deleted file mode 100644
index 5580beb4b68..00000000000
--- a/selftest/knownfail.d/umask-leak
+++ /dev/null
@@ -1,3 +0,0 @@
-^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
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 1431925efd0..179a1ee2943 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -85,27 +85,19 @@ static int set_sys_acl_conn(const char *fname,
 {
 	int ret;
 	struct smb_filename *smb_fname = NULL;
-	mode_t saved_umask;
 
 	TALLOC_CTX *frame = talloc_stackframe();
 
-	/* we want total control over the permissions on created files,
-	   so set our umask to 0 */
-	saved_umask = umask(0);
-
 	smb_fname = synthetic_smb_fname_split(frame,
 					fname,
 					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		TALLOC_FREE(frame);
-		umask(saved_umask);
 		return -1;
 	}
 
 	ret = SMB_VFS_SYS_ACL_SET_FILE( conn, smb_fname, acltype, theacl);
 
-	umask(saved_umask);
-
 	TALLOC_FREE(frame);
 	return ret;
 }
@@ -132,22 +124,26 @@ static NTSTATUS init_files_struct(TALLOC_CTX *mem_ctx,
 	}
 	fsp->conn = conn;
 
-	/* we want total control over the permissions on created files,
-	   so set our umask to 0 */
-	saved_umask = umask(0);
-
 	smb_fname = synthetic_smb_fname_split(fsp,
 					      fname,
 					      lp_posix_pathnames());
 	if (smb_fname == NULL) {
-		umask(saved_umask);
 		return NT_STATUS_NO_MEMORY;
 	}
 
 	fsp->fsp_name = smb_fname;
+
+	/*
+	 * we want total control over the permissions on created files,
+	 * so set our umask to 0 (this matters if flags contains O_CREAT)
+	 */
+	saved_umask = umask(0);
+
 	fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, 00644);
+
+	umask(saved_umask);
+
 	if (fsp->fh->fd == -1) {
-		umask(saved_umask);
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
@@ -157,7 +153,6 @@ static NTSTATUS init_files_struct(TALLOC_CTX *mem_ctx,
 		DEBUG(0,("Error doing fstat on open file %s (%s)\n",
 			 smb_fname_str_dbg(smb_fname),
 			 strerror(errno) ));
-		umask(saved_umask);
 		return map_nt_error_from_unix(errno);
 	}
 
@@ -444,7 +439,6 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 	char *fname, *service = NULL;
 	int uid, gid;
 	TALLOC_CTX *frame;
-	mode_t saved_umask;
 	struct smb_filename *smb_fname = NULL;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "sii|z",
@@ -460,10 +454,6 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 		return NULL;
 	}
 
-	/* we want total control over the permissions on created files,
-	   so set our umask to 0 */
-	saved_umask = umask(0);
-
 	smb_fname = synthetic_smb_fname(talloc_tos(),
 					fname,
 					NULL,
@@ -471,7 +461,6 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 					lp_posix_pathnames() ?
 						SMB_FILENAME_POSIX_PATH : 0);
 	if (smb_fname == NULL) {
-		umask(saved_umask);
 		TALLOC_FREE(frame);
 		errno = ENOMEM;
 		return PyErr_SetFromErrno(PyExc_OSError);
@@ -479,14 +468,11 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 
 	ret = SMB_VFS_CHOWN(conn, smb_fname, uid, gid);
 	if (ret != 0) {
-		umask(saved_umask);
 		TALLOC_FREE(frame);
 		errno = ret;
 		return PyErr_SetFromErrno(PyExc_OSError);
 	}
 
-	umask(saved_umask);
-
 	TALLOC_FREE(frame);
 
 	Py_RETURN_NONE;
-- 
2.17.1


From c92ac5ada094a2f3f10f15b65d6ba5c771261acd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 21 Mar 2019 17:24:14 +1300
Subject: [PATCH 5/6] CVE-2019-3870 pysmbd: Ensure a zero umask is set for
 smbd.mkdir()

mkdir() is the other call that requires a umask of 0 in Samba.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---
 selftest/knownfail.d/pymkdir-umask |  1 -
 source3/smbd/pysmbd.c              | 11 ++++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)
 delete mode 100644 selftest/knownfail.d/pymkdir-umask

diff --git a/selftest/knownfail.d/pymkdir-umask b/selftest/knownfail.d/pymkdir-umask
deleted file mode 100644
index 5af01be44e3..00000000000
--- a/selftest/knownfail.d/pymkdir-umask
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.ntacls_backup.samba.tests.ntacls_backup.NtaclsBackupRestoreTests.test_smbd_mkdir
\ No newline at end of file
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 179a1ee2943..845ea25f936 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -739,6 +739,8 @@ static PyObject *py_smbd_mkdir(PyObject *self, PyObject *args, PyObject *kwargs)
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct connection_struct *conn = NULL;
 	struct smb_filename *smb_fname = NULL;
+	int ret;
+	mode_t saved_umask;
 
 	if (!PyArg_ParseTupleAndKeywords(args,
 					 kwargs,
@@ -769,8 +771,15 @@ static PyObject *py_smbd_mkdir(PyObject *self, PyObject *args, PyObject *kwargs)
 		return NULL;
 	}
 
+	/* we want total control over the permissions on created files,
+	   so set our umask to 0 */
+	saved_umask = umask(0);
+
+	ret = SMB_VFS_MKDIR(conn, smb_fname, 00755);
 
-	if (SMB_VFS_MKDIR(conn, smb_fname, 00755) == -1) {
+	umask(saved_umask);
+
+	if (ret == -1) {
 		DBG_ERR("mkdir error=%d (%s)\n", errno, strerror(errno));
 		TALLOC_FREE(frame);
 		return NULL;
-- 
2.17.1


From d53121af8028bb39c1d61e0f5c26ae1d30ab6351 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Thu, 21 Mar 2019 14:51:30 -0700
Subject: [PATCH 6/6] CVE-2019-3880 s3: rpc: winreg: Remove implementations of
 SaveKey/RestoreKey.

The were not using VFS backend calls and could only work
locally, and were unsafe against symlink races and other
security issues.

If the incoming handle is valid, return WERR_BAD_PATHNAME.

[MS-RRP] states "The format of the file name is implementation-specific"
so ensure we don't allow this.

As reported by Michael Hanselmann.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 source3/rpc_server/winreg/srv_winreg_nt.c | 92 +----------------------
 1 file changed, 4 insertions(+), 88 deletions(-)

diff --git a/source3/rpc_server/winreg/srv_winreg_nt.c b/source3/rpc_server/winreg/srv_winreg_nt.c
index d9ee8d0602d..816c6bb2a12 100644
--- a/source3/rpc_server/winreg/srv_winreg_nt.c
+++ b/source3/rpc_server/winreg/srv_winreg_nt.c
@@ -639,46 +639,6 @@ WERROR _winreg_AbortSystemShutdown(struct pipes_struct *p,
 	return (ret == 0) ? WERR_OK : WERR_ACCESS_DENIED;
 }
 
-/*******************************************************************
- ********************************************************************/
-
-static int validate_reg_filename(TALLOC_CTX *ctx, char **pp_fname )
-{
-	char *p = NULL;
-	int num_services = lp_numservices();
-	int snum = -1;
-	const char *share_path = NULL;
-	char *fname = *pp_fname;
-
-	/* convert to a unix path, stripping the C:\ along the way */
-
-	if (!(p = valid_share_pathname(ctx, fname))) {
-		return -1;
-	}
-
-	/* has to exist within a valid file share */
-
-	for (snum=0; snum<num_services; snum++) {
-		if (!lp_snum_ok(snum) || lp_printable(snum)) {
-			continue;
-		}
-
-		share_path = lp_path(talloc_tos(), snum);
-
-		/* make sure we have a path (e.g. [homes] ) */
-		if (strlen(share_path) == 0) {
-			continue;
-		}
-
-		if (strncmp(share_path, p, strlen(share_path)) == 0) {
-			break;
-		}
-	}
-
-	*pp_fname = p;
-	return (snum < num_services) ? snum : -1;
-}
-
 /*******************************************************************
  _winreg_RestoreKey
  ********************************************************************/
@@ -687,36 +647,11 @@ WERROR _winreg_RestoreKey(struct pipes_struct *p,
 			  struct winreg_RestoreKey *r)
 {
 	struct registry_key *regkey = find_regkey_by_hnd( p, r->in.handle );
-	char *fname = NULL;
-	int snum = -1;
 
-	if ( !regkey )
+	if ( !regkey ) {
 		return WERR_INVALID_HANDLE;
-
-	if ( !r->in.filename || !r->in.filename->name )
-		return WERR_INVALID_PARAMETER;
-
-	fname = talloc_strdup(p->mem_ctx, r->in.filename->name);
-	if (!fname) {
-		return WERR_NOT_ENOUGH_MEMORY;
 	}
-
-	DEBUG(8,("_winreg_RestoreKey: verifying restore of key [%s] from "
-		 "\"%s\"\n", regkey->key->name, fname));
-
-	if ((snum = validate_reg_filename(p->mem_ctx, &fname)) == -1)
-		return WERR_BAD_PATHNAME;
-
-	/* user must posses SeRestorePrivilege for this this proceed */
-
-	if ( !security_token_has_privilege(p->session_info->security_token, SEC_PRIV_RESTORE)) {
-		return WERR_ACCESS_DENIED;
-	}
-
-	DEBUG(2,("_winreg_RestoreKey: Restoring [%s] from %s in share %s\n",
-		 regkey->key->name, fname, lp_servicename(talloc_tos(), snum) ));
-
-	return reg_restorekey(regkey, fname);
+	return WERR_BAD_PATHNAME;
 }
 
 /*******************************************************************
@@ -727,30 +662,11 @@ WERROR _winreg_SaveKey(struct pipes_struct *p,
 		       struct winreg_SaveKey *r)
 {
 	struct registry_key *regkey = find_regkey_by_hnd( p, r->in.handle );
-	char *fname = NULL;
-	int snum = -1;
 
-	if ( !regkey )
+	if ( !regkey ) {
 		return WERR_INVALID_HANDLE;
-
-	if ( !r->in.filename || !r->in.filename->name )
-		return WERR_INVALID_PARAMETER;
-
-	fname = talloc_strdup(p->mem_ctx, r->in.filename->name);
-	if (!fname) {
-		return WERR_NOT_ENOUGH_MEMORY;
 	}
-
-	DEBUG(8,("_winreg_SaveKey: verifying backup of key [%s] to \"%s\"\n",
-		 regkey->key->name, fname));
-
-	if ((snum = validate_reg_filename(p->mem_ctx, &fname)) == -1 )
-		return WERR_BAD_PATHNAME;
-
-	DEBUG(2,("_winreg_SaveKey: Saving [%s] to %s in share %s\n",
-		 regkey->key->name, fname, lp_servicename(talloc_tos(), snum) ));
-
-	return reg_savekey(regkey, fname);
+	return WERR_BAD_PATHNAME;
 }
 
 /*******************************************************************
-- 
2.17.1