Patch 1/9: pkinit should free realm identity certificates

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Patch 1/9: pkinit should free realm identity certificates

Alexandr Nedvedicky
Hello,

I'm upgrading kerberos bundled with Solaris to krb5-1.16. Solaris currently
ships krb5-1.15.1. I've noticed there are some memory leaks, while running test
suite, which comes with krb-1.16 (e.g. running 'make check').  I don't think
those memory leaks are critical, though as kerberos newbie I can't be sure, so
I think I'm better to share my findings. All memory leaks were found using
'libumem', which can be found on Solaris (or its OSS sibbling illumos).
All patches are against krb5-1.16 release.

The first patch fixes tiny memory leak in KDC. KDC does not seem to attempt to
release identity credentials on exit at all.

regards
sasha

--------8<---------------8<---------------8<------------------8<--------

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index dbe0acbdf..d3b4ad5d8 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -952,9 +952,32 @@ pkinit_init_certs(pkinit_identity_crypto_context ctx)
     return retval;
 }
 
+static void
+pkinit_free_cred(pkinit_cred_info creds)
+{
+    if (creds != NULL) {
+ if (creds->cert != NULL) {
+    X509_free(creds->cert);
+    creds->cert = NULL;
+ }
+
+ if (creds->key != NULL) {
+    EVP_PKEY_free(creds->key);
+    creds->key = NULL;
+ }
+
+ free(creds->name);
+ creds->name = NULL;
+
+ free(creds);
+    }
+}
+
 static void
 pkinit_fini_certs(pkinit_identity_crypto_context ctx)
 {
+    unsigned int i;
+
     if (ctx == NULL)
         return;
 
@@ -972,6 +995,11 @@ pkinit_fini_certs(pkinit_identity_crypto_context ctx)
 
     if (ctx->revoked != NULL)
         sk_X509_CRL_pop_free(ctx->revoked, X509_CRL_free);
+
+    for (i = 0; i < MAX_CREDS_ALLOWED; i++) {
+ pkinit_free_cred(ctx->creds[i]);
+ ctx->creds[i] = NULL;
+    }
 }
 
 static krb5_error_code
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Patch 1/9: pkinit should free realm identity certificates

Greg Hudson
On 02/19/2018 07:47 PM, Alexandr Nedvedicky wrote:> I'm upgrading
kerberos bundled with Solaris to krb5-1.16. Solaris currently
> ships krb5-1.15.1. I've noticed there are some memory leaks, while running test
> suite, which comes with krb-1.16 (e.g. running 'make check').  I don't think
> those memory leaks are critical, though as kerberos newbie I can't be sure, so
> I think I'm better to share my findings. All memory leaks were found using
> 'libumem', which can be found on Solaris (or its OSS sibbling illumos).
> All patches are against krb5-1.16 release.

Thanks for these.  I will open one or more pull requests at
https://github.com/krb5/krb5 (unless you would prefer to do that
yourself) and review them there.

_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Patch 1/9: pkinit should free realm identity certificates

Alexandr Nedvedicky
On Tue, Feb 20, 2018 at 11:25:58AM -0500, Greg Hudson wrote:

> On 02/19/2018 07:47 PM, Alexandr Nedvedicky wrote:> I'm upgrading
> kerberos bundled with Solaris to krb5-1.16. Solaris currently
> > ships krb5-1.15.1. I've noticed there are some memory leaks, while running test
> > suite, which comes with krb-1.16 (e.g. running 'make check').  I don't think
> > those memory leaks are critical, though as kerberos newbie I can't be sure, so
> > I think I'm better to share my findings. All memory leaks were found using
> > 'libumem', which can be found on Solaris (or its OSS sibbling illumos).
> > All patches are against krb5-1.16 release.
>
> Thanks for these.  I will open one or more pull requests at
> https://github.com/krb5/krb5 (unless you would prefer to do that
> yourself) and review them there.
>

    O.K. I'll try to open those pull request, just to get the chance to 'learn
    the github'

    will do it later today or tomorrow morning my time (Europe)

regards
sasha
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev