The branch, v4-12-test has been updated
       via  3db89a6c880 ldb_ldap: fix off-by-one increment in lldb_add_msg_attr
       via  1049c5776f8 lib/ldb: add unit test for ldb_ldap internal code
       via  0ac77960440 Fix usage of ldap_get_values_len for 
msDS-AdditionalDnsHostName
       via  0e668997c83 Properly handle msDS-AdditionalDnsHostName returned 
from Windows DC
       via  60e73282191 selftest: add tests for binary 
msDS-AdditionalDnsHostName
       via  63c70acd4f4 Fix a typo in recent net man page changes
       via  f50cb3a0fbf libcli ldap tests: remove use of zero length array
      from  7b1bac7d084 Add net-ads-join dnshostname=fqdn option

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-12-test


- Log -----------------------------------------------------------------
commit 3db89a6c8809fd7ff733ab1b7d401ea202f29ab0
Author: Alexander Bokovoy <[email protected]>
Date:   Thu Jun 18 10:45:41 2020 +0300

    ldb_ldap: fix off-by-one increment in lldb_add_msg_attr
    
    Fix regression introduced by commit ce2bf5c72b6423fff680b3d6a9042103a6cdda55
    
    lldb_add_msg_attr() calls ldb_msg_add_empty() which, in turn, calls
    calls _ldb_msg_add_el() which already increments msg->num_elements by one.
    
    As a result, msg->num_elements is bigger than the actual number of
    elements and any iteration over elements would step over elements array
    boundary.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14413
    Signed-off-by: Alexander Bokovoy <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>
    
    Autobuild-User(master): Andreas Schneider <[email protected]>
    Autobuild-Date(master): Fri Jun 19 08:35:33 UTC 2020 on sn-devel-184
    
    (cherry picked from commit 990a0fc4a0481aed817fad7575d8df453fbe7af9)
    
    Autobuild-User(v4-12-test): Karolin Seeger <[email protected]>
    Autobuild-Date(v4-12-test): Wed Jun 24 11:22:16 UTC 2020 on sn-devel-184

commit 1049c5776f8b28ac15c3752eb2becee75e15cd45
Author: Alexander Bokovoy <[email protected]>
Date:   Thu Jun 18 11:49:08 2020 +0300

    lib/ldb: add unit test for ldb_ldap internal code
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14413
    Signed-off-by: Alexander Bokovoy <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>
    (cherry picked from commit 36bd6edd8a603f3aa34aff81c43ef26efd3ad4cf)

commit 0ac77960440f69e47ad52f134ecc95133c3c2353
Author: Isaac Boukris <[email protected]>
Date:   Sat Jun 20 17:17:33 2020 +0200

    Fix usage of ldap_get_values_len for msDS-AdditionalDnsHostName
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14406
    
    Signed-off-by: Isaac Boukris <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>
    
    Autobuild-User(master): Andreas Schneider <[email protected]>
    Autobuild-Date(master): Mon Jun 22 09:59:04 UTC 2020 on sn-devel-184

commit 0e668997c83bcd4c736a86bd78803992225fd4dd
Author: Isaac Boukris <[email protected]>
Date:   Thu Jun 11 16:51:27 2020 +0300

    Properly handle msDS-AdditionalDnsHostName returned from Windows DC
    
    Windows DC adds short names for each specified msDS-AdditionalDnsHostName
    attribute, but these have a suffix of "\0$" and thus fail with
    ldap_get_values(), use ldap_get_values_len() instead.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14406
    
    Signed-off-by: Isaac Boukris <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>

commit 60e7328219169d949e5a864e322fb2de57c4ffd3
Author: Isaac Boukris <[email protected]>
Date:   Tue Jun 16 22:01:49 2020 +0300

    selftest: add tests for binary msDS-AdditionalDnsHostName
    
    Like the short names added implicitly by Windows DC.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14406
    
    Signed-off-by: Isaac Boukris <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>

commit 63c70acd4f473d709c25a7c35398db98d4504981
Author: Isaac Boukris <[email protected]>
Date:   Thu Jun 11 21:05:07 2020 +0300

    Fix a typo in recent net man page changes
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14406
    
    Signed-off-by: Isaac Boukris <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>

