[Patches] Fix GENSEC_FEATURE_LDAP_STYLE handling as server (NTLMSSP NTLM2 packet check failed due to invalid signature!) (bug #13427)

Stefan Metzmacher metze at samba.org
Mon May 14 10:21:20 UTC 2018


Hi,

here's an updated patchset.

metze

Am 14.05.2018 um 12:02 schrieb Andrew Bartlett via samba-technical:
> On Wed, 2018-05-09 at 15:13 +0200, Ralph Böhme via samba-technical
> wrote:
>> On Wed, May 09, 2018 at 02:37:32PM +0200, Stefan Metzmacher via samba-technical wrote:
>>> here're patches to demonstrate and fix a regression of our server side
>>> GENSEC_FEATURE_LDAP_STYLE handling.
>>
>> would you mind explaining the logic behind GENSEC_FEATURE_LDAP_STYLE any why
>> NTLMSSP_NEGOTIATE_SIGN implies NTLMSSP_NEGOTIATE_SEAL over LDAP ? Thanks!
>>
>>> From 109f0487abdafc16a31a221f1ff57dccb0b2a775 Mon Sep 17 00:00:00 2001
>>> From: Stefan Metzmacher <metze at samba.org>
>>> Date: Mon, 7 May 2018 14:50:27 +0200
>>> Subject: [PATCH 3/3] auth/ntlmssp: fix handling of GENSEC_FEATURE_LDAP_STYLE
>>>  as a server
>>>
>>> This fixes "NTLMSSP NTLM2 packet check failed due to invalid signature!"
>>> error messages, which were generated if the client only sends
>>> NTLMSSP_NEGOTIATE_SIGN without NTLMSSP_NEGOTIATE_SEAL on an LDAP
>>> connection.
>>>
>>> This fixes a regession in the combination of commits
>>> 77adac8c3cd2f7419894d18db735782c9646a202 and
>>> 3a0b835408a6efa339e8b34333906bfe3aacd6e3.
>>>
>>> We need to evaluate GENSEC_FEATURE_LDAP_STYLE at the end
>>> of the authentication (as a server), while we need to (any already
>>> do so at the beginning as a client).
>>
>> Oh, and btw, this commit message is in need of some love. :)
> 
> G'Day,
> 
> I'm sorry, but I'm with Ralph on this one.  I tried to make sense of
> what is going on here, but I can't.  Can you explain this a with a bit
> more background?
> 
> Thanks,
> 
> Andrew Bartlett
> 

-------------- next part --------------
From ce5b34604a34097b02baf5e6fefa41649c1e4c94 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 9 May 2018 13:30:13 +0200
Subject: [PATCH 1/3] auth/ntlmssp: add ntlmssp_client:ldap_style_send_seal
 option

This will be used to similate a Windows client only
using NTLMSSP_NEGOTIATE_SIGN without NTLMSSP_NEGOTIATE_SEAL
on an LDAP connection, which is indicated internally by
GENSEC_FEATURE_LDAP_STYLE.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13427

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 auth/ntlmssp/ntlmssp_client.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/auth/ntlmssp/ntlmssp_client.c b/auth/ntlmssp/ntlmssp_client.c
index 7dcf235..ab406a2 100644
--- a/auth/ntlmssp/ntlmssp_client.c
+++ b/auth/ntlmssp/ntlmssp_client.c
@@ -869,13 +869,23 @@ NTSTATUS gensec_ntlmssp_client_start(struct gensec_security *gensec_security)
 			 * is requested.
 			 */
 			ntlmssp_state->force_wrap_seal = true;
-			/*
-			 * We want also work against old Samba servers
-			 * which didn't had GENSEC_FEATURE_LDAP_STYLE
-			 * we negotiate SEAL too. We may remove this
-			 * in a few years. As all servers should have
-			 * GENSEC_FEATURE_LDAP_STYLE by then.
-			 */
+		}
+	}
+	if (ntlmssp_state->force_wrap_seal) {
+		bool ret;
+
+		/*
+		 * We want also work against old Samba servers
+		 * which didn't had GENSEC_FEATURE_LDAP_STYLE
+		 * we negotiate SEAL too. We may remove this
+		 * in a few years. As all servers should have
+		 * GENSEC_FEATURE_LDAP_STYLE by then.
+		 */
+		ret = gensec_setting_bool(gensec_security->settings,
+					  "ntlmssp_client",
+					  "ldap_style_send_seal",
+					  true);
+		if (ret) {
 			ntlmssp_state->required_flags |= NTLMSSP_NEGOTIATE_SEAL;
 		}
 	}
-- 
1.9.1


From 5406e147f56c07e26305145f66dc8e0eb38145fe Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 9 May 2018 13:33:05 +0200
Subject: [PATCH 2/3] s4:selftest: run test_ldb_simple.sh with more auth
 options

This demonstrates the broken GENSEC_FEATURE_LDAP_STYLE
handling in our LDAP server.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13427

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/ntlmssp_ldap_style_send_seal | 1 +
 source4/selftest/tests.py                         | 7 +++++++
 2 files changed, 8 insertions(+)
 create mode 100644 selftest/knownfail.d/ntlmssp_ldap_style_send_seal

diff --git a/selftest/knownfail.d/ntlmssp_ldap_style_send_seal b/selftest/knownfail.d/ntlmssp_ldap_style_send_seal
new file mode 100644
index 0000000..0cd7cc2
--- /dev/null
+++ b/selftest/knownfail.d/ntlmssp_ldap_style_send_seal
@@ -0,0 +1 @@
+^samba4.ldb.simple.ldap.*ldap_style_send_seal=no
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 88af607..9740118 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -113,6 +113,13 @@ for env in ["ad_dc_ntvfs", "fl2008r2dc", "fl2003dc"]:
         '--option=clientldapsaslwrapping=plain',
         '--sign',
         '--encrypt',
