From: Ken Raeburn Date: Wed, 5 Sep 2007 00:12:30 +0000 (+0000) Subject: Rework error-mapping code to preserve status code values when returned X-Git-Tag: krb5-1.7-alpha1~888 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=7868a34cad0e5944f2144f734960ca78e9b4cf8b;p=krb5.git 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 --- diff --git a/src/lib/gssapi/generic/Makefile.in b/src/lib/gssapi/generic/Makefile.in index d32e8b2d6..d09a67860 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 b884eed8b..28b4b0633 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 6f7f17091..5cd554cde 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 #ifndef _WIN32 @@ -16,6 +41,17 @@ struct mecherror { OM_uint32 code; }; +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) { @@ -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 3d6e6fe63..bd185bc89 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 cb1e8f90f..f899c8968 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",