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.

Use explicit KDB version dependency in Fedora 26+ via BuildRequires.

With new DAL version, freeipa package will fail to build and
we'll have to add a support for new DAL version explicitly.

https://pagure.io/freeipa/issue/6776

Reviewed-By: Martin Basti <mbasti@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>

daemons/configure.ac

@@ -66,6 +66,27 @@ AC_SUBST(KRB5_LIBS)
 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 Mozilla LDAP and OpenLDAP SDK
 dnl ---------------------------------------------------------------------------

daemons/ipa-kdb/ipa_kdb.c

@@ -625,45 +625,107 @@ 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,
+    .min_ver = 0,
+    .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,
+    .free_principal = ipadb_free_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,
+    .free_policy = ipadb_free_pwd_policy,
+    .alloc = ipadb_alloc,
+    .free = ipadb_free,
+    .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
+};
+#endif
+
+#if (KRB5_KDB_DAL_MAJOR_VERSION == 6) && !defined(HAVE_KDB_FREEPRINCIPAL_EDATA)
 kdb_vftabl kdb_function_table = {
-    KRB5_KDB_DAL_MAJOR_VERSION,         /* major version number */
-    0,                                  /* minor version number */
-    ipadb_init_library,                 /* init_library */
-    ipadb_fini_library,                 /* fini_library */
-    ipadb_init_module,                  /* init_module */
-    ipadb_fini_module,                  /* fini_module */
-    ipadb_create,                       /* create */
-    NULL,                               /* destroy */
-    ipadb_get_age,                      /* get_age */
-    NULL,                               /* lock */
-    NULL,                               /* unlock */
-    ipadb_get_principal,                /* get_principal */
-    ipadb_free_principal,               /* free_principal */
-    ipadb_put_principal,                /* put_principal */
-    ipadb_delete_principal,             /* delete_principal */
-    ipadb_iterate,                      /* iterate */
-    ipadb_create_pwd_policy,            /* create_policy */
-    ipadb_get_pwd_policy,               /* get_policy */
-    ipadb_put_pwd_policy,               /* put_policy */
-    ipadb_iterate_pwd_policy,           /* iter_policy */
-    ipadb_delete_pwd_policy,            /* delete_policy */
-    ipadb_free_pwd_policy,              /* free_policy */
-    ipadb_alloc,                        /* alloc */
-    ipadb_free,                         /* free */
-    ipadb_fetch_master_key,             /* fetch_master_key */
-    NULL,                               /* fetch_master_key_list */
-    ipadb_store_master_key_list,        /* store_master_key_list */
-    NULL,                               /* dbe_search_enctype */
-    ipadb_change_pwd,                   /* change_pwd */
-    NULL,                               /* promote_db */
-    NULL,                               /* decrypt_key_data */
-    NULL,                               /* encrypt_key_data */
-    ipadb_sign_authdata,                /* sign_authdata */
-    ipadb_check_transited_realms,       /* check_transited_realms */
-    ipadb_check_policy_as,              /* check_policy_as */
-    NULL,                               /* check_policy_tgs */
-    ipadb_audit_as_req,                 /* audit_as_req */
-    NULL,                               /* refresh_config */
-    ipadb_check_allowed_to_delegate     /* check_allowed_to_delegate */
+    .maj_ver = KRB5_KDB_DAL_MAJOR_VERSION,
+    .min_ver = 0,
+    .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
 };
+#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
+
+#if (KRB5_KDB_DAL_MAJOR_VERSION != 5) && (KRB5_KDB_DAL_MAJOR_VERSION != 6)
+#error unsupported DAL major version
+#endif
 

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);

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);

freeipa.spec.in

@@ -57,7 +57,16 @@ BuildRequires:  nspr-devel
 BuildRequires:  nss-devel
 BuildRequires:  openssl-devel
 BuildRequires:  openldap-devel
+# For KDB DAL version, make explicit dependency so that increase of version
+# will cause the build to fail due to unsatisfied dependencies.
+# DAL version change may cause code crash or memory leaks, it is better to fail early.
+%if 0%{?fedora} > 25
+BuildRequires: krb5-devel >= 1.15-5
+BuildRequires: krb5-kdb-version = 6.1
+%else
+# 1.12+: libkrad (http://krbdev.mit.edu/rt/Ticket/Display.html?id=7678)
 BuildRequires:  krb5-devel >= 1.13
+%endif
 BuildRequires:  krb5-workstation
 BuildRequires:  libuuid-devel
 BuildRequires:  libcurl-devel >= 7.21.7-2