Patch 5/9: pkinit_load_fs_cert_and_key() must not blindly overwrite pointer
Alexandr Nedvedicky
alexandr.nedvedicky at oracle.com
Mon Feb 19 19:47:09 EST 2018
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.
I'm sure the patch fixes the leak, however the change might be rather
fixing symptoms than a real problem. The story of this leak
is bit convoluted. The memory culprit has stack as follows:
f9c4f408 libc_hwcap1.so.1`__lwp_sigqueue+0x15(1, 6, f9c4f428, f6af9c7e)
f9c4f428 libc_hwcap1.so.1`raise+0x22(6, 0, f9c4f478, f6acb71e)
f9c4f478 libc_hwcap1.so.1`abort+0xe6(65737341, 6f697472, 6166206e, 64656c69, 6469203a, 7972635f)
f9c4f688 0xf6acc669(f5f805d0, f5f819c0, 1205, f5fb198c, 868bd30, 8692c08)
f9c4f6d8 pkinit.so`pkinit_load_fs_cert_and_key+0x188(8670dc8, 8677d08, 8692c88, 8692c08, 0, 8677d08)
f9c4f718 pkinit.so`pkinit_get_certs_fs+0xc1(8670dc8, 869bf88, 0, 87cff88, 8677d08, 0)
f9c4f768 pkinit.so`crypto_load_certs+0x59(8670dc8, 869bf88, 0, 87cff88, 8677d08, 0)
f9c4f7b8 pkinit.so`pkinit_identity_prompt+0xb7(8670dc8, 869bf88, 0, 87cff88, 8677d08, 0)
f9c4f808 pkinit.so`pkinit_server_plugin_init_realm+0x27a(8670dc8, 86698d0, f9c4f828, f5f8c3cb)
f9c4f858 pkinit.so`pkinit_server_plugin_init+0x169(8670dc8, f9c4f888, 8658868, f9c4f884)
f9c4f8c8 load_preauth_plugins+0x146(80c4a70)
f9c4f968 main+0x2e9(2, f9c4f998, f9c4f9a4)
f9c4f98c _start+0x72(2, f9c4fb2d, f9c4fb35, 0, f9c4fb38, f9c4fb3f)
As you can see the call stack comes from process, which tripped
my interim diagnostic assert here:
4583 static krb5_error_code
4584 pkinit_load_fs_cert_and_key(krb5_context context,
4585 pkinit_identity_crypto_context id_cryptoctx,
4586 char *certname,
4587 char *keyname,
4588 int cindex)
4589 {
4590 krb5_error_code retval;
4591 X509 *x = NULL;
4592 EVP_PKEY *y = NULL;
4593 char *fsname = NULL;
....
4608 /* Load the key. */
4609 retval = get_key(context, id_cryptoctx, keyname, fsname, &y, password);
4610 if (retval != 0 || y == NULL) {
4611 retval = oerr(context, 0, _("Cannot read key file '%s'"), fsname);
4612 goto cleanup;
4613 }
4614
4615 assert(id_cryptoctx->creds[cindex] == NULL);
4616
4617 id_cryptoctx->creds[cindex] = malloc(sizeof(struct _pkinit_cred_info));
4618 if (id_cryptoctx->creds[cindex] == NULL) {
4619 retval = ENOMEM;
4620 goto cleanup;
4621 }
The resulting core file gave me a chance to find out, where buffer, which gets
overwritten got allocated. The story starts at pkinit_server_plugin_init_realm()
1386 static int
1387 pkinit_server_plugin_init_realm(krb5_context context, const char *realmname,
1388 pkinit_kdc_context *pplgctx)
1389 {
....
1421 retval = pkinit_init_identity_opts(&plgctx->idopts);
1422 if (retval)
1423 goto errout;
1424
1425 retval = pkinit_init_kdc_profile(context, plgctx);
1426 if (retval)
1427 goto errout;
1428
/*
* this is where we are going to call crypto_load_certs()
* for the first time. The function will load certificates to
* id_cryptoctx->creds[0]. See call to pkinit_load_fs_cert_and_key() in
* pkinit_get_certs_fs(), which specifically asks to use slot 0.
*/
1429 retval = pkinit_identity_initialize(context, plgctx->cryptoctx, NULL,
1430 plgctx->idopts, plgctx->idctx,
1431 NULL, NULL, NULL);
1432 if (retval)
1433 goto errout;
/*
* this is where we are going to call crypto_load_certs() for the
* second time. The call essentially takes same path as earlier call to
* pkinit_identity_initialize(). It will also load certificate to slot
* 0.
*/
1434 retval = pkinit_identity_prompt(context, plgctx->cryptoctx, NULL,
1435 plgctx->idopts, plgctx->idctx,
1436 NULL, NULL, 0, NULL);
1437 if (retval)
1438 goto errout;
1439
1440 pkiDebug("%s: returning context at %p for realm '%s'\n",
1441 __FUNCTION__, plgctx, realmname);
1442 *pplgctx = plgctx;
1443 retval = 0;
I've tried to fix this bug by using slot 0 for certificate loaded
via pkinit_identity_initialize() and slot 1 for certificate,
loaded via pkinit_identity_prompt(). However such fix breaks
t_pkinit.py test:
krb5-1.16/src/tests/t_pkinit.py
*** Failure: /export/home/sashan/kerberos/components/krb5/build/i86/clients/kinit/kinit failed with code 1.
*** Last command (#2): /export/home/sashan/kerberos/components/krb5/build/i86/clients/kinit/kinit -X X509_user_identity=PKCS12:/export/home/sashan/kerberos/components/krb5/krb5-1.16/src/tests/dejagnu/pkinit-certs/user-upn2.p12 user at KRBTEST.COM
*** Output of last command:
Password for user at KRBTEST.COM:
kinit: Pre-authentication failed: Cannot read password while getting initial credentials
For details, see: /export/home/sashan/kerberos/components/krb5/build/i86/tests/testlog
Or re-run this test script with the -v flag
cd /export/home/sashan/kerberos/components/krb5/build/i86/tests
PYTHONPATH=/export/home/sashan/kerberos/components/krb5/krb5-1.16/src/util /usr/bin/python /export/home/sashan/kerberos/components/krb5/krb5-1.16/src/tests/t_pkinit.py -v
Use --debug=NUM to run a command under a debugger. Use
--stop-after=NUM to stop after a daemon is started in order to
attach to it with a debugger. Use --help to see other options.
make[2]: *** [Makefile:761: check-pytests] Error 1
make[2]: Leaving directory '/export/home/sashan/kerberos/components/krb5/build/i86/tests'
make[1]: *** [Makefile:1525: check-recurse] Error 1
make[1]: Leaving directory '/export/home/sashan/kerberos/components/krb5/build/i86'
gmake: *** [/export/home/sashan/kerberos/make-rules/configure.mk:220: /export/home/sashan/kerberos/components/krb5/build/i86/.tested] Error 2
I'm sure this patch needs an kerberos export eyes.
thanks and
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 d3b4ad5d8..19bf1c359 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -4609,6 +4609,11 @@ pkinit_load_fs_cert_and_key(krb5_context context,
goto cleanup;
}
+ if (id_cryptoctx->creds[cindex] != NULL) {
+ pkinit_free_cred(id_cryptoctx->creds[cindex]);
+ id_cryptoctx->creds[cindex] = NULL;
+ }
+
id_cryptoctx->creds[cindex] = malloc(sizeof(struct _pkinit_cred_info));
if (id_cryptoctx->creds[cindex] == NULL) {
retval = ENOMEM;
@@ -4658,8 +4663,9 @@ pkinit_get_certs_fs(krb5_context context,
}
retval = pkinit_load_fs_cert_and_key(context, id_cryptoctx,
- idopts->cert_filename,
- idopts->key_filename, 0);
+ idopts->cert_filename,
+ idopts->key_filename, 0);
+
cleanup:
return retval;
}
More information about the krbdev
mailing list