[Freeipa-devel] [PATCH] Fixup fix for 4914

[Simo Sorce]

On Fri, 2015-05-29 at 18:59 +0300, Alexander Bokovoy wrote:
> On Fri, 29 May 2015, Simo Sorce wrote:
> >The patches for ticket 4914 worked fine on Fedora 22 (and in general any
> >system that was updated to krb5 1.13) however they fail in Fedora 21 and
> >similar because of a bug in one of the libkrb5 functions used in the new
> >code. The bug is fixed in 1.13 but not in older versions as it causes
> >side effects in kadmin output.
> >
> >The attached patch takes care of using a replacement function if we
> >detect at runtime that the library in use does not have the fixes
> >present in 1.13. This allows us the freedom to backport or not the 1.13
> >fix.
> >
> >Unfortunately I am running out of time today so I could not test it, but
> >I still wanted to put it out there to get this fixed asap.
> >
> >Milan, or Martin, can you please test it ?
> >
> >Simo.
> >
> >-- 
> >Simo Sorce * Red Hat, Inc * New York
> 
> >>From ea7811f7d11b68a34dc357d0e0dcb7d81c7f65c8 Mon Sep 17 00:00:00 2001
> >From: Simo Sorce <simo at redhat.com>
> >Date: Fri, 29 May 2015 11:18:17 -0400
> >Subject: [PATCH] Add compatibility function for older libkrb5
> >
> >Before krb5 1.13 the krb5_salttype_to_string() function was returning
> >incorrect names (display names of some kind instead of the names
> >used by the rest of the library to map saltname to the salt type
> >integer number).
> >This patch adds a function that checks at runtime if we have a working
> >function and uses a fallback map updated to the salt types known up
> >to 1.12, this allows us to use the library provided function in
> >following releases where new salt types may emerge.
> >
> >Signed-off-by: Simo Sorce <simo at redhat.com>
> >---
> > util/ipa_krb5.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 60 insertions(+), 1 deletion(-)
> >
> >diff --git a/util/ipa_krb5.c b/util/ipa_krb5.c
> >index 65e10dd401edf6b54988fc4bfa5a2e08789b7b75..d6992c561830ff682ede3a156ad9efbfff701432 100644
> >--- a/util/ipa_krb5.c
> >+++ b/util/ipa_krb5.c
> >@@ -1075,6 +1075,65 @@ int create_keys(krb5_context krbctx,
> >     return nkeys;
> > }
> > 
> >+/* in older versions of libkrb5 the krb5_salttype_to_string() function is
> >+ * faulty and returns strings that do not match the expected format.
> >+ * Later version of krb5 were fixed to return the proper string.
> >+ * Do lazy detection the first time the function is invoked to determine
> >+ * if we can use the library provided function or if we have to use a
> >+ * fallback map which includes the salt types known up to krb5 1.12 (the
> >+ * fault is fixed upstream in 1.13). */
> >+static int ipa_salttype_to_string(krb5_int32 salttype,
> >+                                  char *buffer, size_t buflen)
> >+{
> >+    static int faulty_function = -1;
> >+
> >+    static const struct {
> >+        krb5_int32 salttype;
> >+        const char *name;
> >+    } fallback_map[] = {
> >+        { KRB5_KDB_SALTTYPE_NORMAL, "normal" },
> >+        { KRB5_KDB_SALTTYPE_V4, "v4" },
> >+        { KRB5_KDB_SALTTYPE_NOREALM, "norealm" },
> >+        { KRB5_KDB_SALTTYPE_ONLYREALM, "onlyrealm" },
> >+        { KRB5_KDB_SALTTYPE_SPECIAL, "speacial" },
> There is a typo in 'special' in the KRB5_KDB_SALTTYPE_SPECIAL entry.
> 
> It needs to be fixed before we get this ACKed.

Sigh, and I re-read the list 3 times because this was the most obvious
error I could do ... the only consolation is that testing would have
failed immediately.
Thanks a lot! I'll send a new patch in minutes

Simo.

> >+        { KRB5_KDB_SALTTYPE_AFS3, "afs3" },
> >+        { -1, NULL }
> >+    };
> >+
> >+    if (faulty_function == -1) {
> >+        /* haven't checked yet, let's find out */
> >+        char testbuf[100];
> >+        size_t len = 100;
> >+        int ret;
> >+
> >+        ret = krb5_salttype_to_string(KRB5_KDB_SALTTYPE_NORMAL, testbuf, len);
> >+        if (ret) return ret;
> >+
> >+        if (strcmp(buffer, "normal") == 0) {
> >+            faulty_function = 0;
> >+        } else {
> >+            faulty_function = 1;
> >+        }
> >+    }
> >+
> >+    if (faulty_function == 0) {
> >+        return krb5_salttype_to_string(salttype, buffer, buflen);
> >+    } else {
> >+        size_t len;
> >+        int i;
> >+        for (i = 0; fallback_map[i].name != NULL; i++) {
> >+            if (salttype == fallback_map[i].salttype) break;
> >+        }
> >+        if (fallback_map[i].name == NULL) return EINVAL;
> >+
> >+        len = strlen(fallback_map[i].name);
> >+        if (len >= buflen) return ENOMEM;
> >+
> >+        memcpy(buffer, fallback_map[i].name, len + 1);
> >+        return 0;
> >+    }
> >+}
> >+
> > int ipa_kstuples_to_string(krb5_key_salt_tuple *kst, int n_kst, char **str)
> > {
> >     char *buf = NULL;
> >@@ -1130,7 +1189,7 @@ int ipa_kstuples_to_string(krb5_key_salt_tuple *kst, int n_kst, char **str)
> >         buf[buf_cur + len] = ':';
> >         len++;
> > 
> >-        ret = krb5_salttype_to_string(kst[i].ks_salttype,
> >+        ret = ipa_salttype_to_string(kst[i].ks_salttype,
> >                                      &buf[buf_cur + len], buf_avail - len);
> >         if (ret == ENOMEM) {
> >             i--;
> >-- 
> >2.4.1
> >
> 
> >-- 
> >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
> 
> 


-- 
Simo Sorce * Red Hat, Inc * New York