* [PATCH] CIFS: Remove encryption from the capabilities flags @ 2017-02-27 23:20 Pavel Shilovsky 2017-02-27 23:32 ` Steve French 0 siblings, 1 reply; 6+ messages in thread From: Pavel Shilovsky @ 2017-02-27 23:20 UTC (permalink / raw) To: stable; +Cc: Steve French Currently the client claims to support encryption but it doesn't. This results in rejecting TreeConnect requests by a server that expects them to be encrypted. Fix it by removing encryption from the capabilities flags. This problem affects kernels from v4.5 to v4.10. Cc: Steve French <smfrench@gmail.com> Cc: Stable <stable@vger.kernel.org> # v4.5-v4.10 Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> --- fs/cifs/smb2ops.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 5d456eb..87a58f0 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -1925,7 +1925,9 @@ struct smb_version_values smb21_values = { struct smb_version_values smb30_values = { .version_string = SMB30_VERSION_STRING, .protocol_id = SMB30_PROT_ID, - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING + | SMB2_GLOBAL_CAP_LARGE_MTU + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, .large_lock_type = 0, .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, @@ -1945,7 +1947,9 @@ struct smb_version_values smb30_values = { struct smb_version_values smb302_values = { .version_string = SMB302_VERSION_STRING, .protocol_id = SMB302_PROT_ID, - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING + | SMB2_GLOBAL_CAP_LARGE_MTU + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, .large_lock_type = 0, .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] CIFS: Remove encryption from the capabilities flags 2017-02-27 23:20 [PATCH] CIFS: Remove encryption from the capabilities flags Pavel Shilovsky @ 2017-02-27 23:32 ` Steve French 2017-04-10 21:29 ` Pavel Shilovskiy 0 siblings, 1 reply; 6+ messages in thread From: Steve French @ 2017-02-27 23:32 UTC (permalink / raw) To: Pavel Shilovsky; +Cc: Stable lthough may be too large - note that an alternative is to backport current mainline's SMB3 encryption patch series (from Pavel) to stable (which was recently merged). It is a very nice security feature - but larger than usual for a backport. On Mon, Feb 27, 2017 at 5:20 PM, Pavel Shilovsky <pshilov@microsoft.com> wrote: > Currently the client claims to support encryption but it doesn't. > This results in rejecting TreeConnect requests by a server that expects > them to be encrypted. Fix it by removing encryption from the capabilities > flags. This problem affects kernels from v4.5 to v4.10. > > Cc: Steve French <smfrench@gmail.com> > Cc: Stable <stable@vger.kernel.org> # v4.5-v4.10 > Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> > --- > fs/cifs/smb2ops.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 5d456eb..87a58f0 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -1925,7 +1925,9 @@ struct smb_version_values smb21_values = { > struct smb_version_values smb30_values = { > .version_string = SMB30_VERSION_STRING, > .protocol_id = SMB30_PROT_ID, > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING > + | SMB2_GLOBAL_CAP_LARGE_MTU > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, > .large_lock_type = 0, > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > @@ -1945,7 +1947,9 @@ struct smb_version_values smb30_values = { > struct smb_version_values smb302_values = { > .version_string = SMB302_VERSION_STRING, > .protocol_id = SMB302_PROT_ID, > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING > + | SMB2_GLOBAL_CAP_LARGE_MTU > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, > .large_lock_type = 0, > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > -- > 2.7.4 > -- Thanks, Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] CIFS: Remove encryption from the capabilities flags 2017-02-27 23:32 ` Steve French @ 2017-04-10 21:29 ` Pavel Shilovskiy 2017-04-11 3:41 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Pavel Shilovskiy @ 2017-04-10 21:29 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Stable, Steve French On Mon, Feb 27, 2017 at 5:20 PM, Pavel Shilovsky <pshilov@microsoft.com> wrote: > Currently the client claims to support encryption but it doesn't. > This results in rejecting TreeConnect requests by a server that > expects them to be encrypted. Fix it by removing encryption from the > capabilities flags. This problem affects kernels from v4.5 to v4.10. > > Cc: Steve French <smfrench@gmail.com> > Cc: Stable <stable@vger.kernel.org> # v4.5-v4.10 > Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> > --- > fs/cifs/smb2ops.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index > 5d456eb..87a58f0 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -1925,7 +1925,9 @@ struct smb_version_values smb21_values = { > struct smb_version_values smb30_values = { > .version_string = SMB30_VERSION_STRING, > .protocol_id = SMB30_PROT_ID, > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING > + | SMB2_GLOBAL_CAP_LARGE_MTU > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, > .large_lock_type = 0, > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, @@ -1945,7 > +1947,9 @@ struct smb_version_values smb30_values = { struct > smb_version_values smb302_values = { > .version_string = SMB302_VERSION_STRING, > .protocol_id = SMB302_PROT_ID, > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING > + | SMB2_GLOBAL_CAP_LARGE_MTU > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, > .large_lock_type = 0, > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > -- > 2.7.4 > Do you have any update on this patch? It seems suitable at least for 4.9 and 4.10 kernels. Best regards, Pavel Shilovskiy ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] CIFS: Remove encryption from the capabilities flags 2017-04-10 21:29 ` Pavel Shilovskiy @ 2017-04-11 3:41 ` Greg Kroah-Hartman 2017-04-11 4:04 ` Steve French 0 siblings, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2017-04-11 3:41 UTC (permalink / raw) To: Pavel Shilovskiy; +Cc: Stable, Steve French On Mon, Apr 10, 2017 at 09:29:41PM +0000, Pavel Shilovskiy wrote: > On Mon, Feb 27, 2017 at 5:20 PM, Pavel Shilovsky <pshilov@microsoft.com> wrote: > > Currently the client claims to support encryption but it doesn't. > > This results in rejecting TreeConnect requests by a server that > > expects them to be encrypted. Fix it by removing encryption from the > > capabilities flags. This problem affects kernels from v4.5 to v4.10. > > > > Cc: Steve French <smfrench@gmail.com> > > Cc: Stable <stable@vger.kernel.org> # v4.5-v4.10 > > Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> > > --- > > fs/cifs/smb2ops.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index > > 5d456eb..87a58f0 100644 > > --- a/fs/cifs/smb2ops.c > > +++ b/fs/cifs/smb2ops.c > > @@ -1925,7 +1925,9 @@ struct smb_version_values smb21_values = { > > struct smb_version_values smb30_values = { > > .version_string = SMB30_VERSION_STRING, > > .protocol_id = SMB30_PROT_ID, > > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, > > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING > > + | SMB2_GLOBAL_CAP_LARGE_MTU > > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, > > .large_lock_type = 0, > > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, @@ -1945,7 > > +1947,9 @@ struct smb_version_values smb30_values = { struct > > smb_version_values smb302_values = { > > .version_string = SMB302_VERSION_STRING, > > .protocol_id = SMB302_PROT_ID, > > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, > > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING > > + | SMB2_GLOBAL_CAP_LARGE_MTU > > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, > > .large_lock_type = 0, > > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, > > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, > > -- > > 2.7.4 > > > > Do you have any update on this patch? It seems suitable at least for 4.9 and 4.10 kernels. What was the commit id for this in Linus's tree? Did you read Documentation/stable_kernel_rules.txt for how to get something merged into a stable kernel? If this isn't in Linus's tree, I need a whole lot of text explaining that, and some signed-off-by from the maintainers. None of which happened here, which is why it is long gone from my patch queue. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] CIFS: Remove encryption from the capabilities flags 2017-04-11 3:41 ` Greg Kroah-Hartman @ 2017-04-11 4:04 ` Steve French 2017-04-11 4:15 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Steve French @ 2017-04-11 4:04 UTC (permalink / raw) To: Greg Kroah-Hartman, Sachin Prabhu; +Cc: Pavel Shilovskiy, Stable Pavel's point is a good one, but we need some feedback from those more familiar with the stable rules to decide if his small patch or a backport of a larger (but more valuable patch series) is the right answer. Basically, 1) A VERY important security feature (SMB3 encryption) was added (by Pavel) to the cifs.ko SMB3 client and is now upstream in mainline. Support for encrypted sessions and shares is crucial in many cases and is required for some servers now, but the patch series which implemented it is rather large for backport to 4.9 (15 patches). It is of huge value, and improves security in some cases obviously, but I don't know if his series is larger than allowed for stable. 2) In the process of writing support for encryption - he noticed that when you don't have his encryption patch series, we had a bug in older clients (which his patch addresses). His patch is unneeded for current mainline. If we backport the encryption series, we wouldn't need it. On Mon, Apr 10, 2017 at 10:41 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Mon, Apr 10, 2017 at 09:29:41PM +0000, Pavel Shilovskiy wrote: >> On Mon, Feb 27, 2017 at 5:20 PM, Pavel Shilovsky <pshilov@microsoft.com> wrote: >> > Currently the client claims to support encryption but it doesn't. >> > This results in rejecting TreeConnect requests by a server that >> > expects them to be encrypted. Fix it by removing encryption from the >> > capabilities flags. This problem affects kernels from v4.5 to v4.10. >> > >> > Cc: Steve French <smfrench@gmail.com> >> > Cc: Stable <stable@vger.kernel.org> # v4.5-v4.10 >> > Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> >> > --- >> > fs/cifs/smb2ops.c | 8 ++++++-- >> > 1 file changed, 6 insertions(+), 2 deletions(-) >> > >> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index >> > 5d456eb..87a58f0 100644 >> > --- a/fs/cifs/smb2ops.c >> > +++ b/fs/cifs/smb2ops.c >> > @@ -1925,7 +1925,9 @@ struct smb_version_values smb21_values = { >> > struct smb_version_values smb30_values = { >> > .version_string = SMB30_VERSION_STRING, >> > .protocol_id = SMB30_PROT_ID, >> > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, >> > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING >> > + | SMB2_GLOBAL_CAP_LARGE_MTU >> > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, >> > .large_lock_type = 0, >> > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, >> > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, @@ -1945,7 >> > +1947,9 @@ struct smb_version_values smb30_values = { struct >> > smb_version_values smb302_values = { >> > .version_string = SMB302_VERSION_STRING, >> > .protocol_id = SMB302_PROT_ID, >> > - .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING | SMB2_GLOBAL_CAP_LARGE_MTU | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES | SMB2_GLOBAL_CAP_ENCRYPTION, >> > + .req_capabilities = SMB2_GLOBAL_CAP_DFS | SMB2_GLOBAL_CAP_LEASING >> > + | SMB2_GLOBAL_CAP_LARGE_MTU >> > + | SMB2_GLOBAL_CAP_PERSISTENT_HANDLES, >> > .large_lock_type = 0, >> > .exclusive_lock_type = SMB2_LOCKFLAG_EXCLUSIVE_LOCK, >> > .shared_lock_type = SMB2_LOCKFLAG_SHARED_LOCK, >> > -- >> > 2.7.4 >> > >> >> Do you have any update on this patch? It seems suitable at least for 4.9 and 4.10 kernels. > > What was the commit id for this in Linus's tree? Did you read > Documentation/stable_kernel_rules.txt for how to get something merged > into a stable kernel? > > If this isn't in Linus's tree, I need a whole lot of text explaining > that, and some signed-off-by from the maintainers. None of which > happened here, which is why it is long gone from my patch queue. > > thanks, > > greg k-h -- Thanks, Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] CIFS: Remove encryption from the capabilities flags 2017-04-11 4:04 ` Steve French @ 2017-04-11 4:15 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2017-04-11 4:15 UTC (permalink / raw) To: Steve French; +Cc: Sachin Prabhu, Pavel Shilovskiy, Stable On Mon, Apr 10, 2017 at 11:04:04PM -0500, Steve French wrote: > Pavel's point is a good one, but we need some feedback from those more > familiar with the stable rules to decide if his small patch or a > backport of a larger (but more valuable patch series) is the right > answer. > > Basically, > 1) A VERY important security feature (SMB3 encryption) was added (by > Pavel) to the cifs.ko SMB3 client and is now upstream in mainline. > Support for encrypted sessions and shares is crucial in many cases and > is required for some servers now, but the patch series which > implemented it is rather large for backport to 4.9 (15 patches). It > is of huge value, and improves security in some cases obviously, but I > don't know if his series is larger than allowed for stable. If it is "huge value", and improves security, and is what is in Linus's tree, then yes, it could be allowed for stable, that would be nice. Feel free to send the patch series if you want them applied. > 2) In the process of writing support for encryption - he noticed that > when you don't have his encryption patch series, we had a bug in older > clients (which his patch addresses). His patch is unneeded for > current mainline. If we backport the encryption series, we wouldn't > need it. I always prefer to stick with what is in Linus's tree because we almost always get something wrong when we don't do that. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-11 4:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-27 23:20 [PATCH] CIFS: Remove encryption from the capabilities flags Pavel Shilovsky 2017-02-27 23:32 ` Steve French 2017-04-10 21:29 ` Pavel Shilovskiy 2017-04-11 3:41 ` Greg Kroah-Hartman 2017-04-11 4:04 ` Steve French 2017-04-11 4:15 ` Greg Kroah-Hartman
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).