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)

diff --git a/source4/libcli/auth/gensec.c b/source4/libcli/auth/gensec.c
index 353c5f562d4f550239b15a4c26be87c6ee315fc4..a00a36e17156e6abbcd1c71239113a5c0b4010be 100644 (file)
--- 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 7895a6f1ed6740145932ef7388a504874651aaf2..26bf0cf6634f4fbc0ca86e42b54312ece4ca5cfb 100644 (file)
--- 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 4409f5ea9d8406117957eafd3fa792a1d01c0c66..dff9cb0c516e9687d8c99b840b5cf88e75091868 100644 (file)
--- 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 03542bf4e9c73e20399af98fe4e14c4e1ddb6f7c..035532a01f0364452a29575257bea1d28d9957bc 100644 (file)
--- 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);