[Patches] The way to remove gensec_update_ev()
Stefan Metzmacher
metze at samba.org
Tue Jun 27 22:51:38 UTC 2017
Am 27.06.2017 um 08:42 schrieb Stefan Metzmacher via samba-technical:
> Hi,
>
>>> here're some preparation patches which passed autobuild twice.
>
> and some more just passed a private autobuild...
>
> Please review and push:-)
These are in master now.
Here's the next chunk, please review and push:-)
Thanks!
metze
-------------- next part --------------
From dfd8a859b07267f6e3f894ce9859ea7dc9ad9067 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Dec 2016 16:36:23 +0100
Subject: [PATCH 01/14] auth/spnego: make use of data_blob_null instead of
using data_blob(NULL, 0)
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 9495933..7699355 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -91,7 +91,7 @@ static NTSTATUS gensec_spnego_client_start(struct gensec_security *gensec_securi
spnego_state->state_position = SPNEGO_CLIENT_START;
spnego_state->sub_sec_security = NULL;
spnego_state->no_response_expected = false;
- spnego_state->mech_types = data_blob(NULL, 0);
+ spnego_state->mech_types = data_blob_null;
spnego_state->out_max_length = gensec_max_update_size(gensec_security);
spnego_state->out_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -115,7 +115,7 @@ static NTSTATUS gensec_spnego_server_start(struct gensec_security *gensec_securi
spnego_state->state_position = SPNEGO_SERVER_START;
spnego_state->sub_sec_security = NULL;
spnego_state->no_response_expected = false;
- spnego_state->mech_types = data_blob(NULL, 0);
+ spnego_state->mech_types = data_blob_null;
spnego_state->out_max_length = gensec_max_update_size(gensec_security);
spnego_state->out_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -212,7 +212,6 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
{
int i;
NTSTATUS nt_status = NT_STATUS_INVALID_PARAMETER;
- DATA_BLOB null_data_blob = data_blob(NULL,0);
bool ok;
const struct gensec_security_ops_wrapper *all_sec
@@ -323,7 +322,7 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
nt_status = gensec_update_ev(spnego_state->sub_sec_security,
out_mem_ctx,
ev,
- null_data_blob,
+ data_blob_null,
unwrapped_out);
/* it is likely that a NULL input token will
@@ -383,7 +382,7 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
* time */
if (NT_STATUS_EQUAL(nt_status, NT_STATUS_INVALID_PARAMETER)) {
- *unwrapped_out = data_blob(NULL, 0);
+ *unwrapped_out = data_blob_null;
nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
}
@@ -426,9 +425,8 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
{
int i;
NTSTATUS nt_status = NT_STATUS_INVALID_PARAMETER;
- DATA_BLOB null_data_blob = data_blob(NULL,0);
const char **mechTypes = NULL;
- DATA_BLOB unwrapped_out = data_blob(NULL, 0);
+ DATA_BLOB unwrapped_out = data_blob_null;
const struct gensec_security_ops_wrapper *all_sec;
mechTypes = gensec_security_oids(gensec_security,
@@ -463,7 +461,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
nt_status = gensec_update_ev(spnego_state->sub_sec_security,
out_mem_ctx,
ev,
- null_data_blob,
+ data_blob_null,
&unwrapped_out);
if (!NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)
@@ -517,14 +515,14 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
/* List the remaining mechs as options */
spnego_out.negTokenInit.mechTypes = send_mech_types;
- spnego_out.negTokenInit.reqFlags = null_data_blob;
+ spnego_out.negTokenInit.reqFlags = data_blob_null;
spnego_out.negTokenInit.reqFlagsPadding = 0;
if (spnego_state->state_position == SPNEGO_SERVER_START) {
spnego_out.negTokenInit.mechListMIC
= data_blob_string_const(ADS_IGNORE_PRINCIPAL);
} else {
- spnego_out.negTokenInit.mechListMIC = null_data_blob;
+ spnego_out.negTokenInit.mechListMIC = data_blob_null;
}
spnego_out.negTokenInit.mechToken = unwrapped_out;
@@ -564,7 +562,6 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct spnego_state *spnego_st
DATA_BLOB *out)
{
struct spnego_data spnego_out;
- DATA_BLOB null_data_blob = data_blob(NULL, 0);
/* compose reply */
spnego_out.type = SPNEGO_NEG_TOKEN_TARG;
@@ -589,7 +586,7 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct spnego_state *spnego_st
spnego_state->state_position = SPNEGO_DONE;
} else {
spnego_out.negTokenTarg.negResult = SPNEGO_REJECT;
- spnego_out.negTokenTarg.mechListMIC = null_data_blob;
+ spnego_out.negTokenTarg.mechListMIC = data_blob_null;
DEBUG(2, ("SPNEGO login failed: %s\n", nt_errstr(nt_status)));
spnego_state->state_position = SPNEGO_DONE;
}
@@ -611,15 +608,14 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
const DATA_BLOB in, DATA_BLOB *out)
{
struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
- DATA_BLOB null_data_blob = data_blob(NULL, 0);
- DATA_BLOB mech_list_mic = data_blob(NULL, 0);
- DATA_BLOB unwrapped_out = data_blob(NULL, 0);
+ DATA_BLOB mech_list_mic = data_blob_null;
+ DATA_BLOB unwrapped_out = data_blob_null;
struct spnego_data spnego_out;
struct spnego_data spnego;
ssize_t len;
- *out = data_blob(NULL, 0);
+ *out = data_blob_null;
if (!out_mem_ctx) {
out_mem_ctx = spnego_state;
@@ -750,9 +746,9 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
/* compose reply */
spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
spnego_out.negTokenInit.mechTypes = my_mechs;
- spnego_out.negTokenInit.reqFlags = null_data_blob;
+ spnego_out.negTokenInit.reqFlags = data_blob_null;
spnego_out.negTokenInit.reqFlagsPadding = 0;
- spnego_out.negTokenInit.mechListMIC = null_data_blob;
+ spnego_out.negTokenInit.mechListMIC = data_blob_null;
spnego_out.negTokenInit.mechToken = unwrapped_out;
if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
@@ -1222,7 +1218,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
} else {
/* all done - server has accepted, and we agree */
- *out = null_data_blob;
+ *out = data_blob_null;
if (spnego.negTokenTarg.negResult != SPNEGO_ACCEPT_COMPLETED) {
/* unless of course it did not accept */
--
1.9.1
From ee85903991749d518d88063b74de2500feabbef8 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 13 Jun 2017 16:53:06 +0200
Subject: [PATCH 02/14] auth/spnego: move gensec_spnego_update_wrapper() into
gensec_spnego_update_send()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 98 ++++++++++++++++++++++++++++------------------------
1 file changed, 53 insertions(+), 45 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 7699355..ac1046d 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1383,36 +1383,69 @@ static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security
return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
-static NTSTATUS gensec_spnego_update_wrapper(struct gensec_security *gensec_security,
- TALLOC_CTX *out_mem_ctx,
- struct tevent_context *ev,
- const DATA_BLOB in, DATA_BLOB *out)
+struct gensec_spnego_update_state {
+ struct gensec_security *gensec;
+ struct spnego_state *spnego;
+ DATA_BLOB full_in;
+ NTSTATUS status;
+ DATA_BLOB out;
+};
+
+static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct gensec_security *gensec_security,
+ const DATA_BLOB in)
{
- struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
- DATA_BLOB full_in = data_blob_null;
+ struct spnego_state *spnego_state =
+ talloc_get_type_abort(gensec_security->private_data,
+ struct spnego_state);
+ struct tevent_req *req = NULL;
+ struct gensec_spnego_update_state *state = NULL;
NTSTATUS status;
- *out = data_blob_null;
+ req = tevent_req_create(mem_ctx, &state,
+ struct gensec_spnego_update_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ state->gensec = gensec_security;
+ state->spnego = spnego_state;
if (spnego_state->out_frag.length > 0) {
if (in.length > 0) {
- return NT_STATUS_INVALID_PARAMETER;
+ tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+ return tevent_req_post(req, ev);
+ }
+
+ status = gensec_spnego_update_out(gensec_security,
+ state, &state->out);
+ state->status = status;
+ if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
}
- return gensec_spnego_update_out(gensec_security,
- out_mem_ctx,
- out);
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
}
status = gensec_spnego_update_in(gensec_security,
- in, &full_in);
- if (!NT_STATUS_IS_OK(status)) {
- return status;
+ in, &state->full_in);
+ state->status = status;
+ if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
}
status = gensec_spnego_update(gensec_security,
- spnego_state, ev,
- full_in,
+ state, ev,
+ state->full_in,
&spnego_state->out_frag);
data_blob_free(&spnego_state->in_frag);
spnego_state->in_needed = 0;
@@ -1430,39 +1463,14 @@ static NTSTATUS gensec_spnego_update_wrapper(struct gensec_security *gensec_secu
* A fatal error, further updates are not allowed.
*/
spnego_state->state_position = SPNEGO_DONE;
- return status;
+ tevent_req_nterror(req, status);
+ return tevent_req_post(req, ev);
}
spnego_state->out_status = status;
- return gensec_spnego_update_out(gensec_security,
- out_mem_ctx,
- out);
-}
-
-struct gensec_spnego_update_state {
- NTSTATUS status;
- DATA_BLOB out;
-};
-
-static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev,
- struct gensec_security *gensec_security,
- const DATA_BLOB in)
-{
- struct tevent_req *req = NULL;
- struct gensec_spnego_update_state *state = NULL;
- NTSTATUS status;
-
- req = tevent_req_create(mem_ctx, &state,
- struct gensec_spnego_update_state);
- if (req == NULL) {
- return NULL;
- }
-
- status = gensec_spnego_update_wrapper(gensec_security,
- state, ev, in,
- &state->out);
+ status = gensec_spnego_update_out(gensec_security,
+ state, &state->out);
state->status = status;
if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
tevent_req_done(req);
--
1.9.1
From ba63ab52019df46239de2300e6b8a03c4557ee96 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 08:43:13 +0200
Subject: [PATCH 03/14] auth/spnego: set state_position = SPNEGO_DONE in
gensec_spnego_update_cleanup()
Every fatal error should mark the spnego_state to reject any further update()
calls.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index ac1046d..cb2c227 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1391,6 +1391,27 @@ struct gensec_spnego_update_state {
DATA_BLOB out;
};
+static void gensec_spnego_update_cleanup(struct tevent_req *req,
+ enum tevent_req_state req_state)
+{
+ struct gensec_spnego_update_state *state =
+ tevent_req_data(req,
+ struct gensec_spnego_update_state);
+
+ switch (req_state) {
+ case TEVENT_REQ_USER_ERROR:
+ case TEVENT_REQ_TIMED_OUT:
+ case TEVENT_REQ_NO_MEMORY:
+ /*
+ * A fatal error, further updates are not allowed.
+ */
+ state->spnego->state_position = SPNEGO_DONE;
+ break;
+ default:
+ break;
+ }
+}
+
static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct gensec_security *gensec_security,
@@ -1410,6 +1431,7 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
}
state->gensec = gensec_security;
state->spnego = spnego_state;
+ tevent_req_set_cleanup_fn(req, gensec_spnego_update_cleanup);
if (spnego_state->out_frag.length > 0) {
if (in.length > 0) {
@@ -1459,10 +1481,6 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
}
if (!NT_STATUS_IS_OK(status) &&
!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
- /*
- * A fatal error, further updates are not allowed.
- */
- spnego_state->state_position = SPNEGO_DONE;
tevent_req_nterror(req, status);
return tevent_req_post(req, ev);
}
--
1.9.1
From 520ef46fb87f73fd8ff8cd8fab15cf5f7ed7b852 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 13 Jun 2017 16:59:02 +0200
Subject: [PATCH 04/14] auth/spnego: move gensec_spnego_update_in() after
gensec_spnego_update_send()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 175 ++++++++++++++++++++++++++-------------------------
1 file changed, 89 insertions(+), 86 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index cb2c227..628652c 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1238,92 +1238,6 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
return NT_STATUS_INVALID_PARAMETER;
}
-static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
- const DATA_BLOB in, DATA_BLOB *full_in)
-{
- struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
- size_t expected;
- bool ok;
-
- *full_in = data_blob_null;
-
- if (spnego_state->in_needed == 0) {
- size_t size = 0;
- int ret;
-
- /*
- * try to work out the size of the full
- * input token, it might be fragmented
- */
- ret = asn1_peek_full_tag(in, ASN1_APPLICATION(0), &size);
- if ((ret != 0) && (ret != EAGAIN)) {
- ret = asn1_peek_full_tag(in, ASN1_CONTEXT(1), &size);
- }
-
- if ((ret == 0) || (ret == EAGAIN)) {
- spnego_state->in_needed = size;
- } else {
- /*
- * If it is not an asn1 message
- * just call the next layer.
- */
- spnego_state->in_needed = in.length;
- }
- }
-
- if (spnego_state->in_needed > UINT16_MAX) {
- /*
- * limit the incoming message to 0xFFFF
- * to avoid DoS attacks.
- */
- return NT_STATUS_INVALID_BUFFER_SIZE;
- }
-
- if ((spnego_state->in_needed > 0) && (in.length == 0)) {
- /*
- * If we reach this, we know we got at least
- * part of an asn1 message, getting 0 means
- * the remote peer wants us to spin.
- */
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- expected = spnego_state->in_needed - spnego_state->in_frag.length;
- if (in.length > expected) {
- /*
- * we got more than expected
- */
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- if (in.length == spnego_state->in_needed) {
- /*
- * if the in.length contains the full blob
- * we are done.
- *
- * Note: this implies spnego_state->in_frag.length == 0,
- * but we do not need to check this explicitly
- * because we already know that we did not get
- * more than expected.
- */
- *full_in = in;
- return NT_STATUS_OK;
- }
-
- ok = data_blob_append(spnego_state, &spnego_state->in_frag,
- in.data, in.length);
- if (!ok) {
- return NT_STATUS_NO_MEMORY;
- }
-
- if (spnego_state->in_needed > spnego_state->in_frag.length) {
- return NT_STATUS_MORE_PROCESSING_REQUIRED;
- }
-
- *full_in = spnego_state->in_frag;
- return NT_STATUS_OK;
-}
-
static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security,
TALLOC_CTX *out_mem_ctx,
DATA_BLOB *_out)
@@ -1412,6 +1326,9 @@ static void gensec_spnego_update_cleanup(struct tevent_req *req,
}
}
+static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
+ const DATA_BLOB in, DATA_BLOB *full_in);
+
static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct gensec_security *gensec_security,
@@ -1502,6 +1419,92 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
+static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
+ const DATA_BLOB in, DATA_BLOB *full_in)
+{
+ struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
+ size_t expected;
+ bool ok;
+
+ *full_in = data_blob_null;
+
+ if (spnego_state->in_needed == 0) {
+ size_t size = 0;
+ int ret;
+
+ /*
+ * try to work out the size of the full
+ * input token, it might be fragmented
+ */
+ ret = asn1_peek_full_tag(in, ASN1_APPLICATION(0), &size);
+ if ((ret != 0) && (ret != EAGAIN)) {
+ ret = asn1_peek_full_tag(in, ASN1_CONTEXT(1), &size);
+ }
+
+ if ((ret == 0) || (ret == EAGAIN)) {
+ spnego_state->in_needed = size;
+ } else {
+ /*
+ * If it is not an asn1 message
+ * just call the next layer.
+ */
+ spnego_state->in_needed = in.length;
+ }
+ }
+
+ if (spnego_state->in_needed > UINT16_MAX) {
+ /*
+ * limit the incoming message to 0xFFFF
+ * to avoid DoS attacks.
+ */
+ return NT_STATUS_INVALID_BUFFER_SIZE;
+ }
+
+ if ((spnego_state->in_needed > 0) && (in.length == 0)) {
+ /*
+ * If we reach this, we know we got at least
+ * part of an asn1 message, getting 0 means
+ * the remote peer wants us to spin.
+ */
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ expected = spnego_state->in_needed - spnego_state->in_frag.length;
+ if (in.length > expected) {
+ /*
+ * we got more than expected
+ */
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ if (in.length == spnego_state->in_needed) {
+ /*
+ * if the in.length contains the full blob
+ * we are done.
+ *
+ * Note: this implies spnego_state->in_frag.length == 0,
+ * but we do not need to check this explicitly
+ * because we already know that we did not get
+ * more than expected.
+ */
+ *full_in = in;
+ return NT_STATUS_OK;
+ }
+
+ ok = data_blob_append(spnego_state, &spnego_state->in_frag,
+ in.data, in.length);
+ if (!ok) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ if (spnego_state->in_needed > spnego_state->in_frag.length) {
+ return NT_STATUS_MORE_PROCESSING_REQUIRED;
+ }
+
+ *full_in = spnego_state->in_frag;
+ return NT_STATUS_OK;
+}
+
static NTSTATUS gensec_spnego_update_recv(struct tevent_req *req,
TALLOC_CTX *out_mem_ctx,
DATA_BLOB *out)
--
1.9.1
From 8c8eaca2569c1a05e63747b6db85bcd64f814276 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 13 Jun 2017 22:41:14 +0200
Subject: [PATCH 05/14] auth/spnego: move some more logic to
gensec_spnego_update_in()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 628652c..7e86d1f 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1327,7 +1327,8 @@ static void gensec_spnego_update_cleanup(struct tevent_req *req,
}
static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
- const DATA_BLOB in, DATA_BLOB *full_in);
+ const DATA_BLOB in, TALLOC_CTX *mem_ctx,
+ DATA_BLOB *full_in);
static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -1371,8 +1372,8 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- status = gensec_spnego_update_in(gensec_security,
- in, &state->full_in);
+ status = gensec_spnego_update_in(gensec_security, in,
+ state, &state->full_in);
state->status = status;
if (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
tevent_req_done(req);
@@ -1386,8 +1387,6 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
state, ev,
state->full_in,
&spnego_state->out_frag);
- data_blob_free(&spnego_state->in_frag);
- spnego_state->in_needed = 0;
if (NT_STATUS_IS_OK(status)) {
bool reset_full = true;
@@ -1420,7 +1419,8 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
}
static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
- const DATA_BLOB in, DATA_BLOB *full_in)
+ const DATA_BLOB in, TALLOC_CTX *mem_ctx,
+ DATA_BLOB *full_in)
{
struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
size_t expected;
@@ -1488,6 +1488,7 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
* more than expected.
*/
*full_in = in;
+ spnego_state->in_needed = 0;
return NT_STATUS_OK;
}
@@ -1502,6 +1503,9 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
}
*full_in = spnego_state->in_frag;
+ talloc_steal(mem_ctx, full_in->data);
+ spnego_state->in_frag = data_blob_null;
+ spnego_state->in_needed = 0;
return NT_STATUS_OK;
}
--
1.9.1
From 913863297fca9007b3d9c54f1f1b2a6552d6b280 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 13 Jun 2017 22:43:59 +0200
Subject: [PATCH 06/14] auth/spnego: move gensec_spnego_update_out() behind
gensec_spnego_update_in()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 121 ++++++++++++++++++++++++++-------------------------
1 file changed, 62 insertions(+), 59 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 7e86d1f..f01db05 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1238,65 +1238,6 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
return NT_STATUS_INVALID_PARAMETER;
}
-static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security,
- TALLOC_CTX *out_mem_ctx,
- DATA_BLOB *_out)
-{
- struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
- DATA_BLOB out = data_blob_null;
- bool ok;
-
- *_out = data_blob_null;
-
- if (spnego_state->out_frag.length <= spnego_state->out_max_length) {
- /*
- * Fast path, we can deliver everything
- */
-
- *_out = spnego_state->out_frag;
- if (spnego_state->out_frag.length > 0) {
- talloc_steal(out_mem_ctx, _out->data);
- spnego_state->out_frag = data_blob_null;
- }
-
- if (!NT_STATUS_IS_OK(spnego_state->out_status)) {
- return spnego_state->out_status;
- }
-
- /*
- * We're completely done, further updates are not allowed.
- */
- spnego_state->state_position = SPNEGO_DONE;
- return gensec_child_ready(gensec_security,
- spnego_state->sub_sec_security);
- }
-
- out = spnego_state->out_frag;
-
- /*
- * copy the remaining bytes
- */
- spnego_state->out_frag = data_blob_talloc(spnego_state,
- out.data + spnego_state->out_max_length,
- out.length - spnego_state->out_max_length);
- if (spnego_state->out_frag.data == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
-
- /*
- * truncate the buffer
- */
- ok = data_blob_realloc(spnego_state, &out,
- spnego_state->out_max_length);
- if (!ok) {
- return NT_STATUS_NO_MEMORY;
- }
-
- talloc_steal(out_mem_ctx, out.data);
- *_out = out;
- return NT_STATUS_MORE_PROCESSING_REQUIRED;
-}
-
struct gensec_spnego_update_state {
struct gensec_security *gensec;
struct spnego_state *spnego;
@@ -1329,6 +1270,9 @@ static void gensec_spnego_update_cleanup(struct tevent_req *req,
static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
const DATA_BLOB in, TALLOC_CTX *mem_ctx,
DATA_BLOB *full_in);
+static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *_out);
static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -1509,6 +1453,65 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
return NT_STATUS_OK;
}
+static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *_out)
+{
+ struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
+ DATA_BLOB out = data_blob_null;
+ bool ok;
+
+ *_out = data_blob_null;
+
+ if (spnego_state->out_frag.length <= spnego_state->out_max_length) {
+ /*
+ * Fast path, we can deliver everything
+ */
+
+ *_out = spnego_state->out_frag;
+ if (spnego_state->out_frag.length > 0) {
+ talloc_steal(out_mem_ctx, _out->data);
+ spnego_state->out_frag = data_blob_null;
+ }
+
+ if (!NT_STATUS_IS_OK(spnego_state->out_status)) {
+ return spnego_state->out_status;
+ }
+
+ /*
+ * We're completely done, further updates are not allowed.
+ */
+ spnego_state->state_position = SPNEGO_DONE;
+ return gensec_child_ready(gensec_security,
+ spnego_state->sub_sec_security);
+ }
+
+ out = spnego_state->out_frag;
+
+ /*
+ * copy the remaining bytes
+ */
+ spnego_state->out_frag = data_blob_talloc(spnego_state,
+ out.data + spnego_state->out_max_length,
+ out.length - spnego_state->out_max_length);
+ if (spnego_state->out_frag.data == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ /*
+ * truncate the buffer
+ */
+ ok = data_blob_realloc(spnego_state, &out,
+ spnego_state->out_max_length);
+ if (!ok) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ talloc_steal(out_mem_ctx, out.data);
+ *_out = out;
+ return NT_STATUS_MORE_PROCESSING_REQUIRED;
+}
+
static NTSTATUS gensec_spnego_update_recv(struct tevent_req *req,
TALLOC_CTX *out_mem_ctx,
DATA_BLOB *out)
--
1.9.1
From 7afc52df13cf98417d728972399d8317e52b1231 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Dec 2016 09:03:08 +0100
Subject: [PATCH 07/14] auth/spnego: rename spnego_state->no_response_expected
to ->sub_sec_ready
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index f01db05..f8be423 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -51,7 +51,7 @@ struct spnego_state {
enum spnego_message_type expected_packet;
enum spnego_state_position state_position;
struct gensec_security *sub_sec_security;
- bool no_response_expected;
+ bool sub_sec_ready;
const char *neg_oid;
@@ -90,7 +90,7 @@ static NTSTATUS gensec_spnego_client_start(struct gensec_security *gensec_securi
spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
spnego_state->state_position = SPNEGO_CLIENT_START;
spnego_state->sub_sec_security = NULL;
- spnego_state->no_response_expected = false;
+ spnego_state->sub_sec_ready = false;
spnego_state->mech_types = data_blob_null;
spnego_state->out_max_length = gensec_max_update_size(gensec_security);
spnego_state->out_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -114,7 +114,7 @@ static NTSTATUS gensec_spnego_server_start(struct gensec_security *gensec_securi
spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
spnego_state->state_position = SPNEGO_SERVER_START;
spnego_state->sub_sec_security = NULL;
- spnego_state->no_response_expected = false;
+ spnego_state->sub_sec_ready = false;
spnego_state->mech_types = data_blob_null;
spnego_state->out_max_length = gensec_max_update_size(gensec_security);
spnego_state->out_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -536,7 +536,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
spnego_state->neg_oid = all_sec[i].oid;
if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->no_response_expected = true;
+ spnego_state->sub_sec_ready = true;
}
return NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -769,7 +769,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
spnego_state->state_position = SPNEGO_CLIENT_TARG;
if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->no_response_expected = true;
+ spnego_state->sub_sec_ready = true;
}
spnego_free_data(&spnego);
@@ -951,7 +951,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
gensec_get_name_by_oid(gensec_security, spnego_state->neg_oid),
gensec_get_name_by_oid(gensec_security, spnego.negTokenTarg.supportedMech)));
spnego_state->downgraded = true;
- spnego_state->no_response_expected = false;
+ spnego_state->sub_sec_ready = false;
talloc_free(spnego_state->sub_sec_security);
nt_status = gensec_subcontext_start(spnego_state,
gensec_security,
@@ -995,7 +995,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
}
if (spnego.negTokenTarg.mechListMIC.length > 0) {
- if (spnego_state->no_response_expected) {
+ if (spnego_state->sub_sec_ready) {
spnego_state->needs_mic_check = true;
}
}
@@ -1041,7 +1041,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
goto client_response;
}
- if (!spnego_state->no_response_expected) {
+ if (!spnego_state->sub_sec_ready) {
nt_status = gensec_update_ev(spnego_state->sub_sec_security,
out_mem_ctx, ev,
spnego.negTokenTarg.responseToken,
@@ -1050,12 +1050,12 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
goto client_response;
}
- spnego_state->no_response_expected = true;
+ spnego_state->sub_sec_ready = true;
} else {
nt_status = NT_STATUS_OK;
}
- if (spnego_state->no_response_expected &&
+ if (spnego_state->sub_sec_ready &&
!spnego_state->done_mic_check)
{
bool have_sign = true;
--
1.9.1
From e0a5a8139d2deadf0f85a22f2cdff126740fbf42 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Dec 2016 09:04:47 +0100
Subject: [PATCH 08/14] auth/spnego: consitently set
spnego_state->sub_sec_ready = true after gensec_update_ev()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index f8be423..c548db4 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -270,6 +270,9 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
ev,
unwrapped_in,
unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
if (NT_STATUS_EQUAL(nt_status, NT_STATUS_INVALID_PARAMETER) ||
NT_STATUS_EQUAL(nt_status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO)) {
/* Pretend we never started it (lets the first run find some incompatible demand) */
@@ -324,6 +327,9 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
ev,
data_blob_null,
unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
/* it is likely that a NULL input token will
* not be liked by most server mechs, but if
@@ -463,6 +469,9 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
ev,
data_blob_null,
&unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
if (!NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)
&& !NT_STATUS_IS_OK(nt_status)) {
@@ -535,10 +544,6 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
/* set next state */
spnego_state->neg_oid = all_sec[i].oid;
- if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->sub_sec_ready = true;
- }
-
return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
talloc_free(spnego_state->sub_sec_security);
@@ -768,10 +773,6 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
spnego_state->expected_packet = SPNEGO_NEG_TOKEN_TARG;
spnego_state->state_position = SPNEGO_CLIENT_TARG;
- if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->sub_sec_ready = true;
- }
-
spnego_free_data(&spnego);
return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
@@ -837,6 +838,9 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
out_mem_ctx, ev,
spnego.negTokenTarg.responseToken,
&unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
if (!NT_STATUS_IS_OK(nt_status)) {
goto server_response;
}
@@ -1046,11 +1050,12 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
out_mem_ctx, ev,
spnego.negTokenTarg.responseToken,
&unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
if (!NT_STATUS_IS_OK(nt_status)) {
goto client_response;
}
-
- spnego_state->sub_sec_ready = true;
} else {
nt_status = NT_STATUS_OK;
}
--
1.9.1
From e7dae261b117ce860140680805a56d88b9a0427e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Dec 2016 09:06:33 +0100
Subject: [PATCH 09/14] auth/spnego: remove useless spnego_state->sub_sec_ready
check
The lines above make sure it's always true.
Check with git show -U15
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index c548db4..e3f9def 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1060,9 +1060,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
nt_status = NT_STATUS_OK;
}
- if (spnego_state->sub_sec_ready &&
- !spnego_state->done_mic_check)
- {
+ if (!spnego_state->done_mic_check) {
bool have_sign = true;
bool new_spnego = false;
--
1.9.1
From 4876d3f7ca6f276891642f9e699b43a6ae867f54 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 10 May 2017 14:44:48 +0200
Subject: [PATCH 10/14] auth/spnego: add gensec_spnego_update_sub_abort()
helper function
This helps to be consistent when destroying a unuseable sub context.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 53 ++++++++++++++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index e3f9def..9ea0dee 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -77,6 +77,11 @@ struct spnego_state {
NTSTATUS out_status;
};
+static void gensec_spnego_update_sub_abort(struct spnego_state *spnego_state)
+{
+ spnego_state->sub_sec_ready = false;
+ TALLOC_FREE(spnego_state->sub_sec_security);
+}
static NTSTATUS gensec_spnego_client_start(struct gensec_security *gensec_security)
{
@@ -246,8 +251,10 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
nt_status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
all_sec[i].op);
if (!NT_STATUS_IS_OK(nt_status)) {
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
break;
}
@@ -275,12 +282,14 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
}
if (NT_STATUS_EQUAL(nt_status, NT_STATUS_INVALID_PARAMETER) ||
NT_STATUS_EQUAL(nt_status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO)) {
- /* Pretend we never started it (lets the first run find some incompatible demand) */
DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed to parse contents: %s\n",
spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
+
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
break;
}
@@ -314,8 +323,10 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
nt_status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
all_sec[i].op);
if (!NT_STATUS_IS_OK(nt_status)) {
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
+ /*
+ * Pretend we never started it.
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
continue;
}
@@ -368,9 +379,10 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
principal,
next, nt_errstr(nt_status)));
- /* Pretend we never started it (lets the first run find some incompatible demand) */
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
+ /*
+ * Pretend we never started it.
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
continue;
}
}
@@ -397,13 +409,12 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
&& !NT_STATUS_IS_OK(nt_status)) {
DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed: %s\n",
spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
/* We started the mech correctly, and the
* input from the other side was valid.
* Return the error (say bad password, invalid
* ticket) */
+ gensec_spnego_update_sub_abort(spnego_state);
return nt_status;
}
@@ -457,8 +468,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
nt_status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
all_sec[i].op);
if (!NT_STATUS_IS_OK(nt_status)) {
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
+ gensec_spnego_update_sub_abort(spnego_state);
continue;
}
@@ -501,10 +511,11 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
spnego_state->sub_sec_security->ops->name,
principal,
next, nt_errstr(nt_status)));
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
- /* Pretend we never started it (lets the first run find some incompatible demand) */
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
continue;
}
}
@@ -545,9 +556,8 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
spnego_state->neg_oid = all_sec[i].oid;
return NT_STATUS_MORE_PROCESSING_REQUIRED;
- }
- talloc_free(spnego_state->sub_sec_security);
- spnego_state->sub_sec_security = NULL;
+ }
+ gensec_spnego_update_sub_abort(spnego_state);
DEBUG(10, ("Failed to setup SPNEGO negTokenInit request: %s\n", nt_errstr(nt_status)));
return nt_status;
@@ -955,8 +965,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
gensec_get_name_by_oid(gensec_security, spnego_state->neg_oid),
gensec_get_name_by_oid(gensec_security, spnego.negTokenTarg.supportedMech)));
spnego_state->downgraded = true;
- spnego_state->sub_sec_ready = false;
- talloc_free(spnego_state->sub_sec_security);
+ gensec_spnego_update_sub_abort(spnego_state);
nt_status = gensec_subcontext_start(spnego_state,
gensec_security,
&spnego_state->sub_sec_security);
--
1.9.1
From 25c471d87927e2e7f76bfd587e5106513c0fe098 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 27 Jun 2017 18:05:04 +0200
Subject: [PATCH 11/14] auth/spnego: remove unused out_mem_ctx = spnego_state
fallback in gensec_spnego_update()
The only caller never passes NULL.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 9ea0dee..3c5b6ae 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -627,15 +627,10 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
DATA_BLOB unwrapped_out = data_blob_null;
struct spnego_data spnego_out;
struct spnego_data spnego;
-
ssize_t len;
*out = data_blob_null;
- if (!out_mem_ctx) {
- out_mem_ctx = spnego_state;
- }
-
/* and switch into the state machine */
switch (spnego_state->state_position) {
--
1.9.1
From 41ee3cc87fa70e734f1ef1f99a5e6a2bd8c63dbf Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 30 Dec 2016 06:56:47 +0100
Subject: [PATCH 12/14] auth/spnego: split out
gensec_spnego_update_{client,server}() functions
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 456 +++++++++++++++++++++++++++++----------------------
1 file changed, 256 insertions(+), 200 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 3c5b6ae..b581e16 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -617,10 +617,10 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct spnego_state *spnego_st
return nt_status;
}
-
-static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TALLOC_CTX *out_mem_ctx,
- struct tevent_context *ev,
- const DATA_BLOB in, DATA_BLOB *out)
+static NTSTATUS gensec_spnego_update_client(struct gensec_security *gensec_security,
+ TALLOC_CTX *out_mem_ctx,
+ struct tevent_context *ev,
+ const DATA_BLOB in, DATA_BLOB *out)
{
struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
DATA_BLOB mech_list_mic = data_blob_null;
@@ -634,69 +634,6 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
/* and switch into the state machine */
switch (spnego_state->state_position) {
- case SPNEGO_FALLBACK:
- return gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx, ev, in, out);
- case SPNEGO_SERVER_START:
- {
- NTSTATUS nt_status;
- if (in.length) {
-
- len = spnego_read_data(gensec_security, in, &spnego);
- if (len == -1) {
- return gensec_spnego_server_try_fallback(gensec_security, spnego_state,
- ev, out_mem_ctx, in, out);
- }
- /* client sent NegTargetInit, we send NegTokenTarg */
-
- /* OK, so it's real SPNEGO, check the packet's the one we expect */
- if (spnego.type != spnego_state->expected_packet) {
- DEBUG(1, ("Invalid SPNEGO request: %d, expected %d\n", spnego.type,
- spnego_state->expected_packet));
- dump_data(1, in.data, in.length);
- spnego_free_data(&spnego);
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- nt_status = gensec_spnego_parse_negTokenInit(gensec_security,
- spnego_state,
- out_mem_ctx,
- ev,
- spnego.negTokenInit.mechTypes,
- spnego.negTokenInit.mechToken,
- &unwrapped_out);
-
- if (spnego_state->simulate_w2k) {
- /*
- * Windows 2000 returns the unwrapped token
- * also in the mech_list_mic field.
- *
- * In order to verify our client code,
- * we need a way to have a server with this
- * broken behaviour
- */
- mech_list_mic = unwrapped_out;
- }
-
- nt_status = gensec_spnego_server_negTokenTarg(spnego_state,
- out_mem_ctx,
- nt_status,
- unwrapped_out,
- mech_list_mic,
- out);
-
- spnego_free_data(&spnego);
-
- return nt_status;
- } else {
- nt_status = gensec_spnego_create_negTokenInit(gensec_security, spnego_state,
- out_mem_ctx, ev, in, out);
- spnego_state->state_position = SPNEGO_SERVER_START;
- spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
- return nt_status;
- }
- }
-
case SPNEGO_CLIENT_START:
{
/* The server offers a list of mechanisms */
@@ -781,140 +718,7 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
spnego_free_data(&spnego);
return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
- case SPNEGO_SERVER_TARG:
- {
- NTSTATUS nt_status;
- bool have_sign = true;
- bool new_spnego = false;
-
- if (!in.length) {
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- len = spnego_read_data(gensec_security, in, &spnego);
-
- if (len == -1) {
- DEBUG(1, ("Invalid SPNEGO request:\n"));
- dump_data(1, in.data, in.length);
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- /* OK, so it's real SPNEGO, check the packet's the one we expect */
- if (spnego.type != spnego_state->expected_packet) {
- DEBUG(1, ("Invalid SPNEGO request: %d, expected %d\n", spnego.type,
- spnego_state->expected_packet));
- dump_data(1, in.data, in.length);
- spnego_free_data(&spnego);
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- spnego_state->num_targs++;
-
- if (!spnego_state->sub_sec_security) {
- DEBUG(1, ("SPNEGO: Did not setup a mech in NEG_TOKEN_INIT\n"));
- spnego_free_data(&spnego);
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- if (spnego_state->needs_mic_check) {
- if (spnego.negTokenTarg.responseToken.length != 0) {
- DEBUG(1, ("SPNEGO: Did not setup a mech in NEG_TOKEN_INIT\n"));
- spnego_free_data(&spnego);
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- nt_status = gensec_check_packet(spnego_state->sub_sec_security,
- spnego_state->mech_types.data,
- spnego_state->mech_types.length,
- spnego_state->mech_types.data,
- spnego_state->mech_types.length,
- &spnego.negTokenTarg.mechListMIC);
- if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->needs_mic_check = false;
- spnego_state->done_mic_check = true;
- } else {
- DEBUG(2,("GENSEC SPNEGO: failed to verify mechListMIC: %s\n",
- nt_errstr(nt_status)));
- }
- goto server_response;
- }
-
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx, ev,
- spnego.negTokenTarg.responseToken,
- &unwrapped_out);
- if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->sub_sec_ready = true;
- }
- if (!NT_STATUS_IS_OK(nt_status)) {
- goto server_response;
- }
- have_sign = gensec_have_feature(spnego_state->sub_sec_security,
- GENSEC_FEATURE_SIGN);
- if (spnego_state->simulate_w2k) {
- have_sign = false;
- }
- new_spnego = gensec_have_feature(spnego_state->sub_sec_security,
- GENSEC_FEATURE_NEW_SPNEGO);
- if (spnego.negTokenTarg.mechListMIC.length > 0) {
- new_spnego = true;
- }
-
- if (have_sign && new_spnego) {
- spnego_state->needs_mic_check = true;
- spnego_state->needs_mic_sign = true;
- }
-
- if (have_sign && spnego.negTokenTarg.mechListMIC.length > 0) {
- nt_status = gensec_check_packet(spnego_state->sub_sec_security,
- spnego_state->mech_types.data,
- spnego_state->mech_types.length,
- spnego_state->mech_types.data,
- spnego_state->mech_types.length,
- &spnego.negTokenTarg.mechListMIC);
- if (!NT_STATUS_IS_OK(nt_status)) {
- DEBUG(2,("GENSEC SPNEGO: failed to verify mechListMIC: %s\n",
- nt_errstr(nt_status)));
- goto server_response;
- }
-
- spnego_state->needs_mic_check = false;
- spnego_state->done_mic_check = true;
- }
-
- if (spnego_state->needs_mic_sign) {
- nt_status = gensec_sign_packet(spnego_state->sub_sec_security,
- out_mem_ctx,
- spnego_state->mech_types.data,
- spnego_state->mech_types.length,
- spnego_state->mech_types.data,
- spnego_state->mech_types.length,
- &mech_list_mic);
- if (!NT_STATUS_IS_OK(nt_status)) {
- DEBUG(2,("GENSEC SPNEGO: failed to sign mechListMIC: %s\n",
- nt_errstr(nt_status)));
- goto server_response;
- }
- spnego_state->needs_mic_sign = false;
- }
-
- if (spnego_state->needs_mic_check) {
- nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- }
-
- server_response:
- nt_status = gensec_spnego_server_negTokenTarg(spnego_state,
- out_mem_ctx,
- nt_status,
- unwrapped_out,
- mech_list_mic,
- out);
-
- spnego_free_data(&spnego);
-
- return nt_status;
- }
case SPNEGO_CLIENT_TARG:
{
NTSTATUS nt_status = NT_STATUS_INTERNAL_ERROR;
@@ -1238,6 +1042,258 @@ static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TA
return nt_status;
}
+
+ default:
+ break;
+ }
+
+ smb_panic(__location__);
+ return NT_STATUS_INTERNAL_ERROR;
+}
+
+static NTSTATUS gensec_spnego_update_server(struct gensec_security *gensec_security,
+ TALLOC_CTX *out_mem_ctx,
+ struct tevent_context *ev,
+ const DATA_BLOB in, DATA_BLOB *out)
+{
+ struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
+ DATA_BLOB mech_list_mic = data_blob_null;
+ DATA_BLOB unwrapped_out = data_blob_null;
+ struct spnego_data spnego;
+ ssize_t len;
+
+ /* and switch into the state machine */
+
+ switch (spnego_state->state_position) {
+ case SPNEGO_SERVER_START:
+ {
+ NTSTATUS nt_status;
+ if (in.length) {
+
+ len = spnego_read_data(gensec_security, in, &spnego);
+ if (len == -1) {
+ return gensec_spnego_server_try_fallback(gensec_security, spnego_state,
+ ev, out_mem_ctx, in, out);
+ }
+ /* client sent NegTargetInit, we send NegTokenTarg */
+
+ /* OK, so it's real SPNEGO, check the packet's the one we expect */
+ if (spnego.type != spnego_state->expected_packet) {
+ DEBUG(1, ("Invalid SPNEGO request: %d, expected %d\n", spnego.type,
+ spnego_state->expected_packet));
+ dump_data(1, in.data, in.length);
+ spnego_free_data(&spnego);
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ nt_status = gensec_spnego_parse_negTokenInit(gensec_security,
+ spnego_state,
+ out_mem_ctx,
+ ev,
+ spnego.negTokenInit.mechTypes,
+ spnego.negTokenInit.mechToken,
+ &unwrapped_out);
+
+ if (spnego_state->simulate_w2k) {
+ /*
+ * Windows 2000 returns the unwrapped token
+ * also in the mech_list_mic field.
+ *
+ * In order to verify our client code,
+ * we need a way to have a server with this
+ * broken behaviour
+ */
+ mech_list_mic = unwrapped_out;
+ }
+
+ nt_status = gensec_spnego_server_negTokenTarg(spnego_state,
+ out_mem_ctx,
+ nt_status,
+ unwrapped_out,
+ mech_list_mic,
+ out);
+
+ spnego_free_data(&spnego);
+
+ return nt_status;
+ } else {
+ nt_status = gensec_spnego_create_negTokenInit(gensec_security, spnego_state,
+ out_mem_ctx, ev, in, out);
+ spnego_state->state_position = SPNEGO_SERVER_START;
+ spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
+ return nt_status;
+ }
+ }
+
+ case SPNEGO_SERVER_TARG:
+ {
+ NTSTATUS nt_status;
+ bool have_sign = true;
+ bool new_spnego = false;
+
+ if (!in.length) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ len = spnego_read_data(gensec_security, in, &spnego);
+
+ if (len == -1) {
+ DEBUG(1, ("Invalid SPNEGO request:\n"));
+ dump_data(1, in.data, in.length);
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ /* OK, so it's real SPNEGO, check the packet's the one we expect */
+ if (spnego.type != spnego_state->expected_packet) {
+ DEBUG(1, ("Invalid SPNEGO request: %d, expected %d\n", spnego.type,
+ spnego_state->expected_packet));
+ dump_data(1, in.data, in.length);
+ spnego_free_data(&spnego);
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ spnego_state->num_targs++;
+
+ if (!spnego_state->sub_sec_security) {
+ DEBUG(1, ("SPNEGO: Did not setup a mech in NEG_TOKEN_INIT\n"));
+ spnego_free_data(&spnego);
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ if (spnego_state->needs_mic_check) {
+ if (spnego.negTokenTarg.responseToken.length != 0) {
+ DEBUG(1, ("SPNEGO: Did not setup a mech in NEG_TOKEN_INIT\n"));
+ spnego_free_data(&spnego);
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ nt_status = gensec_check_packet(spnego_state->sub_sec_security,
+ spnego_state->mech_types.data,
+ spnego_state->mech_types.length,
+ spnego_state->mech_types.data,
+ spnego_state->mech_types.length,
+ &spnego.negTokenTarg.mechListMIC);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->needs_mic_check = false;
+ spnego_state->done_mic_check = true;
+ } else {
+ DEBUG(2,("GENSEC SPNEGO: failed to verify mechListMIC: %s\n",
+ nt_errstr(nt_status)));
+ }
+ goto server_response;
+ }
+
+ nt_status = gensec_update_ev(spnego_state->sub_sec_security,
+ out_mem_ctx, ev,
+ spnego.negTokenTarg.responseToken,
+ &unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ goto server_response;
+ }
+
+ have_sign = gensec_have_feature(spnego_state->sub_sec_security,
+ GENSEC_FEATURE_SIGN);
+ if (spnego_state->simulate_w2k) {
+ have_sign = false;
+ }
+ new_spnego = gensec_have_feature(spnego_state->sub_sec_security,
+ GENSEC_FEATURE_NEW_SPNEGO);
+ if (spnego.negTokenTarg.mechListMIC.length > 0) {
+ new_spnego = true;
+ }
+
+ if (have_sign && new_spnego) {
+ spnego_state->needs_mic_check = true;
+ spnego_state->needs_mic_sign = true;
+ }
+
+ if (have_sign && spnego.negTokenTarg.mechListMIC.length > 0) {
+ nt_status = gensec_check_packet(spnego_state->sub_sec_security,
+ spnego_state->mech_types.data,
+ spnego_state->mech_types.length,
+ spnego_state->mech_types.data,
+ spnego_state->mech_types.length,
+ &spnego.negTokenTarg.mechListMIC);
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ DEBUG(2,("GENSEC SPNEGO: failed to verify mechListMIC: %s\n",
+ nt_errstr(nt_status)));
+ goto server_response;
+ }
+
+ spnego_state->needs_mic_check = false;
+ spnego_state->done_mic_check = true;
+ }
+
+ if (spnego_state->needs_mic_sign) {
+ nt_status = gensec_sign_packet(spnego_state->sub_sec_security,
+ out_mem_ctx,
+ spnego_state->mech_types.data,
+ spnego_state->mech_types.length,
+ spnego_state->mech_types.data,
+ spnego_state->mech_types.length,
+ &mech_list_mic);
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ DEBUG(2,("GENSEC SPNEGO: failed to sign mechListMIC: %s\n",
+ nt_errstr(nt_status)));
+ goto server_response;
+ }
+ spnego_state->needs_mic_sign = false;
+ }
+
+ if (spnego_state->needs_mic_check) {
+ nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+ }
+
+ server_response:
+ nt_status = gensec_spnego_server_negTokenTarg(spnego_state,
+ out_mem_ctx,
+ nt_status,
+ unwrapped_out,
+ mech_list_mic,
+ out);
+
+ spnego_free_data(&spnego);
+
+ return nt_status;
+ }
+
+ default:
+ break;
+ }
+
+ smb_panic(__location__);
+ return NT_STATUS_INTERNAL_ERROR;
+}
+
+
+static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TALLOC_CTX *out_mem_ctx,
+ struct tevent_context *ev,
+ const DATA_BLOB in, DATA_BLOB *out)
+{
+ struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
+
+ *out = data_blob_null;
+
+ /* and switch into the state machine */
+
+ switch (spnego_state->state_position) {
+ case SPNEGO_FALLBACK:
+ return gensec_update_ev(spnego_state->sub_sec_security,
+ out_mem_ctx, ev, in, out);
+
+ case SPNEGO_CLIENT_START:
+ case SPNEGO_CLIENT_TARG:
+ return gensec_spnego_update_client(gensec_security, out_mem_ctx,
+ ev, in, out);
+
+ case SPNEGO_SERVER_START:
+ case SPNEGO_SERVER_TARG:
+ return gensec_spnego_update_server(gensec_security, out_mem_ctx,
+ ev, in, out);
+
case SPNEGO_DONE:
/* We should not be called after we are 'done' */
return NT_STATUS_INVALID_PARAMETER;
--
1.9.1
From 5e1e8e202d26ca2a5e9f71373c170a2d1d972ff0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 13 Jun 2017 23:41:01 +0200
Subject: [PATCH 13/14] auth/spnego: move gensec_spnego_update() into
gensec_spnego_update_send()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 73 ++++++++++++++++++++++++++--------------------------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index b581e16..f085885 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1268,39 +1268,6 @@ static NTSTATUS gensec_spnego_update_server(struct gensec_security *gensec_secur
return NT_STATUS_INTERNAL_ERROR;
}
-
-static NTSTATUS gensec_spnego_update(struct gensec_security *gensec_security, TALLOC_CTX *out_mem_ctx,
- struct tevent_context *ev,
- const DATA_BLOB in, DATA_BLOB *out)
-{
- struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
-
- *out = data_blob_null;
-
- /* and switch into the state machine */
-
- switch (spnego_state->state_position) {
- case SPNEGO_FALLBACK:
- return gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx, ev, in, out);
-
- case SPNEGO_CLIENT_START:
- case SPNEGO_CLIENT_TARG:
- return gensec_spnego_update_client(gensec_security, out_mem_ctx,
- ev, in, out);
-
- case SPNEGO_SERVER_START:
- case SPNEGO_SERVER_TARG:
- return gensec_spnego_update_server(gensec_security, out_mem_ctx,
- ev, in, out);
-
- case SPNEGO_DONE:
- /* We should not be called after we are 'done' */
- return NT_STATUS_INVALID_PARAMETER;
- }
- return NT_STATUS_INVALID_PARAMETER;
-}
-
struct gensec_spnego_update_state {
struct gensec_security *gensec;
struct spnego_state *spnego;
@@ -1390,10 +1357,42 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- status = gensec_spnego_update(gensec_security,
- state, ev,
- state->full_in,
- &spnego_state->out_frag);
+ /* and switch into the state machine */
+
+ switch (spnego_state->state_position) {
+ case SPNEGO_FALLBACK:
+ status = gensec_update_ev(spnego_state->sub_sec_security,
+ state, ev,
+ state->full_in,
+ &spnego_state->out_frag);
+ break;
+
+ case SPNEGO_CLIENT_START:
+ case SPNEGO_CLIENT_TARG:
+ status = gensec_spnego_update_client(gensec_security,
+ state, ev,
+ state->full_in,
+ &spnego_state->out_frag);
+ break;
+
+ case SPNEGO_SERVER_START:
+ case SPNEGO_SERVER_TARG:
+ status = gensec_spnego_update_server(gensec_security,
+ state, ev,
+ state->full_in,
+ &spnego_state->out_frag);
+ break;
+
+ case SPNEGO_DONE:
+ /* We should not be called after we are 'done' */
+ tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+ return tevent_req_post(req, ev);
+
+ default:
+ tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+ return tevent_req_post(req, ev);
+ }
+
if (NT_STATUS_IS_OK(status)) {
bool reset_full = true;
--
1.9.1
From 02058a6a14e0d779d722e6f781669d3a14832793 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 03:29:58 +0200
Subject: [PATCH 14/14] auth/spnego: do basic state_position checking in
gensec_spnego_update_in()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index f085885..c5681cb 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1383,14 +1383,9 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
&spnego_state->out_frag);
break;
- case SPNEGO_DONE:
- /* We should not be called after we are 'done' */
- tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
- return tevent_req_post(req, ev);
-
default:
- tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
- return tevent_req_post(req, ev);
+ smb_panic(__location__);
+ return NULL;
}
if (NT_STATUS_IS_OK(status)) {
@@ -1434,6 +1429,23 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
*full_in = data_blob_null;
+ switch (spnego_state->state_position) {
+ case SPNEGO_FALLBACK:
+ *full_in = in;
+ spnego_state->in_needed = 0;
+ return NT_STATUS_OK;
+
+ case SPNEGO_CLIENT_START:
+ case SPNEGO_CLIENT_TARG:
+ case SPNEGO_SERVER_START:
+ case SPNEGO_SERVER_TARG:
+ break;
+
+ case SPNEGO_DONE:
+ default:
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
if (spnego_state->in_needed == 0) {
size_t size = 0;
int ret;
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170628/9633a822/signature-0001.sig>
More information about the samba-technical
mailing list