+        '-k yes --option=clientldapsaslwrapping=plain',
+        '-k yes --sign',
+        '-k yes --encrypt',
+        '-k no --option=clientldapsaslwrapping=plain',
+        '-k no --sign --option=ntlmssp_client:ldap_style_send_seal=no',
+        '-k no --sign',
+        '-k no --encrypt',
     ]
 
     for auth_option in auth_options:
-- 
1.9.1


From 61762d8f658cd6fc6bbbf12de1083ff3cac17c60 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 7 May 2018 14:50:27 +0200
Subject: [PATCH 3/3] auth/ntlmssp: fix handling of GENSEC_FEATURE_LDAP_STYLE
 as a server

This fixes "NTLMSSP NTLM2 packet check failed due to invalid signature!"
error messages, which were generated if the client only sends
NTLMSSP_NEGOTIATE_SIGN without NTLMSSP_NEGOTIATE_SEAL on an LDAP
connection.

This fixes a regession in the combination of commits
77adac8c3cd2f7419894d18db735782c9646a202 and
3a0b835408a6efa339e8b34333906bfe3aacd6e3.

We need to evaluate GENSEC_FEATURE_LDAP_STYLE at the end
of the authentication (as a server, while we already
do so at the beginning as a client).

As a reminder I introduced GENSEC_FEATURE_LDAP_STYLE
(as an internal flag) in order to let us work as a
Windows using NTLMSSP for LDAP. Even if only signing is
negotiated during the authentication the following PDUs
will still be encrypted if NTLMSSP is used. This is exactly the
same as if the client would have negotiated NTLMSSP_NEGOTIATE_SEAL.
I guess it's a bug in Windows, but we have to reimplement that
bug. Note this only applies to NTLMSSP and only to LDAP!
Signing only works fine for LDAP with Kerberos
or DCERPC and NTLMSSP.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13427

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 auth/ntlmssp/gensec_ntlmssp_server.c              | 19 -------------------
 auth/ntlmssp/ntlmssp_server.c                     |  8 ++++++++
 selftest/knownfail.d/ntlmssp_ldap_style_send_seal |  1 -
 3 files changed, 8 insertions(+), 20 deletions(-)
 delete mode 100644 selftest/knownfail.d/ntlmssp_ldap_style_send_seal

diff --git a/auth/ntlmssp/gensec_ntlmssp_server.c b/auth/ntlmssp/gensec_ntlmssp_server.c
index c0e6cff..ab92f4d 100644
--- a/auth/ntlmssp/gensec_ntlmssp_server.c
+++ b/auth/ntlmssp/gensec_ntlmssp_server.c
@@ -179,25 +179,6 @@ NTSTATUS gensec_ntlmssp_server_start(struct gensec_security *gensec_security)
 	ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_SIGN;
 	ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_SEAL;
 
-	if (gensec_security->want_features & GENSEC_FEATURE_SESSION_KEY) {
-		ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_SIGN;
-	}
-	if (gensec_security->want_features & GENSEC_FEATURE_SIGN) {
-		ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_SIGN;
-
-		if (gensec_security->want_features & GENSEC_FEATURE_LDAP_STYLE) {
-			/*
-			 * We need to handle NTLMSSP_NEGOTIATE_SIGN as
-			 * NTLMSSP_NEGOTIATE_SEAL if GENSEC_FEATURE_LDAP_STYLE
-			 * is requested.
-			 */
-			ntlmssp_state->force_wrap_seal = true;
-		}
-	}
-	if (gensec_security->want_features & GENSEC_FEATURE_SEAL) {
-		ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_SIGN;
-		ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_SEAL;
-	}
 
 	if (role == ROLE_STANDALONE) {
 		ntlmssp_state->server.is_standalone = true;
diff --git a/auth/ntlmssp/ntlmssp_server.c b/auth/ntlmssp/ntlmssp_server.c
index 37ed2bc..140e89d 100644
--- a/auth/ntlmssp/ntlmssp_server.c
+++ b/auth/ntlmssp/ntlmssp_server.c
@@ -1080,6 +1080,14 @@ static NTSTATUS ntlmssp_server_postauth(struct gensec_security *gensec_security,
 	data_blob_free(&ntlmssp_state->challenge_blob);
 
 	if (gensec_ntlmssp_have_feature(gensec_security, GENSEC_FEATURE_SIGN)) {
+		if (gensec_security->want_features & GENSEC_FEATURE_LDAP_STYLE) {
+			/*
+			 * We need to handle NTLMSSP_NEGOTIATE_SIGN as
+			 * NTLMSSP_NEGOTIATE_SEAL if GENSEC_FEATURE_LDAP_STYLE
+			 * is requested.
+			 */
+			ntlmssp_state->force_wrap_seal = true;
+		}
 		nt_status = ntlmssp_sign_init(ntlmssp_state);
 	}
 
diff --git a/selftest/knownfail.d/ntlmssp_ldap_style_send_seal b/selftest/knownfail.d/ntlmssp_ldap_style_send_seal
deleted file mode 100644
index 0cd7cc2..0000000
--- a/selftest/knownfail.d/ntlmssp_ldap_style_send_seal
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.ldb.simple.ldap.*ldap_style_send_seal=no
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180514/284decd5/signature.sig>


More information about the samba-technical mailing list