Merge tag 'samba-4.9.15' into v4-9-test

samba: tag release samba-4.9.15
Signed-off-by: Karolin Seeger <kseeger@samba.org>

9 files changed, 247 insertions, 17 deletions

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index cf34f50129f..377a1aa7c1e 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,78 @@
                    ==============================
+                   Release Notes for Samba 4.9.15
+                          October 29, 2019
+                   ==============================
+
+
+This is a security release in order to address the following defects:
+
+o CVE-2019-10218: Client code can return filenames containing path separators.
+o CVE-2019-14833: Samba AD DC check password script does not receive the full
+		  password.
+o CVE-2019-14847: User with "get changes" permission can crash AD DC LDAP server
+		  via dirsync.
+
+=======
+Details
+=======
+
+o  CVE-2019-10218:
+   Malicious servers can cause Samba client code to return filenames containing
+   path separators to calling code.
+
+o  CVE-2019-14833:
+   When the password contains multi-byte (non-ASCII) characters, the check
+   password script does not receive the full password string.
+
+o  CVE-2019-14847:
+   Users with the "get changes" extended access right can crash the AD DC LDAP
+   server by requesting an attribute using the range= syntax.
+
+For more details and workarounds, please refer to the security advisories.
+
+
+Changes since 4.9.14:
+---------------------
+
+o  Jeremy Allison <jra@samba.org>
+   * BUG 14071: CVE-2019-10218 - s3: libsmb: Protect SMB1 and SMB2 client code
+     from evil server returned names.
+
+o  Andrew Bartlett <abartlet@samba.org>
+   * BUG 12438: CVE-2019-14833: Use utf8 characters in the unacceptable
+     password.
+   * BUG 14040: CVE-2019-14847 dsdb: Correct behaviour of ranged_results when
+     combined with dirsync.
+
+o  Björn Baumbach <bb@sernet.de>
+   * BUG 12438: CVE-2019-14833 dsdb: Send full password to check password
+     script.
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the "Samba 4.1 and newer" product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
+                   ==============================
                    Release Notes for Samba 4.9.14
                           October 22, 2019
                    ==============================
@@ -77,8 +151,8 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
 
                    ==============================
                    Release Notes for Samba 4.9.13
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index b565d466477..d7c22ce4e23 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -1986,7 +1986,7 @@ sub provision_chgdcpass($$)
 	my $extra_provision_options = undef;
 	# This environment disallows the use of this password
 	# (and also removes the default AD complexity checks)
-	my $unacceptable_password = "widk3Dsle32jxdBdskldsk55klASKQ";
+	my $unacceptable_password = "Paßßword-widk3Dsle32jxdBdskldsk55klASKQ";
 	push (@{$extra_provision_options}, "--dns-backend=BIND9_DLZ");
 	my $ret = $self->provision($prefix,
 				   "domain controller",
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 1cfa50ffbac..3cdf68dc24b 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -1017,6 +1017,13 @@ NTSTATUS cli_smb2_list(struct cli_state *cli,
 				goto fail;
 			}
 
+			/* Protect against server attack. */
+			status = is_bad_finfo_name(cli, finfo);
+			if (!NT_STATUS_IS_OK(status)) {
+				smbXcli_conn_disconnect(cli->conn, status);
+				goto fail;
+			}
+
 			if (dir_check_ftype((uint32_t)finfo->mode,
 					(uint32_t)attribute)) {
 				/*
diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c
index 5cb1fce4338..4f518339e2b 100644
--- a/source3/libsmb/clilist.c
+++ b/source3/libsmb/clilist.c
@@ -25,6 +25,66 @@
 #include "../libcli/smb/smbXcli_base.h"
 
 /****************************************************************************
+ Check if a returned directory name is safe.
+****************************************************************************/
+
+static NTSTATUS is_bad_name(bool windows_names, const char *name)
+{
+	const char *bad_name_p = NULL;
+
+	bad_name_p = strchr(name, '/');
+	if (bad_name_p != NULL) {
+		/*
+		 * Windows and POSIX names can't have '/'.
+		 * Server is attacking us.
+		 */
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+	if (windows_names) {
+		bad_name_p = strchr(name, '\\');
+		if (bad_name_p != NULL) {
+			/*
+			 * Windows names can't have '\\'.
+			 * Server is attacking us.
+			 */
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+	}
+	return NT_STATUS_OK;
+}
+
+/****************************************************************************
+ Check if a returned directory name is safe. Disconnect if server is
+ sending bad names.
+****************************************************************************/
+
+NTSTATUS is_bad_finfo_name(const struct cli_state *cli,
+			const struct file_info *finfo)
+{
+	NTSTATUS status = NT_STATUS_OK;
+	bool windows_names = true;
+
+	if (cli->requested_posix_capabilities & CIFS_UNIX_POSIX_PATHNAMES_CAP) {
+		windows_names = false;
+	}
+	if (finfo->name != NULL) {
+		status = is_bad_name(windows_names, finfo->name);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_ERR("bad finfo->name\n");
+			return status;
+		}
+	}
+	if (finfo->short_name != NULL) {
+		status = is_bad_name(windows_names, finfo->short_name);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_ERR("bad finfo->short_name\n");
+			return status;
+		}
+	}
+	return NT_STATUS_OK;
+}
+
+/****************************************************************************
  Calculate a safe next_entry_offset.
 ****************************************************************************/
 
@@ -492,6 +552,13 @@ static NTSTATUS cli_list_old_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 			TALLOC_FREE(finfo);
 			return NT_STATUS_NO_MEMORY;
 		}
+
+		status = is_bad_finfo_name(state->cli, finfo);
+		if (!NT_STATUS_IS_OK(status)) {
+			smbXcli_conn_disconnect(state->cli->conn, status);
+			TALLOC_FREE(finfo);
+			return status;
+		}
 	}
 	*pfinfo = finfo;
 	return NT_STATUS_OK;
