From 95bd79c14715d69399338dfff8acedd6bdf6e93e Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Tue, 23 Aug 2016 12:35:50 -0400 Subject: Fix GSSRPC server credential memory leak In svc_auth_gss.c, stop using the global svcauth_gss_creds, and instead keep a credential in struct svc_rpc_gss_data. This change ensures that the same credential is used for each accept_sec_context call for a particular context, and ensures that the credential is freed when the authentication data is destroyed. Also, do not acquire a credential when the default name is used (as it is in kadmind) as it is not needed. Leave the svcauth_gss_creds around for the backportable fix as it is in the library export list. It will be removed in a subsequent commit. (cherry picked from commit 670d9828086e979d5cdfd26f00ca88958a03754e) ticket: 8480 version_fixed: 1.13.7 --- src/lib/rpc/svc_auth_gss.c | 40 ++++++++++++++-------------------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/src/lib/rpc/svc_auth_gss.c b/src/lib/rpc/svc_auth_gss.c index b81c4a3..efd3e51 100644 --- a/src/lib/rpc/svc_auth_gss.c +++ b/src/lib/rpc/svc_auth_gss.c @@ -91,6 +91,7 @@ struct svc_auth_ops svc_auth_gss_ops = { struct svc_rpc_gss_data { bool_t established; /* context established */ + gss_cred_id_t cred; /* credential */ gss_ctx_id_t ctx; /* context id */ struct rpc_gss_sec sec; /* security triple */ gss_buffer_desc cname; /* GSS client name */ @@ -139,15 +140,23 @@ svcauth_gss_set_svc_name(gss_name_t name) } static bool_t -svcauth_gss_acquire_cred(void) +svcauth_gss_acquire_cred(struct svc_rpc_gss_data *gd) { OM_uint32 maj_stat, min_stat; log_debug("in svcauth_gss_acquire_cred()"); + /* We don't need to acquire a credential if using the default name. */ + if (svcauth_gss_name == GSS_C_NO_NAME) + return (TRUE); + + /* Only acquire a credential once per authentication. */ + if (gd->cred != GSS_C_NO_CREDENTIAL) + return (TRUE); + maj_stat = gss_acquire_cred(&min_stat, svcauth_gss_name, 0, GSS_C_NULL_OID_SET, GSS_C_ACCEPT, - &svcauth_gss_creds, NULL, NULL); + &gd->cred, NULL, NULL); if (maj_stat != GSS_S_COMPLETE) { log_status("gss_acquire_cred", maj_stat, min_stat); @@ -156,25 +165,6 @@ svcauth_gss_acquire_cred(void) return (TRUE); } -static bool_t -svcauth_gss_release_cred(void) -{ - OM_uint32 maj_stat, min_stat; - - log_debug("in svcauth_gss_release_cred()"); - - maj_stat = gss_release_cred(&min_stat, &svcauth_gss_creds); - - if (maj_stat != GSS_S_COMPLETE) { - log_status("gss_release_cred", maj_stat, min_stat); - return (FALSE); - } - - svcauth_gss_creds = NULL; - - return (TRUE); -} - /* Invoke log_badauth callbacks for an authentication failure. */ static void badauth(OM_uint32 maj, OM_uint32 minor, SVCXPRT *xprt) @@ -210,7 +200,7 @@ svcauth_gss_accept_sec_context(struct svc_req *rqst, gr->gr_major = gss_accept_sec_context(&gr->gr_minor, &gd->ctx, - svcauth_gss_creds, + gd->cred, &recv_tok, GSS_C_NO_CHANNEL_BINDINGS, &gd->client_name, @@ -494,7 +484,7 @@ gssrpc__svcauth_gss(struct svc_req *rqst, struct rpc_msg *msg, if (rqst->rq_proc != NULLPROC) ret_freegc (AUTH_FAILED); /* XXX ? */ - if (!svcauth_gss_acquire_cred()) + if (!svcauth_gss_acquire_cred(gd)) ret_freegc (AUTH_FAILED); if (!svcauth_gss_accept_sec_context(rqst, &gr)) @@ -544,9 +534,6 @@ gssrpc__svcauth_gss(struct svc_req *rqst, struct rpc_msg *msg, log_debug("sendreply in destroy: %d", call_stat); - if (!svcauth_gss_release_cred()) - ret_freegc (AUTH_FAILED); - SVCAUTH_DESTROY(rqst->rq_xprt->xp_auth); rqst->rq_xprt->xp_auth = &svc_auth_none; @@ -574,6 +561,7 @@ svcauth_gss_destroy(SVCAUTH *auth) gd = SVCAUTH_PRIVATE(auth); gss_delete_sec_context(&min_stat, &gd->ctx, GSS_C_NO_BUFFER); + gss_release_cred(&min_stat, &gd->cred); gss_release_buffer(&min_stat, &gd->cname); gss_release_buffer(&min_stat, &gd->checksum); -- cgit v1.1