[SCM] Samba Shared Repository - branch master updated
Gary Lockyer
gary at samba.org
Sun May 10 23:22:02 UTC 2020
The branch, master has been updated
via a699256f438 lib ldb: Limit depth of ldb_parse_tree
via ac800011006 lib util ASN.1: Panic on ASN.1 tag mismatch
via 80d9ea13867 s4 cldap server tests: request size limit tests
via aa7cc50ae2a s4 ldap server tests: request size limit tests
from e907f002a7f Fix clang 9 for-loop-analysis warnings
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit a699256f438527455aaff6c73c88ee87ac7083ef
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue Apr 21 15:37:40 2020 +1200
lib ldb: Limit depth of ldb_parse_tree
Limit the number of nested conditionals allowed by ldb_parse tree to
128, to avoid potential stack overflow issues.
Credit Oss-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19508
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Gary Lockyer <gary at samba.org>
Autobuild-Date(master): Sun May 10 23:21:08 UTC 2020 on sn-devel-184
commit ac8000110064055a986c0c1fee896fddd302114b
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed Apr 29 11:09:34 2020 +1200
lib util ASN.1: Panic on ASN.1 tag mismatch
If the ASN.1 depth is zero in asn1_end_tag, call smb_panic. Rather than
ignoring the condition.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 80d9ea1386707c33783b76e5bc4ec37523424439
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed Apr 29 09:17:52 2020 +1200
s4 cldap server tests: request size limit tests
Add tests for packet size limits.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit aa7cc50ae2a99d926e495efd2982e4c656b87bf3
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue Apr 28 13:55:04 2020 +1200
s4 ldap server tests: request size limit tests
Extra tests for ldap maximum request size limits.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
-----------------------------------------------------------------------
Summary of changes:
lib/ldb/common/ldb_parse.c | 72 +++-
lib/ldb/tests/ldb_parse_test.c | 83 ++++-
lib/util/asn1.c | 5 +-
python/samba/tests/ldap_raw.py | 729 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 860 insertions(+), 29 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/ldb/common/ldb_parse.c b/lib/ldb/common/ldb_parse.c
index 452c5830ed5..7e15206b168 100644
--- a/lib/ldb/common/ldb_parse.c
+++ b/lib/ldb/common/ldb_parse.c
@@ -43,6 +43,16 @@
#include "ldb_private.h"
#include "system/locale.h"
+/*
+ * Maximum depth of the filter parse tree, the value chosen is small enough to
+ * avoid triggering ASAN stack overflow checks. But large enough to be useful.
+ *
+ * On Windows clients the maximum number of levels of recursion allowed is 100.
+ * In the LDAP server, Windows restricts clients to 512 nested
+ * (eg) OR statements.
+ */
+#define LDB_MAX_PARSE_TREE_DEPTH 128
+
static int ldb_parse_hex2char(const char *x)
{
if (isxdigit(x[0]) && isxdigit(x[1])) {
@@ -231,7 +241,11 @@ static struct ldb_val **ldb_wildcard_decode(TALLOC_CTX *mem_ctx, const char *str
return ret;
}
-static struct ldb_parse_tree *ldb_parse_filter(TALLOC_CTX *mem_ctx, const char **s);
+static struct ldb_parse_tree *ldb_parse_filter(
+ TALLOC_CTX *mem_ctx,
+ const char **s,
+ unsigned depth,
+ unsigned max_depth);
/*
@@ -498,7 +512,11 @@ static struct ldb_parse_tree *ldb_parse_simple(TALLOC_CTX *mem_ctx, const char *
<or> ::= '|' <filterlist>
<filterlist> ::= <filter> | <filter> <filterlist>
*/
-static struct ldb_parse_tree *ldb_parse_filterlist(TALLOC_CTX *mem_ctx, const char **s)
+static struct ldb_parse_tree *ldb_parse_filterlist(
+ TALLOC_CTX *mem_ctx,
+ const char **s,
+ unsigned depth,
+ unsigned max_depth)
{
struct ldb_parse_tree *ret, *next;
enum ldb_parse_op op;
@@ -533,7 +551,8 @@ static struct ldb_parse_tree *ldb_parse_filterlist(TALLOC_CTX *mem_ctx, const ch
return NULL;
}
- ret->u.list.elements[0] = ldb_parse_filter(ret->u.list.elements, &p);
+ ret->u.list.elements[0] =
+ ldb_parse_filter(ret->u.list.elements, &p, depth, max_depth);
if (!ret->u.list.elements[0]) {
talloc_free(ret);
return NULL;
@@ -547,7 +566,8 @@ static struct ldb_parse_tree *ldb_parse_filterlist(TALLOC_CTX *mem_ctx, const ch
break;
}
- next = ldb_parse_filter(ret->u.list.elements, &p);
+ next = ldb_parse_filter(
+ ret->u.list.elements, &p, depth, max_depth);
if (next == NULL) {
/* an invalid filter element */
talloc_free(ret);
@@ -576,7 +596,11 @@ static struct ldb_parse_tree *ldb_parse_filterlist(TALLOC_CTX *mem_ctx, const ch
/*
<not> ::= '!' <filter>
*/
-static struct ldb_parse_tree *ldb_parse_not(TALLOC_CTX *mem_ctx, const char **s)
+static struct ldb_parse_tree *ldb_parse_not(
+ TALLOC_CTX *mem_ctx,
+ const char **s,
+ unsigned depth,
+ unsigned max_depth)
{
struct ldb_parse_tree *ret;
const char *p = *s;
@@ -593,7 +617,7 @@ static struct ldb_parse_tree *ldb_parse_not(TALLOC_CTX *mem_ctx, const char **s)
}
ret->operation = LDB_OP_NOT;
- ret->u.isnot.child = ldb_parse_filter(ret, &p);
+ ret->u.isnot.child = ldb_parse_filter(ret, &p, depth, max_depth);
if (!ret->u.isnot.child) {
talloc_free(ret);
return NULL;
@@ -608,7 +632,11 @@ static struct ldb_parse_tree *ldb_parse_not(TALLOC_CTX *mem_ctx, const char **s)
parse a filtercomp
<filtercomp> ::= <and> | <or> | <not> | <simple>
*/
-static struct ldb_parse_tree *ldb_parse_filtercomp(TALLOC_CTX *mem_ctx, const char **s)
+static struct ldb_parse_tree *ldb_parse_filtercomp(
+ TALLOC_CTX *mem_ctx,
+ const char **s,
+ unsigned depth,
+ unsigned max_depth)
{
struct ldb_parse_tree *ret;
const char *p = *s;
@@ -617,15 +645,15 @@ static struct ldb_parse_tree *ldb_parse_filtercomp(TALLOC_CTX *mem_ctx, const ch
switch (*p) {
case '&':
- ret = ldb_parse_filterlist(mem_ctx, &p);
+ ret = ldb_parse_filterlist(mem_ctx, &p, depth, max_depth);
break;
case '|':
- ret = ldb_parse_filterlist(mem_ctx, &p);
+ ret = ldb_parse_filterlist(mem_ctx, &p, depth, max_depth);
break;
case '!':
- ret = ldb_parse_not(mem_ctx, &p);
+ ret = ldb_parse_not(mem_ctx, &p, depth, max_depth);
break;
case '(':
@@ -641,21 +669,34 @@ static struct ldb_parse_tree *ldb_parse_filtercomp(TALLOC_CTX *mem_ctx, const ch
return ret;
}
-
/*
<filter> ::= '(' <filtercomp> ')'
*/
-static struct ldb_parse_tree *ldb_parse_filter(TALLOC_CTX *mem_ctx, const char **s)
+static struct ldb_parse_tree *ldb_parse_filter(
+ TALLOC_CTX *mem_ctx,
+ const char **s,
+ unsigned depth,
+ unsigned max_depth)
{
struct ldb_parse_tree *ret;
const char *p = *s;
+ /*
+ * Check the depth of the parse tree, and reject the input if
+ * max_depth exceeded. This avoids stack overflow
+ * issues.
+ */
+ if (depth > max_depth) {
+ return NULL;
+ }
+ depth++;
+
if (*p != '(') {
return NULL;
}
p++;
- ret = ldb_parse_filtercomp(mem_ctx, &p);
+ ret = ldb_parse_filtercomp(mem_ctx, &p, depth, max_depth);
if (*p != ')') {
return NULL;
@@ -679,6 +720,8 @@ static struct ldb_parse_tree *ldb_parse_filter(TALLOC_CTX *mem_ctx, const char *
*/
struct ldb_parse_tree *ldb_parse_tree(TALLOC_CTX *mem_ctx, const char *s)
{
+ unsigned depth = 0;
+
while (s && isspace((unsigned char)*s)) s++;
if (s == NULL || *s == 0) {
@@ -686,7 +729,8 @@ struct ldb_parse_tree *ldb_parse_tree(TALLOC_CTX *mem_ctx, const char *s)
}
if (*s == '(') {
- return ldb_parse_filter(mem_ctx, &s);
+ return ldb_parse_filter(
+ mem_ctx, &s, depth, LDB_MAX_PARSE_TREE_DEPTH);
}
return ldb_parse_simple(mem_ctx, &s);
diff --git a/lib/ldb/tests/ldb_parse_test.c b/lib/ldb/tests/ldb_parse_test.c
index a739d7795d1..d7442b954ea 100644
--- a/lib/ldb/tests/ldb_parse_test.c
+++ b/lib/ldb/tests/ldb_parse_test.c
@@ -81,10 +81,91 @@ static void test_parse_filtertype(void **state)
test_roundtrip(ctx, " ", "(|(objectClass=*)(distinguishedName=*))");
}
+/*
+ * Test that a nested query with 128 levels of nesting is accepted
+ */
+static void test_nested_filter_eq_limit(void **state)
+{
+ struct test_ctx *ctx =
+ talloc_get_type_abort(*state, struct test_ctx);
+
+ /*
+ * 128 nested clauses
+ */
+ const char *nested_query = ""
+ "(|(!(|(&(|(|(|(|(|(|(|(|(|(|(|(|"
+ "(|(!(|(&(|(|(|(|(|(|(!(|(!(|(|(|"
+ "(|(!(|(&(|(|(&(|(|(|(|(|(!(!(!(|"
+ "(|(!(|(&(|(|(|(|(|(|(|(|(|(|(|(|"
+ "(|(!(|(&(|(|(|(!(|(|(&(|(|(|(|(|"
+ "(|(!(|(&(|(|(&(|(|(|(|(|(&(&(|(|"
+ "(|(!(|(&(|(|(|(|(|(|(!(|(|(|(|(|"
+ "(|(!(|(&(|(|(!(|(|(|(|(|(|(|(|(|"
+ "(a=b)"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))";
+
+ struct ldb_parse_tree *tree = ldb_parse_tree(ctx, nested_query);
+
+ assert_non_null(tree);
+ /*
+ * Check that we get the same query back
+ */
+ test_roundtrip(ctx, nested_query, nested_query);
+}
+
+/*
+ * Test that a nested query with 129 levels of nesting is rejected.
+ */
+static void test_nested_filter_gt_limit(void **state)
+{
+ struct test_ctx *ctx =
+ talloc_get_type_abort(*state, struct test_ctx);
+
+ /*
+ * 129 nested clauses
+ */
+ const char *nested_query = ""
+ "(|(!(|(|(&(|(|(|(|(&(|(|(|(|(|(|"
+ "(|(!(|(|(&(|(|(|(|(|(|(|(|(|(|(|"
+ "(|(!(|(|(&(|(|(!(|(|(|(|(!(|(|(|"
+ "(|(!(|(|(&(|(|(|(|(|(|(|(|(|(|(|"
+ "(|(!(|(|(&(|(|(|(!(&(|(|(|(|(|(|"
+ "(|(!(|(|(&(|(|(|(|(|(|(|(|(|(|(|"
+ "(|(!(|(|(&(|(|(|(|(|(|(|(|(|(|(|"
+ "(|(!(|(|(&(|(|(|(|(|(|(|(|(&(|(|"
+ "(|"
+ "(a=b)"
+ ")"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))"
+ "))))))))))))))))";
+
+ struct ldb_parse_tree *tree = ldb_parse_tree(ctx, nested_query);
+
+ assert_null(tree);
+}
+
int main(int argc, const char **argv)
{
const struct CMUnitTest tests[] = {
- cmocka_unit_test_setup_teardown(test_parse_filtertype, setup, teardown),
+ cmocka_unit_test_setup_teardown(
+ test_parse_filtertype, setup, teardown),
+ cmocka_unit_test_setup_teardown(
+ test_nested_filter_eq_limit, setup, teardown),
+ cmocka_unit_test_setup_teardown(
+ test_nested_filter_gt_limit, setup, teardown),
};
cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
diff --git a/lib/util/asn1.c b/lib/util/asn1.c
index 397f6b3c271..88d96d4544b 100644
--- a/lib/util/asn1.c
+++ b/lib/util/asn1.c
@@ -713,9 +713,10 @@ bool asn1_end_tag(struct asn1_data *data)
{
struct nesting *nesting;
- if (data->depth > 0) {
- data->depth--;
+ if (data->depth == 0) {
+ smb_panic("Unbalanced ASN.1 Tag nesting");
}
+ data->depth--;
/* make sure we read it all */
if (asn1_tag_remaining(data) != 0) {
data->has_error = true;
diff --git a/python/samba/tests/ldap_raw.py b/python/samba/tests/ldap_raw.py
index 334fabce230..f4ea97d120e 100644
--- a/python/samba/tests/ldap_raw.py
+++ b/python/samba/tests/ldap_raw.py
@@ -19,6 +19,7 @@
#
import socket
+import ssl
import samba.tests
from samba.tests import TestCase
@@ -27,11 +28,29 @@ from samba.tests import TestCase
#
# LDAP Operations
#
-SEARCH = b'\x63'
+DELETE = b'\x4a'
+DELETE_RES = b'\x6b'
+
+# Bind
+BIND = b'\x60'
+BIND_RES = b'\x61'
+SIMPLE_AUTH = b'\x80'
+SASL_AUTH = b'\xa3'
+# Search
+SEARCH = b'\x63'
+SEARCH_RES = b'\x64'
EQUALS = b'\xa3'
+#
+# LDAP response codes.
+#
+SUCCESS = b'\x00'
+OPERATIONS_ERROR = b'\x01'
+INVALID_CREDENTIALS = b'\x31'
+INVALID_DN_SYNTAX = b'\x22'
+
#
# ASN.1 Element types
#
@@ -97,15 +116,52 @@ def encode_sequence(sequence):
return encode_element(SEQUENCE, sequence)
+def decode_element(data):
+ '''
+ decode an ASN.1 element
+ '''
+ if data is None:
+ return None
+
+ if len(data) < 2:
+ return None
+
+ ber_type = data[0:1]
+ enc = int.from_bytes(data[1:2], byteorder='big')
+ if enc & 0x80:
+ l_end = 2 + (enc & ~0x80)
+ length = int.from_bytes(data[2:l_end], byteorder='big')
+ element = data[l_end:l_end + length]
+ rest = data[l_end + length:]
+ else:
+ length = enc
+ element = data[2:2 + length]
+ rest = data[2 + length:]
+
+ return (ber_type, length, element, rest)
+
+
class RawLdapTest(TestCase):
- """A raw Ldap Test case."""
+ """
+ A raw Ldap Test case.
+ The ldap connections are made over https on port 636
+
+ Uses the following environment variables:
+ SERVER
+ USERNAME
+ PASSWORD
+ DNSNAME
+ """
def setUp(self):
super(RawLdapTest, self).setUp()
self.host = samba.tests.env_get_var_value('SERVER')
- self.port = 389
+ self.port = 636
self.socket = None
+ self.user = samba.tests.env_get_var_value('USERNAME')
+ self.password = samba.tests.env_get_var_value('PASSWORD')
+ self.dns_name = samba.tests.env_get_var_value('DNSNAME')
self.connect()
def tearDown(self):
@@ -120,13 +176,23 @@ class RawLdapTest(TestCase):
self.socket = None
def connect(self):
- ''' Open a socket stream connection to the server '''
+ ''' Establish an ldaps connection to the test server '''
+ #
+ # Disable host name and certificate verification
+ context = ssl.create_default_context()
+ context.check_hostname = False
+ context.verify_mode = ssl.CERT_NONE
+
+ sock = None
try:
- self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
- self.socket.settimeout(10)
- self.socket.connect((self.host, self.port))
+ sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ sock.settimeout(10)
+ sock.connect((self.host, self.port))
+ self.socket = context.wrap_socket(sock, server_hostname=self.host)
except socket.error:
- self.socket.close()
+ sock.close()
+ if self.socket is not None:
+ self.socket.close()
raise
def send(self, req):
@@ -138,7 +204,7 @@ class RawLdapTest(TestCase):
raise
def recv(self, num_recv=0xffff, timeout=None):
- ''' recv an array of bytes from the server '''
+ ''' receive an array of bytes from the server '''
data = None
try:
if timeout is not None:
@@ -158,10 +224,126 @@ class RawLdapTest(TestCase):
raise
return data
+ def bind(self):
+ '''
+ Perform a simple bind
+ '''
+
+ user = self.user.encode('UTF8')
+ ou = self.dns_name.replace('.', ',dc=').encode('UTF8')
+ dn = b'cn=' + user + b',cn=users,dc=' + ou
+
+ password = self.password.encode('UTF8')
+
+ # Lets build an simple bind request
+ bind = encode_integer(3) # ldap version
+ bind += encode_string(dn)
+ bind += encode_element(SIMPLE_AUTH, password)
+
+ bind_op = encode_element(BIND, bind)
+
+ msg_no = encode_integer(1)
+ packet = encode_sequence(msg_no + bind_op)
+
+ self.send(packet)
+ data = self.recv()
+ self.assertIsNotNone(data)
+
+ #
+ # Decode and validate the response
+
+ # Should be a sequence
+ (ber_type, length, element, rest) = decode_element(data)
+ self.assertEqual(SEQUENCE.hex(), ber_type.hex())
+ self.assertTrue(length > 0)
+ self.assertEqual(0, len(rest))
+
+ # message id should be 1
+ (ber_type, length, element, rest) = decode_element(element)
+ self.assertEqual(INTEGER.hex(), ber_type.hex())
+ msg_no = int.from_bytes(element, byteorder='big')
+ self.assertEqual(1, msg_no)
+ self.assertGreater(len(rest), 0)
+
+ # Should have a Bind response element
+ (ber_type, length, element, rest) = decode_element(rest)
+ self.assertEqual(BIND_RES.hex(), ber_type.hex())
+ self.assertEqual(0, len(rest))
+
+ # Check the response code
+ (ber_type, length, element, rest) = decode_element(element)
+ self.assertEqual(ENUMERATED.hex(), ber_type.hex())
+ self.assertEqual(SUCCESS.hex(), element.hex())
+ self.assertGreater(len(rest), 0)
+
+ def test_decode_element(self):
+ ''' Tests for the decode_element method '''
+
+ # Boolean true value
+ data = b'\x01\x01\xff'
+ (ber_type, length, element, rest) = decode_element(data)
+ self.assertEqual(BOOLEAN.hex(), ber_type.hex())
+ self.assertEqual(1, length)
+ self.assertEqual(b'\xff'.hex(), element.hex())
+ self.assertEqual(0, len(rest))
+
+ # Boolean false value
+ data = b'\x01\x01\x00'
+ (ber_type, length, element, rest) = decode_element(data)
+ self.assertEqual(BOOLEAN.hex(), ber_type.hex())
+ self.assertEqual(1, length)
--
Samba Shared Repository
More information about the samba-cvs
mailing list