URL: https://github.com/freeipa/freeipa/pull/410 Author: abbra Title: #410: ipa-kdb: support KDB DAL version 6.1 Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/410/head:pr410 git checkout pr410
From 284a8d4917b41fbe38ce879919196459a4a3ddfe Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> Date: Tue, 24 Jan 2017 11:02:30 +0200 Subject: [PATCH 1/2] ipa-kdb: support KDB DAL version 6.1 DAL version 6.0 removed support for a callback to free principal. This broke KDB drivers which had complex e_data structure within the principal structure. As result, FreeIPA KDB driver was leaking memory with DAL version 6.0 (krb5 1.15). DAL version 6.1 added a special callback for freeing e_data structure. See details at krb5/krb5#596 Restructure KDB driver code to provide this callback in case we are built against DAL version that supports it. For DAL version prior to 6.0 use this callback in the free_principal callback to tidy the code. https://fedorahosted.org/freeipa/ticket/6619 --- configure.ac | 21 ++++++++++++++++++ daemons/ipa-kdb/ipa_kdb.c | 42 ++++++++++++++++++++++++++++++++++-- daemons/ipa-kdb/ipa_kdb.h | 2 ++ daemons/ipa-kdb/ipa_kdb_principals.c | 42 ++++++++++++++++++++---------------- freeipa.spec.in | 4 ++++ 5 files changed, 91 insertions(+), 20 deletions(-) diff --git a/configure.ac b/configure.ac index 6cd3a89..e2f71d7 100644 --- a/configure.ac +++ b/configure.ac @@ -65,6 +65,27 @@ krb5rundir="${localstatedir}/run/krb5kdc" AC_SUBST(KRAD_LIBS) AC_SUBST(krb5rundir) +AC_CHECK_HEADER(kdb.h, [], [AC_MSG_ERROR([kdb.h not found])]) +AC_CHECK_MEMBER( + [kdb_vftabl.free_principal], + [AC_DEFINE([HAVE_KDB_FREEPRINCIPAL], [1], + [KDB driver API has free_principal callback])], + [AC_MSG_NOTICE([KDB driver API has no free_principal callback])], + [[#include <kdb.h>]]) +AC_CHECK_MEMBER( + [kdb_vftabl.free_principal_e_data], + [AC_DEFINE([HAVE_KDB_FREEPRINCIPAL_EDATA], [1], + [KDB driver API has free_principal_e_data callback])], + [AC_MSG_NOTICE([KDB driver API has no free_principal_e_data callback])], + [[#include <kdb.h>]]) + +if test "x$ac_cv_member_kdb_vftabl_free_principal" = "xno" \ + -a "x$ac_cv_member_kdb_vftable_free_principal_e_data" = "xno" ; then + AC_MSG_WARN([KDB driver API does not allow to free Kerberos principal data.]) + AC_MSG_WARN([KDB driver will leak memory on Kerberos principal use]) + AC_MSG_WARN([See https://github.com/krb5/krb5/pull/596 for details]) +fi + dnl --------------------------------------------------------------------------- dnl - Check for OpenLDAP SDK dnl --------------------------------------------------------------------------- diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index e96353f..e74ab56 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -625,6 +625,9 @@ static void ipadb_free(krb5_context context, void *ptr) /* KDB Virtual Table */ +/* We explicitly want to keep different ABI tables below separate. */ +/* Do not merge them together. Older ABI does not need to be updated */ + #if KRB5_KDB_DAL_MAJOR_VERSION == 5 kdb_vftabl kdb_function_table = { .maj_ver = KRB5_KDB_DAL_MAJOR_VERSION, @@ -657,8 +660,9 @@ kdb_vftabl kdb_function_table = { .audit_as_req = ipadb_audit_as_req, .check_allowed_to_delegate = ipadb_check_allowed_to_delegate }; +#endif -#elif KRB5_KDB_DAL_MAJOR_VERSION == 6 +#if (KRB5_KDB_DAL_MAJOR_VERSION == 6) && !defined(HAVE_KDB_FREEPRINCIPAL_EDATA) kdb_vftabl kdb_function_table = { .maj_ver = KRB5_KDB_DAL_MAJOR_VERSION, .min_ver = 0, @@ -686,8 +690,42 @@ kdb_vftabl kdb_function_table = { .audit_as_req = ipadb_audit_as_req, .check_allowed_to_delegate = ipadb_check_allowed_to_delegate }; +#endif + +#if (KRB5_KDB_DAL_MAJOR_VERSION == 6) && defined(HAVE_KDB_FREEPRINCIPAL_EDATA) +kdb_vftabl kdb_function_table = { + .maj_ver = KRB5_KDB_DAL_MAJOR_VERSION, + .min_ver = 1, + .init_library = ipadb_init_library, + .fini_library = ipadb_fini_library, + .init_module = ipadb_init_module, + .fini_module = ipadb_fini_module, + .create = ipadb_create, + .get_age = ipadb_get_age, + .get_principal = ipadb_get_principal, + .put_principal = ipadb_put_principal, + .delete_principal = ipadb_delete_principal, + .iterate = ipadb_iterate, + .create_policy = ipadb_create_pwd_policy, + .get_policy = ipadb_get_pwd_policy, + .put_policy = ipadb_put_pwd_policy, + .iter_policy = ipadb_iterate_pwd_policy, + .delete_policy = ipadb_delete_pwd_policy, + .fetch_master_key = ipadb_fetch_master_key, + .store_master_key_list = ipadb_store_master_key_list, + .change_pwd = ipadb_change_pwd, + .sign_authdata = ipadb_sign_authdata, + .check_transited_realms = ipadb_check_transited_realms, + .check_policy_as = ipadb_check_policy_as, + .audit_as_req = ipadb_audit_as_req, + .check_allowed_to_delegate = ipadb_check_allowed_to_delegate, + /* The order is important, DAL version 6.1 added + * the free_principal_e_data callback */ + .free_principal_e_data = ipadb_free_principal_e_data, +}; +#endif -#else +#if (KRB5_KDB_DAL_MAJOR_VERSION != 5) && (KRB5_KDB_DAL_MAJOR_VERSION != 6) #error unsupported DAL major version #endif diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h index 1fdb409..d5a3433 100644 --- a/daemons/ipa-kdb/ipa_kdb.h +++ b/daemons/ipa-kdb/ipa_kdb.h @@ -180,6 +180,8 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, unsigned int flags, krb5_db_entry **entry); void ipadb_free_principal(krb5_context kcontext, krb5_db_entry *entry); +/* Helper function for DAL API 6.1 or later */ +void ipadb_free_principal_e_data(krb5_context kcontext, krb5_octet *e_data); krb5_error_code ipadb_put_principal(krb5_context kcontext, krb5_db_entry *entry, char **db_args); diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 5b80909..3bd8fb8 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -1274,12 +1274,33 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, return kerr; } -void ipadb_free_principal(krb5_context kcontext, krb5_db_entry *entry) +void ipadb_free_principal_e_data(krb5_context kcontext, krb5_octet *e_data) { struct ipadb_e_data *ied; - krb5_tl_data *prev, *next; int i; + ied = (struct ipadb_e_data *)e_data; + if (ied->magic == IPA_E_DATA_MAGIC) { + ldap_memfree(ied->entry_dn); + free(ied->passwd); + free(ied->pw_policy_dn); + for (i = 0; ied->pw_history && ied->pw_history[i]; i++) { + free(ied->pw_history[i]); + } + free(ied->pw_history); + for (i = 0; ied->authz_data && ied->authz_data[i]; i++) { + free(ied->authz_data[i]); + } + free(ied->authz_data); + free(ied->pol); + free(ied); + } +} + +void ipadb_free_principal(krb5_context kcontext, krb5_db_entry *entry) +{ + krb5_tl_data *prev, *next; + if (entry) { krb5_free_principal(kcontext, entry->princ); prev = entry->tl_data; @@ -1292,22 +1313,7 @@ void ipadb_free_principal(krb5_context kcontext, krb5_db_entry *entry) ipa_krb5_free_key_data(entry->key_data, entry->n_key_data); if (entry->e_data) { - ied = (struct ipadb_e_data *)entry->e_data; - if (ied->magic == IPA_E_DATA_MAGIC) { - ldap_memfree(ied->entry_dn); - free(ied->passwd); - free(ied->pw_policy_dn); - for (i = 0; ied->pw_history && ied->pw_history[i]; i++) { - free(ied->pw_history[i]); - } - free(ied->pw_history); - for (i = 0; ied->authz_data && ied->authz_data[i]; i++) { - free(ied->authz_data[i]); - } - free(ied->authz_data); - free(ied->pol); - free(ied); - } + ipadb_free_principal_e_data(kcontext, entry->e_data); } free(entry); diff --git a/freeipa.spec.in b/freeipa.spec.in index 99820d1..104c368 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -57,8 +57,12 @@ Source0: freeipa-%{version}.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRequires: openldap-devel +%if 0%{?fedora} > 25 +BuildRequires: krb5-devel >= 1.15-5 +%else # 1.12: libkrad (http://krbdev.mit.edu/rt/Ticket/Display.html?id=7678) BuildRequires: krb5-devel >= 1.12 +%endif # 1.27.4: xmlrpc_curl_xportparms.gssapi_delegation BuildRequires: xmlrpc-c-devel >= 1.27.4 BuildRequires: popt-devel From 1d1fcf75453d6f240d44dd4d66db81e64388bc0a Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> Date: Mon, 6 Feb 2017 14:08:17 +0200 Subject: [PATCH 2/2] WIP: local build --- configure.ac | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/configure.ac b/configure.ac index e2f71d7..2ee65b7 100644 --- a/configure.ac +++ b/configure.ac @@ -1,9 +1,7 @@ -AC_PREREQ(2.59) +AC_PREREQ([2.69]) AC_CONFIG_MACRO_DIRS([m4]) m4_include(VERSION.m4) -AC_INIT([freeipa], - IPA_VERSION, - [https://hosted.fedoraproject.org/projects/freeipa/newticket]) +AC_INIT([freeipa],[IPA_VERSION],[https://hosted.fedoraproject.org/projects/freeipa/newticket]) dnl Make sure the build directory name does not contain spaces! dnl Spaces are causing problems in libtool, makefiles, autoconf itself, @@ -24,7 +22,7 @@ LT_INIT AC_HEADER_STDC -AM_CONDITIONAL([HAVE_GCC], [test "$ac_cv_prog_gcc" = yes]) +AM_CONDITIONAL([HAVE_GCC], [test "$ac_cv_c_compiler_gnu" = yes]) dnl --------------------------------------------------------------------------- dnl - Check for NSPR/NSS @@ -328,7 +326,7 @@ AC_CONFIG_COMMANDS([po/POTFILES.in], > po/POTFILES.in && dnl cd "${find_start_pwd}"]) AC_SUBST(GETTEXT_DOMAIN, [ipa]) -AM_GNU_GETTEXT_VERSION([0.18.2]) +AM_GNU_GETTEXT_VERSION([0.19.8]) AM_GNU_GETTEXT([external]) dnl integrate our custom hacks into gettextize infrastructure @@ -350,8 +348,7 @@ dnl --------------------------------------------------------------------------- dnl IPA platform dnl --------------------------------------------------------------------------- AC_ARG_WITH([ipaplatform], - [AC_HELP_STRING([--with-ipaplatform], - [IPA platform module to use])], + [AS_HELP_STRING([--with-ipaplatform],[IPA platform module to use])], [IPAPLATFORM=${withval}], [IPAPLATFORM=""]) AC_MSG_CHECKING([supported IPA platform]) @@ -411,8 +408,7 @@ dnl --------------------------------------------------------------------------- # Turn on the additional warnings last, so -Werror doesn't affect other tests. AC_ARG_ENABLE(more-warnings, - [AC_HELP_STRING([--enable-more-warnings], - [Maximum compiler warnings])], + [AS_HELP_STRING([--enable-more-warnings],[Maximum compiler warnings])], set_more_warnings="$enableval",[ if test -d $srcdir/../.hg; then set_more_warnings=yes @@ -434,9 +430,7 @@ if test "$GCC" = "yes" -a "$set_more_warnings" != "no"; then SAVE_CFLAGS="$CFLAGS" CFLAGS="$CFLAGS $option" AC_MSG_CHECKING([whether gcc understands $option]) - AC_TRY_COMPILE([], [], - has_option=yes, - has_option=no,) + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],[has_option=yes],[has_option=no]) if test $has_option = no; then CFLAGS="$SAVE_CFLAGS" fi @@ -455,8 +449,7 @@ dnl --------------------------------------------------------------------------- dnl Linters dnl --------------------------------------------------------------------------- AC_ARG_ENABLE([i18ntests], - AC_HELP_STRING([--disable-i18ntests], - [do not execute ipatests/i18n.py + AS_HELP_STRING([--disable-i18ntests],[do not execute ipatests/i18n.py (depends on python-polib)]), , [enable_i18ntests="yes"]
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
