The branch, v4-10-test has been updated via f86e09dcd48 VERSION: Bump version up to 4.10.11. via 9d50efa5183 Merge tag 'samba-4.10.10' into v4-10-test via e6f6df7658d VERSION: Disable GIT_SNAPSHOT for the 4.10.10 release. via bd0462b3062 WHATSNEW: Add release notes for Samba 4.10.10. via 489ef124459 CVE-2019-14847 dsdb: Correct behaviour of ranged_results when combined with dirsync via 6957ec76a5a CVE-2019-14847 dsdb: Demonstrate the correct interaction of ranged_results style attributes and dirsync via da8f00d1be8 CVE-2019-14847 dsdb/modules/dirsync: ensure attrs exist (CID 1107212) via d58567ee733 CVE-2019-14833 dsdb: send full password to check password script via 9c8e722fea1 CVE-2019-14833: Use utf8 characters in the unacceptable password via 284c98515b9 CVE-2019-10218 - s3: libsmb: Protect SMB2 client code from evil server returned names. via 13bc9d73657 CVE-2019-10218 - s3: libsmb: Protect SMB1 client code from evil server returned names. via 844c353ca5d VERSION: Bump version up to 4.10.10... from b19b75e324f VERSION: Bump version up to 4.10.10...
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-10-test - Log ----------------------------------------------------------------- commit f86e09dcd48fa837e15439fcc10eac1b8dec862b Author: Karolin Seeger <ksee...@samba.org> Date: Tue Oct 29 11:12:31 2019 +0100 VERSION: Bump version up to 4.10.11. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 9d50efa518371f3b6936d58b0bb1b23e3f65f40e Merge: b19b75e324f e6f6df7658d Author: Karolin Seeger <ksee...@samba.org> Date: Tue Oct 29 11:12:04 2019 +0100 Merge tag 'samba-4.10.10' into v4-10-test samba: tag release samba-4.10.10 ----------------------------------------------------------------------- Summary of changes: VERSION | 2 +- WHATSNEW.txt | 78 ++++++++++++++++++++++++- selftest/target/Samba4.pm | 2 +- source3/libsmb/cli_smb2_fnum.c | 7 +++ source3/libsmb/clilist.c | 75 ++++++++++++++++++++++++ source3/libsmb/proto.h | 3 + source4/dsdb/common/util.c | 33 +++++++++-- source4/dsdb/samdb/ldb_modules/dirsync.c | 15 +++-- source4/dsdb/samdb/ldb_modules/ranged_results.c | 25 +++++++- source4/dsdb/tests/python/dirsync.py | 26 +++++++++ 10 files changed, 248 insertions(+), 18 deletions(-) Changeset truncated at 500 lines: diff --git a/VERSION b/VERSION index c3257b43cfb..18b56dd9d87 100644 --- a/VERSION +++ b/VERSION @@ -25,7 +25,7 @@ ######################################################## SAMBA_VERSION_MAJOR=4 SAMBA_VERSION_MINOR=10 -SAMBA_VERSION_RELEASE=10 +SAMBA_VERSION_RELEASE=11 ######################################################## # If a official release has a serious bug # diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 085acc45245..83a4f3b994d 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,3 +1,77 @@ + =============================== + Release Notes for Samba 4.10.10 + 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.10.9: +--------------------- + +o Jeremy Allison <j...@samba.org> + * BUG 14071: CVE-2019-10218 - s3: libsmb: Protect SMB1 and SMB2 client code + from evil server returned names. + +o Andrew Bartlett <abart...@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 <b...@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.10.9 October 17, 2019 @@ -111,8 +185,8 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- + ============================== Release Notes for Samba 4.10.8 diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index b662776a847..c29e1c81ff8 100755 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -2168,7 +2168,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 = sed -e '/$unacceptable_password/{;q1}; /$unacceptable_password/!{q0}' allow dcerpc auth level connect:lsarpc = yes diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index 3a64438a5b9..7b6c7e87cd4 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -1026,6 +1026,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 @@ -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 b0cfcb5aa90..34349701104 100644 --- a/source3/libsmb/proto.h +++ b/source3/libsmb/proto.h @@ -723,6 +723,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 dd9a5dcadf5..0e81a4ec1dc 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 2a9895ae641..face6790754 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 c6a1df5ea43..e177bfbbfdc 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 @@ -588,6 +589,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) -- Samba Shared Repository