@@ -727,6 +794,14 @@ static void cli_list_trans_done(struct tevent_req *subreq)
 			ff_eos = true;
 			break;
 		}
+
+		status = is_bad_finfo_name(state->cli, finfo);
+		if (!NT_STATUS_IS_OK(status)) {
+			smbXcli_conn_disconnect(state->cli->conn, status);
+			tevent_req_nterror(req, status);
+			return;
+		}
+
 		if (!state->first && (state->mask[0] != '\0') &&
 		    strcsequal(finfo->name, state->mask)) {
 			DEBUG(1, ("Error: Looping in FIND_NEXT as name %s has "
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index 2bd61b1d2c2..e708e911b97 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -722,6 +722,9 @@ NTSTATUS cli_posix_whoami(struct cli_state *cli,
 
 /* The following definitions come from libsmb/clilist.c  */
 
+NTSTATUS is_bad_finfo_name(const struct cli_state *cli,
+			const struct file_info *finfo);
+
 NTSTATUS cli_list_old(struct cli_state *cli,const char *Mask,uint16_t attribute,
 		      NTSTATUS (*fn)(const char *, struct file_info *,
 				 const char *, void *), void *state);
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 18f700370a3..c7893bff43b 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -2088,21 +2088,36 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 						const uint32_t pwdProperties,
 						const uint32_t minPwdLength)
 {
-	const char *utf8_pw = (const char *)utf8_blob->data;
-	size_t utf8_len = strlen_m(utf8_pw);
 	char *password_script = NULL;
+	const char *utf8_pw = (const char *)utf8_blob->data;
+
+	/*
+	 * This looks strange because it is.
+	 *
+	 * The check for the number of characters in the password
+	 * should clearly not be against the byte length, or else a
+	 * single UTF8 character would count for more than one.
+	 *
+	 * We have chosen to use the number of 16-bit units that the
+	 * password encodes to as the measure of length.  This is not
+	 * the same as the number of codepoints, if a password
+	 * contains a character beyond the Basic Multilingual Plane
+	 * (above 65535) it will count for more than one "character".
+	 */
+
+	size_t password_characters_roughly = strlen_m(utf8_pw);
 
 	/* checks if the "minPwdLength" property is satisfied */
-	if (minPwdLength > utf8_len) {
+	if (minPwdLength > password_characters_roughly) {
 		return SAMR_VALIDATION_STATUS_PWD_TOO_SHORT;
 	}
 
-	/* checks the password complexity */
+	/* We might not be asked to check the password complexity */
 	if (!(pwdProperties & DOMAIN_PASSWORD_COMPLEX)) {
 		return SAMR_VALIDATION_STATUS_SUCCESS;
 	}
 
-	if (utf8_len == 0) {
+	if (password_characters_roughly == 0) {
 		return SAMR_VALIDATION_STATUS_NOT_COMPLEX_ENOUGH;
 	}
 
@@ -2110,6 +2125,7 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 	if (password_script != NULL && *password_script != '\0') {
 		int check_ret = 0;
 		int error = 0;
+		ssize_t nwritten = 0;
 		struct tevent_context *event_ctx = NULL;
 		struct tevent_req *req = NULL;
 		struct samba_runcmd_state *run_cmd = NULL;
@@ -2134,7 +2150,12 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 					tevent_timeval_current_ofs(10, 0),
 					100, 100, cmd, NULL);
 		run_cmd = tevent_req_data(req, struct samba_runcmd_state);
-		if (write(run_cmd->fd_stdin, utf8_pw, utf8_len) != utf8_len) {
+		nwritten = write(run_cmd->fd_stdin,
+				 utf8_blob->data,
+				 utf8_blob->length);
+		if (nwritten != utf8_blob->length) {
+			close(run_cmd->fd_stdin);
+			run_cmd->fd_stdin = -1;
 			TALLOC_FREE(password_script);
 			TALLOC_FREE(event_ctx);
 			return SAMR_VALIDATION_STATUS_PASSWORD_FILTER_ERROR;
diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
index b5510eccd24..4ac5faad403 100644
--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
@@ -343,6 +343,10 @@ skip:
 
 		attr = dsdb_attribute_by_lDAPDisplayName(dsc->schema,
 				el->name);
+		if (attr == NULL) {
+			continue;
+		}
+
 		keep = false;
 
 		if (attr->linkID & 1) {
@@ -994,7 +998,7 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
 	}
 
 	/*
-	 * check if there's an extended dn control
+	 * check if there's a dirsync control
 	 */
 	control = ldb_request_get_control(req, LDB_CONTROL_DIRSYNC_OID);
 	if (control == NULL) {
@@ -1323,11 +1327,12 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
 
 	}
 	/*
-	 * Remove our control from the list of controls
+	 * Mark dirsync control as uncritical (done)
+	 *
+	 * We need this so ranged_results knows how to behave with
+	 * dirsync
 	 */
-	if (!ldb_save_controls(control, req, NULL)) {
-		return ldb_operr(ldb);
-	}
+	control->critical = false;
 	dsc->schema = dsdb_get_schema(ldb, dsc);
 	/*
 	 * At the begining we make the hypothesis that we will return a complete
diff --git a/source4/dsdb/samdb/ldb_modules/ranged_results.c b/source4/dsdb/samdb/ldb_modules/ranged_results.c
index 13bf3a2d0a9..98438799997 100644
--- a/source4/dsdb/samdb/ldb_modules/ranged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/ranged_results.c
@@ -35,14 +35,14 @@
 struct rr_context {
 	struct ldb_module *module;
 	struct ldb_request *req;
+	bool dirsync_in_use;
 };
 
 static struct rr_context *rr_init_context(struct ldb_module *module,
 					  struct ldb_request *req)
 {
-	struct rr_context *ac;
-
-	ac = talloc_zero(req, struct rr_context);
+	struct ldb_control *dirsync_control = NULL;
+	struct rr_context *ac = talloc_zero(req, struct rr_context);
 	if (ac == NULL) {
 		ldb_set_errstring(ldb_module_get_ctx(module), "Out of Memory");
 		return NULL;
@@ -51,6 +51,16 @@ static struct rr_context *rr_init_context(struct ldb_module *module,
 	ac->module = module;
 	ac->req = req;
 
+	/*
+	 * check if there's a dirsync control (as there is an
+	 * interaction between these modules)
+	 */
+	dirsync_control = ldb_request_get_control(req,
+						  LDB_CONTROL_DIRSYNC_OID);
+	if (dirsync_control != NULL) {
+		ac->dirsync_in_use = true;
+	}
+
 	return ac;
 }
 
@@ -82,6 +92,15 @@ static int rr_search_callback(struct ldb_request *req, struct ldb_reply *ares)
 					ares->response, ares->error);
 	}
 
+	if (ac->dirsync_in_use) {
+		/*
+		 * We return full attribute values when mixed with
+		 * dirsync
+		 */
+		return ldb_module_send_entry(ac->req,
+					     ares->message,
+					     ares->controls);
+	}
 	/* LDB_REPLY_ENTRY */
 
 	temp_ctx = talloc_new(ac->req);
diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py
index 136f4d3bba6..b6f7022a50b 100755
--- a/source4/dsdb/tests/python/dirsync.py
+++ b/source4/dsdb/tests/python/dirsync.py
@@ -28,6 +28,7 @@ from samba.tests.subunitrun import TestProgram, SubunitOptions
 import samba.getopt as options
 import base64
 
+import ldb
 from ldb import LdbError, SCOPE_BASE
 from ldb import Message, MessageElement, Dn
 from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE
@@ -590,6 +591,31 @@ class SimpleDirsyncTests(DirsyncBaseTests):
 
 class ExtendedDirsyncTests(SimpleDirsyncTests):
 
+    def test_dirsync_linkedattributes_range(self):
+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
+        res = self.ldb_admin.search(self.base_dn,
+                                    attrs=["member;range=1-1"],
+                                    expression="(name=Administrators)",
+                                    controls=["dirsync:1:0:0"])
+
+        self.assertTrue(len(res) > 0)
+        self.assertTrue(res[0].get("member;range=1-1") is None)
+        self.assertTrue(res[0].get("member") is not None)
+        self.assertTrue(len(res[0].get("member")) > 0)
+
+    def test_dirsync_linkedattributes_range_user(self):
+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
+        try:
+            res = self.ldb_simple.search(self.base_dn,
+                                         attrs=["member;range=1-1"],
+                                         expression="(name=Administrators)",
+                                        controls=["dirsync:1:0:0"])
+        except LdbError as e:
+            (num, _) = e.args
+            self.assertEquals(num, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS)
+        else:
+            self.fail()
+
     def test_dirsync_linkedattributes(self):
         flag_incr_linked = 2147483648
         self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)