commit f50cb3a0fbf403a8c40329a446d15648a04a3c79
Author: Gary Lockyer <[email protected]>
Date:   Mon Jun 22 13:42:56 2020 +1200

    libcli ldap tests: remove use of zero length array
    
    libcli/ldap/tests/ldap_message_test.c defines a zero length array
    (uint8_t buf[0]), which is a GCC extension and breaks the build with
    some strict compilers like xlc.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14387
    
    Signed-off-by: Gary Lockyer <[email protected]>
    Reviewed-by: Andrew Bartlett <[email protected]>
    
    Autobuild-User(master): Andrew Bartlett <[email protected]>
    Autobuild-Date(master): Tue Jun 23 02:15:35 UTC 2020 on sn-devel-184
    
    (cherry picked from commit d701bc1518766f36b1c7a3a00a82485098a8ee3d)

-----------------------------------------------------------------------

Summary of changes:
 docs-xml/manpages/net.8.xml           |   2 +-
 lib/ldb/ldb_ldap/ldb_ldap.c           |   2 -
 lib/ldb/tests/lldb_ldap.c             | 105 ++++++++++++++++++++++++++++++++++
 lib/ldb/wscript                       |  14 +++++
 libcli/ldap/tests/ldap_message_test.c |   2 +-
 source3/libads/ldap.c                 |  42 +++++++++++++-
 testprogs/blackbox/test_net_ads.sh    |  22 +++++++
 7 files changed, 182 insertions(+), 7 deletions(-)
 create mode 100644 lib/ldb/tests/lldb_ldap.c


Changeset truncated at 500 lines:

diff --git a/docs-xml/manpages/net.8.xml b/docs-xml/manpages/net.8.xml
index cbab9c63a5e..951ddcd7c3a 100644
--- a/docs-xml/manpages/net.8.xml
+++ b/docs-xml/manpages/net.8.xml
@@ -497,7 +497,7 @@ joining the domain.
 </para>
 
 <para>
-[FQDN] (ADS only) set the dnsHosName attribute during the join.
+[FQDN] (ADS only) set the dnsHostName attribute during the join.
 The default format is netbiosname.dnsdomain.
 </para>
 
diff --git a/lib/ldb/ldb_ldap/ldb_ldap.c b/lib/ldb/ldb_ldap/ldb_ldap.c
index d7222997732..0531f8a62ae 100644
--- a/lib/ldb/ldb_ldap/ldb_ldap.c
+++ b/lib/ldb/ldb_ldap/ldb_ldap.c
@@ -176,8 +176,6 @@ static int lldb_add_msg_attr(struct ldb_context *ldb,
                el->num_values++;
        }
 
-       msg->num_elements++;
-
        return 0;
 }
 
