Rework error-mapping code to preserve status code values when returned

by only one mechanism.  Revert RPC code to relying on this.

Build error-mapping code on a bidirectional map instead of a simple
array.  When a status code is returned but has been seen returned from
a different mechanism already, generate a new number, starting at
100,000.

Use gssrpcint_printf for some more debugging code.

ticket: 5654

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@19919 dc483132-0cff-0310-8789-dd5450dbe970

5 files changed, 179 insertions, 84 deletions

diff --git a/src/lib/gssapi/generic/Makefile.in b/src/lib/gssapi/generic/Makefile.in
index d32e8b2..d09a678 100644
--- a/src/lib/gssapi/generic/Makefile.in
+++ b/src/lib/gssapi/generic/Makefile.in
@@ -114,18 +114,20 @@ all-unix:: $(EXPORTED_HEADERS) $(ETHDRS) $(HDRS)
 all-unix:: all-libobjs
 
 errmap.h: $(SRCTOP)/util/gen.pl $(SRCTOP)/util/t_array.pm \
-		$(SRCTOP)/util/t_enum.pm $(SRCTOP)/util/t_tsenum.pm
-	$(PERL) -w -I$(SRCTOP)/util $(SRCTOP)/util/gen.pl tsenum errmap.h \
-		NAME=mecherrmap TYPE="struct mecherror" COMPARE=mecherror_cmp \
-		COPY=mecherror_copy PRINT=mecherror_print
+		$(SRCTOP)/util/t_bimap.pm
+	$(PERL) -w -I$(SRCTOP)/util $(SRCTOP)/util/gen.pl bimap errmap.h \
+		NAME=mecherrmap LEFT=OM_uint32 RIGHT="struct mecherror" \
+		LEFTPRINT=print_OM_uint32 RIGHTPRINT=mecherror_print \
+		LEFTCMP=cmp_OM_uint32 RIGHTCMP=mecherror_cmp
 
 maptest.h: $(SRCTOP)/util/gen.pl $(SRCTOP)/util/t_array.pm \
-		$(SRCTOP)/util/t_enum.pm $(SRCTOP)/util/t_tsenum.pm
-	$(PERL) -w -I$(SRCTOP)/util $(SRCTOP)/util/gen.pl tsenum maptest.h \
-		NAME=foo TYPE=elt COMPARE=eltcmp COPY=eltcp PRINT=eltprt
+		$(SRCTOP)/util/t_bimap.pm
+	$(PERL) -w -I$(SRCTOP)/util $(SRCTOP)/util/gen.pl bimap maptest.h \
+		NAME=foo LEFT=int RIGHT=elt LEFTPRINT=intprt RIGHTPRINT=eltprt \
+		LEFTCMP=intcmp RIGHTCMP=eltcmp
 maptest.o: maptest.c maptest.h
 maptest: maptest.o
-	$(CC_LINK) -o maptest maptest.o $(SUPPORT_LIB)
+	$(CC_LINK) -o maptest maptest.o
 
 ##DOS##LIBOBJS = $(OBJS)
 
diff --git a/src/lib/gssapi/generic/maptest.c b/src/lib/gssapi/generic/maptest.c
index b884eed..28b4b06 100644
--- a/src/lib/gssapi/generic/maptest.c
+++ b/src/lib/gssapi/generic/maptest.c
@@ -24,6 +24,18 @@ static void eltprt(elt v, FILE *f)
 {
     fprintf(f, "{%d,%d}", v.a, v.b);
 }
+static int intcmp(int left, int right)
+{
+    if (left < right)
+	return -1;
+    if (left > right)
+	return 1;
+    return 0;
+}
+static void intprt(int v, FILE *f)
+{
+    fprintf(f, "%d", v);
+}
 
 #include "maptest.h"
 
