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

diff --git a/src/lib/krb5/krb/init_ctx.c b/src/lib/krb5/krb/init_ctx.c
index 2c2beb6bfc5bbea77ae959f67bbfb74c61fca21c..e72534cccbf3dc12c1ca0cd8ebd3cc66e5274132 100644 (file)
--- 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 47763e594258a801f93d34f8b32769fd0415f91d..f84ffee4fbeabea42e312232585eaa278a88ec6e 100644 (file)
--- 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);
         }
     }