On Fri, 2011-11-04 at 16:14 -0400, Simo Sorce wrote: > On Fri, 2011-11-04 at 15:59 -0400, Simo Sorce wrote: > > On Fri, 2011-11-04 at 15:15 -0400, Nalin Dahyabhai wrote: > > > On Thu, Nov 03, 2011 at 06:26:15PM -0400, Simo Sorce wrote: > > > > As stated in the bug in order to attain better interoperability with > > > > Windows clients we need to change the way we generate the random salt. > > > > > > Nack. The data in a krb5_data is of type 'char', and if it's signed, > > > the math used here doesn't produce a printable result. Might also want > > > to increase KRB5P_SALT_SIZE. > > > > Ah crap, right. > > > > I initially used a safe construct: data[i] &= 0x5F > > Then realized that one of the possible values (5F + 20 = 7F) is > > unprintable, so I switched to this unsafe one. > > > > Will get a revised patch for ipa-2-1 and an amendment for master. > > > > Thanks a lot for spotting this one! > > Attached amendment patch for master and an already amended new patch for > ipa-2-1.
After a quick review with nalin offline I decided for a different approach that properly covers the range of values we want and is more similar to the initial code. New patches attached. -- Simo Sorce * Red Hat, Inc * New York
>From cae692dc4ed817185d51f438a4f1a170b92c324c Mon Sep 17 00:00:00 2001 From: Simo Sorce <sso...@redhat.com> Date: Fri, 4 Nov 2011 16:40:25 -0400 Subject: [PATCH] Amend #2038 fix The math was unsafe, thanks to Nalin for spotting it. --- util/ipa_krb5.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/util/ipa_krb5.c b/util/ipa_krb5.c index ba9d3cefce0944d790715c3249f158b9f0ae232d..d03680a6ed3bceb73516d17f5dcef8594fbc382e 100644 --- a/util/ipa_krb5.c +++ b/util/ipa_krb5.c @@ -13,7 +13,7 @@ static krb5_error_code ipa_get_random_salt(krb5_context krbctx, krb5_data *salt) { krb5_error_code kerr; - int i; + int i, v; /* make random salt */ salt->length = KRB5P_SALT_SIZE; @@ -30,8 +30,10 @@ static krb5_error_code ipa_get_random_salt(krb5_context krbctx, * To avoid any compatibility issue, limits octects only to * the ASCII printable range, or 0x20 <= val <= 0x7E */ for (i = 0; i < salt->length; i++) { - salt->data[i] %= 0x5E; /* 7E - 20 */ - salt->data[i] += 0x20; /* add base */ + v = (unsigned char)salt->data[i]; + v %= 0x5E; /* 7E - 20 */ + v += 0x20; /* add base */ + salt->data[i] = v; } return 0; -- 1.7.7
>From e82ee7c2fed958b2532adb224a8dcb21fa7f6caa Mon Sep 17 00:00:00 2001 From: Simo Sorce <sso...@redhat.com> Date: Thu, 3 Nov 2011 16:15:10 -0400 Subject: [PATCH] Modify random salt creation for interoperability port to ipa-2-1 ameneded math safety issue See: https://fedorahosted.org/freeipa/ticket/2038 --- .../ipa-pwd-extop/ipapwd_encoding.c | 40 ++++++++++++++++---- 1 files changed, 32 insertions(+), 8 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c index cd4610c6ffd6f1b4eae61521335a7e26d319fa9d..fd51ed5db50eb25935b7943859c6d29097d73445 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_encoding.c @@ -47,6 +47,7 @@ #include <sys/stat.h> #include <fcntl.h> #include <unistd.h> +#include <errno.h> #include <dirsrv/slapi-plugin.h> #include <lber.h> @@ -249,6 +250,36 @@ void encode_int16(unsigned int val, unsigned char *p) p[0] = (val ) & 0xff; } +static krb5_error_code ipa_get_random_salt(krb5_context krbctx, + krb5_data *salt) +{ + krb5_error_code kerr; + int i, v; + + /* make random salt */ + salt->length = KRB5P_SALT_SIZE; + salt->data = malloc(KRB5P_SALT_SIZE); + if (!salt->data) { + return ENOMEM; + } + kerr = krb5_c_random_make_octets(krbctx, salt); + if (kerr) { + return kerr; + } + + /* Windows treats the salt as a string. + * To avoid any compatibility issue, limits octects only to + * the ASCII printable range, or 0x20 <= val <= 0x7E */ + for (i = 0; i < salt->length; i++) { + v = (unsigned char)salt->data[i]; + v %= 0x5E; /* 7E - 20 */ + v += 0x20; /* add base */ + salt->data[i] = v; + } + + return 0; +} + static Slapi_Value **encrypt_encode_key(struct ipapwd_krbcfg *krbcfg, struct ipapwd_data *data, char **errMesg) @@ -376,14 +407,7 @@ static Slapi_Value **encrypt_encode_key(struct ipapwd_krbcfg *krbcfg, case KRB5_KDB_SALTTYPE_SPECIAL: - /* make random salt */ - salt.length = KRB5P_SALT_SIZE; - salt.data = malloc(KRB5P_SALT_SIZE); - if (!salt.data) { - LOG_OOM(); - goto enc_error; - } - krberr = krb5_c_random_make_octets(krbctx, &salt); + krberr = ipa_get_random_salt(krbctx, &salt); if (krberr) { LOG_FATAL("krb5_c_random_make_octets failed [%s]\n", krb5_get_error_message(krbctx, krberr)); -- 1.7.7
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel