update-4.11.6

This commit is contained in:
z00507040 2020-02-17 16:29:33 +08:00
parent b48e650436
commit 97d716cd2b
60 changed files with 2142 additions and 7161 deletions

View File

@ -0,0 +1,371 @@
From 21073bff847fbc41d3dab0a649fa400d8188fa16 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Sat, 19 Oct 2019 23:48:19 +0300
Subject: [PATCH 1/2] smbdes: add des_crypt56_gnutls() using use DES-CBC with
zeroed IV
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
---
libcli/auth/smbdes.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/libcli/auth/smbdes.c b/libcli/auth/smbdes.c
index 6d9a6dc2ce8..37ede91ad22 100644
--- a/libcli/auth/smbdes.c
+++ b/libcli/auth/smbdes.c
@@ -23,6 +23,9 @@
#include "includes.h"
#include "libcli/auth/libcli_auth.h"
+#include <gnutls/gnutls.h>
+#include <gnutls/crypto.h>
+
/* NOTES:
This code makes no attempt to be fast! In fact, it is a very
@@ -273,6 +276,50 @@ static void str_to_key(const uint8_t *str,uint8_t *key)
}
}
+static int des_crypt56_gnutls(uint8_t out[8], const uint8_t in[8],
+ const uint8_t key_in[7], bool enc)
+{
+ static uint8_t iv8[8];
+ gnutls_datum_t iv = { iv8, 8 };
+ gnutls_datum_t key;
+ gnutls_cipher_hd_t ctx;
+ uint8_t key2[8];
+ uint8_t outb[8];
+ int ret;
+
+ memset(out, 0, 8);
+
+ str_to_key(key_in, key2);
+
+ key.data = key2;
+ key.size = 8;
+
+ ret = gnutls_global_init();
+ if (ret != 0) {
+ return ret;
+ }
+
+ ret = gnutls_cipher_init(&ctx, GNUTLS_CIPHER_DES_CBC, &key, &iv);
+ if (ret != 0) {
+ return ret;
+ }
+
+ memcpy(outb, in, 8);
+ if (enc) {
+ ret = gnutls_cipher_encrypt(ctx, outb, 8);
+ } else {
+ ret = gnutls_cipher_decrypt(ctx, outb, 8);
+ }
+
+ if (ret == 0) {
+ memcpy(out, outb, 8);
+ }
+
+ gnutls_cipher_deinit(ctx);
+
+ return ret;
+}
+
/*
basic des crypt using a 56 bit (7 byte) key
*/
--
2.22.0
From 6d6651213f391840e3004ec3b055f8f25be9b360 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Mon, 21 Oct 2019 20:03:04 +0300
Subject: [PATCH 2/2] smbdes: use the new des_crypt56_gnutls()
and remove builtin DES crypto.
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
---
libcli/auth/smbdes.c | 258 +------------------------------------------
1 file changed, 1 insertion(+), 257 deletions(-)
diff --git a/libcli/auth/smbdes.c b/libcli/auth/smbdes.c
index 37ede91ad22..7de05b75303 100644
--- a/libcli/auth/smbdes.c
+++ b/libcli/auth/smbdes.c
@@ -26,239 +26,6 @@
#include <gnutls/gnutls.h>
#include <gnutls/crypto.h>
-/* NOTES:
-
- This code makes no attempt to be fast! In fact, it is a very
- slow implementation
-
- This code is NOT a complete DES implementation. It implements only
- the minimum necessary for SMB authentication, as used by all SMB
- products (including every copy of Microsoft Windows95 ever sold)
-
- In particular, it can only do a unchained forward DES pass. This
- means it is not possible to use this code for encryption/decryption
- of data, instead it is only useful as a "hash" algorithm.
-
- There is no entry point into this code that allows normal DES operation.
-
- I believe this means that this code does not come under ITAR
- regulations but this is NOT a legal opinion. If you are concerned
- about the applicability of ITAR regulations to this code then you
- should confirm it for yourself (and maybe let me know if you come
- up with a different answer to the one above)
-*/
-
-
-static const uint8_t perm1[56] = {57, 49, 41, 33, 25, 17, 9,
- 1, 58, 50, 42, 34, 26, 18,
- 10, 2, 59, 51, 43, 35, 27,
- 19, 11, 3, 60, 52, 44, 36,
- 63, 55, 47, 39, 31, 23, 15,
- 7, 62, 54, 46, 38, 30, 22,
- 14, 6, 61, 53, 45, 37, 29,
- 21, 13, 5, 28, 20, 12, 4};
-
-static const uint8_t perm2[48] = {14, 17, 11, 24, 1, 5,
- 3, 28, 15, 6, 21, 10,
- 23, 19, 12, 4, 26, 8,
- 16, 7, 27, 20, 13, 2,
- 41, 52, 31, 37, 47, 55,
- 30, 40, 51, 45, 33, 48,
- 44, 49, 39, 56, 34, 53,
- 46, 42, 50, 36, 29, 32};
-
-static const uint8_t perm3[64] = {58, 50, 42, 34, 26, 18, 10, 2,
- 60, 52, 44, 36, 28, 20, 12, 4,
- 62, 54, 46, 38, 30, 22, 14, 6,
- 64, 56, 48, 40, 32, 24, 16, 8,
- 57, 49, 41, 33, 25, 17, 9, 1,
- 59, 51, 43, 35, 27, 19, 11, 3,
- 61, 53, 45, 37, 29, 21, 13, 5,
- 63, 55, 47, 39, 31, 23, 15, 7};
-
-static const uint8_t perm4[48] = { 32, 1, 2, 3, 4, 5,
- 4, 5, 6, 7, 8, 9,
- 8, 9, 10, 11, 12, 13,
- 12, 13, 14, 15, 16, 17,
- 16, 17, 18, 19, 20, 21,
- 20, 21, 22, 23, 24, 25,
- 24, 25, 26, 27, 28, 29,
- 28, 29, 30, 31, 32, 1};
-
-static const uint8_t perm5[32] = { 16, 7, 20, 21,
- 29, 12, 28, 17,
- 1, 15, 23, 26,
- 5, 18, 31, 10,
- 2, 8, 24, 14,
- 32, 27, 3, 9,
- 19, 13, 30, 6,
- 22, 11, 4, 25};
-
-
-static const uint8_t perm6[64] ={ 40, 8, 48, 16, 56, 24, 64, 32,
- 39, 7, 47, 15, 55, 23, 63, 31,
- 38, 6, 46, 14, 54, 22, 62, 30,
- 37, 5, 45, 13, 53, 21, 61, 29,
- 36, 4, 44, 12, 52, 20, 60, 28,
- 35, 3, 43, 11, 51, 19, 59, 27,
- 34, 2, 42, 10, 50, 18, 58, 26,
- 33, 1, 41, 9, 49, 17, 57, 25};
-
-
-static const uint8_t sc[16] = {1, 1, 2, 2, 2, 2, 2, 2, 1, 2, 2, 2, 2, 2, 2, 1};
-
-static const uint8_t sbox[8][4][16] = {
- {{14, 4, 13, 1, 2, 15, 11, 8, 3, 10, 6, 12, 5, 9, 0, 7},
- {0, 15, 7, 4, 14, 2, 13, 1, 10, 6, 12, 11, 9, 5, 3, 8},
- {4, 1, 14, 8, 13, 6, 2, 11, 15, 12, 9, 7, 3, 10, 5, 0},
- {15, 12, 8, 2, 4, 9, 1, 7, 5, 11, 3, 14, 10, 0, 6, 13}},
-
- {{15, 1, 8, 14, 6, 11, 3, 4, 9, 7, 2, 13, 12, 0, 5, 10},
- {3, 13, 4, 7, 15, 2, 8, 14, 12, 0, 1, 10, 6, 9, 11, 5},
- {0, 14, 7, 11, 10, 4, 13, 1, 5, 8, 12, 6, 9, 3, 2, 15},
- {13, 8, 10, 1, 3, 15, 4, 2, 11, 6, 7, 12, 0, 5, 14, 9}},
-
- {{10, 0, 9, 14, 6, 3, 15, 5, 1, 13, 12, 7, 11, 4, 2, 8},
- {13, 7, 0, 9, 3, 4, 6, 10, 2, 8, 5, 14, 12, 11, 15, 1},
- {13, 6, 4, 9, 8, 15, 3, 0, 11, 1, 2, 12, 5, 10, 14, 7},
- {1, 10, 13, 0, 6, 9, 8, 7, 4, 15, 14, 3, 11, 5, 2, 12}},
-
- {{7, 13, 14, 3, 0, 6, 9, 10, 1, 2, 8, 5, 11, 12, 4, 15},
- {13, 8, 11, 5, 6, 15, 0, 3, 4, 7, 2, 12, 1, 10, 14, 9},
- {10, 6, 9, 0, 12, 11, 7, 13, 15, 1, 3, 14, 5, 2, 8, 4},
- {3, 15, 0, 6, 10, 1, 13, 8, 9, 4, 5, 11, 12, 7, 2, 14}},
-
- {{2, 12, 4, 1, 7, 10, 11, 6, 8, 5, 3, 15, 13, 0, 14, 9},
- {14, 11, 2, 12, 4, 7, 13, 1, 5, 0, 15, 10, 3, 9, 8, 6},
- {4, 2, 1, 11, 10, 13, 7, 8, 15, 9, 12, 5, 6, 3, 0, 14},
- {11, 8, 12, 7, 1, 14, 2, 13, 6, 15, 0, 9, 10, 4, 5, 3}},
-
- {{12, 1, 10, 15, 9, 2, 6, 8, 0, 13, 3, 4, 14, 7, 5, 11},
- {10, 15, 4, 2, 7, 12, 9, 5, 6, 1, 13, 14, 0, 11, 3, 8},
- {9, 14, 15, 5, 2, 8, 12, 3, 7, 0, 4, 10, 1, 13, 11, 6},
- {4, 3, 2, 12, 9, 5, 15, 10, 11, 14, 1, 7, 6, 0, 8, 13}},
-
- {{4, 11, 2, 14, 15, 0, 8, 13, 3, 12, 9, 7, 5, 10, 6, 1},
- {13, 0, 11, 7, 4, 9, 1, 10, 14, 3, 5, 12, 2, 15, 8, 6},
- {1, 4, 11, 13, 12, 3, 7, 14, 10, 15, 6, 8, 0, 5, 9, 2},
- {6, 11, 13, 8, 1, 4, 10, 7, 9, 5, 0, 15, 14, 2, 3, 12}},
-
- {{13, 2, 8, 4, 6, 15, 11, 1, 10, 9, 3, 14, 5, 0, 12, 7},
- {1, 15, 13, 8, 10, 3, 7, 4, 12, 5, 6, 11, 0, 14, 9, 2},
- {7, 11, 4, 1, 9, 12, 14, 2, 0, 6, 10, 13, 15, 3, 5, 8},
- {2, 1, 14, 7, 4, 10, 8, 13, 15, 12, 9, 0, 3, 5, 6, 11}}};
-
-static void permute(char *out, const char *in, const uint8_t *p, int n)
-{
- int i;
- for (i=0;i<n;i++)
- out[i] = in[p[i]-1];
-}
-
-static void lshift(char *d, int count, int n)
-{
- char out[64];
- int i;
- for (i=0;i<n;i++)
- out[i] = d[(i+count)%n];
- for (i=0;i<n;i++)
- d[i] = out[i];
-}
-
-static void concat(char *out, char *in1, char *in2, int l1, int l2)
-{
- while (l1--)
- *out++ = *in1++;
- while (l2--)
- *out++ = *in2++;
-}
-
-static void xor(char *out, char *in1, char *in2, int n)
-{
- int i;
- for (i=0;i<n;i++)
- out[i] = in1[i] ^ in2[i];
-}
-
-static void dohash(char *out, char *in, char *key, int forw)
-{
- int i, j, k;
- char pk1[56];
- char c[28];
- char d[28];
- char cd[56];
- char ki[16][48];
- char pd1[64];
- char l[32], r[32];
- char rl[64];
-
- permute(pk1, key, perm1, 56);
-
- for (i=0;i<28;i++)
- c[i] = pk1[i];
- for (i=0;i<28;i++)
- d[i] = pk1[i+28];
-
- for (i=0;i<16;i++) {
- lshift(c, sc[i], 28);
- lshift(d, sc[i], 28);
-
- concat(cd, c, d, 28, 28);
- permute(ki[i], cd, perm2, 48);
- }
-
- permute(pd1, in, perm3, 64);
-
- for (j=0;j<32;j++) {
- l[j] = pd1[j];
- r[j] = pd1[j+32];
- }
-
- for (i=0;i<16;i++) {
- char er[48];
- char erk[48];
- char b[8][6];
- char cb[32];
- char pcb[32];
- char r2[32];
-
- permute(er, r, perm4, 48);
-
- xor(erk, er, ki[forw ? i : 15 - i], 48);
-
- for (j=0;j<8;j++)
- for (k=0;k<6;k++)
- b[j][k] = erk[j*6 + k];
-
- for (j=0;j<8;j++) {
- int m, n;
- m = (b[j][0]<<1) | b[j][5];
-
- n = (b[j][1]<<3) | (b[j][2]<<2) | (b[j][3]<<1) | b[j][4];
-
- for (k=0;k<4;k++)
- b[j][k] = (sbox[j][m][n] & (1<<(3-k)))?1:0;
- }
-
- for (j=0;j<8;j++)
- for (k=0;k<4;k++)
- cb[j*4+k] = b[j][k];
- permute(pcb, cb, perm5, 32);
-
- xor(r2, l, pcb, 32);
-
- for (j=0;j<32;j++)
- l[j] = r[j];
-
- for (j=0;j<32;j++)
- r[j] = r2[j];
- }
-
- concat(rl, r, l, 32, 32);
-
- permute(out, rl, perm6, 64);
-}
-
static void str_to_key(const uint8_t *str,uint8_t *key)
{
int i;
@@ -325,30 +92,7 @@ static int des_crypt56_gnutls(uint8_t out[8], const uint8_t in[8],
*/
void des_crypt56(uint8_t out[8], const uint8_t in[8], const uint8_t key[7], int forw)
{
- int i;
- char outb[64];
- char inb[64];
- char keyb[64];
- uint8_t key2[8];
-
- str_to_key(key, key2);
-
- for (i=0;i<64;i++) {
- inb[i] = (in[i/8] & (1<<(7-(i%8)))) ? 1 : 0;
- keyb[i] = (key2[i/8] & (1<<(7-(i%8)))) ? 1 : 0;
- outb[i] = 0;
- }
-
- dohash(outb, inb, keyb, forw);
-
- for (i=0;i<8;i++) {
- out[i] = 0;
- }
-
- for (i=0;i<64;i++) {
- if (outb[i])
- out[i/8] |= (1<<(7-(i%8)));
- }
+ (void)des_crypt56_gnutls(out, in, key, forw);
}
void E_P16(const uint8_t *p14,uint8_t *p16)
--
2.22.0

View File

@ -1,133 +0,0 @@
From fc6022b9b19473076c4236fdf4ac474f44ca73e2 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/7] 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 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);
--
2.17.1

View File

@ -1,39 +0,0 @@
From e6de467a763b93152eef27726957a32879268fb7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 19 Sep 2019 11:50:01 +1200
Subject: [PATCH 3/7] 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 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",
--
2.17.1

View File

