Fix error clobbering bug and code review comments

appl/dbutils/bsearch.c

@@ -66,29 +66,9 @@ struct getargs args[] = {
 static int num_args = sizeof(args) / sizeof(args[0]);
 
 static void
-usage(const char *progname, int status)
+usage(int status)
 {
-    arg_printusage(args, num_args, progname, "\n"
+    arg_printusage(args, num_args, NULL, "file [key ...]");
-"\tThis program does a binary search of the given file for the\n"
-"\tgiven keys.  Two binary search algorithms are implemented\n"
-"\twhole-file and block-wise.\n\n"
-"\tIf keys are not given as arguments keys are read from stdin.\n\n"
-"\tExit status will be 1 for errors, 2 if any keys are not found,\n"
-"\tand 0 if all keys are found.\n\n"
-"\tOptions:\n"
-"\t\t-K \tPrint keys\n"
-"\t\t-V \tDon't print values\n"
-"\t\t-b size\tUse block-wise search with give blocksize\n"
-"\t\t-m size\tRead DB in if its size is less than given\n"
-"\t\t-v \tVerbose (includes count of reads and comparisons)\n"
-"\t\t-h \tPrint usage message and exit\n"
-"\tIf blocksize is not given, empty, or zero then the\n"
-"\tfilesystem's block size (st_blksize) will be used.\n"
-"\tBlock sizes should be powers of two, and larger than 256.\n"
-"\tIf the max file size is not given or empty then the max\n"
-"\tfile size for non-block-wise search will be 1MB.\n"
-"\tKeys from stdin must not be longer than 1023 bytes.\n\n"
-	    );
     exit(status);
 }
 
@@ -99,7 +79,6 @@ int
 main(int argc, char **argv)
 {
     char keybuf[1024];
-    char *progname = argv[0];
     char *fname;
     char *key = keybuf;
     char *value;
@@ -116,8 +95,9 @@ main(int argc, char **argv)
     int blockwise;
     int ret = 0;
 
+    setprogname(argv[0]);
     if (getarg(args, num_args, argc, argv, &optidx))
-	usage(progname, 1);
+	usage(1);
 
     if (version_flag) {
 	print_version(NULL);
@@ -125,7 +105,7 @@ main(int argc, char **argv)
     }
 
     if (help_flag)
-	usage(progname, 0);
+	usage(0);
 
     if (block_size_int != 0 && block_size_int < 512) {
 	fprintf(stderr, "Invalid block size: too small\n");
@@ -147,14 +127,14 @@ main(int argc, char **argv)
 	block_size = block_size_int;
     }
     if (max_size_int < 0)
-	usage(progname, 1);
+	usage(1);
     max_size = max_size_int;
 
     argc -= optind;
     argv += optind;
 
     if (argc == 0)
-	usage(progname, 1);
+	usage(1);
 
     fname = argv[0];
     argc--;

lib/krb5/aname_to_localname.c

@@ -186,7 +186,7 @@ an2ln_plugin(krb5_context context, const char *rule, krb5_const_principal aname,
 	ret = KRB5_CONFIG_NOTENUFSPACE;
 
     heim_release(ctx.luser);
-    return 0;
+    return ret;
 }
 
 static void
@@ -287,12 +287,23 @@ an2ln_local_names(krb5_context context,
  */
 static krb5_error_code
 an2ln_default(krb5_context context,
-	      int root_princs_ok,
+	      char *rule,
 	      krb5_const_principal aname,
 	      size_t lnsize, char *lname)
 {
     krb5_error_code ret;
     const char *res;
+    int root_princs_ok;
+
+    if (strcmp(rule, "NONE") == 0)
+	return KRB5_NO_LOCALNAME;
+
+    if (strcmp(rule, "DEFAULT") == 0)
+	root_princs_ok = 0;
+    else if (strcmp(rule, "HEIMDAL_DEFAULT") == 0)
+	root_princs_ok = 1;
+    else
+	return KRB5_PLUGIN_NO_HANDLE;
 
     if (!princ_realm_is_default(context, aname))
 	return KRB5_PLUGIN_NO_HANDLE;
@@ -303,7 +314,7 @@ an2ln_default(krb5_context context,
 	 * component is the username.
 	 */
 	res = aname->name.name_string.val[0];
-    } else if (aname->name.name_string.len == 2 &&
+    } else if (root_princs_ok && aname->name.name_string.len == 2 &&
 	       strcmp (aname->name.name_string.val[1], "root") == 0) {
 	/*
 	 * Two-component principal names in default realm where the
@@ -372,41 +383,42 @@ krb5_aname_to_localname(krb5_context context,
 				    "auth_to_local", NULL);
     if (!rules) {
 	/* Heimdal's default rule */
-	ret = an2ln_default(context, 1, aname, lnsize, lname);
+	ret = an2ln_default(context, "HEIMDAL_DEFAULT", aname, lnsize, lname);
 	if (ret == KRB5_PLUGIN_NO_HANDLE)
 	    return KRB5_NO_LOCALNAME;
 	return ret;
     }
 
-    /* MIT rules */
+    /*
+     * MIT rules.
+     *
+     * Note that RULEs and DBs only have white-list functionality,
+     * thus RULEs and DBs that we don't understand we simply ignore.
+     *
+     * This means that plugins that implement black-lists are
+     * dangerous: if a black-list plugin isn't found, the black-list
+     * won't be enforced.  But black-lists are dangerous anyways.
+     */
     for (ret = KRB5_PLUGIN_NO_HANDLE, i = 0; rules[i]; i++) {
 	rule = rules[i];
-	if (!*rule || strcmp(rule, "NONE") == 0)
+
-	    break;
+	/* Try NONE, DEFAULT, and HEIMDAL_DEFAULT rules */
-	else if (strcmp(rule, "HEIMDAL_DEFAULT") == 0)
+	ret = an2ln_default(context, rule, aname, lnsize, lname);
-	    ret = an2ln_default(context, 1, aname, lnsize, lname);
+	if (ret == KRB5_PLUGIN_NO_HANDLE)
-	else if (strcmp(rule, "DEFAULT") == 0)
+	    /* Try DB, RULE, ... plugins */
-	    ret = an2ln_default(context, 0, aname, lnsize, lname);
-	else
-	    /* Let the plugins handle DBs and RULEs and anything else*/
 	    ret = an2ln_plugin(context, rule, aname, lnsize, lname);
 
-	if (ret == 0 && lnsize && lname[0])
+	if (ret == 0 && lnsize && !lname[0])
-	    break;
+	    continue; /* Success but no lname?!  lies! */
-	/*
+	else if (ret != KRB5_PLUGIN_NO_HANDLE)
-	 * Note that RULEs and DBs only have white-list functionality,
-	 * thus RULEs and DBs that we don't understand we simply ignore.
-	 *
-	 * This means that plugins that implement black-lists are
-	 * dangerous: if a black-list plugin isn't found, the black-list
-	 * won't be enforced.  But black-lists are dangerous anyways.
-	 */
-	if (ret != KRB5_PLUGIN_NO_HANDLE)
 	    break;
     }
 
-    if (ret == KRB5_PLUGIN_NO_HANDLE)
+    if (ret == KRB5_PLUGIN_NO_HANDLE) {
+	if (lnsize)
+	    lname[0] = '\0';
 	ret = KRB5_NO_LOCALNAME;
+    }
 
     krb5_config_free_strings(rules);
     return ret;

lib/krb5/test_alname.c

@@ -34,13 +34,33 @@
 #include <getarg.h>
 #include <err.h>
 
+char localname[1024];
+static size_t lname_size = sizeof (localname);
+static int lname_size_arg = 0;
+static int simple_flag = 0;
+static int verbose_flag = 0;
+static int version_flag = 0;
+static int help_flag	= 0;
+
+static struct getargs args[] = {
+    {"lname-size",	0,	arg_integer,	&lname_size_arg,
+     "set localname size (0 means use default, must be 0..1023)", "integer" },
+    {"simple",	0,	arg_flag,	&simple_flag, /* Used for scripting */
+     "map the given principal and print the resulting localname", NULL },
+    {"verbose",	0,	arg_flag,	&verbose_flag,
+     "print the actual principal name as well as the localname", NULL },
+    {"version",	0,	arg_flag,	&version_flag,
+     "print version", NULL },
+    {"help",	0,	arg_flag,	&help_flag,
+     NULL, NULL }
+};
+
 static void
 test_alname(krb5_context context, krb5_const_realm realm,
 	    const char *user, const char *inst,
 	    const char *localuser, int ok)
 {
     krb5_principal p;
-    char localname[1024];
     krb5_error_code ret;
     char *princ;
 
@@ -52,7 +72,7 @@ test_alname(krb5_context context, krb5_const_realm realm,
     if (ret)
 	krb5_err(context, 1, ret, "krb5_unparse_name");
 
-    ret = krb5_aname_to_localname(context, p, sizeof(localname), localname);
+    ret = krb5_aname_to_localname(context, p, lname_size, localname);
     krb5_free_principal(context, p);
     if (ret) {
 	if (!ok) {
@@ -76,22 +96,6 @@ test_alname(krb5_context context, krb5_const_realm realm,
 
 }
 
-static int simple_flag = 0;
-static int verbose_flag = 0;
-static int version_flag = 0;
-static int help_flag	= 0;
-
-static struct getargs args[] = {
-    {"simple",	0,	arg_flag,	&simple_flag, /* Used for scripting */
-     "map the given principal and print the resulting localname", NULL },
-    {"verbose",	0,	arg_flag,	&verbose_flag,
-     "print the actual principal name as well as the localname", NULL },
-    {"version",	0,	arg_flag,	&version_flag,
-     "print version", NULL },
-    {"help",	0,	arg_flag,	&help_flag,
-     NULL, NULL }
-};
-
 static void
 usage (int ret)
 {
@@ -133,9 +137,9 @@ main(int argc, char **argv)
 
     if (simple_flag) {
 	krb5_principal princ;
-	char localname[1024];
 	char *unparsed;
 	krb5_error_code ret;
+	int status = 0;
 
 	/* Map then print the result and exit */
 	if (argc != 1)
@@ -149,23 +153,40 @@ main(int argc, char **argv)
 	if (ret)
 	    krb5_err(context, 1, ret, "krb5_unparse_name");
 
-	ret = krb5_aname_to_localname(context, princ, sizeof(localname),
+	if (lname_size_arg > 0 && lname_size_arg < 1024)
-				      localname);
+	    lname_size = lname_size_arg;
+	else if (lname_size_arg != 0)
+	    errx(1, "local name size must be between 0 and 1023 (inclusive)");
+
+	ret = krb5_aname_to_localname(context, princ, lname_size, localname);
 	if (ret == KRB5_NO_LOCALNAME) {
 	    if (verbose_flag)
 		fprintf(stderr, "No mapping obtained for %s\n", unparsed);
 	    exit(1);
 	}
-	if (ret == KRB5_PLUGIN_NO_HANDLE) {
+	switch (ret) {
+	case KRB5_PLUGIN_NO_HANDLE:
 	    fprintf(stderr, "Error: KRB5_PLUGIN_NO_HANDLE leaked!\n");
-	    exit(2);
+	    status = 2;
+	    break;
+	case KRB5_CONFIG_NOTENUFSPACE:
+	    fprintf(stderr, "Error: lname-size (%lu) too small\n",
+		    (long unsigned)lname_size);
+	    status = 3;
+	    break;
+	case 0:
+	    if (verbose_flag)
+		printf("%s ", unparsed);
+	    printf("%s\n", localname);
+	    break;
+	default:
+	    krb5_err(context, 4, ret, "krb5_aname_to_localname");
+	    break;
 	}
-	if (verbose_flag)
-	    printf("%s ", unparsed);
 	free(unparsed);
 	krb5_free_principal(context, princ);
-	printf("%s\n", localname);
+	krb5_free_context(context);
-	exit(0);
+	exit(status);
     }
 
     if (argc != 1)

tests/kdc/check-an2ln.in

@@ -43,6 +43,9 @@ test_alname="${test_alname} --simple"
 rm -f localname
 
 check_localname() {
+    if test "$2" -ne 0; then
+	exec 2> /dev/null
+    fi
     ${test_alname} "$1" > localname
     status=$?
     if test $status -ne "$2"; then
@@ -99,6 +102,15 @@ check_localname bar/mapped1@${R4} 1
 check_localname foo/notmapped1@${R4} 1
 check_localname bar/notmapped1@${R4} 1
 
+echo "Checking for overflow"
+test_alname="${test_alname} --simple --lname-size=1"
+check_localname mapped1@${R} 3
+check_localname mapped2@${R} 3
+check_localname mapped1@${R2} 3
+check_localname mapped2@${R2} 3
+check_localname mapped1@${R3} 3
+check_localname mapped2@${R3} 3
+
 rm -f messages.log
 
 exit 0