[PATCH] smb encrypt - new value desired

[Michael Adam]

Thanks!

The patches have just landed.

I found one more case of SMB_SIGNING_DESIRED
not treated in a case.

Patch to fix this is attached.

The reason was that the type of encrypt_level
in the connection_struct is just an int, not
the appropriate enum.

The second attached patch changes this to the enum.
But I am not sure about that: Does this entail a
version bump of our vfs?

Comments/review/push appreciated!

Cheers - Michael

On 2015-07-07 at 13:01 +0200, Guenther Deschner wrote:
> Hi Michael,
> 
> LGTM. RB+ and pushed to autobuild.
> 
> Thanks,
> Guenther
> 
> On 07/07/15 01:04, Michael Adam wrote:
> > On 2015-07-02 at 15:32 +0200, Michael Adam wrote:
> >> On 2015-07-01 at 23:29 +0200, Michael Adam wrote:
> >>> On 2015-07-01 at 18:22 +0200, Michael Adam wrote:
> >>>> On 2015-07-01 at 16:30 +0200, Michael Adam wrote:
> >>>>>
> >>>>> Update:
> >>>>>
> >>>>> The difference in behaviour is in treating a 'disobedient'
> >>>>> client that does not send encrypted requests although we
> >>>>> (the server) send ENCRYPT_DATA in tree connect or session
> >>>>> setup response.
> >>>>>
> >>>>> I just tested against windows.
> >>>>> Windows is generous in that it permits unencrypted request
> >>>>> packets, but sends encrypted responses.
> >>>>>
> >>>>> With the proposed patch we would be less generous and
> >>>>> deny unecrypted requests after having sent ENCRYPT_DATA.
> >>>>>
> >>>>> With Metze's proposed change, we would accept unencrypted
> >>>>> requests but without further changes send unencrypted
> >>>>> responses to those.
> >>>>>
> >>>>> I'll see what I can do regarding this last approach to
> >>>>> match windows behaviour more exactly...
> >>>>
> >>>> Attached find an updated patchset that implements the
> >>>> exact windows behaviour described above.
> >>>> It is not sooo big after all. Maybe we can take and
> >>>> backport it.
> >>>>
> >>>> Feedback/Review welcome!
> >>>
> >>> Oh, apparently it is not complete yet. :-/
> >>> Some tests fail with this patchset.
> >>
> >> Attached is the new version of this patchset.
> >> It now survives all smb2 related tests.
> >> I am currently running a full autobuild for verification.
> >>
> >> The only issue that needs resolution is the
> >> addition of encryption desired to
> >> smbXsrv_session->global and smbXsrv_tcon->global.
> >> Currently I have inserted them in the logically
> >> best place (imho), but with respect to alignment
> >> and structure size we may need another solution.
> >>
> >> Apart from this, I think the patchset should be good.
> > 
> > Attached is the (hopefully final) updated patchset.
> > It fixes the abovementioned issue by putting the
> > encryption_desired variable not into smbXsrv_session|tcon->global
> > but into smbXsrv_session|tcon directly so that it
> > does not get marshalled and put to disk.
> > 
> > Review/comments welcome.
> > 
> > Michael
> > 
> 
> 
> -- 
> Günther Deschner                    GPG-ID: 8EE11688
> Red Hat                         gdeschner at redhat.com
> Samba Team                              gd at samba.org
-------------- next part --------------
From b9b596c9d64b75c096e73dff6ba9fd2c07f472b5 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 7 Jul 2015 17:15:00 +0200
Subject: [PATCH 1/2] smbd:trans2: treat new SMB_SIGNING_DESIRED in case

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/trans2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 88c69a9..8816402 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -3639,6 +3639,7 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)bsize, (unsigned
 			case SMB_SIGNING_OFF:
 				encrypt_caps = 0;
 				break;
+			case SMB_SIGNING_DESIRED:
 			case SMB_SIGNING_IF_REQUIRED:
 			case SMB_SIGNING_DEFAULT:
 				encrypt_caps = CIFS_UNIX_TRANSPORT_ENCRYPTION_CAP;
-- 
2.4.3


From ab1d598b2ba0dd0701bcaf84583c154159b3830b Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 7 Jul 2015 17:18:12 +0200
Subject: [PATCH 2/2] vfs: fix type of conn->encrypt_level to the correct enum

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/include/vfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 081030c..a5472a3 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -365,7 +365,7 @@ typedef struct connection_struct {
 	time_t lastused_count;
 	int num_files_open;
 	unsigned int num_smb_operations; /* Count of smb operations on this tree. */
-	int encrypt_level;
+	enum smb_signing_setting encrypt_level;
 	bool encrypted_tid;
 
 	/* Semantics requested by the client or forced by the server config. */
-- 
2.4.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150707/bb5fcbbd/attachment.pgp>