@@ -31,24 +43,25 @@ foo foo1;
 
 int main ()
 {
-    int err;
     elt v1 = { 1, 2 }, v2 = { 3, 4 };
-    long idx;
-    int added;
+    const elt *vp;
+    const int *ip;
 
-    err = foo_init(&foo1);
-    assert(err == 0);
-    err = foo_find_or_append(&foo1, v1, &idx, &added);
-    assert(err == 0);
-    printf("v1: idx=%ld added=%d\n", idx, added);
-    err = foo_find_or_append(&foo1, v2, &idx, &added);
-    assert(err == 0);
-    printf("v2: idx=%ld added=%d\n", idx, added);
-    err = foo_find_or_append(&foo1, v2, &idx, &added);
-    assert(err == 0);
-    printf("v2: idx=%ld added=%d\n", idx, added);
-    err = foo_find_or_append(&foo1, v1, &idx, &added);
-    assert(err == 0);
-    printf("v1: idx=%ld added=%d\n", idx, added);
+    assert(0 == foo_init(&foo1));
+    vp = foo_findleft(&foo1, 47);
+    assert(vp == NULL);
+    assert(0 == foo_add(&foo1, 47, v1));
+    vp = foo_findleft(&foo1, 47);
+    assert(vp != NULL);
+    assert(0 == eltcmp(*vp, v1));
+    vp = foo_findleft(&foo1, 3);
+    assert(vp == NULL);
+    assert(0 == foo_add(&foo1, 93, v2));
+    ip = foo_findright(&foo1, v1);
+    assert(ip != NULL);
+    assert(*ip == 47);
+    printf("Map content: ");
+    foo_printmap(&foo1, stdout);
+    printf("\n");
     return 0;
 }
diff --git a/src/lib/gssapi/generic/util_errmap.c b/src/lib/gssapi/generic/util_errmap.c
index 6f7f170..5cd554c 100644
--- a/src/lib/gssapi/generic/util_errmap.c
+++ b/src/lib/gssapi/generic/util_errmap.c
@@ -1,3 +1,28 @@
+/*
+ * Copyright 2007 by the Massachusetts Institute of Technology.
+ * All Rights Reserved.
+ *
+ * Export of this software from the United States of America may
+ *   require a specific license from the United States Government.
+ *   It is the responsibility of any person or organization contemplating
+ *   export to obtain such a license before exporting.
+ * 
+ * WITHIN THAT CONSTRAINT, permission to use, copy, modify, and
+ * distribute this software and its documentation for any purpose and
+ * without fee is hereby granted, provided that the above copyright
+ * notice appear in all copies and that both that copyright notice and
+ * this permission notice appear in supporting documentation, and that
+ * the name of M.I.T. not be used in advertising or publicity pertaining
+ * to distribution of the software without specific, written prior
+ * permission.  Furthermore if you modify this software you must label
+ * your software as modified software and not distribute it in such a
+ * fashion that it might be confused with the original M.I.T. software.
+ * M.I.T. makes no representations about the suitability of
+ * this software for any purpose.  It is provided "as is" without express
+ * or implied warranty.
+ * 
+ */
+
 #include "gssapiP_generic.h"
 #include <string.h>
 #ifndef _WIN32
@@ -17,6 +42,17 @@ struct mecherror {
 };
 
 static inline int
+cmp_OM_uint32(OM_uint32 m1, OM_uint32 m2)
+{
+    if (m1 < m2)
+	return -1;
+    else if (m1 > m2)
+	return 1;
+    else
+	return 0;
+}
+
+static inline int
 mecherror_cmp(struct mecherror m1, struct mecherror m2)
 {
     if (m1.code < m2.code)
@@ -32,14 +68,22 @@ mecherror_cmp(struct mecherror m1, struct mecherror m2)
     return memcmp(m1.mech.elements, m2.mech.elements, m1.mech.length);
 }
 
