[PATCH] Fix smbclient regression not printing session setup anymore
Jeremy Allison
jra at samba.org
Tue Jun 6 16:08:09 UTC 2017
On Tue, Jun 06, 2017 at 05:56:21PM +0200, Andreas Schneider via samba-technical wrote:
> On Tuesday, 6 June 2017 17:35:52 CEST Andreas Schneider via samba-technical
> wrote:
> > Hi,
> >
> > with Samba 4.6 we have a regression. We do not get the information of the
> > session setup filled out so smbclient can't print it.
> >
> > Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=[Samba 4.7.0pre1-DEVELOPERBUILD]
> > smb: \>
> >
> > The attached patchset addresses the issue. I've open a bug for this:
> >
> > https://bugzilla.samba.org/show_bug.cgi?id=12824
>
> I've added a test to confirm that the message is actually printed so we do not
> regress again ...
The change from 'bool align_odd = true' to 'bool align_odd = false'
concerns me.
smb_bytes_pull_str() is used from all SMB1 session code inside
libcli/smb/smb1cli_session.c
especially the protocols lm21 and nt1. In addition it's used
inside
cli_sesssetup_blob_send() -> cli_sesssetup_blob_next() ->
smb1cli_session_setup_ext_send()-> smb1cli_session_setup_ext_done() ->
smb_bytes_pull_str()
which I believe is part of the SPNEGO code.
Are you sure you want to change that for all these code
paths ?
> From 09ec6a2c21a8a7c97d34526ee9330bfd42aba198 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 6 Jun 2017 17:27:44 +0200
> Subject: [PATCH 1/3] libcli:smb: Fix pulling strings from the wire
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> libcli/smb/util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libcli/smb/util.c b/libcli/smb/util.c
> index ef8c9fafa35..80d34281adf 100644
> --- a/libcli/smb/util.c
> +++ b/libcli/smb/util.c
> @@ -372,6 +372,6 @@ NTSTATUS smb_bytes_pull_str(TALLOC_CTX *mem_ctx, char **_str, bool ucs2,
> const uint8_t *buf, size_t buf_len,
> size_t *_buf_consumed)
> {
> - return internal_bytes_pull_str(mem_ctx, _str, ucs2, true,
> + return internal_bytes_pull_str(mem_ctx, _str, ucs2, false,
> buf, buf_len, _buf_consumed);
> }
> --
> 2.13.0
>
>
> From c35e71bc71a1d81d3d7b54d29ebe9dbf860ffb5b Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 6 Jun 2017 17:29:16 +0200
> Subject: [PATCH 2/3] s3:libsmb: Fix printing the session setup information
>
> This fixes a regression and prints the session setup on connect again:
>
> Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=[Samba 4.7.0pre1-DEVELOPERBUILD]
> smb: \>
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/libsmb/cliconnect.c | 3 ++-
> source3/libsmb/clidfs.c | 32 +++++++++++++++++++++++++-------
> source3/libsmb/clientgen.c | 13 -------------
> source3/torture/masktest.c | 4 +++-
> 4 files changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
> index a28a5824dc2..08627226629 100644
> --- a/source3/libsmb/cliconnect.c
> +++ b/source3/libsmb/cliconnect.c
> @@ -1122,7 +1122,8 @@ static void cli_session_setup_gensec_ready(struct tevent_req *req)
> server_domain = gensec_ntlmssp_server_domain(
> state->auth_generic->gensec_security);
>
> - if (state->cli->server_domain[0] == '\0' && server_domain != NULL) {
> + if ((state->cli->server_domain == NULL ||
> + state->cli->server_domain[0] == '\0') && server_domain != NULL) {
> TALLOC_FREE(state->cli->server_domain);
> state->cli->server_domain = talloc_strdup(state->cli,
> server_domain);
> diff --git a/source3/libsmb/clidfs.c b/source3/libsmb/clidfs.c
> index c477d7c6a46..09e21ac7f8d 100644
> --- a/source3/libsmb/clidfs.c
> +++ b/source3/libsmb/clidfs.c
> @@ -259,13 +259,31 @@ static NTSTATUS do_connect(TALLOC_CTX *ctx,
> return status;
> }
>
> - if ( show_sessetup ) {
> - if (*c->server_domain) {
> - DEBUG(0,("Domain=[%s] OS=[%s] Server=[%s]\n",
> - c->server_domain,c->server_os,c->server_type));
> - } else if (*c->server_os || *c->server_type) {
> - DEBUG(0,("OS=[%s] Server=[%s]\n",
> - c->server_os,c->server_type));
> + if (show_sessetup) {
> + const char *server_os = "unknown";
> + const char *server_type = "unknown";
> + bool do_print = false;
> +
> + if (c->server_os != NULL && c->server_os[0] != '\0') {
> + server_os = c->server_os;
> + do_print = true;
> + }
> + if (c->server_type != NULL && c->server_type[0] != '\0') {
> + server_type = c->server_type;
> + do_print = true;
> + }
> +
> + if (c->server_domain != NULL && c->server_domain[0] != '\0') {
> + DEBUG(0, ("Domain=[%s] OS=[%s] Server=[%s]\n",
> + c->server_domain,
> + server_os,
> + server_type));
> + } else {
> + if (do_print) {
> + DEBUG(0, ("OS=[%s] Server=[%s]\n",
> + server_os,
> + server_type));
> + }
> }
> }
> DEBUG(4,(" session setup ok\n"));
> diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c
> index bc5c1b1ce3c..2c0b8c5f081 100644
> --- a/source3/libsmb/clientgen.c
> +++ b/source3/libsmb/clientgen.c
> @@ -104,19 +104,6 @@ struct cli_state *cli_state_create(TALLOC_CTX *mem_ctx,
> return NULL;
> }
>
> - cli->server_domain = talloc_strdup(cli, "");
> - if (!cli->server_domain) {
> - goto error;
> - }
> - cli->server_os = talloc_strdup(cli, "");
> - if (!cli->server_os) {
> - goto error;
> - }
> - cli->server_type = talloc_strdup(cli, "");
> - if (!cli->server_type) {
> - goto error;
> - }
> -
> cli->dfs_mountpoint = talloc_strdup(cli, "");
> if (!cli->dfs_mountpoint) {
> goto error;
> diff --git a/source3/torture/masktest.c b/source3/torture/masktest.c
> index 95e0b04b040..57607b069a8 100644
> --- a/source3/torture/masktest.c
> +++ b/source3/torture/masktest.c
> @@ -212,7 +212,9 @@ static struct cli_state *connect_one(char *share)
> * mode to turn these on/off ? JRA.
> */
>
> - if (*c->server_domain || *c->server_os || *c->server_type)
> + if (c->server_domain != NULL ||
> + c->server_os != NULL ||
> + c->server_type != NULL)
> DEBUG(1,("Domain=[%s] OS=[%s] Server=[%s]\n",
> c->server_domain,c->server_os,c->server_type));
>
> --
> 2.13.0
>
>
> From e600314dd2812fc464a862258e7bdfa96fb3146b Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 6 Jun 2017 17:54:18 +0200
> Subject: [PATCH 3/3] s3:tests: Add a test which checks that the smbclient
> session setup works
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/script/tests/test_smbclient_s3.sh | 34 +++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
> index 0ae85e8e647..1359ed2b7bd 100755
> --- a/source3/script/tests/test_smbclient_s3.sh
> +++ b/source3/script/tests/test_smbclient_s3.sh
> @@ -1212,6 +1212,36 @@ EOF
> fi
> }
>
> +test_server_os_message()
> +{
> + tmpfile=$PREFIX/smbclient_interactive_prompt_commands
> + cat > $tmpfile <<EOF
> +ls
> +quit
> +EOF
> + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/tmp -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
> + eval echo "$cmd"
> + out=`eval $cmd`
> + ret=$?
> + rm -f $tmpfile
> +
> + if [ $ret -ne 0 ] ; then
> + echo "$out"
> + echo "failed to connect error $ret"
> + false
> + return
> + fi
> +
> + echo "$out" | grep 'Domain=\[SAMBA.*\] OS=\[Windows [0-9]\.[0-9]\] Server=\[Samba'
> + ret=$?
> + if [ $ret -ne 0 ] ; then
> + echo "$out"
> + echo "failed - should get: Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=..."
> + false
> + return
> + fi
> +}
> +
> LOGDIR_PREFIX=test_smbclient_s3
>
> # possibly remove old logdirs:
> @@ -1315,6 +1345,10 @@ testit "follow symlinks = no" \
> test_nosymlinks || \
> failed=`expr $failed + 1`
>
> +testit "server os message" \
> + test_server_os_message || \
> + failed=`expr $failed + 1`
> +
> testit "rm -rf $LOGDIR" \
> rm -rf $LOGDIR || \
> failed=`expr $failed + 1`
> --
> 2.13.0
>
More information about the samba-technical
mailing list