[PATCH] libcli: ignore bad DataLength in negotiate response

[Philipp Gesang]

Hi Ralph,

-<| Quoting Philipp Gesang <philipp.gesang at intra2net.com>, on Thursday, 2019-02-28 10:05:21 AM |>-
> -<| Quoting Ralph Böhme via samba-technical <slow at samba.org>, on Thursday, 2019-02-28 09:33:46 AM |>-
> > On Thu, Feb 28, 2019 at 08:49:44AM +0100, Philipp Gesang via samba-technical wrote:
> > > Anyways, please consider the attached patch that makes Samba
> > > behave less strictly (but still conforming) in this situation by
> > > accepting a SMB2_ENCRYPTION_CAPABILITIES context whose DataLength
> > > field is larger than necessary. Preceding checks on the value
> > > ensure it does not point outside the response. Only the first
> > > item of data is used anyways.
> > > 
> > > The rationale for relaxing the check is that we should expect
> > > the affected Netapp versions to be around for some time despite a
> > > fix being available because apparently, admins think they’re a
> > > pain to update. Also, other SMB clients like Windows don’t seem
> > > to have any trouble connecting to the same server which would
> > > make this patch “correct” wrt. to bug-for-bug compatibility.
> > 
> > as we can't be sure that MS is going to stick with this behaviour, we should
> > as MS dochelp for clarification. Can you take care of that? Ie write a mail
> > to dochelp at microsoft.com (ideally ccing cifs-protocol at lists.samba.org) and
> > ask for clarification on the MS-SMB2 parapgraph in question. I could also
> > take care of this if you prefer.
> 
> I’ll do that, give me a day or so.

I updated the patch now that we have an official statement.

Regards,
Philipp

-------------- next part --------------
From 3ad4bb4b4cde3ec7013e61ac31d3938f5407530b Mon Sep 17 00:00:00 2001
From: Philipp Gesang <philipp.gesang at intra2net.com>
Date: Thu, 14 Feb 2019 10:17:28 +0100
Subject: [PATCH v2] libcli: permit larger values of DataLength in
 SMB2_ENCRYPTION_CAPABILITIES of negotiate response

Certain Netapp versions are sending SMB2_ENCRYPTION_CAPABILITIES
structures containing DataLength field that includes the padding
[0]. Microsoft has since clarified that only values smaller than
the size are considered invalid [1].

While parsing the NegotiateContext it is ensured that DataLength
does not exceed the message bounds. Also, the value is not
actually used anywhere outside the validation. Thus values
greater than the actual data size are safe to use. This patch
makes Samba fail only on values that are too small for the (fixed
size) payload.

[0] https://lists.samba.org/archive/samba/2019-February/221139.html
[1] https://lists.samba.org/archive/cifs-protocol/2019-March/003210.html

Signed-off-by: Philipp Gesang <philipp.gesang at intra2net.com>
---
 libcli/smb/smbXcli_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 9105b7c84a4..19eeec13c27 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -5062,7 +5062,7 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq)
 			return;
 		}
 
-		if (cipher->data.length != (2 + 2 * cipher_count)) {
+		if (cipher->data.length < (2 + 2 * cipher_count)) {
 			tevent_req_nterror(req,
 					NT_STATUS_INVALID_NETWORK_RESPONSE);
 			return;
-- 
2.20.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190305/c6e1c9ee/signature.sig>