[PATCH] Fix linking of vfstest and smbd with latest gold linker
Jeremy Allison
jra at samba.org
Thu Nov 17 17:24:37 UTC 2016
On Thu, Nov 17, 2016 at 04:39:32PM +0100, Andreas Schneider wrote:
> On Thursday, 17 November 2016 10:51:51 CET Volker Lendecke wrote:
> > On Thu, Nov 17, 2016 at 10:23:34AM +0100, Andreas Schneider wrote:
> > > make test TESTS="samba4.rpc.lsa.secrets" SAMBA_OPTIONS=-d9
> > >
> > > All tests pass
> > >
> > > This is really bizarre. Race condition?
> >
> > Indeed. Sorry, but I am too busy right now to dig into it.
> >
> > Volker
>
> I've looked into the issue with the test I was able to reproduce here. The fix
> is in the attached fix_lsa_session_key.patch. (random() is not random enough
> :-)).
>
> Please take a look and push it if you're fine with it. I would just push that
> patchset alone first.
Oh good catch ! Thanks Andreas. I'll push the lsa fix first
and if that gets through the other queued up patches.
> Then the gold.patch and see what happens ...
>
>
> Andreas
>
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> From dae8485a425cc13bfa2848dc42ef67898206a3f7 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Thu, 17 Nov 2016 15:35:47 +0100
> Subject: [PATCH 1/3] s4:torture: Strip trailing whitespaces in session_key.c
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12433
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Guenther Deschner <gd at samba.org>
> ---
> source4/torture/rpc/session_key.c | 58 +++++++++++++++++++--------------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/source4/torture/rpc/session_key.c b/source4/torture/rpc/session_key.c
> index 11f6a0b..b460036 100644
> --- a/source4/torture/rpc/session_key.c
> +++ b/source4/torture/rpc/session_key.c
> @@ -1,20 +1,20 @@
> -/*
> +/*
> Unix SMB/CIFS implementation.
> test suite for lsa rpc operations
>
> Copyright (C) Andrew Tridgell 2003
> Copyright (C) Andrew Bartlett <abartlet at samba.org> 2004-2005
> -
> +
> 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/>.
> */
> @@ -32,7 +32,7 @@ static void init_lsa_String(struct lsa_String *name, const char *s)
> name->string = s;
> }
>
> -static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> +static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> struct torture_context *tctx,
> struct policy_handle *handle)
> {
> @@ -56,66 +56,66 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> secname = talloc_asprintf(tctx, "torturesecret-%u", (unsigned int)random());
>
> torture_comment(tctx, "Testing CreateSecret of %s\n", secname);
> -
> +
> init_lsa_String(&r.in.name, secname);
> -
> +
> r.in.handle = handle;
> r.in.access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
> r.out.sec_handle = &sec_handle;
> -
> +
> torture_assert_ntstatus_ok(tctx, dcerpc_lsa_CreateSecret_r(b, tctx, &r),
> "CreateSecret failed");
> torture_assert_ntstatus_ok(tctx, r.out.result, "CreateSecret failed");
> -
> +
> status = dcerpc_fetch_session_key(p, &session_key);
> torture_assert_ntstatus_ok(tctx, status, "dcerpc_fetch_session_key failed");
> -
> +
> enc_key = sess_encrypt_string(secret1, &session_key);
> -
> +
> r3.in.sec_handle = &sec_handle;
> r3.in.new_val = &buf1;
> r3.in.old_val = NULL;
> r3.in.new_val->data = enc_key.data;
> r3.in.new_val->length = enc_key.length;
> r3.in.new_val->size = enc_key.length;
> -
> +
> torture_comment(tctx, "Testing SetSecret\n");
> -
> +
> torture_assert_ntstatus_ok(tctx, dcerpc_lsa_SetSecret_r(b, tctx, &r3),
> "SetSecret failed");
> torture_assert_ntstatus_ok(tctx, r3.out.result, "SetSecret failed");
> -
> +
> r3.in.sec_handle = &sec_handle;
> r3.in.new_val = &buf1;
> r3.in.old_val = NULL;
> r3.in.new_val->data = enc_key.data;
> r3.in.new_val->length = enc_key.length;
> r3.in.new_val->size = enc_key.length;
> -
> +
> /* break the encrypted data */
> enc_key.data[0]++;
> -
> +
> torture_comment(tctx, "Testing SetSecret with broken key\n");
> -
> +
> torture_assert_ntstatus_ok(tctx, dcerpc_lsa_SetSecret_r(b, tctx, &r3),
> "SetSecret failed");
> torture_assert_ntstatus_equal(tctx, r3.out.result, NT_STATUS_UNKNOWN_REVISION,
> "SetSecret should have failed UNKNOWN_REVISION");
> -
> +
> data_blob_free(&enc_key);
> -
> +
> ZERO_STRUCT(new_mtime);
> ZERO_STRUCT(old_mtime);
> -
> +
> /* fetch the secret back again */
> r4.in.sec_handle = &sec_handle;
> r4.in.new_val = &bufp1;
> r4.in.new_mtime = &new_mtime;
> r4.in.old_val = NULL;
> r4.in.old_mtime = NULL;
> -
> +
> bufp1.buf = NULL;
> -
> +
> torture_comment(tctx, "Testing QuerySecret\n");
> torture_assert_ntstatus_ok(tctx, dcerpc_lsa_QuerySecret_r(b, tctx, &r4),
> "QuerySecret failed");
> @@ -126,7 +126,7 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> blob1.length = r4.out.new_val->buf->size;
>
> secret2 = sess_decrypt_string(tctx, &blob1, &session_key);
> -
> +
> torture_assert_str_equal(tctx, secret1, secret2, "Returned secret invalid");
>
> d.in.handle = &sec_handle;
> @@ -149,7 +149,7 @@ static bool test_secrets(struct torture_context *torture, const void *_data)
> struct dcerpc_pipe *p;
> struct policy_handle *handle;
> struct dcerpc_binding *binding;
> - const struct secret_settings *settings =
> + const struct secret_settings *settings =
> (const struct secret_settings *)_data;
> NTSTATUS status;
> struct dcerpc_binding_handle *b;
> @@ -158,7 +158,7 @@ static bool test_secrets(struct torture_context *torture, const void *_data)
> lpcfg_set_cmdline(torture->lp_ctx, "ntlmssp_client:ntlm2", settings->ntlm2?"True":"False");
> lpcfg_set_cmdline(torture->lp_ctx, "ntlmssp_client:lm_key", settings->lm_key?"True":"False");
>
> - torture_assert_ntstatus_ok(torture, torture_rpc_binding(torture, &binding),
> + torture_assert_ntstatus_ok(torture, torture_rpc_binding(torture, &binding),
> "Getting bindoptions");
>
> status = dcerpc_binding_set_flags(binding, settings->bindoptions, 0);
> @@ -179,7 +179,7 @@ static bool test_secrets(struct torture_context *torture, const void *_data)
> }
>
> torture_assert(torture, handle, "OpenPolicy2 failed. This test cannot run against this server");
> -
> +
> if (!test_CreateSecret_basic(p, torture, handle)) {
> talloc_free(p);
> return false;
> @@ -190,7 +190,7 @@ static bool test_secrets(struct torture_context *torture, const void *_data)
> return true;
> }
>
> -static struct torture_tcase *add_test(struct torture_suite *suite, uint32_t bindoptions,
> +static struct torture_tcase *add_test(struct torture_suite *suite, uint32_t bindoptions,
> bool keyexchange, bool ntlm2, bool lm_key)
> {
> char *name = NULL;
> @@ -203,7 +203,7 @@ static struct torture_tcase *add_test(struct torture_suite *suite, uint32_t bind
> name = talloc_strdup(suite, "bigendian");
> else if (bindoptions == DCERPC_SEAL)
> name = talloc_strdup(suite, "seal");
> - else if (bindoptions == 0)
> + else if (bindoptions == 0)
> name = talloc_strdup(suite, "none");
> else
> name = talloc_strdup(suite, "unknown");
> @@ -232,7 +232,7 @@ struct torture_suite *torture_rpc_lsa_secrets(TALLOC_CTX *mem_ctx)
> for (keyexchange = 0; keyexchange < ARRAY_SIZE(bool_vals); keyexchange++) {
> for (ntlm2 = 0; ntlm2 < ARRAY_SIZE(bool_vals); ntlm2++) {
> for (lm_key = 0; lm_key < ARRAY_SIZE(bool_vals); lm_key++) {
> - add_test(suite, DCERPC_PUSH_BIGENDIAN, bool_vals[keyexchange], bool_vals[ntlm2],
> + add_test(suite, DCERPC_PUSH_BIGENDIAN, bool_vals[keyexchange], bool_vals[ntlm2],
> bool_vals[lm_key]);
> add_test(suite, DCERPC_SEAL, bool_vals[keyexchange], bool_vals[ntlm2], bool_vals[lm_key]);
> add_test(suite, 0, bool_vals[keyexchange], bool_vals[ntlm2], bool_vals[lm_key]);
> --
> 2.10.2
>
>
> From 6c2ae64c61a78c79fc3e7232fcdba7ce0de39bb7 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Thu, 17 Nov 2016 15:44:13 +0100
> Subject: [PATCH 2/3] s4:torture: Normalizes names in session_key test
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12433
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Guenther Deschner <gd at samba.org>
> ---
> source4/torture/rpc/session_key.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source4/torture/rpc/session_key.c b/source4/torture/rpc/session_key.c
> index b460036..7ddc339 100644
> --- a/source4/torture/rpc/session_key.c
> +++ b/source4/torture/rpc/session_key.c
> @@ -53,7 +53,7 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> char *secname;
> struct dcerpc_binding_handle *b = p->binding_handle;
>
> - secname = talloc_asprintf(tctx, "torturesecret-%u", (unsigned int)random());
> + secname = talloc_asprintf(tctx, "torturesecret-%08x", (unsigned int)random());
>
> torture_comment(tctx, "Testing CreateSecret of %s\n", secname);
>
> --
> 2.10.2
>
>
> From 62c0a1e6b13358343daec28b89a710295d7ffca7 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Thu, 17 Nov 2016 16:15:54 +0100
> Subject: [PATCH 3/3] s4:torture: Fix cleanup of the secrets object in
> session_key test
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12433
>
> The test is known to be failing if sealing is turned on in some
> circumstances. In this case a secret is created and then the function
> dcerpc_fetch_session_key() fails. The secret is not removed!
>
> We use torturesecret-%08x with random() to fill in the number. Sometimes
> it happens that random() returns a number we already used. So we end up
> trying to create a secret for an entry which already exists and run
> into a collision
>
> This change makes sure we always cleanup behind us and do not leave
> secret objects we created.
>
> Pair-Programmed-With: Guenther Deschner <gd at samba.org>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Signed-off-by: Guenther Deschner <gd at samba.org>
> ---
> source4/torture/rpc/session_key.c | 40 ++++++++++++++++++++++-----------------
> 1 file changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/source4/torture/rpc/session_key.c b/source4/torture/rpc/session_key.c
> index 7ddc339..3b7ba1f 100644
> --- a/source4/torture/rpc/session_key.c
> +++ b/source4/torture/rpc/session_key.c
> @@ -34,14 +34,13 @@ static void init_lsa_String(struct lsa_String *name, const char *s)
>
> static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> struct torture_context *tctx,
> - struct policy_handle *handle)
> + struct policy_handle *handle,
> + struct policy_handle *sec_handle)
> {
> NTSTATUS status;
> struct lsa_CreateSecret r;
> struct lsa_SetSecret r3;
> struct lsa_QuerySecret r4;
> - struct policy_handle sec_handle;
> - struct lsa_DeleteObject d;
> struct lsa_DATA_BUF buf1;
> struct lsa_DATA_BUF_PTR bufp1;
> DATA_BLOB enc_key;
> @@ -61,7 +60,7 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
>
> r.in.handle = handle;
> r.in.access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
> - r.out.sec_handle = &sec_handle;
> + r.out.sec_handle = sec_handle;
>
> torture_assert_ntstatus_ok(tctx, dcerpc_lsa_CreateSecret_r(b, tctx, &r),
> "CreateSecret failed");
> @@ -72,7 +71,7 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
>
> enc_key = sess_encrypt_string(secret1, &session_key);
>
> - r3.in.sec_handle = &sec_handle;
> + r3.in.sec_handle = sec_handle;
> r3.in.new_val = &buf1;
> r3.in.old_val = NULL;
> r3.in.new_val->data = enc_key.data;
> @@ -85,7 +84,7 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> "SetSecret failed");
> torture_assert_ntstatus_ok(tctx, r3.out.result, "SetSecret failed");
>
> - r3.in.sec_handle = &sec_handle;
> + r3.in.sec_handle = sec_handle;
> r3.in.new_val = &buf1;
> r3.in.old_val = NULL;
> r3.in.new_val->data = enc_key.data;
> @@ -108,7 +107,7 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
> ZERO_STRUCT(old_mtime);
>
> /* fetch the secret back again */
> - r4.in.sec_handle = &sec_handle;
> + r4.in.sec_handle = sec_handle;
> r4.in.new_val = &bufp1;
> r4.in.new_mtime = &new_mtime;
> r4.in.old_val = NULL;
> @@ -129,11 +128,6 @@ static bool test_CreateSecret_basic(struct dcerpc_pipe *p,
>
> torture_assert_str_equal(tctx, secret1, secret2, "Returned secret invalid");
>
> - d.in.handle = &sec_handle;
> - d.out.handle = &sec_handle;
> - torture_assert_ntstatus_ok(tctx, dcerpc_lsa_DeleteObject_r(b, tctx, &d),
> - "DeleteObject failed");
> - torture_assert_ntstatus_ok(tctx, d.out.result, "delete should have returned OKINVALID_HANDLE");
> return true;
> }
>
> @@ -153,6 +147,8 @@ static bool test_secrets(struct torture_context *torture, const void *_data)
> (const struct secret_settings *)_data;
> NTSTATUS status;
> struct dcerpc_binding_handle *b;
> + struct policy_handle sec_handle = {0};
> + bool ok;
>
> lpcfg_set_cmdline(torture->lp_ctx, "ntlmssp client:keyexchange", settings->keyexchange?"True":"False");
> lpcfg_set_cmdline(torture->lp_ctx, "ntlmssp_client:ntlm2", settings->ntlm2?"True":"False");
> @@ -180,14 +176,24 @@ static bool test_secrets(struct torture_context *torture, const void *_data)
>
> torture_assert(torture, handle, "OpenPolicy2 failed. This test cannot run against this server");
>
> - if (!test_CreateSecret_basic(p, torture, handle)) {
> - talloc_free(p);
> - return false;
> + ok = test_CreateSecret_basic(p, torture, handle, &sec_handle);
> +
> + if (is_valid_policy_hnd(&sec_handle)) {
> + struct lsa_DeleteObject d;
> +
> + d.in.handle = &sec_handle;
> + d.out.handle = &sec_handle;
> +
> + status = dcerpc_lsa_DeleteObject_r(b, torture, &d);
> + if (!NT_STATUS_IS_OK(status) ||
> + !NT_STATUS_IS_OK(d.out.result)) {
> + torture_warning(torture,
> + "Failed to delete secrets object");
> + }
> }
>
> talloc_free(p);
> -
> - return true;
> + return ok;
> }
>
> static struct torture_tcase *add_test(struct torture_suite *suite, uint32_t bindoptions,
> --
> 2.10.2
>
> From a4b6cf912129d630d030c8fc344c052b49eaa3ad Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 16 Nov 2016 10:17:46 +0100
> Subject: [PATCH 1/3] s3:waf: Add missing param dependency to vfstest
>
> Detected by gold linker version 2.27.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12432
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Volker Lendecke <vl at samba.org>
> ---
> source3/wscript_build | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/source3/wscript_build b/source3/wscript_build
> index d6d2be2..d04c1cc 100755
> --- a/source3/wscript_build
> +++ b/source3/wscript_build
> @@ -1041,6 +1041,7 @@ bld.SAMBA3_BINARY('vfstest',
> torture/vfstest.c
> torture/vfstest_chain.c''',
> deps='''
> + param
> vfs
> popt_samba3
> SMBREADLINE''',
> --
> 2.10.2
>
>
> From e8377965e1f2796aa6bd810aa86b0c036af732dd Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 16 Nov 2016 10:19:16 +0100
> Subject: [PATCH 2/3] s3:waf: Add missing param depencency to smbd
>
> Detected by gold linker version 2.27.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12432
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Volker Lendecke <vl at samba.org>
> ---
> source3/wscript_build | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/source3/wscript_build b/source3/wscript_build
> index d04c1cc..9d29cee 100755
> --- a/source3/wscript_build
> +++ b/source3/wscript_build
> @@ -867,6 +867,7 @@ bld.SAMBA3_SUBSYSTEM('SPOOLSSD',
> bld.SAMBA3_BINARY('smbd/smbd',
> source='smbd/server.c smbd/smbd_cleanupd.c',
> deps='''
> + param
> smbd_base
> EPMD
> LSASD
> --
> 2.10.2
>
>
> From 419a02d054e54dd53f80f3236fa75a2d49dad447 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Thu, 17 Nov 2016 11:27:41 +0100
> Subject: [PATCH 3/3] s3:waf: Move some dependencies up to smbd
>
> pysmbd doesn't use talloc tevent and popt directly.
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/wscript_build | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/source3/wscript_build b/source3/wscript_build
> index 9d29cee..ba400b7 100755
> --- a/source3/wscript_build
> +++ b/source3/wscript_build
> @@ -615,8 +615,6 @@ bld.SAMBA3_LIBRARY('smbd_base',
> smbd/notify_msg.c
> smbd/build_options.c''' + NOTIFY_SOURCES,
> deps='''
> - talloc
> - tevent
> pdb
> libsmb
> msrpc3
> @@ -624,7 +622,6 @@ bld.SAMBA3_LIBRARY('smbd_base',
> vfs_default
> vfs_posixacl
> inotify
> - popt_samba3
> samba3core
> smbd_conn
> param_service
> @@ -867,6 +864,9 @@ bld.SAMBA3_SUBSYSTEM('SPOOLSSD',
> bld.SAMBA3_BINARY('smbd/smbd',
> source='smbd/server.c smbd/smbd_cleanupd.c',
> deps='''
> + talloc
> + tevent
> + popt_samba3
> param
> smbd_base
> EPMD
> --
> 2.10.2
>
More information about the samba-technical
mailing list