[SCM] Samba Shared Repository - branch master updated

Andreas Schneider asn at samba.org
Sat Oct 12 15:52:03 UTC 2019


The branch, master has been updated
       via  23ea12e98ed spnego: fix server handling of no optimistic exchange
       via  8a963599772 python/tests/gensec: add spnego downgrade python tests
       via  eee1e8b6ac6 python/tests/gensec: make it possible to add knownfail tests for gensec.update()
       via  02f538816b4 selftest: add tests for no optimistic spnego exchange
       via  d7e57ef7dd7 spnego: add client option to omit sending an optimistic token
       via  90f557f3a1e selftest: s3: add a test for spnego downgrade from krb5 to ntlm
       via  7e36de99d7d s3:libsmb: Do not check the SPNEGO neg token for KRB5
       via  37daeb220e4 spnego: ignore server mech_types list
      from  efb43ecb8e3 wscript: split function check to one per line and sort alphabetically

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 23ea12e98ed34d41aee78d8afbe574dfc7e0ff74
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Wed Sep 4 17:04:12 2019 +0300

    spnego: fix server handling of no optimistic exchange
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Signed-off-by: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Andreas Schneider <asn at cryptomilk.org>
    Autobuild-Date(master): Sat Oct 12 15:51:42 UTC 2019 on sn-devel-184

commit 8a96359977249e8b19f50e5f2fe3f6ad7b7da52f
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Fri Oct 11 00:20:16 2019 +0300

    python/tests/gensec: add spnego downgrade python tests
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Pair-Programmed-With: Andreas Schneider <asn at samba.org>
    
    Signed-off-by: Isaac Boukris <iboukris at gmail.com>
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit eee1e8b6ac622e22a34ebfb684e70626cdd20fc1
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Oct 11 13:23:17 2019 +0200

    python/tests/gensec: make it possible to add knownfail tests for gensec.update()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>

commit 02f538816b409f1a122fae8fd08d761e3617e798
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Wed Sep 4 16:39:43 2019 +0300

    selftest: add tests for no optimistic spnego exchange
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Signed-off-by: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit d7e57ef7dd7d583cffb7abbe42d71f4f33f2a9af
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Wed Sep 4 16:31:21 2019 +0300

    spnego: add client option to omit sending an optimistic token
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Signed-off-by: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 90f557f3a1ed0a49e88ab0db29999f1289486cfe
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Mon Oct 7 23:51:19 2019 +0300

    selftest: s3: add a test for spnego downgrade from krb5 to ntlm
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Signed-off-by: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 7e36de99d7d028a01d4c52c2974d990ef91017bb
Author: Andreas Schneider <asn at samba.org>
Date:   Thu Oct 10 16:18:21 2019 +0200

    s3:libsmb: Do not check the SPNEGO neg token for KRB5
    
    The list is not protected and this could be a downgrade attack.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Pair-Programmed-With: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Signed-off-by: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 37daeb220e461b842ad9708497699f15c5fa5df3
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Thu Oct 3 13:09:29 2019 +0300

    spnego: ignore server mech_types list
    
    We should not use the mech list sent by the server in the last
    'negotiate' packet in CIFS protocol, as it is not protected and
    may be subject to downgrade attacks.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14106
    
    Signed-off-by: Isaac Boukris <iboukris at redhat.com>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

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

Summary of changes:
 auth/gensec/spnego.c                      | 55 ++++++++++++++++++++++++++++---
 python/samba/tests/gensec.py              | 34 +++++++++++++++++--
 selftest/target/Samba3.pm                 |  9 +++++
 source3/libsmb/cliconnect.c               | 50 ----------------------------
 source3/script/tests/test_smbd_no_krb5.sh | 46 ++++++++++++++++++++++++++
 source3/selftest/tests.py                 |  4 +++
 source4/selftest/tests.py                 |  4 +++
 7 files changed, 144 insertions(+), 58 deletions(-)
 create mode 100755 source3/script/tests/test_smbd_no_krb5.sh


Changeset truncated at 500 lines:

diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 565d5a975bf..db8a91b6f34 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -136,6 +136,7 @@ struct spnego_state {
 	bool done_mic_check;
 
 	bool simulate_w2k;
+	bool no_optimistic;
 
 	/*
 	 * The following is used to implement
@@ -187,6 +188,10 @@ static NTSTATUS gensec_spnego_client_start(struct gensec_security *gensec_securi
 
 	spnego_state->simulate_w2k = gensec_setting_bool(gensec_security->settings,
 						"spnego", "simulate_w2k", false);
+	spnego_state->no_optimistic = gensec_setting_bool(gensec_security->settings,
+							  "spnego",
+							  "client_no_optimistic",
+							  false);
 
 	gensec_security->private_data = spnego_state;
 	return NT_STATUS_OK;
@@ -511,7 +516,11 @@ static NTSTATUS gensec_spnego_client_negTokenInit_start(
 	}
 
 	n->mech_idx = 0;
-	n->mech_types = spnego_in->negTokenInit.mechTypes;
+
+	/* Do not use server mech list as it isn't protected. Instead, get all
+	 * supported mechs (excluding SPNEGO). */
+	n->mech_types = gensec_security_oids(gensec_security, n,
+					     GENSEC_OID_SPNEGO);
 	if (n->mech_types == NULL) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
@@ -658,13 +667,30 @@ static NTSTATUS gensec_spnego_client_negTokenInit_finish(
 					DATA_BLOB *out)
 {
 	struct spnego_data spnego_out;
-	const char *my_mechs[] = {NULL, NULL};
+	const char * const *mech_types = NULL;
 	bool ok;
 
-	my_mechs[0] = spnego_state->neg_oid;
+	if (n->mech_types == NULL) {
+		DBG_WARNING("No mech_types list\n");
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
+	for (mech_types = n->mech_types; *mech_types != NULL; mech_types++) {
+		int cmp = strcmp(*mech_types, spnego_state->neg_oid);
+
+		if (cmp == 0) {
+			break;
+		}
+	}
+
+	if (*mech_types == NULL) {
+		DBG_ERR("Can't find selected sub mechanism in mech_types\n");
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
 	/* compose reply */
 	spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
-	spnego_out.negTokenInit.mechTypes = my_mechs;
+	spnego_out.negTokenInit.mechTypes = mech_types;
 	spnego_out.negTokenInit.reqFlags = data_blob_null;
 	spnego_out.negTokenInit.reqFlagsPadding = 0;
 	spnego_out.negTokenInit.mechListMIC = data_blob_null;
@@ -676,7 +702,7 @@ static NTSTATUS gensec_spnego_client_negTokenInit_finish(
 	}
 
 	ok = spnego_write_mech_types(spnego_state,
-				     my_mechs,
+				     mech_types,
 				     &spnego_state->mech_types);
 	if (!ok) {
 		DBG_ERR("failed to write mechTypes\n");
@@ -1293,6 +1319,10 @@ static NTSTATUS gensec_spnego_server_negTokenInit_step(
 			spnego_state->mic_requested = true;
 		}
 
+		if (sub_in.length == 0) {
+			spnego_state->no_optimistic = true;
+		}
+
 		/*
 		 * Note that 'cur_sec' is temporary memory, but
 		 * cur_sec->oid points to a const string in the
@@ -1921,6 +1951,21 @@ static void gensec_spnego_update_pre(struct tevent_req *req)
 		 * blob and NT_STATUS_OK.
 		 */
 		state->sub.status = NT_STATUS_OK;
+	} else if (spnego_state->state_position == SPNEGO_CLIENT_START &&
+		   spnego_state->no_optimistic) {
+		/*
+		 * Skip optimistic token per conf.
+		 */
+		state->sub.status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+	} else if (spnego_state->state_position == SPNEGO_SERVER_START &&
+		   state->sub.in.length == 0 && spnego_state->no_optimistic) {
+		/*
+		 * If we didn't like the mechanism for which the client sent us
+		 * an optimistic token, or if he didn't send any, don't call
+		 * the sub mechanism just yet.
+		 */
+		state->sub.status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+		spnego_state->no_optimistic = false;
 	} else {
 		/*
 		 * MORE_PROCESSING_REQUIRED =>
diff --git a/python/samba/tests/gensec.py b/python/samba/tests/gensec.py
index b5ce51de756..47bb6c82a01 100644
--- a/python/samba/tests/gensec.py
+++ b/python/samba/tests/gensec.py
@@ -47,11 +47,17 @@ class GensecTests(samba.tests.TestCase):
     def test_info_uninitialized(self):
         self.assertRaises(RuntimeError, self.gensec.session_info)
 
-    def _test_update(self, mech, client_mech=None):
+    def _test_update(self, mech, client_mech=None, client_only_opt=None):
         """Test GENSEC by doing an exchange with ourselves using GSSAPI against a KDC"""
 
         """Start up a client and server GENSEC instance to test things with"""
 
+        if client_only_opt:
+            orig_client_opt = self.lp_ctx.get(client_only_opt)
+            if not orig_client_opt:
+                orig_client_opt = ''
+            self.lp_ctx.set(client_only_opt, "yes")
+
         self.gensec_client = gensec.Security.start_client(self.settings)
         self.gensec_client.set_credentials(self.get_credentials())
         self.gensec_client.want_feature(gensec.FEATURE_SEAL)
@@ -60,6 +66,9 @@ class GensecTests(samba.tests.TestCase):
         else:
             self.gensec_client.start_mech_by_sasl_name(mech)
 
+        if client_only_opt:
+            self.lp_ctx.set(client_only_opt, "no")
+
         self.gensec_server = gensec.Security.start_server(settings=self.settings,
                                                           auth_context=auth.AuthContext(lp_ctx=self.lp_ctx))
         creds = Credentials()
@@ -78,15 +87,28 @@ class GensecTests(samba.tests.TestCase):
         """Run the actual call loop"""
         while True:
             if not client_finished:
+                if client_only_opt:
+                    self.lp_ctx.set(client_only_opt, "yes")
                 print("running client gensec_update")
-                (client_finished, client_to_server) = self.gensec_client.update(server_to_client)
+                try:
+                    (client_finished, client_to_server) = self.gensec_client.update(server_to_client)
+                except samba.NTSTATUSError as nt:
+                    raise AssertionError(nt)
+                if client_only_opt:
+                    self.lp_ctx.set(client_only_opt, "no")
             if not server_finished:
                 print("running server gensec_update")
-                (server_finished, server_to_client) = self.gensec_server.update(client_to_server)
+                try:
+                    (server_finished, server_to_client) = self.gensec_server.update(client_to_server)
+                except samba.NTSTATUSError as nt:
+                    raise AssertionError(nt)
 
             if client_finished and server_finished:
                 break
 
+        if client_only_opt:
+            self.lp_ctx.set(client_only_opt, orig_client_opt)
+
         self.assertTrue(server_finished)
         self.assertTrue(client_finished)
 
@@ -115,6 +137,12 @@ class GensecTests(samba.tests.TestCase):
     def test_update_spnego(self):
         self._test_update("GSS-SPNEGO")
 
+    def test_update_spnego_downgrade(self):
+        self._test_update("GSS-SPNEGO", "spnego", "gensec:gssapi_krb5")
+
+    def test_update_no_optimistic_spnego(self):
+        self._test_update("GSS-SPNEGO", "spnego", "spnego:client_no_optimistic")
+
     def test_update_w2k_spnego_client(self):
         self.lp_ctx.set("spnego:simulate_w2k", "yes")
 
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 92d058f02ba..fab8bdda86e 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1621,6 +1621,7 @@ sub provision($$$$$$$$$)
 	my $dfqconffile="$libdir/dfq.conf";
 	my $errorinjectconf="$libdir/error_inject.conf";
 	my $delayinjectconf="$libdir/delay_inject.conf";
+	my $globalinjectconf="$libdir/global_inject.conf";
 
 	my $nss_wrapper_pl = "$ENV{PERL} $self->{srcdir}/third_party/nss_wrapper/nss_wrapper.pl";
 	my $nss_wrapper_passwd = "$privatedir/passwd";
@@ -1809,6 +1810,8 @@ sub provision($$$$$$$$$)
 	#it just means we ALLOW one to be configured.
 	allow insecure wide links = yes
 
+	include = $globalinjectconf
+
 	# Begin extra options
 	$extra_options
 	# End extra options
@@ -2344,6 +2347,12 @@ sub provision($$$$$$$$$)
 	}
 	close(DFQCONF);
 
+	unless (open(DELAYCONF, ">$globalinjectconf")) {
+		warn("Unable to open $globalinjectconf");
+		return undef;
+	}
+	close(DELAYCONF);
+
 	##
 	## create a test account
 	##
diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index b6accbab4df..82d8384e91c 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -232,8 +232,6 @@ NTSTATUS cli_session_creds_prepare_krb5(struct cli_state *cli,
 	char *canon_principal = NULL;
 	char *canon_realm = NULL;
 	const char *target_hostname = NULL;
-	const DATA_BLOB *server_blob = NULL;
-	bool got_kerberos_mechanism = false;
 	enum credentials_use_kerberos krb5_state;
 	bool try_kerberos = false;
 	bool need_kinit = false;
@@ -242,48 +240,6 @@ NTSTATUS cli_session_creds_prepare_krb5(struct cli_state *cli,
 	bool ok;
 
 	target_hostname = smbXcli_conn_remote_name(cli->conn);
-	server_blob = smbXcli_conn_server_gss_blob(cli->conn);
-
-	/* the server might not even do spnego */
-	if (server_blob != NULL && server_blob->length != 0) {
-		char *OIDs[ASN1_MAX_OIDS] = { NULL, };
-		size_t i;
-
-		/*
-		 * The server sent us the first part of the SPNEGO exchange in the
-		 * negprot reply. It is WRONG to depend on the principal sent in the
-		 * negprot reply, but right now we do it. If we don't receive one,
-		 * we try to best guess, then fall back to NTLM.
-		 */
-		ok = spnego_parse_negTokenInit(frame,
-					       *server_blob,
-					       OIDs,
-					       NULL,
-					       NULL);
-		if (!ok) {
-			TALLOC_FREE(frame);
-			return NT_STATUS_INVALID_PARAMETER;
-		}
-		if (OIDs[0] == NULL) {
-			TALLOC_FREE(frame);
-			return NT_STATUS_INVALID_PARAMETER;
-		}
-
-		/* make sure the server understands kerberos */
-		for (i = 0; OIDs[i] != NULL; i++) {
-			if (i == 0) {
-				DEBUG(3,("got OID=%s\n", OIDs[i]));
-			} else {
-				DEBUGADD(3,("got OID=%s\n", OIDs[i]));
-			}
-
-			if (strcmp(OIDs[i], OID_KERBEROS5_OLD) == 0 ||
-			    strcmp(OIDs[i], OID_KERBEROS5) == 0) {
-				got_kerberos_mechanism = true;
-				break;
-			}
-		}
-	}
 
 	auth_requested = cli_credentials_authentication_requested(creds);
 	if (auth_requested) {
@@ -333,12 +289,6 @@ NTSTATUS cli_session_creds_prepare_krb5(struct cli_state *cli,
 		need_kinit = false;
 	} else if (krb5_state == CRED_MUST_USE_KERBEROS) {
 		need_kinit = try_kerberos;
-	} else if (!got_kerberos_mechanism) {
-		/*
-		 * Most likely the server doesn't support
-		 * Kerberos, don't waste time doing a kinit
-		 */
-		need_kinit = false;
 	} else {
 		need_kinit = try_kerberos;
 	}
diff --git a/source3/script/tests/test_smbd_no_krb5.sh b/source3/script/tests/test_smbd_no_krb5.sh
new file mode 100755
index 00000000000..e9dbb4ae80e
--- /dev/null
+++ b/source3/script/tests/test_smbd_no_krb5.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+if [ $# -lt 1 ]; then
+cat <<EOF
+Usage: test_smbd_no_krb5.sh SERVER USERNAME PASSWORD PREFIX
+EOF
+exit 1;
+fi
+
+smbclient=$1
+SERVER=$2
+USERNAME=$3
+PASSWORD=$4
+PREFIX=$5
+shift 5
+
+samba_bindir="$BINDIR"
+samba_kinit=kinit
+if test -x ${samba_bindir}/samba4kinit; then
+	samba_kinit=${samba_bindir}/samba4kinit
+fi
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+. $incdir/common_test_fns.inc
+
+failed=0
+
+opt="--option=gensec:gse_krb5=yes -U${USERNAME}%${PASSWORD}"
+
+# check kerberos access
+test_smbclient "test_krb5" "ls" "//$SERVER/tmp" $opt -k || failed=`expr $failed + 1`
+
+# disbale krb5 globally so smbd won't accept it
+global_inject_conf=$(dirname $SMB_CONF_PATH)/global_inject.conf
+echo 'gensec:gse_krb5=no' > $global_inject_conf
+
+# verify that kerberos fails
+test_smbclient_expect_failure "smbd_no_krb5" "ls" "//$SERVER/tmp" -k $opt || failed=`expr $failed + 1`
+
+# verify downgrade to ntlmssp
+test_smbclient "test_spnego_downgrade" "ls" "//$SERVER/tmp" $opt || failed=`expr $failed + 1`
+
+echo '' > $global_inject_conf
+
+testok $0 $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index f3676a879a3..7b600ec64a2 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -775,6 +775,10 @@ plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local",
 plantestsuite("samba3.blackbox.smbd_error", "simpleserver:local",
               [os.path.join(samba3srcdir, "script/tests/test_smbd_error.sh")])
 
+plantestsuite("samba3.blackbox.smbd_no_krb5", "ad_member:local",
+              [os.path.join(samba3srcdir, "script/tests/test_smbd_no_krb5.sh"),
+               smbclient3, '$SERVER', "$DC_USERNAME", "$DC_PASSWORD", "$PREFIX"])
+
 plantestsuite("samba3.blackbox.durable_v2_delay", "simpleserver:local",
               [os.path.join(samba3srcdir, "script/tests/test_durable_handle_reconnect.sh")])
 
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index b7ef956ec79..2c5a754e89e 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -550,6 +550,10 @@ plansmbtorture4testsuite('base.xcopy', "ad_dc_ntvfs", ['//$NETBIOSNAME/xcopy_sha
 plansmbtorture4testsuite('base.xcopy', "ad_dc_ntvfs", ['//$NETBIOSNAME/xcopy_share', '-k', 'no', '--signing=required', '-U%'], modname="samba4.smb.signing --signing=required anon")
 plansmbtorture4testsuite('base.xcopy', "s4member", ['//$NETBIOSNAME/xcopy_share', '-k', 'no', '--signing=no', '-U%'], modname="samba4.smb.signing --signing=no anon")
 
+# Test SPNEGO without issuing an optimistic token
+opt='--option=spnego:client_no_optimistic=yes'
+plansmbtorture4testsuite('base.xcopy', "ad_dc", ['//$NETBIOSNAME/xcopy_share', '-U$USERNAME%$PASSWORD', opt, '-k', 'no'], modname="samba4.smb.spnego.ntlmssp.no_optimistic")
+plansmbtorture4testsuite('base.xcopy', "ad_dc", ['//$NETBIOSNAME/xcopy_share', '-U$USERNAME%$PASSWORD', opt, '-k', 'yes'], modname="samba4.smb.spnego.krb5.no_optimistic")
 
 wb_opts_default = ["--option=\"torture:strict mode=no\"", "--option=\"torture:timelimit=1\"", "--option=\"torture:winbindd_separator=/\"", "--option=\"torture:winbindd_netbios_name=$SERVER\"", "--option=\"torture:winbindd_netbios_domain=$DOMAIN\""]
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list