The branch, master has been updated
via e242d72 selftest: Ensure that if the SAMBA_PID is not set, that the
env is not OK
via b0aa686 selftest: Do not start tests on an environment that has
failed to start up
via af16d52 ldb torture: test ldb_unpack_data_only_attr_list
via 8644dd4 lib/ldb: Use talloc_memdup() because we know the length of
the attribute already
via 8731e0c lib/ldb: Rename variable for clarity
via 315049e lib/ldb Add checks for overflow during ldb pack and parse
via 486fd45 lib/ldb: Use better variable names in
ldb_unpack_only_attr_list
via 000249f ldb: increment version due to added
ldb_unpack_data_only_attr_list
via 61a84ca lib/ldb: Clarify the intent of ldb_data_unpack_withlist
via abcd35f ldb: introduce ldb_unpack_data_withlist to unpack partial
list of attributes
from 1595f56 CVE-2015-8467: samdb: Match MS15-096 behaviour for
userAccountControl
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit e242d7264e50b1f13b95497d9cb759205931e7a2
Author: Andrew Bartlett <abart...@samba.org>
Date: Mon Dec 7 13:32:25 2015 +1300
selftest: Ensure that if the SAMBA_PID is not set, that the env is not OK
This ensures that we must instead start the selftest environment, it is not
already running
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Thu Dec 17 06:27:14 CET 2015 on sn-devel-104
commit b0aa686eb6a36857a5f2687d159ed4caf2f3a62e
Author: Andrew Bartlett <abart...@samba.org>
Date: Mon Dec 7 13:18:38 2015 +1300
selftest: Do not start tests on an environment that has failed to start up
This avoids debugging subsequent test failures, which may not be as clear
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit af16d52f7dbf2ed8c13b6a52abb6d88ef08d9ff6
Author: Adrian Cochrane <adri...@catalyst.net.nz>
Date: Fri Aug 14 14:27:03 2015 +1200
ldb torture: test ldb_unpack_data_only_attr_list
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
Signed-off-by: Adrian Cochrane <adri...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 8644dd4e52e402256f030276de675a9031495031
Author: Adrian Cochrane <adri...@catalyst.net.nz>
Date: Tue Sep 1 13:33:52 2015 +1200
lib/ldb: Use talloc_memdup() because we know the length of the attribute
already
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
Signed-off-by: Adrian Cochrane <adri...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 8731e0c6cd267dbf23e9834d0022713d3a17d791
Author: Garming Sam <garm...@catalyst.net.nz>
Date: Thu Dec 17 11:41:13 2015 +1300
lib/ldb: Rename variable for clarity
The variable p is the same as attr at this point since p is only
incremented when a continue is invoked in the loop.
Signed-off-by: Garming Sam <garm...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 315049e083814d529af6973be263e296ed78ca75
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri Nov 13 18:45:23 2015 +1300
lib/ldb Add checks for overflow during ldb pack and parse
Both as requested by Jeremy Allison <j...@samba.org> during
patch review and as found by american fuzzy lop.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 486fd453805770e1cd17cce380f497781dfeca33
Author: Adrian Cochrane <adri...@catalyst.net.nz>
Date: Tue Sep 1 13:27:52 2015 +1200
lib/ldb: Use better variable names in ldb_unpack_only_attr_list
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
Signed-off-by: Adrian Cochrane <adri...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
commit 000249fa110d3fb9ca1658b2bcdd8f75728cb358
Author: Garming Sam <garm...@catalyst.net.nz>
Date: Thu Dec 17 11:53:12 2015 +1300
ldb: increment version due to added ldb_unpack_data_only_attr_list
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
Signed-off-by: Garming Sam <garm...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 61a84ca583412bba1c9b18a57808e46268abe8f5
Author: Garming Sam <garm...@catalyst.net.nz>
Date: Thu Dec 17 11:24:44 2015 +1300
lib/ldb: Clarify the intent of ldb_data_unpack_withlist
This patch renames the function to indicate that you are unpacking with
respect to some
attribute list, as well as adding some comments.
Signed-off-by: Garming Sam <garm...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
commit abcd35f942468e8e51dbc7b976055232df41d421
Author: Matthieu Patou <m...@matws.net>
Date: Thu Dec 27 21:38:29 2012 -0800
ldb: introduce ldb_unpack_data_withlist to unpack partial list of attributes
When provided with non NULL list ldb_unpack_data_withlist will only
unpack attributes that are specified in the list (+ distinguished name)
ldb_unpack_data is changed to call ldb_unpack_data_withlist behind the
scene.
(for modifications found by testing, and re-indentation requested in review)
Signed-off-by: Adrian Cochrane <adri...@catalyst.net.nz>
Sadly a signed-off-by was not available from Matthieu Patou for the original
version of this patch posted to samba-technical for comment, so instead:
(for supervision of Adrian)
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Garming Sam <garm...@catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11602
-----------------------------------------------------------------------
Summary of changes:
lib/ldb/ABI/{ldb-1.1.23.sigs => ldb-1.1.25.sigs} | 1 +
...ldb-util-1.1.10.sigs => pyldb-util-1.1.25.sigs} | 0
lib/ldb/common/ldb_pack.c | 274 +++++++++++++++++----
lib/ldb/include/ldb_module.h | 6 +
lib/ldb/wscript | 2 +-
selftest/target/Samba4.pm | 109 ++++----
source4/torture/ldb/ldb.c | 134 +++++++++-
7 files changed, 433 insertions(+), 93 deletions(-)
copy lib/ldb/ABI/{ldb-1.1.23.sigs => ldb-1.1.25.sigs} (99%)
copy lib/ldb/ABI/{pyldb-util-1.1.10.sigs => pyldb-util-1.1.25.sigs} (100%)
Changeset truncated at 500 lines:
diff --git a/lib/ldb/ABI/ldb-1.1.23.sigs b/lib/ldb/ABI/ldb-1.1.25.sigs
similarity index 99%
copy from lib/ldb/ABI/ldb-1.1.23.sigs
copy to lib/ldb/ABI/ldb-1.1.25.sigs
index 6d9767b..3f33df9 100644
--- a/lib/ldb/ABI/ldb-1.1.23.sigs
+++ b/lib/ldb/ABI/ldb-1.1.25.sigs
@@ -253,6 +253,7 @@ ldb_transaction_commit: int (struct ldb_context *)
ldb_transaction_prepare_commit: int (struct ldb_context *)
ldb_transaction_start: int (struct ldb_context *)
ldb_unpack_data: int (struct ldb_context *, const struct ldb_val *, struct
ldb_message *)
+ldb_unpack_data_only_attr_list: int (struct ldb_context *, const struct
ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned
int *)
ldb_val_dup: struct ldb_val (TALLOC_CTX *, const struct ldb_val *)
ldb_val_equal_exact: int (const struct ldb_val *, const struct ldb_val *)
ldb_val_map_local: struct ldb_val (struct ldb_module *, void *, const struct
ldb_map_attribute *, const struct ldb_val *)
diff --git a/lib/ldb/ABI/pyldb-util-1.1.10.sigs
b/lib/ldb/ABI/pyldb-util-1.1.25.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util-1.1.10.sigs
copy to lib/ldb/ABI/pyldb-util-1.1.25.sigs
diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index 4382d5b..7970b9d 100644
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -77,7 +77,7 @@ int ldb_pack_data(struct ldb_context *ldb,
struct ldb_val *data)
{
unsigned int i, j, real_elements=0;
- size_t size;
+ size_t size, dn_len, attr_len, value_len;
const char *dn;
uint8_t *p;
size_t len;
@@ -91,8 +91,19 @@ int ldb_pack_data(struct ldb_context *ldb,
/* work out how big it needs to be */
size = 8;
- size += 1 + strlen(dn);
+ size += 1;
+ dn_len = strlen(dn);
+ if (size + dn_len < size) {
+ errno = ENOMEM;
+ return -1;
+ }
+ size += dn_len;
+
+ /*
+ * First calcuate the buffer size we need, and check for
+ * overflows
+ */
for (i=0;i<message->num_elements;i++) {
if (attribute_storable_values(&message->elements[i]) == 0) {
continue;
@@ -100,9 +111,32 @@ int ldb_pack_data(struct ldb_context *ldb,
real_elements++;
- size += 1 + strlen(message->elements[i].name) + 4;
+ if (size + 5 < size) {
+ errno = ENOMEM;
+ return -1;
+ }
+ size += 5;
+
+ attr_len = strlen(message->elements[i].name);
+ if (size + attr_len < size) {
+ errno = ENOMEM;
+ return -1;
+ }
+ size += attr_len;
+
for (j=0;j<message->elements[i].num_values;j++) {
- size += 4 + message->elements[i].values[j].length + 1;
+ if (size + 5 < size) {
+ errno = ENOMEM;
+ return -1;
+ }
+ size += 5;
+
+ value_len = message->elements[i].values[j].length;
+ if (size + value_len < size) {
+ errno = ENOMEM;
+ return -1;
+ }
+ size += value_len;
}
}
@@ -121,7 +155,7 @@ int ldb_pack_data(struct ldb_context *ldb,
/* the dn needs to be packed so we can be case preserving
while hashing on a case folded dn */
- len = strlen(dn);
+ len = dn_len;
memcpy(p, dn, len+1);
p += len + 1;
@@ -146,20 +180,64 @@ int ldb_pack_data(struct ldb_context *ldb,
return 0;
}
-/*
- unpack a ldb message from a linear buffer in ldb_val
+static bool ldb_consume_element_data(uint8_t **pp, size_t *premaining)
+{
+ unsigned int remaining = *premaining;
+ uint8_t *p = *pp;
+ uint32_t num_values = pull_uint32(p, 0);
+ uint32_t len;
+ int j;
+
+ p += 4;
+ if (remaining < 4) {
+ return false;
+ }
+ remaining -= 4;
+ for (j = 0; j < num_values; j++) {
+ len = pull_uint32(p, 0);
+ if (remaining < 5) {
+ return false;
+ }
+ remaining -= 5;
+ if (len > remaining) {
+ return false;
+ }
+ remaining -= len;
+ p += len + 4 + 1;
+ }
- Free with ldb_unpack_data_free()
-*/
-int ldb_unpack_data(struct ldb_context *ldb,
- const struct ldb_val *data,
- struct ldb_message *message)
+ *premaining = remaining;
+ *pp = p;
+ return true;
+}
+
+/*
+ * Unpack a ldb message from a linear buffer in ldb_val
+ *
+ * Providing a list of attributes to this function allows selective unpacking.
+ * Giving a NULL list (or a list_size of 0) unpacks all the attributes.
+ *
+ * Free with ldb_unpack_data_free()
+ */
+int ldb_unpack_data_only_attr_list(struct ldb_context *ldb,
+ const struct ldb_val *data,
+ struct ldb_message *message,
+ const char * const *list,
+ unsigned int list_size,
+ unsigned int *nb_elements_in_db)
{
uint8_t *p;
- unsigned int remaining;
+ size_t remaining;
+ size_t dn_len;
unsigned int i, j;
unsigned format;
+ unsigned int nelem = 0;
size_t len;
+ unsigned int found = 0;
+
+ if (list == NULL) {
+ list_size = 0;
+ }
message->elements = NULL;
@@ -172,6 +250,9 @@ int ldb_unpack_data(struct ldb_context *ldb,
format = pull_uint32(p, 0);
message->num_elements = pull_uint32(p, 4);
p += 8;
+ if (nb_elements_in_db) {
+ *nb_elements_in_db = message->num_elements;
+ }
remaining = data->length - 8;
@@ -181,8 +262,12 @@ int ldb_unpack_data(struct ldb_context *ldb,
break;
case LDB_PACKING_FORMAT:
- len = strnlen((char *)p, remaining);
- if (len == remaining) {
+ /*
+ * With this check, we know that the DN at p is \0
+ * terminated.
+ */
+ dn_len = strnlen((char *)p, remaining);
+ if (dn_len == remaining) {
errno = EIO;
goto failed;
}
@@ -191,8 +276,17 @@ int ldb_unpack_data(struct ldb_context *ldb,
errno = ENOMEM;
goto failed;
}
- remaining -= len + 1;
- p += len + 1;
+ /*
+ * Redundant: by definition, remaining must be more
+ * than one less than dn_len, as otherwise it would be
+ * == dn_len
+ */
+ if (remaining < dn_len + 1) {
+ errno = EIO;
+ goto failed;
+ }
+ remaining -= dn_len + 1;
+ p += dn_len + 1;
break;
default:
@@ -209,74 +303,157 @@ int ldb_unpack_data(struct ldb_context *ldb,
goto failed;
}
- message->elements = talloc_array(message, struct ldb_message_element,
message->num_elements);
+ message->elements = talloc_zero_array(message, struct
ldb_message_element,
+ message->num_elements);
if (!message->elements) {
errno = ENOMEM;
goto failed;
}
- memset(message->elements, 0,
- message->num_elements * sizeof(struct ldb_message_element));
-
for (i=0;i<message->num_elements;i++) {
+ const char *attr = NULL;
+ size_t attr_len;
+ struct ldb_message_element *element = NULL;
+
if (remaining < 10) {
errno = EIO;
goto failed;
}
- len = strnlen((char *)p, remaining-6);
- if (len == remaining-6) {
+ /*
+ * With this check, we know that the attribute name at
+ * p is \0 terminated.
+ */
+ attr_len = strnlen((char *)p, remaining-6);
+ if (attr_len == remaining-6) {
errno = EIO;
goto failed;
}
- if (len == 0) {
+ if (attr_len == 0) {
errno = EIO;
goto failed;
}
- message->elements[i].flags = 0;
- message->elements[i].name = talloc_strndup(message->elements,
(char *)p, len);
- if (message->elements[i].name == NULL) {
+ attr = (char *)p;
+
+ /*
+ * The general idea is to reduce allocations by skipping over
+ * attributes that we do not actually care about.
+ *
+ * This is a bit expensive but normally the list is pretty small
+ * also the cost of freeing unused attributes is quite important
+ * and can dwarf the cost of looping.
+ */
+ if (list_size != 0) {
+ bool keep = false;
+ int h;
+
+ /*
+ * We know that p has a \0 terminator before the
+ * end of the buffer due to the check above.
+ */
+ for (h = 0; h < list_size && found < list_size; h++) {
+ if (ldb_attr_cmp(attr, list[h]) == 0) {
+ keep = true;
+ found++;
+ break;
+ }
+ }
+
+ if (!keep) {
+ if (remaining < (attr_len + 1)) {
+ errno = EIO;
+ goto failed;
+ }
+ remaining -= attr_len + 1;
+ p += attr_len + 1;
+ if (!ldb_consume_element_data(&p, &remaining)) {
+ errno = EIO;
+ goto failed;
+ }
+ continue;
+ }
+ }
+ element = &message->elements[nelem];
+ element->name = talloc_memdup(message->elements, attr,
attr_len+1);
+
+ if (element->name == NULL) {
errno = ENOMEM;
goto failed;
}
- remaining -= len + 1;
- p += len + 1;
- message->elements[i].num_values = pull_uint32(p, 0);
- message->elements[i].values = NULL;
- if (message->elements[i].num_values != 0) {
- message->elements[i].values =
talloc_array(message->elements,
- struct
ldb_val,
-
message->elements[i].num_values);
- if (!message->elements[i].values) {
+ element->flags = 0;
+
+ if (remaining < (attr_len + 1)) {
+ errno = EIO;
+ goto failed;
+ }
+ remaining -= attr_len + 1;
+ p += attr_len + 1;
+ element->num_values = pull_uint32(p, 0);
+ element->values = NULL;
+ if (element->num_values != 0) {
+ element->values = talloc_array(message->elements,
+ struct ldb_val,
+ element->num_values);
+ if (!element->values) {
errno = ENOMEM;
goto failed;
}
}
p += 4;
+ if (remaining < 4) {
+ errno = EIO;
+ goto failed;
+ }
remaining -= 4;
- for (j=0;j<message->elements[i].num_values;j++) {
+ for (j = 0; j < element->num_values; j++) {
+ if (remaining < 5) {
+ errno = EIO;
+ goto failed;
+ }
+ remaining -= 5;
+
len = pull_uint32(p, 0);
- if (len > remaining-5) {
+ if (remaining < len) {
+ errno = EIO;
+ goto failed;
+ }
+ if (len + 1 < len) {
errno = EIO;
goto failed;
}
- message->elements[i].values[j].length = len;
- message->elements[i].values[j].data =
talloc_size(message->elements[i].values, len+1);
- if (message->elements[i].values[j].data == NULL) {
+ element->values[j].length = len;
+ element->values[j].data = talloc_size(element->values,
len+1);
+ if (element->values[j].data == NULL) {
errno = ENOMEM;
goto failed;
}
- memcpy(message->elements[i].values[j].data, p+4, len);
- message->elements[i].values[j].data[len] = 0;
+ memcpy(element->values[j].data, p + 4,
+ len);
+ element->values[j].data[len] = 0;
- remaining -= len+4+1;
+ remaining -= len;
p += len+4+1;
}
+ nelem++;
}
+ /*
+ * Adapt the number of elements to the real number of unpacked elements,
+ * it means that we overallocated elements array.
+ */
+ message->num_elements = nelem;
+
+ /*
+ * Shrink the allocated size. On current talloc behaviour
+ * this will help if we skipped 32 or more attributes.
+ */
+ message->elements = talloc_realloc(message, message->elements,
+ struct ldb_message_element,
+ message->num_elements);
if (remaining != 0) {
ldb_debug(ldb, LDB_DEBUG_ERROR,
- "Error: %d bytes unread in ldb_unpack_data",
remaining);
+ "Error: %zu bytes unread in
ldb_unpack_data_only_attr_list",
+ remaining);
}
return 0;
@@ -285,3 +462,10 @@ failed:
talloc_free(message->elements);
return -1;
}
+
+int ldb_unpack_data(struct ldb_context *ldb,
+ const struct ldb_val *data,
+ struct ldb_message *message)
+{
+ return ldb_unpack_data_only_attr_list(ldb, data, message, NULL, 0,
NULL);
+}
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index f00ba50..c6a24d3 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -390,6 +390,12 @@ int ldb_register_extended_match_rule(struct ldb_context
*ldb,
int ldb_pack_data(struct ldb_context *ldb,
const struct ldb_message *message,
struct ldb_val *data);
+int ldb_unpack_data_only_attr_list(struct ldb_context *ldb,
+ const struct ldb_val *data,
+ struct ldb_message *message,
+ const char* const * list,
+ unsigned int list_size,
+ unsigned int *nb_attributes_indb);
int ldb_unpack_data(struct ldb_context *ldb,
const struct ldb_val *data,
struct ldb_message *message);
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 2796243..6ff0c7c 100755
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'ldb'
-VERSION = '1.1.24'
+VERSION = '1.1.25'
blddir = 'bin'
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index cbd54c1..fbefda7 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -89,7 +89,10 @@ sub check_or_start($$$)
my ($self, $env_vars, $process_model) = @_;
my $STDIN_READER;
- return 0 if $self->check_env($env_vars);
+ my $env_ok = $self->check_env($env_vars);
+ if ($env_ok) {
+ return $env_vars->{SAMBA_PID};
+ }
# use a pipe for stdin in the child processes. This allows
# those processes to monitor the pipe for EOF to ensure they
@@ -150,17 +153,28 @@ sub check_or_start($$$)
exec(@preargs, Samba::bindir_path($self, "samba"), "-M",
$process_model, "-i", "--maximum-runtime=$self->{server_maxtime}",
$env_vars->{CONFIGURATION}, @optargs) or die("Unable to start samba: $!");
}
$env_vars->{SAMBA_PID} = $pid;
- print "DONE\n";
+ print "DONE ($pid)\n";
close($STDIN_READER);
+ if ($self->wait_for_start($env_vars) != 0) {
+ warn("Samba $pid failed to start up");
+ return undef;
+ }
+
return $pid;
}
sub wait_for_start($$)
{
my ($self, $testenv_vars) = @_;
- my $ret;
+ my $ret = 0;
+
+ if (not $self->check_env($testenv_vars)) {
+ warn("unable to confirm Samba $testenv_vars->{SAMBA_PID} is
running");
+ return -1;
+ }
+
# give time for nbt server to register its names
print "delaying for nbt name registration\n";
sleep 2;
@@ -200,7 +214,7 @@ sub wait_for_start($$)
while (system("$ldbsearch -H ldap://$testenv_vars->{SERVER}
-U$testenv_vars->{USERNAME}%$testenv_vars->{PASSWORD} -s base -b
\"$rid_set_dn\" rIDAllocationPool > /dev/null") != 0) {
$count++;
if ($count > 40) {
- $ret = 1;
+ $ret = -1;
last;
}
sleep(1);
@@ -2002,11 +2016,19 @@ sub check_env($$)
my ($self, $envvars) = @_;
--
Samba Shared Repository
