* Disable key exchange if ARC4 is not available @ 2021-08-18 4:10 Ronnie Sahlberg 2021-08-18 4:10 ` [PATCH] cifs: disable ntlmssp " Ronnie Sahlberg 2021-08-18 13:18 ` Disable " Tom Talpey 0 siblings, 2 replies; 8+ messages in thread From: Ronnie Sahlberg @ 2021-08-18 4:10 UTC (permalink / raw) To: linux-cifs; +Cc: Steve French Steve, We depend on ARC4 for generating the encrypted session key in key exchange. This patch disables the key exchange/encrypted session key for ntlmssp IF the kernel does not have any ARC4 support. This allows to build the cifs module even if ARC4 has been removed though with a weaker type of NTLMSSP support. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] cifs: disable ntlmssp key exchange if ARC4 is not available 2021-08-18 4:10 Disable key exchange if ARC4 is not available Ronnie Sahlberg @ 2021-08-18 4:10 ` Ronnie Sahlberg 2021-08-18 13:18 ` Disable " Tom Talpey 1 sibling, 0 replies; 8+ messages in thread From: Ronnie Sahlberg @ 2021-08-18 4:10 UTC (permalink / raw) To: linux-cifs; +Cc: Steve French This allows to build cifs.ko when ARC4 is not available. It comes with the drawback that key-exchange is no longer negotiated. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/cifsencrypt.c | 10 ++++++++++ fs/cifs/sess.c | 6 ++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index 7680e0a9bea3..a5cf604f1864 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -22,7 +22,9 @@ #include <linux/random.h> #include <linux/highmem.h> #include <linux/fips.h> +#ifdef CRYPTO_ARC4 #include <crypto/arc4.h> +#endif #include <crypto/aead.h> int __cifs_calc_signature(struct smb_rqst *rqst, @@ -682,6 +684,13 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) return rc; } +#ifndef CRYPTO_ARC4 +int +calc_seckey(struct cifs_ses *ses) +{ + return -ENODEV; +} +#else int calc_seckey(struct cifs_ses *ses) { @@ -712,6 +721,7 @@ calc_seckey(struct cifs_ses *ses) kfree_sensitive(ctx_arc4); return 0; } +#endif void cifs_crypto_secmech_release(struct TCP_Server_Info *server) diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 34a990e1ae44..a05ef87b0560 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -622,9 +622,10 @@ void build_ntlmssp_negotiate_blob(unsigned char *pbuffer, NTLMSSP_NEGOTIATE_SEAL; if (server->sign) flags |= NTLMSSP_NEGOTIATE_SIGN; +#ifdef CRYPTO_ARC4 if (!server->session_estab || ses->ntlmssp->sesskey_per_smbsess) flags |= NTLMSSP_NEGOTIATE_KEY_XCH; - +#endif sec_blob->NegotiateFlags = cpu_to_le32(flags); sec_blob->WorkstationName.BufferOffset = 0; @@ -690,9 +691,10 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer, NTLMSSP_NEGOTIATE_SEAL; if (ses->server->sign) flags |= NTLMSSP_NEGOTIATE_SIGN; +#ifdef CRYPTO_ARC4 if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess) flags |= NTLMSSP_NEGOTIATE_KEY_XCH; - +#endif tmp = *pbuffer + sizeof(AUTHENTICATE_MESSAGE); sec_blob->NegotiateFlags = cpu_to_le32(flags); -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Disable key exchange if ARC4 is not available 2021-08-18 4:10 Disable key exchange if ARC4 is not available Ronnie Sahlberg 2021-08-18 4:10 ` [PATCH] cifs: disable ntlmssp " Ronnie Sahlberg @ 2021-08-18 13:18 ` Tom Talpey 2021-08-18 16:27 ` ronnie sahlberg 2021-08-18 16:29 ` ronnie sahlberg 1 sibling, 2 replies; 8+ messages in thread From: Tom Talpey @ 2021-08-18 13:18 UTC (permalink / raw) To: Ronnie Sahlberg, linux-cifs; +Cc: Steve French On 8/18/2021 12:10 AM, Ronnie Sahlberg wrote: > Steve, > > We depend on ARC4 for generating the encrypted session key in key exchange. > This patch disables the key exchange/encrypted session key for ntlmssp > IF the kernel does not have any ARC4 support. > > This allows to build the cifs module even if ARC4 has been removed > though with a weaker type of NTLMSSP support. It's a good goal but it seems wrong to downgrade the security so silently. Wouldn't it be a better approach to select ARC4, and thereby force the build to succeed or fail? Alternatively, change the #ifndef ARC4 to a positive option named (for example) DOWNGRADED_NTLMSSP or something equally foreboding? Tom. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Disable key exchange if ARC4 is not available 2021-08-18 13:18 ` Disable " Tom Talpey @ 2021-08-18 16:27 ` ronnie sahlberg 2021-08-18 16:29 ` ronnie sahlberg 1 sibling, 0 replies; 8+ messages in thread From: ronnie sahlberg @ 2021-08-18 16:27 UTC (permalink / raw) To: Tom Talpey; +Cc: Ronnie Sahlberg, linux-cifs, Steve French On Wed, Aug 18, 2021 at 11:18 PM Tom Talpey <tom@talpey.com> wrote: > > On 8/18/2021 12:10 AM, Ronnie Sahlberg wrote: > > Steve, > > > > We depend on ARC4 for generating the encrypted session key in key exchange. > > This patch disables the key exchange/encrypted session key for ntlmssp > > IF the kernel does not have any ARC4 support. > > > > This allows to build the cifs module even if ARC4 has been removed > > though with a weaker type of NTLMSSP support. > > It's a good goal but it seems wrong to downgrade the security > so silently. Wouldn't it be a better approach to select ARC4, > and thereby force the build to succeed or fail? Alternatively, > change the #ifndef ARC4 to a positive option named (for example) > DOWNGRADED_NTLMSSP or something equally foreboding? > > Tom. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Disable key exchange if ARC4 is not available 2021-08-18 13:18 ` Disable " Tom Talpey 2021-08-18 16:27 ` ronnie sahlberg @ 2021-08-18 16:29 ` ronnie sahlberg 2021-08-18 16:51 ` Steve French 1 sibling, 1 reply; 8+ messages in thread From: ronnie sahlberg @ 2021-08-18 16:29 UTC (permalink / raw) To: Tom Talpey; +Cc: Ronnie Sahlberg, linux-cifs, Steve French On Wed, Aug 18, 2021 at 11:18 PM Tom Talpey <tom@talpey.com> wrote: > > On 8/18/2021 12:10 AM, Ronnie Sahlberg wrote: > > Steve, > > > > We depend on ARC4 for generating the encrypted session key in key exchange. > > This patch disables the key exchange/encrypted session key for ntlmssp > > IF the kernel does not have any ARC4 support. > > > > This allows to build the cifs module even if ARC4 has been removed > > though with a weaker type of NTLMSSP support. > > It's a good goal but it seems wrong to downgrade the security > so silently. Wouldn't it be a better approach to select ARC4, > and thereby force the build to succeed or fail? Alternatively, > change the #ifndef ARC4 to a positive option named (for example) > DOWNGRADED_NTLMSSP or something equally foreboding? Good point. Maybe we should drop this patch and instead copy ARC4 into fs/cifs so we have a private version of the code in cifs.ko. And do the same for md4 and md5. > > Tom. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Disable key exchange if ARC4 is not available 2021-08-18 16:29 ` ronnie sahlberg @ 2021-08-18 16:51 ` Steve French 2021-08-18 18:33 ` Tom Talpey 0 siblings, 1 reply; 8+ messages in thread From: Steve French @ 2021-08-18 16:51 UTC (permalink / raw) To: ronnie sahlberg; +Cc: Tom Talpey, Ronnie Sahlberg, linux-cifs On Wed, Aug 18, 2021 at 11:29 AM ronnie sahlberg <ronniesahlberg@gmail.com> wrote: > > On Wed, Aug 18, 2021 at 11:18 PM Tom Talpey <tom@talpey.com> wrote: > > > > On 8/18/2021 12:10 AM, Ronnie Sahlberg wrote: > > > Steve, > > > > > > We depend on ARC4 for generating the encrypted session key in key exchange. > > > This patch disables the key exchange/encrypted session key for ntlmssp > > > IF the kernel does not have any ARC4 support. > > > > > > This allows to build the cifs module even if ARC4 has been removed > > > though with a weaker type of NTLMSSP support. > > > > It's a good goal but it seems wrong to downgrade the security > > so silently. Wouldn't it be a better approach to select ARC4, > > and thereby force the build to succeed or fail? Alternatively, > > change the #ifndef ARC4 to a positive option named (for example) > > DOWNGRADED_NTLMSSP or something equally foreboding? > > Good point. > Maybe we should drop this patch and instead copy ARC4 into fs/cifs > so we have a private version of the code in cifs.ko. > And do the same for md4 and md5. Yes ... and allow a build option where ARC4/MD4 are removed from the build and NTLMSSP disabled, forcing kerberos in the short term, and then we need to get working ASAP on adding some choices in the future, perhaps something similar to https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852232(v=ws.11) where Windows allows plugging in additional auth mechanisms to SPNEGO (and pick at least one new mechanism beyond KRB5 to support in the kernel client ...) -- Thanks, Steve ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Disable key exchange if ARC4 is not available 2021-08-18 16:51 ` Steve French @ 2021-08-18 18:33 ` Tom Talpey 2021-08-18 21:04 ` ronnie sahlberg 0 siblings, 1 reply; 8+ messages in thread From: Tom Talpey @ 2021-08-18 18:33 UTC (permalink / raw) To: Steve French, ronnie sahlberg; +Cc: Ronnie Sahlberg, linux-cifs On 8/18/2021 12:51 PM, Steve French wrote: > On Wed, Aug 18, 2021 at 11:29 AM ronnie sahlberg > <ronniesahlberg@gmail.com> wrote: >> >> On Wed, Aug 18, 2021 at 11:18 PM Tom Talpey <tom@talpey.com> wrote: >>> >>> On 8/18/2021 12:10 AM, Ronnie Sahlberg wrote: >>>> Steve, >>>> >>>> We depend on ARC4 for generating the encrypted session key in key exchange. >>>> This patch disables the key exchange/encrypted session key for ntlmssp >>>> IF the kernel does not have any ARC4 support. >>>> >>>> This allows to build the cifs module even if ARC4 has been removed >>>> though with a weaker type of NTLMSSP support. >>> >>> It's a good goal but it seems wrong to downgrade the security >>> so silently. Wouldn't it be a better approach to select ARC4, >>> and thereby force the build to succeed or fail? Alternatively, >>> change the #ifndef ARC4 to a positive option named (for example) >>> DOWNGRADED_NTLMSSP or something equally foreboding? >> >> Good point. >> Maybe we should drop this patch and instead copy ARC4 into fs/cifs >> so we have a private version of the code in cifs.ko. >> And do the same for md4 and md5. Copying such code makes me uneasy. It's going to confuse everyone who tries to turn off one and misses the other. To say nothing of the risk of testing and addressing bugs. BTW, are we sure that servers even work if the client selects something other than ARC4, or whatever? Tom. > Yes ... and allow a build option where ARC4/MD4 are removed from the > build and NTLMSSP disabled, > forcing kerberos in the short term, and then we need to get working > ASAP on adding some choices in the future, > perhaps something similar to > > https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852232(v=ws.11) > > where Windows allows plugging in additional auth mechanisms to SPNEGO > (and pick at least one new mechanism beyond > KRB5 to support in the kernel client ...) > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Disable key exchange if ARC4 is not available 2021-08-18 18:33 ` Tom Talpey @ 2021-08-18 21:04 ` ronnie sahlberg 0 siblings, 0 replies; 8+ messages in thread From: ronnie sahlberg @ 2021-08-18 21:04 UTC (permalink / raw) To: Tom Talpey; +Cc: Steve French, Ronnie Sahlberg, linux-cifs On Thu, Aug 19, 2021 at 4:33 AM Tom Talpey <tom@talpey.com> wrote: > > On 8/18/2021 12:51 PM, Steve French wrote: > > On Wed, Aug 18, 2021 at 11:29 AM ronnie sahlberg > > <ronniesahlberg@gmail.com> wrote: > >> > >> On Wed, Aug 18, 2021 at 11:18 PM Tom Talpey <tom@talpey.com> wrote: > >>> > >>> On 8/18/2021 12:10 AM, Ronnie Sahlberg wrote: > >>>> Steve, > >>>> > >>>> We depend on ARC4 for generating the encrypted session key in key exchange. > >>>> This patch disables the key exchange/encrypted session key for ntlmssp > >>>> IF the kernel does not have any ARC4 support. > >>>> > >>>> This allows to build the cifs module even if ARC4 has been removed > >>>> though with a weaker type of NTLMSSP support. > >>> > >>> It's a good goal but it seems wrong to downgrade the security > >>> so silently. Wouldn't it be a better approach to select ARC4, > >>> and thereby force the build to succeed or fail? Alternatively, > >>> change the #ifndef ARC4 to a positive option named (for example) > >>> DOWNGRADED_NTLMSSP or something equally foreboding? > >> > >> Good point. > >> Maybe we should drop this patch and instead copy ARC4 into fs/cifs > >> so we have a private version of the code in cifs.ko. > >> And do the same for md4 and md5. > > Copying such code makes me uneasy. It's going to confuse everyone > who tries to turn off one and misses the other. To say nothing of > the risk of testing and addressing bugs. > > BTW, are we sure that servers even work if the client selects > something other than ARC4, or whatever? > Removing ARC4 does work, at least against windows servers. But it comes at the cost that we can not do key exchange, which weakens the authentication. Thinking about it, removing ARC4 because people want to make the kernel "more secure" would come at a cost of making cifs "less secure" :-( The same situation, removal, exist for MD4 which is a core part of NTLMSSP and can not be disabled without full removal of NTLMSSP in its entirety. For that case it was suggested that we "fork the md4 code and move it into fs/cifs". I don't think it is viable to remove NTLMSSP from the cifs module as this would in reality break cifs for most users so the only viable option might be that we have to create a private fork of this. And if we are already going to fork md4 and copy it into fs/cifs we can just as well do the same for ARC4. There are other modules depending on md4 too that can not disable it without breaking all the users that need it so I guess they will have to do the same. I imagine that the kernel will then end up with no single common md4 hash in its cryptographic library but several identical private copies of the md4 code in different modules. lol -- ronnie > T > > > Yes ... and allow a build option where ARC4/MD4 are removed from the > > build and NTLMSSP disabled, > > forcing kerberos in the short term, and then we need to get working > > ASAP on adding some choices in the future, > > perhaps something similar to > > > > https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852232(v=ws.11) > > > > where Windows allows plugging in additional auth mechanisms to SPNEGO > > (and pick at least one new mechanism beyond > > KRB5 to support in the kernel client ...) > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-18 21:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-18 4:10 Disable key exchange if ARC4 is not available Ronnie Sahlberg 2021-08-18 4:10 ` [PATCH] cifs: disable ntlmssp " Ronnie Sahlberg 2021-08-18 13:18 ` Disable " Tom Talpey 2021-08-18 16:27 ` ronnie sahlberg 2021-08-18 16:29 ` ronnie sahlberg 2021-08-18 16:51 ` Steve French 2021-08-18 18:33 ` Tom Talpey 2021-08-18 21:04 ` ronnie sahlberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).