[PATCH] smb encrypt - new value desired
Michael Adam
obnox at samba.org
Tue Jul 7 01:04:31 CEST 2015
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
-------------- next part --------------
From a30a72fd370874085338e8cae78cd3b287edbaab Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 30 Jun 2015 14:16:19 +0200
Subject: [PATCH 1/7] Introduce setting "desired" for 'smb encrypt' and
'client/server signing'
This should trigger the behaviour where the server requires
signing when the client supports it, but does not reject
clients that don't support it.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
lib/param/loadparm.c | 1 +
lib/param/param_table.c | 1 +
libcli/smb/smbXcli_base.c | 6 ++++++
libcli/smb/smb_constants.h | 1 +
source4/smb_server/smb2/negprot.c | 1 +
5 files changed, 10 insertions(+)
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index bb215b2..0e11428 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -3207,6 +3207,7 @@ bool lpcfg_server_signing_allowed(struct loadparm_context *lp_ctx, bool *mandato
case SMB_SIGNING_REQUIRED:
*mandatory = true;
break;
+ case SMB_SIGNING_DESIRED:
case SMB_SIGNING_IF_REQUIRED:
break;
case SMB_SIGNING_DEFAULT:
diff --git a/lib/param/param_table.c b/lib/param/param_table.c
index 287839f..ff31038 100644
--- a/lib/param/param_table.c
+++ b/lib/param/param_table.c
@@ -115,6 +115,7 @@ static const struct enum_list enum_smb_signing_vals[] = {
{SMB_SIGNING_IF_REQUIRED, "On"},
{SMB_SIGNING_IF_REQUIRED, "enabled"},
{SMB_SIGNING_IF_REQUIRED, "auto"},
+ {SMB_SIGNING_DESIRED, "desired"},
{SMB_SIGNING_REQUIRED, "required"},
{SMB_SIGNING_REQUIRED, "mandatory"},
{SMB_SIGNING_REQUIRED, "force"},
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index c8ae5b0..6c35430 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -376,6 +376,12 @@ struct smbXcli_conn *smbXcli_conn_create(TALLOC_CTX *mem_ctx,
conn->desire_signing = false;
conn->mandatory_signing = false;
break;
+ case SMB_SIGNING_DESIRED:
+ /* if the server desires it */
+ conn->allow_signing = true;
+ conn->desire_signing = true;
+ conn->mandatory_signing = false;
+ break;
case SMB_SIGNING_REQUIRED:
/* always */
conn->allow_signing = true;
diff --git a/libcli/smb/smb_constants.h b/libcli/smb/smb_constants.h
index 589b1a63..c4cca15 100644
--- a/libcli/smb/smb_constants.h
+++ b/libcli/smb/smb_constants.h
@@ -98,6 +98,7 @@ enum smb_signing_setting {
SMB_SIGNING_DEFAULT = -1,
SMB_SIGNING_OFF = 0,
SMB_SIGNING_IF_REQUIRED = 1,
+ SMB_SIGNING_DESIRED = 2,
SMB_SIGNING_REQUIRED = 3,
};
diff --git a/source4/smb_server/smb2/negprot.c b/source4/smb_server/smb2/negprot.c
index 81f2547..b48b170 100644
--- a/source4/smb_server/smb2/negprot.c
+++ b/source4/smb_server/smb2/negprot.c
@@ -150,6 +150,7 @@ static NTSTATUS smb2srv_negprot_backend(struct smb2srv_request *req, struct smb2
case SMB_SIGNING_OFF:
io->out.security_mode = 0;
break;
+ case SMB_SIGNING_DESIRED:
case SMB_SIGNING_IF_REQUIRED:
io->out.security_mode = SMB2_NEGOTIATE_SIGNING_ENABLED;
break;
--
2.4.3
From 9114e0f76f81c09df2c63d5e39158d032e36bab2 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 1 Jul 2015 17:34:45 +0200
Subject: [PATCH 2/7] smbXsrv: add bools encryption_desired to session and tcon
This is to indicate that we should sen the ENCRYPT_DATA
flag on session or tcon replies.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/librpc/idl/smbXsrv.idl | 2 ++
1 file changed, 2 insertions(+)
diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl
index 4367d72..77959ce 100644
--- a/source3/librpc/idl/smbXsrv.idl
+++ b/source3/librpc/idl/smbXsrv.idl
@@ -193,6 +193,7 @@ interface smbXsrv
[ignore] user_struct *compat;
[ignore] smbXsrv_tcon_table *tcon_table;
[ignore] smbXsrv_preauth *preauth;
+ boolean8 encryption_desired;
} smbXsrv_session;
typedef union {
@@ -287,6 +288,7 @@ interface smbXsrv
NTSTATUS status;
NTTIME idle_time;
[ignore] connection_struct *compat;
+ boolean8 encryption_desired;
} smbXsrv_tcon;
typedef union {
--
2.4.3
From 32a3f3587a6a71e473c564dcea2096f40f1785ee Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 1 Jul 2015 17:42:58 +0200
Subject: [PATCH 3/7] smbd:smb2: separate between encryption required and enc
desired
this means we:
- accept unencrypted requests if encryption only desired
and not required,
- but we always send encrypted responses in the desired
case, not only when the request was encrypted.
For this purpose, the do_encryption in the request
structure is separated into was_encrypted and do_encryption.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/smbd/globals.h | 3 +++
source3/smbd/smb2_server.c | 18 ++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 3ddafaf..2ca23aa 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -654,6 +654,9 @@ struct smbd_smb2_request {
int current_idx;
bool do_signing;
+ /* Was the request encrypted? */
+ bool was_encrypted;
+ /* Should we encrypt? */
bool do_encryption;
struct tevent_timer *async_te;
bool compound_related;
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index a8d54cb..97e99a0 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2000,6 +2000,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
NTSTATUS return_value;
struct smbXsrv_session *x = NULL;
bool signing_required = false;
+ bool encryption_desired = false;
bool encryption_required = false;
inhdr = SMBD_SMB2_IN_HDR_PTR(req);
@@ -2047,11 +2048,13 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
x = req->session;
if (x != NULL) {
signing_required = x->global->signing_required;
+ encryption_desired = x->encryption_desired;
encryption_required = x->global->encryption_required;
}
req->do_signing = false;
req->do_encryption = false;
+ req->was_encrypted = false;
if (intf_v->iov_len == SMB2_TF_HDR_SIZE) {
const uint8_t *intf = SMBD_SMB2_IN_TF_PTR(req);
uint64_t tf_session_id = BVAL(intf, SMB2_TF_SESSION_ID);
@@ -2073,10 +2076,10 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
NT_STATUS_ACCESS_DENIED);
}
- req->do_encryption = true;
+ req->was_encrypted = true;
}
- if (encryption_required && !req->do_encryption) {
+ if (encryption_required && !req->was_encrypted) {
return smbd_smb2_request_error(req,
NT_STATUS_ACCESS_DENIED);
}
@@ -2116,7 +2119,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
req->compat_chain_fsp = NULL;
}
- if (req->do_encryption) {
+ if (req->was_encrypted) {
signing_required = false;
} else if (signing_required || (flags & SMB2_HDR_FLAG_SIGNED)) {
DATA_BLOB signing_key = data_blob_null;
@@ -2202,15 +2205,22 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
if (!NT_STATUS_IS_OK(status)) {
return smbd_smb2_request_error(req, status);
}
+ if (req->tcon->encryption_desired) {
+ encryption_desired = true;
+ }
if (req->tcon->global->encryption_required) {
encryption_required = true;
}
- if (encryption_required && !req->do_encryption) {
+ if (encryption_required && !req->was_encrypted) {
return smbd_smb2_request_error(req,
NT_STATUS_ACCESS_DENIED);
}
}
+ if (req->was_encrypted || encryption_desired) {
+ req->do_encryption = true;
+ }
+
if (call->fileid_ofs != 0) {
size_t needed = call->fileid_ofs + 16;
const uint8_t *body = SMBD_SMB2_IN_BODY_PTR(req);
--
2.4.3
From be7ecd797dfb9ce605c56805dbf44c53575c2f08 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 1 Jul 2015 18:07:26 +0200
Subject: [PATCH 4/7] smbd:smb2: only enable encryption in session if desired
Don't enforce it but only announce ENCRYPT_DATA, using the
encryption_desired flag in session setup.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/smbd/smb2_sesssetup.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index da7adb3..11d381f 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -262,12 +262,13 @@ static NTSTATUS smbd_smb2_auth_generic_return(struct smbXsrv_session *session,
x->global->signing_required = true;
}
- 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;
+ x->encryption_desired = true;
}
if (lp_smb_encrypt(-1) == SMB_SIGNING_REQUIRED) {
+ x->encryption_desired = true;
x->global->encryption_required = true;
}
@@ -294,7 +295,7 @@ static NTSTATUS smbd_smb2_auth_generic_return(struct smbXsrv_session *session,
}
}
- if (x->global->encryption_required) {
+ if (x->encryption_desired) {
*out_session_flags |= SMB2_SESSION_FLAG_ENCRYPT_DATA;
}
--
2.4.3
From 9533def45521c353350e39640a799bcde9a5050b Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 1 Jul 2015 18:07:52 +0200
Subject: [PATCH 5/7] smbd:smb2: only enable encryption in tcon if desired
Don't enforce it but only announce DATA_ENCRYPT,
making use of encryption_desired in tcon.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/smbd/smb2_tcon.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
index eb66ea0..99e2f21 100644
--- a/source3/smbd/smb2_tcon.c
+++ b/source3/smbd/smb2_tcon.c
@@ -193,6 +193,7 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
connection_struct *compat_conn = NULL;
struct user_struct *compat_vuser = req->session->compat;
NTSTATUS status;
+ bool encryption_desired = req->session->encryption_desired;
bool encryption_required = req->session->global->encryption_required;
bool guest_session = false;
bool require_signed_tcon = false;
@@ -266,12 +267,13 @@ 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;
+ encryption_desired = true;
}
if (lp_smb_encrypt(snum) == SMB_SIGNING_REQUIRED) {
+ encryption_desired = true;
encryption_required = true;
}
@@ -296,6 +298,7 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
return status;
}
+ tcon->encryption_desired = encryption_desired;
tcon->global->encryption_required = encryption_required;
compat_conn = make_connection_smb2(req,
@@ -366,7 +369,7 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
*out_share_flags |= SMB2_SHAREFLAG_ACCESS_BASED_DIRECTORY_ENUM;
}
- if (encryption_required) {
+ if (encryption_desired) {
*out_share_flags |= SMB2_SHAREFLAG_ENCRYPT_DATA;
}
--
2.4.3
From 80d38f4f77666482ab77e081c1310caf7863017f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 1 Jul 2015 17:41:38 +0200
Subject: [PATCH 6/7] smbd:smb2: use encryption_desired in send_break
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/smbd/smb2_server.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 97e99a0..2ea997e 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2853,8 +2853,8 @@ static NTSTATUS smbd_smb2_send_break(struct smbXsrv_connection *xconn,
if (session != NULL) {
session_wire_id = session->global->session_wire_id;
- do_encryption = session->global->encryption_required;
- if (tcon->global->encryption_required) {
+ do_encryption = session->encryption_desired;
+ if (tcon->encryption_desired) {
do_encryption = true;
}
}
--
2.4.3
From 0e2e989009bba0859a32800d63d1321e3ae72202 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 30 Jun 2015 17:46:36 +0200
Subject: [PATCH 7/7] docs:smb.conf: explain effect of new setting 'desired' of
smb encrypt
Thereby clarify some details.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11372
Signed-off-by: Michael Adam <obnox at samba.org>
---
docs-xml/smbdotconf/security/smbencrypt.xml | 66 ++++++++++++++++++++---------
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/docs-xml/smbdotconf/security/smbencrypt.xml b/docs-xml/smbdotconf/security/smbencrypt.xml
index 17248e6..ae0682b 100644
--- a/docs-xml/smbdotconf/security/smbencrypt.xml
+++ b/docs-xml/smbdotconf/security/smbencrypt.xml
@@ -30,11 +30,15 @@
<para>
This parameter can be set globally and on a per-share bases.
Possible values are
- <emphasis>off</emphasis> or <emphasis>disabled</emphasis>,
- <emphasis>auto</emphasis> or <emphasis>enabled</emphasis>, and
- <emphasis>mandatory</emphasis> or <emphasis>required</emphasis>.
+ <emphasis>off</emphasis> (or <emphasis>disabled</emphasis>),
+ <emphasis>enabled</emphasis> (or <emphasis>auto</emphasis>, or
+ <emphasis>if_required</emphasis>),
+ <emphasis>desired</emphasis>,
+ and
+ <emphasis>required</emphasis>
+ (or <emphasis>mandatory</emphasis>).
A special value is <emphasis>default</emphasis> which is
- the implicit default setting.
+ the implicit default setting of <emphasis>enabled</emphasis>.
</para>
<variablelist>
@@ -103,7 +107,7 @@
<listitem>
<para>
The capability to perform SMB encryption can be
- negotiated during prorocol negotiation.
+ negotiated during protocol negotiation.
</para>
</listitem>
@@ -145,8 +149,9 @@
<itemizedlist>
<listitem>
<para>
- Leaving it as default or explicitly setting
- <emphasis>default</emphasis> globally will enable
+ Leaving it as default, explicitly setting
+ <emphasis>default</emphasis>, or setting it to
+ <emphasis>enabled</emphasis> globally will enable
negotiation of encryption but will not turn on
data encryption globally or per share.
</para>
@@ -154,16 +159,20 @@
<listitem>
<para>
- Setting it to <emphasis>enabled</emphasis> globally will
- enable negotiation and turn on data encryption globally.
+ Setting it to <emphasis>desired</emphasis> globally
+ will enable negotiation and will turn on data encryption
+ on sessions and share connections for those clients
+ that support it.
</para>
</listitem>
<listitem>
<para>
Setting it to <emphasis>required</emphasis> globally
- will enable negotiation and enforce data encryption
- globally.
+ will enable negotiation and turn on data encryption
+ on sessions and share connections. Clients that do
+ not support encryption will be denied access to the
+ server.
</para>
</listitem>
@@ -176,9 +185,10 @@
<listitem>
<para>
- Setting it to <emphasis>enabled</emphasis> on a share
- will turn on data encryption for this share if
- negotiation has been enabled globally.
+ Setting it to <emphasis>desired</emphasis> on a share
+ will turn on data encryption for this share for clients
+ that support encryption if negotiation has been
+ enabled globally.
</para>
</listitem>
@@ -186,16 +196,34 @@
<para>
Setting it to <emphasis>required</emphasis> on a share
will enforce data encryption for this share if
- negotiation has been enabled globally. Note that this
- allows enforcing to be controlled in Samba more
- fine-grainedly than in Windows. This is a small
- deviation from the MS-SMB2 protocol document.
+ negotiation has been enabled globally. I.e. clients that
+ do not support encryption will be denied access to the
+ share.
+ </para>
+ <para>
+ Note that this allows per-share enforcing to be
+ controlled in Samba differently from Windows:
+ In Windows, <emphasis>RejectUnencryptedAccess</emphasis>
+ is a global setting, and if it is set, all shares with
+ data encryption turned on
+ are automatically enforcing encryption. In order to
+ achieve the same effect in Samba, one
+ has to globally set <emphasis>smb encrypt</emphasis> to
+ <emphasis>enabled</emphasis>, and then set all shares
+ that should be encrypted to
+ <emphasis>required</emphasis>.
+ Additionally, it is possible in Samba to have some
+ shares with encryption <emphasis>required</emphasis>
+ and some other shares with encryption only
+ <emphasis>desired</emphasis>, which is not possible in
+ Windows.
</para>
</listitem>
<listitem>
<para>
- Setting it to <emphasis>off</emphasis> for a share has
+ Setting it to <emphasis>off</emphasis> or
+ <emphasis>enabled</emphasis> for a share has
no effect.
</para>
</listitem>
--
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/59dc594b/attachment.pgp>
More information about the samba-technical
mailing list