[Patches] The way to remove gensec_update_ev() part6
Stefan Metzmacher
metze at samba.org
Thu Jul 20 22:25:32 UTC 2017
Hi,
here's the next part...
Most of them are already reviewed by Andreas.
They also passed private autobuilds a few times.
Please review and push:-)
Thanks!
metze
-------------- next part --------------
From 0fc7d8ad45ee5c29c0970a2a0f09b2033b371f0d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 10:54:54 +0200
Subject: [PATCH 01/24] auth/spnego: remove unused indentation level from
gensec_spnego_parse_negTokenInit()
gensec_spnego_parse_negTokenInit() is only used as server now.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 128 ++++++++++++++++++++++++++-------------------------
1 file changed, 65 insertions(+), 63 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 80f0af3..f943463 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -221,6 +221,11 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
DATA_BLOB unwrapped_in = data_blob_null;
bool ok;
const struct gensec_security_ops_wrapper *all_sec = NULL;
+ uint32_t j;
+
+ if (spnego_state->state_position != SPNEGO_SERVER_START) {
+ return NT_STATUS_INTERNAL_ERROR;
+ }
if (spnego_in->type != SPNEGO_NEG_TOKEN_INIT) {
return NT_STATUS_INTERNAL_ERROR;
@@ -249,80 +254,77 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
return NT_STATUS_NO_MEMORY;
}
- if (spnego_state->state_position == SPNEGO_SERVER_START) {
- uint32_t j;
- for (j=0; mechType && mechType[j]; j++) {
- for (i=0; all_sec && all_sec[i].op; i++) {
- if (strcmp(mechType[j], all_sec[i].oid) != 0) {
- continue;
- }
-
- nt_status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(nt_status)) {
- return nt_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)) {
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- break;
- }
-
- if (j > 0) {
- /* no optimistic token */
- spnego_state->neg_oid = all_sec[i].oid;
- *unwrapped_out = data_blob_null;
- nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- /*
- * Indicate the downgrade and request a
- * mic.
- */
- spnego_state->downgraded = true;
- spnego_state->mic_requested = true;
- break;
- }
-
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
- 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)) {
-
- DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed to parse contents: %s\n",
- spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
+ for (j=0; mechType && mechType[j]; j++) {
+ for (i=0; all_sec && all_sec[i].op; i++) {
+ if (strcmp(mechType[j], all_sec[i].oid) != 0) {
+ continue;
+ }
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- break;
- }
+ nt_status = gensec_subcontext_start(spnego_state,
+ gensec_security,
+ &spnego_state->sub_sec_security);
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ return nt_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)) {
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
+ break;
+ }
+ if (j > 0) {
+ /* no optimistic token */
spnego_state->neg_oid = all_sec[i].oid;
+ *unwrapped_out = data_blob_null;
+ nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+ /*
+ * Indicate the downgrade and request a
+ * mic.
+ */
+ spnego_state->downgraded = true;
+ spnego_state->mic_requested = true;
break;
}
- if (spnego_state->sub_sec_security) {
+
+ nt_status = gensec_update_ev(spnego_state->sub_sec_security,
+ out_mem_ctx,
+ 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)) {
+
+ DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed to parse contents: %s\n",
+ spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
+
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
break;
}
- }
- if (!spnego_state->sub_sec_security) {
- DEBUG(1, ("SPNEGO: Could not find a suitable mechtype in NEG_TOKEN_INIT\n"));
- return NT_STATUS_INVALID_PARAMETER;
+ spnego_state->neg_oid = all_sec[i].oid;
+ break;
+ }
+ if (spnego_state->sub_sec_security) {
+ break;
}
}
+ if (!spnego_state->sub_sec_security) {
+ DEBUG(1, ("SPNEGO: Could not find a suitable mechtype in NEG_TOKEN_INIT\n"));
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
/* Having tried any optimistic token from the client (if we
* were the server), if we didn't get anywhere, walk our list
* in our preference order */
--
1.9.1
From e5b079697e7420f967f8b56f51edc4c1ad5630ce Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 10:57:52 +0200
Subject: [PATCH 02/24] auth/spnego: remove dead code from
gensec_spnego_parse_negTokenInit()
Check with git show -U15
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 85 ----------------------------------------------------
1 file changed, 85 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index f943463..17cf911 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -325,91 +325,6 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
return NT_STATUS_INVALID_PARAMETER;
}
- /* Having tried any optimistic token from the client (if we
- * were the server), if we didn't get anywhere, walk our list
- * in our preference order */
- unwrapped_in = data_blob_null;
-
- if (!spnego_state->sub_sec_security) {
- for (i=0; all_sec && all_sec[i].op; i++) {
- nt_status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(nt_status)) {
- return nt_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)) {
- /*
- * Pretend we never started it.
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
- }
-
- spnego_state->neg_oid = all_sec[i].oid;
-
- /* only get the helping start blob for the first OID */
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
- ev,
- unwrapped_in,
- 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
- * we are in the client, we want the first
- * update packet to be able to abort the use
- * of this mech */
- if (spnego_state->state_position != SPNEGO_SERVER_START) {
- if (NT_STATUS_EQUAL(nt_status, NT_STATUS_INVALID_PARAMETER) ||
- NT_STATUS_EQUAL(nt_status, NT_STATUS_NO_LOGON_SERVERS) ||
- NT_STATUS_EQUAL(nt_status, NT_STATUS_TIME_DIFFERENCE_AT_DC) ||
- NT_STATUS_EQUAL(nt_status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO)) {
- 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)));
-
- /*
- * Pretend we never started it.
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
- }
- }
-
- break;
- }
- }
-
if (spnego_state->sub_sec_security) {
/* it is likely that a NULL input token will
* not be liked by most server mechs, but this
--
1.9.1
From f7d58ff91ce75753747e61fd90bef1f8daf902e0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 11:03:37 +0200
Subject: [PATCH 03/24] auth/spnego: remove one more useless indentation level
from gensec_spnego_parse_negTokenInit()
Check with 'git show -w -U45' and carefully check the 'break' vs. 'continue'
changes.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 112 ++++++++++++++++++++++++++-------------------------
1 file changed, 58 insertions(+), 54 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 17cf911..9543494 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -255,69 +255,73 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
}
for (j=0; mechType && mechType[j]; j++) {
- for (i=0; all_sec && all_sec[i].op; i++) {
- if (strcmp(mechType[j], all_sec[i].oid) != 0) {
- continue;
- }
-
- nt_status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(nt_status)) {
- return nt_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)) {
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- break;
- }
+ const struct gensec_security_ops_wrapper *cur_sec = NULL;
- if (j > 0) {
- /* no optimistic token */
- spnego_state->neg_oid = all_sec[i].oid;
- *unwrapped_out = data_blob_null;
- nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- /*
- * Indicate the downgrade and request a
- * mic.
- */
- spnego_state->downgraded = true;
- spnego_state->mic_requested = true;
+ for (i=0; all_sec && all_sec[i].op; i++) {
+ if (strcmp(mechType[j], all_sec[i].oid) == 0) {
+ cur_sec = &all_sec[i];
break;
}
+ }
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
- 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)) {
-
- DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed to parse contents: %s\n",
- spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
+ if (cur_sec == NULL) {
+ continue;
+ }
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- break;
- }
+ nt_status = gensec_subcontext_start(spnego_state,
+ gensec_security,
+ &spnego_state->sub_sec_security);
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ return nt_status;
+ }
+ /* select the sub context */
+ nt_status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
+ cur_sec->op);
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
+ continue;
+ }
- spnego_state->neg_oid = all_sec[i].oid;
+ if (j > 0) {
+ /* no optimistic token */
+ spnego_state->neg_oid = cur_sec->oid;
+ *unwrapped_out = data_blob_null;
+ nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+ /*
+ * Indicate the downgrade and request a
+ * mic.
+ */
+ spnego_state->downgraded = true;
+ spnego_state->mic_requested = true;
break;
}
- if (spnego_state->sub_sec_security) {
- break;
+
+ nt_status = gensec_update_ev(spnego_state->sub_sec_security,
+ out_mem_ctx,
+ 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)) {
+
+ DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed to parse contents: %s\n",
+ spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
+
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
+ continue;
+ }
+
+ spnego_state->neg_oid = cur_sec->oid;
+ break;
}
if (!spnego_state->sub_sec_security) {
--
1.9.1
From 7662c31da096a11f91871f8dae531790bd98b059 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 11:05:39 +0200
Subject: [PATCH 04/24] auth/spnego: do an early return when we downgraded the
mech in gensec_spnego_parse_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 9543494..1e1fd87 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -296,7 +296,7 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
*/
spnego_state->downgraded = true;
spnego_state->mic_requested = true;
- break;
+ return nt_status;
}
nt_status = gensec_update_ev(spnego_state->sub_sec_security,
--
1.9.1
From 64989c3dd0f0a2c61a02f1cceef7491dc2083651 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 11:07:41 +0200
Subject: [PATCH 05/24] auth/spnego: add an early return for a hard error in
gensec_spnego_parse_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 1e1fd87..631e5b1 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -320,6 +320,13 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
continue;
}
+ if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
+ DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed: %s\n",
+ spnego_state->sub_sec_security->ops->name,
+ nt_errstr(nt_status)));
+ return nt_status;
+ }
+
spnego_state->neg_oid = cur_sec->oid;
break;
}
--
1.9.1
From 4614bcd143e24af28a7433eb9cc571447f8e2d56 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 11:09:59 +0200
Subject: [PATCH 06/24] auth/spnego: add an early return for OK or MORE
PROCESSING in gensec_spnego_parse_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 631e5b1..1a4c28b 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -328,7 +328,7 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
}
spnego_state->neg_oid = cur_sec->oid;
- break;
+ return nt_status; /* OK or MORE PROCESSING */
}
if (!spnego_state->sub_sec_security) {
--
1.9.1
From 17b71c2a29019de69b89a4cb2858c7abdbaa7750 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 11:11:57 +0200
Subject: [PATCH 07/24] auth/spnego: remove more dead code from
gensec_spnego_parse_negTokenInit()
Now we finally have a logic that someone can understand while reading it.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 40 +---------------------------------------
1 file changed, 1 insertion(+), 39 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 1a4c28b..defffe4 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -331,46 +331,8 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
return nt_status; /* OK or MORE PROCESSING */
}
- if (!spnego_state->sub_sec_security) {
- DEBUG(1, ("SPNEGO: Could not find a suitable mechtype in NEG_TOKEN_INIT\n"));
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- if (spnego_state->sub_sec_security) {
- /* it is likely that a NULL input token will
- * not be liked by most server mechs, but this
- * does the right thing in the CIFS client.
- * just push us along the merry-go-round
- * again, and hope for better luck next
- * time */
-
- if (NT_STATUS_EQUAL(nt_status, NT_STATUS_INVALID_PARAMETER)) {
- *unwrapped_out = data_blob_null;
- nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- }
-
- if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
- DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed: %s\n",
- spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
-
- /* 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;
- }
-
- return nt_status; /* OK or MORE PROCESSING */
- }
-
DEBUG(1, ("SPNEGO: Could not find a suitable mechtype in NEG_TOKEN_INIT\n"));
- /* we could re-negotiate here, but it would only work
- * if the client or server lied about what it could
- * support the first time. Lets keep this code to
- * reality */
-
- return nt_status;
+ return NT_STATUS_INVALID_PARAMETER;
}
/** create a negTokenInit
--
1.9.1
From 6ecb9c28e7cf070f7f152db0abf5fa7aa9553b65 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Jul 2017 11:39:39 +0200
Subject: [PATCH 08/24] auth/spnego: inline gensec_spnego_parse_negTokenInit()
into gensec_spnego_server_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 267 ++++++++++++++++++++++++---------------------------
1 file changed, 123 insertions(+), 144 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index defffe4..af554cd 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -203,138 +203,6 @@ static NTSTATUS gensec_spnego_server_try_fallback(struct gensec_security *gensec
return NT_STATUS_INVALID_PARAMETER;
}
-/*
- Parse the netTokenInit, either from the client, to the server, or
- from the server to the client.
-*/
-
-static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_security,
- struct spnego_state *spnego_state,
- TALLOC_CTX *out_mem_ctx,
- struct tevent_context *ev,
- struct spnego_data *spnego_in,
- DATA_BLOB *unwrapped_out)
-{
- int i;
- NTSTATUS nt_status = NT_STATUS_INVALID_PARAMETER;
- const char * const *mechType = NULL;
- DATA_BLOB unwrapped_in = data_blob_null;
- bool ok;
- const struct gensec_security_ops_wrapper *all_sec = NULL;
- uint32_t j;
-
- if (spnego_state->state_position != SPNEGO_SERVER_START) {
- return NT_STATUS_INTERNAL_ERROR;
- }
-
- if (spnego_in->type != SPNEGO_NEG_TOKEN_INIT) {
- return NT_STATUS_INTERNAL_ERROR;
- }
-
- mechType = spnego_in->negTokenInit.mechTypes;
- if (mechType == NULL) {
- return NT_STATUS_INVALID_PARAMETER;
- }
- unwrapped_in = spnego_in->negTokenInit.mechToken;
-
- all_sec = gensec_security_by_oid_list(gensec_security,
- out_mem_ctx,
- mechType,
- GENSEC_OID_SPNEGO);
- if (all_sec == NULL) {
- DBG_WARNING("gensec_security_by_oid_list() failed\n");
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- ok = spnego_write_mech_types(spnego_state,
- mechType,
- &spnego_state->mech_types);
- if (!ok) {
- DEBUG(1, ("SPNEGO: Failed to write mechTypes\n"));
- return NT_STATUS_NO_MEMORY;
- }
-
- for (j=0; mechType && mechType[j]; j++) {
- const struct gensec_security_ops_wrapper *cur_sec = NULL;
-
- for (i=0; all_sec && all_sec[i].op; i++) {
- if (strcmp(mechType[j], all_sec[i].oid) == 0) {
- cur_sec = &all_sec[i];
- break;
- }
- }
-
- if (cur_sec == NULL) {
- continue;
- }
-
- nt_status = gensec_subcontext_start(spnego_state,
- gensec_security,
- &spnego_state->sub_sec_security);
- if (!NT_STATUS_IS_OK(nt_status)) {
- return nt_status;
- }
- /* select the sub context */
- nt_status = gensec_start_mech_by_ops(spnego_state->sub_sec_security,
- cur_sec->op);
- if (!NT_STATUS_IS_OK(nt_status)) {
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
- }
-
- if (j > 0) {
- /* no optimistic token */
- spnego_state->neg_oid = cur_sec->oid;
- *unwrapped_out = data_blob_null;
- nt_status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- /*
- * Indicate the downgrade and request a
- * mic.
- */
- spnego_state->downgraded = true;
- spnego_state->mic_requested = true;
- return nt_status;
- }
-
- nt_status = gensec_update_ev(spnego_state->sub_sec_security,
- out_mem_ctx,
- 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)) {
-
- DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed to parse contents: %s\n",
- spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
-
- /*
- * Pretend we never started it
- */
- gensec_spnego_update_sub_abort(spnego_state);
- continue;
- }
-
- if (GENSEC_UPDATE_IS_NTERROR(nt_status)) {
- DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed: %s\n",
- spnego_state->sub_sec_security->ops->name,
- nt_errstr(nt_status)));
- return nt_status;
- }
-
- spnego_state->neg_oid = cur_sec->oid;
- return nt_status; /* OK or MORE PROCESSING */
- }
-
- DEBUG(1, ("SPNEGO: Could not find a suitable mechtype in NEG_TOKEN_INIT\n"));
- 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
@@ -1034,17 +902,126 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
TALLOC_CTX *out_mem_ctx,
DATA_BLOB *out)
{
+ TALLOC_CTX *frame = talloc_stackframe();
DATA_BLOB sub_out = data_blob_null;
DATA_BLOB mech_list_mic = data_blob_null;
+ const char * const *mech_types = NULL;
+ size_t all_idx = 0;
+ const struct gensec_security_ops_wrapper *all_sec = NULL;
+ size_t mech_idx = 0;
NTSTATUS status;
+ bool ok;
- status = gensec_spnego_parse_negTokenInit(gensec_security,
- spnego_state,
- out_mem_ctx,
- ev,
- spnego_in,
- &sub_out);
+ mech_types = spnego_in->negTokenInit.mechTypes;
+ if (mech_types == NULL) {
+ TALLOC_FREE(frame);
+ 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;
+ }
+
+ ok = spnego_write_mech_types(spnego_state, mech_types,
+ &spnego_state->mech_types);
+ if (!ok) {
+ DBG_ERR("Failed to write mechTypes\n");
+ TALLOC_FREE(frame);
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ /*
+ * First try the preferred mechs from the client.
+ */
+ for (; mech_types[mech_idx]; mech_idx++) {
+ const char *cur_mech = mech_types[mech_idx];
+ const struct gensec_security_ops_wrapper *cur_sec = NULL;
+ DATA_BLOB sub_in = data_blob_null;
+ for (all_idx = 0; all_sec[all_idx].op; all_idx++) {
+ if (strcmp(cur_mech, all_sec[all_idx].oid) == 0) {
+ cur_sec = &all_sec[all_idx];
+ break;
+ }
+ }
+
+ if (cur_sec == NULL) {
+ continue;
+ }
+
+ 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;
+ }
+
+ if (mech_idx > 0) {
+ /*
+ * Indicate the downgrade and request a
+ * mic.
+ */
+ spnego_state->downgraded = true;
+ spnego_state->mic_requested = true;
+ /* no optimistic token */
+ spnego_state->neg_oid = cur_sec->oid;
+ sub_out = data_blob_null;
+ status = NT_STATUS_MORE_PROCESSING_REQUIRED;
+ goto reply;
+ }
+
+ /*
+ * Try the optimistic token from the client
+ */
+ sub_in = spnego_in->negTokenInit.mechToken;
+ status = gensec_update_ev(spnego_state->sub_sec_security,
+ frame, ev, sub_in, &sub_out);
+ if (NT_STATUS_IS_OK(status)) {
+ spnego_state->sub_sec_ready = true;
+ }
+ if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER) ||
+ NT_STATUS_EQUAL(status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO)) {
+
+ DBG_WARNING("%s: NEG_TOKEN_INIT failed to parse contents: %s\n",
+ cur_sec->op->name, nt_errstr(status));
+
+ /*
+ * Pretend we never started it
+ */
+ gensec_spnego_update_sub_abort(spnego_state);
+ continue;
+ }
+
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ DBG_WARNING("%s: NEG_TOKEN_INIT failed: %s\n",
+ cur_sec->op->name, nt_errstr(status));
+ goto reply;
+ }
+
+ spnego_state->neg_oid = cur_sec->oid;
+ goto reply; /* OK or MORE PROCESSING */
+ }
+
+ DBG_WARNING("Could not find a suitable mechtype in NEG_TOKEN_INIT\n");
+ status = NT_STATUS_INVALID_PARAMETER;
+
+ reply:
if (spnego_state->simulate_w2k) {
/*
* Windows 2000 returns the unwrapped token
@@ -1057,12 +1034,14 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
mech_list_mic = sub_out;
}
- return gensec_spnego_server_response(spnego_state,
- out_mem_ctx,
- status,
- sub_out,
- mech_list_mic,
- out);
+ status = gensec_spnego_server_response(spnego_state,
+ out_mem_ctx,
+ status,
+ sub_out,
+ mech_list_mic,
+ out);
+ TALLOC_FREE(frame);
+ return status;
}
static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec_security,
--
1.9.1
From 9d7983c9c4a1a5a88d122921df87a7c283a6b10d 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 09/24] auth/spnego: split out gensec_spnego_update_pre/post()
functions
For now we keep doing sync processing only, in future
we'll do some preprocessing before a gensec_update_send()
on the subcontext in gensec_spnego_update_pre()
and handle the the result of gensec_update_recv()
in gensec_spnego_update_post().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 234 +++++++++++++++++++++++++++++++++------------------
1 file changed, 153 insertions(+), 81 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index af554cd..f8ab114 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1166,6 +1166,7 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
}
struct gensec_spnego_update_state {
+ struct tevent_context *ev;
struct gensec_security *gensec;
struct spnego_state *spnego;
DATA_BLOB full_in;
@@ -1199,6 +1200,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, TALLOC_CTX *mem_ctx,
DATA_BLOB *full_in);
+static void gensec_spnego_update_pre(struct tevent_req *req);
+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,
DATA_BLOB *_out);
@@ -1221,6 +1224,7 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
if (req == NULL) {
return NULL;
}
+ state->ev = ev;
state->gensec = gensec_security;
state->spnego = spnego_state;
tevent_req_set_cleanup_fn(req, gensec_spnego_update_cleanup);
@@ -1326,94 +1330,21 @@ static struct tevent_req *gensec_spnego_update_send(TALLOC_CTX *mem_ctx,
return NULL;
}
- /* 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:
- if (state->spnego_in == NULL) {
- /* client to produce negTokenInit */
- status = gensec_spnego_create_negTokenInit(gensec_security,
- spnego_state, state, ev,
- &spnego_state->out_frag);
- break;
- }
-
- status = gensec_spnego_client_negTokenInit(gensec_security,
- spnego_state, ev,
- state->spnego_in, state,
- &spnego_state->out_frag);
- break;
-
- case SPNEGO_CLIENT_TARG:
- status = gensec_spnego_client_negTokenTarg(gensec_security,
- spnego_state, ev,
- state->spnego_in, state,
- &spnego_state->out_frag);
- break;
-
- case SPNEGO_SERVER_START:
- if (state->spnego_in == NULL) {
- /* server to produce negTokenInit */
- status = gensec_spnego_create_negTokenInit(gensec_security,
- spnego_state, state, ev,
- &spnego_state->out_frag);
- break;
- }
-
- status = gensec_spnego_server_negTokenInit(gensec_security,
- spnego_state, ev,
- state->spnego_in, state,
- &spnego_state->out_frag);
- break;
-
- case SPNEGO_SERVER_TARG:
- status = gensec_spnego_server_negTokenTarg(gensec_security,
- spnego_state, ev,
- state->spnego_in, state,
- &spnego_state->out_frag);
- break;
-
- default:
- smb_panic(__location__);
- return NULL;
- }
-
- if (GENSEC_UPDATE_IS_NTERROR(status)) {
- tevent_req_nterror(req, status);
+ gensec_spnego_update_pre(req);
+ if (!tevent_req_is_in_progress(req)) {
return tevent_req_post(req, ev);
}
- if (NT_STATUS_IS_OK(status)) {
- bool reset_full = true;
-
- reset_full = !spnego_state->done_mic_check;
-
- status = gensec_may_reset_crypto(spnego_state->sub_sec_security,
- reset_full);
- if (tevent_req_nterror(req, status)) {
- return tevent_req_post(req, ev);
- }
- }
-
- spnego_state->out_status = status;
+ /*
+ * TODO: prepare async processing here in future.
+ */
- status = gensec_spnego_update_out(gensec_security,
- state, &state->out);
- if (GENSEC_UPDATE_IS_NTERROR(status)) {
- tevent_req_nterror(req, status);
+ gensec_spnego_update_post(req);
+ if (!tevent_req_is_in_progress(req)) {
return tevent_req_post(req, ev);
}
- state->status = status;
- tevent_req_done(req);
- return tevent_req_post(req, ev);
+ return req;
}
static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
@@ -1524,6 +1455,147 @@ static NTSTATUS gensec_spnego_update_in(struct gensec_security *gensec_security,
return NT_STATUS_OK;
}
+static void gensec_spnego_update_pre(struct tevent_req *req)
+{
+ struct gensec_spnego_update_state *state =
+ tevent_req_data(req,
+ struct gensec_spnego_update_state);
+ struct gensec_security *gensec_security = state->gensec;
+ struct spnego_state *spnego_state = state->spnego;
+ struct tevent_context *ev = state->ev;
+ NTSTATUS status;
+
+ 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;
+ return;
+ }
+
+ switch (spnego_state->state_position) {
+ case SPNEGO_CLIENT_START:
+ if (state->spnego_in == NULL) {
+ /* client to produce negTokenInit */
+ status = gensec_spnego_create_negTokenInit(gensec_security,
+ spnego_state, state, ev,
+ &spnego_state->out_frag);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+ break;
+ }
+
+ status = gensec_spnego_client_negTokenInit(gensec_security,
+ spnego_state, ev,
+ state->spnego_in, state,
+ &spnego_state->out_frag);
+ break;
+
+ case SPNEGO_CLIENT_TARG:
+ status = gensec_spnego_client_negTokenTarg(gensec_security,
+ spnego_state, ev,
+ state->spnego_in, state,
+ &spnego_state->out_frag);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+ break;
+
+ case SPNEGO_SERVER_START:
+ if (state->spnego_in == NULL) {
+ /* server to produce negTokenInit */
+ status = gensec_spnego_create_negTokenInit(gensec_security,
+ spnego_state, state, ev,
+ &spnego_state->out_frag);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+ break;
+ }
+
+ status = gensec_spnego_server_negTokenInit(gensec_security,
+ spnego_state, ev,
+ state->spnego_in, state,
+ &spnego_state->out_frag);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+ break;
+
+ case SPNEGO_SERVER_TARG:
+ status = gensec_spnego_server_negTokenTarg(gensec_security,
+ spnego_state, ev,
+ state->spnego_in, state,
+ &spnego_state->out_frag);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+ break;
+
+ default:
+ smb_panic(__location__);
+ return;
+ }
+
+ spnego_state->out_status = status;
+}
+
+static void gensec_spnego_update_post(struct tevent_req *req)
+{
+ struct gensec_spnego_update_state *state =
+ tevent_req_data(req,
+ struct gensec_spnego_update_state);
+ struct spnego_state *spnego_state = state->spnego;
+ NTSTATUS status;
+
+ if (spnego_state->state_position == SPNEGO_FALLBACK) {
+ status = spnego_state->out_status;
+ goto respond;
+ }
+
+ /*
+ * For now just handle the sync processing done
+ * in gensec_spnego_update_pre()
+ */
+ status = spnego_state->out_status;
+
+ if (NT_STATUS_IS_OK(status)) {
+ bool reset_full = true;
+
+ reset_full = !spnego_state->done_mic_check;
+
+ status = gensec_may_reset_crypto(spnego_state->sub_sec_security,
+ reset_full);
+ if (tevent_req_nterror(req, status)) {
+ return;
+ }
+ }
+
+respond:
+ spnego_state->out_status = status;
+
+ status = gensec_spnego_update_out(state->gensec,
+ state, &state->out);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+
+ state->status = status;
+ tevent_req_done(req);
+ return;
+}
+
static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security,
TALLOC_CTX *out_mem_ctx,
DATA_BLOB *_out)
--
1.9.1
From 3211153803b1689fd9654e80bba44740d70da229 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 10/24] 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>
Reviewed-by: Andreas Schneider <asn 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 f8ab114..f16b39a 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -470,19 +470,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 e1ca5505338f0fb5da9ae58fcdb2b1c55d12a0d9 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 11/24] auth/spnego: make the SPNEGO_FALLBACK continuation
completely async
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn 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 f16b39a..0e37c3f 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1168,9 +1168,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;
};
@@ -1200,6 +1209,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,
@@ -1334,9 +1344,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)) {
@@ -1464,15 +1489,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;
}
@@ -1549,6 +1574,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 =
@@ -1557,8 +1601,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 e96c3788f27c56c32b245bad18a397602eb0517b 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 12/24] 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 0e37c3f..bface12 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -218,6 +218,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);
@@ -227,10 +230,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);
@@ -292,55 +291,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 3a093ef47b503402300fef397169e0b2284fab10 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 13/24] 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>
Reviewed-by: Andreas Schneider <asn 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 bface12..6645913 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -244,51 +244,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 f73ef2fe25e646deefb014e00eb9b0cd8f03d446 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 14/24] 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>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 64 +++++++++++++++++++++++++---------------------------
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 6645913..5d71ae1 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -230,6 +230,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);
@@ -261,44 +265,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 31e9f63993cbf20ff5a45a62621d59b54eb75a2c 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 15/24] auth/spnego: make the debug messages in
gensec_spnego_create_negTokenInit() more useful
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn 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 5d71ae1..f7faf61 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -287,10 +287,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
@@ -298,7 +299,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:
@@ -311,7 +313,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;
}
@@ -330,8 +332,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 3bbe5c55e5a5a6806ae5aa7306f59eb31b59246c 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 16/24] 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>
Reviewed-by: Andreas Schneider <asn 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 f7faf61..6d03370 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -214,7 +214,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;
@@ -234,16 +234,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;
}
@@ -256,16 +256,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;
}
@@ -291,7 +291,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
@@ -300,8 +300,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 bae703d44e94d72425934de239b2a888693928a6 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 17/24] auth/spnego: add more error checking to
gensec_spnego_create_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn 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 6d03370..c91ae38 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -224,11 +224,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;
@@ -308,6 +317,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 3d17179445e02c0088c4563e6e0c06e27acc1b42 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 18/24] 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>
Reviewed-by: Andreas Schneider <asn 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 c91ae38..d8c62b3 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -214,7 +214,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;
@@ -302,15 +302,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 78bc38cc092c8e52ad2d10043c995adb009158ce 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 19/24] auth/spnego: use better variable names in
gensec_spnego_create_negTokenInit()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn 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 d8c62b3..9a2fdbd 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -213,11 +213,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;
@@ -238,11 +239,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);
@@ -251,7 +254,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;
@@ -278,8 +281,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;
}
@@ -298,8 +301,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) {
@@ -322,7 +324,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;
@@ -355,9 +357,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 51b526d3c7654185071b05ba50f2f7ad0c397a34 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 17 Jul 2017 20:47:57 +0200
Subject: [PATCH 20/24] auth/spnego: do an early return for the success case in
gensec_spnego_client_negTokenTarg()
Check with git show -w
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 9a2fdbd..87a0791 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -841,37 +841,38 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
return status;
}
- if (sub_out.length || mech_list_mic.length) {
- /* compose reply */
- spnego_out.type = SPNEGO_NEG_TOKEN_TARG;
- spnego_out.negTokenTarg.negResult = SPNEGO_NONE_RESULT;
- spnego_out.negTokenTarg.supportedMech = NULL;
- spnego_out.negTokenTarg.responseToken = sub_out;
- spnego_out.negTokenTarg.mechListMIC = mech_list_mic;
-
- if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
- DBG_WARNING("Failed to write NEG_TOKEN_TARG\n");
- return NT_STATUS_INVALID_PARAMETER;
- }
-
- spnego_state->num_targs++;
- spnego_state->state_position = SPNEGO_CLIENT_TARG;
- status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- } else {
-
- /* all done - server has accepted, and we agree */
+ if (sub_out.length == 0 && mech_list_mic.length == 0) {
*out = data_blob_null;
if (ta->negResult != SPNEGO_ACCEPT_COMPLETED) {
/* unless of course it did not accept */
DBG_WARNING("gensec_update ok but not accepted\n");
- status = NT_STATUS_INVALID_PARAMETER;
+ return NT_STATUS_INVALID_PARAMETER;
}
spnego_state->state_position = SPNEGO_DONE;
+ return status;
}
- return status;
+ /* compose reply */
+ spnego_out.type = SPNEGO_NEG_TOKEN_TARG;
+ spnego_out.negTokenTarg.negResult = SPNEGO_NONE_RESULT;
+ spnego_out.negTokenTarg.supportedMech = NULL;
+ spnego_out.negTokenTarg.responseToken = sub_out;
+ spnego_out.negTokenTarg.mechListMIC = mech_list_mic;
+
+ if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
+ DBG_WARNING("Failed to write NEG_TOKEN_TARG\n");
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ spnego_state->num_targs++;
+
+ /* set next state */
+ spnego_state->state_position = SPNEGO_CLIENT_TARG;
+ spnego_state->expected_packet = SPNEGO_NEG_TOKEN_TARG;
+
+ return NT_STATUS_MORE_PROCESSING_REQUIRED;
}
/** create a server negTokenTarg
--
1.9.1
From 95549a4abd9e85cc67bd384f80fbf0fb7672f4fc Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 17 Jul 2017 20:49:34 +0200
Subject: [PATCH 21/24] auth/spnego: make sure we don't return OK without
sub_sec_ready in gensec_spnego_client_negTokenTarg()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 87a0791..22abad3 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -844,6 +844,12 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
if (sub_out.length == 0 && mech_list_mic.length == 0) {
*out = data_blob_null;
+ if (!spnego_state->sub_sec_ready) {
+ /* somethings wrong here... */
+ DBG_ERR("gensec_update not ready without output\n");
+ return NT_STATUS_INTERNAL_ERROR;
+ }
+
if (ta->negResult != SPNEGO_ACCEPT_COMPLETED) {
/* unless of course it did not accept */
DBG_WARNING("gensec_update ok but not accepted\n");
--
1.9.1
From 8b6ea44915d15b88c29380021a73966ca2c4ff5a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 18 Jul 2017 11:42:43 +0200
Subject: [PATCH 22/24] auth/spnego: generate a valid packet if
gensec_spnego_client_negTokenTarg() gives MORE_PROCESSING_REQUIRED
If we wait for the mechListMIC from the server we should send a valid paket
instead of an empty blob.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/spnego.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 22abad3..5eb75ad 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -829,10 +829,6 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
spnego_state->needs_mic_sign = false;
}
- if (spnego_state->needs_mic_check) {
- status = NT_STATUS_MORE_PROCESSING_REQUIRED;
- }
-
client_response:
if (GENSEC_UPDATE_IS_NTERROR(status)) {
DBG_WARNING("SPNEGO(%s) login failed: %s\n",
@@ -856,8 +852,10 @@ static NTSTATUS gensec_spnego_client_negTokenTarg(struct gensec_security *gensec
return NT_STATUS_INVALID_PARAMETER;
}
- spnego_state->state_position = SPNEGO_DONE;
- return status;
+ if (!spnego_state->needs_mic_check) {
+ spnego_state->state_position = SPNEGO_DONE;
+ return NT_STATUS_OK;
+ }
}
/* compose reply */
--
1.9.1
From b116dba97f3ea86702147be9dbe05056d4a618c5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 17 Jul 2017 21:54:51 +0200
Subject: [PATCH 23/24] auth/spnego: don't call gensec_spnego_server_response()
with a fatal error
It doesn't make sense to produce an output token without
returning OK or MORE_PROCESSING_REQUIRED.
Even in v4-0-test we had gensec_spnego_update_wrapper()
which only passed the constructed output token to the caller
with OK or MORE_PROCESSING_REQUIRED.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
auth/gensec/spnego.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 5eb75ad..474f0a9 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1048,7 +1048,8 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
if (GENSEC_UPDATE_IS_NTERROR(status)) {
DBG_WARNING("%s: NEG_TOKEN_INIT failed: %s\n",
cur_sec->op->name, nt_errstr(status));
- goto reply;
+ TALLOC_FREE(frame);
+ return status;
}
spnego_state->neg_oid = cur_sec->oid;
@@ -1056,7 +1057,8 @@ static NTSTATUS gensec_spnego_server_negTokenInit(struct gensec_security *gensec
}
DBG_WARNING("Could not find a suitable mechtype in NEG_TOKEN_INIT\n");
- status = NT_STATUS_INVALID_PARAMETER;
+ TALLOC_FREE(frame);
+ return NT_STATUS_INVALID_PARAMETER;
reply:
if (spnego_state->simulate_w2k) {
@@ -1118,7 +1120,7 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
if (!NT_STATUS_IS_OK(status)) {
DBG_WARNING("failed to verify mechListMIC: %s\n",
nt_errstr(status));
- goto server_response;
+ return status;
}
spnego_state->needs_mic_check = false;
@@ -1130,6 +1132,11 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
status = gensec_update_ev(spnego_state->sub_sec_security,
out_mem_ctx, ev,
sub_in, &sub_out);
+ if (GENSEC_UPDATE_IS_NTERROR(status)) {
+ DEBUG(2, ("SPNEGO login failed: %s\n",
+ nt_errstr(status)));
+ return status;
+ }
if (NT_STATUS_IS_OK(status)) {
spnego_state->sub_sec_ready = true;
}
@@ -1166,7 +1173,7 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
if (!NT_STATUS_IS_OK(status)) {
DBG_WARNING("failed to verify mechListMIC: %s\n",
nt_errstr(status));
- goto server_response;
+ return status;
}
spnego_state->needs_mic_check = false;
--
1.9.1
From c20716ba912f720f30ccacc99577c17fe7d86f66 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 17 Jul 2017 22:00:10 +0200
Subject: [PATCH 24/24] auth/spnego: don't produce an output token for errors
in gensec_spnego_server_response()
gensec_spnego_server_response() is never called with a fatal error anymore.
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 474f0a9..aae3cab 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -914,11 +914,6 @@ static NTSTATUS gensec_spnego_server_response(struct spnego_state *spnego_state,
}
spnego_out.negTokenTarg.negResult = SPNEGO_ACCEPT_COMPLETED;
spnego_state->state_position = SPNEGO_DONE;
- } else {
- spnego_out.negTokenTarg.negResult = SPNEGO_REJECT;
- spnego_out.negTokenTarg.mechListMIC = data_blob_null;
- DEBUG(2, ("SPNEGO login failed: %s\n", nt_errstr(nt_status)));
- spnego_state->state_position = SPNEGO_DONE;
}
if (spnego_write_data(out_mem_ctx, out, &spnego_out) == -1) {
--
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/20170721/bc99d7d7/signature.sig>
More information about the samba-technical
mailing list