diff --git a/lib/ldb/tests/lldb_ldap.c b/lib/ldb/tests/lldb_ldap.c
new file mode 100644
index 00000000000..eea9f22f6b9
--- /dev/null
+++ b/lib/ldb/tests/lldb_ldap.c
@@ -0,0 +1,105 @@
+/*
+ * 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 <errno.h>
+#include <unistd.h>
+#include <talloc.h>
+
+#include <ldb.h>
+#include <ldb_private.h>
+#include <string.h>
+#include <ctype.h>
+
+int ldb_ldap_init(const char *version);
+
+#include "ldb_ldap/ldb_ldap.c"
+
+struct test_ctx {
+       struct tevent_context *ev;
+       struct ldb_context *ldb;
+       struct ldb_message *msg;
+};
+
+static int lldb_msg_setup(void **state)
+{
+       struct test_ctx *test_ctx;
+
+       test_ctx = talloc_zero(NULL, struct test_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);
+
+       test_ctx->msg = ldb_msg_new(test_ctx);
+       assert_non_null(test_ctx->msg);
+
+       *state = test_ctx;
+       return 0;
+}
+
+static int lldb_msg_teardown(void **state)
+{
+       struct test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                         struct test_ctx);
+
+       talloc_free(test_ctx);
+       return 0;
+}
+
+static void test_lldb_add_msg_attr(void **state)
+{
+       struct test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                         struct test_ctx);
+       struct ldb_message *msg = test_ctx->msg;
+       int ret;
+       unsigned int num_elements = 0;
+       struct berval **v = NULL;
+
+       v = talloc_zero_array(test_ctx, struct berval *, 2);
+       assert_non_null(v);
+
+       v[0] = talloc_zero(v, struct berval);
+       assert_non_null(v[0]);
+
+       v[0]->bv_val = talloc_strdup(msg, "dc=example,dc=test");
+       assert_non_null(v[0]->bv_val);
+
+       v[0]->bv_len = strlen(v[0]->bv_val);
+
+       num_elements = msg->num_elements;
+
+       ret = lldb_add_msg_attr(test_ctx->ldb, msg, "defaultNamingContext", v);
+       assert_int_equal(ret, LDB_SUCCESS);
+       assert_int_equal(msg->num_elements, num_elements + 1);
+}
+
+
+int main(int argc, const char **argv)
+{
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test_setup_teardown(test_lldb_add_msg_attr,
+                                               lldb_msg_setup,
+                                               lldb_msg_teardown),
+       };
+
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 86b83c1b5cc..526fe497c13 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -527,6 +527,15 @@ def build(bld):
                          deps='cmocka ldb ldb_tdb_err_map',
                          install=False)
 
+        # If both libldap and liblber are available, test ldb_ldap
+        # code for a regression of bz#14413 -- even if we don't build
+        # it ourselves and simply using the system version
+        if bld.env.LIB_LDAP and bld.env.LIB_LBER:
+            bld.SAMBA_BINARY('lldb_ldap_test',
+                             source='tests/lldb_ldap.c',
+                             deps='cmocka talloc lber ldap ldb',
+                             install=False)
+
         if bld.CONFIG_SET('HAVE_LMDB'):
             bld.SAMBA_BINARY('ldb_mdb_mod_op_test',
                              source='tests/ldb_mod_op_test.c',
@@ -628,6 +637,11 @@ def test(ctx):
                  # 'ldb_key_value_sub_txn_tdb_test'
                  'ldb_parse_test']
 
+    # if LIB_LDAP and LIB_LBER defined, then we can test ldb_ldap backend
+    # behavior regression for bz#14413
+    if env.LIB_LDAP and env.LIB_LBER:
+        test_exes += ["lldb_ldap_test"]
+
     if env.HAVE_LMDB:
         test_exes += ['ldb_mdb_mod_op_test',
                      'ldb_lmdb_test',
diff --git a/libcli/ldap/tests/ldap_message_test.c 
b/libcli/ldap/tests/ldap_message_test.c
index c5aacd4bc6b..f4b49bc47bc 100644
--- a/libcli/ldap/tests/ldap_message_test.c
+++ b/libcli/ldap/tests/ldap_message_test.c
@@ -115,7 +115,7 @@ static void test_empty_input(void **state)
        struct asn1_data *asn1;
        struct ldap_message *ldap_msg;
        NTSTATUS status;
-       uint8_t buf[0];
+       uint8_t *buf = NULL;
        size_t len = 0;
        struct ldap_request_limits limits = {
                .max_search_size = 256000,
diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index f6fde5e19e1..7ef7e7e8420 100755
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -3669,6 +3669,44 @@ out:
 /********************************************************************
 ********************************************************************/
 
