On 04/06/2015 12:53 AM, Simo Sorce wrote:
Fix for bug 4914.

I've tested it locally and seem to do exactly what is needed. I couldn't
detect any side effects, except that if you use kadmin to get a
randomized password for a service then you'll get a key for all
supported types (currently aes256, aes128, des3, rc4, camellia128,
camellia256) instead of just the default ones (aes256, aes128, des3,
rc4) if you do not specify enctypes. I think that is fine, we use
ipa-getkeytab anyway in the normal course of business and that one uses
a different code path.

Simo.




Hi Simo,

the patch works as expected.

My only gripe is with the duplicate code in 'daemons/ipa-kdb/ipa_kdb.c' between lines 389 and 455. It could be made into a single function to get key encoding/salt types from LDAP (see my feeble and untested attempt which I attached).

--
Martin^3 Babinsky
From 455ee89dc8d449732e7f27c6c5ccd542963bd74e Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 22 May 2015 17:23:00 +0200
Subject: [PATCH] common function to get salt types from LDAP

---
 daemons/ipa-kdb/ipa_kdb.c | 129 +++++++++++++++++++++-------------------------
 daemons/ipa-kdb/ipa_kdb.h |   2 +
 2 files changed, 60 insertions(+), 71 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index fff35c9c9b4cf0a1c7fd9a4e13d1029aa01160c3..3cbd19a2e78aee53e7f24dd97a4704641d1b8d61 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -317,19 +317,66 @@ ipadb_get_global_config(struct ipadb_context *ipactx)
     return &ipactx->config;
 }
 
-int ipadb_get_connection(struct ipadb_context *ipactx)
+int ipadb_get_salt_types(struct ipadb_context *ipactx,
+                         LDAPMessage *entry, char *attr,
+                         krb5_key_salt_tuple *encs, int n_encs)
 {
     struct berval **vals = NULL;
-    struct timeval tv = { 5, 0 };
-    LDAPMessage *res = NULL;
-    LDAPMessage *first;
+    char **cvals = NULL;
+    int c = 0;
+    int i;
+    int ret = 0;
     krb5_key_salt_tuple *kst;
     int n_kst;
+
+    vals = ldap_get_values_len(ipactx->lcontext, entry, attr);
+    if (!vals || !vals[0]) {
+        goto done;
+    }
+
+    for (c = 0; vals[c]; c++) /* count */ ;
+    cvals = calloc(c, sizeof(char *));
+    if (!cvals) {
+        ret = ENOMEM;
+        goto done;
+    }
+    for (i = 0; i < c; i++) {
+        cvals[i] = strndup(vals[i]->bv_val, vals[i]->bv_len);
+        if (!cvals[i]) {
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+
+    ret = parse_bval_key_salt_tuples(ipactx->kcontext,
+                                     (const char * const *)cvals, c,
+                                     &kst, n_kst);
+    if (ret) {
+        goto done;
+    }
+
+    if (encs) {
+        free(encs);
+    }
+    encs = &kst;
+    n_encs = n_kst;
+
+done:
+    ldap_value_free_len(vals);
+    for (i = 0; i < c && cvals[i]; i++) {
+        free(cvals[i]);
+    }
+    free(cvals);
+    return ret;
+}
+
+int ipadb_get_connection(struct ipadb_context *ipactx)
+{
+    struct timeval tv = { 5, 0 };
+    LDAPMessage *res = NULL;
+    LDAPMessage *first;
     int ret;
     int v3;
-    int i;
-    char **cvals = NULL;
-    int c = 0;
 
     if (!ipactx->uri) {
         return EINVAL;
@@ -386,74 +433,20 @@ int ipadb_get_connection(struct ipadb_context *ipactx)
 
     /* defaults first, this is used to tell what default enc:salts to use
      * for kadmin password changes */
-    vals = ldap_get_values_len(ipactx->lcontext, first,
-                               "krbDefaultEncSaltTypes");
-    if (!vals || !vals[0]) {
-        goto done;
-    }
-
-    for (c = 0; vals[c]; c++) /* count */ ;
-    cvals = calloc(c, sizeof(char *));
-    if (!cvals) {
-        ret = ENOMEM;
-        goto done;
-    }
-    for (i = 0; i < c; i++) {
-        cvals[i] = strndup(vals[i]->bv_val, vals[i]->bv_len);
-        if (!cvals[i]) {
-            ret = ENOMEM;
-            goto done;
-        }
-    }
-
-    ret = parse_bval_key_salt_tuples(ipactx->kcontext,
-                                     (const char * const *)cvals, c,
-                                     &kst, &n_kst);
+    ret = ipadb_get_salt_types(ipactx, first,  "krbDefaultEncSaltTypes",
+                               ipactx->def_encs, ipactx->n_def_encs);
     if (ret) {
         goto done;
     }
 
-    if (ipactx->def_encs) {
-        free(ipactx->def_encs);
-    }
-    ipactx->def_encs = kst;
-    ipactx->n_def_encs = n_kst;
-
     /* supported enc salt types, use to tell kadmin what to accept
      * but also to detect if kadmin is requesting the default set */
-    vals = ldap_get_values_len(ipactx->lcontext, first,
-                               "krbSupportedEncSaltTypes");
-    if (!vals || !vals[0]) {
-        goto done;
-    }
-
-    for (c = 0; vals[c]; c++) /* count */ ;
-    cvals = calloc(c, sizeof(char *));
-    if (!cvals) {
-        ret = ENOMEM;
-        goto done;
-    }
-    for (i = 0; i < c; i++) {
-        cvals[i] = strndup(vals[i]->bv_val, vals[i]->bv_len);
-        if (!cvals[i]) {
-            ret = ENOMEM;
-            goto done;
-        }
-    }
-
-    ret = parse_bval_key_salt_tuples(ipactx->kcontext,
-                                     (const char * const *)cvals, c,
-                                     &kst, &n_kst);
+    ret = ipadb_get_salt_types(ipactx, first, "krbSupportedEncSaltTypes",
+                               ipactx->supp_encs, ipactx->n_supp_encs);
     if (ret) {
         goto done;
     }
 
-    if (ipactx->supp_encs) {
-        free(ipactx->supp_encs);
-    }
-    ipactx->supp_encs = kst;
-    ipactx->n_supp_encs = n_kst;
-
     /* get additional options */
     ret = ipadb_load_global_config(ipactx);
     if (ret) {
@@ -471,12 +464,6 @@ int ipadb_get_connection(struct ipadb_context *ipactx)
 done:
     ldap_msgfree(res);
 
-    ldap_value_free_len(vals);
-    for (i = 0; i < c && cvals[i]; i++) {
-        free(cvals[i]);
-    }
-    free(cvals);
-
     if (ret) {
         if (ipactx->lcontext) {
             ldap_unbind_ext_s(ipactx->lcontext, NULL, NULL);
diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index 3c6138599fe202029cfc47d3f635525e4701b4be..a72ddd0e2692d93534a7a74732cb9766e55e5273 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -130,6 +130,8 @@ struct ipadb_e_data {
 };
 
 struct ipadb_context *ipadb_get_context(krb5_context kcontext);
+int ipadb_get_salt_types(struct ipadb_context *ipactx, LDAPMessage *entry,
+                         char *attr, krb5_key_salt_tuple *encs, int n_encs);
 int ipadb_get_connection(struct ipadb_context *ipactx);
 
 /* COMMON LDAP FUNCTIONS */
-- 
2.1.0

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

Reply via email to