@ -1,33 +0,0 @@
From ea39bdd6293041af668f1bfdfea39a725733bad3 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 3 May 2019 17:27:51 +1200
Subject: [PATCH 5/7] CVE-2019-14847 dsdb/modules/dirsync: ensure attrs exist
(CID 1107212)
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14040
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
(cherry picked from commit 23f72c4d712f8d1fec3d67a66d477709d5b0abe2)
---
source4/dsdb/samdb/ldb_modules/dirsync.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
index b5510eccd24..62a66fef8d4 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) {
--
2.17.1

View File

@ -1,92 +0,0 @@
From 3674b0891afb016c83763520b87e9f190dcfe884 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn@fedoraproject.org>
Date: Fri, 18 Jan 2019 16:37:24 +0100
Subject: [PATCH] CVE-2019-3824 ldb: Out of bound read in ldb_wildcard_compare
There is valgrind error in few tests tests/test-generic.sh
91 echo "Test wildcard match"
92 $VALGRIND ldbadd $LDBDIR/tests/test-wildcard.ldif || exit 1
93 $VALGRIND ldbsearch '(cn=test*multi)' || exit 1
95 $VALGRIND ldbsearch '(cn=*test_multi)' || exit 1
97 $VALGRIND ldbsearch '(cn=test*multi*test*multi)' || exit 1
e.g.
==3098== Memcheck, a memory error detector
==3098== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3098== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==3098== Command: ./bin/ldbsearch (cn=test*multi)
==3098==
==3098== Invalid read of size 1
==3098== at 0x483CEE7: memchr (vg_replace_strmem.c:890)
==3098== by 0x49A9073: memmem (in /usr/lib64/libc-2.28.9000.so)
==3098== by 0x485DFE9: ldb_wildcard_compare (ldb_match.c:313)
==3098== by 0x485DFE9: ldb_match_substring (ldb_match.c:360)
==3098== by 0x485DFE9: ldb_match_message (ldb_match.c:572)
==3098== by 0x558F8FA: search_func (ldb_kv_search.c:549)
==3098== by 0x48C78CA: ??? (in /usr/lib64/libtdb.so.1.3.17)
==3098== by 0x48C7A60: tdb_traverse_read (in /usr/lib64/libtdb.so.1.3.17)
==3098== by 0x557B7C4: ltdb_traverse_fn (ldb_tdb.c:274)
==3098== by 0x558FBFA: ldb_kv_search_full (ldb_kv_search.c:594)
==3098== by 0x558FBFA: ldb_kv_search (ldb_kv_search.c:854)
==3098== by 0x558E497: ldb_kv_callback (ldb_kv.c:1713)
==3098== by 0x48FCD58: tevent_common_invoke_timer_handler (in /usr/lib64/libtevent.so.0.9.38)
==3098== by 0x48FCEFD: tevent_common_loop_timer_delay (in /usr/lib64/libtevent.so.0.9.38)
==3098== by 0x48FE14A: ??? (in /usr/lib64/libtevent.so.0.9.38)
==3098== Address 0x4b4ab81 is 0 bytes after a block of size 129 alloc'd
==3098== at 0x483880B: malloc (vg_replace_malloc.c:309)
==3098== by 0x491048B: talloc_strndup (in /usr/lib64/libtalloc.so.2.1.15)
==3098== by 0x48593CA: ldb_casefold_default (ldb_utf8.c:59)
==3098== by 0x485F68D: ldb_handler_fold (attrib_handlers.c:64)
==3098== by 0x485DB88: ldb_wildcard_compare (ldb_match.c:257)
==3098== by 0x485DB88: ldb_match_substring (ldb_match.c:360)
==3098== by 0x485DB88: ldb_match_message (ldb_match.c:572)
==3098== by 0x558F8FA: search_func (ldb_kv_search.c:549)
==3098== by 0x48C78CA: ??? (in /usr/lib64/libtdb.so.1.3.17)
==3098== by 0x48C7A60: tdb_traverse_read (in /usr/lib64/libtdb.so.1.3.17)
==3098== by 0x557B7C4: ltdb_traverse_fn (ldb_tdb.c:274)
==3098== by 0x558FBFA: ldb_kv_search_full (ldb_kv_search.c:594)
==3098== by 0x558FBFA: ldb_kv_search (ldb_kv_search.c:854)
==3098== by 0x558E497: ldb_kv_callback (ldb_kv.c:1713)
==3098== by 0x48FCD58: tevent_common_invoke_timer_handler (in /usr/lib64/libtevent.so.0.9.38)
==3098==
# record 1
dn: cn=test_multi_test_multi_test_multi,o=University of Michigan,c=TEST
cn: test_multi_test_multi_test_multi
description: test multi wildcards matching
objectclass: person
sn: multi_test
name: test_multi_test_multi_test_multi
distinguishedName: cn=test_multi_test_multi_test_multi,o=University of Michiga
n,c=TEST
# returned 1 records
# 1 entries
# 0 referrals
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Lukas Slebodnik <lslebodn@fedoraproject.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
lib/ldb/common/ldb_match.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 25fe3f9c21b..8eeedfb12e0 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -308,9 +308,10 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
if (p == NULL) goto mismatch;
if ( (! tree->u.substring.chunks[c + 1]) && (! tree->u.substring.end_with_wildcard) ) {
uint8_t *g;
+ uint8_t *end = val.data + val.length;
do { /* greedy */
g = memmem(p + cnk.length,
- val.length - (p - val.data),
+ end - (p + cnk.length),
(const uint8_t *)cnk.data,
cnk.length);
if (g) p = g;
--
2.24.0

View File

@ -0,0 +1,314 @@
From 3828e798da8e0b44356039dd927f0624d5d182f9 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Wed, 6 Nov 2019 12:12:55 +0200
Subject: [PATCH] Remove DES support if MIT Kerberos version does not support
it
---
source3/libads/kerberos_keytab.c | 2 -
source3/passdb/machine_account_secrets.c | 36 ------------------
source4/auth/kerberos/kerberos.h | 2 +-
.../dsdb/samdb/ldb_modules/password_hash.c | 12 ++++++
source4/kdc/db-glue.c | 4 +-
source4/torture/rpc/remote_pac.c | 37 -------------------
testprogs/blackbox/dbcheck-oldrelease.sh | 2 +-
testprogs/blackbox/functionalprep.sh | 2 +-
.../blackbox/test_export_keytab_heimdal.sh | 16 ++++----
.../blackbox/upgradeprovision-oldrelease.sh | 2 +-
10 files changed, 26 insertions(+), 89 deletions(-)
diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
index 97d5535041c..7d193e1a600 100644
--- a/source3/libads/kerberos_keytab.c
+++ b/source3/libads/kerberos_keytab.c
@@ -240,8 +240,6 @@ int ads_keytab_add_entry(ADS_STRUCT *ads, const char *srvPrinc, bool update_ads)
krb5_data password;
krb5_kvno kvno;
krb5_enctype enctypes[6] = {
- ENCTYPE_DES_CBC_CRC,
- ENCTYPE_DES_CBC_MD5,
#ifdef HAVE_ENCTYPE_AES128_CTS_HMAC_SHA1_96
ENCTYPE_AES128_CTS_HMAC_SHA1_96,
#endif
diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
index dfc21f295a1..efba80f1474 100644
--- a/source3/passdb/machine_account_secrets.c
+++ b/source3/passdb/machine_account_secrets.c
@@ -1031,7 +1031,6 @@ static int secrets_domain_info_kerberos_keys(struct secrets_domain_info1_passwor
krb5_keyblock key;
DATA_BLOB aes_256_b = data_blob_null;
DATA_BLOB aes_128_b = data_blob_null;
- DATA_BLOB des_md5_b = data_blob_null;
bool ok;
#endif /* HAVE_ADS */
DATA_BLOB arc4_b = data_blob_null;
@@ -1177,32 +1176,6 @@ static int secrets_domain_info_kerberos_keys(struct secrets_domain_info1_passwor
return ENOMEM;
}
- krb5_ret = smb_krb5_create_key_from_string(krb5_ctx,
- NULL,
- &salt,
- &cleartext_utf8,
- ENCTYPE_DES_CBC_MD5,
- &key);
- if (krb5_ret != 0) {
- DBG_ERR("generation of a des-cbc-md5 key failed: %s\n",
- smb_get_krb5_error_message(krb5_ctx, krb5_ret, keys));
- krb5_free_context(krb5_ctx);
- TALLOC_FREE(keys);
- TALLOC_FREE(salt_data);
- return krb5_ret;
- }
- des_md5_b = data_blob_talloc(keys,
- KRB5_KEY_DATA(&key),
- KRB5_KEY_LENGTH(&key));
- krb5_free_keyblock_contents(krb5_ctx, &key);
- if (des_md5_b.data == NULL) {
- DBG_ERR("data_blob_talloc failed for des-cbc-md5.\n");
- krb5_free_context(krb5_ctx);
- TALLOC_FREE(keys);
- TALLOC_FREE(salt_data);
- return ENOMEM;
- }
-
krb5_free_context(krb5_ctx);
no_kerberos:
@@ -1227,15 +1200,6 @@ no_kerberos:
keys[idx].value = arc4_b;
idx += 1;
-#ifdef HAVE_ADS
- if (des_md5_b.length != 0) {
- keys[idx].keytype = ENCTYPE_DES_CBC_MD5;
- keys[idx].iteration_count = 4096;
- keys[idx].value = des_md5_b;
- idx += 1;
- }
-#endif /* HAVE_ADS */
-
p->salt_data = salt_data;
p->default_iteration_count = 4096;
p->num_keys = idx;
diff --git a/source4/auth/kerberos/kerberos.h b/source4/auth/kerberos/kerberos.h
index 2ff9e3868af..1dd63acc838 100644
--- a/source4/auth/kerberos/kerberos.h
+++ b/source4/auth/kerberos/kerberos.h
@@ -50,7 +50,7 @@ struct keytab_container {
#define TOK_ID_GSS_GETMIC ((const uint8_t *)"\x01\x01")
#define TOK_ID_GSS_WRAP ((const uint8_t *)"\x02\x01")
-#define ENC_ALL_TYPES (ENC_CRC32 | ENC_RSA_MD5 | ENC_RC4_HMAC_MD5 | \
+#define ENC_ALL_TYPES (ENC_RC4_HMAC_MD5 | \
ENC_HMAC_SHA1_96_AES128 | ENC_HMAC_SHA1_96_AES256)
#ifndef HAVE_KRB5_SET_DEFAULT_TGS_KTYPES
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index 006e35c46d5..f16937c6cab 100644
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -786,6 +786,7 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
* create ENCTYPE_DES_CBC_MD5 key out of
* the salt and the cleartext password
*/
+#ifdef SAMBA4_USES_HEIMDAL
krb5_ret = smb_krb5_create_key_from_string(io->smb_krb5_context->krb5_context,
NULL,
&salt,
@@ -804,6 +805,11 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
KRB5_KEY_DATA(&key),
KRB5_KEY_LENGTH(&key));
krb5_free_keyblock_contents(io->smb_krb5_context->krb5_context, &key);
+#else
+ /* MIT has dropped support for DES enctypes, store a random key instead. */
+ io->g.des_md5 = data_blob_talloc(io->ac, NULL, 8);
+ generate_secret_buffer(io->g.des_md5.data, 8);
+#endif
if (!io->g.des_md5.data) {
return ldb_oom(ldb);
}
@@ -812,6 +818,7 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
* create ENCTYPE_DES_CBC_CRC key out of
* the salt and the cleartext password
*/
+#ifdef SAMBA4_USES_HEIMDAL
krb5_ret = smb_krb5_create_key_from_string(io->smb_krb5_context->krb5_context,
NULL,
&salt,
@@ -830,6 +837,11 @@ static int setup_kerberos_keys(struct setup_password_fields_io *io)
KRB5_KEY_DATA(&key),
KRB5_KEY_LENGTH(&key));
krb5_free_keyblock_contents(io->smb_krb5_context->krb5_context, &key);
+#else
+ /* MIT has dropped support for DES enctypes, store a random key instead. */
+ io->g.des_crc = data_blob_talloc(io->ac, NULL, 8);
+ generate_secret_buffer(io->g.des_crc.data, 8);
+#endif
if (!io->g.des_crc.data) {
return ldb_oom(ldb);
}
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index f62a633c6c7..023ae7b580d 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -359,10 +359,10 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
/* If UF_USE_DES_KEY_ONLY has been set, then don't allow use of the newer enc types */
if (userAccountControl & UF_USE_DES_KEY_ONLY) {
- supported_enctypes = ENC_CRC32|ENC_RSA_MD5;
+ supported_enctypes = 0;
} else {
/* Otherwise, add in the default enc types */
- supported_enctypes |= ENC_CRC32 | ENC_RSA_MD5 | ENC_RC4_HMAC_MD5;
+ supported_enctypes |= ENC_RC4_HMAC_MD5;
}
/* Is this the krbtgt or a RODC krbtgt */
diff --git a/source4/torture/rpc/remote_pac.c b/source4/torture/rpc/remote_pac.c
index 7a5cda74b74..f12060e3c8f 100644
--- a/source4/torture/rpc/remote_pac.c
+++ b/source4/torture/rpc/remote_pac.c
@@ -38,7 +38,6 @@
#define TEST_MACHINE_NAME_BDC "torturepacbdc"
#define TEST_MACHINE_NAME_WKSTA "torturepacwksta"
-#define TEST_MACHINE_NAME_WKSTA_DES "torturepacwkdes"
#define TEST_MACHINE_NAME_S4U2SELF_BDC "tests4u2selfbdc"
#define TEST_MACHINE_NAME_S4U2SELF_WKSTA "tests4u2selfwk"
@@ -581,39 +580,6 @@ static bool test_PACVerify_workstation_aes(struct torture_context *tctx,
NETLOGON_NEG_AUTH2_ADS_FLAGS | NETLOGON_NEG_SUPPORTS_AES);
}
-static bool test_PACVerify_workstation_des(struct torture_context *tctx,
- struct dcerpc_pipe *p, struct cli_credentials *credentials, struct test_join *join_ctx)
-{
- struct samr_SetUserInfo r;
- union samr_UserInfo user_info;
- struct dcerpc_pipe *samr_pipe = torture_join_samr_pipe(join_ctx);
- struct smb_krb5_context *smb_krb5_context;
- krb5_error_code ret;
-
- ret = cli_credentials_get_krb5_context(popt_get_cmdline_credentials(),
- tctx->lp_ctx, &smb_krb5_context);
- torture_assert_int_equal(tctx, ret, 0, "cli_credentials_get_krb5_context() failed");
-
- if (smb_krb5_get_allowed_weak_crypto(smb_krb5_context->krb5_context) == FALSE) {
- torture_skip(tctx, "Cannot test DES without [libdefaults] allow_weak_crypto = yes");
- }
-
- /* Mark this workstation with DES-only */
- user_info.info16.acct_flags = ACB_USE_DES_KEY_ONLY | ACB_WSTRUST;
- r.in.user_handle = torture_join_samr_user_policy(join_ctx);
- r.in.level = 16;
- r.in.info = &user_info;
-
- torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(samr_pipe->binding_handle, tctx, &r),
- "failed to set DES info account flags");
- torture_assert_ntstatus_ok(tctx, r.out.result,
- "failed to set DES into account flags");
-
- return test_PACVerify(tctx, p, credentials, SEC_CHAN_WKSTA,
- TEST_MACHINE_NAME_WKSTA_DES,
- NETLOGON_NEG_AUTH2_ADS_FLAGS);
-}
-
#ifdef SAMBA4_USES_HEIMDAL
static NTSTATUS check_primary_group_in_validation(TALLOC_CTX *mem_ctx,
uint16_t validation_level,
@@ -1000,9 +966,6 @@ struct torture_suite *torture_rpc_remote_pac(TALLOC_CTX *mem_ctx)
&ndr_table_netlogon, TEST_MACHINE_NAME_WKSTA);
torture_rpc_tcase_add_test_creds(tcase, "verify-sig-aes", test_PACVerify_workstation_aes);
- tcase = torture_suite_add_machine_workstation_rpc_iface_tcase(suite, "netlogon-member-des",
- &ndr_table_netlogon, TEST_MACHINE_NAME_WKSTA_DES);
- torture_rpc_tcase_add_test_join(tcase, "verify-sig", test_PACVerify_workstation_des);
#ifdef SAMBA4_USES_HEIMDAL
tcase = torture_suite_add_machine_bdc_rpc_iface_tcase(suite, "netr-bdc-arcfour",
&ndr_table_netlogon, TEST_MACHINE_NAME_S4U2SELF_BDC);
diff --git a/testprogs/blackbox/dbcheck-oldrelease.sh b/testprogs/blackbox/dbcheck-oldrelease.sh
index 3d0ee2c165a..41c55178d4e 100755
--- a/testprogs/blackbox/dbcheck-oldrelease.sh
+++ b/testprogs/blackbox/dbcheck-oldrelease.sh
@@ -388,7 +388,7 @@ referenceprovision() {
ldapcmp() {
if [ x$RELEASE = x"release-4-0-0" ]; then
- $PYTHON $BINDIR/samba-tool ldapcmp tdb://$PREFIX_ABS/${RELEASE}_reference/private/sam.ldb tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --two --skip-missing-dn --filter=dnsRecord,displayName
+ $PYTHON $BINDIR/samba-tool ldapcmp tdb://$PREFIX_ABS/${RELEASE}_reference/private/sam.ldb tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --two --skip-missing-dn --filter=dnsRecord,displayName,msDS-SupportedEncryptionTypes
fi
}
diff --git a/testprogs/blackbox/functionalprep.sh b/testprogs/blackbox/functionalprep.sh
index 80e82252d45..1d37611ef7a 100755
--- a/testprogs/blackbox/functionalprep.sh
+++ b/testprogs/blackbox/functionalprep.sh
@@ -61,7 +61,7 @@ provision_2012r2() {
ldapcmp_ignore() {
# At some point we will need to ignore, but right now, it should be perfect
IGNORE_ATTRS=$1
- $PYTHON $BINDIR/samba-tool ldapcmp tdb://$PREFIX_ABS/$2/private/sam.ldb tdb://$PREFIX_ABS/$3/private/sam.ldb --two --skip-missing-dn
+ $PYTHON $BINDIR/samba-tool ldapcmp tdb://$PREFIX_ABS/$2/private/sam.ldb tdb://$PREFIX_ABS/$3/private/sam.ldb --two --skip-missing-dn --filter msDS-SupportedEncryptionTypes
}
ldapcmp() {
diff --git a/testprogs/blackbox/test_export_keytab_heimdal.sh b/testprogs/blackbox/test_export_keytab_heimdal.sh
index cfa245fd4de..6a2595cd684 100755
--- a/testprogs/blackbox/test_export_keytab_heimdal.sh
+++ b/testprogs/blackbox/test_export_keytab_heimdal.sh
@@ -43,7 +43,7 @@ test_keytab() {
echo "test: $testname"
- NKEYS=$($VALGRIND $samba4ktutil $keytab | grep -i "$principal" | egrep -c "des|aes|arcfour")
+ NKEYS=$($VALGRIND $samba4ktutil $keytab | grep -i "$principal" | egrep -c "aes|arcfour")
status=$?
if [ x$status != x0 ]; then
echo "failure: $testname"
@@ -64,22 +64,22 @@ unc="//$SERVER/tmp"
testit "create user locally" $VALGRIND $PYTHON $newuser nettestuser $USERPASS $@ || failed=`expr $failed + 1`
testit "dump keytab from domain" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab $@ || failed=`expr $failed + 1`
-test_keytab "read keytab from domain" "$PREFIX/tmpkeytab" "$SERVER\\\$" 5
+test_keytab "read keytab from domain" "$PREFIX/tmpkeytab" "$SERVER\\\$" 3
testit "dump keytab from domain (2nd time)" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab $@ || failed=`expr $failed + 1`
-test_keytab "read keytab from domain (2nd time)" "$PREFIX/tmpkeytab" "$SERVER\\\$" 5
+test_keytab "read keytab from domain (2nd time)" "$PREFIX/tmpkeytab" "$SERVER\\\$" 3
testit "dump keytab from domain for cifs principal" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab-server --principal=cifs/$SERVER_FQDN $@ || failed=`expr $failed + 1`
-test_keytab "read keytab from domain for cifs principal" "$PREFIX/tmpkeytab-server" "cifs/$SERVER_FQDN" 5
+test_keytab "read keytab from domain for cifs principal" "$PREFIX/tmpkeytab-server" "cifs/$SERVER_FQDN" 3
testit "dump keytab from domain for cifs principal (2nd time)" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab-server --principal=cifs/$SERVER_FQDN $@ || failed=`expr $failed + 1`
-test_keytab "read keytab from domain for cifs principal (2nd time)" "$PREFIX/tmpkeytab-server" "cifs/$SERVER_FQDN" 5
+test_keytab "read keytab from domain for cifs principal (2nd time)" "$PREFIX/tmpkeytab-server" "cifs/$SERVER_FQDN" 3
testit "dump keytab from domain for user principal" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab-2 --principal=nettestuser $@ || failed=`expr $failed + 1`
-test_keytab "dump keytab from domain for user principal" "$PREFIX/tmpkeytab-2" "nettestuser@$REALM" 5
+test_keytab "dump keytab from domain for user principal" "$PREFIX/tmpkeytab-2" "nettestuser@$REALM" 3
testit "dump keytab from domain for user principal (2nd time)" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab-2 --principal=nettestuser@$REALM $@ || failed=`expr $failed + 1`
-test_keytab "dump keytab from domain for user principal (2nd time)" "$PREFIX/tmpkeytab-2" "nettestuser@$REALM" 5
+test_keytab "dump keytab from domain for user principal (2nd time)" "$PREFIX/tmpkeytab-2" "nettestuser@$REALM" 3
testit "dump keytab from domain for user principal with SPN as UPN" $VALGRIND $PYTHON $samba_tool domain exportkeytab $PREFIX/tmpkeytab-3 --principal=http/testupnspn.$DNSDOMAIN $@ || failed=`expr $failed + 1`
-test_keytab "dump keytab from domain for user principal" "$PREFIX/tmpkeytab-3" "http/testupnspn.$DNSDOMAIN@$REALM" 5
+test_keytab "dump keytab from domain for user principal" "$PREFIX/tmpkeytab-3" "http/testupnspn.$DNSDOMAIN@$REALM" 3
KRB5CCNAME="$PREFIX/tmpuserccache"
export KRB5CCNAME
diff --git a/testprogs/blackbox/upgradeprovision-oldrelease.sh b/testprogs/blackbox/upgradeprovision-oldrelease.sh
index 76276168011..208baa54a02 100755
--- a/testprogs/blackbox/upgradeprovision-oldrelease.sh
+++ b/testprogs/blackbox/upgradeprovision-oldrelease.sh
@@ -106,7 +106,7 @@ referenceprovision() {
ldapcmp() {
if [ x$RELEASE != x"alpha13" ]; then
- $PYTHON $BINDIR/samba-tool ldapcmp tdb://$PREFIX_ABS/${RELEASE}_upgrade_reference/private/sam.ldb tdb://$PREFIX_ABS/${RELEASE}_upgrade/private/sam.ldb --two --skip-missing-dn --filter=dnsRecord,displayName
+ $PYTHON $BINDIR/samba-tool ldapcmp tdb://$PREFIX_ABS/${RELEASE}_upgrade_reference/private/sam.ldb tdb://$PREFIX_ABS/${RELEASE}_upgrade/private/sam.ldb --two --skip-missing-dn --filter=dnsRecord,displayName,msDS-SupportedEncryptionTypes
fi
}
--
2.23.0

View File

@ -1,36 +0,0 @@
From 167f78aa97af6502cb2027dc9dad40399b0a9c4f 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/7] 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 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)) {
/*
--
2.17.1

View File

@ -1,105 +0,0 @@
From 70078d4ddf3b842eeadee058dadeef82ec4edf0b 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 4/7] 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 | 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 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 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;
--
2.17.1

View File

@ -1,72 +0,0 @@
From bdb3e3f669bd991da819040e726e003e4e2b841d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 15 Oct 2019 16:28:46 +1300
Subject: [PATCH 6/7] CVE-2019-14847 dsdb: Demonstrate the correct interaction
of ranged_results style attributes and dirsync
Incremental results are provided by a flag on the dirsync control, not
by changing the attribute name.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14040
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
selftest/knownfail.d/dirsync | 1 +
source4/dsdb/tests/python/dirsync.py | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
create mode 100644 selftest/knownfail.d/dirsync
diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
new file mode 100644
index 00000000000..bc49fe0d9bb
--- /dev/null
+++ b/selftest/knownfail.d/dirsync
@@ -0,0 +1 @@
+^samba4.ldap.dirsync.python\(ad_dc_ntvfs\).__main__.ExtendedDirsyncTests.test_dirsync_linkedattributes_range\(
\ No newline at end of file
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)
--
2.17.1

View File

@ -1,56 +0,0 @@
From 745b99fc6b75db33cdb0a58df1a3f2a5063bc76e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 4 Feb 2019 11:22:34 +1300
Subject: [PATCH] CVE-2019-3824 ldb: Extra comments to clarify no pointer wrap
in wildcard processing
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
lib/ldb/common/ldb_match.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 8eeedfb12e0..1920b661f75 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -306,12 +306,33 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
p = memmem((const void *)val.data,val.length,
(const void *)cnk.data, cnk.length);
if (p == NULL) goto mismatch;
+
+ /*
+ * At this point we know cnk.length <= val.length as
+ * otherwise there could be no match
+ */
+
if ( (! tree->u.substring.chunks[c + 1]) && (! tree->u.substring.end_with_wildcard) ) {
uint8_t *g;
uint8_t *end = val.data + val.length;
do { /* greedy */
- g = memmem(p + cnk.length,
- end - (p + cnk.length),
+
+ /*
+ * haystack is a valid pointer in val
+ * because the memmem() can only
+ * succeed if the needle (cnk.length)
+ * is <= haystacklen
+ *
+ * p will be a pointer at least
+ * cnk.length from the end of haystack
+ */
+ uint8_t *haystack
+ = p + cnk.length;
+ size_t haystacklen
+ = end - (haystack);
+
+ g = memmem(haystack,
+ haystacklen,
(const uint8_t *)cnk.data,
cnk.length);
if (g) p = g;
--
2.24.0

View File

@ -0,0 +1,42 @@
From 5a084994144704a6c146b94f8a22cf57ce08deab Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab@samba.org>
Date: Mon, 7 Oct 2019 18:24:28 +0300
Subject: [PATCH] samba-tool: create working private krb5.conf
DNS update tool uses private krb5.conf which should have enough details
to authenticate with GSS-TSIG when running nsupdate.
Unfortunately, the configuration we provide is not enough. We set
defaults to not lookup REALM via DNS but at the same time we don't
provide any realm definition. As result, MIT Kerberos cannot actually
find a working realm for Samba AD deployment because it cannot query DNS
for a realm discovery or pick it up from the configuration.
Extend private krb5.conf with a realm definition that will allow MIT
Kerberos to look up KDC over DNS.
Signed-off-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
---
source4/setup/krb5.conf | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/source4/setup/krb5.conf b/source4/setup/krb5.conf
index b1bf6cf907d..ad6f2818fb5 100644
--- a/source4/setup/krb5.conf
+++ b/source4/setup/krb5.conf
@@ -2,3 +2,11 @@
default_realm = ${REALM}
dns_lookup_realm = false
dns_lookup_kdc = true
+
+[realms]
+${REALM} = {
+ default_domain = ${DNSDOMAIN}
+}
+
+[domain_realm]
+ ${HOSTNAME} = ${REALM}
--
2.21.0

View File

@ -1,112 +0,0 @@
From 77b10b360f4ffb7ac90bc5fce0a80306515c1aca Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 15 Oct 2019 15:44:34 +1300
Subject: [PATCH 7/7] CVE-2019-14847 dsdb: Correct behaviour of ranged_results
when combined with dirsync
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14040
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
selftest/knownfail.d/dirsync | 1 -
source4/dsdb/samdb/ldb_modules/dirsync.c | 11 ++++----
.../dsdb/samdb/ldb_modules/ranged_results.c | 25 ++++++++++++++++---
3 files changed, 28 insertions(+), 9 deletions(-)
delete mode 100644 selftest/knownfail.d/dirsync
diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
deleted file mode 100644
index bc49fe0d9bb..00000000000
--- a/selftest/knownfail.d/dirsync
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.ldap.dirsync.python\(ad_dc_ntvfs\).__main__.ExtendedDirsyncTests.test_dirsync_linkedattributes_range\(
\ No newline at end of file
diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
index 62a66fef8d4..4ac5faad403 100644
--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
@@ -998,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) {
@@ -1327,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);
--
2.17.1

View File

@ -1,35 +0,0 @@
From 9427806f7298d71bd7edfbdda7506ec63f15dda1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 4 Feb 2019 11:22:50 +1300
Subject: [PATCH] CVE-2019-3824 ldb: Improve code style and layout in wildcard
processing
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
lib/ldb/common/ldb_match.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 1920b661f75..ab0a89888f0 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -333,9 +333,11 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
g = memmem(haystack,
haystacklen,
- (const uint8_t *)cnk.data,
- cnk.length);
- if (g) p = g;
+ (const uint8_t *)cnk.data,
+ cnk.length);
+ if (g) {
+ p = g;
+ }
} while(g);
}
val.length = val.length - (p - (uint8_t *)(val.data)) - cnk.length;
--
2.24.0

View File

@ -1,32 +0,0 @@
From 8d34d172092f71baad0d777567e49aebfa07313d Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 19 Feb 2019 10:25:24 +1300
Subject: [PATCH] CVE-2019-3824 ldb: ldb_parse_tree use talloc_zero
Initialise the created ldb_parse_tree with talloc_zero, this ensures
that it is correctly initialised if inadvertently passed to a function
expecting a different operation type.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
lib/ldb/common/ldb_parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/ldb/common/ldb_parse.c b/lib/ldb/common/ldb_parse.c
index 5fa5a74afa9..db420091311 100644
--- a/lib/ldb/common/ldb_parse.c
+++ b/lib/ldb/common/ldb_parse.c
@@ -389,7 +389,7 @@ static struct ldb_parse_tree *ldb_parse_simple(TALLOC_CTX *mem_ctx, const char *
struct ldb_parse_tree *ret;
enum ldb_parse_op filtertype;
- ret = talloc(mem_ctx, struct ldb_parse_tree);
+ ret = talloc_zero(mem_ctx, struct ldb_parse_tree);
if (!ret) {
errno = ENOMEM;
return NULL;
--
2.24.0

View File

@ -1,38 +0,0 @@
From 34383981a0c40860f71a4451ff8fd752e1b67666 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 19 Feb 2019 10:26:25 +1300
Subject: [PATCH] CVE-2019-3824 ldb: wildcard_match check tree operation
Check the operation type of the passed parse tree, and return
LDB_INAPPROPRIATE_MATCH if the operation is not LDB_OP_SUBSTRING.
A query of "attribute=*" gets parsed as LDB_OP_PRESENT, checking the
operation and failing ldb_wildcard_match should help prevent confusion
writing tests.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
lib/ldb/common/ldb_match.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index ab0a89888f0..59f48b52b70 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -244,6 +244,11 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
uint8_t *save_p = NULL;
unsigned int c = 0;
+ if (tree->operation != LDB_OP_SUBSTRING) {
+ *matched = false;
+ return LDB_ERR_INAPPROPRIATE_MATCHING;
+ }
+
a = ldb_schema_attribute_by_name(ldb, tree->u.substring.attr);
if (!a) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
--
2.24.0

View File

@ -1,34 +0,0 @@
From 42f0f57eb819ce6b68a8c5b3b53123b83ec917e3 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 19 Feb 2019 10:26:56 +1300
Subject: [PATCH] CVE-2019-3824 ldb: wildcard_match end of data check
ldb_handler_copy and ldb_val_dup over allocate by one and add a trailing '\0'
to the data, to make them safe to use the C string functions on.
However testing for the trailing '\0' is not the correct way to test for
the end of a value, the length should be checked instead.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
lib/ldb/common/ldb_match.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 59f48b52b70..829afa77e71 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -353,7 +353,7 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
}
/* last chunk may not have reached end of string */
- if ( (! tree->u.substring.end_with_wildcard) && (*(val.data) != 0) ) goto mismatch;
+ if ( (! tree->u.substring.end_with_wildcard) && (val.length != 0) ) goto mismatch;
talloc_free(save_p);
*matched = true;
return LDB_SUCCESS;
--
2.24.0

View File

@ -1,270 +0,0 @@
From 45b75db50f5c1a7c8c38af59a62fccee5401c845 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 19 Feb 2019 10:24:38 +1300
Subject: [PATCH] CVE-2019-3824 ldb: Add tests for ldb_wildcard_match
Add cmocka tests for ldb_wildcard_match.
Running test_wildcard_match under valgrind reproduces
CVE-2019-3824 out of bounds read in wildcard compare (bug 13773)
valgrind --suppressions=lib/ldb/tests/ldb_match_test.valgrind\
bin/ldb_match_test
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13773
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
lib/ldb/tests/ldb_match_test.c | 191 ++++++++++++++++++++++++++
lib/ldb/tests/ldb_match_test.valgrind | 16 +++
lib/ldb/wscript | 8 +-
3 files changed, 214 insertions(+), 1 deletion(-)
create mode 100644 lib/ldb/tests/ldb_match_test.c
create mode 100644 lib/ldb/tests/ldb_match_test.valgrind
diff --git a/lib/ldb/tests/ldb_match_test.c b/lib/ldb/tests/ldb_match_test.c
new file mode 100644
index 00000000000..e09f50c86ba
--- /dev/null
+++ b/lib/ldb/tests/ldb_match_test.c
@@ -0,0 +1,191 @@
+/*
+ * Tests exercising the ldb match operations.
+ *
+ *
+ * Copyright (C) Catalyst.NET Ltd 2017
+ *
+ * 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 cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "../common/ldb_match.c"
+
+#include "../include/ldb.h"
+
+struct ldbtest_ctx {
+ struct tevent_context *ev;
+ struct ldb_context *ldb;
+};
+
+static int ldb_test_canonicalise(
+ struct ldb_context *ldb,
+ void *mem_ctx,
+ const struct ldb_val *in,
+ struct ldb_val *out)
+{
+ out->length = in->length;
+ out->data = in->data;
+ return 0;
+}
+
+static int setup(void **state)
+{
+ struct ldbtest_ctx *test_ctx;
+ struct ldb_schema_syntax *syntax = NULL;
+ int ret;
+
+ test_ctx = talloc_zero(NULL, struct ldbtest_ctx);
+ assert_non_null(test_ctx);
+
+ test_ctx->ev = tevent_context_init(test_ctx);
+ assert_non_null(test_ctx->ev);
+
+ test_ctx->ldb = ldb_init(test_ctx, test_ctx->ev);
+ assert_non_null(test_ctx->ldb);
+
+ syntax = talloc_zero(test_ctx, struct ldb_schema_syntax);
+ assert_non_null(syntax);
+ syntax->canonicalise_fn = ldb_test_canonicalise;
+
+ ret = ldb_schema_attribute_add_with_syntax(
+ test_ctx->ldb, "a", LDB_ATTR_FLAG_FIXED, syntax);
+ assert_int_equal(LDB_SUCCESS, ret);
+
+ *state = test_ctx;
+ return 0;
+}
+
+static int teardown(void **state)
+{
+ talloc_free(*state);
+ return 0;
+}
+
+
+/*
+ * The wild card pattern "attribute=*" is parsed as an LDB_OP_PRESENT operation
+ * rather than a LDB_OP_????
+ *
+ * This test serves to document that behaviour, and to confirm that
+ * ldb_wildcard_compare handles this case appropriately.
+ */
+static void test_wildcard_match_star(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ bool matched = false;
+ int ret;
+
+ uint8_t value[] = "The value.......end";
+ struct ldb_val val = {
+ .data = value,
+ .length = (sizeof(value))
+ };
+ struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "a=*");
+ assert_non_null(tree);
+
+ ret = ldb_wildcard_compare(ctx->ldb, tree, val, &matched);
+ assert_false(matched);
+ assert_int_equal(LDB_ERR_INAPPROPRIATE_MATCHING, ret);
+}
+
+/*
+ * Test basic wild card matching
+ *
+ */
+static void test_wildcard_match(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ bool matched = false;
+
+ uint8_t value[] = "The value.......end";
+ struct ldb_val val = {
+ .data = value,
+ .length = (sizeof(value))
+ };
+ struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "objectClass=*end");
+ assert_non_null(tree);
+
+ ldb_wildcard_compare(ctx->ldb, tree, val, &matched);
+ assert_true(matched);
+}
+
+
+/*
+ * ldb_handler_copy and ldb_val_dup over allocate by one and add a trailing '\0'
+ * to the data, to make them safe to use the C string functions on.
+ *
+ * However testing for the trailing '\0' is not the correct way to test for
+ * the end of a value, the length should be checked instead.
+ */
+static void test_wildcard_match_end_condition(void **state)
+{
+ struct ldbtest_ctx *ctx = *state;
+ bool matched = false;
+
+ uint8_t value[] = "hellomynameisbobx";
+ struct ldb_val val = {
+ .data = talloc_memdup(NULL, value, sizeof(value)),
+ .length = (sizeof(value) - 2)
+ };
+ struct ldb_parse_tree *tree = ldb_parse_tree(ctx, "a=*hello*mynameis*bob");
+ assert_non_null(tree);
+
+ ldb_wildcard_compare(ctx->ldb, tree, val, &matched);
+ assert_true(matched);
+}
+
+/*
+ * Note: to run under valgrind use:
+ * valgrind \
+ * --suppressions=lib/ldb/tests/ldb_match_test.valgrind \
+ * bin/ldb_match_test
+ */
+int main(int argc, const char **argv)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test_setup_teardown(
+ test_wildcard_match_star,
+ setup,
+ teardown),
+ cmocka_unit_test_setup_teardown(
+ test_wildcard_match,
+ setup,
+ teardown),
+ cmocka_unit_test_setup_teardown(
+ test_wildcard_match_end_condition,
+ setup,
+ teardown),
+ };
+
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/lib/ldb/tests/ldb_match_test.valgrind b/lib/ldb/tests/ldb_match_test.valgrind
new file mode 100644
index 00000000000..660bd5a6b46
--- /dev/null
+++ b/lib/ldb/tests/ldb_match_test.valgrind
@@ -0,0 +1,16 @@
+{
+ Memory allocated in set-up
+ Memcheck:Leak
+ match-leak-kinds: possible
+ fun:malloc
+ ...
+ fun:setup
+}
+{
+ Memory allocated by ldb_init
+ Memcheck:Leak
+ match-leak-kinds: possible
+ fun:malloc
+ ...
+ fun:ldb_init
+}
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 6e224e7b4b7..5355585f69c 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -500,6 +500,11 @@ def build(bld):
deps='cmocka ldb',
install=False)
+ bld.SAMBA_BINARY('ldb_match_test',
+ source='tests/ldb_match_test.c',
+ deps='cmocka ldb',
+ install=False)
+
if bld.CONFIG_SET('HAVE_LMDB'):
bld.SAMBA_BINARY('ldb_mdb_mod_op_test',
source='tests/ldb_mod_op_test.c',
@@ -567,7 +572,8 @@ def test(ctx):
# we don't want to run ldb_lmdb_size_test (which proves we can
# fit > 4G of data into the DB), it would fill up the disk on
# many of our test instances
- 'ldb_mdb_kv_ops_test']
+ 'ldb_mdb_kv_ops_test',
+ 'ldb_match_test']
for test_exe in test_exes:
cmd = os.path.join(Utils.g_module.blddir, test_exe)

View File

@ -1,92 +0,0 @@
From bf596c14c2462b9a15ea738ef4f32b3abb8b63d1 Mon Sep 17 00:00:00 2001
From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Date: Tue, 23 Oct 2018 17:25:51 +1300
Subject: [PATCH 01/17] CVE-2018-14629 dns: CNAME loop prevention using counter
Count number of answers generated by internal DNS query routine and stop at
20 to match Microsoft's loop prevention mechanism.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13600
Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
---
python/samba/tests/dns.py | 22 ++++++++++++++++++++++
selftest/knownfail.d/dns | 6 ++++++
source4/dns_server/dns_query.c | 6 ++++++
3 files changed, 34 insertions(+)
diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
index 6771e3bb8c4..3e6306e2be8 100644
--- a/python/samba/tests/dns.py
+++ b/python/samba/tests/dns.py
@@ -844,6 +844,28 @@ class TestComplexQueries(DNSTest):
self.assertEquals(response.answers[1].name, name2)
self.assertEquals(response.answers[1].rdata, name0)
+ def test_cname_loop(self):
+ cname1 = "cnamelooptestrec." + self.get_dns_domain()
+ cname2 = "cnamelooptestrec2." + self.get_dns_domain()
+ cname3 = "cnamelooptestrec3." + self.get_dns_domain()
+ self.make_dns_update(cname1, cname2, dnsp.DNS_TYPE_CNAME)
+ self.make_dns_update(cname2, cname3, dnsp.DNS_TYPE_CNAME)
+ self.make_dns_update(cname3, cname1, dnsp.DNS_TYPE_CNAME)
+
+ p = self.make_name_packet(dns.DNS_OPCODE_QUERY)
+ questions = []
+
+ q = self.make_name_question(cname1,
+ dns.DNS_QTYPE_A,
+ dns.DNS_QCLASS_IN)
+ questions.append(q)
+ self.finish_name_packet(p, questions)
+
+ (response, response_packet) =\
+ self.dns_transaction_udp(p, host=self.server_ip)
+
+ max_recursion_depth = 20
+ self.assertEquals(len(response.answers), max_recursion_depth)
class TestInvalidQueries(DNSTest):
def setUp(self):
diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns
index a5176654cc2..a248432aafa 100644
--- a/selftest/knownfail.d/dns
+++ b/selftest/knownfail.d/dns
@@ -69,3 +69,9 @@ samba.tests.dns.__main__.TestSimpleQueries.test_qtype_all_query\(rodc:local\)
# The SOA override should not pass against the RODC, it must not overstamp
samba.tests.dns.__main__.TestSimpleQueries.test_one_SOA_query\(rodc:local\)
+
+#
+# rodc and vampire_dc require signed dns updates, so the test setup
+# fails, but the test does run on fl2003dc
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(rodc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(vampire_dc:local\)
diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c
index 923f7233eb9..65faeac3b6a 100644
--- a/source4/dns_server/dns_query.c
+++ b/source4/dns_server/dns_query.c
@@ -40,6 +40,7 @@
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_DNS
+#define MAX_Q_RECURSION_DEPTH 20
struct forwarder_string {
const char *forwarder;
@@ -419,6 +420,11 @@ static struct tevent_req *handle_dnsrpcrec_send(
state->answers = answers;
state->nsrecs = nsrecs;
+ if (talloc_array_length(*answers) >= MAX_Q_RECURSION_DEPTH) {
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
+
resolve_cname = ((rec->wType == DNS_TYPE_CNAME) &&
((question->question_type == DNS_QTYPE_A) ||
(question->question_type == DNS_QTYPE_AAAA)));
--
2.17.1

View File

@ -1,41 +0,0 @@
From b38900c353ca92365f144734c99d156cc39611d4 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 23 Oct 2018 17:33:46 +1300
Subject: [PATCH 3/5] CVE-2018-16841 heimdal: Fix segfault on PKINIT with
mis-matching principal
In Heimdal KRB5_KDC_ERR_CLIENT_NAME_MISMATCH is an enum, so we tried to double-free
mem_ctx.
This was introduced in 9a0263a7c316112caf0265237bfb2cfb3a3d370d for the
MIT KDC effort.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13628
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
source4/kdc/db-glue.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 8ccc34cd665..519060a5641 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -2606,10 +2606,10 @@ samba_kdc_check_pkinit_ms_upn_match(krb5_context context,
* comparison */
if (!(orig_sid && target_sid && dom_sid_equal(orig_sid, target_sid))) {
talloc_free(mem_ctx);
-#ifdef KRB5_KDC_ERR_CLIENT_NAME_MISMATCH /* Heimdal */
- return KRB5_KDC_ERR_CLIENT_NAME_MISMATCH;
-#elif defined(KRB5KDC_ERR_CLIENT_NAME_MISMATCH) /* MIT */
+#if defined(KRB5KDC_ERR_CLIENT_NAME_MISMATCH) /* MIT */
return KRB5KDC_ERR_CLIENT_NAME_MISMATCH;
+#else /* Heimdal (where this is an enum) */
+ return KRB5_KDC_ERR_CLIENT_NAME_MISMATCH;
#endif
}
--
2.11.0

View File

@ -1,40 +0,0 @@
From 58733073f6eb78e8b157ee55493e92ffa361b73c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 24 Oct 2018 15:41:28 +1300
Subject: [PATCH 4/5] CVE-2018-16841 selftest: Check for mismatching principal
in certficate compared with principal in AS-REQ
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13628
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
testprogs/blackbox/test_pkinit_heimdal.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/testprogs/blackbox/test_pkinit_heimdal.sh b/testprogs/blackbox/test_pkinit_heimdal.sh
index 0a13aa293e7..0912e0dbfe8 100755
--- a/testprogs/blackbox/test_pkinit_heimdal.sh
+++ b/testprogs/blackbox/test_pkinit_heimdal.sh
@@ -75,10 +75,18 @@ testit "STEP1 kinit with pkinit (name specified) " $samba4kinit $enctype --reque
testit "STEP1 kinit renew ticket (name specified)" $samba4kinit --request-pac -R || failed=`expr $failed + 1`
test_smbclient "STEP1 Test login with kerberos ccache (name specified)" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
+testit_expect_failure "STEP1 kinit with pkinit (wrong name specified) " $samba4kinit $enctype --request-pac --renewable $PKUSER not$USERNAME@$REALM || failed=`expr $failed + 1`
+
+testit_expect_failure "STEP1 kinit with pkinit (wrong name specified 2) " $samba4kinit $enctype --request-pac --renewable $PKUSER $SERVER@$REALM || failed=`expr $failed + 1`
+
testit "STEP1 kinit with pkinit (enterprise name specified)" $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise $USERNAME@$REALM || failed=`expr $failed + 1`
testit "STEP1 kinit renew ticket (enterprise name specified)" $samba4kinit --request-pac -R || failed=`expr $failed + 1`
test_smbclient "STEP1 Test login with kerberos ccache (enterprise name specified)" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
+testit_expect_failure "STEP1 kinit with pkinit (wrong enterprise name specified) " $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise not$USERNAME@$REALM || failed=`expr $failed + 1`
+
+testit_expect_failure "STEP1 kinit with pkinit (wrong enterprise name specified 2) " $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise $SERVER$@$REALM || failed=`expr $failed + 1`
+
testit "STEP1 kinit with pkinit (enterprise name in cert)" $samba4kinit $enctype --request-pac --renewable $PKUSER --pk-enterprise || failed=`expr $failed + 1`
testit "STEP1 kinit renew ticket (enterprise name in cert)" $samba4kinit --request-pac -R || failed=`expr $failed + 1`
test_smbclient "STEP1 Test login with kerberos ccache (enterprise name in cert)" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
--
2.11.0

View File

@ -1,43 +0,0 @@
From f33f52c366f7cf140f470de44579dcb7eb832629 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming@catalyst.net.nz>
Date: Mon, 5 Nov 2018 16:18:18 +1300
Subject: [PATCH 07/17] CVE-2018-16851 ldap_server: Check ret before
manipulating blob
In the case of hitting the talloc ~256MB limit, this causes a crash in
the server.
Note that you would actually need to load >256MB of data into the LDAP.
Although there is some generated/hidden data which would help you reach that
limit (descriptors and RMD blobs).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13674
Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
source4/ldap_server/ldap_server.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c
index b5251e3623e..bc2f54bc146 100644
--- a/source4/ldap_server/ldap_server.c
+++ b/source4/ldap_server/ldap_server.c
@@ -690,13 +690,13 @@ static void ldapsrv_call_writev_start(struct ldapsrv_call *call)
ret = data_blob_append(call, &blob, b.data, b.length);
data_blob_free(&b);
- talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet");
-
if (!ret) {
ldapsrv_terminate_connection(conn, "data_blob_append failed");
return;
}
+ talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet");
+
DLIST_REMOVE(call->replies, call->replies);
}
--
2.17.1

View File

@ -1,398 +0,0 @@
From f40e1b3b42ce23b574a4c530545ff8170ddc7330 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 6 Nov 2018 12:10:07 +1300
Subject: [PATCH 04/17] CVE-2018-16852 dcerpc dnsserver: Verification tests
Tests to verify
Bug 13669 - (CVE-2018-16852) NULL
pointer de-reference in Samba AD DC DNS management
The presence of the ZONE_MASTER_SERVERS property or the
ZONE_SCAVENGING_SERVERS property in a zone record causes the server to
follow a null pointer and terminate.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
---
selftest/knownfail.d/bug13669 | 4 +
.../tests/rpc_dns_server_dnsutils_test.c | 304 ++++++++++++++++++
source4/rpc_server/wscript_build | 17 +-
source4/selftest/tests.py | 2 +
4 files changed, 325 insertions(+), 2 deletions(-)
create mode 100644 selftest/knownfail.d/bug13669
create mode 100644 source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
diff --git a/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669
new file mode 100644
index 00000000000..74c8c130674
--- /dev/null
+++ b/selftest/knownfail.d/bug13669
@@ -0,0 +1,4 @@
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers
diff --git a/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
new file mode 100644
index 00000000000..89721135658
--- /dev/null
+++ b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
@@ -0,0 +1,304 @@
+/*
+ * Unit tests for source4/rpc_server/dnsserver/dnsutils.c
+ *
+ * Copyright (C) Catalyst.NET Ltd 2018
+ *
+ * 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 cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ *
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+
+#include "../dnsserver/dnsutils.c"
+
+
+/*
+ * Test setting of an empty ZONE_MASTER_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_master_servers_empty(void **state)
+{
+ struct dnsserver_zone *zone = NULL;
+ struct dnsserver_serverinfo *serverinfo = NULL;
+ struct dnsserver_zoneinfo *zoneinfo = NULL;
+ struct dnsp_DnsProperty *property = NULL;
+
+ TALLOC_CTX *ctx = talloc_new(NULL);
+
+ /*
+ * Setup the zone data
+ */
+ zone = talloc_zero(ctx, struct dnsserver_zone);
+ assert_non_null(zone);
+ zone->name = "test";
+
+ /*
+ * Set up an empty ZONE_MASTER_SERVERS property
+ */
+ property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+ assert_non_null(property);
+ property->id = DSPROPERTY_ZONE_MASTER_SERVERS;
+ property->data.master_servers.addrCount = 0;
+ property->data.master_servers.addr = NULL;
+
+ zone->tmp_props = property;
+ zone->num_props = 1;
+
+
+ /*
+ * Setup the server info
+ */
+ serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+ assert_non_null(serverinfo);
+
+ /*
+ * call dnsserver_init_zoneinfo
+ */
+ zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+ /*
+ * Check results
+ */
+ assert_non_null(zoneinfo);
+ assert_non_null(zoneinfo->aipLocalMasters);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 0);
+ assert_null(zoneinfo->aipLocalMasters->AddrArray);
+
+ TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of a non empty ZONE_MASTER_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_master_servers(void **state)
+{
+ struct dnsserver_zone *zone = NULL;
+ struct dnsserver_serverinfo *serverinfo = NULL;
+ struct dnsserver_zoneinfo *zoneinfo = NULL;
+ struct dnsp_DnsProperty *property = NULL;
+
+ TALLOC_CTX *ctx = talloc_new(NULL);
+
+ /*
+ * Setup the zone data
+ */
+ zone = talloc_zero(ctx, struct dnsserver_zone);
+ assert_non_null(zone);
+ zone->name = "test";
+
+ /*
+ * Set up an empty ZONE_MASTER_SERVERS property
+ */
+ property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+ assert_non_null(property);
+ property->id = DSPROPERTY_ZONE_MASTER_SERVERS;
+ property->data.master_servers.addrCount = 4;
+ property->data.master_servers.addr =
+ talloc_zero_array(ctx, uint32_t, 4);
+ property->data.master_servers.addr[0] = 1000;
+ property->data.master_servers.addr[1] = 1001;
+ property->data.master_servers.addr[2] = 1002;
+ property->data.master_servers.addr[3] = 1003;
+
+ zone->tmp_props = property;
+ zone->num_props = 1;
+
+
+ /*
+ * Setup the server info
+ */
+ serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+ assert_non_null(serverinfo);
+
+ /*
+ * call dnsserver_init_zoneinfo
+ */
+ zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+ /*
+ * Check results
+ */
+ assert_non_null(zoneinfo);
+ assert_non_null(zoneinfo->aipLocalMasters);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 4);
+ assert_non_null(zoneinfo->aipLocalMasters->AddrArray);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003);
+
+ /*
+ * Ensure that we're working with a copy of the property data
+ * and not a reference.
+ * The pointers should be different, and we should be able to change
+ * the values in the property without affecting the zoneinfo data
+ */
+ assert_true(zoneinfo->aipLocalMasters->AddrArray !=
+ property->data.master_servers.addr);
+ property->data.master_servers.addr[0] = 0;
+ property->data.master_servers.addr[1] = 1;
+ property->data.master_servers.addr[2] = 2;
+ property->data.master_servers.addr[3] = 3;
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002);
+ assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003);
+
+ TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of an empty ZONE_SCAVENGING_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_scavenging_servers_empty(void **state)
+{
+ struct dnsserver_zone *zone = NULL;
+ struct dnsserver_serverinfo *serverinfo = NULL;
+ struct dnsserver_zoneinfo *zoneinfo = NULL;
+ struct dnsp_DnsProperty *property = NULL;
+
+ TALLOC_CTX *ctx = talloc_new(NULL);
+
+ /*
+ * Setup the zone data
+ */
+ zone = talloc_zero(ctx, struct dnsserver_zone);
+ assert_non_null(zone);
+ zone->name = "test";
+
+ property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+ assert_non_null(property);
+ property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS;
+ property->data.servers.addrCount = 0;
+ property->data.servers.addr = NULL;
+
+ zone->tmp_props = property;
+ zone->num_props = 1;
+
+
+ serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+ assert_non_null(serverinfo);
+
+ zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+ assert_non_null(zoneinfo);
+ assert_non_null(zoneinfo->aipScavengeServers);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 0);
+ assert_null(zoneinfo->aipScavengeServers->AddrArray);
+
+ TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of a non empty ZONE_SCAVENGING_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_scavenging_servers(void **state)
+{
+ struct dnsserver_zone *zone = NULL;
+ struct dnsserver_serverinfo *serverinfo = NULL;
+ struct dnsserver_zoneinfo *zoneinfo = NULL;
+ struct dnsp_DnsProperty *property = NULL;
+
+ TALLOC_CTX *ctx = talloc_new(NULL);
+
+ /*
+ * Setup the zone data
+ */
+ zone = talloc_zero(ctx, struct dnsserver_zone);
+ assert_non_null(zone);
+ zone->name = "test";
+
+ property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+ assert_non_null(property);
+ property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS;
+ property->data.servers.addrCount = 4;
+ property->data.servers.addr = talloc_zero_array(ctx, uint32_t, 4);
+ property->data.servers.addr[0] = 1000;
+ property->data.servers.addr[1] = 1001;
+ property->data.servers.addr[2] = 1002;
+ property->data.servers.addr[3] = 1003;
+
+ zone->tmp_props = property;
+ zone->num_props = 1;
+
+
+ serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+ assert_non_null(serverinfo);
+
+ zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+ assert_non_null(zoneinfo);
+ assert_non_null(zoneinfo->aipScavengeServers);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 4);
+ assert_non_null(zoneinfo->aipScavengeServers->AddrArray);
+ assert_non_null(zoneinfo->aipScavengeServers->AddrArray);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003);
+
+ /*
+ * Ensure that we're working with a copy of the property data
+ * and not a reference.
+ * The pointers should be different, and we should be able to change
+ * the values in the property without affecting the zoneinfo data
+ */
+ assert_true(zoneinfo->aipScavengeServers->AddrArray !=
+ property->data.servers.addr);
+ property->data.servers.addr[0] = 0;
+ property->data.servers.addr[1] = 1;
+ property->data.servers.addr[2] = 2;
+ property->data.servers.addr[3] = 3;
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002);
+ assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003);
+
+
+ TALLOC_FREE(ctx);
+}
+int main(int argc, const char **argv)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(
+ test_dnsserver_init_zoneinfo_master_servers_empty),
+ cmocka_unit_test(
+ test_dnsserver_init_zoneinfo_master_servers),
+ cmocka_unit_test(
+ test_dnsserver_init_zoneinfo_scavenging_servers_empty),
+ cmocka_unit_test(
+ test_dnsserver_init_zoneinfo_scavenging_servers),
+ };
+
+ cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build
index 8e05eb8a0c3..14b68c7ce0f 100644
--- a/source4/rpc_server/wscript_build
+++ b/source4/rpc_server/wscript_build
@@ -3,7 +3,7 @@
bld.SAMBA_SUBSYSTEM('DCERPC_SHARE',
source='common/share_info.c',
autoproto='common/share.h',
- deps='ldb',
+ deps='ldb share',
enabled=bld.CONFIG_SET('WITH_NTVFS_FILESERVER'),
)
@@ -163,7 +163,7 @@ bld.SAMBA_MODULE('dcerpc_dnsserver',
source='dnsserver/dcerpc_dnsserver.c dnsserver/dnsutils.c dnsserver/dnsdata.c dnsserver/dnsdb.c',
subsystem='dcerpc_server',
init_function='dcerpc_server_dnsserver_init',
- deps='DCERPC_COMMON dnsserver_common'
+ deps='DCERPC_COMMON dnsserver_common netif'
)
@@ -176,3 +176,16 @@ bld.SAMBA_MODULE('service_dcerpc',
deps='dcerpc_server'
)
+if bld.CONFIG_GET('ENABLE_SELFTEST'):
+ bld.SAMBA_BINARY(
+ 'test_rpc_dns_server_dnsutils',
+ source='tests/rpc_dns_server_dnsutils_test.c',
+ deps='''
+ dnsserver_common
+ DCERPC_COMMON
+ cmocka
+ talloc
+ ''',
+ install=False,
+ enabled=bld.AD_DC_BUILD_IS_ENABLED()
+ )
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 9dec0adb05c..18b2c1162b0 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1164,3 +1164,5 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_util", "none",
[os.path.join(bindir(), "test_audit_util")])
plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log", "none",
[os.path.join(bindir(), "test_audit_log")])
+plantestsuite("samba4.dcerpc.dnsserver.dnsutils", "none",
+ [os.path.join(bindir(), "test_rpc_dns_server_dnsutils")])
--
2.17.1

