[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Wed Dec 4 21:28:03 UTC 2019
The branch, master has been updated
via 8db0c1bff6f s3: smbd: Only set xconn->smb1.negprot.done = true after supported_protocols[protocol].proto_reply_fn() succeeds.
via 46899ecf836 python: tests. Add test for fuzzing smbd crash bug.
via e84910d919e s3: smbd: Ensure we exit if supported_protocols[protocol].proto_reply_fn() fails.
via f4caa4159bd s3: smbd: Change (*proto_reply_fn()) to return an NTSTATUS.
via 836219c479b s3: smbd: Change reply_smb20xx() to return NTSTATUS.
via a2d81d77c11 s3: smbd: Ensure we exit on smbd_smb2_process_negprot() fail.
via 868bc05cf5d s3: smbd: Allow smbd_smb2_process_negprot() to return NTSTATUS as it can fail.
from eddd6c52678 pidl: don't export parser class methods
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 8db0c1bff6f42feabd2e4d9dfb13ae12cc29607b
Author: Jeremy Allison <jra at samba.org>
Date: Tue Nov 26 12:53:09 2019 -0800
s3: smbd: Only set xconn->smb1.negprot.done = true after supported_protocols[protocol].proto_reply_fn() succeeds.
Otherwise we can end up with negprot.done set, but
without smbXsrv_connection_init_tables() being called.
This can cause a client self-crash.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Wed Dec 4 21:27:24 UTC 2019 on sn-devel-184
commit 46899ecf836d350c0c29b615869851da7d0ad6fb
Author: Jeremy Allison <jra at samba.org>
Date: Mon Dec 2 17:23:21 2019 -0800
python: tests. Add test for fuzzing smbd crash bug.
Mark knownfail for now.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit e84910d919e02feab2a297fccbbf95f333e32119
Author: Jeremy Allison <jra at samba.org>
Date: Tue Nov 26 12:46:16 2019 -0800
s3: smbd: Ensure we exit if supported_protocols[protocol].proto_reply_fn() fails.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit f4caa4159bd3db5127e114718e606867348a4f47
Author: Jeremy Allison <jra at samba.org>
Date: Tue Nov 26 12:43:25 2019 -0800
s3: smbd: Change (*proto_reply_fn()) to return an NTSTATUS.
That way the caller can know if the negprot really
succeeded or not.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit 836219c479b047403d2b0839a6b92ad637dbaea0
Author: Jeremy Allison <jra at samba.org>
Date: Tue Nov 26 12:21:06 2019 -0800
s3: smbd: Change reply_smb20xx() to return NTSTATUS.
Not yet used.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit a2d81d77c111379cbb6bd732c717307974eace0a
Author: Jeremy Allison <jra at samba.org>
Date: Tue Nov 26 12:17:29 2019 -0800
s3: smbd: Ensure we exit on smbd_smb2_process_negprot() fail.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
commit 868bc05cf5d575e20edcce241e3af1d0fa6d9824
Author: Jeremy Allison <jra at samba.org>
Date: Tue Nov 26 12:14:29 2019 -0800
s3: smbd: Allow smbd_smb2_process_negprot() to return NTSTATUS as it can fail.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14205
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
-----------------------------------------------------------------------
Summary of changes:
python/samba/tests/smbd_fuzztest.py | 77 +++++++++++++++++++++++++++++++++++++
selftest/tests.py | 1 +
source3/smbd/globals.h | 6 +--
source3/smbd/negprot.c | 39 +++++++++++--------
source3/smbd/process.c | 9 ++++-
source3/smbd/smb2_negprot.c | 15 ++++----
source3/smbd/smb2_server.c | 15 ++++----
7 files changed, 127 insertions(+), 35 deletions(-)
create mode 100644 python/samba/tests/smbd_fuzztest.py
Changeset truncated at 500 lines:
diff --git a/python/samba/tests/smbd_fuzztest.py b/python/samba/tests/smbd_fuzztest.py
new file mode 100644
index 00000000000..900cddf3880
--- /dev/null
+++ b/python/samba/tests/smbd_fuzztest.py
@@ -0,0 +1,77 @@
+# Unix SMB/CIFS implementation. Tests for smbd fuzzing.
+# Copyright (C) Jeremy Allison 2019.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import sys
+import samba
+import os
+import binascii
+import socket
+
+class fuzzsmbd(samba.tests.TestCase):
+ def test_bug_14205(self):
+ #
+ # badblob consists of an incorrectly
+ # terminated SMB1 Negprot, with a valid SessionSetup after.
+ # BUG: #14205 causes the smbd server to crash.
+ #
+ state = True;
+ badblob = binascii.a2b_base64("AAAA1P9TTUJyAAAAABhDyAAAAAAAAAAAAAAAACcA/v8AAAAAALEAAlBDIE5F"
+ "VFdPUksgUFJPR1JBTSD//jAAAk1JQ1JPU09GVCBOR1RXT1JLUyAxLjANDAJN"
+ "SR3hkXOl0mb+QXW4Da/jp0f+AAAA1P9TTUJyAAAAABgDyAAABDQAAAAAAAAA"
+ "ACcA/v8AAAAAALEAAlBDIE5FVFdPUksgUFJPR1JBFBX//jAAAk1JQ1JPU09G"
+ "VCBOR1RXT1JLUyAxLjANDAJNSR3hkUal0mb+QXW4Da/jp0f+AAAA1P9TTUJz"
+ "LTE0OEF1uA2v46dH/gqAIIwiAoRiVHWgODu8OdksJQAAAAAnAP7/AAAAAACx"
+ "AAJQQyBORVRXT1JLIFBST0dSQU0g//4wAAJNSUNST1NPRlQgTkdUV09SS1Mg"
+ "MS4wDQwCTUkd4ZFGpdJm/kF1uA2v46dH/gAAANT/U01Ccy0xNDgyMTIyOTE3"
+ "Nzk2MzIAAAAAGAPIAAAAAAAAAAAAAAAAJwD+/wAAAAAAsQACUEMgTkVUV09S"
+ "SyBQUk9HUkFNIP/+MAACTUlDUk9TT0ZUIE5HVFdPUktTIDEuMA0GAAAAAAAA"
+ "AKXSZv5BdbgNr+OnR/4AAADU/1NNQnMtMTQ4MjEyMjkxNzc5NjMyNDQ4NDNA"
+ "ujcyNjgAsQACUEMgTkVUF09SSyAgAAAAAAAAAP/+MAACTUlDUk9TT0bAIE5H"
+ "BwAtMjMxODIxMjE4MTM5OTU0ODA2OP5BdbgNr+OnR/4KgCCMIgKEYlR1oDg7"
+ "vDnZLCWy")
+ s = None
+ try:
+ s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ s.connect(("fileserver", 445))
+ s.send(badblob)
+ # Read the 39-byte SMB1 reply to the SMB1 Negprot.
+ # This is an error message saying the Negprot was
+ # invalid.
+ rb = s.recv(1024)
+ try:
+ # Read again to wait for the server to exit.
+ rb = s.recv(1024)
+ except socket.error as e:
+ # We expect a socket error here as
+ # in both success and fail cases the
+ # server just resets the connection.
+ pass
+ finally:
+ pass
+ finally:
+ if s is not None:
+ s.close()
+ #
+ # If the server crashed there is the
+ # following message in the debug log.
+ #
+ for line in open(os.environ['SMBD_TEST_LOG']):
+ if "INTERNAL ERROR: Signal 11 in pid" in line:
+ print("Found crash in smbd log")
+ state = False;
+ break
+ self.assertTrue(state)
diff --git a/selftest/tests.py b/selftest/tests.py
index cca2ad02c9e..32b76b4bc5b 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -90,6 +90,7 @@ planpythontestsuite(
"none", "wafsamba.tests.test_suite",
extra_path=[os.path.join(samba4srcdir, "..", "buildtools"),
os.path.join(samba4srcdir, "..", "third_party", "waf")])
+planpythontestsuite("fileserver", "samba.tests.smbd_fuzztest")
def cmdline(script, *args):
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index c791eb0fa6f..e6641837668 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -232,9 +232,9 @@ bool smbd_smb2_is_compound(const struct smbd_smb2_request *req);
NTSTATUS smbd_add_connection(struct smbXsrv_client *client, int sock_fd,
struct smbXsrv_connection **_xconn);
-void reply_smb2002(struct smb_request *req, uint16_t choice);
-void reply_smb20ff(struct smb_request *req, uint16_t choice);
-void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
+NTSTATUS reply_smb2002(struct smb_request *req, uint16_t choice);
+NTSTATUS reply_smb20ff(struct smb_request *req, uint16_t choice);
+NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
uint64_t expected_seq_low,
const uint8_t *inpdu, size_t size);
diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c
index 2d5edf1282c..e77c8f52261 100644
--- a/source3/smbd/negprot.c
+++ b/source3/smbd/negprot.c
@@ -66,7 +66,7 @@ static void get_challenge(struct smbXsrv_connection *xconn, uint8_t buff[8])
Reply for the lanman 1.0 protocol.
****************************************************************************/
-static void reply_lanman1(struct smb_request *req, uint16_t choice)
+static NTSTATUS reply_lanman1(struct smb_request *req, uint16_t choice)
{
int secword=0;
time_t t = time(NULL);
@@ -100,7 +100,7 @@ static void reply_lanman1(struct smb_request *req, uint16_t choice)
status = smbXsrv_connection_init_tables(xconn, PROTOCOL_LANMAN1);
if (!NT_STATUS_IS_OK(status)) {
reply_nterror(req, status);
- return;
+ return status;
}
/* Reply, SMBlockread, SMBwritelock supported. */
@@ -115,14 +115,14 @@ static void reply_lanman1(struct smb_request *req, uint16_t choice)
srv_put_dos_date((char *)req->outbuf,smb_vwv8,t);
- return;
+ return NT_STATUS_OK;
}
/****************************************************************************
Reply for the lanman 2.0 protocol.
****************************************************************************/
-static void reply_lanman2(struct smb_request *req, uint16_t choice)
+static NTSTATUS reply_lanman2(struct smb_request *req, uint16_t choice)
{
int secword=0;
time_t t = time(NULL);
@@ -158,7 +158,7 @@ static void reply_lanman2(struct smb_request *req, uint16_t choice)
status = smbXsrv_connection_init_tables(xconn, PROTOCOL_LANMAN2);
if (!NT_STATUS_IS_OK(status)) {
reply_nterror(req, status);
- return;
+ return status;
}
/* Reply, SMBlockread, SMBwritelock supported. */
@@ -169,6 +169,7 @@ static void reply_lanman2(struct smb_request *req, uint16_t choice)
SSVAL(req->outbuf,smb_vwv5,raw); /* readbraw and/or writebraw */
SSVAL(req->outbuf,smb_vwv10, set_server_zone_offset(t)/60);
srv_put_dos_date((char *)req->outbuf,smb_vwv8,t);
+ return NT_STATUS_OK;
}
/****************************************************************************
@@ -266,7 +267,7 @@ DATA_BLOB negprot_spnego(TALLOC_CTX *ctx, struct smbXsrv_connection *xconn)
Reply for the nt protocol.
****************************************************************************/
-static void reply_nt1(struct smb_request *req, uint16_t choice)
+static NTSTATUS reply_nt1(struct smb_request *req, uint16_t choice)
{
/* dual names + lock_and_read + nt SMBs + remote API calls */
int capabilities = CAP_NT_FIND|CAP_LOCK_AND_READ|
@@ -359,7 +360,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
status = smbXsrv_connection_init_tables(xconn, PROTOCOL_NT1);
if (!NT_STATUS_IS_OK(status)) {
reply_nterror(req, status);
- return;
+ return status;
}
SSVAL(req->outbuf,smb_vwv1+1, lp_max_mux()); /* maxmpx */
@@ -385,7 +386,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
if (ret == -1) {
DEBUG(0, ("Could not push challenge\n"));
reply_nterror(req, NT_STATUS_NO_MEMORY);
- return;
+ return NT_STATUS_NO_MEMORY;
}
SCVAL(req->outbuf, smb_vwv16+1, ret);
}
@@ -395,7 +396,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
if (ret == -1) {
DEBUG(0, ("Could not push workgroup string\n"));
reply_nterror(req, NT_STATUS_NO_MEMORY);
- return;
+ return NT_STATUS_NO_MEMORY;
}
ret = message_push_string(&req->outbuf, lp_netbios_name(),
STR_UNICODE|STR_TERMINATE
@@ -403,7 +404,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
if (ret == -1) {
DEBUG(0, ("Could not push netbios name string\n"));
reply_nterror(req, NT_STATUS_NO_MEMORY);
- return;
+ return NT_STATUS_NO_MEMORY;
}
DEBUG(3,("not using SPNEGO\n"));
} else {
@@ -411,14 +412,14 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
if (spnego_blob.data == NULL) {
reply_nterror(req, NT_STATUS_NO_MEMORY);
- return;
+ return NT_STATUS_NO_MEMORY;
}
ret = message_push_blob(&req->outbuf, spnego_blob);
if (ret == -1) {
DEBUG(0, ("Could not push spnego blob\n"));
reply_nterror(req, NT_STATUS_NO_MEMORY);
- return;
+ return NT_STATUS_NO_MEMORY;
}
data_blob_free(&spnego_blob);
@@ -426,7 +427,7 @@ static void reply_nt1(struct smb_request *req, uint16_t choice)
DEBUG(3,("using SPNEGO\n"));
}
- return;
+ return NT_STATUS_OK;
}
/* these are the protocol lists used for auto architecture detection:
@@ -540,7 +541,7 @@ protocol [SMB 2.???]
static const struct {
const char *proto_name;
const char *short_name;
- void (*proto_reply_fn)(struct smb_request *req, uint16_t choice);
+ NTSTATUS (*proto_reply_fn)(struct smb_request *req, uint16_t choice);
int protocol_level;
} supported_protocols[] = {
{"SMB 2.???", "SMB2_FF", reply_smb20ff, PROTOCOL_SMB2_10},
@@ -579,6 +580,7 @@ void reply_negprot(struct smb_request *req)
bool signing_required = true;
int max_proto;
int min_proto;
+ NTSTATUS status;
START_PROFILE(SMBnegprot);
@@ -586,7 +588,6 @@ void reply_negprot(struct smb_request *req)
END_PROFILE(SMBnegprot);
exit_server_cleanly("multiple negprot's are not permitted");
}
- xconn->smb1.negprot.done = true;
if (req->buflen == 0) {
DEBUG(0, ("negprot got no protocols\n"));
@@ -767,11 +768,17 @@ void reply_negprot(struct smb_request *req)
fstrcpy(remote_proto,supported_protocols[protocol].short_name);
reload_services(sconn, conn_snum_used, true);
- supported_protocols[protocol].proto_reply_fn(req, choice);
+ status = supported_protocols[protocol].proto_reply_fn(req, choice);
+ if (!NT_STATUS_IS_OK(status)) {
+ exit_server_cleanly("negprot function failed\n");
+ }
+
DEBUG(3,("Selected protocol %s\n",supported_protocols[protocol].proto_name));
DBG_INFO("negprot index=%zu\n", choice);
+ xconn->smb1.negprot.done = true;
+
/* We always have xconn->smb1.signing_state also for >= SMB2_02 */
signing_required = smb_signing_is_mandatory(xconn->smb1.signing_state);
if (signing_required && (chosen_level < PROTOCOL_NT1)) {
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index df6f22bc370..e1211ad16a4 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1970,7 +1970,14 @@ static void process_smb(struct smbXsrv_connection *xconn,
if (smbd_is_smb2_header(inbuf, nread)) {
const uint8_t *inpdu = inbuf + NBT_HDR_SIZE;
size_t pdulen = nread - NBT_HDR_SIZE;
- smbd_smb2_process_negprot(xconn, 0, inpdu, pdulen);
+ NTSTATUS status = smbd_smb2_process_negprot(
+ xconn,
+ 0,
+ inpdu,
+ pdulen);
+ if (!NT_STATUS_IS_OK(status)) {
+ exit_server_cleanly("SMB2 negprot fail");
+ }
return;
}
if (nread >= smb_size && valid_smb_header(inbuf)
diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
index 6e7201b1cd8..2c1d4185b28 100644
--- a/source3/smbd/smb2_negprot.c
+++ b/source3/smbd/smb2_negprot.c
@@ -36,7 +36,7 @@ extern fstring remote_proto;
* this is the entry point if SMB2 is selected via
* the SMB negprot and the given dialect.
*/
-static void reply_smb20xx(struct smb_request *req, uint16_t dialect)
+static NTSTATUS reply_smb20xx(struct smb_request *req, uint16_t dialect)
{
uint8_t *smb2_inpdu;
uint8_t *smb2_hdr;
@@ -48,7 +48,7 @@ static void reply_smb20xx(struct smb_request *req, uint16_t dialect)
if (smb2_inpdu == NULL) {
DEBUG(0, ("Could not push spnego blob\n"));
reply_nterror(req, NT_STATUS_NO_MEMORY);
- return;
+ return NT_STATUS_NO_MEMORY;
}
smb2_hdr = smb2_inpdu;
smb2_body = smb2_hdr + SMB2_HDR_BODY;
@@ -64,28 +64,27 @@ static void reply_smb20xx(struct smb_request *req, uint16_t dialect)
req->outbuf = NULL;
- smbd_smb2_process_negprot(req->xconn, 0, smb2_inpdu, len);
- return;
+ return smbd_smb2_process_negprot(req->xconn, 0, smb2_inpdu, len);
}
/*
* this is the entry point if SMB2 is selected via
* the SMB negprot and the "SMB 2.002" dialect.
*/
-void reply_smb2002(struct smb_request *req, uint16_t choice)
+NTSTATUS reply_smb2002(struct smb_request *req, uint16_t choice)
{
- reply_smb20xx(req, SMB2_DIALECT_REVISION_202);
+ return reply_smb20xx(req, SMB2_DIALECT_REVISION_202);
}
/*
* this is the entry point if SMB2 is selected via
* the SMB negprot and the "SMB 2.???" dialect.
*/
-void reply_smb20ff(struct smb_request *req, uint16_t choice)
+NTSTATUS reply_smb20ff(struct smb_request *req, uint16_t choice)
{
struct smbXsrv_connection *xconn = req->xconn;
xconn->smb2.allow_2ff = true;
- reply_smb20xx(req, SMB2_DIALECT_REVISION_2FF);
+ return reply_smb20xx(req, SMB2_DIALECT_REVISION_2FF);
}
enum protocol_types smbd_smb2_protocol_dialect_match(const uint8_t *indyn,
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 7044ecb991a..9cf8841d827 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -3629,7 +3629,7 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbXsrv_connection *xconn
return NT_STATUS_OK;
}
-void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
+NTSTATUS smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
uint64_t expected_seq_low,
const uint8_t *inpdu, size_t size)
{
@@ -3643,25 +3643,25 @@ void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
status = smbd_initialize_smb2(xconn, expected_seq_low);
if (!NT_STATUS_IS_OK(status)) {
smbd_server_connection_terminate(xconn, nt_errstr(status));
- return;
+ return status;
}
status = smbd_smb2_request_create(xconn, inpdu, size, &req);
if (!NT_STATUS_IS_OK(status)) {
smbd_server_connection_terminate(xconn, nt_errstr(status));
- return;
+ return status;
}
status = smbd_smb2_request_validate(req);
if (!NT_STATUS_IS_OK(status)) {
smbd_server_connection_terminate(xconn, nt_errstr(status));
- return;
+ return status;
}
status = smbd_smb2_request_setup_out(req);
if (!NT_STATUS_IS_OK(status)) {
smbd_server_connection_terminate(xconn, nt_errstr(status));
- return;
+ return status;
}
#ifdef WITH_PROFILE
@@ -3676,16 +3676,17 @@ void smbd_smb2_process_negprot(struct smbXsrv_connection *xconn,
status = smbd_smb2_request_dispatch(req);
if (!NT_STATUS_IS_OK(status)) {
smbd_server_connection_terminate(xconn, nt_errstr(status));
- return;
+ return status;
}
status = smbd_smb2_request_next_incoming(xconn);
if (!NT_STATUS_IS_OK(status)) {
smbd_server_connection_terminate(xconn, nt_errstr(status));
- return;
+ return status;
}
sconn->num_requests++;
+ return NT_STATUS_OK;
}
static int socket_error_from_errno(int ret,
--
Samba Shared Repository
More information about the samba-cvs
mailing list