+static void
+print_OM_uint32 (OM_uint32 value, FILE *f)
+{
+    fprintf(f, "%lu", (unsigned long) value);
+}
+
 static inline int
 mecherror_copy(struct mecherror *dest, struct mecherror src)
 {
     *dest = src;
-    if (src.mech.length) {
-	dest->mech.elements = malloc(src.mech.length);
-	if (dest->mech.elements == NULL)
+    dest->mech.elements = malloc(src.mech.length);
+    if (dest->mech.elements == NULL) {
+	if (src.mech.length)
 	    return ENOMEM;
+	else
+	    return 0;
     }
     memcpy(dest->mech.elements, src.mech.elements, src.mech.length);
     return 0;
@@ -66,6 +110,7 @@ mecherror_print(struct mecherror value, FILE *f)
 	fprintf(f, "(com_err)");
 	return;
     }
+    fprintf(f, "%p=", value.mech.elements);
     if (generic_gss_oid_to_str(&minor, &value.mech, &str)) {
 	fprintf(f, "(error in conversion)");
 	return;
@@ -86,44 +131,20 @@ mecherror_print(struct mecherror value, FILE *f)
 #include "krb5.h"		/* for KRB5KRB_AP_WRONG_PRINC */
 
 static mecherrmap m;
+static k5_mutex_t mutex = K5_MUTEX_PARTIAL_INITIALIZER;
+static OM_uint32 next_fake = 100000;
 
 int gssint_mecherrmap_init(void)
 {
     int err;
-    OM_uint32 n;
 
     err = mecherrmap_init(&m);
     if (err)
 	return err;
-
-    /* This is *so* gross.
-
-       The RPC code depends on being able to recognize the "wrong
-       principal" minor status return from the Kerberos mechanism.
-       But a totally generic enumeration of status codes as they come
-       up makes that impossible.  So "register" that status code
-       early, and always with the same value.
-
-       Of course, to make things worse, we're treating each mechanism
-       OID separately, and there are three for Kerberos.  */
-    {
-	/* Declare here to avoid including header files not generated
-	   yet.  */
-	extern const gss_OID_desc *const gss_mech_krb5;
-	extern const gss_OID_desc *const gss_mech_krb5_old;
-	extern const gss_OID_desc *const gss_mech_krb5_wrong;
-
-	const OM_uint32 wrong_princ = (OM_uint32) KRB5KRB_AP_WRONG_PRINC;
-
-	n = gssint_mecherrmap_map(wrong_princ, gss_mech_krb5);
-	if (n <= 0)
-	    return ENOMEM;
-	n = gssint_mecherrmap_map(wrong_princ, gss_mech_krb5_old);
-	if (n <= 0)
-	    return ENOMEM;
-	n = gssint_mecherrmap_map(wrong_princ, gss_mech_krb5_wrong);
-	if (n <= 0)
-	    return ENOMEM;
+    err = k5_mutex_finish_init(&mutex);
+    if (err) {
+	mecherrmap_destroy(&m);
+	return err;
     }
 
     return 0;
@@ -142,21 +163,87 @@ void gssint_mecherrmap_destroy(void)
 {
     mecherrmap_foreach(&m, free_one, NULL);
     mecherrmap_destroy(&m);
+    k5_mutex_destroy(&mutex);
 }
 
 OM_uint32 gssint_mecherrmap_map(OM_uint32 minor, const gss_OID_desc * oid)
 {
-    struct mecherror me;
-    int err, added;
-    long idx;
+    const struct mecherror *mep;
+    struct mecherror me, me_copy;
+    const OM_uint32 *p;
+    int err;
+    OM_uint32 new_status;
+
+#ifdef DEBUG
+    FILE *f;
+    f = fopen("/dev/pts/9", "w+");
+    if (f == NULL)
+	f = stderr;
+#endif
 
     me.code = minor;
     me.mech = *oid;
-    err = mecherrmap_find_or_append(&m, me, &idx, &added);
+    err = k5_mutex_lock(&mutex);
     if (err) {
+#ifdef DEBUG
+	if (f != stderr) fclose(f);
+#endif
 	return 0;
     }
-    return idx+1;
+
+    /* Is this status+oid already mapped?  */
+    p = mecherrmap_findright(&m, me);
+    if (p != NULL) {
+	k5_mutex_unlock(&mutex);
+#ifdef DEBUG
+	fprintf(f, "%s: found ", __func__);
+	mecherror_print(me, f);
+	fprintf(f, " in map as %lu\n", (unsigned long) *p);
+	if (f != stderr) fclose(f);
+#endif
+	return *p;
+    }
+    /* Is this status code already mapped to something else
+       mech-specific?  */
+    mep = mecherrmap_findleft(&m, minor);
+    if (mep == NULL) {
+	/* Map it to itself plus this mech-oid.  */
+	new_status = minor;
+    } else {
+	/* Already assigned.  Pick a fake new value and map it.  */
+	/* There's a theoretical infinite loop risk here, if we fill
+	   in 2**32 values.  Also, returning 0 has a special
+	   meaning.  */
+	do {
+	    next_fake++;
+	    new_status = next_fake;
+	    if (new_status == 0)
+		/* ??? */;
+	} while (mecherrmap_findleft(&m, new_status) != NULL);
+    }
+    err = mecherror_copy(&me_copy, me);
+    if (err) {
+	k5_mutex_unlock(&mutex);
+	return err;
+    }
+    err = mecherrmap_add(&m, new_status, me_copy);
+    k5_mutex_unlock(&mutex);
+    if (err) {
+	if (me_copy.mech.length)
+	    free(me_copy.mech.elements);
+    }
+#ifdef DEBUG
+    fprintf(f, "%s: mapping ", __func__);
+    mecherror_print(me, f);
+    fprintf(f, " to %lu: err=%d\nnew map: ", (unsigned long) new_status, err);
+    mecherrmap_printmap(&m, f);
+    fprintf(f, "\n");
+    if (f != stderr) fclose(f);
+#endif
+    if (err)
+	return 0;
+    else
+	return new_status;
 }
 
 static gss_OID_desc no_oid = { 0, 0 };
@@ -168,25 +255,21 @@ OM_uint32 gssint_mecherrmap_map_errcode(OM_uint32 errcode)
 int gssint_mecherrmap_get(OM_uint32 minor, gss_OID mech_oid,
 			  OM_uint32 *mech_minor)
 {
-    struct mecherror me;
+    const struct mecherror *p;
     int err;
-    long size;
 
     if (minor == 0) {
 	return EINVAL;
     }
-    err = mecherrmap_size(&m, &size);
-    if (err) {
+    err = k5_mutex_lock(&mutex);
+    if (err)
 	return err;
-    }
-    if (minor > size) {
+    p = mecherrmap_findleft(&m, minor);
+    k5_mutex_unlock(&mutex);
+    if (!p) {
 	return EINVAL;
     }
-    err = mecherrmap_get(&m, minor-1, &me);
-    if (err) {
-	return err;
-    }
-    *mech_oid = me.mech;
-    *mech_minor = me.code;
+    *mech_oid = p->mech;
+    *mech_minor = p->code;
     return 0;
 }
diff --git a/src/lib/rpc/auth_gssapi.c b/src/lib/rpc/auth_gssapi.c
index 3d6e6fe..bd185bc 100644
--- a/src/lib/rpc/auth_gssapi.c
+++ b/src/lib/rpc/auth_gssapi.c
@@ -22,7 +22,8 @@
 
 #ifdef DEBUG_GSSAPI
 int auth_debug_gssapi = DEBUG_GSSAPI;
-#define L_PRINTF(l,args) if (auth_debug_gssapi >= l) printf args
+extern void gssrpcint_printf(const char *format, ...);
+#define L_PRINTF(l,args) if (auth_debug_gssapi >= l) gssrpcint_printf args
 #define PRINTF(args) L_PRINTF(99, args)
 #define AUTH_GSSAPI_DISPLAY_STATUS(args) \
 	if (auth_debug_gssapi) auth_gssapi_display_status args
diff --git a/src/lib/rpc/svc_auth_gssapi.c b/src/lib/rpc/svc_auth_gssapi.c
index cb1e8f9..f899c89 100644
--- a/src/lib/rpc/svc_auth_gssapi.c
+++ b/src/lib/rpc/svc_auth_gssapi.c
@@ -402,8 +402,8 @@ enum auth_stat gssrpc__svcauth_gssapi(
 	       if (server_creds == client_data->server_creds)
 		    break;
 
-	       gssrpcint_printf("accept_sec_context returned 0x%x 0x%x\n",
-				call_res.gss_major, call_res.gss_minor);
+	       PRINTF(("accept_sec_context returned 0x%x 0x%x wrong-princ=%#x\n",
+		       call_res.gss_major, call_res.gss_minor, KRB5KRB_AP_WRONG_PRINC));
 	       if (call_res.gss_major == GSS_S_COMPLETE ||
 		   call_res.gss_major == GSS_S_CONTINUE_NEEDED) {
 		    /* server_creds was right, set it! */
@@ -419,12 +419,8 @@ enum auth_stat gssrpc__svcauth_gssapi(
 			   * returning a "wrong principal in request"
 			   * error
 			   */
-#if 0 /* old */
 			  || ((krb5_error_code) call_res.gss_minor !=
 			      (krb5_error_code) KRB5KRB_AP_WRONG_PRINC)
-#else
-			  || (call_res.gss_minor <= 0 || call_res.gss_minor > 3)
-#endif
 #endif
 			  ) {
 		    break;
@@ -437,8 +433,8 @@ enum auth_stat gssrpc__svcauth_gssapi(
 	  /* done with call args */
 	  xdr_free(xdr_authgssapi_init_arg, &call_arg);
 
-	  PRINTF(("svcauth_gssapi: accept_sec_context returned %#x\n",
-		  call_res.gss_major));
+	  PRINTF(("svcauth_gssapi: accept_sec_context returned %#x %#x\n",
+		  call_res.gss_major, call_res.gss_minor));
 	  if (call_res.gss_major != GSS_S_COMPLETE &&
 	      call_res.gss_major != GSS_S_CONTINUE_NEEDED) {
 	       AUTH_GSSAPI_DISPLAY_STATUS(("accepting context",