debian/changelog

+samba (2:4.11.1+dfsg-2) unstable; urgency=high
+
+  * New upstream security release
+    - CVE-2019-10218: Malicious servers can cause Samba client code to return
+      filenames containing path separators to calling code.
+    - CVE-2019-14833: When the password contains multi-byte (non-ASCII)
+      characters, the check password script does not receive the full password
+      string.
+
+ -- Mathieu Parent <sathieu@debian.org>  Fri, 18 Oct 2019 20:26:45 +0200
+
 samba (2:4.11.1+dfsg-1) unstable; urgency=medium
 
   * New upstream release

debian/patches/CVE-2019-10218.master.patch

+From 024b18d663d5ab84fe0e9a0991708d89639471b8 Mon Sep 17 00:00:00 2001
+From: Jeremy Allison <jra@samba.org>
+Date: Mon, 5 Aug 2019 13:39:53 -0700
+Subject: [PATCH 1/2] CVE-2019-10218 - s3: libsmb: Protect SMB1 client code
+ from evil server returned names.
+
+Disconnect with NT_STATUS_INVALID_NETWORK_RESPONSE if so.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=14071
+
+Signed-off-by: Jeremy Allison <jra@samba.org>
+---
+ source3/libsmb/clilist.c | 75 ++++++++++++++++++++++++++++++++++++++++
+ source3/libsmb/proto.h   |  3 ++
+ 2 files changed, 78 insertions(+)
+
+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
+@@ -24,6 +24,66 @@
+ #include "trans2.h"
+ #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 6a647da58c8..48855d7112c 100644
+--- a/source3/libsmb/proto.h
++++ b/source3/libsmb/proto.h
+@@ -760,6 +760,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);
+-- 
+2.23.0.rc1.153.gdeed80330f-goog
+
+
+From bfa8a5991b15f69168587b88dc2d81c172f7617c Mon Sep 17 00:00:00 2001
+From: Jeremy Allison <jra@samba.org>
+Date: Tue, 6 Aug 2019 12:08:09 -0700
+Subject: [PATCH 2/2] CVE-2019-10218 - s3: libsmb: Protect SMB2 client code
+ from evil server returned names.
+
+Disconnect with NT_STATUS_INVALID_NETWORK_RESPONSE if so.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=14071
+
+Signed-off-by: Jeremy Allison <jra@samba.org>
+---
+ source3/libsmb/cli_smb2_fnum.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
+index 535beaab841..3fa322c243b 100644
+--- a/source3/libsmb/cli_smb2_fnum.c
++++ b/source3/libsmb/cli_smb2_fnum.c
+@@ -1442,6 +1442,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)) {
+ 				/*
+-- 
+2.23.0.rc1.153.gdeed80330f-goog
+

debian/patches/CVE-2019-14833-4.11-01.patch

+From 52f8d4b40c1a7b88ec5e1cf4752ffb2921e96383 Mon Sep 17 00:00:00 2001
+From: Andrew Bartlett <abartlet@samba.org>
+Date: Thu, 19 Sep 2019 11:50:01 +1200
+Subject: [PATCH 1/2] CVE-2019-14833: Use utf8 characters in the unacceptable
+ password
+
+This shows that the "check password script" handling has a bug.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12438
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/unacceptable-passwords | 1 +
+ selftest/target/Samba4.pm                   | 2 +-
+ 2 files changed, 2 insertions(+), 1 deletion(-)
+ create mode 100644 selftest/knownfail.d/unacceptable-passwords
+
+diff --git a/selftest/knownfail.d/unacceptable-passwords b/selftest/knownfail.d/unacceptable-passwords
+new file mode 100644
+index 00000000000..75fa2fc32b8
+--- /dev/null
++++ b/selftest/knownfail.d/unacceptable-passwords
+@@ -0,0 +1 @@
++^samba.tests.samba_tool.user_check_password_script.samba.tests.samba_tool.user_check_password_script.UserCheckPwdTestCase.test_checkpassword_unacceptable\(chgdcpass:local\)
+\ No newline at end of file
+diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
+index 02cdfc18bad..195e9b88044 100755
+--- a/selftest/target/Samba4.pm
++++ b/selftest/target/Samba4.pm
+@@ -1993,7 +1993,7 @@ sub provision_chgdcpass($$)
+ 	print "PROVISIONING CHGDCPASS...\n";
+ 	# 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";
+ 	my $extra_smb_conf = "
+ 	check password script = $self->{srcdir}/selftest/checkpassword_arg1.sh ${unacceptable_password}
+ 	allow dcerpc auth level connect:lsarpc = yes
+-- 
+2.17.1
+
+
+From d99d4d9c336fc8ad5a41af64cd963f548653c7da Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Bj=C3=B6rn=20Baumbach?= <bb@sernet.de>
+Date: Tue, 6 Aug 2019 16:32:32 +0200
+Subject: [PATCH 2/2] CVE-2019-14833 dsdb: send full password to check password
+ script
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+utf8_len represents the number of characters (not bytes) of the
+password. If the password includes multi-byte characters it is required
+to write the total number of bytes to the check password script.
+Otherwise the last bytes of the password string would be ignored.
+
+Therefore we rename utf8_len to be clear what it does and does
+not represent.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=12438
+
+Signed-off-by: Björn Baumbach <bb@sernet.de>
+Signed-off-by: Andrew Bartlett <abartlet@samba.org>
+---
+ selftest/knownfail.d/unacceptable-passwords |  1 -
+ source4/dsdb/common/util.c                  | 30 ++++++++++++++++-----
+ 2 files changed, 24 insertions(+), 7 deletions(-)
+ delete mode 100644 selftest/knownfail.d/unacceptable-passwords
+
+diff --git a/selftest/knownfail.d/unacceptable-passwords b/selftest/knownfail.d/unacceptable-passwords
+deleted file mode 100644
+index 75fa2fc32b8..00000000000
+--- a/selftest/knownfail.d/unacceptable-passwords
++++ /dev/null
+@@ -1 +0,0 @@
+-^samba.tests.samba_tool.user_check_password_script.samba.tests.samba_tool.user_check_password_script.UserCheckPwdTestCase.test_checkpassword_unacceptable\(chgdcpass:local\)
+\ No newline at end of file
+diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
+index e521ed09999..3ebec827404 100644
+--- a/source4/dsdb/common/util.c
++++ b/source4/dsdb/common/util.c
+@@ -2036,21 +2036,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;
+ 	}
+ 
+@@ -2058,6 +2073,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;
+ 		int cps_stdin = -1;
+@@ -2120,7 +2136,9 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
+ 
+ 		cps_stdin = samba_runcmd_export_stdin(req);
+ 
+-		if (write(cps_stdin, utf8_pw, utf8_len) != utf8_len) {
++		nwritten = write(cps_stdin, utf8_blob->data,
++				 utf8_blob->length);
++		if (nwritten != utf8_blob->length) {
+ 			close(cps_stdin);
+ 			cps_stdin = -1;
+ 			TALLOC_FREE(password_script);
+-- 
+2.17.1
+

debian/patches/series

@@ -11,3 +11,5 @@ fix-nfs-service-name-to-nfs-kernel-server.patch
 build-Remove-tests-for-getdents-and-getdirentries.patch
 wscript-remove-all-checks-for-_FUNC-and-__FUNC.patch
 wscript-split-function-check-to-one-per-line-and-sor.patch
+CVE-2019-10218.master.patch
+CVE-2019-14833-4.11-01.patch

selftest/target/Samba4.pm

@@ -1993,7 +1993,7 @@ sub provision_chgdcpass($$)
 	print "PROVISIONING CHGDCPASS...\n";
 	# 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";
 	my $extra_smb_conf = "
 	check password script = $self->{srcdir}/selftest/checkpassword_arg1.sh ${unacceptable_password}
 	allow dcerpc auth level connect:lsarpc = yes

source3/libsmb/cli_smb2_fnum.c

@@ -1442,6 +1442,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)) {
 				/*

source3/libsmb/clilist.c

@@ -24,6 +24,66 @@
 #include "trans2.h"
 #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 "

source3/libsmb/proto.h

@@ -760,6 +760,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);

source4/dsdb/common/util.c

@@ -2036,21 +2036,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;
 	}
 
@@ -2058,6 +2073,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;
 		int cps_stdin = -1;
@@ -2120,7 +2136,9 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 
 		cps_stdin = samba_runcmd_export_stdin(req);
 
-		if (write(cps_stdin, utf8_pw, utf8_len) != utf8_len) {
+		nwritten = write(cps_stdin, utf8_blob->data,
+				 utf8_blob->length);
+		if (nwritten != utf8_blob->length) {
 			close(cps_stdin);
 			cps_stdin = -1;
 			TALLOC_FREE(password_script);