[PATCH] smbclient and libsmbclient fixes and tests

Jeremy Allison jra at samba.org
Tue Sep 4 20:35:10 UTC 2018


On Tue, Sep 04, 2018 at 05:02:34PM +0200, Andreas Schneider via samba-technical wrote:
> Hi,
> 
> attached is patchset to address some issues in smbclient and libsmbclient. It 
> also will run libsmbclient tests with NT1 and SMB3.
> 
> Please review and push if OK.

Looks really great Andreas, only one minor comment
(inline) for future consideration but not enough to derail
the push.

The addition of smbc_setOptionProtocols() is a great
addition to the library !

RB+ and pushed !

Thanks,

	Jeremy.

> From 40cb078b1a6590aebb2ff8fca7f966e7c122819b Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 4 Sep 2018 11:11:49 +0200
> Subject: [PATCH 2/6] s3:smbclient: Do not call cli_RNetShareEnum if SMB1 is
>  disabled
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  source3/client/client.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/source3/client/client.c b/source3/client/client.c
> index adc3fcab476..738739f7909 100644
> --- a/source3/client/client.c
> +++ b/source3/client/client.c
> @@ -4900,7 +4900,6 @@ static bool browse_host_rpc(bool sort)
>  
>  static bool browse_host(bool sort)
>  {
> -	int ret;
>  	if (!grepable) {
>  	        d_printf("\n\tSharename       Type      Comment\n");
>  	        d_printf("\t---------       ----      -------\n");
> @@ -4910,13 +4909,20 @@ static bool browse_host(bool sort)
>  		return true;
>  	}
>  
> -	if((ret = cli_RNetShareEnum(cli, browse_fn, NULL)) == -1) {
> -		NTSTATUS status = cli_nt_error(cli);
> -		d_printf("Error returning browse list: %s\n",
> -			 nt_errstr(status));

For future patches would probably be better as an early
return, i.e.:

	if (lp_client_min_protocol() >= PROTOCOL_SMB2_02) {
		return;
	}

Then the rest of the SMB1-specific code.

> +	if (lp_client_min_protocol() <= PROTOCOL_NT1) {
> +		int ret;
> +
> +		ret = cli_RNetShareEnum(cli, browse_fn, NULL);
> +		if (ret == -1) {
> +			NTSTATUS status = cli_nt_error(cli);
> +			d_printf("Error returning browse list: %s\n",
> +				 nt_errstr(status));
> +		}
> +
> +		return (ret != -1);
>  	}
>  
> -	return (ret != -1);
> +	return false;
>  }
> +++ b/source3/include/libsmbclient.h
> @@ -831,7 +831,24 @@ smbc_getOptionUseNTHash(SMBCCTX *c);
>  void
>  smbc_setOptionUseNTHash(SMBCCTX *c, smbc_bool b);
>  
> -
> +/**
> + * @brief Set the 'client min protocol' and the 'client max protocol'.
> + *
> + * IMPORTANT: This overrrides the values 'client min protocol' and 'client max
> + * protocol' set in the smb.conf file!
> + *
> + * @param[in]  c  The smbc context to use.
> + *
> + * @param[in]  min_proto  The minimal protocol to use or NULL for leaving it
> + *                        untouched.
> + *
> + * @param[in]  max_proto  The maximum protocol to use or NULL for leaving it
> + *                        untouched.
> + *
> + * @returns true for success, false otherwise
> + */
> +smbc_bool
> +smbc_setOptionProtocols(SMBCCTX *c, const char *min_proto, const char *max_proto);
>  
>  /*************************************
>   * Getters and setters for FUNCTIONS *
> diff --git a/source3/libsmb/libsmb_setget.c b/source3/libsmb/libsmb_setget.c
> index 60b822a395c..b1c4ff3b557 100644
> --- a/source3/libsmb/libsmb_setget.c
> +++ b/source3/libsmb/libsmb_setget.c
> @@ -526,6 +526,24 @@ smbc_setOptionUseNTHash(SMBCCTX *c, smbc_bool b)
>          }
>  }
>  
> +smbc_bool
> +smbc_setOptionProtocols(SMBCCTX *c,
> +			const char *min_proto,
> +			const char *max_proto)
> +{
> +	bool ok = true;
> +
> +	if (min_proto != NULL) {
> +		ok = lp_set_cmdline("client min protocol", min_proto);
> +	}
> +
> +	if (max_proto != NULL) {
> +		ok &= lp_set_cmdline("client min protocol", max_proto);
> +	}
> +
> +	return ok;
> +}
> +
>  /** Get the function for obtaining authentication data */
>  smbc_get_auth_data_fn
>  smbc_getFunctionAuthData(SMBCCTX *c)
> diff --git a/source3/libsmb/wscript b/source3/libsmb/wscript
> index 5482aea7d9c..298afc3c0e3 100644
> --- a/source3/libsmb/wscript
> +++ b/source3/libsmb/wscript
> @@ -27,5 +27,5 @@ def build(bld):
>                         public_headers='../include/libsmbclient.h',
>                         abi_directory='ABI',
>                         abi_match='smbc_*',
> -                       vnum='0.4.0',
> +                       vnum='0.5.0',
>                         pc_files='smbclient.pc')
> -- 
> 2.18.0
> 
> 
> From a33baa48017e271df62c684b6216666935dccbeb Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Mon, 3 Sep 2018 16:36:54 +0200
> Subject: [PATCH 5/6] s4:torture: Set credentials for libsmbclient correctly
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  source4/torture/libsmbclient/libsmbclient.c | 40 +++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/source4/torture/libsmbclient/libsmbclient.c b/source4/torture/libsmbclient/libsmbclient.c
> index fb27ddd0326..853d002aa8d 100644
> --- a/source4/torture/libsmbclient/libsmbclient.c
> +++ b/source4/torture/libsmbclient/libsmbclient.c
> @@ -40,9 +40,39 @@ static void debug_callback(void *private_ptr, int level, const char *msg)
>  	return;
>  }
>  
> +static void auth_callback(const char *srv,
> +			  const char *shr,
> +			  char *wg, int wglen,
> +			  char *un, int unlen,
> +			  char *pw, int pwlen)
> +{
> +	const char *workgroup =
> +		cli_credentials_get_domain(popt_get_cmdline_credentials());
> +	const char *username =
> +		cli_credentials_get_username(popt_get_cmdline_credentials());
> +	const char *password =
> +		cli_credentials_get_password(popt_get_cmdline_credentials());
> +
> +	if (workgroup != NULL) {
> +		snprintf(wg, wglen, "%s", workgroup);
> +	}
> +
> +	if (username != NULL) {
> +		snprintf(un, unlen, "%s", username);
> +	}
> +
> +	if (password != NULL) {
> +		snprintf(pw, pwlen, "%s", password);
> +	}
> +};
> +
>  bool torture_libsmbclient_init_context(struct torture_context *tctx,
>  				       SMBCCTX **ctx_p)
>  {
> +	const char *workgroup =
> +		cli_credentials_get_domain(popt_get_cmdline_credentials());
> +	const char *username =
> +		cli_credentials_get_username(popt_get_cmdline_credentials());
>  	SMBCCTX *ctx = NULL;
>  	SMBCCTX *p = NULL;
>  	bool ok = true;
> @@ -64,8 +94,14 @@ bool torture_libsmbclient_init_context(struct torture_context *tctx,
>  	smbc_setDebug(ctx, DEBUGLEVEL);
>  	smbc_setOptionDebugToStderr(ctx, 1);
>  
> -	smbc_setUser(ctx,
> -		     cli_credentials_get_username(popt_get_cmdline_credentials()));
> +	if (workgroup != NULL) {
> +		smbc_setWorkgroup(ctx, workgroup);
> +	}
> +	if (username != NULL) {
> +		smbc_setUser(ctx, username);
> +	}
> +
> +	smbc_setFunctionAuthData(ctx, auth_callback);
>  
>  	*ctx_p = ctx;
>  
> -- 
> 2.18.0
> 
> 
> From a72af76f7ec2d0317ddba62bb5b00014670a1a36 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Mon, 3 Sep 2018 16:55:02 +0200
> Subject: [PATCH 6/6] selftest: Run libsmbclient tests with NT1 and SMB3
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  selftest/knownfail                          | 2 +-
>  source4/selftest/tests.py                   | 9 ++++++---
>  source4/torture/libsmbclient/libsmbclient.c | 6 ++++++
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 93c1a3511ac..eef8134c9f5 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -125,7 +125,7 @@
>  ^samba4.smb2.acls.*.ACCESSBASED
>  ^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.test_dirsync_deleted_items
>  #^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.*
> -^samba4.libsmbclient.opendir.opendir # This requires netbios browsing
> +^samba4.libsmbclient.opendir.(NT1|SMB3).opendir # This requires netbios browsing
>  ^samba4.rpc.drsuapi.*.drsuapi.DsGetDomainControllerInfo\(.*\)$
>  ^samba4.smb2.oplock.exclusive2\(.*\)$ # samba 4 oplocks are a mess
>  ^samba4.smb2.oplock.exclusive5\(.*\)$ # samba 4 oplocks are a mess
> diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
> index 1626b9312f4..c1747ddcd50 100755
> --- a/source4/selftest/tests.py
> +++ b/source4/selftest/tests.py
> @@ -332,14 +332,17 @@ for t in base + raw + smb2 + netapi:
>      plansmbtorture4testsuite(t, "ad_dc_ntvfs", ['//$SERVER/tmp', '-U$USERNAME%$PASSWORD'] + ntvfsargs)
>  
>  libsmbclient = smbtorture4_testsuites("libsmbclient.")
> +protocols = [ 'NT1', 'SMB3' ]
>  for t in libsmbclient:
>      url = "smb://$USERNAME:$PASSWORD@$SERVER/tmp"
>      if t == "libsmbclient.list_shares":
>          url = "smb://$USERNAME:$PASSWORD@$SERVER"
>  
> -    libsmbclient_testargs = ["--option=torture:smburl=" + url,
> -                             "--option=torture:replace_smbconf=%s/testdata/samba3/smb_new.conf" % srcdir()]
> -    plansmbtorture4testsuite(t, "ad_dc", ['//$SERVER/tmp', '-U$USERNAME%$PASSWORD'] + libsmbclient_testargs)
> +    for proto in protocols:
> +        libsmbclient_testargs = ["--option=torture:smburl=" + url,
> +                                 "--option=torture:replace_smbconf=%s/testdata/samba3/smb_new.conf" % srcdir(),
> +                                 "--option=torture:clientprotocol=%s" % proto]
> +        plansmbtorture4testsuite(t, "ad_dc", ['//$SERVER/tmp', '-U$USERNAME%$PASSWORD'] + libsmbclient_testargs, "samba4.%s.%s" % (t, proto))
>  
>  plansmbtorture4testsuite("raw.qfileinfo.ipc", "ad_dc_ntvfs", '//$SERVER/ipc\$ -U$USERNAME%$PASSWORD')
>  
> diff --git a/source4/torture/libsmbclient/libsmbclient.c b/source4/torture/libsmbclient/libsmbclient.c
> index 853d002aa8d..f9154e8a19c 100644
> --- a/source4/torture/libsmbclient/libsmbclient.c
> +++ b/source4/torture/libsmbclient/libsmbclient.c
> @@ -73,6 +73,8 @@ bool torture_libsmbclient_init_context(struct torture_context *tctx,
>  		cli_credentials_get_domain(popt_get_cmdline_credentials());
>  	const char *username =
>  		cli_credentials_get_username(popt_get_cmdline_credentials());
> +	const char *client_proto =
> +		torture_setting_string(tctx, "clientprotocol", NULL);
>  	SMBCCTX *ctx = NULL;
>  	SMBCCTX *p = NULL;
>  	bool ok = true;
> @@ -103,6 +105,10 @@ bool torture_libsmbclient_init_context(struct torture_context *tctx,
>  
>  	smbc_setFunctionAuthData(ctx, auth_callback);
>  
> +	if (client_proto != NULL) {
> +		smbc_setOptionProtocols(ctx, client_proto, client_proto);
> +	}
> +
>  	*ctx_p = ctx;
>  
>  out:
> -- 
> 2.18.0
> 




More information about the samba-technical mailing list