[PATCH] smb encrypt - new value desired

[Michael Adam]

On 2015-07-01 at 18:22 +0200, Michael Adam wrote:
> On 2015-07-01 at 16:30 +0200, Michael Adam wrote:
> > On 2015-06-30 at 21:45 +0200, Michael Adam wrote:
> > > On 2015-06-30 at 18:07 +0200, Stefan (metze) Metzmacher wrote:
> > > > Am 30.06.2015 um 17:58 schrieb Michael Adam:
> > > > >  
> > > > > -	if ((lp_smb_encrypt(-1) > SMB_SIGNING_OFF) &&
> > > > > +	if ((lp_smb_encrypt(-1) >= SMB_SIGNING_DESIRED) &&
> > > > >  	    (xconn->smb2.client.capabilities & SMB2_CAP_ENCRYPTION)) {
> > > > >  		x->global->encryption_required = true;
> > > > >  	}
> > > > > diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
> > > > > index eb66ea0..0e3f580 100644
> > > > > --- a/source3/smbd/smb2_tcon.c
> > > > > +++ b/source3/smbd/smb2_tcon.c
> > > > > @@ -266,7 +266,7 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
> > > > >  		return NT_STATUS_BAD_NETWORK_NAME;
> > > > >  	}
> > > > >  
> > > > > -	if ((lp_smb_encrypt(snum) > SMB_SIGNING_OFF) &&
> > > > > +	if ((lp_smb_encrypt(snum) >= SMB_SIGNING_DESIRED) &&
> > > > >  	    (conn->smb2.client.capabilities & SMB2_CAP_ENCRYPTION)) {
> > > > >  		encryption_required = true;
> > > > >  	}
> > > > 
> > > > We should not set 'encryption_required', smbd_smb2_request_dispatch()
> > > > would then reject non encrypted requests.
> > > 
> > > Practice shows that this works, though:
> > > smbclient -mSMB2 against a samba configured with DESIRED
> > > works without problems.
> > > 
> > > Setting encryption_required is also not added in this
> > > patch, I just raise the bar. And it had worked before... :-)
> > > 
> > > Also note that we only set encryption_required = true if
> > > - SMB_SIGNING_DESIRED is set and
> > > - the client supports encryption
> > > ==> This is precisely what we want to achieve. We want to enforce
> > >     encryption for those clients who support it.
> > > 
> > > > We should add a new bool encryption_requested, which just enounce
> > > > the flags to the client.
> > > 
> > > Maybe we can add it as a code improvement for master?
> > > I think this minimal patch would work well for 4.2 and 4.1.
> > 
> > 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.

Michael

-------------- 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/20150701/9ba798f4/attachment.pgp>