+static char **get_addl_hosts(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx,
+                             LDAPMessage *msg, size_t *num_values)
+{
+       const char *field = "msDS-AdditionalDnsHostName";
+       struct berval **values = NULL;
+       char **ret = NULL;
+       size_t i, converted_size;
+
+       values = ldap_get_values_len(ads->ldap.ld, msg, field);
+       if (values == NULL) {
+               return NULL;
+       }
+
+       *num_values = ldap_count_values_len(values);
+
+       ret = talloc_array(mem_ctx, char *, *num_values + 1);
+       if (ret == NULL) {
+               ldap_value_free_len(values);
+               return NULL;
+       }
+
+       for (i = 0; i < *num_values; i++) {
+               ret[i] = NULL;
+               if (!convert_string_talloc(mem_ctx, CH_UTF8, CH_UNIX,
+                                          values[i]->bv_val,
+                                          strnlen(values[i]->bv_val,
+                                                  values[i]->bv_len),
+                                          &ret[i], &converted_size)) {
+                       ldap_value_free_len(values);
+                       return NULL;
+               }
+       }
+       ret[i] = NULL;
+
+       ldap_value_free_len(values);
+       return ret;
+}
+
 ADS_STATUS ads_get_additional_dns_hostnames(TALLOC_CTX *mem_ctx,
                                            ADS_STRUCT *ads,
                                            const char *machine_name,
@@ -3694,9 +3732,7 @@ ADS_STATUS ads_get_additional_dns_hostnames(TALLOC_CTX 
*mem_ctx,
                goto done;
        }
 
-       *hostnames_array = ads_pull_strings(ads, mem_ctx, res,
-                                           "msDS-AdditionalDnsHostName",
-                                           num_hostnames);
+       *hostnames_array = get_addl_hosts(ads, mem_ctx, res, num_hostnames);
        if (*hostnames_array == NULL) {
                DEBUG(1, ("Host account for %s does not have 
msDS-AdditionalDnsHostName.\n",
                          machine_name));
diff --git a/testprogs/blackbox/test_net_ads.sh 
b/testprogs/blackbox/test_net_ads.sh
index 85257f445d8..eef4a31a6a7 100755
--- a/testprogs/blackbox/test_net_ads.sh
+++ b/testprogs/blackbox/test_net_ads.sh
@@ -41,6 +41,11 @@ if [ -x "$BINDIR/ldbdel" ]; then
        ldbdel="$BINDIR/ldbdel"
 fi
 
+ldbmodify="ldbmodify"
+if [ -x "$BINDIR/ldbmodify" ]; then
+       ldbmodify="$BINDIR/ldbmodify"
+fi
+
 # Load test functions
 . `dirname $0`/subunit.sh
 
@@ -217,12 +222,29 @@ testit_grep "dns alias SPN" $dns_alias2 $VALGRIND 
$net_tool ads search -P samacc
 testit_grep "dns alias addl" $dns_alias1 $VALGRIND $net_tool ads search -P 
samaccountname=$netbios\$ msDS-AdditionalDnsHostName || failed=`expr $failed + 
1`
 testit_grep "dns alias addl" $dns_alias2 $VALGRIND $net_tool ads search -P 
samaccountname=$netbios\$ msDS-AdditionalDnsHostName || failed=`expr $failed + 
1`
 
+# Test binary msDS-AdditionalDnsHostName like ones added by Windows DC
+short_alias_file="$PREFIX_ABS/short_alias_file"
+printf 'short_alias\0$' > $short_alias_file
+cat > $PREFIX_ABS/tmpldbmodify <<EOF
+dn: CN=$HOSTNAME,$computers_dn
+changetype: modify
+add: msDS-AdditionalDnsHostName
+msDS-AdditionalDnsHostName:< file://$short_alias_file
+EOF
+
+testit "add binary msDS-AdditionalDnsHostName" $VALGRIND $ldbmodify -k yes 
-U$DC_USERNAME%$DC_PASSWORD -H ldap://$SERVER.$REALM $PREFIX_ABS/tmpldbmodify 
|| failed=`expr $failed + 1`
+
+testit_grep "addl short alias" short_alias $ldbsearch --show-binary 
-U$DC_USERNAME%$DC_PASSWORD -H ldap://$SERVER.$REALM -s base -b 
"CN=$HOSTNAME,CN=Computers,$base_dn" msDS-AdditionalDnsHostName || failed=`expr 
$failed + 1`
+
+rm -f $PREFIX_ABS/tmpldbmodify $short_alias_file
+
 dedicated_keytab_file="$PREFIX_ABS/test_dns_aliases_dedicated_krb5.keytab"
 
 testit "dns alias create_keytab" $VALGRIND $net_tool ads keytab create 
--option="kerberosmethod=dedicatedkeytab" 
--option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 
1`
 
 testit_grep "dns alias1 check keytab" "host/${dns_alias1}@$REALM" $net_tool 
ads keytab list --option="kerberosmethod=dedicatedkeytab" 
--option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 
1`
 testit_grep "dns alias2 check keytab" "host/${dns_alias2}@$REALM" $net_tool 
ads keytab list --option="kerberosmethod=dedicatedkeytab" 
--option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 
1`
+testit_grep "addl short check keytab" "host/short_alias@$REALM" $net_tool ads 
keytab list --option="kerberosmethod=dedicatedkeytab" 
--option="dedicatedkeytabfile=$dedicated_keytab_file" || failed=`expr $failed + 
1`
 
 rm -f $dedicated_keytab_file
 


-- 
Samba Shared Repository

Reply via email to