[PATCH] ssh-client gssapi leak and code cleanup.

[Markus Schmidt <markus@xxxxxxxxxxxx>]

https://bugzilla.mindrot.org/show_bug.cgi?id=2952

userauth_gssapi allocates a bit of memory for the authctxt->methoddata 
pointer but doesn't clean up.

Side issue: userauth_gssapi is also using two function-static variables. 
 One of these leaks.  The other one makes prevents reusability (e.g. 
porting to OO languages) because there is no way to reset it.  They 
should be moved to authctxt.

Another side issue: some gssapi-userauth related functions could be made 
static and there is a function prototype (input_gssapi_hash) that is no 
longer used.

Patch attached.


Markus

diff --git a/sshconnect2.c b/sshconnect2.c
index 0e8f323..0dab40a 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -264,6 +264,11 @@ struct cauthctxt {
 	struct cauthmethod *method;
 	sig_atomic_t success;
 	char *authlist;
+#ifdef GSSAPI
+	/* gssapi */
+	gss_OID_set gss_supported_mechs;
+	u_int mech_tried;
+#endif
 	/* pubkey */
 	struct idlist keys;
 	int agent_fd;
@@ -307,11 +312,11 @@ int	userauth_hostbased(Authctxt *);
 
 #ifdef GSSAPI
 int	userauth_gssapi(Authctxt *authctxt);
-int	input_gssapi_response(int type, u_int32_t, struct ssh *);
-int	input_gssapi_token(int type, u_int32_t, struct ssh *);
-int	input_gssapi_hash(int type, u_int32_t, struct ssh *);
-int	input_gssapi_error(int, u_int32_t, struct ssh *);
-int	input_gssapi_errtok(int, u_int32_t, struct ssh *);
+void	userauth_gssapi_cleanup(Authctxt *authctxt);
+static int input_gssapi_response(int type, u_int32_t, struct ssh *);
+static int input_gssapi_token(int type, u_int32_t, struct ssh *);
+static int input_gssapi_error(int, u_int32_t, struct ssh *);
+static int input_gssapi_errtok(int, u_int32_t, struct ssh *);
 #endif
 
 void	userauth(Authctxt *, char *);
@@ -330,7 +335,7 @@ Authmethod authmethods[] = {
 #ifdef GSSAPI
 	{"gssapi-with-mic",
 		userauth_gssapi,
-		NULL,
+		userauth_gssapi_cleanup,
 		&options.gss_authentication,
 		NULL},
 #endif
@@ -389,6 +394,10 @@ ssh_userauth2(struct ssh *ssh, const char *local_user,
 	authctxt.info_req_seen = 0;
 	authctxt.attempt_kbdint = 0;
 	authctxt.attempt_passwd = 0;
+#if GSSAPI
+	authctxt.gss_supported_mechs = NULL;;
+	authctxt.mech_tried = 0;
+#endif
 	authctxt.agent_fd = -1;
 	pubkey_prepare(&authctxt);
 	if (authctxt.method == NULL) {
@@ -683,26 +692,24 @@ userauth_gssapi(Authctxt *authctxt)
 {
 	struct ssh *ssh = active_state; /* XXX */
 	Gssctxt *gssctxt = NULL;
-	static gss_OID_set gss_supported = NULL;
-	static u_int mech = 0;
 	OM_uint32 min;
 	int r, ok = 0;
 
 	/* Try one GSSAPI method at a time, rather than sending them all at
 	 * once. */
 
-	if (gss_supported == NULL)
-		gss_indicate_mechs(&min, &gss_supported);
+	if (authctxt->gss_supported_mechs == NULL)
+		gss_indicate_mechs(&min, &authctxt->gss_supported_mechs);
 
 	/* Check to see if the mechanism is usable before we offer it */
-	while (mech < gss_supported->count && !ok) {
+	while (authctxt->mech_tried < authctxt->gss_supported_mechs->count && !ok) {
 		/* My DER encoding requires length<128 */
-		if (gss_supported->elements[mech].length < 128 &&
+		if (authctxt->gss_supported_mechs->elements[authctxt->mech_tried].length < 128 &&
 		    ssh_gssapi_check_mechanism(&gssctxt,
-		    &gss_supported->elements[mech], authctxt->host)) {
+		    &authctxt->gss_supported_mechs->elements[authctxt->mech_tried], authctxt->host)) {
 			ok = 1; /* Mechanism works */
 		} else {
-			mech++;
+			authctxt->mech_tried++;
 		}
 	}
 
@@ -717,13 +724,13 @@ userauth_gssapi(Authctxt *authctxt)
 	    (r = sshpkt_put_cstring(ssh, authctxt->method->name)) != 0 ||
 	    (r = sshpkt_put_u32(ssh, 1)) != 0 ||
 	    (r = sshpkt_put_u32(ssh,
-	    (gss_supported->elements[mech].length) + 2)) != 0 ||
+	    (authctxt->gss_supported_mechs->elements[authctxt->mech_tried].length) + 2)) != 0 ||
 	    (r = sshpkt_put_u8(ssh, SSH_GSS_OIDTYPE)) != 0 ||
 	    (r = sshpkt_put_u8(ssh,
-	    gss_supported->elements[mech].length)) != 0 ||
+	    authctxt->gss_supported_mechs->elements[authctxt->mech_tried].length)) != 0 ||
 	    (r = sshpkt_put(ssh,
-	    gss_supported->elements[mech].elements,
-	    gss_supported->elements[mech].length)) != 0 ||
+	    authctxt->gss_supported_mechs->elements[authctxt->mech_tried].elements,
+	    authctxt->gss_supported_mechs->elements[authctxt->mech_tried].length)) != 0 ||
 	    (r = sshpkt_send(ssh)) != 0)
 		fatal("%s: %s", __func__, ssh_err(r));
 
@@ -732,11 +739,23 @@ userauth_gssapi(Authctxt *authctxt)
 	ssh_dispatch_set(ssh, SSH2_MSG_USERAUTH_GSSAPI_ERROR, &input_gssapi_error);
 	ssh_dispatch_set(ssh, SSH2_MSG_USERAUTH_GSSAPI_ERRTOK, &input_gssapi_errtok);
 
-	mech++; /* Move along to next candidate */
+	authctxt->mech_tried++; /* Move along to next candidate */
 
 	return 1;
 }
 
+void
+userauth_gssapi_cleanup(Authctxt *authctxt)
+{
+
+	Gssctxt *gssctxt = (Gssctxt *)authctxt->methoddata;
+	ssh_gssapi_delete_ctx(&gssctxt);
+	authctxt->methoddata = NULL;
+
+	free(authctxt->gss_supported_mechs);
+	authctxt->gss_supported_mechs = NULL;
+}
+
 static OM_uint32
 process_gssapi_token(struct ssh *ssh, gss_buffer_t recv_tok)
 {

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev