gensec: clarify memory ownership for gensec_session_info() and gensec_session_key()

This is slightly less efficient, because we no longer keep a cache on
the gensec structures, but much clearer in terms of memory ownership.
Both gensec_session_info() and gensec_session_key() now take a mem_ctx
and put the result only on that context.

Some duplication of memory in the callers (who were rightly uncertain
about who was the rightful owner of the returned memory) has been
removed to compensate for the internal copy.

Andrew Bartlett

6 files changed, 28 insertions, 38 deletions

diff --git a/source4/auth/gensec/gensec_gssapi.c b/source4/auth/gensec/gensec_gssapi.c
index 4dd809856c..55610f5742 100644
--- a/source4/auth/gensec/gensec_gssapi.c
+++ b/source4/auth/gensec/gensec_gssapi.c
@@ -169,9 +169,6 @@ static NTSTATUS gensec_gssapi_start(struct gensec_security *gensec_security)
 		break;
 	}
 
-	gensec_gssapi_state->session_key = data_blob(NULL, 0);
-	gensec_gssapi_state->pac = data_blob(NULL, 0);
-
 	ret = smb_krb5_init_context(gensec_gssapi_state,
 				    NULL,
 				    gensec_security->settings->lp_ctx,
@@ -1242,6 +1239,7 @@ static bool gensec_gssapi_have_feature(struct gensec_security *gensec_security,
  * This breaks all the abstractions, but what do you expect...
  */
 static NTSTATUS gensec_gssapi_session_key(struct gensec_security *gensec_security, 
+					  TALLOC_CTX *mem_ctx,
 					  DATA_BLOB *session_key) 
 {
 	struct gensec_gssapi_state *gensec_gssapi_state
@@ -1253,11 +1251,6 @@ static NTSTATUS gensec_gssapi_session_key(struct gensec_security *gensec_securit
 		return NT_STATUS_NO_USER_SESSION_KEY;
 	}
 
-	if (gensec_gssapi_state->session_key.data) {
-		*session_key = gensec_gssapi_state->session_key;
-		return NT_STATUS_OK;
-	}
-
 	maj_stat = gsskrb5_get_subkey(&min_stat,
 				      gensec_gssapi_state->gssapi_context,
 				      &subkey);
@@ -1269,10 +1262,9 @@ static NTSTATUS gensec_gssapi_session_key(struct gensec_security *gensec_securit
 	DEBUG(10, ("Got KRB5 session key of length %d%s\n",
 		   (int)KRB5_KEY_LENGTH(subkey),
 		   (gensec_gssapi_state->sasl_state == STAGE_DONE)?" (done)":""));
-	*session_key = data_blob_talloc(gensec_gssapi_state,
+	*session_key = data_blob_talloc(mem_ctx,
 					KRB5_KEY_DATA(subkey), KRB5_KEY_LENGTH(subkey));
 	krb5_free_keyblock(gensec_gssapi_state->smb_krb5_context->krb5_context, subkey);
-	gensec_gssapi_state->session_key = *session_key;
 	dump_data_pw("KRB5 Session Key:\n", session_key->data, session_key->length);
 
 	return NT_STATUS_OK;
@@ -1282,6 +1274,7 @@ static NTSTATUS gensec_gssapi_session_key(struct gensec_security *gensec_securit
  * this session.  This uses either the PAC (if present) or a local
  * database lookup */
 static NTSTATUS gensec_gssapi_session_info(struct gensec_security *gensec_security,
+					   TALLOC_CTX *mem_ctx_out,
 					   struct auth_session_info **_session_info) 
 {
 	NTSTATUS nt_status;
@@ -1302,7 +1295,7 @@ static NTSTATUS gensec_gssapi_session_info(struct gensec_security *gensec_securi
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 		
-	mem_ctx = talloc_named(gensec_gssapi_state, 0, "gensec_gssapi_session_info context"); 
+	mem_ctx = talloc_named(mem_ctx_out, 0, "gensec_gssapi_session_info context");
 	NT_STATUS_HAVE_NO_MEMORY(mem_ctx);
 
 	nt_status = gssapi_obtain_pac_blob(mem_ctx,  gensec_gssapi_state->gssapi_context,
@@ -1391,7 +1384,7 @@ static NTSTATUS gensec_gssapi_session_info(struct gensec_security *gensec_securi
 		return nt_status;
 	}
 
-	nt_status = gensec_gssapi_session_key(gensec_security, &session_info->session_key);
+	nt_status = gensec_gssapi_session_key(gensec_security, session_info, &session_info->session_key);
 	if (!NT_STATUS_IS_OK(nt_status)) {
 		talloc_free(mem_ctx);
 		return nt_status;
@@ -1436,9 +1429,8 @@ static NTSTATUS gensec_gssapi_session_info(struct gensec_security *gensec_securi
 		/* It has been taken from this place... */
 		gensec_gssapi_state->delegated_cred_handle = GSS_C_NO_CREDENTIAL;
 	}
-	talloc_steal(gensec_gssapi_state, session_info);
+	*_session_info = talloc_steal(mem_ctx_out, session_info);
 	talloc_free(mem_ctx);
-	*_session_info = session_info;
 
 	return NT_STATUS_OK;
 }
diff --git a/source4/auth/gensec/gensec_gssapi.h b/source4/auth/gensec/gensec_gssapi.h
index 1b826b963d..246fc99a72 100644
--- a/source4/auth/gensec/gensec_gssapi.h
+++ b/source4/auth/gensec/gensec_gssapi.h
@@ -43,9 +43,6 @@ struct gensec_gssapi_state {
 	OM_uint32 want_flags, got_flags;
 	gss_OID gss_oid;
 
-	DATA_BLOB session_key;
-	DATA_BLOB pac;
-
 	struct smb_krb5_context *smb_krb5_context;
 	struct gssapi_creds_container *client_cred;
 	struct gssapi_creds_container *server_cred;
diff --git a/source4/auth/gensec/gensec_krb5.c b/source4/auth/gensec/gensec_krb5.c
index 90794b850c..b3a20e4b63 100644
--- a/source4/auth/gensec/gensec_krb5.c
+++ b/source4/auth/gensec/gensec_krb5.c
@@ -50,8 +50,6 @@ enum GENSEC_KRB5_STATE {
 };
 
 struct gensec_krb5_state {
-	DATA_BLOB session_key;
-	DATA_BLOB pac;
 	enum GENSEC_KRB5_STATE state_position;
 	struct smb_krb5_context *smb_krb5_context;
 	krb5_auth_context auth_context;
@@ -115,8 +113,6 @@ static NTSTATUS gensec_krb5_start(struct gensec_security *gensec_security, bool
 	gensec_krb5_state->ticket = NULL;
 	ZERO_STRUCT(gensec_krb5_state->enc_ticket);
 	gensec_krb5_state->keyblock = NULL;
-	gensec_krb5_state->session_key = data_blob(NULL, 0);
-	gensec_krb5_state->pac = data_blob(NULL, 0);
 	gensec_krb5_state->gssapi = gssapi;
 
 	talloc_set_destructor(gensec_krb5_state, gensec_krb5_destroy); 
@@ -559,6 +555,7 @@ static NTSTATUS gensec_krb5_update(struct gensec_security *gensec_security,
 }
 
 static NTSTATUS gensec_krb5_session_key(struct gensec_security *gensec_security, 
+					TALLOC_CTX *mem_ctx,
 					DATA_BLOB *session_key) 
 {
 	struct gensec_krb5_state *gensec_krb5_state = (struct gensec_krb5_state *)gensec_security->private_data;
@@ -571,11 +568,6 @@ static NTSTATUS gensec_krb5_session_key(struct gensec_security *gensec_security,
 		return NT_STATUS_NO_USER_SESSION_KEY;
 	}
 
-	if (gensec_krb5_state->session_key.data) {
-		*session_key = gensec_krb5_state->session_key;
-		return NT_STATUS_OK;
-	}
-
 	switch (gensec_security->gensec_role) {
 	case GENSEC_CLIENT:
 		err = krb5_auth_con_getlocalsubkey(context, auth_context, &skey);
@@ -587,9 +579,8 @@ static NTSTATUS gensec_krb5_session_key(struct gensec_security *gensec_security,
 	if (err == 0 && skey != NULL) {
 		DEBUG(10, ("Got KRB5 session key of length %d\n",  
 			   (int)KRB5_KEY_LENGTH(skey)));
-		gensec_krb5_state->session_key = data_blob_talloc(gensec_krb5_state, 
-						KRB5_KEY_DATA(skey), KRB5_KEY_LENGTH(skey));
-		*session_key = gensec_krb5_state->session_key;
+		*session_key = data_blob_talloc(mem_ctx,
+					       KRB5_KEY_DATA(skey), KRB5_KEY_LENGTH(skey));
 		dump_data_pw("KRB5 Session Key:\n", session_key->data, session_key->length);
 
 		krb5_free_keyblock(context, skey);
@@ -601,6 +592,7 @@ static NTSTATUS gensec_krb5_session_key(struct gensec_security *gensec_security,
 }
 
 static NTSTATUS gensec_krb5_session_info(struct gensec_security *gensec_security,
+					 TALLOC_CTX *mem_ctx_out,
 					 struct auth_session_info **_session_info) 
 {
 	NTSTATUS nt_status = NT_STATUS_UNSUCCESSFUL;
@@ -618,7 +610,7 @@ static NTSTATUS gensec_krb5_session_info(struct gensec_security *gensec_security
 
 	krb5_error_code ret;
 
-	TALLOC_CTX *mem_ctx = talloc_new(gensec_security);
+	TALLOC_CTX *mem_ctx = talloc_new(mem_ctx_out);
 	if (!mem_ctx) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -736,16 +728,15 @@ static NTSTATUS gensec_krb5_session_info(struct gensec_security *gensec_security
 		return nt_status;
 	}
 
-	nt_status = gensec_krb5_session_key(gensec_security, &session_info->session_key);
+	nt_status = gensec_krb5_session_key(gensec_security, session_info, &session_info->session_key);
 
 	if (!NT_STATUS_IS_OK(nt_status)) {
 		talloc_free(mem_ctx);
 		return nt_status;
 	}
 
-	*_session_info = session_info;
+	*_session_info = talloc_steal(mem_ctx_out, session_info);
 
-	talloc_steal(gensec_krb5_state, session_info);
 	talloc_free(mem_ctx);
 	return NT_STATUS_OK;
 }
diff --git a/source4/auth/gensec/pygensec.c b/source4/auth/gensec/pygensec.c
index 102836533c..6a69fd3f9d 100644
--- a/source4/auth/gensec/pygensec.c
+++ b/source4/auth/gensec/pygensec.c
@@ -257,6 +257,7 @@ static PyObject *py_gensec_set_credentials(PyObject *self, PyObject *args)
 
 static PyObject *py_gensec_session_info(PyObject *self)
 {
+	TALLOC_CTX *mem_ctx;
 	NTSTATUS status;
 	PyObject *py_session_info;
 	struct gensec_security *security = py_talloc_get_type(self, struct gensec_security);
@@ -265,7 +266,9 @@ static PyObject *py_gensec_session_info(PyObject *self)
 		PyErr_SetString(PyExc_RuntimeError, "no mechanism selected");
 		return NULL;
 	}
-	status = gensec_session_info(security, &info);
+	mem_ctx = talloc_new(NULL);
+
+	status = gensec_session_info(security, mem_ctx, &info);
 	if (NT_STATUS_IS_ERR(status)) {
 		PyErr_SetNTSTATUS(status);
 		return NULL;
@@ -273,6 +276,7 @@ static PyObject *py_gensec_session_info(PyObject *self)
 
 	py_session_info = py_return_ndr_struct("samba.dcerpc.auth", "session_info",
 						 info, info);
+	talloc_free(mem_ctx);
 	return py_session_info;
 }
 
diff --git a/source4/auth/gensec/schannel.c b/source4/auth/gensec/schannel.c
index 8f9aa921a9..67d928e710 100644
--- a/source4/auth/gensec/schannel.c
+++ b/source4/auth/gensec/schannel.c
@@ -43,7 +43,8 @@ static size_t schannel_sig_size(struct gensec_security *gensec_security, size_t
 }
 
 static NTSTATUS schannel_session_key(struct gensec_security *gensec_security,
-					    DATA_BLOB *session_key)
+				     TALLOC_CTX *mem_ctx,
+				     DATA_BLOB *session_key)
 {
 	return NT_STATUS_NOT_IMPLEMENTED;
 }
@@ -215,10 +216,11 @@ _PUBLIC_ NTSTATUS dcerpc_schannel_creds(struct gensec_security *gensec_security,
  */
 
 static NTSTATUS schannel_session_info(struct gensec_security *gensec_security,
-					 struct auth_session_info **_session_info)
+				      TALLOC_CTX *mem_ctx,
+				      struct auth_session_info **_session_info)
 {
 	struct schannel_state *state = talloc_get_type(gensec_security->private_data, struct schannel_state);
-	return auth_anonymous_session_info(state, gensec_security->settings->lp_ctx, _session_info);
+	return auth_anonymous_session_info(mem_ctx, gensec_security->settings->lp_ctx, _session_info);
 }
 
 static NTSTATUS schannel_start(struct gensec_security *gensec_security)
diff --git a/source4/auth/gensec/spnego.c b/source4/auth/gensec/spnego.c
index c48e87e8b5..66204ee262 100644
--- a/source4/auth/gensec/spnego.c
+++ b/source4/auth/gensec/spnego.c
@@ -295,6 +295,7 @@ static size_t gensec_spnego_max_wrapped_size(struct gensec_security *gensec_secu
 }
 
 static NTSTATUS gensec_spnego_session_key(struct gensec_security *gensec_security, 
+					  TALLOC_CTX *mem_ctx,
 					  DATA_BLOB *session_key)
 {
 	struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
@@ -303,11 +304,13 @@ static NTSTATUS gensec_spnego_session_key(struct gensec_security *gensec_securit
 	}
 	
 	return gensec_session_key(spnego_state->sub_sec_security, 
+				  mem_ctx,
 				  session_key);
 }
 
 static NTSTATUS gensec_spnego_session_info(struct gensec_security *gensec_security,
-								      struct auth_session_info **session_info) 
+					   TALLOC_CTX *mem_ctx,
+					   struct auth_session_info **session_info)
 {
 	struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
 	if (!spnego_state->sub_sec_security) {
@@ -315,6 +318,7 @@ static NTSTATUS gensec_spnego_session_info(struct gensec_security *gensec_securi
 	}
 	
 	return gensec_session_info(spnego_state->sub_sec_security, 
+				   mem_ctx,
 				   session_info);
 }