From e4b60e92d4f878b774eca22fa4c00fa04f6354b4 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 16 Jul 2021 14:51:30 +0900 Subject: ksmbd: fix wrong compression context size Use smb2_compression_ctx instead of smb2_encryption_neg_context. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index c1a594599431..f9e6e2bd4cbf 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -924,7 +924,7 @@ static int decode_compress_ctxt(struct ksmbd_conn *conn, * Return compression context size in request. * So need to plus extra number of CompressionAlgorithms size. */ - return sizeof(struct smb2_encryption_neg_context) + + return sizeof(struct smb2_compression_ctx) + ((algo_cnt - 1) * 2); } -- cgit From 58090b175271870842d823622013d4499f462a10 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 16 Jul 2021 14:52:09 +0900 Subject: ksmbd: fix wrong error status return on session setup When user insert wrong password, ksmbd return STATUS_INVALID_PARAMETER error status to client. It will make user confusing whether it is not password problem. This patch change error status to STATUS_LOGON_FAILURE. and return STATUS_INSUFFICIENT_RESOURCES if memory allocation failed on session setup. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index f9e6e2bd4cbf..77e42a572825 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -1338,8 +1338,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) user = session_user(conn, req); if (!user) { ksmbd_debug(SMB, "Unknown user name or an error\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return -EINVAL; + return -EPERM; } /* Check for previous session */ @@ -1363,8 +1362,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) if (user_guest(sess->user)) { if (conn->sign) { ksmbd_debug(SMB, "Guest login not allowed when signing enabled\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return -EACCES; + return -EPERM; } rsp->SessionFlags = SMB2_SESSION_FLAG_IS_GUEST_LE; @@ -1377,8 +1375,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) if (rc) { set_user_flag(sess->user, KSMBD_USER_FLAG_BAD_PASSWORD); ksmbd_debug(SMB, "authentication failed\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return -EINVAL; + return -EPERM; } /* @@ -1403,8 +1400,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) if (rc) { ksmbd_debug(SMB, "SMB3 encryption key generation failed\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return rc; + return -EINVAL; } sess->enc = true; rsp->SessionFlags = SMB2_SESSION_FLAG_ENCRYPT_DATA_LE; @@ -1434,16 +1430,14 @@ binding_session: rc = conn->ops->generate_signingkey(sess, conn); if (rc) { ksmbd_debug(SMB, "SMB3 signing key generation failed\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return rc; + return -EINVAL; } } if (conn->dialect > SMB20_PROT_ID) { if (!ksmbd_conn_lookup_dialect(conn)) { pr_err("fail to verify the dialect\n"); - rsp->hdr.Status = STATUS_USER_SESSION_DELETED; - return -EPERM; + return -ENOENT; } } return 0; @@ -1483,8 +1477,7 @@ static int krb5_authenticate(struct ksmbd_work *work) out_blob, &out_len); if (retval) { ksmbd_debug(SMB, "krb5 authentication failed\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return retval; + return -EINVAL; } rsp->SecurityBufferLength = cpu_to_le16(out_len); inc_rfc1001_len(rsp, out_len - 1); @@ -1499,8 +1492,7 @@ static int krb5_authenticate(struct ksmbd_work *work) if (retval) { ksmbd_debug(SMB, "SMB3 encryption key generation failed\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return retval; + return -EINVAL; } sess->enc = true; rsp->SessionFlags = SMB2_SESSION_FLAG_ENCRYPT_DATA_LE; @@ -1524,16 +1516,14 @@ static int krb5_authenticate(struct ksmbd_work *work) retval = conn->ops->generate_signingkey(sess, conn); if (retval) { ksmbd_debug(SMB, "SMB3 signing key generation failed\n"); - rsp->hdr.Status = STATUS_LOGON_FAILURE; - return retval; + return -EINVAL; } } if (conn->dialect > SMB20_PROT_ID) { if (!ksmbd_conn_lookup_dialect(conn)) { pr_err("fail to verify the dialect\n"); - rsp->hdr.Status = STATUS_USER_SESSION_DELETED; - return -EPERM; + return -ENOENT; } } return 0; @@ -1709,6 +1699,8 @@ out_err: rsp->hdr.Status = STATUS_REQUEST_NOT_ACCEPTED; else if (rc == -EFAULT) rsp->hdr.Status = STATUS_NETWORK_SESSION_EXPIRED; + else if (rc == -ENOMEM) + rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES; else if (rc) rsp->hdr.Status = STATUS_LOGON_FAILURE; -- cgit From 67307023d02b1339e0b930b742fe5a9cd81284ca Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 16 Jul 2021 14:52:46 +0900 Subject: ksmbd: set STATUS_INVALID_PARAMETER error status if credit charge is invalid MS-SMB2 specification describe : If the calculated credit number is greater than the CreditCharge, the server MUST fail the request with the error code STATUS_INVALID_PARAMETER. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/server.c | 20 ++++++++++---------- fs/ksmbd/smb2misc.c | 9 +++++++-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c index a8c59e96a2f7..e6a9f6aa47eb 100644 --- a/fs/ksmbd/server.c +++ b/fs/ksmbd/server.c @@ -101,8 +101,8 @@ static inline int check_conn_state(struct ksmbd_work *work) return 0; } -#define TCP_HANDLER_CONTINUE 0 -#define TCP_HANDLER_ABORT 1 +#define SERVER_HANDLER_CONTINUE 0 +#define SERVER_HANDLER_ABORT 1 static int __process_request(struct ksmbd_work *work, struct ksmbd_conn *conn, u16 *cmd) @@ -112,10 +112,10 @@ static int __process_request(struct ksmbd_work *work, struct ksmbd_conn *conn, int ret; if (check_conn_state(work)) - return TCP_HANDLER_CONTINUE; + return SERVER_HANDLER_CONTINUE; if (ksmbd_verify_smb_message(work)) - return TCP_HANDLER_ABORT; + return SERVER_HANDLER_ABORT; command = conn->ops->get_cmd_val(work); *cmd = command; @@ -123,21 +123,21 @@ static int __process_request(struct ksmbd_work *work, struct ksmbd_conn *conn, andx_again: if (command >= conn->max_cmds) { conn->ops->set_rsp_status(work, STATUS_INVALID_PARAMETER); - return TCP_HANDLER_CONTINUE; + return SERVER_HANDLER_CONTINUE; } cmds = &conn->cmds[command]; if (!cmds->proc) { ksmbd_debug(SMB, "*** not implemented yet cmd = %x\n", command); conn->ops->set_rsp_status(work, STATUS_NOT_IMPLEMENTED); - return TCP_HANDLER_CONTINUE; + return SERVER_HANDLER_CONTINUE; } if (work->sess && conn->ops->is_sign_req(work, command)) { ret = conn->ops->check_sign_req(work); if (!ret) { conn->ops->set_rsp_status(work, STATUS_ACCESS_DENIED); - return TCP_HANDLER_CONTINUE; + return SERVER_HANDLER_CONTINUE; } } @@ -153,8 +153,8 @@ andx_again: } if (work->send_no_response) - return TCP_HANDLER_ABORT; - return TCP_HANDLER_CONTINUE; + return SERVER_HANDLER_ABORT; + return SERVER_HANDLER_CONTINUE; } static void __handle_ksmbd_work(struct ksmbd_work *work, @@ -203,7 +203,7 @@ static void __handle_ksmbd_work(struct ksmbd_work *work, do { rc = __process_request(work, conn, &command); - if (rc == TCP_HANDLER_ABORT) + if (rc == SERVER_HANDLER_ABORT) break; /* diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c index 4508631c5706..e68aa7d718ed 100644 --- a/fs/ksmbd/smb2misc.c +++ b/fs/ksmbd/smb2misc.c @@ -423,8 +423,13 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work) return 1; } - return work->conn->vals->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU ? - smb2_validate_credit_charge(hdr) : 0; + if ((work->conn->vals->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU) && + smb2_validate_credit_charge(hdr)) { + work->conn->ops->set_rsp_status(work, STATUS_INVALID_PARAMETER); + return 1; + } + + return 0; } int smb2_negotiate_request(struct ksmbd_work *work) -- cgit From d347d745f06c7e6503abc08f68dc3b71da71596d Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 16 Jul 2021 16:39:54 +0900 Subject: ksmbd: move credit charge verification over smb2 request size verification Move credit charge verification over smb2 request size verification to avoid being skipped. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2misc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c index e68aa7d718ed..9aa46bb3e10d 100644 --- a/fs/ksmbd/smb2misc.c +++ b/fs/ksmbd/smb2misc.c @@ -385,6 +385,12 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work) } } + if ((work->conn->vals->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU) && + smb2_validate_credit_charge(hdr)) { + work->conn->ops->set_rsp_status(work, STATUS_INVALID_PARAMETER); + return 1; + } + clc_len = smb2_calc_size(hdr); if (len != clc_len) { /* server can return one byte more due to implied bcc[0] */ @@ -423,12 +429,6 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work) return 1; } - if ((work->conn->vals->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU) && - smb2_validate_credit_charge(hdr)) { - work->conn->ops->set_rsp_status(work, STATUS_INVALID_PARAMETER); - return 1; - } - return 0; } -- cgit From 9223958816f9df133ae936c9371378ba1203e0da Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 16 Jul 2021 17:16:11 +0900 Subject: ksmbd: fix typo of MS-SMBD Fix typo : "MS-KSMBD" => "MS-SMBD". Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/transport_rdma.c | 2 +- fs/ksmbd/transport_rdma.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c index f818fe358f31..f2ae6bae83f1 100644 --- a/fs/ksmbd/transport_rdma.c +++ b/fs/ksmbd/transport_rdma.c @@ -58,7 +58,7 @@ /* * User configurable initial values per SMB_DIRECT transport connection - * as defined in [MS-KSMBD] 3.1.1.1 + * as defined in [MS-SMBD] 3.1.1.1 * Those may change after a SMB_DIRECT negotiation */ /* The local peer's maximum number of credits to grant to the peer */ diff --git a/fs/ksmbd/transport_rdma.h b/fs/ksmbd/transport_rdma.h index 72e2574079f3..0fa8adc0776f 100644 --- a/fs/ksmbd/transport_rdma.h +++ b/fs/ksmbd/transport_rdma.h @@ -9,7 +9,7 @@ #define SMB_DIRECT_PORT 5445 -/* SMB DIRECT negotiation request packet [MS-KSMBD] 2.2.1 */ +/* SMB DIRECT negotiation request packet [MS-SMBD] 2.2.1 */ struct smb_direct_negotiate_req { __le16 min_version; __le16 max_version; @@ -20,7 +20,7 @@ struct smb_direct_negotiate_req { __le32 max_fragmented_size; } __packed; -/* SMB DIRECT negotiation response packet [MS-KSMBD] 2.2.2 */ +/* SMB DIRECT negotiation response packet [MS-SMBD] 2.2.2 */ struct smb_direct_negotiate_resp { __le16 min_version; __le16 max_version; @@ -37,7 +37,7 @@ struct smb_direct_negotiate_resp { #define SMB_DIRECT_RESPONSE_REQUESTED 0x0001 -/* SMB DIRECT data transfer packet with payload [MS-KSMBD] 2.2.3 */ +/* SMB DIRECT data transfer packet with payload [MS-SMBD] 2.2.3 */ struct smb_direct_data_transfer { __le16 credits_requested; __le16 credits_granted; -- cgit From af320a739029f6f8c5c05e769fadaf88e9b7d34f Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 21 Jul 2021 10:03:19 +0900 Subject: ksmbd: add negotiate context verification This patch add negotiate context verification code to check bounds. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 118 ++++++++++++++++++++++++++++------------------------- fs/ksmbd/smb2pdu.h | 6 +-- 2 files changed, 65 insertions(+), 59 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 77e42a572825..64a4d66997a3 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -835,10 +835,10 @@ static void assemble_neg_contexts(struct ksmbd_conn *conn, build_encrypt_ctxt((struct smb2_encryption_neg_context *)pneg_ctxt, conn->cipher_type); rsp->NegotiateContextCount = cpu_to_le16(++neg_ctxt_cnt); - ctxt_size += sizeof(struct smb2_encryption_neg_context); + ctxt_size += sizeof(struct smb2_encryption_neg_context) + 2; /* Round to 8 byte boundary */ pneg_ctxt += - round_up(sizeof(struct smb2_encryption_neg_context), + round_up(sizeof(struct smb2_encryption_neg_context) + 2, 8); } @@ -850,9 +850,10 @@ static void assemble_neg_contexts(struct ksmbd_conn *conn, build_compression_ctxt((struct smb2_compression_ctx *)pneg_ctxt, conn->compress_algorithm); rsp->NegotiateContextCount = cpu_to_le16(++neg_ctxt_cnt); - ctxt_size += sizeof(struct smb2_compression_ctx); + ctxt_size += sizeof(struct smb2_compression_ctx) + 2; /* Round to 8 byte boundary */ - pneg_ctxt += round_up(sizeof(struct smb2_compression_ctx), 8); + pneg_ctxt += round_up(sizeof(struct smb2_compression_ctx) + 2, + 8); } if (conn->posix_ext_supported) { @@ -881,16 +882,23 @@ static __le32 decode_preauth_ctxt(struct ksmbd_conn *conn, return err; } -static int decode_encrypt_ctxt(struct ksmbd_conn *conn, - struct smb2_encryption_neg_context *pneg_ctxt) +static void decode_encrypt_ctxt(struct ksmbd_conn *conn, + struct smb2_encryption_neg_context *pneg_ctxt, + int len_of_ctxts) { - int i; int cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount); + int i, cphs_size = cph_cnt * sizeof(__le16); conn->cipher_type = 0; + if (sizeof(struct smb2_encryption_neg_context) + cphs_size > + len_of_ctxts) { + pr_err("Invalid cipher count(%d)\n", cph_cnt); + return; + } + if (!(server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_ENCRYPTION)) - goto out; + return; for (i = 0; i < cph_cnt; i++) { if (pneg_ctxt->Ciphers[i] == SMB2_ENCRYPTION_AES128_GCM || @@ -903,90 +911,88 @@ static int decode_encrypt_ctxt(struct ksmbd_conn *conn, break; } } - -out: - /* - * Return encrypt context size in request. - * So need to plus extra number of ciphers size. - */ - return sizeof(struct smb2_encryption_neg_context) + - ((cph_cnt - 1) * 2); } -static int decode_compress_ctxt(struct ksmbd_conn *conn, - struct smb2_compression_ctx *pneg_ctxt) +static void decode_compress_ctxt(struct ksmbd_conn *conn, + struct smb2_compression_ctx *pneg_ctxt) { - int algo_cnt = le16_to_cpu(pneg_ctxt->CompressionAlgorithmCount); - conn->compress_algorithm = SMB3_COMPRESS_NONE; - - /* - * Return compression context size in request. - * So need to plus extra number of CompressionAlgorithms size. - */ - return sizeof(struct smb2_compression_ctx) + - ((algo_cnt - 1) * 2); } static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn, struct smb2_negotiate_req *req) { - int i = 0; - __le32 status = 0; /* +4 is to account for the RFC1001 len field */ - char *pneg_ctxt = (char *)req + - le32_to_cpu(req->NegotiateContextOffset) + 4; - __le16 *ContextType = (__le16 *)pneg_ctxt; + struct smb2_neg_context *pctx = (struct smb2_neg_context *)((char *)req + 4); + int i = 0, len_of_ctxts; + int offset = le32_to_cpu(req->NegotiateContextOffset); int neg_ctxt_cnt = le16_to_cpu(req->NegotiateContextCount); - int ctxt_size; + int len_of_smb = be32_to_cpu(req->hdr.smb2_buf_length); + __le32 status = STATUS_INVALID_PARAMETER; + + ksmbd_debug(SMB, "decoding %d negotiate contexts\n", neg_ctxt_cnt); + if (len_of_smb <= offset) { + ksmbd_debug(SMB, "Invalid response: negotiate context offset\n"); + return status; + } + + len_of_ctxts = len_of_smb - offset; - ksmbd_debug(SMB, "negotiate context count = %d\n", neg_ctxt_cnt); - status = STATUS_INVALID_PARAMETER; while (i++ < neg_ctxt_cnt) { - if (*ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) { + int clen; + + /* check that offset is not beyond end of SMB */ + if (len_of_ctxts == 0) + break; + + if (len_of_ctxts < sizeof(struct smb2_neg_context)) + break; + + pctx = (struct smb2_neg_context *)((char *)pctx + offset); + clen = le16_to_cpu(pctx->DataLength); + if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts) + break; + + if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) { ksmbd_debug(SMB, "deassemble SMB2_PREAUTH_INTEGRITY_CAPABILITIES context\n"); if (conn->preauth_info->Preauth_HashId) break; status = decode_preauth_ctxt(conn, - (struct smb2_preauth_neg_context *)pneg_ctxt); - pneg_ctxt += DIV_ROUND_UP(sizeof(struct smb2_preauth_neg_context), 8) * 8; - } else if (*ContextType == SMB2_ENCRYPTION_CAPABILITIES) { + (struct smb2_preauth_neg_context *)pctx); + if (status != STATUS_SUCCESS) + break; + } else if (pctx->ContextType == SMB2_ENCRYPTION_CAPABILITIES) { ksmbd_debug(SMB, "deassemble SMB2_ENCRYPTION_CAPABILITIES context\n"); if (conn->cipher_type) break; - ctxt_size = decode_encrypt_ctxt(conn, - (struct smb2_encryption_neg_context *)pneg_ctxt); - pneg_ctxt += DIV_ROUND_UP(ctxt_size, 8) * 8; - } else if (*ContextType == SMB2_COMPRESSION_CAPABILITIES) { + decode_encrypt_ctxt(conn, + (struct smb2_encryption_neg_context *)pctx, + len_of_ctxts); + } else if (pctx->ContextType == SMB2_COMPRESSION_CAPABILITIES) { ksmbd_debug(SMB, "deassemble SMB2_COMPRESSION_CAPABILITIES context\n"); if (conn->compress_algorithm) break; - ctxt_size = decode_compress_ctxt(conn, - (struct smb2_compression_ctx *)pneg_ctxt); - pneg_ctxt += DIV_ROUND_UP(ctxt_size, 8) * 8; - } else if (*ContextType == SMB2_NETNAME_NEGOTIATE_CONTEXT_ID) { + decode_compress_ctxt(conn, + (struct smb2_compression_ctx *)pctx); + } else if (pctx->ContextType == SMB2_NETNAME_NEGOTIATE_CONTEXT_ID) { ksmbd_debug(SMB, "deassemble SMB2_NETNAME_NEGOTIATE_CONTEXT_ID context\n"); - ctxt_size = sizeof(struct smb2_netname_neg_context); - ctxt_size += DIV_ROUND_UP(le16_to_cpu(((struct smb2_netname_neg_context *) - pneg_ctxt)->DataLength), 8) * 8; - pneg_ctxt += ctxt_size; - } else if (*ContextType == SMB2_POSIX_EXTENSIONS_AVAILABLE) { + } else if (pctx->ContextType == SMB2_POSIX_EXTENSIONS_AVAILABLE) { ksmbd_debug(SMB, "deassemble SMB2_POSIX_EXTENSIONS_AVAILABLE context\n"); conn->posix_ext_supported = true; - pneg_ctxt += DIV_ROUND_UP(sizeof(struct smb2_posix_neg_context), 8) * 8; } - ContextType = (__le16 *)pneg_ctxt; - if (status != STATUS_SUCCESS) - break; + /* offsets must be 8 byte aligned */ + clen = (clen + 7) & ~0x7; + offset = clen + sizeof(struct smb2_neg_context); + len_of_ctxts -= clen + sizeof(struct smb2_neg_context); } return status; } diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h index 0eac40e1ba65..21cb93e771f7 100644 --- a/fs/ksmbd/smb2pdu.h +++ b/fs/ksmbd/smb2pdu.h @@ -299,7 +299,7 @@ struct smb2_encryption_neg_context { __le32 Reserved; /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */ __le16 CipherCount; /* AES-128-GCM and AES-128-CCM by default */ - __le16 Ciphers[1]; + __le16 Ciphers[]; } __packed; #define SMB3_COMPRESS_NONE cpu_to_le16(0x0000) @@ -314,7 +314,7 @@ struct smb2_compression_ctx { __le16 CompressionAlgorithmCount; __u16 Padding; __le32 Reserved1; - __le16 CompressionAlgorithms[1]; + __le16 CompressionAlgorithms[]; } __packed; #define POSIX_CTXT_DATA_LEN 16 @@ -329,7 +329,7 @@ struct smb2_netname_neg_context { __le16 ContextType; /* 0x100 */ __le16 DataLength; __le32 Reserved; - __le16 NetName[0]; /* hostname of target converted to UCS-2 */ + __le16 NetName[]; /* hostname of target converted to UCS-2 */ } __packed; struct smb2_negotiate_rsp { -- cgit From 378087cd17eea71c4e78e6053597e38429ccee0f Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 21 Jul 2021 10:05:53 +0900 Subject: ksmbd: add support for negotiating signing algorithm Support for faster packet signing (using GMAC instead of CMAC) can now be negotiated to some newer servers, including Windows. See MS-SMB2 section 2.2.3.17. This patch adds support for sending the new negotiate context with two supported signing algorithms(AES-CMAC, HMAC-SHA256). If client add support for AES_GMAC, Server will be supported later depend on it. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/connection.h | 2 ++ fs/ksmbd/smb2ops.c | 4 ++++ fs/ksmbd/smb2pdu.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++ fs/ksmbd/smb2pdu.h | 14 +++++++++++++ 4 files changed, 78 insertions(+) diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h index 487c2024b0d5..e5403c587a58 100644 --- a/fs/ksmbd/connection.h +++ b/fs/ksmbd/connection.h @@ -109,6 +109,8 @@ struct ksmbd_conn { __le16 cipher_type; __le16 compress_algorithm; bool posix_ext_supported; + bool signing_negotiated; + __le16 signing_algorithm; bool binding; }; diff --git a/fs/ksmbd/smb2ops.c b/fs/ksmbd/smb2ops.c index 8262908e467c..197473871aa4 100644 --- a/fs/ksmbd/smb2ops.c +++ b/fs/ksmbd/smb2ops.c @@ -204,6 +204,7 @@ void init_smb2_1_server(struct ksmbd_conn *conn) conn->cmds = smb2_0_server_cmds; conn->max_cmds = ARRAY_SIZE(smb2_0_server_cmds); conn->max_credits = SMB2_MAX_CREDITS; + conn->signing_algorithm = SIGNING_ALG_HMAC_SHA256; if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES) conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING; @@ -221,6 +222,7 @@ void init_smb3_0_server(struct ksmbd_conn *conn) conn->cmds = smb2_0_server_cmds; conn->max_cmds = ARRAY_SIZE(smb2_0_server_cmds); conn->max_credits = SMB2_MAX_CREDITS; + conn->signing_algorithm = SIGNING_ALG_AES_CMAC; if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES) conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING; @@ -245,6 +247,7 @@ void init_smb3_02_server(struct ksmbd_conn *conn) conn->cmds = smb2_0_server_cmds; conn->max_cmds = ARRAY_SIZE(smb2_0_server_cmds); conn->max_credits = SMB2_MAX_CREDITS; + conn->signing_algorithm = SIGNING_ALG_AES_CMAC; if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES) conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING; @@ -269,6 +272,7 @@ int init_smb3_11_server(struct ksmbd_conn *conn) conn->cmds = smb2_0_server_cmds; conn->max_cmds = ARRAY_SIZE(smb2_0_server_cmds); conn->max_credits = SMB2_MAX_CREDITS; + conn->signing_algorithm = SIGNING_ALG_AES_CMAC; if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES) conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING; diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 64a4d66997a3..7e6e3d8c20e8 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -786,6 +786,18 @@ static void build_compression_ctxt(struct smb2_compression_ctx *pneg_ctxt, pneg_ctxt->CompressionAlgorithms[0] = comp_algo; } +static void build_sign_cap_ctxt(struct smb2_signing_capabilities *pneg_ctxt, + __le16 sign_algo) +{ + pneg_ctxt->ContextType = SMB2_SIGNING_CAPABILITIES; + pneg_ctxt->DataLength = + cpu_to_le16((sizeof(struct smb2_signing_capabilities) + 2) + - sizeof(struct smb2_neg_context)); + pneg_ctxt->Reserved = cpu_to_le32(0); + pneg_ctxt->SigningAlgorithmCount = cpu_to_le16(1); + pneg_ctxt->SigningAlgorithms[0] = sign_algo; +} + static void build_posix_ctxt(struct smb2_posix_neg_context *pneg_ctxt) { pneg_ctxt->ContextType = SMB2_POSIX_EXTENSIONS_AVAILABLE; @@ -863,6 +875,18 @@ static void assemble_neg_contexts(struct ksmbd_conn *conn, build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt); rsp->NegotiateContextCount = cpu_to_le16(++neg_ctxt_cnt); ctxt_size += sizeof(struct smb2_posix_neg_context); + /* Round to 8 byte boundary */ + pneg_ctxt += round_up(sizeof(struct smb2_posix_neg_context), 8); + } + + if (conn->signing_negotiated) { + ctxt_size = round_up(ctxt_size, 8); + ksmbd_debug(SMB, + "assemble SMB2_SIGNING_CAPABILITIES context\n"); + build_sign_cap_ctxt((struct smb2_signing_capabilities *)pneg_ctxt, + conn->signing_algorithm); + rsp->NegotiateContextCount = cpu_to_le16(++neg_ctxt_cnt); + ctxt_size += sizeof(struct smb2_signing_capabilities) + 2; } inc_rfc1001_len(rsp, ctxt_size); @@ -919,6 +943,34 @@ static void decode_compress_ctxt(struct ksmbd_conn *conn, conn->compress_algorithm = SMB3_COMPRESS_NONE; } +static void decode_sign_cap_ctxt(struct ksmbd_conn *conn, + struct smb2_signing_capabilities *pneg_ctxt, + int len_of_ctxts) +{ + int sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount); + int i, sign_alos_size = sign_algo_cnt * sizeof(__le16); + + conn->signing_negotiated = false; + + if (sizeof(struct smb2_signing_capabilities) + sign_alos_size > + len_of_ctxts) { + pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt); + return; + } + + for (i = 0; i < sign_algo_cnt; i++) { + if (pneg_ctxt->SigningAlgorithms[i] == SIGNING_ALG_HMAC_SHA256 || + pneg_ctxt->SigningAlgorithms[i] == SIGNING_ALG_AES_CMAC) { + ksmbd_debug(SMB, "Signing Algorithm ID = 0x%x\n", + pneg_ctxt->SigningAlgorithms[i]); + conn->signing_negotiated = true; + conn->signing_algorithm = + pneg_ctxt->SigningAlgorithms[i]; + break; + } + } +} + static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn, struct smb2_negotiate_req *req) { @@ -987,6 +1039,12 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn, ksmbd_debug(SMB, "deassemble SMB2_POSIX_EXTENSIONS_AVAILABLE context\n"); conn->posix_ext_supported = true; + } else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) { + ksmbd_debug(SMB, + "deassemble SMB2_SIGNING_CAPABILITIES context\n"); + decode_sign_cap_ctxt(conn, + (struct smb2_signing_capabilities *)pctx, + len_of_ctxts); } /* offsets must be 8 byte aligned */ diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h index 21cb93e771f7..89019f67234c 100644 --- a/fs/ksmbd/smb2pdu.h +++ b/fs/ksmbd/smb2pdu.h @@ -268,6 +268,7 @@ struct preauth_integrity_info { #define SMB2_ENCRYPTION_CAPABILITIES cpu_to_le16(2) #define SMB2_COMPRESSION_CAPABILITIES cpu_to_le16(3) #define SMB2_NETNAME_NEGOTIATE_CONTEXT_ID cpu_to_le16(5) +#define SMB2_SIGNING_CAPABILITIES cpu_to_le16(8) #define SMB2_POSIX_EXTENSIONS_AVAILABLE cpu_to_le16(0x100) struct smb2_neg_context { @@ -332,6 +333,19 @@ struct smb2_netname_neg_context { __le16 NetName[]; /* hostname of target converted to UCS-2 */ } __packed; +/* Signing algorithms */ +#define SIGNING_ALG_HMAC_SHA256 cpu_to_le16(0) +#define SIGNING_ALG_AES_CMAC cpu_to_le16(1) +#define SIGNING_ALG_AES_GMAC cpu_to_le16(2) + +struct smb2_signing_capabilities { + __le16 ContextType; /* 8 */ + __le16 DataLength; + __le32 Reserved; + __le16 SigningAlgorithmCount; + __le16 SigningAlgorithms[]; +} __packed; + struct smb2_negotiate_rsp { struct smb2_hdr hdr; __le16 StructureSize; /* Must be 65 */ -- cgit From 654c8876f93677915b1a009bc7f2421ab8750bf1 Mon Sep 17 00:00:00 2001 From: Marios Makassikis Date: Fri, 23 Jul 2021 12:58:41 +0900 Subject: ksmbd: Fix potential memory leak in tcp_destroy_socket() ksmbd_socket must be freed even if kernel_sock_shutdown() somehow fails. Signed-off-by: Marios Makassikis Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/transport_tcp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/ksmbd/transport_tcp.c b/fs/ksmbd/transport_tcp.c index 56ec11ff5a9f..dc15a5ecd2e0 100644 --- a/fs/ksmbd/transport_tcp.c +++ b/fs/ksmbd/transport_tcp.c @@ -381,8 +381,7 @@ static void tcp_destroy_socket(struct socket *ksmbd_socket) ret = kernel_sock_shutdown(ksmbd_socket, SHUT_RDWR); if (ret) pr_err("Failed to shutdown socket: %d\n", ret); - else - sock_release(ksmbd_socket); + sock_release(ksmbd_socket); } /** -- cgit From 1d904eaf3f99565bdeffbed359e44dd88efbef02 Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Fri, 23 Jul 2021 13:01:06 +0900 Subject: ksmbd: fix -Wstringop-truncation warnings Kernel test bot reports the following warnings: In function 'ndr_write_string', inlined from 'ndr_encode_dos_attr' at fs/ksmbd/ndr.c:136:3: >> fs/ksmbd/ndr.c:70:2: warning: 'strncpy' destination unchanged after copying no bytes [-Wstringop-truncation] 70 | strncpy(PAYLOAD_HEAD(n), value, sz); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In function 'ndr_write_string', inlined from 'ndr_encode_dos_attr' at fs/ksmbd/ndr.c:134:3: >> fs/ksmbd/ndr.c:70:2: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation] 70 | strncpy(PAYLOAD_HEAD(n), value, sz); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/ksmbd/ndr.c: In function 'ndr_encode_dos_attr': fs/ksmbd/ndr.c:134:3: note: length computed here 134 | ndr_write_string(n, hex_attr, strlen(hex_attr)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Reported-by: kernel test robot Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/ndr.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/ksmbd/ndr.c b/fs/ksmbd/ndr.c index cf0df78259c9..df23dfbaf657 100644 --- a/fs/ksmbd/ndr.c +++ b/fs/ksmbd/ndr.c @@ -65,13 +65,15 @@ static int ndr_write_bytes(struct ndr *n, void *value, size_t sz) return 0; } -static int ndr_write_string(struct ndr *n, void *value, size_t sz) +static int ndr_write_string(struct ndr *n, char *value) { + size_t sz; + + sz = strlen(value) + 1; if (n->length <= n->offset + sz) try_to_realloc_ndr_blob(n, sz); - strncpy(ndr_get_field(n), value, sz); - sz++; + memcpy(ndr_get_field(n), value, sz); n->offset += sz; n->offset = ALIGN(n->offset, 2); return 0; @@ -134,9 +136,9 @@ int ndr_encode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da) if (da->version == 3) { snprintf(hex_attr, 10, "0x%x", da->attr); - ndr_write_string(n, hex_attr, strlen(hex_attr)); + ndr_write_string(n, hex_attr); } else { - ndr_write_string(n, "", strlen("")); + ndr_write_string(n, ""); } ndr_write_int16(n, da->version); ndr_write_int32(n, da->version); -- cgit