v4 of patch (includes minor change to set local variable "num_algs" once to number of algorithms and use that throughout build_signing_context to make code a little clearer) On Wed, Jul 7, 2021 at 11:27 PM Steve French wrote: > > v3 of patch. Updated with additional feedback from Ronnie (to make it > more context len and datalength clearer) > > > On Wed, Jul 7, 2021 at 9:44 PM Steve French wrote: > > > > 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 the first of three supported signing algorithms (AES-CMAC) > > and decoding the response. A followon patch will add support > > for sending the other two (including AES-GMAC, which is fastest) > > and changing the signing algorithm used based on what was > > negotiated. > > > > To allow the client to request GMAC signing set module parameter > > "enable_negotiate_signing" to 1. > > > > Reviewed-by: Ronnie Sahlberg > > Signed-off-by: Steve French > > --- > > fs/cifs/cifsfs.c | 4 +++ > > fs/cifs/cifsglob.h | 3 ++ > > fs/cifs/smb2pdu.c | 83 ++++++++++++++++++++++++++++++++++++++++------ > > fs/cifs/smb2pdu.h | 5 ++- > > 4 files changed, 84 insertions(+), 11 deletions(-) > > > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > > index 9fb874dd8d24..476b07213fcd 100644 > > --- a/fs/cifs/cifsfs.c > > +++ b/fs/cifs/cifsfs.c > > @@ -65,6 +65,7 @@ bool lookupCacheEnabled = true; > > bool disable_legacy_dialects; /* false by default */ > > bool enable_gcm_256 = true; > > bool require_gcm_256; /* false by default */ > > +bool enable_negotiate_signing; /* false by default */ > > unsigned int global_secflags = CIFSSEC_DEF; > > /* unsigned int ntlmv2_support = 0; */ > > unsigned int sign_CIFS_PDUs = 1; > > @@ -104,6 +105,9 @@ MODULE_PARM_DESC(enable_gcm_256, "Enable > > requesting strongest (256 bit) GCM encr > > module_param(require_gcm_256, bool, 0644); > > MODULE_PARM_DESC(require_gcm_256, "Require strongest (256 bit) GCM > > encryption. Default: n/N/0"); > > > > +module_param(enable_negotiate_signing, bool, 0644); > > +MODULE_PARM_DESC(enable_GMAC_signing, "Enable requesting faster > > (GMAC) packet signing. Default: n/N/0"); > > + > > module_param(disable_legacy_dialects, bool, 0644); > > MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be " > > "helpful to restrict the ability to " > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > > index 921680fb7931..3c2e117bb926 100644 > > --- a/fs/cifs/cifsglob.h > > +++ b/fs/cifs/cifsglob.h > > @@ -667,9 +667,11 @@ struct TCP_Server_Info { > > unsigned int max_write; > > unsigned int min_offload; > > __le16 compress_algorithm; > > + __u16 signing_algorithm; > > __le16 cipher_type; > > /* save initital negprot hash */ > > __u8 preauth_sha_hash[SMB2_PREAUTH_HASH_SIZE]; > > + bool signing_negotiated; /* true if valid signing context rcvd from server */ > > bool posix_ext_supported; > > struct delayed_work reconnect; /* reconnect workqueue job */ > > struct mutex reconnect_mutex; /* prevent simultaneous reconnects */ > > @@ -1869,6 +1871,7 @@ extern unsigned int global_secflags; /* if on, > > session setup sent > > extern unsigned int sign_CIFS_PDUs; /* enable smb packet signing */ > > extern bool enable_gcm_256; /* allow optional negotiate of strongest > > signing (aes-gcm-256) */ > > extern bool require_gcm_256; /* require use of strongest signing > > (aes-gcm-256) */ > > +extern bool enable_negotiate_signing; /* request use of faster (GMAC) > > signing if available */ > > extern bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/ > > extern unsigned int CIFSMaxBufSize; /* max size not including hdr */ > > extern unsigned int cifs_min_rcv; /* min size of big ntwrk buf pool */ > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > > index 962826dc3316..757f145e70e5 100644 > > --- a/fs/cifs/smb2pdu.c > > +++ b/fs/cifs/smb2pdu.c > > @@ -433,6 +433,23 @@ build_compression_ctxt(struct > > smb2_compression_capabilities_context *pneg_ctxt) > > pneg_ctxt->CompressionAlgorithms[2] = SMB3_COMPRESS_LZNT1; > > } > > > > +static void > > +build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt) > > +{ > > + pneg_ctxt->ContextType = SMB2_SIGNING_CAPABILITIES; > > + /* > > + * Data length must be increased here if more than 3 signing algorithms > > + * sent but currently only 3 are defined. And context Data length must > > + * be rounded to multiple of 8 for some servers. > > + */ > > + pneg_ctxt->DataLength = > > + cpu_to_le16(DIV_ROUND_UP(sizeof(struct smb2_signing_capabilities) - > > + sizeof(struct smb2_neg_context), 8) * 8); > > + pneg_ctxt->SigningAlgorithmCount = cpu_to_le16(1); > > + pneg_ctxt->SigningAlgorithms[0] = cpu_to_le16(SIGNING_ALG_AES_CMAC); > > + /* TBD add SIGNING_ALG_AES_GMAC and/or SIGNING_ALG_HMAC_SHA256 */ > > +} > > + > > static void > > build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt) > > { > > @@ -498,7 +515,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, > > struct TCP_Server_Info *server, unsigned int *total_len) > > { > > char *pneg_ctxt; > > - unsigned int ctxt_len; > > + unsigned int ctxt_len, neg_context_count; > > > > if (*total_len > 200) { > > /* In case length corrupted don't want to overrun smb buffer */ > > @@ -525,6 +542,17 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, > > *total_len += ctxt_len; > > pneg_ctxt += ctxt_len; > > > > + ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt, > > + server->hostname); > > + *total_len += ctxt_len; > > + pneg_ctxt += ctxt_len; > > + > > + build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt); > > + *total_len += sizeof(struct smb2_posix_neg_context); > > + pneg_ctxt += sizeof(struct smb2_posix_neg_context); > > + > > + neg_context_count = 4; > > + > > if (server->compress_algorithm) { > > build_compression_ctxt((struct smb2_compression_capabilities_context *) > > pneg_ctxt); > > @@ -533,17 +561,24 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, > > 8) * 8; > > *total_len += ctxt_len; > > pneg_ctxt += ctxt_len; > > - req->NegotiateContextCount = cpu_to_le16(5); > > - } else > > - req->NegotiateContextCount = cpu_to_le16(4); > > + neg_context_count++; > > + } > > > > - ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt, > > - server->hostname); > > - *total_len += ctxt_len; > > - pneg_ctxt += ctxt_len; > > + if (enable_negotiate_signing) { > > + pr_warn_once("requesting GMAC signing is experimental\n"); > > + build_signing_ctxt((struct smb2_signing_capabilities *) > > + pneg_ctxt); > > + ctxt_len = DIV_ROUND_UP( > > + sizeof(struct smb2_signing_capabilities), > > + 8) * 8; > > + *total_len += ctxt_len; > > + pneg_ctxt += ctxt_len; > > + neg_context_count++; > > + } > > + > > + /* check for and add transport_capabilities and signing capabilities */ > > + req->NegotiateContextCount = cpu_to_le16(neg_context_count); > > > > - build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt); > > - *total_len += sizeof(struct smb2_posix_neg_context); > > } > > > > static void decode_preauth_context(struct smb2_preauth_neg_context *ctxt) > > @@ -632,6 +667,31 @@ static int decode_encrypt_ctx(struct > > TCP_Server_Info *server, > > return 0; > > } > > > > +static void decode_signing_ctx(struct TCP_Server_Info *server, > > + struct smb2_signing_capabilities *pctxt) > > +{ > > + unsigned int len = le16_to_cpu(pctxt->DataLength); > > + > > + if ((len < 4) || (len > 16)) { > > + pr_warn_once("server sent bad signing negcontext\n"); > > + return; > > + } > > + if (le16_to_cpu(pctxt->SigningAlgorithmCount) != 1) { > > + pr_warn_once("Invalid signing algorithm count\n"); > > + return; > > + } > > + if (le16_to_cpu(pctxt->SigningAlgorithms[0]) > 2) { > > + pr_warn_once("unknown signing algorithm\n"); > > + return; > > + } > > + > > + server->signing_negotiated = true; > > + server->signing_algorithm = le16_to_cpu(pctxt->SigningAlgorithms[0]); > > + cifs_dbg(FYI, "signing algorithm %d chosen\n", > > + server->signing_algorithm); > > +} > > + > > + > > static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp, > > struct TCP_Server_Info *server, > > unsigned int len_of_smb) > > @@ -675,6 +735,9 @@ static int smb311_decode_neg_context(struct > > smb2_negotiate_rsp *rsp, > > (struct smb2_compression_capabilities_context *)pctx); > > else if (pctx->ContextType == SMB2_POSIX_EXTENSIONS_AVAILABLE) > > server->posix_ext_supported = true; > > + else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) > > + decode_signing_ctx(server, > > + (struct smb2_signing_capabilities *)pctx); > > else > > cifs_server_dbg(VFS, "unknown negcontext of type %d ignored\n", > > le16_to_cpu(pctx->ContextType)); > > diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h > > index ba75e65924ac..4b27cb9105fd 100644 > > --- a/fs/cifs/smb2pdu.h > > +++ b/fs/cifs/smb2pdu.h > > @@ -329,7 +329,7 @@ struct smb2_neg_context { > > __le16 ContextType; > > __le16 DataLength; > > __le32 Reserved; > > - /* Followed by array of data */ > > + /* Followed by array of data. NOTE: some servers require padding to > > 8 byte boundary */ > > } __packed; > > > > #define SMB311_LINUX_CLIENT_SALT_SIZE 32 > > @@ -394,6 +394,7 @@ struct smb2_compression_capabilities_context { > > __u16 Padding; > > __u32 Flags; > > __le16 CompressionAlgorithms[3]; > > + /* Check if pad needed */ > > } __packed; > > > > /* > > @@ -420,6 +421,7 @@ struct smb2_transport_capabilities_context { > > __le16 DataLength; > > __u32 Reserved; > > __le32 Flags; > > + __u32 Pad; > > } __packed; > > > > /* > > @@ -458,6 +460,7 @@ struct smb2_signing_capabilities { > > __u32 Reserved; > > __le16 SigningAlgorithmCount; > > __le16 SigningAlgorithms[]; > > + /* Followed by padding to 8 byte boundary (required by some servers) */ > > } __packed; > > > > #define POSIX_CTXT_DATA_LEN 16 > > > > -- > > Thanks, > > > > Steve > > > > -- > Thanks, > > Steve -- Thanks, Steve