| Message ID | 20220929015637.14400-7-ematsumiya@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: introduce support for AES-GMAC signing | expand |
could you fix these minor checkpatch warnings?
ERROR: code indent should use tabs where possible
#74: FILE: fs/cifs/cifsglob.h:2035:
+^I^I^I^I XXX: deprecated, remove it at some point */$
WARNING: Block comments use * on subsequent lines
#74: FILE: fs/cifs/cifsglob.h:2035:
+extern bool enable_negotiate_signing; /* request use of faster (GMAC)
signing if available
+ XXX: deprecated, remove it at some point */
WARNING: Block comments use a trailing */ on a separate line
#74: FILE: fs/cifs/cifsglob.h:2035:
+ XXX: deprecated, remove it at some point */
total: 1 errors, 5 warnings, 58 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
NOTE: Whitespace errors detected.
You may wish to use scripts/cleanpatch or scripts/cleanfile
0006-cifs-deprecate-enable_negotiate_signing-module-param.patch has
style problems, please review.
On Wed, Sep 28, 2022 at 8:57 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> Deprecate enable_negotiate_signing module parameter as it's irrelevant
> since the requested dialect and server support will dictate which
> algorithm will actually be used.
>
> Send a negotiate signing context on every SMB 3.1.1 negotiation now.
> AES-CMAC will still be used instead, iff, SMB 3.0.x negotiated or
> SMB 3.1.1 negotiated, but server doesn't support AES-GMAC.
>
> Warn the user if, for whatever reason, the module was loaded with
> 'enable_negotiate_signing=0'.
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/cifsfs.c | 14 +++++++++++---
> fs/cifs/cifsglob.h | 3 ++-
> fs/cifs/smb2pdu.c | 11 ++++-------
> 3 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 8042d7280dec..c46dc9edf6ec 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -65,7 +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 */
> +bool enable_negotiate_signing = true; /* deprecated -- always true now */
> unsigned int global_secflags = CIFSSEC_DEF;
> /* unsigned int ntlmv2_support = 0; */
> unsigned int sign_CIFS_PDUs = 1;
> @@ -133,8 +133,12 @@ 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_negotiate_signing, "Enable negotiating packet signing algorithm with server. Default: n/N/0");
> +/* XXX: remove this at some point */
> +module_param(enable_negotiate_signing, bool, 0);
> +MODULE_PARM_DESC(enable_negotiate_signing,
> + "(deprecated) Enable negotiating packet signing algorithm with the server. "
> + "This parameter is ignored as cifs.ko will always try to negotiate the signing "
> + "algorithm on SMB 3.1.1 mounts.");
>
> module_param(disable_legacy_dialects, bool, 0644);
> MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be "
> @@ -1712,6 +1716,10 @@ init_cifs(void)
> goto out_init_cifs_idmap;
> }
>
> + if (!enable_negotiate_signing)
> + pr_warn_once("ignoring deprecated module parameter 'enable_negotiate_signing=0', "
> + "will try to negotiate signing capabilities anyway...\n");
> +
> return 0;
>
> out_init_cifs_idmap:
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 81a8eff06467..cadde6c451e5 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -2031,7 +2031,8 @@ 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 enable_negotiate_signing; /* request use of faster (GMAC) signing if available
> + XXX: deprecated, remove it at some point */
> 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 2c2bf28382bc..6c1d58492b18 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -609,13 +609,10 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
> neg_context_count++;
> }
>
> - if (enable_negotiate_signing) {
> - ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *)
> - pneg_ctxt);
> - *total_len += ctxt_len;
> - pneg_ctxt += ctxt_len;
> - neg_context_count++;
> - }
> + ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *)pneg_ctxt);
> + *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);
> --
> 2.35.3
>
On 09/29, Steve French wrote:
>could you fix these minor checkpatch warnings?
Ugh, I ran checkpatch but forgot to fix things, sorry about that. Will
resubmit.
Cheers,
Enzo
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 8042d7280dec..c46dc9edf6ec 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -65,7 +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 */ +bool enable_negotiate_signing = true; /* deprecated -- always true now */ unsigned int global_secflags = CIFSSEC_DEF; /* unsigned int ntlmv2_support = 0; */ unsigned int sign_CIFS_PDUs = 1; @@ -133,8 +133,12 @@ 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_negotiate_signing, "Enable negotiating packet signing algorithm with server. Default: n/N/0"); +/* XXX: remove this at some point */ +module_param(enable_negotiate_signing, bool, 0); +MODULE_PARM_DESC(enable_negotiate_signing, + "(deprecated) Enable negotiating packet signing algorithm with the server. " + "This parameter is ignored as cifs.ko will always try to negotiate the signing " + "algorithm on SMB 3.1.1 mounts."); module_param(disable_legacy_dialects, bool, 0644); MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be " @@ -1712,6 +1716,10 @@ init_cifs(void) goto out_init_cifs_idmap; } + if (!enable_negotiate_signing) + pr_warn_once("ignoring deprecated module parameter 'enable_negotiate_signing=0', " + "will try to negotiate signing capabilities anyway...\n"); + return 0; out_init_cifs_idmap: diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 81a8eff06467..cadde6c451e5 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -2031,7 +2031,8 @@ 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 enable_negotiate_signing; /* request use of faster (GMAC) signing if available + XXX: deprecated, remove it at some point */ 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 2c2bf28382bc..6c1d58492b18 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -609,13 +609,10 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, neg_context_count++; } - if (enable_negotiate_signing) { - ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *) - pneg_ctxt); - *total_len += ctxt_len; - pneg_ctxt += ctxt_len; - neg_context_count++; - } + ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *)pneg_ctxt); + *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);
Deprecate enable_negotiate_signing module parameter as it's irrelevant since the requested dialect and server support will dictate which algorithm will actually be used. Send a negotiate signing context on every SMB 3.1.1 negotiation now. AES-CMAC will still be used instead, iff, SMB 3.0.x negotiated or SMB 3.1.1 negotiated, but server doesn't support AES-GMAC. Warn the user if, for whatever reason, the module was loaded with 'enable_negotiate_signing=0'. Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de> --- fs/cifs/cifsfs.c | 14 +++++++++++--- fs/cifs/cifsglob.h | 3 ++- fs/cifs/smb2pdu.c | 11 ++++------- 3 files changed, 17 insertions(+), 11 deletions(-)