[Patches] The way to remove gensec_update_ev()
Stefan Metzmacher
metze at samba.org
Mon Jul 17 12:33:03 UTC 2017
Hi,
> here's the next chunk ready for master.
>
> Please review and push:-)
The next 12 patches on top...
Please review and push:-)
Thanks!
metze
-------------- next part --------------
From 8dda7e975c73382e55702dabd31033b0ee995b0d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 16:49:57 +0200
Subject: [PATCH 01/12] auth/spnego: invert the fallback logic in
gensec_spnego_client_negTokenInit()
We should do the return first, that will simplify further changes.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 1395063..993b7f4 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -468,19 +468,18 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
spnego_state->sub_sec_security->ops->name,
principal, next, nt_errstr(status)));
- if (allow_fallback && next != NULL) {
+ if (next == NULL) {
/*
- * Pretend we never started it.
+ * A hard error without a possible fallback.
*/
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
+ TALLOC_FREE(frame);
+ return status;
}
/*
- * Hard error.
+ * Pretend we never started it.
*/
- TALLOC_FREE(frame);
- return status;
+ gensec_spnego_update_sub_abort(spnego_state);
}
DBG_WARNING("Could not find a suitable mechtype in NEG_TOKEN_INIT\n");
--
1.9.1
From 10293c145056529bcb631f8b6c6a420213730927 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 13 Jun 2017 23:43:01 +0200
Subject: [PATCH 02/12] auth/spnego: make the SPNEGO_FALLBACK continuation
completely async
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 74 +++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 62 insertions(+), 12 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 993b7f4..d11ef53 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1166,9 +1166,18 @@ struct gensec_spnego_update_state {
struct tevent_context *ev;
struct gensec_security *gensec;
struct spnego_state *spnego;
+
DATA_BLOB full_in;
struct spnego_data _spnego_in;
struct spnego_data *spnego_in;
+
+ struct {
+ bool needed;
+ DATA_BLOB in;
+ NTSTATUS status;
+ DATA_BLOB out;
+ } sub;
+
NTSTATUS status;
DATA_BLOB out;
};
@@ -1198,6 +1207,7 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
const DATA_BLOB in, TALLOC_CTX *mem_ctx,
DATA_BLOB *full_in);
static void gensec_spnego_update_pre(struct tevent_req *req);
+static void gensec_spnego_update_done(struct tevent_req *subreq);
static void gensec_spnego_update_post(struct tevent_req *req);
static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security,
TALLOC_CTX *out_mem_ctx,
@@ -1327,9 +1337,24 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- /*
- * TODO: prepare async processing here in future.
- */
+ if (state->sub.needed) {
+ struct tevent_req *subreq = NULL;
+
+ /*
+ * We may need one more roundtrip...
+ */
+ subreq = gensec_update_send(state, state->ev,
+ spnego_state->sub_sec_security,
+ state->sub.in);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq,
+ gensec_spnego_update_done,
+ req);
+ state->sub.needed = false;
+ return req;
+ }
gensec_spnego_update_post(req);
if (!tevent_req_is_in_progress(req)) {
@@ -1457,15 +1482,15 @@ static void gensec_spnego_update_pre(struct tevent_req *req)
struct tevent_context *ev = state->ev;
NTSTATUS status;
+ state->sub.needed = false;
+ state->sub.in = data_blob_null;
+ state->sub.status = NT_STATUS_INTERNAL_ERROR;
+ state->sub.out = data_blob_null;
+
if (spnego_state->state_position == SPNEGO_FALLBACK) {
- status = gensec_update_ev(spnego_state->sub_sec_security,
- state, ev,
- state->full_in,
- &spnego_state->out_frag);
- /*
- * We don't check status here.
- */
- spnego_state->out_status = status;
+ state->sub.in = state->full_in;
+ state->full_in = data_blob_null;
+ state->sub.needed = true;
return;
}
@@ -1542,6 +1567,25 @@ static void gensec_spnego_update_pre(struct tevent_req *req)
spnego_state->out_status = status;
}
+static void gensec_spnego_update_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req =
+ tevent_req_callback_data(subreq,
+ struct tevent_req);
+ struct gensec_spnego_update_state *state =
+ tevent_req_data(req,
+ struct gensec_spnego_update_state);
+ struct spnego_state *spnego_state = state->spnego;
+
+ state->sub.status = gensec_update_recv(subreq, state, &state->sub.out);
+ TALLOC_FREE(subreq);
+ if (NT_STATUS_IS_OK(state->sub.status)) {
+ spnego_state->sub_sec_ready = true;
+ }
+
+ gensec_spnego_update_post(req);
+}
+
static void gensec_spnego_update_post(struct tevent_req *req)
{
struct gensec_spnego_update_state *state =
@@ -1550,8 +1594,14 @@ static void gensec_spnego_update_post(struct tevent_req *req)
struct spnego_state *spnego_state = state->spnego;
NTSTATUS status;
+ state->sub.in = data_blob_null;
+ state->sub.needed = false;
+
if (spnego_state->state_position == SPNEGO_FALLBACK) {
- status = spnego_state->out_status;
+ status = state->sub.status;
+ spnego_state->out_frag = state->sub.out;
+ talloc_steal(spnego_state, spnego_state->out_frag.data);
+ state->sub.out = data_blob_null;
goto respond;
}
--
1.9.1
From b538c0f50fde20d7ff274b55709ac0e54ad4ce2e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 15:41:23 +0200
Subject: [PATCH 03/12] auth/spnego: move the output generation to the end of
gensec_spnego_create_negTokenInit()
This will simplify the diff of future patches.
Check with git show -w
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 86 +++++++++++++++++++++++++++-------------------------
1 file changed, 44 insertions(+), 42 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index d11ef53..a2e34f2 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -216,6 +216,9 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
const char **mechTypes = NULL;
DATA_BLOB unwrapped_out = data_blob_null;
const struct gensec_security_ops_wrapper *all_sec;
+ const char **send_mech_types = NULL;
+ struct spnego_data spnego_out;
+ bool ok;
mechTypes = gensec_security_oids(gensec_security,
out_mem_ctx, GENSEC_OID_SPNEGO);
@@ -225,10 +228,6 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
mechTypes,
GENSEC_OID_SPNEGO);
for (i=0; all_sec && all_sec[i].op; i++) {
- struct spnego_data spnego_out;
- const char **send_mech_types;
- bool ok;
-
nt_status = gensec_subcontext_start(spnego_state,
gensec_security,
&spnego_state->sub_sec_security);
@@ -290,55 +289,58 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
}
}
- spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
+ goto reply;
+ }
+ gensec_spnego_update_sub_abort(spnego_state);
- send_mech_types = gensec_security_oids_from_ops_wrapped(out_mem_ctx,
- &all_sec[i]);
+ DEBUG(10, ("Failed to setup SPNEGO negTokenInit request: %s\n", nt_errstr(nt_status)));
+ return nt_status;
- ok = spnego_write_mech_types(spnego_state,
- send_mech_types,
- &spnego_state->mech_types);
- if (!ok) {
- DEBUG(1, ("SPNEGO: Failed to write mechTypes\n"));
- return NT_STATUS_NO_MEMORY;
- }
+reply:
+ spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
- /* List the remaining mechs as options */
- spnego_out.negTokenInit.mechTypes = send_mech_types;
- spnego_out.negTokenInit.reqFlags = data_blob_null;
- spnego_out.negTokenInit.reqFlagsPadding = 0;
+ send_mech_types = gensec_security_oids_from_ops_wrapped(out_mem_ctx,
+ &all_sec[i]);
- if (spnego_state->state_position == SPNEGO_SERVER_START) {
- spnego_out.negTokenInit.mechListMIC
- = data_blob_string_const(ADS_IGNORE_PRINCIPAL);
- } else {
- spnego_out.negTokenInit.mechListMIC = data_blob_null;
- }
+ ok = spnego_write_mech_types(spnego_state,
+ send_mech_types,
+ &spnego_state->mech_types);
+ if (!ok) {
+ DEBUG(1, ("SPNEGO: Failed to write mechTypes\n"));
+ return NT_STATUS_NO_MEMORY;
+ }
- spnego_out.negTokenInit.mechToken = unwrapped_out;
+ /* List the remaining mechs as options */
+ spnego_out.negTokenInit.mechTypes = send_mech_types;
+ spnego_out.negTokenInit.reqFlags = data_blob_null;
+ spnego_out.negTokenInit.reqFlagsPadding = 0;
- if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
- DEBUG(1, ("Failed to write NEG_TOKEN_INIT\n"));
- return NT_STATUS_INVALID_PARAMETER;
- }
+ if (spnego_state->state_position == SPNEGO_SERVER_START) {
+ spnego_out.negTokenInit.mechListMIC
+ = data_blob_string_const(ADS_IGNORE_PRINCIPAL);
+ } else {
+ spnego_out.negTokenInit.mechListMIC = data_blob_null;
+ }
- /* set next state */
- spnego_state->neg_oid = all_sec[i].oid;
+ spnego_out.negTokenInit.mechToken = unwrapped_out;
- if (spnego_state->state_position == SPNEGO_SERVER_START) {
- spnego_state->state_position = SPNEGO_SERVER_START;
- spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
- } else {
- spnego_state->state_position = SPNEGO_CLIENT_TARG;
- spnego_state->expected_packet = SPNEGO_NEG_TOKEN_TARG;
- }
+ if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
+ DEBUG(1, ("Failed to write NEG_TOKEN_INIT\n"));
+ return NT_STATUS_INVALID_PARAMETER;
+ }
- return NT_STATUS_MORE_PROCESSING_REQUIRED;
+ /* set next state */
+ spnego_state->neg_oid = all_sec[i].oid;
+
+ if (spnego_state->state_position == SPNEGO_SERVER_START) {
+ spnego_state->state_position = SPNEGO_SERVER_START;
+ spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
+ } else {
+ spnego_state->state_position = SPNEGO_CLIENT_TARG;
+ spnego_state->expected_packet = SPNEGO_NEG_TOKEN_TARG;
}
- gensec_spnego_update_sub_abort(spnego_state);
- DEBUG(10, ("Failed to setup SPNEGO negTokenInit request: %s\n", nt_errstr(nt_status)));
- return nt_status;
+ return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
--
1.9.1
From 53ab052f2eb9bb028ce65df6100cfb8270d7732a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 15:44:53 +0200
Subject: [PATCH 04/12] auth/spnego: introduce an early goto reply: for the
server in gensec_spnego_create_negTokenInit()
This removes a useless indentation level and simplifies future patches.
Check with git show -w
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 87 +++++++++++++++++++++++++++-------------------------
1 file changed, 46 insertions(+), 41 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index a2e34f2..7bb6906 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -242,51 +242,56 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
continue;
}
+ if (spnego_state->state_position != SPNEGO_CLIENT_START) {
+ /*
+ * The server doesn't generate an optimistic token.
+ */
+ goto reply;
+ }
+
/* In the client, try and produce the first (optimistic) packet */
- if (spnego_state->state_position == SPNEGO_CLIENT_START) {
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
- ev,
- data_blob_null,
- &unwrapped_out);
- if (NT_STATUS_IS_OK(nt_status)) {
- spnego_state->sub_sec_ready = true;
- }
+ nt_status = gensec_update_ev(spnego_state->sub_sec_security,
+ out_mem_ctx,
+ ev,
+ data_blob_null,
+ &unwrapped_out);
+ if (NT_STATUS_IS_OK(nt_status)) {
+ spnego_state->sub_sec_ready = true;
+ }
- if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
- const char *next = NULL;
- const char *principal = NULL;
- int dbg_level = DBGLVL_WARNING;
-
- if (all_sec[i+1].op != NULL) {
- next = all_sec[i+1].op->name;
- dbg_level = DBGLVL_NOTICE;
- }
-
- if (gensec_security->target.principal != NULL) {
- principal = gensec_security->target.principal;
- } else if (gensec_security->target.service != NULL &&
- gensec_security->target.hostname != NULL)
- {
- principal = talloc_asprintf(spnego_state->sub_sec_security,
- "%s/%s",
- gensec_security->target.service,
- gensec_security->target.hostname);
- } else {
- principal = gensec_security->target.hostname;
- }
-
- DEBUG(dbg_level, ("SPNEGO(%s) creating NEG_TOKEN_INIT for %s failed (next[%s]): %s\n",
- spnego_state->sub_sec_security->ops->name,
- principal,
- next, nt_errstr(nt_status)));
+ if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
+ const char *next = NULL;
+ const char *principal = NULL;
+ int dbg_level = DBGLVL_WARNING;
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
+ if (all_sec[i+1].op != NULL) {
+ next = all_sec[i+1].op->name;
+ dbg_level = DBGLVL_NOTICE;
}
+
+ if (gensec_security->target.principal != NULL) {
+ principal = gensec_security->target.principal;
+ } else if (gensec_security->target.service != NULL &&
+ gensec_security->target.hostname != NULL)
+ {
+ principal = talloc_asprintf(spnego_state->sub_sec_security,
+ "%s/%s",
+ gensec_security->target.service,
+ gensec_security->target.hostname);
+ } else {
+ principal = gensec_security->target.hostname;
+ }
+
+ DEBUG(dbg_level, ("SPNEGO(%s) creating NEG_TOKEN_INIT for %s failed (next[%s]): %s\n",
+ spnego_state->sub_sec_security->ops->name,
+ principal,
+ next, nt_errstr(nt_status)));
+
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
+ continue;
}
goto reply;
--
1.9.1
From 0702260c64e842ba6df453c3121b49f89ceaed8c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 15:49:32 +0200
Subject: [PATCH 05/12] auth/spnego: remove one more useless indentation level
in gensec_spnego_create_negTokenInit()
Check with git show -w -U20
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 66 +++++++++++++++++++++++++---------------------------
1 file changed, 32 insertions(+), 34 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 7bb6906..3fa50cb 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -228,6 +228,10 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
mechTypes,
GENSEC_OID_SPNEGO);
for (i=0; all_sec && all_sec[i].op; i++) {
+ const char *next = NULL;
+ const char *principal = NULL;
+ int dbg_level = DBGLVL_WARNING;
+
nt_status = gensec_subcontext_start(spnego_state,
gensec_security,
&spnego_state->sub_sec_security);
@@ -251,7 +255,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
/* In the client, try and produce the first (optimistic) packet */
nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
+ out_mem_ctx,
ev,
data_blob_null,
&unwrapped_out);
@@ -259,44 +263,38 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
spnego_state->sub_sec_ready = true;
}
- if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
- const char *next = NULL;
- const char *principal = NULL;
- int dbg_level = DBGLVL_WARNING;
-
- if (all_sec[i+1].op != NULL) {
- next = all_sec[i+1].op->name;
- dbg_level = DBGLVL_NOTICE;
- }
-
- if (gensec_security->target.principal != NULL) {
- principal = gensec_security->target.principal;
- } else if (gensec_security->target.service != NULL &&
- gensec_security->target.hostname != NULL)
- {
- principal = talloc_asprintf(spnego_state->sub_sec_security,
- "%s/%s",
- gensec_security->target.service,
- gensec_security->target.hostname);
- } else {
- principal = gensec_security->target.hostname;
- }
+ if (!GENSEC_UPDATE_IS_NTERROR(nt_status)) {
+ goto reply;
+ }
- DEBUG(dbg_level, ("SPNEGO(%s) creating NEG_TOKEN_INIT for %s failed (next[%s]): %s\n",
- spnego_state->sub_sec_security->ops->name,
- principal,
- next, nt_errstr(nt_status)));
+ if (all_sec[i+1].op != NULL) {
+ next = all_sec[i+1].op->name;
+ dbg_level = DBGLVL_NOTICE;
+ }
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
+ if (gensec_security->target.principal != NULL) {
+ principal = gensec_security->target.principal;
+ } else if (gensec_security->target.service != NULL &&
+ gensec_security->target.hostname != NULL)
+ {
+ principal = talloc_asprintf(spnego_state->sub_sec_security,
+ "%s/%s",
+ gensec_security->target.service,
+ gensec_security->target.hostname);
+ } else {
+ principal = gensec_security->target.hostname;
}
- goto reply;
+ DEBUG(dbg_level, ("SPNEGO(%s) creating NEG_TOKEN_INIT for %s failed (next[%s]): %s\n",
+ spnego_state->sub_sec_security->ops->name,
+ principal,
+ next, nt_errstr(nt_status)));
+
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
}
- gensec_spnego_update_sub_abort(spnego_state);
DEBUG(10, ("Failed to setup SPNEGO negTokenInit request: %s\n", nt_errstr(nt_status)));
return nt_status;
--
1.9.1
From 92726ba89c367dfb2f3ba5bf3205fdd3c507db59 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 16:05:39 +0200
Subject: [PATCH 06/12] auth/spnego: make the debug messages in
gensec_spnego_create_negTokenInit() more useful
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 3fa50cb..9378d4d 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -285,10 +285,11 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
principal = gensec_security->target.hostname;
}
- DEBUG(dbg_level, ("SPNEGO(%s) creating NEG_TOKEN_INIT for %s failed (next[%s]): %s\n",
- spnego_state->sub_sec_security->ops->name,
- principal,
- next, nt_errstr(nt_status)));
+ DBG_PREFIX(dbg_level, (
+ "%s: creating NEG_TOKEN_INIT for %s failed "
+ "(next[%s]): %s\n",
+ spnego_state->sub_sec_security->ops->name,
+ principal, next, nt_errstr(nt_status)));
/*
* Pretend we never started it
@@ -296,7 +297,8 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
gensec_spnego_update_sub_abort(spnego_state);
}
- DEBUG(10, ("Failed to setup SPNEGO negTokenInit request: %s\n", nt_errstr(nt_status)));
+ DBG_WARNING("Failed to setup SPNEGO negTokenInit request: %s\n",
+ nt_errstr(nt_status));
return nt_status;
reply:
@@ -309,7 +311,7 @@ reply:
send_mech_types,
&spnego_state->mech_types);
if (!ok) {
- DEBUG(1, ("SPNEGO: Failed to write mechTypes\n"));
+ DBG_ERR("Failed to write mechTypes\n");
return NT_STATUS_NO_MEMORY;
}
@@ -328,8 +330,8 @@ reply:
spnego_out.negTokenInit.mechToken = unwrapped_out;
if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
- DEBUG(1, ("Failed to write NEG_TOKEN_INIT\n"));
- return NT_STATUS_INVALID_PARAMETER;
+ DBG_ERR("Failed to write NEG_TOKEN_INIT\n");
+ return NT_STATUS_INVALID_PARAMETER;
}
/* set next state */
--
1.9.1
From 28de2c5563a3c3a851131913669a6de2e2706595 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 16:08:05 +0200
Subject: [PATCH 07/12] auth/spnego: rename 'nt_status' to 'status' in
gensec_spnego_create_negTokenInit()
This makes future diffs smaller.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 9378d4d..4ad3465 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -212,7 +212,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
DATA_BLOB *out)
{
int i;
- NTSTATUS nt_status = NT_STATUS_INVALID_PARAMETER;
+ NTSTATUS status = NT_STATUS_INVALID_PARAMETER;
const char **mechTypes = NULL;
DATA_BLOB unwrapped_out = data_blob_null;
const struct gensec_security_ops_wrapper *all_sec;
@@ -232,16 +232,16 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
const char *principal = NULL;
int dbg_level = DBGLVL_WARNING;
- nt_status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(nt_status)) {
- return nt_status;
+ status = gensec_subcontext_start(spnego_state,
+ gensec_security,
+ &spnego_state->sub_sec_security);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
/* select the sub context */
- nt_status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
- all_sec[i].op);
- if (!NT_STATUS_IS_OK(nt_status)) {
+ status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
+ all_sec[i].op);
+ if (!NT_STATUS_IS_OK(status)) {
gensec_spnego_update_sub_abort(spnego_state);
continue;
}
@@ -254,16 +254,16 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
}
/* In the client, try and produce the first (optimistic) packet */
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
+ status = gensec_update_ev(spnego_state->sub_sec_security,
out_mem_ctx,
ev,
data_blob_null,
&unwrapped_out);
- if (NT_STATUS_IS_OK(nt_status)) {
+ if (NT_STATUS_IS_OK(status)) {
spnego_state->sub_sec_ready = true;
}
- if (!GENSEC_UPDATE_IS_NTERROR(nt_status)) {
+ if (!GENSEC_UPDATE_IS_NTERROR(status)) {
goto reply;
}
@@ -289,7 +289,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
"%s: creating NEG_TOKEN_INIT for %s failed "
"(next[%s]): %s\n",
spnego_state->sub_sec_security->ops->name,
- principal, next, nt_errstr(nt_status)));
+ principal, next, nt_errstr(status)));
/*
* Pretend we never started it
@@ -298,8 +298,8 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
}
DBG_WARNING("Failed to setup SPNEGO negTokenInit request: %s\n",
- nt_errstr(nt_status));
- return nt_status;
+ nt_errstr(status));
+ return status;
reply:
spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
--
1.9.1
From 853336e627e60c7d5a1e33c041c3d75c5b726630 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 16:16:35 +0200
Subject: [PATCH 08/12] auth/spnego: add more error checking to
gensec_spnego_create_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 4ad3465..b6c67fc 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -222,11 +222,20 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
mechTypes = gensec_security_oids(gensec_security,
out_mem_ctx, GENSEC_OID_SPNEGO);
+ if (mechTypes == NULL) {
+ DBG_WARNING("gensec_security_oids() failed\n");
+ return NT_STATUS_NO_MEMORY;
+ }
all_sec = gensec_security_by_oid_list(gensec_security,
out_mem_ctx,
mechTypes,
GENSEC_OID_SPNEGO);
+ if (all_sec == NULL) {
+ DBG_WARNING("gensec_security_by_oid_list() failed\n");
+ return NT_STATUS_NO_MEMORY;
+ }
+
for (i=0; all_sec && all_sec[i].op; i++) {
const char *next = NULL;
const char *principal = NULL;
@@ -306,6 +315,10 @@ reply:
send_mech_types = gensec_security_oids_from_ops_wrapped(out_mem_ctx,
&all_sec[i]);
+ if (send_mech_types == NULL) {
+ DBG_WARNING("gensec_security_oids_from_ops_wrapped() failed\n");
+ return NT_STATUS_NO_MEMORY;
+ }
ok = spnego_write_mech_types(spnego_state,
send_mech_types,
--
1.9.1
From 6761aa992a0e67010e56e22a1adf71dce7cb9144 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 16:20:59 +0200
Subject: [PATCH 09/12] auth/spnego: introduce an early return in
gensec_spnego_create_negTokenInit()
This avoids print two debug message for the same failure.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index b6c67fc..0be30ce 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -212,7 +212,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
DATA_BLOB *out)
{
int i;
- NTSTATUS status = NT_STATUS_INVALID_PARAMETER;
+ NTSTATUS status;
const char **mechTypes = NULL;
DATA_BLOB unwrapped_out = data_blob_null;
const struct gensec_security_ops_wrapper *all_sec;
@@ -300,15 +300,21 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
spnego_state->sub_sec_security->ops->name,
principal, next, nt_errstr(status)));
+ if (next == NULL) {
+ /*
+ * A hard error without a possible fallback.
+ */
+ return status;
+ }
+
/*
* Pretend we never started it
*/
gensec_spnego_update_sub_abort(spnego_state);
}
- DBG_WARNING("Failed to setup SPNEGO negTokenInit request: %s\n",
- nt_errstr(status));
- return status;
+ DBG_WARNING("Failed to setup SPNEGO negTokenInit request\n");
+ return NT_STATUS_INVALID_PARAMETER;
reply:
spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
--
1.9.1
From a6668a580b4ccf422ac96f1fd0819c3938ec0f93 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 13 Jul 2017 16:26:42 +0200
Subject: [PATCH 10/12] auth/spnego: use better variable names in
gensec_spnego_create_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 0be30ce..31198ad 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -211,11 +211,12 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
struct tevent_context *ev,
DATA_BLOB *out)
{
- int i;
NTSTATUS status;
const char **mechTypes = NULL;
DATA_BLOB unwrapped_out = data_blob_null;
+ size_t all_idx = 0;
const struct gensec_security_ops_wrapper *all_sec;
+ const struct gensec_security_ops_wrapper *cur_sec = NULL;
const char **send_mech_types = NULL;
struct spnego_data spnego_out;
bool ok;
@@ -236,11 +237,13 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
return NT_STATUS_NO_MEMORY;
}
- for (i=0; all_sec && all_sec[i].op; i++) {
+ for (; all_sec[all_idx].op != NULL; all_idx++) {
const char *next = NULL;
const char *principal = NULL;
int dbg_level = DBGLVL_WARNING;
+ cur_sec = &all_sec[all_idx];
+
status = gensec_subcontext_start(spnego_state,
gensec_security,
&spnego_state->sub_sec_security);
@@ -249,7 +252,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
}
/* select the sub context */
status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
- all_sec[i].op);
+ cur_sec->op);
if (!NT_STATUS_IS_OK(status)) {
gensec_spnego_update_sub_abort(spnego_state);
continue;
@@ -276,8 +279,8 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
goto reply;
}
- if (all_sec[i+1].op != NULL) {
- next = all_sec[i+1].op->name;
+ if (cur_sec[1].op != NULL) {
+ next = cur_sec[1].op->name;
dbg_level = DBGLVL_NOTICE;
}
@@ -296,8 +299,7 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
DBG_PREFIX(dbg_level, (
"%s: creating NEG_TOKEN_INIT for %s failed "
- "(next[%s]): %s\n",
- spnego_state->sub_sec_security->ops->name,
+ "(next[%s]): %s\n", cur_sec->op->name,
principal, next, nt_errstr(status)));
if (next == NULL) {
@@ -320,7 +322,7 @@ reply:
spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
send_mech_types = gensec_security_oids_from_ops_wrapped(out_mem_ctx,
- &all_sec[i]);
+ cur_sec);
if (send_mech_types == NULL) {
DBG_WARNING("gensec_security_oids_from_ops_wrapped() failed\n");
return NT_STATUS_NO_MEMORY;
@@ -353,9 +355,14 @@ reply:
return NT_STATUS_INVALID_PARAMETER;
}
- /* set next state */
- spnego_state->neg_oid = all_sec[i].oid;
+ /*
+ * Note that 'cur_sec' is temporary memory, but
+ * cur_sec->oid points to a const string in the
+ * backends gensec_security_ops structure.
+ */
+ spnego_state->neg_oid = cur_sec->oid;
+ /* set next state */
if (spnego_state->state_position == SPNEGO_SERVER_START) {
spnego_state->state_position = SPNEGO_SERVER_START;
spnego_state->expected_packet = SPNEGO_NEG_TOKEN_INIT;
--
1.9.1
From 337c5e5f1cec1dd887db58b84c90aca57d2bef26 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 01:52:09 +0200
Subject: [PATCH 11/12] auth/spnego: split gensec_spnego_create_negTokenInit()
into subfunctions
This adds and used gensec_spnego_neg_loop() abstraction, which
abstracts start, step and finish hooks.
The start hook does the initial processing on the incoming paket and
may starts the first possible subcontext. It indicates that
gensec_update() is required on the subcontext by returning
NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
caller should treat 'in_next' as const and don't attempt to free the
content. NT_STATUS_OK indicates the finish hook should be invoced
directly withing the need of gensec_update() on the subcontext.
Every other error indicated an error that's returned to the caller.
The step hook processes the result of a failed gensec_update() and
can decide to ignore a failure and continue the negotiation by
setting up the next possible subcontext. It indicates that
gensec_update() is required on the subcontext by returning
NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
caller should treat 'in_next' as const and don't attempt to free the
content. NT_STATUS_OK indicates the finish hook should be invoced
directly withing the need of gensec_update() on the subcontext.
Every other error indicated an error that's returned to the caller.
The finish hook processes the result of a successful gensec_update()
(NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED). It forms the
response pdu that will be returned from the toplevel gensec_update()
together with NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED. It
may also alters the state machine to prepare receiving the next pdu
from the peer.
This is the start of using this abstraction for the initial client or server
start with on empty input token from the peer.
This abstraction will be applied to all 4 other spnego states,
gensec_spnego_{client,server}_negToken{Init,Targ}() in the following
commits.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 325 +++++++++++++++++++++++++++++++++++++++------------
1 file changed, 252 insertions(+), 73 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 31198ad..4d0d80b 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -47,6 +47,74 @@ enum spnego_state_position {
SPNEGO_DONE
};
+struct spnego_state;
+struct spnego_neg_ops;
+struct spnego_neg_state;
+
+struct spnego_neg_state {
+ const struct spnego_neg_ops *ops;
+ const struct gensec_security_ops_wrapper *all_sec;
+ size_t all_idx;
+ const char * const *mech_types;
+ size_t mech_idx;
+};
+
+struct spnego_neg_ops {
+ const char *name;
+ /*
+ * The start hook does the initial processing on the incoming paket and
+ * may starts the first possible subcontext. It indicates that
+ * gensec_update() is required on the subcontext by returning
+ * NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
+ * 'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
+ * caller should treat 'in_next' as const and don't attempt to free the
+ * content. NT_STATUS_OK indicates the finish hook should be invoced
+ * directly withing the need of gensec_update() on the subcontext.
+ * Every other error indicated an error that's returned to the caller.
+ */
+ NTSTATUS (*start_fn)(struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ TALLOC_CTX *in_mem_ctx,
+ DATA_BLOB *in_next);
+ /*
+ * The step hook processes the result of a failed gensec_update() and
+ * can decide to ignore a failure and continue the negotiation by
+ * setting up the next possible subcontext. It indicates that
+ * gensec_update() is required on the subcontext by returning
+ * NT_STATUS_MORE_PROCESSING_REQUIRED and return something useful in
+ * 'in_next'. Note that 'in_mem_ctx' is just passed as a hint, the
+ * caller should treat 'in_next' as const and don't attempt to free the
+ * content. NT_STATUS_OK indicates the finish hook should be invoced
+ * directly withing the need of gensec_update() on the subcontext.
+ * Every other error indicated an error that's returned to the caller.
+ */
+ NTSTATUS (*step_fn)(struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ NTSTATUS last_status,
+ TALLOC_CTX *in_mem_ctx,
+ DATA_BLOB *in_next);
+ /*
+ * The finish hook processes the result of a successful gensec_update()
+ * (NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED). It forms the
+ * response pdu that will be returned from the toplevel gensec_update()
+ * together with NT_STATUS_OK or NT_STATUS_MORE_PROCESSING_REQUIRED. It
+ * may also alters the state machine to prepare receiving the next pdu
+ * from the peer.
+ */
+ NTSTATUS (*finish_fn)(struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ NTSTATUS sub_status,
+ const DATA_BLOB sub_out,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *out);
+};
+
struct spnego_state {
enum spnego_message_type expected_packet;
enum spnego_state_position state_position;
@@ -77,6 +145,74 @@ struct spnego_state {
NTSTATUS out_status;
};
+static struct spnego_neg_state *gensec_spnego_neg_state(TALLOC_CTX *mem_ctx,
+ const const struct spnego_neg_ops *ops)
+{
+ struct spnego_neg_state *n = NULL;
+
+ n = talloc_zero(mem_ctx, struct spnego_neg_state);
+ if (n == NULL) {
+ return NULL;
+ }
+ n->ops = ops;
+
+ return n;
+}
+
+static NTSTATUS gensec_spnego_neg_loop(struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ const const struct spnego_neg_ops *ops,
+ struct tevent_context *ev,
+ struct spnego_data *spnego_in,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *out)
+{
+ struct spnego_neg_state *n = NULL;
+ NTSTATUS status;
+ DATA_BLOB sub_in = data_blob_null;
+ DATA_BLOB sub_out = data_blob_null;
+
+ *out = data_blob_null;
+
+ n = gensec_spnego_neg_state(out_mem_ctx, ops);
+ if (n == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ status = n->ops->start_fn(gensec_security, spnego_state, n,
+ spnego_in, n, &sub_in);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ TALLOC_FREE(n);
+ return status;
+ }
+
+ while (NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+ status = gensec_update_ev(spnego_state->sub_sec_security,
+ n, ev, sub_in, &sub_out);
+ sub_in = data_blob_null;
+ if (NT_STATUS_IS_OK(status)) {
+ spnego_state->sub_sec_ready = true;
+ }
+ if (!GENSEC_UPDATE_IS_NTERROR(status)) {
+ break;
+ }
+ sub_out = data_blob_null;
+
+ status = n->ops->step_fn(gensec_security, spnego_state, n,
+ spnego_in, status, n, &sub_in);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ TALLOC_FREE(n);
+ return status;
+ }
+ }
+
+ status = n->ops->finish_fn(gensec_security, spnego_state, n,
+ spnego_in, status, sub_out,
+ out_mem_ctx, out);
+ TALLOC_FREE(n);
+ return status;
+}
+
static void gensec_spnego_update_sub_abort(struct spnego_state *spnego_state)
{
spnego_state->sub_sec_ready = false;
@@ -201,83 +337,54 @@ static NTSTATUS gensec_spnego_server_try_fallback(struct gensec_security *gensec
return NT_STATUS_INVALID_PARAMETER;
}
-/** create a negTokenInit
- *
- * This is the same packet, no matter if the client or server sends it first, but it is always the first packet
-*/
-static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec_security,
- struct spnego_state *spnego_state,
- TALLOC_CTX *out_mem_ctx,
- struct tevent_context *ev,
- DATA_BLOB *out)
+static NTSTATUS gensec_spnego_create_negTokenInit_start(
+ struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ TALLOC_CTX *in_mem_ctx,
+ DATA_BLOB *in_next)
{
NTSTATUS status;
- const char **mechTypes = NULL;
- DATA_BLOB unwrapped_out = data_blob_null;
- size_t all_idx = 0;
- const struct gensec_security_ops_wrapper *all_sec;
- const struct gensec_security_ops_wrapper *cur_sec = NULL;
- const char **send_mech_types = NULL;
- struct spnego_data spnego_out;
- bool ok;
- mechTypes = gensec_security_oids(gensec_security,
- out_mem_ctx, GENSEC_OID_SPNEGO);
- if (mechTypes == NULL) {
- DBG_WARNING("gensec_security_oids() failed\n");
+ n->mech_idx = 0;
+ n->mech_types = gensec_security_oids(gensec_security, n,
+ GENSEC_OID_SPNEGO);
+ if (n->mech_types == NULL) {
return NT_STATUS_NO_MEMORY;
}
- all_sec = gensec_security_by_oid_list(gensec_security,
- out_mem_ctx,
- mechTypes,
- GENSEC_OID_SPNEGO);
- if (all_sec == NULL) {
- DBG_WARNING("gensec_security_by_oid_list() failed\n");
- return NT_STATUS_NO_MEMORY;
+ n->all_idx = 0;
+ n->all_sec = gensec_security_by_oid_list(gensec_security,
+ n, n->mech_types,
+ GENSEC_OID_SPNEGO);
+ if (n->all_sec == NULL) {
+ status = NT_STATUS_INVALID_PARAMETER;
+ DBG_DEBUG("Failed to setup SPNEGO negTokenInit request: "
+ "%s\n", nt_errstr(status));
+ return status;
}
- for (; all_sec[all_idx].op != NULL; all_idx++) {
+ return n->ops->step_fn(gensec_security, spnego_state, n,
+ spnego_in, NT_STATUS_OK, in_mem_ctx, in_next);
+}
+
+static NTSTATUS gensec_spnego_create_negTokenInit_step(
+ struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ NTSTATUS last_status,
+ TALLOC_CTX *in_mem_ctx,
+ DATA_BLOB *in_next)
+{
+ if (!NT_STATUS_IS_OK(last_status)) {
+ const struct gensec_security_ops_wrapper *cur_sec =
+ &n->all_sec[n->all_idx];
const char *next = NULL;
const char *principal = NULL;
int dbg_level = DBGLVL_WARNING;
-
- cur_sec = &all_sec[all_idx];
-
- status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(status)) {
- return status;
- }
- /* select the sub context */
- status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
- cur_sec->op);
- if (!NT_STATUS_IS_OK(status)) {
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
- }
-
- if (spnego_state->state_position != SPNEGO_CLIENT_START) {
- /*
- * The server doesn't generate an optimistic token.
- */
- goto reply;
- }
-
- /* In the client, try and produce the first (optimistic) packet */
- status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
- ev,
- data_blob_null,
- &unwrapped_out);
- if (NT_STATUS_IS_OK(status)) {
- spnego_state->sub_sec_ready = true;
- }
-
- if (!GENSEC_UPDATE_IS_NTERROR(status)) {
- goto reply;
- }
+ NTSTATUS status = last_status;
if (cur_sec[1].op != NULL) {
next = cur_sec[1].op->name;
@@ -313,23 +420,72 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
* Pretend we never started it
*/
gensec_spnego_update_sub_abort(spnego_state);
+
+ /*
+ * And try the next one...
+ */
+ n->all_idx += 1;
+ }
+
+ for (; n->all_sec[n->all_idx].op != NULL; n->all_idx++) {
+ const struct gensec_security_ops_wrapper *cur_sec =
+ &n->all_sec[n->all_idx];
+ NTSTATUS status;
+
+ status = gensec_subcontext_start(spnego_state,
+ gensec_security,
+ &spnego_state->sub_sec_security);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+
+ /* select the sub context */
+ status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
+ cur_sec->op);
+ if (!NT_STATUS_IS_OK(status)) {
+ gensec_spnego_update_sub_abort(spnego_state);
+ continue;
+ }
+
+ /* In the client, try and produce the first (optimistic) packet */
+ if (spnego_state->state_position == SPNEGO_CLIENT_START) {
+ *in_next = data_blob_null;
+ return NT_STATUS_MORE_PROCESSING_REQUIRED;
+ }
+
+ *in_next = data_blob_null;
+ return NT_STATUS_OK;
}
DBG_WARNING("Failed to setup SPNEGO negTokenInit request\n");
return NT_STATUS_INVALID_PARAMETER;
+}
+
+static NTSTATUS gensec_spnego_create_negTokenInit_finish(
+ struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ NTSTATUS sub_status,
+ const DATA_BLOB sub_out,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *out)
+{
+ const struct gensec_security_ops_wrapper *cur_sec =
+ &n->all_sec[n->all_idx];
+ struct spnego_data spnego_out;
+ bool ok;
-reply:
spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
- send_mech_types = gensec_security_oids_from_ops_wrapped(out_mem_ctx,
- cur_sec);
- if (send_mech_types == NULL) {
+ n->mech_types = gensec_security_oids_from_ops_wrapped(n, cur_sec);
+ if (n->mech_types == NULL) {
DBG_WARNING("gensec_security_oids_from_ops_wrapped() failed\n");
return NT_STATUS_NO_MEMORY;
}
ok = spnego_write_mech_types(spnego_state,
- send_mech_types,
+ n->mech_types,
&spnego_state->mech_types);
if (!ok) {
DBG_ERR("Failed to write mechTypes\n");
@@ -337,7 +493,7 @@ reply:
}
/* List the remaining mechs as options */
- spnego_out.negTokenInit.mechTypes = send_mech_types;
+ spnego_out.negTokenInit.mechTypes = n->mech_types;
spnego_out.negTokenInit.reqFlags = data_blob_null;
spnego_out.negTokenInit.reqFlagsPadding = 0;
@@ -348,7 +504,7 @@ reply:
spnego_out.negTokenInit.mechListMIC = data_blob_null;
}
- spnego_out.negTokenInit.mechToken = unwrapped_out;
+ spnego_out.negTokenInit.mechToken = sub_out;
if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
DBG_ERR("Failed to write NEG_TOKEN_INIT\n");
@@ -374,6 +530,29 @@ reply:
return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
+static const struct spnego_neg_ops gensec_spnego_create_negTokenInit_ops = {
+ .name = "create_negTokenInit",
+ .start_fn = gensec_spnego_create_negTokenInit_start,
+ .step_fn = gensec_spnego_create_negTokenInit_step,
+ .finish_fn = gensec_spnego_create_negTokenInit_finish,
+};
+
+/** create a negTokenInit
+ *
+ * This is the same packet, no matter if the client or server sends it first, but it is always the first packet
+*/
+static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ TALLOC_CTX *out_mem_ctx,
+ struct tevent_context *ev,
+ DATA_BLOB *out)
+{
+ struct spnego_data *spnego_in = NULL;
+ return gensec_spnego_neg_loop(gensec_security, spnego_state,
+ &gensec_spnego_create_negTokenInit_ops,
+ ev, spnego_in, out_mem_ctx, out);
+}
+
static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
struct spnego_state *spnego_state,
struct tevent_context *ev,
--
1.9.1
From 423265b20b3e0f598f55f1deec1ab3908d014f22 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 14 Jun 2017 12:59:43 +0200
Subject: [PATCH 12/12] auth/spnego: split gensec_spnego_client_negTokenInit()
into subfunctions
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 178 +++++++++++++++++++++++++++++++--------------------
1 file changed, 109 insertions(+), 69 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 4d0d80b..8c5344e 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -553,25 +553,16 @@ static NTSTATUS gensec_spnego_create_negTokenInit(struct gensec_security *gensec
ev, spnego_in, out_mem_ctx, out);
}
-static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
- struct spnego_state *spnego_state,
- struct tevent_context *ev,
- struct spnego_data *spnego_in,
- TALLOC_CTX *out_mem_ctx,
- DATA_BLOB *out)
+static NTSTATUS gensec_spnego_client_negTokenInit_start(
+ struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ TALLOC_CTX *in_mem_ctx,
+ DATA_BLOB *in_next)
{
- TALLOC_CTX *frame = talloc_stackframe();
- DATA_BLOB sub_out = data_blob_null;
const char *tp = NULL;
- const char * const *mech_types = NULL;
- size_t all_idx = 0;
- const struct gensec_security_ops_wrapper *all_sec = NULL;
- struct spnego_data spnego_out;
- const char *my_mechs[] = {NULL, NULL};
NTSTATUS status;
- bool ok;
-
- *out = data_blob_null;
/* The server offers a list of mechanisms */
@@ -583,63 +574,44 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
}
}
- mech_types = spnego_in->negTokenInit.mechTypes;
- if (mech_types == NULL) {
- TALLOC_FREE(frame);
+ n->mech_idx = 0;
+ n->mech_types = spnego_in->negTokenInit.mechTypes;
+ if (n->mech_types == NULL) {
return NT_STATUS_INVALID_PARAMETER;
}
- all_sec = gensec_security_by_oid_list(gensec_security,
- frame, mech_types,
- GENSEC_OID_SPNEGO);
- if (all_sec == NULL) {
- DBG_WARNING("gensec_security_by_oid_list() failed\n");
- TALLOC_FREE(frame);
- return NT_STATUS_INVALID_PARAMETER;
+ n->all_idx = 0;
+ n->all_sec = gensec_security_by_oid_list(gensec_security,
+ n, n->mech_types,
+ GENSEC_OID_SPNEGO);
+ if (n->all_sec == NULL) {
+ status = NT_STATUS_INVALID_PARAMETER;
+ DBG_DEBUG("Failed to setup SPNEGO negTokenInit request: "
+ "%s\n", nt_errstr(status));
+ return status;
}
- for (; all_sec[all_idx].op; all_idx++) {
+ return n->ops->step_fn(gensec_security, spnego_state, n,
+ spnego_in, NT_STATUS_OK, in_mem_ctx, in_next);
+}
+
+static NTSTATUS gensec_spnego_client_negTokenInit_step(
+ struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ NTSTATUS last_status,
+ TALLOC_CTX *in_mem_ctx,
+ DATA_BLOB *in_next)
+{
+ if (!NT_STATUS_IS_OK(last_status)) {
const struct gensec_security_ops_wrapper *cur_sec =
- &all_sec[all_idx];
+ &n->all_sec[n->all_idx];
const char *next = NULL;
const char *principal = NULL;
int dbg_level = DBGLVL_WARNING;
bool allow_fallback = false;
-
- status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(frame);
- return status;
- }
-
- /* select the sub context */
- status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
- cur_sec->op);
- if (!NT_STATUS_IS_OK(status)) {
- /*
- * Pretend we never started it.
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
- }
-
- spnego_state->neg_oid = cur_sec->oid;
-
- /*
- * As client we don't use an optimitic token from the server.
- */
- status = gensec_update_ev(spnego_state->sub_sec_security,
- frame, ev, data_blob_null, &sub_out);
- if (NT_STATUS_IS_OK(status)) {
- spnego_state->sub_sec_ready = true;
- }
-
- if (!GENSEC_UPDATE_IS_NTERROR(status)) {
- /* OK or MORE_PROCESSING_REQUIRED */
- goto reply;
- }
+ NTSTATUS status = last_status;
/*
* it is likely that a NULL input token will
@@ -684,7 +656,6 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
/*
* A hard error without a possible fallback.
*/
- TALLOC_FREE(frame);
return status;
}
@@ -692,13 +663,66 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
* Pretend we never started it.
*/
gensec_spnego_update_sub_abort(spnego_state);
+
+ /*
+ * And try the next one...
+ */
+ n->all_idx += 1;
+ }
+
+ for (; n->all_sec[n->all_idx].op != NULL; n->all_idx++) {
+ const struct gensec_security_ops_wrapper *cur_sec =
+ &n->all_sec[n->all_idx];
+ NTSTATUS status;
+
+ status = gensec_subcontext_start(spnego_state,
+ gensec_security,
+ &spnego_state->sub_sec_security);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+
+ /* select the sub context */
+ status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
+ cur_sec->op);
+ if (!NT_STATUS_IS_OK(status)) {
+ gensec_spnego_update_sub_abort(spnego_state);
+ continue;
+ }
+
+ /*
+ * Note that 'cur_sec' is temporary memory, but
+ * cur_sec->oid points to a const string in the
+ * backends gensec_security_ops structure.
+ */
+ spnego_state->neg_oid = cur_sec->oid;
+
+ /*
+ * As client we don't use an optimitic token from the server.
+ * But try to produce one for the server.
+ */
+ *in_next = data_blob_null;
+ return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
DBG_WARNING("Could not find a suitable mechtype in NEG_TOKEN_INIT\n");
- TALLOC_FREE(frame);
return NT_STATUS_INVALID_PARAMETER;
+}
+
+static NTSTATUS gensec_spnego_client_negTokenInit_finish(
+ struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct spnego_neg_state *n,
+ struct spnego_data *spnego_in,
+ NTSTATUS sub_status,
+ const DATA_BLOB sub_out,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *out)
+{
+ struct spnego_data spnego_out;
+ const char *my_mechs[] = {NULL, NULL};
+ bool ok;
- reply:
my_mechs[0] = spnego_state->neg_oid;
/* compose reply */
spnego_out.type = SPNEGO_NEG_TOKEN_INIT;
@@ -710,7 +734,6 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
DBG_ERR("Failed to write SPNEGO reply to NEG_TOKEN_INIT\n");
- TALLOC_FREE(frame);
return NT_STATUS_INVALID_PARAMETER;
}
@@ -719,7 +742,6 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
&spnego_state->mech_types);
if (!ok) {
DBG_ERR("failed to write mechTypes\n");
- TALLOC_FREE(frame);
return NT_STATUS_NO_MEMORY;
}
@@ -727,10 +749,28 @@ static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec
spnego_state->expected_packet = SPNEGO_NEG_TOKEN_TARG;
spnego_state->state_position = SPNEGO_CLIENT_TARG;
- TALLOC_FREE(frame);
return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
+static const struct spnego_neg_ops gensec_spnego_client_negTokenInit_ops = {
+ .name = "client_negTokenInit",
+ .start_fn = gensec_spnego_client_negTokenInit_start,
+ .step_fn = gensec_spnego_client_negTokenInit_step,
+ .finish_fn = gensec_spnego_client_negTokenInit_finish,
+};
+
+static NTSTATUS gensec_spnego_client_negTokenInit(struct gensec_security *gensec_security,
+ struct spnego_state *spnego_state,
+ struct tevent_context *ev,
+ struct spnego_data *spnego_in,
+ TALLOC_CTX *out_mem_ctx,
+ DATA_BLOB *out)
+{
+ return gensec_spnego_neg_loop(gensec_security, spnego_state,
+ &gensec_spnego_client_negTokenInit_ops,
+ ev, spnego_in, out_mem_ctx, out);
+}
+
static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec_security,
struct spnego_state *spnego_state,
struct tevent_context *ev,
--
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/20170717/acdb65fb/signature.sig>
More information about the samba-technical
mailing list