View File

@ -1,124 +0,0 @@
From 05f867db81f118215445f2c49eda4b9c3451d14a Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 6 Nov 2018 12:16:30 +1300
Subject: [PATCH 05/17] CVE-2018-16852 dcerpc dnsserver: Ensure properties are
handled correctly
Fixes for
Bug 13669 - (CVE-2018-16852) NULL
pointer de-reference in Samba AD DC DNS management
The presence of the ZONE_MASTER_SERVERS property or the
ZONE_SCAVENGING_SERVERS property in a zone record causes the server to
follow a null pointer and terminate.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
selftest/knownfail.d/bug13669 | 4 --
source4/rpc_server/dnsserver/dnsutils.c | 64 +++++++++++++++++++++----
2 files changed, 56 insertions(+), 12 deletions(-)
delete mode 100644 selftest/knownfail.d/bug13669
diff --git a/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669
deleted file mode 100644
index 74c8c130674..00000000000
--- a/selftest/knownfail.d/bug13669
+++ /dev/null
@@ -1,4 +0,0 @@
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers
diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c
index a1c749074af..e4055c99e74 100644
--- a/source4/rpc_server/dnsserver/dnsutils.c
+++ b/source4/rpc_server/dnsserver/dnsutils.c
@@ -209,6 +209,46 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx,
}
+/*
+ * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
+ * The new structure and it's data are allocated on the supplied talloc context
+ */
+static struct IP4_ARRAY *copy_ip4_array(
+ TALLOC_CTX *ctx,
+ const char *name,
+ struct dnsp_ip4_array array) {
+
+ struct IP4_ARRAY *ip4_array = NULL;
+ unsigned int i;
+
+ ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
+ if (ip4_array == NULL) {
+ DBG_ERR("Out of memory copying property [%s]\n",
+ name);
+ return NULL;
+ }
+
+ ip4_array->AddrCount = array.addrCount;
+ if (ip4_array->AddrCount == 0) {
+ return ip4_array;
+ }
+
+ ip4_array->AddrArray = talloc_array(ip4_array, uint32_t,
+ ip4_array->AddrCount);
+ if (ip4_array->AddrArray == NULL) {
+ TALLOC_FREE(ip4_array);
+ DBG_ERR("Out of memory copying property [%s] values\n",
+ name);
+ return NULL;
+ }
+
+ for (i = 0; i < ip4_array->AddrCount; i++) {
+ ip4_array->AddrArray[i] = array.addr[i];
+ }
+
+ return ip4_array;
+}
+
struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
struct dnsserver_serverinfo *serverinfo)
{
@@ -309,20 +349,28 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
prop->aging_enabled;
break;
case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
- zoneinfo->aipScavengeServers->AddrCount =
- prop->servers.addrCount;
- zoneinfo->aipScavengeServers->AddrArray =
- prop->servers.addr;
+ zoneinfo->aipScavengeServers =
+ copy_ip4_array(zoneinfo,
+ "ZONE_SCAVENGING_SERVERS",
+ prop->servers);
+ if (zoneinfo->aipScavengeServers == NULL) {
+ TALLOC_FREE(zoneinfo);
+ return NULL;
+ }
break;
case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
zoneinfo->dwAvailForScavengeTime =
prop->next_scavenging_cycle_hours;
break;
case DSPROPERTY_ZONE_MASTER_SERVERS:
- zoneinfo->aipLocalMasters->AddrCount =
- prop->master_servers.addrCount;
- zoneinfo->aipLocalMasters->AddrArray =
- prop->master_servers.addr;
+ zoneinfo->aipLocalMasters =
+ copy_ip4_array(zoneinfo,
+ "ZONE_MASTER_SERVERS",
+ prop->master_servers);
+ if (zoneinfo->aipLocalMasters == NULL) {
+ TALLOC_FREE(zoneinfo);
+ return NULL;
+ }
break;
case DSPROPERTY_ZONE_EMPTY:
case DSPROPERTY_ZONE_SECURE_TIME:
--
2.17.1

View File

@ -1,328 +0,0 @@
From c78ca8b9b48a19e71f4d6ddd2e300f282fb0b247 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Wed, 7 Nov 2018 15:08:04 +1300
Subject: [PATCH 06/17] CVE-2018-16852 dcerpc dnsserver: refactor common
properties handling
dnsserver_common.c and dnsutils.c both share similar code to process
zone properties. This patch extracts the common code and moves it to
dnsserver_common.c.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13669
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
source4/dns_server/dnsserver_common.c | 129 +++++++++++++++++-------
source4/dns_server/dnsserver_common.h | 3 +
source4/rpc_server/dnsserver/dnsutils.c | 107 ++------------------
3 files changed, 104 insertions(+), 135 deletions(-)
diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index bbbfe920f4e..cc24a6c1b52 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -742,6 +742,94 @@ bool dns_name_is_static(struct dnsp_DnssrvRpcRecord *records,
return false;
}
+/*
+ * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
+ * The new structure and it's data are allocated on the supplied talloc context
+ */
+static struct IP4_ARRAY *copy_ip4_array(TALLOC_CTX *ctx,
+ const char *name,
+ struct dnsp_ip4_array array)
+{
+
+ struct IP4_ARRAY *ip4_array = NULL;
+ unsigned int i;
+
+ ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
+ if (ip4_array == NULL) {
+ DBG_ERR("Out of memory copying property [%s]\n", name);
+ return NULL;
+ }
+
+ ip4_array->AddrCount = array.addrCount;
+ if (ip4_array->AddrCount == 0) {
+ return ip4_array;
+ }
+
+ ip4_array->AddrArray =
+ talloc_array(ip4_array, uint32_t, ip4_array->AddrCount);
+ if (ip4_array->AddrArray == NULL) {
+ TALLOC_FREE(ip4_array);
+ DBG_ERR("Out of memory copying property [%s] values\n", name);
+ return NULL;
+ }
+
+ for (i = 0; i < ip4_array->AddrCount; i++) {
+ ip4_array->AddrArray[i] = array.addr[i];
+ }
+
+ return ip4_array;
+}
+
+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo,
+ struct dnsp_DnsProperty *prop)
+{
+ switch (prop->id) {
+ case DSPROPERTY_ZONE_TYPE:
+ zoneinfo->dwZoneType = prop->data.zone_type;
+ break;
+ case DSPROPERTY_ZONE_ALLOW_UPDATE:
+ zoneinfo->fAllowUpdate = prop->data.allow_update_flag;
+ break;
+ case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
+ zoneinfo->dwNoRefreshInterval = prop->data.norefresh_hours;
+ break;
+ case DSPROPERTY_ZONE_REFRESH_INTERVAL:
+ zoneinfo->dwRefreshInterval = prop->data.refresh_hours;
+ break;
+ case DSPROPERTY_ZONE_AGING_STATE:
+ zoneinfo->fAging = prop->data.aging_enabled;
+ break;
+ case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
+ zoneinfo->aipScavengeServers = copy_ip4_array(
+ zoneinfo, "ZONE_SCAVENGING_SERVERS", prop->data.servers);
+ if (zoneinfo->aipScavengeServers == NULL) {
+ return false;
+ }
+ break;
+ case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
+ zoneinfo->dwAvailForScavengeTime =
+ prop->data.next_scavenging_cycle_hours;
+ break;
+ case DSPROPERTY_ZONE_MASTER_SERVERS:
+ zoneinfo->aipLocalMasters = copy_ip4_array(
+ zoneinfo, "ZONE_MASTER_SERVERS", prop->data.master_servers);
+ if (zoneinfo->aipLocalMasters == NULL) {
+ return false;
+ }
+ break;
+ case DSPROPERTY_ZONE_EMPTY:
+ case DSPROPERTY_ZONE_SECURE_TIME:
+ case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
+ case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
+ case DSPROPERTY_ZONE_DCPROMO_CONVERT:
+ case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
+ case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
+ case DSPROPERTY_ZONE_NS_SERVERS_DA:
+ case DSPROPERTY_ZONE_NODE_DBFLAGS:
+ break;
+ }
+ return true;
+}
WERROR dns_get_zone_properties(struct ldb_context *samdb,
TALLOC_CTX *mem_ctx,
struct ldb_dn *zone_dn,
@@ -774,6 +862,7 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb,
}
for (i = 0; i < element->num_values; i++) {
+ bool valid_property;
prop = talloc_zero(mem_ctx, struct dnsp_DnsProperty);
if (prop == NULL) {
return WERR_NOT_ENOUGH_MEMORY;
@@ -787,42 +876,10 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb,
return DNS_ERR(SERVER_FAILURE);
}
- switch (prop->id) {
- case DSPROPERTY_ZONE_AGING_STATE:
- zoneinfo->fAging = prop->data.aging_enabled;
- break;
- case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
- zoneinfo->dwNoRefreshInterval =
- prop->data.norefresh_hours;
- break;
- case DSPROPERTY_ZONE_REFRESH_INTERVAL:
- zoneinfo->dwRefreshInterval = prop->data.refresh_hours;
- break;
- case DSPROPERTY_ZONE_ALLOW_UPDATE:
- zoneinfo->fAllowUpdate = prop->data.allow_update_flag;
- break;
- case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
- zoneinfo->dwAvailForScavengeTime =
- prop->data.next_scavenging_cycle_hours;
- break;
- case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
- zoneinfo->aipScavengeServers->AddrCount =
- prop->data.servers.addrCount;
- zoneinfo->aipScavengeServers->AddrArray =
- prop->data.servers.addr;
- break;
- case DSPROPERTY_ZONE_EMPTY:
- case DSPROPERTY_ZONE_TYPE:
- case DSPROPERTY_ZONE_SECURE_TIME:
- case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
- case DSPROPERTY_ZONE_MASTER_SERVERS:
- case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
- case DSPROPERTY_ZONE_DCPROMO_CONVERT:
- case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
- case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
- case DSPROPERTY_ZONE_NS_SERVERS_DA:
- case DSPROPERTY_ZONE_NODE_DBFLAGS:
- break;
+ valid_property =
+ dns_zoneinfo_load_zone_property(zoneinfo, prop);
+ if (!valid_property) {
+ return DNS_ERR(SERVER_FAILURE);
}
}
diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
index 380f61b8dbc..60ecde4fa91 100644
--- a/source4/dns_server/dnsserver_common.h
+++ b/source4/dns_server/dnsserver_common.h
@@ -87,4 +87,7 @@ NTSTATUS dns_common_zones(struct ldb_context *samdb,
TALLOC_CTX *mem_ctx,
struct ldb_dn *base_dn,
struct dns_server_zone **zones_ret);
+
+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo,
+ struct dnsp_DnsProperty *prop);
#endif /* __DNSSERVER_COMMON_H__ */
diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c
index e4055c99e74..bb52a1797a9 100644
--- a/source4/rpc_server/dnsserver/dnsutils.c
+++ b/source4/rpc_server/dnsserver/dnsutils.c
@@ -25,6 +25,7 @@
#include "dsdb/samdb/samdb.h"
#include "lib/socket/netif.h"
#include "lib/util/util_net.h"
+#include "dnsserver_common.h"
static struct DNS_ADDR_ARRAY *fill_dns_addr_array(TALLOC_CTX *mem_ctx,
struct loadparm_context *lp_ctx,
@@ -208,47 +209,6 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx,
return serverinfo;
}
-
-/*
- * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
- * The new structure and it's data are allocated on the supplied talloc context
- */
-static struct IP4_ARRAY *copy_ip4_array(
- TALLOC_CTX *ctx,
- const char *name,
- struct dnsp_ip4_array array) {
-
- struct IP4_ARRAY *ip4_array = NULL;
- unsigned int i;
-
- ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
- if (ip4_array == NULL) {
- DBG_ERR("Out of memory copying property [%s]\n",
- name);
- return NULL;
- }
-
- ip4_array->AddrCount = array.addrCount;
- if (ip4_array->AddrCount == 0) {
- return ip4_array;
- }
-
- ip4_array->AddrArray = talloc_array(ip4_array, uint32_t,
- ip4_array->AddrCount);
- if (ip4_array->AddrArray == NULL) {
- TALLOC_FREE(ip4_array);
- DBG_ERR("Out of memory copying property [%s] values\n",
- name);
- return NULL;
- }
-
- for (i = 0; i < ip4_array->AddrCount; i++) {
- ip4_array->AddrArray[i] = array.addr[i];
- }
-
- return ip4_array;
-}
-
struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
struct dnsserver_serverinfo *serverinfo)
{
@@ -257,8 +217,7 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
const char *revzone = "in-addr.arpa";
const char *revzone6 = "ip6.arpa";
int len1, len2;
- union dnsPropertyData *prop = NULL;
- int i=0;
+ unsigned int i = 0;
zoneinfo = talloc_zero(zone, struct dnsserver_zoneinfo);
if (zoneinfo == NULL) {
@@ -326,62 +285,12 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
zoneinfo->dwLastXfrResult = 0;
for(i=0; i<zone->num_props; i++){
- prop=&(zone->tmp_props[i].data);
- switch (zone->tmp_props[i].id) {
- case DSPROPERTY_ZONE_TYPE:
- zoneinfo->dwZoneType =
- prop->zone_type;
- break;
- case DSPROPERTY_ZONE_ALLOW_UPDATE:
- zoneinfo->fAllowUpdate =
- prop->allow_update_flag;
- break;
- case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
- zoneinfo->dwNoRefreshInterval =
- prop->norefresh_hours;
- break;
- case DSPROPERTY_ZONE_REFRESH_INTERVAL:
- zoneinfo->dwRefreshInterval =
- prop->refresh_hours;
- break;
- case DSPROPERTY_ZONE_AGING_STATE:
- zoneinfo->fAging =
- prop->aging_enabled;
- break;
- case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
- zoneinfo->aipScavengeServers =
- copy_ip4_array(zoneinfo,
- "ZONE_SCAVENGING_SERVERS",
- prop->servers);
- if (zoneinfo->aipScavengeServers == NULL) {
- TALLOC_FREE(zoneinfo);
- return NULL;
- }
- break;
- case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
- zoneinfo->dwAvailForScavengeTime =
- prop->next_scavenging_cycle_hours;
- break;
- case DSPROPERTY_ZONE_MASTER_SERVERS:
- zoneinfo->aipLocalMasters =
- copy_ip4_array(zoneinfo,
- "ZONE_MASTER_SERVERS",
- prop->master_servers);
- if (zoneinfo->aipLocalMasters == NULL) {
- TALLOC_FREE(zoneinfo);
- return NULL;
- }
- break;
- case DSPROPERTY_ZONE_EMPTY:
- case DSPROPERTY_ZONE_SECURE_TIME:
- case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
- case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
- case DSPROPERTY_ZONE_DCPROMO_CONVERT:
- case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
- case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
- case DSPROPERTY_ZONE_NS_SERVERS_DA:
- case DSPROPERTY_ZONE_NODE_DBFLAGS:
- break;
+ bool valid_property;
+ valid_property = dns_zoneinfo_load_zone_property(
+ zoneinfo, &zone->tmp_props[i]);
+ if (!valid_property) {
+ TALLOC_FREE(zoneinfo);
+ return NULL;
}
}
--
2.17.1

View File

@ -1,246 +0,0 @@
From 403c007b2309fe7ff264240cd3d07eb8a94a63f9 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Sat, 18 Aug 2018 15:32:43 +0300
Subject: [PATCH 1/5] CVE-2018-16853: Fix kinit test on system lacking
ldbsearch
By fixing bindir variable name.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13571
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
testprogs/blackbox/test_kinit_mit.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/testprogs/blackbox/test_kinit_mit.sh b/testprogs/blackbox/test_kinit_mit.sh
index dabf9915ed1..370542536e1 100755
--- a/testprogs/blackbox/test_kinit_mit.sh
+++ b/testprogs/blackbox/test_kinit_mit.sh
@@ -32,13 +32,13 @@ samba_enableaccount="$samba_tool user enable"
machineaccountccache="$samba_srcdir/scripting/bin/machineaccountccache"
ldbmodify="ldbmodify"
-if [ -x "$samba4bindir/ldbmodify" ]; then
- ldbmodify="$samba4bindir/ldbmodify"
+if [ -x "$samba_bindir/ldbmodify" ]; then
+ ldbmodify="$samba_bindir/ldbmodify"
fi
ldbsearch="ldbsearch"
-if [ -x "$samba4bindir/ldbsearch" ]; then
- ldbsearch="$samba4bindir/ldbsearch"
+if [ -x "$samba_bindir/ldbsearch" ]; then
+ ldbsearch="$samba_bindir/ldbsearch"
fi
. `dirname $0`/subunit.sh
--
2.19.1
From fbae2d0135b4ab998e771db2a8052574d7e34ad9 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Sat, 18 Aug 2018 00:40:30 +0300
Subject: [PATCH 2/5] CVE-2018-16853: The ticket in check_policy_as can
actually be a TGS
This happens when we are called from S4U2Self flow, and in that case
kdcreq->client is NULL. Use the name from client entry instead.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13571
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
source4/kdc/mit-kdb/kdb_samba_policies.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
index de5813bde2f..81ac73582e0 100644
--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
@@ -81,6 +81,7 @@ krb5_error_code kdb_samba_db_check_policy_as(krb5_context context,
char *netbios_name = NULL;
char *realm = NULL;
bool password_change = false;
+ krb5_const_principal client_princ;
DATA_BLOB int_data = { NULL, 0 };
krb5_data d;
krb5_pa_data **e_data;
@@ -90,7 +91,10 @@ krb5_error_code kdb_samba_db_check_policy_as(krb5_context context,
return KRB5_KDB_DBNOTINITED;
}
- if (ks_is_kadmin(context, kdcreq->client)) {
+ /* Prefer canonicalised name from client entry */
+ client_princ = client ? client->princ : kdcreq->client;
+
+ if (client_princ == NULL || ks_is_kadmin(context, client_princ)) {
return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN;
}
@@ -111,7 +115,7 @@ krb5_error_code kdb_samba_db_check_policy_as(krb5_context context,
goto done;
}
- code = krb5_unparse_name(context, kdcreq->client, &client_name);
+ code = krb5_unparse_name(context, client_princ, &client_name);
if (code) {
goto done;
}
--
2.19.1
From a49cb0d8b694d7cb579bf9b97208c7c1083be711 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Sat, 18 Aug 2018 16:01:59 +0300
Subject: [PATCH 3/5] CVE-2018-16853: Add a test to verify s4u2self doesn't
crash
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13571
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
testprogs/blackbox/test_kinit_mit.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/testprogs/blackbox/test_kinit_mit.sh b/testprogs/blackbox/test_kinit_mit.sh
index 370542536e1..f691b0f15d7 100755
--- a/testprogs/blackbox/test_kinit_mit.sh
+++ b/testprogs/blackbox/test_kinit_mit.sh
@@ -24,6 +24,7 @@ samba_srcdir="$SRCDIR/source4"
samba_kinit=kinit
samba_kdestroy=kdestroy
samba_kpasswd=kpasswd
+samba_kvno=kvno
samba_tool="$samba_bindir/samba-tool"
samba_texpect="$samba_bindir/texpect"
@@ -299,6 +300,17 @@ test_smbclient "Test machine account login with kerberos ccache" 'ls' -k yes ||
testit "reset password policies" $VALGRIND $samba_tool domain passwordsettings set $ADMIN_LDBMODIFY_CONFIG --complexity=default --history-length=default --min-pwd-length=default --min-pwd-age=default --max-pwd-age=default || failed=`expr $failed + 1`
+###########################################################
+### Test basic s4u2self request
+###########################################################
+
+# Use previous acquired machine creds to request a ticket for self.
+# We expect it to fail for now.
+MACHINE_ACCOUNT="$(hostname -s | tr [a-z] [A-Z])\$@$REALM"
+$samba_kvno -U$MACHINE_ACCOUNT $MACHINE_ACCOUNT
+# But we expect the KDC to be up and running still
+testit "kinit with machineaccountccache after s4u2self" $machineaccountccache $CONFIGURATION $KRB5CCNAME || failed=`expr $failed + 1`
+
### Cleanup
$samba_kdestroy
--
2.19.1
From 3e5ed4ad4a7ee1a42d4db73da35932d0acabe959 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Wed, 28 Sep 2016 07:22:32 +0200
Subject: [PATCH 4/5] CVE-2018-16853: Do not segfault if client is not set
This can be triggered with FAST but we don't support this yet.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13571
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
source4/kdc/mit-kdb/kdb_samba_policies.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
index 81ac73582e0..fc80329f221 100644
--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
@@ -461,6 +461,14 @@ void kdb_samba_db_audit_as_req(krb5_context context,
krb5_timestamp authtime,
krb5_error_code error_code)
{
+ /*
+ * FIXME: This segfaulted with a FAST test
+ * FIND_FAST: <unknown client> for <unknown server>, Unknown FAST armor type 0
+ */
+ if (client == NULL) {
+ return;
+ }
+
samba_bad_password_count(client, error_code);
/* TODO: perform proper audit logging for addresses */
@@ -473,6 +481,14 @@ void kdb_samba_db_audit_as_req(krb5_context context,
krb5_timestamp authtime,
krb5_error_code error_code)
{
+ /*
+ * FIXME: This segfaulted with a FAST test
+ * FIND_FAST: <unknown client> for <unknown server>, Unknown FAST armor type 0
+ */
+ if (client == NULL) {
+ return;
+ }
+
samba_bad_password_count(client, error_code);
}
#endif
--
2.19.1
From d67c462cd36ee525eb9122bd5d525d10eac7d06a Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Wed, 7 Nov 2018 22:53:35 +0200
Subject: [PATCH 5/5] CVE-2018-16853: fix crash in expired passowrd case
When calling encode_krb5_padata_sequence() make sure to
pass a null terminated array as required.
Fixes expired passowrd case in samba4.blackbox.kinit test.
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
source4/kdc/mit_samba.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c
index 414e67c6a98..eacca0903ec 100644
--- a/source4/kdc/mit_samba.c
+++ b/source4/kdc/mit_samba.c
@@ -865,7 +865,7 @@ krb5_error_code encode_krb5_padata_sequence(krb5_pa_data *const *rep, krb5_data
static void samba_kdc_build_edata_reply(NTSTATUS nt_status, DATA_BLOB *e_data)
{
krb5_error_code ret = 0;
- krb5_pa_data pa, *ppa = NULL;
+ krb5_pa_data pa, *ppa[2];
krb5_data *d = NULL;
if (!e_data)
@@ -886,9 +886,10 @@ static void samba_kdc_build_edata_reply(NTSTATUS nt_status, DATA_BLOB *e_data)
SIVAL(pa.contents, 4, 0);
SIVAL(pa.contents, 8, 1);
- ppa = &pa;
+ ppa[0] = &pa;
+ ppa[1] = NULL;
- ret = encode_krb5_padata_sequence(&ppa, &d);
+ ret = encode_krb5_padata_sequence(ppa, &d);
free(pa.contents);
if (ret) {
return;
--
2.19.1

View File

@ -1,54 +0,0 @@
From 4aabfecd290cd2769376abf7f170e832becc4112 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 6 Nov 2018 13:32:05 +1300
Subject: [PATCH 08/17] CVE-2018-16853 build: The Samba AD DC, when build with
MIT Kerberos is experimental
This matches https://wiki.samba.org/index.php/Running_a_Samba_AD_DC_with_MIT_Kerberos_KDC
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13678
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
wscript | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/wscript b/wscript
index 19fc6d12118..7c265e7befb 100644
--- a/wscript
+++ b/wscript
@@ -56,6 +56,14 @@ def set_options(opt):
help='build Samba with system MIT Kerberos. ' +
'You may specify list of paths where Kerberos is installed (e.g. /usr/local /usr/kerberos) to search krb5-config',
action='callback', callback=system_mitkrb5_callback, dest='with_system_mitkrb5', default=False)
+
+ opt.add_option('--with-experimental-mit-ad-dc',
+ help='Enable the experimental MIT Kerberos-backed AD DC. ' +
+ 'Note that security patches are not issued for this configuration',
+ action='store_true',
+ dest='with_experimental_mit_ad_dc',
+ default=False)
+
opt.add_option('--with-system-mitkdc',
help=('Specify the path to the krb5kdc binary from MIT Kerberos'),
type="string",
@@ -210,7 +218,16 @@ def configure(conf):
conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1)
if Options.options.with_system_mitkrb5:
+ if not Options.options.with_experimental_mit_ad_dc and \
+ not Options.options.without_ad_dc:
+ raise Utils.WafError('The MIT Kerberos build of Samba as an AD DC ' +
+ 'is experimental. Therefore '
+ '--with-system-mitkrb5 requires either ' +
+ '--with-experimental-mit-ad-dc or ' +
+ '--without-ad-dc')
+
conf.PROCESS_SEPARATE_RULE('system_mitkrb5')
+
if not (Options.options.without_ad_dc or Options.options.with_system_mitkrb5):
conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1)
--
2.17.1

View File

@ -1,68 +0,0 @@
From 862d4909eccd18942e3de8e8b0dc6e1594ec27f1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Sun, 2 Sep 2018 17:34:03 +1200
Subject: [PATCH 09/17] CVE-2018-16857 selftest: Prepare to allow override of
lockout duration in password_lockout tests
This will make it easier to avoid flapping tests.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
(cherry picked from commit a740a6131c967f9640b19a6964fd5d6f85ce853a)
Backported as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
source4/dsdb/tests/python/password_lockout.py | 9 ++++-----
source4/dsdb/tests/python/password_lockout_base.py | 11 +++++++++--
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index ec6cf13fe66..e817e656a2a 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -616,15 +616,14 @@ userPassword: thatsAcomplPASS2XYZ
initial_lastlogon_relation='greater')
def use_pso_lockout_settings(self, creds):
+
# create a PSO with the lockout settings the test cases normally expect
+ #
+ # Some test cases sleep() for self.account_lockout_duration
pso = PasswordSettings("lockout-PSO", self.ldb, lockout_attempts=3,
- lockout_duration=3)
+ lockout_duration=self.account_lockout_duration)
self.addCleanup(self.ldb.delete, pso.dn)
- # the test cases should sleep() for the PSO's lockoutDuration/obsvWindow
- self.account_lockout_duration = 3
- self.lockout_observation_window = 3
-
userdn = "cn=%s,cn=users,%s" % (creds.get_username(), self.base_dn)
pso.apply_to(userdn)
diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 4a320684702..9d82e088bb8 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -323,8 +323,15 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
""")
self.base_dn = self.ldb.domain_dn()
- self.account_lockout_duration = 3
- self.lockout_observation_window = 3
+
+ #
+ # Some test cases sleep() for self.account_lockout_duration
+ # so allow it to be controlled via the subclass
+ #
+ if not hasattr(self, 'account_lockout_duration'):
+ self.account_lockout_duration = 3
+ if not hasattr(self, 'lockout_observation_window'):
+ self.lockout_observation_window = 3
self.update_lockout_settings(threshold=3,
duration=self.account_lockout_duration,
observation_window=self.lockout_observation_window)
--
2.17.1

View File

@ -1,31 +0,0 @@
From 31198d39a76474d55c3d391e04d76758ee115d8e Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg@catalyst.net.nz>
Date: Mon, 30 Jul 2018 18:21:29 +1200
Subject: [PATCH 10/17] CVE-2018-16857 PEP8: fix E305: expected 2 blank lines
after class or function definition, found 1
Signed-off-by: Joe Guo <joeg@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Partial backport of commit 115f2a71b88 (only password_lockout.py
change) as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
source4/dsdb/tests/python/password_lockout.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index e817e656a2a..d8710866f39 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -1400,6 +1400,7 @@ userPassword: """ + userpass + """
self._test_samr_password_change(self.lockout1ntlm_creds,
other_creds=self.lockout2ntlm_creds)
+
host_url = "ldap://%s" % host
TestProgram(module=__name__, opts=subunitopts)
--
2.17.1

View File

@ -1,366 +0,0 @@
From 4d0fd1a421ad4a3ca19ed954ee91fcc36413b017 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Sun, 2 Sep 2018 18:03:06 +1200
Subject: [PATCH 11/17] CVE-2018-16857 selftest: Split up password_lockout into
tests with and without a call to sleep()
This means we can have a long observation window for many of the tests and
so make them much more reliable. Many of these cause frustrating flapping
failures in our CI systems.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon Sep 3 06:14:55 CEST 2018 on sn-devel-144
(cherry picked from commit 74357bf347348d3a8b7483c58e5250e98f7e8810)
Backported as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
source4/dsdb/tests/python/password_lockout.py | 299 +++++++++---------
1 file changed, 157 insertions(+), 142 deletions(-)
diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index d8710866f39..0022dee21ba 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -88,6 +88,42 @@ class PasswordTests(password_lockout_base.BasePasswordTestCase):
self.lockout2ntlm_ldb = self._readd_user(self.lockout2ntlm_creds,
lockOutObservationWindow=self.lockout_observation_window)
+
+ def use_pso_lockout_settings(self, creds):
+
+ # create a PSO with the lockout settings the test cases normally expect
+ #
+ # Some test cases sleep() for self.account_lockout_duration
+ pso = PasswordSettings("lockout-PSO", self.ldb, lockout_attempts=3,
+ lockout_duration=self.account_lockout_duration)
+ self.addCleanup(self.ldb.delete, pso.dn)
+
+ userdn = "cn=%s,cn=users,%s" % (creds.get_username(), self.base_dn)
+ pso.apply_to(userdn)
+
+ # update the global lockout settings to be wildly different to what
+ # the test cases normally expect
+ self.update_lockout_settings(threshold=10, duration=600,
+ observation_window=600)
+
+ def _reset_samr(self, res):
+
+ # Now reset the lockout, by removing ACB_AUTOLOCK (which removes the lock, despite being a generated attribute)
+ samr_user = self._open_samr_user(res)
+ acb_info = self.samr.QueryUserInfo(samr_user, 16)
+ acb_info.acct_flags &= ~samr.ACB_AUTOLOCK
+ self.samr.SetUserInfo(samr_user, 16, acb_info)
+ self.samr.Close(samr_user)
+
+
+class PasswordTestsWithoutSleep(PasswordTests):
+ def setUp(self):
+ # The tests in this class do not sleep, so we can have a
+ # longer window and not flap on slower hosts
+ self.account_lockout_duration = 30
+ self.lockout_observation_window = 30
+ super(PasswordTestsWithoutSleep, self).setUp()
+
def _reset_ldap_lockoutTime(self, res):
self.ldb.modify_ldif("""
dn: """ + str(res[0].dn) + """
@@ -615,22 +651,130 @@ userPassword: thatsAcomplPASS2XYZ
"samr",
initial_lastlogon_relation='greater')
- def use_pso_lockout_settings(self, creds):
+ def test_multiple_logon_krb5(self):
+ self._test_multiple_logon(self.lockout1krb5_creds)
- # create a PSO with the lockout settings the test cases normally expect
- #
- # Some test cases sleep() for self.account_lockout_duration
- pso = PasswordSettings("lockout-PSO", self.ldb, lockout_attempts=3,
- lockout_duration=self.account_lockout_duration)
- self.addCleanup(self.ldb.delete, pso.dn)
+ def test_multiple_logon_ntlm(self):
+ self._test_multiple_logon(self.lockout1ntlm_creds)
- userdn = "cn=%s,cn=users,%s" % (creds.get_username(), self.base_dn)
- pso.apply_to(userdn)
+ def _test_samr_password_change(self, creds, other_creds, lockout_threshold=3):
+ """Tests user lockout by using bad password in SAMR password_change"""
- # update the global lockout settings to be wildly different to what
- # the test cases normally expect
- self.update_lockout_settings(threshold=10, duration=600,
- observation_window=600)
+ # create a connection for SAMR using another user's credentials
+ lp = self.get_loadparm()
+ net = Net(other_creds, lp, server=self.host)
+
+ # work out the initial account values for this user
+ username = creds.get_username()
+ userdn = "cn=%s,cn=users,%s" % (username, self.base_dn)
+ res = self._check_account(userdn,
+ badPwdCount=0,
+ badPasswordTime=("greater", 0),
+ badPwdCountOnly=True)
+ badPasswordTime = int(res[0]["badPasswordTime"][0])
+ logonCount = int(res[0]["logonCount"][0])
+ lastLogon = int(res[0]["lastLogon"][0])
+ lastLogonTimestamp = int(res[0]["lastLogonTimestamp"][0])
+
+ # prove we can change the user password (using the correct password)
+ new_password = "thatsAcomplPASS2"
+ net.change_password(newpassword=new_password.encode('utf-8'),
+ username=username,
+ oldpassword=creds.get_password())
+ creds.set_password(new_password)
+
+ # try entering 'x' many bad passwords in a row to lock the user out
+ new_password = "thatsAcomplPASS3"
+ for i in range(lockout_threshold):
+ badPwdCount = i + 1
+ try:
+ print("Trying bad password, attempt #%u" % badPwdCount)
+ net.change_password(newpassword=new_password.encode('utf-8'),
+ username=creds.get_username(),
+ oldpassword="bad-password")
+ self.fail("Invalid SAMR change_password accepted")
+ except NTSTATUSError as e:
+ enum = ctypes.c_uint32(e[0]).value
+ self.assertEquals(enum, ntstatus.NT_STATUS_WRONG_PASSWORD)
+
+ # check the status of the account is updated after each bad attempt
+ account_flags = 0
+ lockoutTime = None
+ if badPwdCount >= lockout_threshold:
+ account_flags = dsdb.UF_LOCKOUT
+ lockoutTime = ("greater", badPasswordTime)
+
+ res = self._check_account(userdn,
+ badPwdCount=badPwdCount,
+ badPasswordTime=("greater", badPasswordTime),
+ logonCount=logonCount,
+ lastLogon=lastLogon,
+ lastLogonTimestamp=lastLogonTimestamp,
+ lockoutTime=lockoutTime,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
+ msDSUserAccountControlComputed=account_flags)
+ badPasswordTime = int(res[0]["badPasswordTime"][0])
+
+ # the user is now locked out
+ lockoutTime = int(res[0]["lockoutTime"][0])
+
+ # check the user remains locked out regardless of whether they use a
+ # good or a bad password now
+ for password in (creds.get_password(), "bad-password"):
+ try:
+ print("Trying password %s" % password)
+ net.change_password(newpassword=new_password.encode('utf-8'),
+ username=creds.get_username(),
+ oldpassword=password)
+ self.fail("Invalid SAMR change_password accepted")
+ except NTSTATUSError as e:
+ enum = ctypes.c_uint32(e[0]).value
+ self.assertEquals(enum, ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT)
+
+ res = self._check_account(userdn,
+ badPwdCount=lockout_threshold,
+ badPasswordTime=badPasswordTime,
+ logonCount=logonCount,
+ lastLogon=lastLogon,
+ lastLogonTimestamp=lastLogonTimestamp,
+ lockoutTime=lockoutTime,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
+ msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
+
+ # reset the user account lockout
+ self._reset_samr(res)
+
+ # check bad password counts are reset
+ res = self._check_account(userdn,
+ badPwdCount=0,
+ badPasswordTime=badPasswordTime,
+ logonCount=logonCount,
+ lockoutTime=0,
+ lastLogon=lastLogon,
+ lastLogonTimestamp=lastLogonTimestamp,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
+ msDSUserAccountControlComputed=0)
+
+ # check we can change the user password successfully now
+ net.change_password(newpassword=new_password.encode('utf-8'),
+ username=username,
+ oldpassword=creds.get_password())
+ creds.set_password(new_password)
+
+ def test_samr_change_password(self):
+ self._test_samr_password_change(self.lockout1ntlm_creds,
+ other_creds=self.lockout2ntlm_creds)
+
+ # same as above, but use a PSO to enforce the lockout
+ def test_pso_samr_change_password(self):
+ self.use_pso_lockout_settings(self.lockout1ntlm_creds)
+ self._test_samr_password_change(self.lockout1ntlm_creds,
+ other_creds=self.lockout2ntlm_creds)
+
+
+class PasswordTestsWithSleep(PasswordTests):
+ def setUp(self):
+ super(PasswordTestsWithSleep, self).setUp()
def _test_unicodePwd_lockout_with_clear_change(self, creds, other_ldb,
initial_logoncount_relation=None):
@@ -1065,12 +1209,6 @@ unicodePwd:: """ + base64.b64encode(new_utf16).decode('utf8') + """
self.use_pso_lockout_settings(self.lockout1ntlm_creds)
self._test_login_lockout(self.lockout1ntlm_creds)
- def test_multiple_logon_krb5(self):
- self._test_multiple_logon(self.lockout1krb5_creds)
-
- def test_multiple_logon_ntlm(self):
- self._test_multiple_logon(self.lockout1ntlm_creds)
-
def _testing_add_user(self, creds, lockOutObservationWindow=0):
username = creds.get_username()
userpass = creds.get_password()
@@ -1251,15 +1389,6 @@ userPassword: """ + userpass + """
msDSUserAccountControlComputed=0)
return ldb
- def _reset_samr(self, res):
-
- # Now reset the lockout, by removing ACB_AUTOLOCK (which removes the lock, despite being a generated attribute)
- samr_user = self._open_samr_user(res)
- acb_info = self.samr.QueryUserInfo(samr_user, 16)
- acb_info.acct_flags &= ~samr.ACB_AUTOLOCK
- self.samr.SetUserInfo(samr_user, 16, acb_info)
- self.samr.Close(samr_user)
-
def test_lockout_observation_window(self):
lockout3krb5_creds = self.insta_creds(self.template_creds,
username="lockout3krb5",
@@ -1286,120 +1415,6 @@ userPassword: """ + userpass + """
self._testing_add_user(lockout4ntlm_creds,
lockOutObservationWindow=self.lockout_observation_window)
- def _test_samr_password_change(self, creds, other_creds, lockout_threshold=3):
- """Tests user lockout by using bad password in SAMR password_change"""
-
- # create a connection for SAMR using another user's credentials
- lp = self.get_loadparm()
- net = Net(other_creds, lp, server=self.host)
-
- # work out the initial account values for this user
- username = creds.get_username()
- userdn = "cn=%s,cn=users,%s" % (username, self.base_dn)
- res = self._check_account(userdn,
- badPwdCount=0,
- badPasswordTime=("greater", 0),
- badPwdCountOnly=True)
- badPasswordTime = int(res[0]["badPasswordTime"][0])
- logonCount = int(res[0]["logonCount"][0])
- lastLogon = int(res[0]["lastLogon"][0])
- lastLogonTimestamp = int(res[0]["lastLogonTimestamp"][0])
-
- # prove we can change the user password (using the correct password)
- new_password = "thatsAcomplPASS2"
- net.change_password(newpassword=new_password.encode('utf-8'),
- username=username,
- oldpassword=creds.get_password())
- creds.set_password(new_password)
-
- # try entering 'x' many bad passwords in a row to lock the user out
- new_password = "thatsAcomplPASS3"
- for i in range(lockout_threshold):
- badPwdCount = i + 1
- try:
- print("Trying bad password, attempt #%u" % badPwdCount)
- net.change_password(newpassword=new_password.encode('utf-8'),
- username=creds.get_username(),
- oldpassword="bad-password")
- self.fail("Invalid SAMR change_password accepted")
- except NTSTATUSError as e:
- enum = ctypes.c_uint32(e[0]).value
- self.assertEquals(enum, ntstatus.NT_STATUS_WRONG_PASSWORD)
-
- # check the status of the account is updated after each bad attempt
- account_flags = 0
- lockoutTime = None
- if badPwdCount >= lockout_threshold:
- account_flags = dsdb.UF_LOCKOUT
- lockoutTime = ("greater", badPasswordTime)
-
- res = self._check_account(userdn,
- badPwdCount=badPwdCount,
- badPasswordTime=("greater", badPasswordTime),
- logonCount=logonCount,
- lastLogon=lastLogon,
- lastLogonTimestamp=lastLogonTimestamp,
- lockoutTime=lockoutTime,
- userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
- msDSUserAccountControlComputed=account_flags)
- badPasswordTime = int(res[0]["badPasswordTime"][0])
-
- # the user is now locked out
- lockoutTime = int(res[0]["lockoutTime"][0])
-
- # check the user remains locked out regardless of whether they use a
- # good or a bad password now
- for password in (creds.get_password(), "bad-password"):
- try:
- print("Trying password %s" % password)
- net.change_password(newpassword=new_password.encode('utf-8'),
- username=creds.get_username(),
- oldpassword=password)
- self.fail("Invalid SAMR change_password accepted")
- except NTSTATUSError as e:
- enum = ctypes.c_uint32(e[0]).value
- self.assertEquals(enum, ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT)
-
- res = self._check_account(userdn,
- badPwdCount=lockout_threshold,
- badPasswordTime=badPasswordTime,
- logonCount=logonCount,
- lastLogon=lastLogon,
- lastLogonTimestamp=lastLogonTimestamp,
- lockoutTime=lockoutTime,
- userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
- msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
-
- # reset the user account lockout
- self._reset_samr(res)
-
- # check bad password counts are reset
- res = self._check_account(userdn,
- badPwdCount=0,
- badPasswordTime=badPasswordTime,
- logonCount=logonCount,
- lockoutTime=0,
- lastLogon=lastLogon,
- lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
- msDSUserAccountControlComputed=0)
-
- # check we can change the user password successfully now
- net.change_password(newpassword=new_password.encode('utf-8'),
- username=username,
- oldpassword=creds.get_password())
- creds.set_password(new_password)
-
- def test_samr_change_password(self):
- self._test_samr_password_change(self.lockout1ntlm_creds,
- other_creds=self.lockout2ntlm_creds)
-
- # same as above, but use a PSO to enforce the lockout
- def test_pso_samr_change_password(self):
- self.use_pso_lockout_settings(self.lockout1ntlm_creds)
- self._test_samr_password_change(self.lockout1ntlm_creds,
- other_creds=self.lockout2ntlm_creds)
-
host_url = "ldap://%s" % host
--
2.17.1

View File

@ -1,183 +0,0 @@
From fe8e05a9ea8185325ff87ac73ef0106a85cd662a Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg@catalyst.net.nz>
Date: Mon, 30 Jul 2018 18:15:34 +1200
Subject: [PATCH 12/17] CVE-2018-16857 PEP8: fix E127: continuation line
over-indented for visual indent
Signed-off-by: Joe Guo <joeg@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Partial backport of commit bbb9f57603d (only password_lockout_base.py
change) as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
.../tests/python/password_lockout_base.py | 36 +++++++++----------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 9d82e088bb8..1b408c75166 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -390,7 +390,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=("greater", 0),
lastLogonTimestamp=("greater", 0),
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
logonCount = int(res[0]["logonCount"][0])
@@ -421,7 +421,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg='lastlogontimestamp with wrong password')
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -440,7 +440,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=('greater', lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg='LLTimestamp is updated to lastlogon')
@@ -461,7 +461,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -483,7 +483,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -508,7 +508,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=("greater", badPasswordTime),
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
badPasswordTime = int(res[0]["badPasswordTime"][0])
lockoutTime = int(res[0]["lockoutTime"][0])
@@ -530,7 +530,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=lockoutTime,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
# The wrong password
@@ -550,7 +550,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=lockoutTime,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
# The correct password, but we are locked out
@@ -570,7 +570,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=lockoutTime,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
# wait for the lockout to end
@@ -585,7 +585,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
# The correct password after letting the timeout expire
@@ -605,7 +605,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=0,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg="lastLogon is way off")
@@ -629,7 +629,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -650,7 +650,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -664,7 +664,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
# The wrong password
@@ -684,7 +684,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -700,7 +700,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=("greater", lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
def _test_multiple_logon(self, creds):
@@ -734,7 +734,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=("greater", 0),
lastLogonTimestamp=("greater", 0),
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
logonCount = int(res[0]["logonCount"][0])
@@ -774,5 +774,5 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=(lastlogon_relation, lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
--
2.17.1

View File

@ -1,221 +0,0 @@
From 9cb6b4e9131afac71a39a2f6a3c142723cb6ca19 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg@catalyst.net.nz>
Date: Mon, 30 Jul 2018 18:19:21 +1200
Subject: [PATCH 13/17] CVE-2018-16857 PEP8: fix E251: unexpected spaces around
keyword / parameter equals
Signed-off-by: Joe Guo <joeg@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Partial backport of commit 1ccc36b4010cd63 (only password_lockout_base.py
change) as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
.../tests/python/password_lockout_base.py | 60 +++++++------------
1 file changed, 20 insertions(+), 40 deletions(-)
diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 1b408c75166..48a74018624 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -93,8 +93,7 @@ class BasePasswordTestCase(PasswordTestCase):
logonCount=0,
lastLogon=0,
lastLogonTimestamp=("absent", None),
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
def _check_account(self, dn,
@@ -389,8 +388,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=(logoncount_relation, 0),
lastLogon=("greater", 0),
lastLogonTimestamp=("greater", 0),
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
logonCount = int(res[0]["logonCount"][0])
@@ -420,8 +418,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=logonCount,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg='lastlogontimestamp with wrong password')
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -439,8 +436,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=(logoncount_relation, logonCount),
lastLogon=('greater', lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg='LLTimestamp is updated to lastlogon')
@@ -460,8 +456,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=logonCount,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -482,8 +477,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=logonCount,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -507,8 +501,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=("greater", badPasswordTime),
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
badPasswordTime = int(res[0]["badPasswordTime"][0])
lockoutTime = int(res[0]["lockoutTime"][0])
@@ -529,8 +522,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=lockoutTime,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
# The wrong password
@@ -549,8 +541,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=lockoutTime,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
# The correct password, but we are locked out
@@ -569,8 +560,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=lockoutTime,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
# wait for the lockout to end
@@ -584,8 +574,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lockoutTime=lockoutTime,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
# The correct password after letting the timeout expire
@@ -604,8 +593,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lastLogon=(lastlogon_relation, lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
lockoutTime=0,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg="lastLogon is way off")
@@ -628,8 +616,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lockoutTime=0,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -649,8 +636,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lockoutTime=0,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -663,8 +649,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lockoutTime=0,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
# The wrong password
@@ -683,8 +668,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lockoutTime=0,
lastLogon=lastLogon,
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -699,8 +683,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
lockoutTime=0,
lastLogon=("greater", lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
def _test_multiple_logon(self, creds):
@@ -733,8 +716,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=(logoncount_relation, 0),
lastLogon=("greater", 0),
lastLogonTimestamp=("greater", 0),
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
badPasswordTime = int(res[0]["badPasswordTime"][0])
logonCount = int(res[0]["logonCount"][0])
@@ -754,8 +736,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=(logoncount_relation, logonCount),
lastLogon=(lastlogon_relation, lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0,
msg=("second logon, firstlogon was %s" %
firstLogon))
@@ -773,6 +754,5 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
logonCount=(logoncount_relation, logonCount),
lastLogon=(lastlogon_relation, lastLogon),
lastLogonTimestamp=lastLogonTimestamp,
- userAccountControl=
- dsdb.UF_NORMAL_ACCOUNT,
+ userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=0)
--
2.17.1

View File

@ -1,104 +0,0 @@
From ec9cc4ed5a05490297cde3fcaac50eeeaaca8469 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 11:49:56 +1300
Subject: [PATCH 14/17] CVE-2018-16857 tests: Sanity-check password lockout
works with default values
Sanity-check that when we use the default lockOutObservationWindow that
user lockout actually works.
The easiest way to do this is to reuse the _test_login_lockout()
test-case, but stop at the point where we wait for the lockout duration
to expire (because we don't want the test to wait 30 mins).
This highlights a problem currently where the default values don't work.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
selftest/knownfail.d/password_lockout | 4 +++
source4/dsdb/tests/python/password_lockout.py | 30 +++++++++++++++++++
.../tests/python/password_lockout_base.py | 6 +++-
3 files changed, 39 insertions(+), 1 deletion(-)
create mode 100644 selftest/knownfail.d/password_lockout
diff --git a/selftest/knownfail.d/password_lockout b/selftest/knownfail.d/password_lockout
new file mode 100644
index 00000000000..305bcbdef25
--- /dev/null
+++ b/selftest/knownfail.d/password_lockout
@@ -0,0 +1,4 @@
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_krb5\(ad_dc_ntvfs\)
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_ntlm\(ad_dc_ntvfs\)
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_ntlm\(ad_dc_ntvfs\)
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_krb5\(ad_dc_ntvfs\)
diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index 0022dee21ba..b09a732e179 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -1415,6 +1415,36 @@ userPassword: """ + userpass + """
self._testing_add_user(lockout4ntlm_creds,
lockOutObservationWindow=self.lockout_observation_window)
+class PasswordTestsWithDefaults(PasswordTests):
+ def setUp(self):
+ # The tests in this class do not sleep, so we can use the default
+ # timeout windows here
+ self.account_lockout_duration = 30 * 60
+ self.lockout_observation_window = 30 * 60
+ super(PasswordTestsWithDefaults, self).setUp()
+
+ # sanity-check that user lockout works with the default settings (we just
+ # check the user is locked out - we don't wait for the lockout to expire)
+ def test_login_lockout_krb5(self):
+ self._test_login_lockout(self.lockout1krb5_creds,
+ wait_lockout_duration=False)
+
+ def test_login_lockout_ntlm(self):
+ self._test_login_lockout(self.lockout1ntlm_creds,
+ wait_lockout_duration=False)
+
+ # Repeat the login lockout tests using PSOs
+ def test_pso_login_lockout_krb5(self):
+ """Check the PSO lockout settings get applied to the user correctly"""
+ self.use_pso_lockout_settings(self.lockout1krb5_creds)
+ self._test_login_lockout(self.lockout1krb5_creds,
+ wait_lockout_duration=False)
+
+ def test_pso_login_lockout_ntlm(self):
+ """Check the PSO lockout settings get applied to the user correctly"""
+ self.use_pso_lockout_settings(self.lockout1ntlm_creds)
+ self._test_login_lockout(self.lockout1ntlm_creds,
+ wait_lockout_duration=False)
host_url = "ldap://%s" % host
diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 48a74018624..e8ac46dcd97 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -365,7 +365,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
def tearDown(self):
super(BasePasswordTestCase, self).tearDown()
- def _test_login_lockout(self, creds):
+ def _test_login_lockout(self, creds, wait_lockout_duration=True):
username = creds.get_username()
userpass = creds.get_password()
userdn = "cn=%s,cn=users,%s" % (username, self.base_dn)
@@ -563,6 +563,10 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
+ # if we're just checking the user gets locked out, we can stop here
+ if not wait_lockout_duration:
+ return
+
# wait for the lockout to end
time.sleep(self.account_lockout_duration + 1)
print(self.account_lockout_duration + 1)
--
2.17.1

View File

@ -1,58 +0,0 @@
From 4f86beeaf3408383385ee99a74520a805dd63c0f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 12:24:16 +1300
Subject: [PATCH 15/17] CVE-2018-16857 dsdb/util: Correctly treat
lockOutObservationWindow as 64-bit int
Commit 442a38c918ae1666b35 refactored some code into a new
get_lockout_observation_window() function. However, in moving the code,
an ldb_msg_find_attr_as_int64() inadvertently got converted to a
ldb_msg_find_attr_as_int().
ldb_msg_find_attr_as_int() will only work for values up to -2147483648
(about 3.5 minutes in MS timestamp form). Unfortunately, the automated
tests used a low enough timeout that they still worked, however,
password lockout would not work with the Samba default settings.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
selftest/knownfail.d/password_lockout | 2 --
source4/dsdb/common/util.c | 10 +++++-----
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/selftest/knownfail.d/password_lockout b/selftest/knownfail.d/password_lockout
index 305bcbdef25..a4e37a84c21 100644
--- a/selftest/knownfail.d/password_lockout
+++ b/selftest/knownfail.d/password_lockout
@@ -1,4 +1,2 @@
samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_krb5\(ad_dc_ntvfs\)
samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_ntlm\(ad_dc_ntvfs\)
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_ntlm\(ad_dc_ntvfs\)
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_krb5\(ad_dc_ntvfs\)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 193fa2ae653..438a29e1773 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -5400,12 +5400,12 @@ static int64_t get_lockout_observation_window(struct ldb_message *domain_msg,
struct ldb_message *pso_msg)
{
if (pso_msg != NULL) {
- return ldb_msg_find_attr_as_int(pso_msg,
- "msDS-LockoutObservationWindow",
- 0);
+ return ldb_msg_find_attr_as_int64(pso_msg,
+ "msDS-LockoutObservationWindow",
+ 0);
} else {
- return ldb_msg_find_attr_as_int(domain_msg,
- "lockOutObservationWindow", 0);
+ return ldb_msg_find_attr_as_int64(domain_msg,
+ "lockOutObservationWindow", 0);
}
}
--
2.17.1

View File

@ -1,47 +0,0 @@
From d12b02c78842786969557b9be7c953e9594d90dd Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 13:19:04 +1300
Subject: [PATCH 16/17] CVE-2018-16857 dsdb/util: Fix lockOutObservationWindow
for PSOs
Fix a remaining place where we were trying to read the
msDS-LockoutObservationWindow as an int instead of an int64.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
selftest/knownfail.d/password_lockout | 2 --
source4/dsdb/common/util.c | 6 +++---
2 files changed, 3 insertions(+), 5 deletions(-)
delete mode 100644 selftest/knownfail.d/password_lockout
diff --git a/selftest/knownfail.d/password_lockout b/selftest/knownfail.d/password_lockout
deleted file mode 100644
index a4e37a84c21..00000000000
--- a/selftest/knownfail.d/password_lockout
+++ /dev/null
@@ -1,2 +0,0 @@
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_krb5\(ad_dc_ntvfs\)
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_ntlm\(ad_dc_ntvfs\)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 438a29e1773..8d863f85a29 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -5361,9 +5361,9 @@ int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb,
if (res != NULL) {
lockOutObservationWindow =
- ldb_msg_find_attr_as_int(res->msgs[0],
- "msDS-LockoutObservationWindow",
- 0);
+ ldb_msg_find_attr_as_int64(res->msgs[0],
+ "msDS-LockoutObservationWindow",
+ 0);
talloc_free(res);
} else {
--
2.17.1

View File

@ -1,59 +0,0 @@
From 60b2cd50f4d0554cc5ca8c53b2d1fa89e56a6d06 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 13:22:41 +1300
Subject: [PATCH 17/17] CVE-2018-16857 dsdb/util: Add better default
lockOutObservationWindow
Clearly the lockOutObservationWindow value is important, and using a
default value of zero doesn't work very well.
This patch adds a better default value (the domain default setting of 30
minutes).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
source4/dsdb/common/util.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 8d863f85a29..18f700370a3 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -56,6 +56,9 @@
*/
#include "dsdb/samdb/ldb_modules/util.h"
+/* default is 30 minutes: -1e7 * 30 * 60 */
+#define DEFAULT_OBSERVATION_WINDOW -18000000000
+
/*
search the sam for the specified attributes in a specific domain, filter on
objectSid being in domain_sid.
@@ -5363,7 +5366,7 @@ int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb,
lockOutObservationWindow =
ldb_msg_find_attr_as_int64(res->msgs[0],
"msDS-LockoutObservationWindow",
- 0);
+ DEFAULT_OBSERVATION_WINDOW);
talloc_free(res);
} else {
@@ -5402,10 +5405,11 @@ static int64_t get_lockout_observation_window(struct ldb_message *domain_msg,
if (pso_msg != NULL) {
return ldb_msg_find_attr_as_int64(pso_msg,
"msDS-LockoutObservationWindow",
- 0);
+ DEFAULT_OBSERVATION_WINDOW);
} else {
return ldb_msg_find_attr_as_int64(domain_msg,
- "lockOutObservationWindow", 0);
+ "lockOutObservationWindow",
+ DEFAULT_OBSERVATION_WINDOW);
}
}
--
2.17.1

View File

@ -1,36 +0,0 @@
From 43958af1d50f0185e21e6cd74110c455ee8996af Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Wed, 30 Jan 2019 23:49:07 +0200
Subject: [PATCH] CVE-2018-16860 Heimdal KDC: Reject PA-S4U2Self with unkeyed
checksum
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13685
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(master): Tue May 14 11:45:13 UTC 2019 on sn-devel-184
---
source4/heimdal/kdc/krb5tgs.c | 7 +
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index a888788bb6f..ff7d93138c0 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1925,6 +1925,13 @@ tgs_build_reply(krb5_context context,
goto out;
}
+ if (!krb5_checksum_is_keyed(context, self.cksum.cksumtype)) {
+ free_PA_S4U2Self(&self);
+ kdc_log(context, config, 0, "Reject PA-S4U2Self with unkeyed checksum");
+ ret = KRB5KRB_AP_ERR_INAPP_CKSUM;
+ goto out;
+ }
+
ret = _krb5_s4u2self_to_checksumdata(context, &self, &datack);
if (ret)
goto out;

View File

@ -1,75 +0,0 @@
From 5e94fe726e9af81374c697ce603b3728ccaaebf3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Fri, 12 Jul 2019 12:10:35 -0700
Subject: [PATCH 1/6] CVE-2019-10197: smbd: separate out impersonation debug
info into a new function.
Will be called on elsewhere on successful impersonation.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035
Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
source3/smbd/uid.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index a4bcb747d37e..ce8e8d92131c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -279,6 +279,28 @@ static bool check_user_ok(connection_struct *conn,
return(True);
}
+static void print_impersonation_info(connection_struct *conn)
+{
+ struct smb_filename *cwdfname = NULL;
+
+ if (!CHECK_DEBUGLVL(DBGLVL_INFO)) {
+ return;
+ }
+
+ cwdfname = vfs_GetWd(talloc_tos(), conn);
+ if (cwdfname == NULL) {
+ return;
+ }
+
+ DBG_INFO("Impersonated user: uid=(%d,%d), gid=(%d,%d), cwd=[%s]\n",
+ (int)getuid(),
+ (int)geteuid(),
+ (int)getgid(),
+ (int)getegid(),
+ cwdfname->base_name);
+ TALLOC_FREE(cwdfname);
+}
+
/****************************************************************************
Become the user of a connection number without changing the security context
stack, but modify the current_user entries.
@@ -415,20 +437,7 @@ static bool change_to_user_internal(connection_struct *conn,
current_user.done_chdir = true;
}
- if (CHECK_DEBUGLVL(DBGLVL_INFO)) {
- struct smb_filename *cwdfname = vfs_GetWd(talloc_tos(), conn);
- if (cwdfname == NULL) {
- return false;
- }
- DBG_INFO("Impersonated user: uid=(%d,%d), gid=(%d,%d), cwd=[%s]\n",
- (int)getuid(),
- (int)geteuid(),
- (int)getgid(),
- (int)getegid(),
- cwdfname->base_name);
- TALLOC_FREE(cwdfname);
- }
-
+ print_impersonation_info(conn);
return true;
}
--
2.17.1

View File

@ -1,36 +0,0 @@
From b4cd0dcbc38ae61cfb075e5f659384df889e99f7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 11 Jul 2019 17:01:29 +0200
Subject: [PATCH 2/6] CVE-2019-10197: smbd: make sure that
change_to_user_internal() always resets current_user.done_chdir
We should not leave current_user.done_chdir as true if we didn't call
chdir_current_service() with success.
This caused problems in when calling vfs_ChDir() in pop_conn_ctx() when
chdir_current_service() worked once on one share but later failed on another
share.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---
source3/smbd/uid.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index ce8e8d92131c..77a81f602988 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -427,6 +427,7 @@ static bool change_to_user_internal(connection_struct *conn,
current_user.conn = conn;
current_user.vuid = vuid;
current_user.need_chdir = conn->tcon_done;
+ current_user.done_chdir = false;
if (current_user.need_chdir) {
ok = chdir_current_service(conn);
--
2.17.1

View File

@ -1,30 +0,0 @@
From b1496ce793129302c9959ebc6330219c6a3143f0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 18 Jun 2019 14:04:08 +0200
Subject: [PATCH 3/6] CVE-2019-10197: smbd: make sure we reset
current_user.{need,done}_chdir in become_root()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
source3/smbd/uid.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 77a81f602988..50868ba8572a 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -624,6 +624,9 @@ void smbd_become_root(void)
}
push_conn_ctx();
set_root_sec_ctx();
+
+ current_user.need_chdir = false;
+ current_user.done_chdir = false;
}
/* Unbecome the root user */
--
2.17.1

View File

@ -1,49 +0,0 @@
From 03a0719d6d5c1a81b44bc3cedc76563a1eb04491 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 30 Jul 2019 17:16:59 +0200
Subject: [PATCH 4/6] CVE-2019-10197: selftest: make fsrvp_share its own
independent subdirectory
The next patch will otherwise break the fsrvp related tests.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
selftest/target/Samba3.pm | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 9d88253c9fe7..f7eb314138a0 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1540,6 +1540,9 @@ sub provision($$$$$$$$$)
my $widelinks_linkdir="$shrdir/widelinks_foo";
push(@dirs,$widelinks_linkdir);
+ my $fsrvp_shrdir="$shrdir/fsrvp";
+ push(@dirs,$fsrvp_shrdir);
+
my $shadow_tstdir="$shrdir/shadow";
push(@dirs,$shadow_tstdir);
my $shadow_mntdir="$shadow_tstdir/mount";
@@ -2083,14 +2086,14 @@ sub provision($$$$$$$$$)
guest ok = yes
[fsrvp_share]
- path = $shrdir
+ path = $fsrvp_shrdir
comment = fake shapshots using rsync
vfs objects = shell_snap shadow_copy2
shell_snap:check path command = $fake_snap_pl --check
shell_snap:create command = $fake_snap_pl --create
shell_snap:delete command = $fake_snap_pl --delete
# a relative path here fails, the snapshot dir is no longer found
- shadow:snapdir = $shrdir/.snapshots
+ shadow:snapdir = $fsrvp_shrdir/.snapshots
[shadow1]
path = $shadow_shrdir
--
2.17.1

View File

@ -1,111 +0,0 @@
From 409447f3258b87745a2248570278b1c6da8991f4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 16 Jul 2019 15:40:38 +0200
Subject: [PATCH 5/6] CVE-2019-10197: test_smbclient_s3.sh: add regression test
for the no permission on share root problem
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
selftest/knownfail.d/CVE-2019-10197 | 1 +
selftest/target/Samba3.pm | 12 +++++++++
source3/script/tests/test_smbclient_s3.sh | 30 +++++++++++++++++++++++
3 files changed, 43 insertions(+)
create mode 100644 selftest/knownfail.d/CVE-2019-10197
diff --git a/selftest/knownfail.d/CVE-2019-10197 b/selftest/knownfail.d/CVE-2019-10197
new file mode 100644
index 000000000000..f7056bbf3ad4
--- /dev/null
+++ b/selftest/knownfail.d/CVE-2019-10197
@@ -0,0 +1 @@
+^samba3.blackbox.smbclient_s3.*.noperm.share.regression
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index f7eb314138a0..2f491441815f 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1516,6 +1516,9 @@ sub provision($$$$$$$$$)
my $ro_shrdir="$shrdir/root-tmp";
push(@dirs,$ro_shrdir);
+ my $noperm_shrdir="$shrdir/noperm-tmp";
+ push(@dirs,$noperm_shrdir);
+
my $msdfs_shrdir="$shrdir/msdfsshare";
push(@dirs,$msdfs_shrdir);
@@ -1586,6 +1589,11 @@ sub provision($$$$$$$$$)
chmod 0755, $piddir;
+ ##
+ ## Create a directory without permissions to enter
+ ##
+ chmod 0000, $noperm_shrdir;
+
##
## create ro and msdfs share layout
##
@@ -1902,6 +1910,10 @@ sub provision($$$$$$$$$)
[ro-tmp]
path = $ro_shrdir
guest ok = yes
+[noperm]
+ path = $noperm_shrdir
+ wide links = yes
+ guest ok = yes
[write-list-tmp]
path = $shrdir
read only = yes
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index bf033ccd2fbf..0bae1d78fac9 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -1329,6 +1329,32 @@ EOF
fi
}
+#
+# Regression test for CVE-2019-10197
+# we should always get ACCESS_DENIED
+#
+test_noperm_share_regression()
+{
+ cmd='$SMBCLIENT -U$USERNAME%$PASSWORD //$SERVER/noperm -I $SERVER_IP $LOCAL_ADDARGS -c "ls;ls" 2>&1'
+ eval echo "$cmd"
+ out=`eval $cmd`
+ ret=$?
+ if [ $ret -eq 0 ] ; then
+ echo "$out"
+ echo "failed accessing no perm share should not work"
+ return 1
+ fi
+
+ num=`echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' | wc -l`
+ if [ "$num" -ne "2" ] ; then
+ echo "$out"
+ echo "failed num[$num] - two NT_STATUS_ACCESS_DENIED lines expected"
+ return 1
+ fi
+
+ return 0
+}
+
# Test smbclient deltree command
test_deltree()
{
@@ -1857,6 +1883,10 @@ testit "follow local symlinks" \
test_local_symlinks || \
failed=`expr $failed + 1`
+testit "noperm share regression" \
+ test_noperm_share_regression || \
+ failed=`expr $failed + 1`
+
testit "smbclient deltree command" \
test_deltree || \
failed=`expr $failed + 1`
--
2.17.1

View File

@ -1,87 +0,0 @@
From 501e034aa5b6ba50bf14e41c59674fbbc28a2e9c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 11 Jul 2019 17:02:15 +0200
Subject: [PATCH 6/6] CVE-2019-10197: smbd: split change_to_user_impersonate()
out of change_to_user_internal()
This makes sure we always call chdir_current_service() even
when we still impersonated the user. Which is important
in order to run the SMB* request within the correct working directory
and only if the user has permissions to enter that directory.
It makes sure we always update conn->lastused_count
in chdir_current_service() for each request.
Note that vfs_ChDir() (called from chdir_current_service())
maintains its own cache and avoids calling SMB_VFS_CHDIR()
if possible.
It means we still avoid syscalls if we get a multiple requests
for the same session/tcon tuple.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---
selftest/knownfail.d/CVE-2019-10197 | 1 -
source3/smbd/uid.c | 21 +++++++++++++++++----
2 files changed, 17 insertions(+), 5 deletions(-)
delete mode 100644 selftest/knownfail.d/CVE-2019-10197
diff --git a/selftest/knownfail.d/CVE-2019-10197 b/selftest/knownfail.d/CVE-2019-10197
deleted file mode 100644
index f7056bbf3ad4..000000000000
--- a/selftest/knownfail.d/CVE-2019-10197
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.smbclient_s3.*.noperm.share.regression
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 50868ba8572a..5c39baade5cf 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -306,9 +306,9 @@ static void print_impersonation_info(connection_struct *conn)
stack, but modify the current_user entries.
****************************************************************************/
-static bool change_to_user_internal(connection_struct *conn,
- const struct auth_session_info *session_info,
- uint64_t vuid)
+static bool change_to_user_impersonate(connection_struct *conn,
+ const struct auth_session_info *session_info,
+ uint64_t vuid)
{
int snum;
gid_t gid;
@@ -321,7 +321,6 @@ static bool change_to_user_internal(connection_struct *conn,
if ((current_user.conn == conn) &&
(current_user.vuid == vuid) &&
- (current_user.need_chdir == conn->tcon_done) &&
(current_user.ut.uid == session_info->unix_token->uid))
{
DBG_INFO("Skipping user change - already user\n");
@@ -426,6 +425,20 @@ static bool change_to_user_internal(connection_struct *conn,
current_user.conn = conn;
current_user.vuid = vuid;
+ return true;
+}
+
+static bool change_to_user_internal(connection_struct *conn,
+ const struct auth_session_info *session_info,
+ uint64_t vuid)
+{
+ bool ok;
+
+ ok = change_to_user_impersonate(conn, session_info, vuid);
+ if (!ok) {
+ return false;
+ }
+
current_user.need_chdir = conn->tcon_done;
current_user.done_chdir = false;
--
2.17.1

View File

@ -1,59 +0,0 @@
diff -Nurp samba-4.9.1/python/samba/tests/dcerpc/dnsserver.py samba-4.9.1-bak/python/samba/tests/dcerpc/dnsserver.py
--- samba-4.9.1/python/samba/tests/dcerpc/dnsserver.py 2018-07-12 04:23:36.000000000 -0400
+++ samba-4.9.1-bak/python/samba/tests/dcerpc/dnsserver.py 2019-07-29 11:47:02.701000000 -0400
@@ -28,6 +28,7 @@ from samba.dcerpc import dnsp, dnsserver
from samba.tests import RpcInterfaceTestCase, env_get_var_value
from samba.netcmd.dns import ARecord, AAAARecord, PTRRecord, CNameRecord, NSRecord, MXRecord, SRVRecord, TXTRecord
from samba import sd_utils, descriptor
+from samba import WERRORError, werror
class DnsserverTests(RpcInterfaceTestCase):
@@ -707,6 +708,29 @@ class DnsserverTests(RpcInterfaceTestCas
'ServerInfo')
self.assertEquals(dnsserver.DNSSRV_TYPEID_SERVER_INFO, typeid)
+ # This test is to confirm that we do not support multizone operations,
+ # which are designated by a non-zero dwContext value (the 3rd argument
+ # to DnssrvOperation).
+ def test_operation_invalid(self):
+ non_zone = 'a-zone-that-does-not-exist'
+ typeid = dnsserver.DNSSRV_TYPEID_NAME_AND_PARAM
+ name_and_param = dnsserver.DNS_RPC_NAME_AND_PARAM()
+ name_and_param.pszNodeName = 'AllowUpdate'
+ name_and_param.dwParam = dnsp.DNS_ZONE_UPDATE_SECURE
+ try:
+ res = self.conn.DnssrvOperation(self.server,
+ non_zone,
+ 1,
+ 'ResetDwordProperty',
+ typeid,
+ name_and_param)
+ except WERRORError as e:
+ if e.args[0] == werror.WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST:
+ return
+
+ # We should always encounter a DOES_NOT_EXIST error.
+ self.fail()
+
def test_operation2(self):
client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
rev_zone = '1.168.192.in-addr.arpa'
diff -Nurp samba-4.9.1/source4/rpc_server/dnsserver/dcerpc_dnsserver.c samba-4.9.1-bak/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
--- samba-4.9.1/source4/rpc_server/dnsserver/dcerpc_dnsserver.c 2018-07-12 04:23:36.000000000 -0400
+++ samba-4.9.1-bak/source4/rpc_server/dnsserver/dcerpc_dnsserver.c 2019-07-29 11:51:52.408000000 -0400
@@ -1955,7 +1955,13 @@ static WERROR dcesrv_DnssrvOperation(str
&r->in.pData);
} else {
z = dnsserver_find_zone(dsstate->zones, r->in.pszZone);
- if (z == NULL && request_filter == 0) {
+ /*
+ * In the case that request_filter is not 0 and z is NULL,
+ * the request is for a multizone operation, which we do not
+ * yet support, so just error on NULL zone name.
+ */
+ if (z == NULL) {
+
return WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST;
}

View File

@ -1,53 +0,0 @@
diff -Nurp samba-4.9.1/python/samba/tests/dcerpc/dnsserver.py samba-4.9.1-bak/python/samba/tests/dcerpc/dnsserver.py
--- samba-4.9.1/python/samba/tests/dcerpc/dnsserver.py 2019-07-29 11:56:23.948000000 -0400
+++ samba-4.9.1-bak/python/samba/tests/dcerpc/dnsserver.py 2019-07-29 11:58:35.410000000 -0400
@@ -731,6 +731,32 @@ class DnsserverTests(RpcInterfaceTestCas
# We should always encounter a DOES_NOT_EXIST error.
self.fail()
+ # This test is to confirm that we do not support multizone operations,
+ # which are designated by a non-zero dwContext value (the 5th argument
+ # to DnssrvOperation2).
+ def test_operation2_invalid(self):
+ client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
+ non_zone = 'a-zone-that-does-not-exist'
+ typeid = dnsserver.DNSSRV_TYPEID_NAME_AND_PARAM
+ name_and_param = dnsserver.DNS_RPC_NAME_AND_PARAM()
+ name_and_param.pszNodeName = 'AllowUpdate'
+ name_and_param.dwParam = dnsp.DNS_ZONE_UPDATE_SECURE
+ try:
+ res = self.conn.DnssrvOperation2(client_version,
+ 0,
+ self.server,
+ non_zone,
+ 1,
+ 'ResetDwordProperty',
+ typeid,
+ name_and_param)
+ except WERRORError as e:
+ if e.args[0] == werror.WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST:
+ return
+
+ # We should always encounter a DOES_NOT_EXIST error.
+ self.fail()
+
def test_operation2(self):
client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
rev_zone = '1.168.192.in-addr.arpa'
diff -Nurp samba-4.9.1/source4/rpc_server/dnsserver/dcerpc_dnsserver.c samba-4.9.1-bak/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
--- samba-4.9.1/source4/rpc_server/dnsserver/dcerpc_dnsserver.c 2019-07-29 11:56:23.950000000 -0400
+++ samba-4.9.1-bak/source4/rpc_server/dnsserver/dcerpc_dnsserver.c 2019-07-29 12:00:03.852000000 -0400
@@ -2168,7 +2168,12 @@ static WERROR dcesrv_DnssrvOperation2(st
&r->in.pData);
} else {
z = dnsserver_find_zone(dsstate->zones, r->in.pszZone);
- if (z == NULL && request_filter == 0) {
+ /*
+ * In the case that request_filter is not 0 and z is NULL,
+ * the request is for a multizone operation, which we do not
+ * yet support, so just error on NULL zone name.
+ */
+ if (z == NULL) {
return WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST;
}

View File

@ -1,390 +0,0 @@
From 9501741466ba2c0740ffc703c5d242d6b41510e8 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 29 Oct 2019 17:25:28 +1300
Subject: [PATCH 1/9] CVE-2019-14861: s4-rpc/dnsserver: Confirm sort behaviour
in dcesrv_DnssrvEnumRecords
The sort behaviour for child records is not correct in Samba so
we add a flapping entry.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
python/samba/tests/dcerpc/dnsserver.py | 101 +++++++++++++++++++++++++
selftest/flapping.d/dnsserver | 2 +
2 files changed, 103 insertions(+)
create mode 100644 selftest/flapping.d/dnsserver
diff --git a/python/samba/tests/dcerpc/dnsserver.py b/python/samba/tests/dcerpc/dnsserver.py
index 7264a290ef2..14ce308e38f 100644
--- a/python/samba/tests/dcerpc/dnsserver.py
+++ b/python/samba/tests/dcerpc/dnsserver.py
@@ -156,6 +156,107 @@ class DnsserverTests(RpcInterfaceTestCase):
None)
super(DnsserverTests, self).tearDown()
+ def test_enum_is_sorted(self):
+ """
+ Confirm the zone is sorted
+ """
+
+ record_str = "192.168.50.50"
+ record_type_str = "A"
+ self.add_record(self.custom_zone, "atestrecord-1", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-2", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-3", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-4", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-0", record_type_str, record_str)
+
+ # This becomes an extra A on the zone itself by server-side magic
+ self.add_record(self.custom_zone, self.custom_zone, record_type_str, record_str)
+
+ _, result = self.conn.DnssrvEnumRecords2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
+ 0,
+ self.server,
+ self.custom_zone,
+ "@",
+ None,
+ self.record_type_int(record_type_str),
+ dnsserver.DNS_RPC_VIEW_AUTHORITY_DATA,
+ None,
+ None)
+
+ self.assertEqual(len(result.rec), 6)
+ self.assertEqual(result.rec[0].dnsNodeName.str, "")
+ self.assertEqual(result.rec[1].dnsNodeName.str, "atestrecord-0")
+ self.assertEqual(result.rec[2].dnsNodeName.str, "atestrecord-1")
+ self.assertEqual(result.rec[3].dnsNodeName.str, "atestrecord-2")
+ self.assertEqual(result.rec[4].dnsNodeName.str, "atestrecord-3")
+ self.assertEqual(result.rec[5].dnsNodeName.str, "atestrecord-4")
+
+ def test_enum_is_sorted_children_prefix_first(self):
+ """
+ Confirm the zone returns the selected prefix first but no more
+ as Samba is flappy for the full sort
+ """
+
+ record_str = "192.168.50.50"
+ record_type_str = "A"
+ self.add_record(self.custom_zone, "atestrecord-1.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-2.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-3.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-4.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-0.a.b", record_type_str, record_str)
+
+ # Not expected to be returned
+ self.add_record(self.custom_zone, "atestrecord-0.b.b", record_type_str, record_str)
+
+ _, result = self.conn.DnssrvEnumRecords2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
+ 0,
+ self.server,
+ self.custom_zone,
+ "a.b",
+ None,
+ self.record_type_int(record_type_str),
+ dnsserver.DNS_RPC_VIEW_AUTHORITY_DATA,
+ None,
+ None)
+
+ self.assertEqual(len(result.rec), 6)
+ self.assertEqual(result.rec[0].dnsNodeName.str, "")
+
+ def test_enum_is_sorted_children(self):
+ """
+ Confirm the zone is sorted
+ """
+
+ record_str = "192.168.50.50"
+ record_type_str = "A"
+ self.add_record(self.custom_zone, "atestrecord-1.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-2.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-3.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-4.a.b", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-0.a.b", record_type_str, record_str)
+
+ # Not expected to be returned
+ self.add_record(self.custom_zone, "atestrecord-0.b.b", record_type_str, record_str)
+
+ _, result = self.conn.DnssrvEnumRecords2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
+ 0,
+ self.server,
+ self.custom_zone,
+ "a.b",
+ None,
+ self.record_type_int(record_type_str),
+ dnsserver.DNS_RPC_VIEW_AUTHORITY_DATA,
+ None,
+ None)
+
+ self.assertEqual(len(result.rec), 6)
+ self.assertEqual(result.rec[0].dnsNodeName.str, "")
+ self.assertEqual(result.rec[1].dnsNodeName.str, "atestrecord-0")
+ self.assertEqual(result.rec[2].dnsNodeName.str, "atestrecord-1")
+ self.assertEqual(result.rec[3].dnsNodeName.str, "atestrecord-2")
+ self.assertEqual(result.rec[4].dnsNodeName.str, "atestrecord-3")
+ self.assertEqual(result.rec[5].dnsNodeName.str, "atestrecord-4")
+
# This test fails against Samba (but passes against Windows),
# because Samba does not return the record when we enum records.
# Records can be given DNS_RANK_NONE when the zone they are in
diff --git a/selftest/flapping.d/dnsserver b/selftest/flapping.d/dnsserver
new file mode 100644
index 00000000000..9b33e8522a3
--- /dev/null
+++ b/selftest/flapping.d/dnsserver
@@ -0,0 +1,2 @@
+# This is not stable in samba due to a bug
+^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_enum_is_sorted_children
\ No newline at end of file
--
2.17.1
From 51fa9a6a805e4221120847ee9dcab6796021175a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 21 Oct 2019 12:12:10 +1300
Subject: [PATCH 2/9] CVE-2019-14861: s4-rpc_server: Remove special case for @
in dns_build_tree()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
source4/rpc_server/dnsserver/dnsdata.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
index 59e29f029a6..f991f4042e3 100644
--- a/source4/rpc_server/dnsserver/dnsdata.c
+++ b/source4/rpc_server/dnsserver/dnsdata.c
@@ -795,10 +795,11 @@ struct dns_tree *dns_build_tree(TALLOC_CTX *mem_ctx, const char *name, struct ld
for (i=0; i<res->count; i++) {
ptr = ldb_msg_find_attr_as_string(res->msgs[i], "name", NULL);
- if (strcmp(ptr, "@") == 0) {
- base->data = res->msgs[i];
- continue;
- } else if (strcasecmp(ptr, name) == 0) {
+ /*
+ * This might be the sub-domain in the zone being
+ * requested, or @ for the root of the zone
+ */
+ if (strcasecmp(ptr, name) == 0) {
base->data = res->msgs[i];
continue;
}
--
2.17.1
From 16405fecc403517574915a49de5f4abcaa964e21 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 29 Oct 2019 14:15:36 +1300
Subject: [PATCH 3/9] CVE-2019-14861: s4-rpc/dnsserver: Avoid crash in
ldb_qsort() via dcesrv_DnssrvEnumRecords)
dns_name_compare() had logic to put @ and the top record in the tree being
enumerated first, but if a domain had both then this would break the
older qsort() implementation in ldb_qsort() and cause a read of memory
before the base pointer.
By removing this special case (not required as the base pointer
is already seperatly located, no matter were it is in the
returned records) the crash is avoided.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
.../rpc_server/dnsserver/dcerpc_dnsserver.c | 21 ++++++++++++-------
source4/rpc_server/dnsserver/dnsdata.c | 19 ++---------------
source4/rpc_server/dnsserver/dnsserver.h | 4 ++--
3 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
index 353754f9261..c0bf3425dae 100644
--- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
+++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
@@ -1733,6 +1733,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
struct DNS_RPC_RECORDS_ARRAY *recs;
char **add_names = NULL;
char *rname;
+ const char *preference_name = NULL;
int add_count = 0;
int i, ret, len;
WERROR status;
@@ -1749,6 +1750,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
ret = ldb_search(dsstate->samdb, tmp_ctx, &res, z->zone_dn,
LDB_SCOPE_ONELEVEL, attrs,
"(&(objectClass=dnsNode)(!(dNSTombstoned=TRUE)))");
+ preference_name = "@";
} else {
char *encoded_name
= ldb_binary_encode_string(tmp_ctx, name);
@@ -1756,6 +1758,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
LDB_SCOPE_ONELEVEL, attrs,
"(&(objectClass=dnsNode)(|(name=%s)(name=*.%s))(!(dNSTombstoned=TRUE)))",
encoded_name, encoded_name);
+ preference_name = name;
}
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
@@ -1769,16 +1772,18 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
recs = talloc_zero(mem_ctx, struct DNS_RPC_RECORDS_ARRAY);
W_ERROR_HAVE_NO_MEMORY_AND_FREE(recs, tmp_ctx);
- /* Sort the names, so that the first record is the parent record */
- ldb_qsort(res->msgs, res->count, sizeof(struct ldb_message *), name,
- (ldb_qsort_cmp_fn_t)dns_name_compare);
+ /*
+ * Sort the names, so that the records are in order by the child
+ * component below "name".
+ *
+ * A full tree sort is not required, so we pass in "name" so
+ * we know which level to sort, as only direct children are
+ * eventually returned
+ */
+ LDB_TYPESAFE_QSORT(res->msgs, res->count, name, dns_name_compare);
/* Build a tree of name components from dns name */
- if (strcasecmp(name, z->name) == 0) {
- tree = dns_build_tree(tmp_ctx, "@", res);
- } else {
- tree = dns_build_tree(tmp_ctx, name, res);
- }
+ tree = dns_build_tree(tmp_ctx, preference_name, res);
W_ERROR_HAVE_NO_MEMORY_AND_FREE(tree, tmp_ctx);
/* Find the parent record in the tree */
diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
index f991f4042e3..acdb87752f8 100644
--- a/source4/rpc_server/dnsserver/dnsdata.c
+++ b/source4/rpc_server/dnsserver/dnsdata.c
@@ -1052,8 +1052,8 @@ WERROR dns_fill_records_array(TALLOC_CTX *mem_ctx,
}
-int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m2,
- char *search_name)
+int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const *m2,
+ const char *search_name)
{
const char *name1, *name2;
const char *ptr1, *ptr2;
@@ -1064,21 +1064,6 @@ int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m
return 0;
}
- /* '@' record and the search_name record gets preference */
- if (name1[0] == '@') {
- return -1;
- }
- if (search_name && strcasecmp(name1, search_name) == 0) {
- return -1;
- }
-
- if (name2[0] == '@') {
- return 1;
- }
- if (search_name && strcasecmp(name2, search_name) == 0) {
- return 1;
- }
-
/* Compare the last components of names.
* If search_name is not NULL, compare the second last components of names */
ptr1 = strrchr(name1, '.');
diff --git a/source4/rpc_server/dnsserver/dnsserver.h b/source4/rpc_server/dnsserver/dnsserver.h
index 93f1d72f2ef..690ab87ed10 100644
--- a/source4/rpc_server/dnsserver/dnsserver.h
+++ b/source4/rpc_server/dnsserver/dnsserver.h
@@ -188,8 +188,8 @@ struct DNS_ADDR_ARRAY *dns_addr_array_copy(TALLOC_CTX *mem_ctx, struct DNS_ADDR_
int dns_split_name_components(TALLOC_CTX *mem_ctx, const char *name, char ***components);
char *dns_split_node_name(TALLOC_CTX *mem_ctx, const char *node_name, const char *zone_name);
-int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m2,
- char *search_name);
+int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const *m2,
+ const char *search_name);
bool dns_record_match(struct dnsp_DnssrvRpcRecord *rec1, struct dnsp_DnssrvRpcRecord *rec2);
void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp,
--
2.17.1
From 90073f0abc495c4b5bd05322b71667c534ee9dd8 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 30 Oct 2019 11:50:57 +1300
Subject: [PATCH 4/9] CVE-2019-14861: Test to demonstrate the bug
This test does not fail every time, but when it does it casues a segfault which
takes out the rpc_server master process, as this hosts the dnsserver pipe.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
python/samba/tests/dcerpc/dnsserver.py | 47 ++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/python/samba/tests/dcerpc/dnsserver.py b/python/samba/tests/dcerpc/dnsserver.py
index 14ce308e38f..a9b8a4ace91 100644
--- a/python/samba/tests/dcerpc/dnsserver.py
+++ b/python/samba/tests/dcerpc/dnsserver.py
@@ -191,6 +191,53 @@ class DnsserverTests(RpcInterfaceTestCase):
self.assertEqual(result.rec[4].dnsNodeName.str, "atestrecord-3")
self.assertEqual(result.rec[5].dnsNodeName.str, "atestrecord-4")
+ def test_enum_is_sorted_with_zone_dup(self):
+ """
+ Confirm the zone is sorted
+ """
+
+ record_str = "192.168.50.50"
+ record_type_str = "A"
+ self.add_record(self.custom_zone, "atestrecord-1", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-2", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-3", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-4", record_type_str, record_str)
+ self.add_record(self.custom_zone, "atestrecord-0", record_type_str, record_str)
+
+ # This triggers a bug in old Samba
+ self.add_record(self.custom_zone, self.custom_zone + "1", record_type_str, record_str)
+
+ dn, record = self.get_record_from_db(self.custom_zone, self.custom_zone + "1")
+
+ new_dn = ldb.Dn(self.samdb, str(dn))
+ new_dn.set_component(0, "dc", self.custom_zone)
+ self.samdb.rename(dn, new_dn)
+
+ _, result = self.conn.DnssrvEnumRecords2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
+ 0,
+ self.server,
+ self.custom_zone,
+ "@",
+ None,
+ self.record_type_int(record_type_str),
+ dnsserver.DNS_RPC_VIEW_AUTHORITY_DATA,
+ None,
+ None)
+
+ self.assertEqual(len(result.rec), 7)
+ self.assertEqual(result.rec[0].dnsNodeName.str, "")
+ self.assertEqual(result.rec[1].dnsNodeName.str, "atestrecord-0")
+ self.assertEqual(result.rec[2].dnsNodeName.str, "atestrecord-1")
+ self.assertEqual(result.rec[3].dnsNodeName.str, "atestrecord-2")
+ self.assertEqual(result.rec[4].dnsNodeName.str, "atestrecord-3")
+ self.assertEqual(result.rec[5].dnsNodeName.str, "atestrecord-4")
+
+ # Windows doesn't reload the zone fast enough, but doesn't
+ # have the bug anyway, it will sort last on both names (where
+ # it should)
+ if result.rec[6].dnsNodeName.str != (self.custom_zone + "1"):
+ self.assertEqual(result.rec[6].dnsNodeName.str, self.custom_zone)
+
def test_enum_is_sorted_children_prefix_first(self):
"""
Confirm the zone returns the selected prefix first but no more
--
2.17.1

View File

@ -1,285 +0,0 @@
From 5249cad8b435d162584f010f492568d6f4526662 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Wed, 30 Oct 2019 15:59:16 +0100
Subject: [PATCH 7/9] CVE-2019-14870: heimdal: add S4U test for
delegation_not_allowed
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
---
selftest/knownfail.d/heimdal_not_delegated | 1 +
source4/selftest/tests.py | 1 +
testprogs/blackbox/test_s4u_heimdal.sh | 73 ++++++++++++++++++++++
3 files changed, 75 insertions(+)
create mode 100644 selftest/knownfail.d/heimdal_not_delegated
create mode 100755 testprogs/blackbox/test_s4u_heimdal.sh
diff --git a/selftest/knownfail.d/heimdal_not_delegated b/selftest/knownfail.d/heimdal_not_delegated
new file mode 100644
index 00000000000..bfc382a3fc2
--- /dev/null
+++ b/selftest/knownfail.d/heimdal_not_delegated
@@ -0,0 +1 @@
+^samba4.blackbox.krb5.s4u
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 9c3c77f1c56..2ec0bee923b 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -425,6 +425,7 @@ if have_heimdal_support:
plantestsuite("samba4.blackbox.kinit_trust(fl2003dc:local)", "fl2003dc:local", [os.path.join(bbdir, "test_kinit_trusts_heimdal.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$TRUST_SERVER', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$PREFIX', "external", "arcfour-hmac-md5"])
plantestsuite("samba4.blackbox.export.keytab(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_export_keytab_heimdal.sh"), '$SERVER', '$USERNAME', '$REALM', '$DOMAIN', "$PREFIX", smbclient4])
plantestsuite("samba4.blackbox.kpasswd(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_kpasswd_heimdal.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', "$PREFIX/ad_dc_ntvfs"])
+ plantestsuite("samba4.blackbox.krb5.s4u", "fl2008r2dc:local", [os.path.join(bbdir, "test_s4u_heimdal.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX', configuration])
else:
plantestsuite("samba4.blackbox.kinit(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_kinit_mit.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX', smbclient4, configuration])
plantestsuite("samba4.blackbox.kinit(fl2000dc:local)", "fl2000dc:local", [os.path.join(bbdir, "test_kinit_mit.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX', smbclient4, configuration])
diff --git a/testprogs/blackbox/test_s4u_heimdal.sh b/testprogs/blackbox/test_s4u_heimdal.sh
new file mode 100755
index 00000000000..0e12c7ec096
--- /dev/null
+++ b/testprogs/blackbox/test_s4u_heimdal.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+if [ $# -lt 5 ]; then
+cat <<EOF
+Usage: test_kinit.sh SERVER USERNAME PASSWORD REALM DOMAIN PREFIX
+EOF
+exit 1;
+fi
+
+SERVER=$1
+USERNAME=$2
+PASSWORD=$3
+REALM=$4
+DOMAIN=$5
+PREFIX=$6
+shift 6
+failed=0
+
+
+samba_tool="$VALGRIND $PYTHON $BINDIR/samba-tool"
+
+samba4kinit=kinit
+if test -x $BINDIR/samba4kinit; then
+ samba4kinit=$BINDIR/samba4kinit
+fi
+
+samba4kgetcred=kgetcred
+if test -x $BINDIR/samba4kgetcred; then
+ samba4kgetcred=$BINDIR/samba4kgetcred
+fi
+
+. `dirname $0`/subunit.sh
+. `dirname $0`/common_test_fns.inc
+
+ocache="$PREFIX/tmpoutcache"
+KRB5CCNAME_PATH="$PREFIX/tmpccache"
+KRB5CCNAME="FILE:$KRB5CCNAME_PATH"
+export KRB5CCNAME
+rm -rf $KRB5CCNAME_PATH
+
+princ=test_impersonate_princ
+impersonator=test_impersonator
+target="CIFS/$SERVER.$REALM"
+
+
+testit "add impersonator principal" $samba_tool user add $impersonator $PASSWORD || failed=`expr $failed + 1`
+testit "become a service" $samba_tool spn add "HOST/$impersonator" $impersonator || failed=`expr $failed + 1`
+
+testit "set TrustedToAuthForDelegation" $samba_tool delegation for-any-protocol $impersonator on || failed=`expr $failed + 1`
+testit "add msDS-AllowedToDelegateTo" $samba_tool delegation add-service $impersonator $target || failed=`expr $failed + 1`
+
+testit "add a new principal" $samba_tool user add $princ --random-password || failed=`expr $failed + 1`
+testit "set not-delegated flag" $samba_tool user sensitive $princ on || failed=`expr $failed + 1`
+
+
+echo $PASSWORD > $PREFIX/tmppassfile
+testit "kinit with password" $samba4kinit -f --password-file=$PREFIX/tmppassfile $impersonator || failed=`expr $failed + 1`
+
+testit "test S4U2Self with normal user" $samba4kgetcred --out-cache=$ocache --forwardable --impersonate=${USERNAME} $impersonator || failed=`expr $failed + 1`
+testit "test S4U2Proxy with normal user" $samba4kgetcred --out-cache=$ocache --delegation-credential-cache=${ocache} $target || failed=`expr $failed + 1`
+
+testit "test S4U2Self with sensitive user" $samba4kgetcred --out-cache=$ocache --forwardable --impersonate=$princ $impersonator || failed=`expr $failed + 1`
+testit_expect_failure "test S4U2Proxy with sensitive user" $samba4kgetcred --out-cache=$ocache --delegation-credential-cache=${ocache} $target || failed=`expr $failed + 1`
+
+rm -f $ocache
+testit "unset not-delegated flag" $samba_tool user sensitive $princ off || failed=`expr $failed + 1`
+
+testit "test S4U2Self after unsetting ND flag" $samba4kgetcred --out-cache=$ocache --forwardable --impersonate=$princ $impersonator || failed=`expr $failed + 1`
+testit "test S4U2Proxy after unsetting ND flag" $samba4kgetcred --out-cache=$ocache --delegation-credential-cache=${ocache} $target || failed=`expr $failed + 1`
+
+
+rm -f $ocache $PREFIX/tmpccache tmppassfile
+exit $failed
--
2.17.1
From d0d4954b9b4643678b6f465959dd69de0faafd07 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Mon, 28 Oct 2019 02:54:09 +0200
Subject: [PATCH 8/9] CVE-2019-14870: heimdal: enforce delegation_not_allowed
in S4U2Self
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
---
selftest/knownfail.d/heimdal_not_delegated | 1 -
source4/heimdal/kdc/krb5tgs.c | 58 ++++++++++++++--------
2 files changed, 36 insertions(+), 23 deletions(-)
delete mode 100644 selftest/knownfail.d/heimdal_not_delegated
diff --git a/selftest/knownfail.d/heimdal_not_delegated b/selftest/knownfail.d/heimdal_not_delegated
deleted file mode 100644
index bfc382a3fc2..00000000000
--- a/selftest/knownfail.d/heimdal_not_delegated
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.blackbox.krb5.s4u
diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index ff7d93138c0..ee3ac3d8f53 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1975,30 +1975,42 @@ server_lookup:
if (ret)
goto out;
+ ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags,
+ NULL, &s4u2self_impersonated_clientdb,
+ &s4u2self_impersonated_client);
+ if (ret) {
+ const char *msg;
+
+ /*
+ * If the client belongs to the same realm as our krbtgt, it
+ * should exist in the local database.
+ *
+ */
+
+ if (ret == HDB_ERR_NOENTRY)
+ ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN;
+ msg = krb5_get_error_message(context, ret);
+ kdc_log(context, config, 1,
+ "S2U4Self principal to impersonate %s not found in database: %s",
+ tpn, msg);
+ krb5_free_error_message(context, msg);
+ goto out;
+ }
+
+ /* Ignore pw_end attributes (as Windows does),
+ * since S4U2Self is not password authentication. */
+ free(s4u2self_impersonated_client->entry.pw_end);
+ s4u2self_impersonated_client->entry.pw_end = NULL;
+
+ ret = kdc_check_flags(context, config, s4u2self_impersonated_client, tpn,
+ NULL, NULL, FALSE);
+ if (ret)
+ goto out;
+
/* If we were about to put a PAC into the ticket, we better fix it to be the right PAC */
if(rspac.data) {
krb5_pac p = NULL;
krb5_data_free(&rspac);
- ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags,
- NULL, &s4u2self_impersonated_clientdb, &s4u2self_impersonated_client);
- if (ret) {
- const char *msg;
-
- /*
- * If the client belongs to the same realm as our krbtgt, it
- * should exist in the local database.
- *
- */
-
- if (ret == HDB_ERR_NOENTRY)
- ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN;
- msg = krb5_get_error_message(context, ret);
- kdc_log(context, config, 1,
- "S2U4Self principal to impersonate %s not found in database: %s",
- tpn, msg);
- krb5_free_error_message(context, msg);
- goto out;
- }
ret = _kdc_pac_generate(context, s4u2self_impersonated_client, NULL, &p);
if (ret) {
kdc_log(context, config, 0, "PAC generation failed for -- %s",
@@ -2034,10 +2046,12 @@ server_lookup:
/*
* If the service isn't trusted for authentication to
- * delegation, remove the forward flag.
+ * delegation or if the impersonate client is disallowed
+ * forwardable, remove the forwardable flag.
*/
- if (client->entry.flags.trusted_for_delegation) {
+ if (client->entry.flags.trusted_for_delegation &&
+ s4u2self_impersonated_client->entry.flags.forwardable) {
str = "[forwardable]";
} else {
b->kdc_options.forwardable = 0;
--
2.17.1
From 277ab21fcf31bf60458410994e188d9c236963a3 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Thu, 21 Nov 2019 11:12:48 +0100
Subject: [PATCH 9/9] CVE-2019-14870: mit-kdc: enforce delegation_not_allowed
flag
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14187
Signed-off-by: Isaac Boukris <iboukris@samba.org>
---
source4/kdc/mit_samba.c | 5 +++++
source4/kdc/sdb_to_kdb.c | 17 ++++++-----------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c
index eacca0903ec..06e680b60e2 100644
--- a/source4/kdc/mit_samba.c
+++ b/source4/kdc/mit_samba.c
@@ -304,6 +304,11 @@ fetch_referral_principal:
sdb_free_entry(&sentry);
+ if ((kflags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) == 0) {
+ kentry->attributes &= ~KRB5_KDB_DISALLOW_FORWARDABLE;
+ kentry->attributes &= ~KRB5_KDB_DISALLOW_PROXIABLE;
+ }
+
done:
krb5_free_principal(ctx->context, referral_principal);
referral_principal = NULL;
diff --git a/source4/kdc/sdb_to_kdb.c b/source4/kdc/sdb_to_kdb.c
index 74d882738f8..b7253ade122 100644
--- a/source4/kdc/sdb_to_kdb.c
+++ b/source4/kdc/sdb_to_kdb.c
@@ -36,18 +36,13 @@ static int SDBFlags_to_kflags(const struct SDBFlags *s,
if (s->initial) {
*k |= KRB5_KDB_DISALLOW_TGT_BASED;
}
- /*
- * Do not set any disallow rules for forwardable, proxiable,
- * renewable, postdate and server.
- *
- * The KDC will take care setting the flags based on the incoming
- * ticket.
- */
- if (s->forwardable) {
- ;
+ /* The forwardable and proxiable flags are set according to client and
+ * server attributes. */
+ if (!s->forwardable) {
+ *k |= KRB5_KDB_DISALLOW_FORWARDABLE;
}
- if (s->proxiable) {
- ;
+ if (!s->proxiable) {
+ *k |= KRB5_KDB_DISALLOW_PROXIABLE;
}
if (s->renewable) {
;
--
2.17.1

View File

@ -1,140 +0,0 @@
From c25348e1f2a7fd0801e06918d67c469f1912f311 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/5] 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.11.0

View File

@ -1,120 +0,0 @@
From b9ccfe0452524d8fdd5751944662856425599af2 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/5] 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.11.0

View File

@ -1,72 +0,0 @@
From be504b486d78133fd28ad3d7adfe589a99338846 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/5] 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.11.0

View File

@ -1,169 +0,0 @@
From c99f2ab22cc93b5194a3477c6a241600fa0f6758 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/5] 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.11.0

View File

@ -1,59 +0,0 @@
From 61414430c6bd6c9c9bfa1512880ecc6adbdbf9b4 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/5] 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.11.0

View File

@ -1,151 +0,0 @@
From a803d2524b8c06e2c360db0c686a212ac49f7321 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Thu, 21 Mar 2019 14:51:30 -0700
Subject: [PATCH] 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
@@ -640,46 +640,6 @@ WERROR _winreg_AbortSystemShutdown(struct pipes_struct *p,
}
/*******************************************************************
- ********************************************************************/
-
-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.11.0

29
README.downgrade Normal file
View File

@ -0,0 +1,29 @@
Downgrading Samba
=================
Short version: data-preserving downgrades between Samba versions are not supported
Long version:
With Samba development there are cases when on-disk database format evolves.
In general, Samba Team attempts to maintain forward compatibility and
automatically upgrade databases during runtime when requires.
However, when downgrade is required Samba will not perform downgrade to
existing databases. It may be impossible if new features that caused database
upgrade are in use. Thus, one needs to consider a downgrade procedure before
actually downgrading Samba setup.
Please always perform back up prior both upgrading and downgrading across major
version changes. Restoring database files is easiest and simplest way to get to
previously working setup.
Easiest way to downgrade is to remove all created databases and start from scratch.
This means losing all authentication and domain relationship data, as well as
user databases (in case of tdb storage), printers, registry settings, and winbindd
caches.
Remove databases in following locations:
/var/lib/samba/*.tdb
/var/lib/samba/private/*.tdb
In particular, registry settings are known to prevent running downgraded versions
(Samba 4 to Samba 3) as registry format has changed between Samba 3 and Samba 4.

7
samba-4.11.6.tar.asc Normal file
View File

@ -0,0 +1,7 @@
-----BEGIN PGP SIGNATURE-----
iHMEABECADMWIQRS+8C4bZVLCEMyTNxvM5FbZWi36gUCXjALthUcc2FtYmEtYnVn
c0BzYW1iYS5vcmcACgkQbzORW2Vot+od+ACgpzREKkVcyLse9EwufX0vS/JMUYIA
n2xGjOlyTFJJUD9heWInjmYzy4W0
=472O
-----END PGP SIGNATURE-----

BIN
samba-4.11.6.tar.xz Normal file

Binary file not shown.

View File

@ -1,117 +0,0 @@
From e2dd47233f467e2ab80564968be4af6da6505161 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 3 Sep 2018 10:35:08 +0200
Subject: [PATCH 1/2] waf: Check for -fstack-protect-strong support
The -fstack-protector* flags are compiler only flags, don't pass them to
the linker.
https://developers.redhat.com/blog/2018/03/21/compiler-and-linker-flags-gcc/
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13601
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 38e97f8b52e85bdfcf2d74a4fb3c848fa46ba371)
---
buildtools/wafsamba/samba_autoconf.py | 36 ++++++++++++++-------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/buildtools/wafsamba/samba_autoconf.py b/buildtools/wafsamba/samba_autoconf.py
index c4391d0c4dc..bfd6f9710db 100644
--- a/buildtools/wafsamba/samba_autoconf.py
+++ b/buildtools/wafsamba/samba_autoconf.py
@@ -674,23 +674,25 @@ def SAMBA_CONFIG_H(conf, path=None):
return
# we need to build real code that can't be optimized away to test
- if conf.check(fragment='''
- #include <stdio.h>
-
- int main(void)
- {
- char t[100000];
- while (fgets(t, sizeof(t), stdin));
- return 0;
- }
- ''',
- execute=0,
- ccflags='-fstack-protector',
- ldflags='-fstack-protector',
- mandatory=False,
- msg='Checking if toolchain accepts -fstack-protector'):
- conf.ADD_CFLAGS('-fstack-protector')
- conf.ADD_LDFLAGS('-fstack-protector')
+ stack_protect_list = ['-fstack-protector-strong', '-fstack-protector']
+ for stack_protect_flag in stack_protect_list:
+ flag_supported = conf.check(fragment='''
+ #include <stdio.h>
+
+ int main(void)
+ {
+ char t[100000];
+ while (fgets(t, sizeof(t), stdin));
+ return 0;
+ }
+ ''',
+ execute=0,
+ ccflags=[ '-Werror', '-Wp,-D_FORTIFY_SOURCE=2', stack_protect_flag],
+ mandatory=False,
+ msg='Checking if compiler accepts %s' % (stack_protect_flag))
+ if flag_supported:
+ conf.ADD_CFLAGS('-Wp,-D_FORTIFY_SOURCE=2 %s' % (stack_protect_flag))
+ break
if Options.options.debug:
conf.ADD_CFLAGS('-g', testflags=True)
--
2.18.0
From 09f3acb3497efb9ebb8a0d7d199726a8c318e4f8 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 3 Sep 2018 10:49:52 +0200
Subject: [PATCH 2/2] waf: Add -fstack-clash-protection
https://developers.redhat.com/blog/2018/03/21/compiler-and-linker-flags-gcc/
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13601
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit fc4df251c88365142515a81bea1120b2b84cc4a0)
---
buildtools/wafsamba/samba_autoconf.py | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/buildtools/wafsamba/samba_autoconf.py b/buildtools/wafsamba/samba_autoconf.py
index bfd6f9710db..f2b3ec8db8d 100644
--- a/buildtools/wafsamba/samba_autoconf.py
+++ b/buildtools/wafsamba/samba_autoconf.py
@@ -694,6 +694,23 @@ def SAMBA_CONFIG_H(conf, path=None):
conf.ADD_CFLAGS('-Wp,-D_FORTIFY_SOURCE=2 %s' % (stack_protect_flag))
break
+ flag_supported = conf.check(fragment='''
+ #include <stdio.h>
+
+ int main(void)
+ {
+ char t[100000];
+ while (fgets(t, sizeof(t), stdin));
+ return 0;
+ }
+ ''',
+ execute=0,
+ ccflags=[ '-Werror', '-fstack-clash-protection'],
+ mandatory=False,
+ msg='Checking if compiler accepts -fstack-clash-protection')
+ if flag_supported:
+ conf.ADD_CFLAGS('-fstack-clash-protection')
+
if Options.options.debug:
conf.ADD_CFLAGS('-g', testflags=True)
--
2.18.0

View File

@ -1,6 +0,0 @@
-----BEGIN PGP SIGNATURE-----
iFwEABECABwFAluomosVHHNhbWJhLWJ1Z3NAc2FtYmEub3JnAAoJEG8zkVtlaLfq
Ef0AoLUiZNu1bqD0YjbzI8KCisfwPF/2AKDGrFuyL4ds6Ege/OiUbg7krCXrOg==
=2NTz
-----END PGP SIGNATURE-----

Binary file not shown.

View File

@ -1,7 +1,10 @@
/var/log/samba/* { /var/log/samba/log.* {
compress
dateext
maxage 365
rotate 99
notifempty notifempty
olddir /var/log/samba/old olddir /var/log/samba/old
missingok missingok
sharedscripts
copytruncate copytruncate
} }

2896
samba.spec

File diff suppressed because it is too large Load Diff