From 15734117beac425fe4e7b5a513af497115eff687 Mon Sep 17 00:00:00 2001 From: Tom Yu Date: Mon, 1 Feb 2010 21:48:19 +0000 Subject: [PATCH] set_default_enctype_var should filter not reject weak enctypes With allow_weak_crypto=false, set_default_enctype_var() (helper function for krb5_set_default_tgs_enctypes(), etc.) was rejecting any application-provided enctype list that contained any weak enctype even when valid strong enctypes were present. This broke some Samba things. Filter the weak enctypes instead. Add test cases. Reported to Debian by Holger Isenberg. (Debian bug #566977) http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=566977 Thanks to Simo Sorce for testing. ticket: 6653 tags: pullup target_version: 1.8 git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@23681 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/krb/init_ctx.c | 28 ++++++--- src/lib/krb5/krb/t_etypes.c | 109 ++++++++++++++++++++++++++++-------- 2 files changed, 105 insertions(+), 32 deletions(-) diff --git a/src/lib/krb5/krb/init_ctx.c b/src/lib/krb5/krb/init_ctx.c index 2c2beb6bf..e72534ccc 100644 --- a/src/lib/krb5/krb/init_ctx.c +++ b/src/lib/krb5/krb/init_ctx.c @@ -277,19 +277,31 @@ set_default_etype_var(krb5_context context, const krb5_enctype *etypes, { krb5_error_code code; krb5_enctype *list; - int i; + size_t src, dst; if (etypes) { - for (i = 0; etypes[i]; i++) { - if (!krb5_c_valid_enctype(etypes[i])) - return KRB5_PROG_ETYPE_NOSUPP; - if (!context->allow_weak_crypto && krb5int_c_weak_enctype(etypes[i])) - return KRB5_PROG_ETYPE_NOSUPP; - } - + /* Empty list passed in. */ + if (etypes[0] == 0) + return EINVAL; code = krb5int_copy_etypes(etypes, &list); if (code) return code; + + /* Filter list in place to exclude invalid and (optionally) weak + * enctypes. */ + for (src = dst = 0; list[src]; src++) { + if (!krb5_c_valid_enctype(list[src])) + continue; + if (!context->allow_weak_crypto + && krb5int_c_weak_enctype(list[src])) + continue; + list[dst++] = list[src]; + } + list[dst] = 0; /* Zero-terminate. */ + if (dst == 0) { + free(list); + return KRB5_CONFIG_ETYPE_NOSUPP; + } } else { list = NULL; } diff --git a/src/lib/krb5/krb/t_etypes.c b/src/lib/krb5/krb/t_etypes.c index 47763e594..f84ffee4f 100644 --- a/src/lib/krb5/krb/t_etypes.c +++ b/src/lib/krb5/krb/t_etypes.c @@ -34,51 +34,60 @@ static struct { krb5_enctype defaults[64]; krb5_enctype expected_noweak[64]; krb5_enctype expected[64]; + krb5_error_code expected_err_noweak; + krb5_error_code expected_err_weak; } tests[] = { /* Empty string, unused default list */ { "", { ENCTYPE_DES_CBC_CRC, 0 }, { 0 }, - { 0 } + { 0 }, + 0, 0 }, /* Single weak enctype */ { "des-cbc-md4", { 0 }, { 0 }, - { ENCTYPE_DES_CBC_MD4, 0 } + { ENCTYPE_DES_CBC_MD4, 0 }, + 0, 0 }, /* Single non-weak enctype */ { "aes128-cts-hmac-sha1-96", { 0 }, { ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 }, - { ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 } + { ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 }, + 0, 0 }, /* Two enctypes, one an alias, one weak */ { "rc4-hmac des-cbc-md5", { 0 }, { ENCTYPE_ARCFOUR_HMAC, 0 }, - { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_DES_CBC_MD5, 0 } + { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_DES_CBC_MD5, 0 }, + 0, 0 }, /* Three enctypes, all weak, case variation, funky separators */ { " deS-HMac-shA1 , arCFour-hmaC-mD5-exp\tdeS3-Cbc-RAw\n", { 0 }, { 0 }, { ENCTYPE_DES_HMAC_SHA1, ENCTYPE_ARCFOUR_HMAC_EXP, - ENCTYPE_DES3_CBC_RAW, 0 } + ENCTYPE_DES3_CBC_RAW, 0 }, + 0, 0 }, /* Default set with enctypes added (one weak in each pair) */ { "DEFAULT des-cbc-raw +des3-hmac-sha1", { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_ARCFOUR_HMAC_EXP, 0 }, { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_DES3_CBC_SHA1, 0 }, { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_ARCFOUR_HMAC_EXP, - ENCTYPE_DES_CBC_RAW, ENCTYPE_DES3_CBC_SHA1, 0 } + ENCTYPE_DES_CBC_RAW, ENCTYPE_DES3_CBC_SHA1, 0 }, + 0, 0 }, /* Default set with enctypes removed */ { "default -aes128-cts -des-hmac-sha1", { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, ENCTYPE_DES_CBC_MD5, ENCTYPE_DES_HMAC_SHA1, 0 }, { ENCTYPE_AES256_CTS_HMAC_SHA1_96, 0 }, - { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES_CBC_MD5, 0 } + { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES_CBC_MD5, 0 }, + 0, 0 }, /* Family followed by enctype */ { "aes des3-cbc-sha1-kd", @@ -86,14 +95,16 @@ static struct { { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, ENCTYPE_DES3_CBC_SHA1, 0 }, { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, - ENCTYPE_DES3_CBC_SHA1, 0 } + ENCTYPE_DES3_CBC_SHA1, 0 }, + 0, 0 }, /* Enctype followed by two families */ { "+rc4-hmAC des3 +des", { 0 }, { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_DES3_CBC_SHA1, 0 }, { ENCTYPE_ARCFOUR_HMAC, ENCTYPE_DES3_CBC_SHA1, ENCTYPE_DES_CBC_CRC, - ENCTYPE_DES_CBC_MD5, ENCTYPE_DES_CBC_MD4 } + ENCTYPE_DES_CBC_MD5, ENCTYPE_DES_CBC_MD4 }, + 0, 0 }, /* Default set with family added and enctype removed */ { "DEFAULT +aes -arcfour-hmac-md5", @@ -101,7 +112,8 @@ static struct { { ENCTYPE_DES3_CBC_SHA1, ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 }, { ENCTYPE_DES3_CBC_SHA1, ENCTYPE_DES_CBC_CRC, - ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 } + ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 }, + 0, 0 }, /* Default set with families removed and enctypes added (one redundant) */ { "DEFAULT -des -des3 rc4-hmac rc4-hmac-exp", @@ -111,7 +123,8 @@ static struct { { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, ENCTYPE_ARCFOUR_HMAC, 0 }, { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, - ENCTYPE_ARCFOUR_HMAC, ENCTYPE_ARCFOUR_HMAC_EXP, 0 } + ENCTYPE_ARCFOUR_HMAC, ENCTYPE_ARCFOUR_HMAC_EXP, 0 }, + 0, 0 }, /* Default set with family moved to front */ { "des3 +DEFAULT", @@ -120,14 +133,38 @@ static struct { { ENCTYPE_DES3_CBC_SHA1, ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 }, { ENCTYPE_DES3_CBC_SHA1, ENCTYPE_AES256_CTS_HMAC_SHA1_96, - ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 } + ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 }, + 0, 0 }, /* Two families with default set removed (exotic case), enctype added */ { "aes +rc4 -DEFaulT des3-hmac-sha1", { ENCTYPE_AES128_CTS_HMAC_SHA1_96, ENCTYPE_DES3_CBC_SHA1, ENCTYPE_ARCFOUR_HMAC, 0 }, { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES3_CBC_SHA1, 0 }, - { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES3_CBC_SHA1, 0 } + { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES3_CBC_SHA1, 0 }, + 0, 0 + }, + /* Test krb5_set_default_in_tkt_ktypes */ + { NULL, + { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES_CBC_CRC, 0 }, + { ENCTYPE_AES256_CTS_HMAC_SHA1_96, 0 }, + { ENCTYPE_AES256_CTS_HMAC_SHA1_96, ENCTYPE_DES_CBC_CRC, 0 }, + 0, 0 + }, + /* Should get KRB5_CONFIG_ETYPE_NOSUPP if app-provided list has no strong + * enctypes and allow_weak_crypto=false. */ + { NULL, + { ENCTYPE_DES_CBC_CRC, 0 }, + { 0 }, + { ENCTYPE_DES_CBC_CRC, 0 }, + KRB5_CONFIG_ETYPE_NOSUPP, 0 + }, + /* Should get EINVAL if app provides an empty list. */ + { NULL, + { 0 }, + { 0 }, + { 0 }, + EINVAL, EINVAL } }; @@ -150,13 +187,16 @@ compare(krb5_context ctx, krb5_enctype *result, krb5_enctype *expected, { unsigned int i; + if (!result) + return; for (i = 0; result[i]; i++) { if (result[i] != expected[i]) break; } if (!result[i] && !expected[i]) /* Success! */ return; - fprintf(stderr, "Unexpected result while parsing: %s\n", profstr); + if (profstr != NULL) + fprintf(stderr, "Unexpected result while parsing: %s\n", profstr); fprintf(stderr, "Expected: "); show_enctypes(ctx, expected); fprintf(stderr, "Result: "); @@ -169,7 +209,7 @@ int main(int argc, char **argv) { krb5_context ctx; - krb5_error_code ret; + krb5_error_code ret, expected_err; krb5_enctype *list; krb5_boolean weak; unsigned int i; @@ -183,18 +223,39 @@ main(int argc, char **argv) for (i = 0; i < sizeof(tests) / sizeof(*tests); i++) { for (weak = FALSE; weak <= TRUE; weak++) { ctx->allow_weak_crypto = weak; - copy = strdup(tests[i].str); - ret = krb5int_parse_enctype_list(ctx, copy, tests[i].defaults, - &list); - if (ret) { - com_err("krb5int_parse_enctype_list", ret, ""); - return 2; + if (weak) + expected_err = tests[i].expected_err_weak; + else + expected_err = tests[i].expected_err_noweak; + + if (tests[i].str != NULL) { + copy = strdup(tests[i].str); + ret = krb5int_parse_enctype_list(ctx, copy, tests[i].defaults, + &list); + if (ret != expected_err) { + com_err("krb5int_parse_enctype_list", ret, ""); + return 2; + } + } else { + /* No string; test the filtering on the set_default_etype_var + * instead. */ + copy = NULL; + list = NULL; + ret = krb5_set_default_in_tkt_ktypes(ctx, tests[i].defaults); + if (ret != expected_err) { + com_err("krb5_set_default_in_tkt_ktypes", ret, ""); + return 2; + } + } + if (!expected_err) { + compare(ctx, tests[i].str ? list : ctx->in_tkt_etypes, + (weak) ? tests[i].expected : tests[i].expected_noweak, + tests[i].str, weak); } - compare(ctx, list, - (weak) ? tests[i].expected : tests[i].expected_noweak, - tests[i].str, weak); free(copy); free(list); + if (!tests[i].str) + krb5_set_default_in_tkt_ktypes(ctx, NULL); } } -- 2.26.2