r3110: Fix the krb5 client and server, so that it doesn't segfault. There

were also gensec bugs that didn't turn up until we hit error paths in
the krb5 code.

Andrew Bartlett
(This used to be commit e08366ffeb52e8c522d3808a2af1aa0bc632b55f)

4 files changed, 57 insertions, 49 deletions

diff --git a/source4/libcli/auth/gensec.c b/source4/libcli/auth/gensec.c
index 353c5f562d..a00a36e171 100644
--- a/source4/libcli/auth/gensec.c
+++ b/source4/libcli/auth/gensec.c
@@ -452,11 +452,8 @@ void gensec_end(struct gensec_security **gensec_security)
 	}
 	(*gensec_security)->private_data = NULL;
 
-	if (!(*gensec_security)->subcontext) {
-		/* don't destory this if this is a subcontext - it belongs to the parent */
-		talloc_free(*gensec_security);
-	}
-	gensec_security = NULL;
+	talloc_free(*gensec_security);
+	*gensec_security = NULL;
 }
 
 /** 
diff --git a/source4/libcli/auth/gensec_krb5.c b/source4/libcli/auth/gensec_krb5.c
index 7895a6f1ed..26bf0cf663 100644
--- a/source4/libcli/auth/gensec_krb5.c
+++ b/source4/libcli/auth/gensec_krb5.c
@@ -224,6 +224,40 @@ static NTSTATUS gensec_krb5_decode_pac(TALLOC_CTX *mem_ctx,
 	return status;
 }
 
+static void gensec_krb5_end(struct gensec_security *gensec_security)
+{
+	struct gensec_krb5_state *gensec_krb5_state = gensec_security->private_data;
+
+	if (gensec_krb5_state->ticket.length) { 
+	/* Hmm, heimdal dooesn't have this - what's the correct call? */
+#ifdef HAVE_KRB5_FREE_DATA_CONTENTS
+		krb5_free_data_contents(gensec_krb5_state->krb5_context, &gensec_krb5_state->ticket); 
+#endif
+	}
+	if (gensec_krb5_state->krb5_ccache) {
+		/* Removed by jra. They really need to fix their kerberos so we don't leak memory. 
+		   JERRY -- disabled since it causes heimdal 0.6.1rc3 to die
+		   SuSE 9.1 Pro 
+		*/
+#if 0 /* redisabled by gd :) at least until any official heimdal version has it fixed. */
+		krb5_cc_close(context, gensec_krb5_state->krb5_ccache);
+#endif
+	}
+
+	if (gensec_krb5_state->krb5_auth_context) {
+		krb5_auth_con_free(gensec_krb5_state->krb5_context, 
+				   gensec_krb5_state->krb5_auth_context);
+	}
+
+	if (gensec_krb5_state->krb5_context) {
+		krb5_free_context(gensec_krb5_state->krb5_context);
+	}
+
+	talloc_free(gensec_krb5_state);
+	gensec_security->private_data = NULL;
+}
+
+
 static NTSTATUS gensec_krb5_start(struct gensec_security *gensec_security)
 {
 	struct gensec_krb5_state *gensec_krb5_state;
@@ -324,6 +358,9 @@ static NTSTATUS gensec_krb5_client_start(struct gensec_security *gensec_security
 				DEBUG(1, ("Could not determine hostname for target computer, cannot use kerberos\n"));
 				return NT_STATUS_ACCESS_DENIED;
 			}
+			
+			in_data.length = 0;
+
 			ret = krb5_mk_req(gensec_krb5_state->krb5_context, 
 					  &gensec_krb5_state->krb5_auth_context,
 					  AP_OPTS_USE_SUBKEY | AP_OPTS_MUTUAL_REQUIRED,
@@ -392,39 +429,6 @@ static NTSTATUS gensec_krb5_client_start(struct gensec_security *gensec_security
 	}
 }
 
-static void gensec_krb5_end(struct gensec_security *gensec_security)
-{
-	struct gensec_krb5_state *gensec_krb5_state = gensec_security->private_data;
-
-	if (gensec_krb5_state->ticket.length) { 
-	/* Hmm, heimdal dooesn't have this - what's the correct call? */
-#ifdef HAVE_KRB5_FREE_DATA_CONTENTS
-		krb5_free_data_contents(gensec_krb5_state->krb5_context, &gensec_krb5_state->ticket); 
-#endif
-	}
-	if (gensec_krb5_state->krb5_ccache) {
-		/* Removed by jra. They really need to fix their kerberos so we don't leak memory. 
-		   JERRY -- disabled since it causes heimdal 0.6.1rc3 to die
-		   SuSE 9.1 Pro 
-		*/
-#if 0 /* redisabled by gd :) at least until any official heimdal version has it fixed. */
-		krb5_cc_close(context, gensec_krb5_state->krb5_ccache);
-#endif
-	}
-
-	if (gensec_krb5_state->krb5_auth_context) {
-		krb5_auth_con_free(gensec_krb5_state->krb5_context, 
-				   gensec_krb5_state->krb5_auth_context);
-	}
-
-	if (gensec_krb5_state->krb5_context) {
-		krb5_free_context(gensec_krb5_state->krb5_context);
-	}
-
-	talloc_free(gensec_krb5_state);
-	gensec_security->private_data = NULL;
-}
-
 
 /**
  * Next state function for the Krb5 GENSEC mechanism
diff --git a/source4/libcli/auth/spnego.c b/source4/libcli/auth/spnego.c
index 4409f5ea9d..dff9cb0c51 100644
--- a/source4/libcli/auth/spnego.c
+++ b/source4/libcli/auth/spnego.c
@@ -277,19 +277,17 @@ static NTSTATUS gensec_spnego_parse_negTokenInit(struct gensec_security *gensec_
 						  null_data_blob, 
 						  unwrapped_out);
 		}
-		if (!NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED) 
-		    && (!NT_STATUS_IS_OK(nt_status))) {
+		if (!NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED) && !NT_STATUS_IS_OK(nt_status)) {
 			DEBUG(1, ("SPNEGO(%s) NEG_TOKEN_INIT failed: %s\n", 
 				  spnego_state->sub_sec_security->ops->name, nt_errstr(nt_status)));
-				gensec_end(&spnego_state->sub_sec_security);
-		} else {
-			break;
+			gensec_end(&spnego_state->sub_sec_security);
 		}
+		return nt_status;
 	}
 	if (!mechType || !mechType[i]) {
 		DEBUG(1, ("SPNEGO: Could not find a suitable mechtype in NEG_TOKEN_INIT\n"));
 	}
-	return nt_status;
+	return NT_STATUS_INVALID_PARAMETER;
 }
 
 /** create a client negTokenInit 
@@ -369,22 +367,23 @@ static NTSTATUS gensec_spnego_server_negTokenTarg(struct gensec_security *gensec
 
 	/* compose reply */
 	spnego_out.type = SPNEGO_NEG_TOKEN_TARG;
-	spnego_out.negTokenTarg.supportedMech 
-		= spnego_state->sub_sec_security->ops->oid;
 	spnego_out.negTokenTarg.responseToken = unwrapped_out;
 	spnego_out.negTokenTarg.mechListMIC = null_data_blob;
 	
 	if (NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+		spnego_out.negTokenTarg.supportedMech 
+			= spnego_state->sub_sec_security->ops->oid;
 		spnego_out.negTokenTarg.negResult = SPNEGO_ACCEPT_INCOMPLETE;
 		spnego_state->state_position = SPNEGO_SERVER_TARG;
 	} else if (NT_STATUS_IS_OK(nt_status)) {
+		spnego_out.negTokenTarg.supportedMech 
+			= spnego_state->sub_sec_security->ops->oid;
 		spnego_out.negTokenTarg.negResult = SPNEGO_ACCEPT_COMPLETED;
 		spnego_state->state_position = SPNEGO_DONE;
 	} else {
+		spnego_out.negTokenTarg.supportedMech = NULL;
 		spnego_out.negTokenTarg.negResult = SPNEGO_REJECT;
-		DEBUG(1, ("SPNEGO(%s) login failed: %s\n", 
-			  spnego_state->sub_sec_security->ops->name, 
-			  nt_errstr(nt_status)));
+		DEBUG(1, ("SPNEGO login failed: %s\n", nt_errstr(nt_status)));
 		spnego_state->state_position = SPNEGO_DONE;
 	}
 	
diff --git a/source4/smbd/rewrite.c b/source4/smbd/rewrite.c
index 03542bf4e9..035532a01f 100644
--- a/source4/smbd/rewrite.c
+++ b/source4/smbd/rewrite.c
@@ -46,6 +46,14 @@ void smbd_process_init(void)
 
 void init_subsystems(void)
 {
+	/* Do *not* remove this, until you have removed
+	 * passdb/secrets.c, and proved that Samba still builds... */
+
+	/* Setup the SECRETS subsystem */
+	if (!secrets_init()) {
+		exit(1);
+	}
+
 	/* Setup the PROCESS_MODEL subsystem */
 	if (!process_model_init())
 		exit(1);