From e0e7fe1be067495a8e921461b911be0d543270d4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:28:35 +0100 Subject: [PATCH 1/9] libcli/smb: add SMB_BUFFER_SIZE_MIN/MAX defines Signed-off-by: Stefan Metzmacher --- libcli/smb/smb_constants.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libcli/smb/smb_constants.h b/libcli/smb/smb_constants.h index 4b24be0..4190e64 100644 --- a/libcli/smb/smb_constants.h +++ b/libcli/smb/smb_constants.h @@ -209,6 +209,14 @@ enum smb_signing_setting { #define NEGOTIATE_SECURITY_SIGNATURES_ENABLED 0x04 #define NEGOTIATE_SECURITY_SIGNATURES_REQUIRED 0x08 +/* + * The negotiated buffer size for non LARGE_READX/WRITEX + * should be limited to uint16_t and has to be at least + * 500, which is the default for MinClientBufferSize on Windows. + */ +#define SMB_BUFFER_SIZE_MIN 500 +#define SMB_BUFFER_SIZE_MAX 65535 + /* Capabilities. see ftp.microsoft.com/developr/drg/cifs/cifs/cifs4.txt */ #define CAP_RAW_MODE 0x00000001 -- 1.7.9.5 From 8d8a05a3d77e31432be868a946f968b4eeceb47d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:45:35 +0100 Subject: [PATCH 2/9] s3:smbd: use SMB_BUFFER_SIZE_MIN/MAX to limit lp_max_xmit() The current limit of 128*1024 causes problems as the value has to be <= UINT16_MAX otherwise some clients get confused, as they want to use the MaxBufferSize value from the negprot response (uint32_t) for the MaxBufferSize value in thet session setup request (uint16_t). E.g. Windows 7 (as client) sends MaxBufferSize = 0 if the server value is > UINT16_MAX. Signed-off-by: Stefan Metzmacher --- source3/smbd/process.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source3/smbd/process.c b/source3/smbd/process.c index 30323a2..e845c3a 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -3332,6 +3332,7 @@ void smbd_process(struct tevent_context *ev_ctx, const char *remaddr = NULL; char *rhost; int ret; + int tmp; conn = talloc_zero(ev_ctx, struct smbXsrv_connection); if (conn == NULL) { @@ -3628,7 +3629,11 @@ void smbd_process(struct tevent_context *ev_ctx, sconn->nbt.got_session = false; - sconn->smb1.negprot.max_recv = MIN(lp_max_xmit(),BUFFER_SIZE); + tmp = lp_max_xmit(); + tmp = MAX(tmp, SMB_BUFFER_SIZE_MIN); + tmp = MIN(tmp, SMB_BUFFER_SIZE_MAX); + + sconn->smb1.negprot.max_recv = tmp; sconn->smb1.sessions.done_sesssetup = false; sconn->smb1.sessions.max_send = BUFFER_SIZE; -- 1.7.9.5 From 38be455bc4fad3fe95e6e13c953fab3462e24614 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:50:49 +0100 Subject: [PATCH 3/9] s3:smbd: use sconn->smb1.sessions.max_send = SMB_BUFFER_SIZE_MAX SMB_BUFFER_SIZE_MAX is UINT16_MAX and the largest value a client can possibly specify in the session setup request. Signed-off-by: Stefan Metzmacher --- source3/smbd/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/process.c b/source3/smbd/process.c index e845c3a..01f4b01 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -3636,7 +3636,7 @@ void smbd_process(struct tevent_context *ev_ctx, sconn->smb1.negprot.max_recv = tmp; sconn->smb1.sessions.done_sesssetup = false; - sconn->smb1.sessions.max_send = BUFFER_SIZE; + sconn->smb1.sessions.max_send = SMB_BUFFER_SIZE_MAX; if (!init_dptrs(sconn)) { exit_server("init_dptrs() failed"); -- 1.7.9.5 From b0a275e0e9a08b9073008750586a1c83cc162788 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:52:09 +0100 Subject: [PATCH 4/9] s3:smbd: reject a MaxBufferSize < SMB_BUFFER_SIZE_MIN (500) in a session setup request This makes sure sconn->smb1.sessions.max_send is always >= SMB_BUFFER_SIZE_MIN. Signed-off-by: Stefan Metzmacher --- source3/smbd/sesssetup.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c index 02cb445..4b86a99 100644 --- a/source3/smbd/sesssetup.c +++ b/source3/smbd/sesssetup.c @@ -383,10 +383,13 @@ static void reply_sesssetup_and_X_spnego(struct smb_request *req) } if (!sconn->smb1.sessions.done_sesssetup) { - sconn->smb1.sessions.max_send = - MIN(sconn->smb1.sessions.max_send,smb_bufsize); + if (smb_bufsize < SMB_BUFFER_SIZE_MIN) { + reply_force_doserror(req, ERRSRV, ERRerror); + return; + } + sconn->smb1.sessions.max_send = smb_bufsize; + sconn->smb1.sessions.done_sesssetup = true; } - sconn->smb1.sessions.done_sesssetup = true; /* current_user_info is changed on new vuid */ reload_services(sconn, conn_snum_used, true); @@ -1088,10 +1091,14 @@ void reply_sesssetup_and_X(struct smb_request *req) req->vuid = sess_vuid; if (!sconn->smb1.sessions.done_sesssetup) { - sconn->smb1.sessions.max_send = - MIN(sconn->smb1.sessions.max_send,smb_bufsize); + if (smb_bufsize < SMB_BUFFER_SIZE_MIN) { + reply_force_doserror(req, ERRSRV, ERRerror); + END_PROFILE(SMBsesssetupX); + return; + } + sconn->smb1.sessions.max_send = smb_bufsize; + sconn->smb1.sessions.done_sesssetup = true; } - sconn->smb1.sessions.done_sesssetup = true; END_PROFILE(SMBsesssetupX); } -- 1.7.9.5 From 258b9191630875dc146bb2c3b15cdac54bf72051 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:53:45 +0100 Subject: [PATCH 5/9] s3:smbd: use SMB_BUFFER_SIZE_MIN instead of '500' in ipc.c Signed-off-by: Stefan Metzmacher --- source3/smbd/ipc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/smbd/ipc.c b/source3/smbd/ipc.c index 91d5047..60cbbd6 100644 --- a/source3/smbd/ipc.c +++ b/source3/smbd/ipc.c @@ -113,8 +113,8 @@ void send_trans_reply(connection_struct *conn, if (buffer_too_large) DEBUG(5,("send_trans_reply: buffer %d too large\n", ldata )); - this_lparam = MIN(lparam,max_send - 500); /* hack */ - this_ldata = MIN(ldata,max_send - (500+this_lparam)); + this_lparam = MIN(lparam,max_send - SMB_BUFFER_SIZE_MIN); /* hack */ + this_ldata = MIN(ldata,max_send - (SMB_BUFFER_SIZE_MIN+this_lparam)); align = ((this_lparam)%4); @@ -163,9 +163,9 @@ void send_trans_reply(connection_struct *conn, while (tot_data_sent < ldata || tot_param_sent < lparam) { this_lparam = MIN(lparam-tot_param_sent, - max_send - 500); /* hack */ + max_send - SMB_BUFFER_SIZE_MIN); /* hack */ this_ldata = MIN(ldata -tot_data_sent, - max_send - (500+this_lparam)); + max_send - (SMB_BUFFER_SIZE_MIN+this_lparam)); if(this_lparam < 0) this_lparam = 0; -- 1.7.9.5 From 4de58f23eb7f72d51a066d47df4ce7fbff2e6fec Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:54:41 +0100 Subject: [PATCH 6/9] s3:smbd: use sconn->smb1.sessions.max_recv instead of BUFFER_SIZE The calculation for the possible buffer size should use the client provided limits. Signed-off-by: Stefan Metzmacher --- source3/smbd/reply.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 72dadf6..3f5ac36 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1700,7 +1700,7 @@ void reply_search(struct smb_request *req) unsigned int i; maxentries = MIN( maxentries, - ((BUFFER_SIZE - + ((req->sconn->smb1.sessions.max_send - ((uint8 *)smb_buf(req->outbuf) + 3 - req->outbuf)) /DIR_STRUCT_SIZE)); @@ -3490,7 +3490,8 @@ void reply_lockread(struct smb_request *req) numtoread = SVAL(req->vwv+1, 0); startpos = IVAL_TO_SMB_OFF_T(req->vwv+2, 0); - numtoread = MIN(BUFFER_SIZE - (smb_size + 3*2 + 3), numtoread); + numtoread = MIN(sconn->smb1.sessions.max_send - (smb_size + 3*2 + 3), + numtoread); reply_outbuf(req, 5, numtoread + 3); @@ -3600,17 +3601,15 @@ void reply_read(struct smb_request *req) numtoread = SVAL(req->vwv+1, 0); startpos = IVAL_TO_SMB_OFF_T(req->vwv+2, 0); - numtoread = MIN(BUFFER_SIZE-outsize,numtoread); - /* * The requested read size cannot be greater than max_recv. JRA. */ - if (numtoread > sconn->smb1.negprot.max_recv) { + if (numtoread > sconn->smb1.sessions.max_recv) { DEBUG(0,("reply_read: requested read size (%u) is greater than maximum allowed (%u). \ Returning short read of maximum allowed for compatibility with Windows 2000.\n", (unsigned int)numtoread, - (unsigned int)sconn->smb1.negprot.max_recv)); - numtoread = MIN(numtoread, sconn->smb1.negprot.max_recv); + (unsigned int)sconn->smb1.sessions.max_recv)); + numtoread = MIN(numtoread, sconn->smb1.sessions.max_recv); } reply_outbuf(req, 5, numtoread+3); -- 1.7.9.5 From 2e13de00078888466ca1f6d7ad2afe970fdbd0db Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:56:12 +0100 Subject: [PATCH 7/9] s3:smbd: s/BUFFER_SIZE/LARGE_WRITEX_BUFFER_SIZE Signed-off-by: Stefan Metzmacher --- source3/include/smb.h | 4 +--- source3/smbd/process.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/include/smb.h b/source3/include/smb.h index 1f6813e..aab4ff5 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -32,11 +32,9 @@ /* logged when starting the various Samba daemons */ #define COPYRIGHT_STARTUP_MESSAGE "Copyright Andrew Tridgell and the Samba Team 1992-2014" - -#define BUFFER_SIZE (128*1024) - #define SAFETY_MARGIN 1024 #define LARGE_WRITEX_HDR_SIZE 65 +#define LARGE_WRITEX_BUFFER_SIZE (128*1024) #define NMB_PORT 137 #define DGRAM_PORT 138 diff --git a/source3/smbd/process.c b/source3/smbd/process.c index 01f4b01..ca8764c 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -238,7 +238,7 @@ static bool valid_packet_size(size_t len) * of header. Don't print the error if this fits.... JRA. */ - if (len > (BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) { + if (len > (LARGE_WRITEX_BUFFER_SIZE + LARGE_WRITEX_HDR_SIZE)) { DEBUG(0,("Invalid packet length! (%lu bytes).\n", (unsigned long)len)); return false; -- 1.7.9.5 From 977ec782ea22e62ea714eff365b268ecacae0c38 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:56:45 +0100 Subject: [PATCH 8/9] DISCUSS fix LARGE_WRITEX_BUFFER_SIZE --- source3/include/smb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/include/smb.h b/source3/include/smb.h index aab4ff5..05b1f8d 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -34,7 +34,7 @@ #define SAFETY_MARGIN 1024 #define LARGE_WRITEX_HDR_SIZE 65 -#define LARGE_WRITEX_BUFFER_SIZE (128*1024) +#define LARGE_WRITEX_BUFFER_SIZE (0x1FFFF - LARGE_WRITEX_HDR_SIZE) #define NMB_PORT 137 #define DGRAM_PORT 138 -- 1.7.9.5 From 1fc9a53a97decc8c51a5aadc2962120d69c6206d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 6 Dec 2013 13:57:15 +0100 Subject: [PATCH 9/9] s3:include: let CLI_BUFFER_SIZE be an alias of SMB_BUFFER_SIZE_MAX Signed-off-by: Stefan Metzmacher --- source3/include/client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/include/client.h b/source3/include/client.h index 35fa2f1..59fb104 100644 --- a/source3/include/client.h +++ b/source3/include/client.h @@ -22,7 +22,7 @@ #ifndef _CLIENT_H #define _CLIENT_H -#define CLI_BUFFER_SIZE (0xFFFF) +#define CLI_BUFFER_SIZE SMB_BUFFER_SIZE_MAX /* default client timeout to 20 seconds on most commands */ #define CLIENT_TIMEOUT (20 * 1000) -- 1.7.9.5