From a043d40dff99bbb66e18c18b2d0af569b8178350 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett 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 --- 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 0000000..75fa2fc --- /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 b662776..c29e1c8 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 -- 1.9.1 From c88e40efea11038377e4b6748598804c833f2a0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Baumbach?= 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 Signed-off-by: Andrew Bartlett --- selftest/knownfail.d/unacceptable-passwords | 1 - source4/dsdb/common/util.c | 33 +++++++++++++++++++++++------ 2 files changed, 27 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 75fa2fc..0000000 --- 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 dd9a5dc..0e81a4e 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; -- 1.9.1