* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.