Skip Menu |
 

Subject: issues with gss_inquire_context and gss_display_context when using SPNEGO
Date: Tue, 22 Dec 2009 20:00:50 -0500
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
Download (untitled) / with headers
text/plain 4.7KiB
When using SPNEGO the resulting security context is a union context with
type SPNEGO which contains another union context which contains the
actual mechanism. This double union context causes gss_inquire_context
to report SPNEGO for the mechanism rather than the actual mechanism and
causes gss_display_status to not work for mechanism error codes since
the SPNEGO version of gss_display_status only translates SPNEGO error
codes. Per Sam Hartman there was talk of collapsing the double union
context into a single context. This patch does that which fixes both
issues. Note that this requires the fix for bug #6598.


Index: src/lib/gssapi/mechglue/g_accept_sec_context.c
===================================================================
--- src/lib/gssapi/mechglue/g_accept_sec_context.c (revision 23482)
+++ src/lib/gssapi/mechglue/g_accept_sec_context.c (working copy)
@@ -333,6 +333,23 @@
}
}

+ if (status == GSS_S_COMPLETE && actual_mech) {
+ gss_OID temp_mech_type = union_ctx_id->mech_type;
+
+ status = generic_gss_copy_oid(minor_status,
actual_mech,
+
&union_ctx_id->mech_type);
+ if (status != GSS_S_COMPLETE) {
+
gssint_delete_internal_sec_context(&temp_minor_status,
+ actual_mech,
+
&union_ctx_id->internal_ctx_id,
+ NULL);
+ free(union_ctx_id);
+ *context_handle = GSS_C_NO_CONTEXT;
+ }
+ free(temp_mech_type->elements);
+ free(temp_mech_type);
+ }
+
if (mech_type != NULL)
*mech_type = actual_mech;
else
Index: src/lib/gssapi/mechglue/g_init_sec_context.c
===================================================================
--- src/lib/gssapi/mechglue/g_init_sec_context.c (revision 23482)
+++ src/lib/gssapi/mechglue/g_init_sec_context.c (working copy)
@@ -117,6 +117,7 @@
gss_name_t internal_name;
gss_union_ctx_id_t union_ctx_id;
gss_OID mech_type = (gss_OID) req_mech_type;
+ gss_OID local_actual_mech = GSS_C_NO_OID;
gss_mechanism mech;
gss_cred_id_t input_cred_handle;

@@ -218,7 +219,7 @@
time_req,
input_chan_bindings,
input_token,
- actual_mech_type,
+ &local_actual_mech,
output_token,
ret_flags,
time_rec);
@@ -240,7 +241,28 @@
union_ctx_id->loopback = union_ctx_id;
*context_handle = (gss_ctx_id_t)union_ctx_id;
}
+ if (status == GSS_S_COMPLETE && local_actual_mech) {
+ gss_OID temp_mech_type = union_ctx_id->mech_type;

+ status = generic_gss_copy_oid(minor_status, local_actual_mech,
+ &union_ctx_id->mech_type);
+ if (status != GSS_S_COMPLETE) {
+ gssint_delete_internal_sec_context(&temp_minor_status,
+ local_actual_mech,
+
&union_ctx_id->internal_ctx_id,
+ NULL);
+ free(union_ctx_id);
+ *context_handle = GSS_C_NO_CONTEXT;
+ }
+
+ free(temp_mech_type->elements);
+ free(temp_mech_type);
+ }
+
+ if (actual_mech_type != NULL) {
+ *actual_mech_type = local_actual_mech;
+ }
+
end:
if (union_name->mech_name == NULL ||
union_name->mech_name != internal_name) {
Index: src/lib/gssapi/spnego/spnego_mech.c
===================================================================
--- src/lib/gssapi/spnego/spnego_mech.c (revision 23482)
+++ src/lib/gssapi/spnego/spnego_mech.c (working copy)
@@ -967,11 +967,14 @@
* Now, switch the output context to refer to the
* negotiated mechanism's context.
*/
- *context_handle = (gss_ctx_id_t)spnego_ctx->ctx_handle;
+ *context_handle =
(gss_union_ctx_id_t)(spnego_ctx->ctx_handle)->internal_ctx_id;
if (actual_mech != NULL)
*actual_mech = spnego_ctx->actual_mech;
if (ret_flags != NULL)
*ret_flags = spnego_ctx->ctx_flags;
+
free(((gss_union_ctx_id_t)spnego_ctx->ctx_handle)->mech_type->elements);
+
free(((gss_union_ctx_id_t)spnego_ctx->ctx_handle)->mech_type);
+ free(spnego_ctx->ctx_handle);
release_spnego_ctx(&spnego_ctx);
} else if (ret != GSS_S_CONTINUE_NEEDED) {
if (spnego_ctx != NULL) {
@@ -1686,11 +1689,14 @@
ret = GSS_S_FAILURE;
}
if (ret == GSS_S_COMPLETE) {
- *context_handle = (gss_ctx_id_t)sc->ctx_handle;
+ *context_handle =
(gss_union_ctx_id_t)(sc->ctx_handle)->internal_ctx_id;
if (sc->internal_name != GSS_C_NO_NAME &&
src_name != NULL) {
*src_name = sc->internal_name;
}
+
free(((gss_union_ctx_id_t)sc->ctx_handle)->mech_type->elements);
+ free(((gss_union_ctx_id_t)sc->ctx_handle)->mech_type);
+ free(sc->ctx_handle);
release_spnego_ctx(&sc);
} else if (ret != GSS_S_CONTINUE_NEEDED) {
if (sc != NULL) {
I don't think we are likely to incorporate an unwrapping patch which
works by making the SPNEGO code delve into the internal structure of a
union context.

It sounds like the design Sam had in mind went more like so:

* When the context is established, SPNEGO sets *context_handle to a
union context instead of the wrapped SPNEGO context structure.

* The mechglue detects this somehow and returns that union context to
the caller in lieu of its own union context.

Thus, SPNEGO would unwrap the SPNEGO part of the chain, and the mechglue
would unwrap the mechglue part of the chain, and neither knows about the
other's structures.

I'm not sure how the mechglue is supposed to detect that the subsidiary
mechanism returned a union context.
Subject: [krbdev.mit.edu #6604] issues with gss_inquire_context and gss_display_context when using SPNEGO
Date: Wed, 24 Feb 2010 17:28:32 -0500
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
RT-Send-Cc:
I agree what I did isn't the prettiest solution but like you I didn't
see any way for the mechglue to determine a union context had been
returned and I didn't see a way for the mechglue to extract the actual
mechanism's union context from the SPNEGO context. Thinking about it
now it occurs to me that maybe what's needed is a helper mechglue
function that SPNEGO can use which returns the actual mechanism context
and frees the rest of the union context.
I discussed this issue with Nico, our resident mechglue expert. Here
was the takeaway:

* There is no fundamental reason to unwrap the double-layered union mech
except perhaps performance (avoiding the two extra function calls for
each message operation).

* gss_inquire_sec_context should be returning the actual mechanism, so
that's a bug. (More on the best way to fix that later; we were
interrupted by lunch.)

* gss_display_status doesn't accept a context as an argument; it accepts
a mech. If the app is passing the correct mech to gss_display_status,
then the negotiated mech's error codes should be displayed correctly.
So if gss_inquire_sec_context is fixed, error codes should not be a
motivation for unwrapping the double-layered union context.

* If we did want to unwrap the double layering, the way to do it would
be to require mechs to return a union context if they set *actual_mech
(for init) or *mech_type (for accept) to a value other than the initial
mech type (for init) or the token mech type (for accept). We already do
something similar for delegated creds.
Download (untitled) / with headers
text/plain 1.1KiB
After further discussion, there are three acceptable options here:

1. gss_inquire_sec_context could call into the mech to get the mech type.

2. gss_union_ctx_id_desc could be extended to contain an actual_mech
field, which could be remembered from the mech's init/accept_sec_context
invocation, and gss_inquire_sec_context could return that.

3. We could do the unwrapping: the mechglue would assume that if the
mech's init/accept_sec_context invocation returns a different mech_type
from req_mech_type, then *context_handle is a union context. The
mechglue would then update the mech_type field of its own union context
to match the returned mech type (as your patch does) and would change
the internal_ctx_id of its own union mech to the internal_ctx_id of the
union context returned by the mech.

The net effect of any of these changes would be for gss_inquire_context
to return the correct (actual) mech type for a context negotiated by
SPNEGO. With that fixed, presumably your error message display problems
should also be solved.

We've hit code freeze for 1.8, but I think any of these would be
reasonable for inclusion in 1.8.1, which is probably only a month or so off.
Date: Mon, 1 Mar 2010 18:16:47 -0500
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
Subject: [krbdev.mit.edu #6604] issues with gss_inquire_context and gss_display_context when using SPNEGO
RT-Send-Cc:
Any option which works is okay with us. What happens if the actual
mechanism returns an error for its gss_init_sec_context or
gss_accept_sec_context? It looks like those errors are returned up the
call chain and I'm not sure how to tell the difference between them and
something SPNEGO itself returns.
The RFCs aren't terribly clear about this, but after talking to Nico, we
think the best solution is for gss_init/accept_sec_context to set
actual_mech/mech_type to the correct mechanism for interpreting minor_code.

I'm not sure how well that matches what the code does now.
Date: Wed, 12 May 2010 19:57:42 -0400
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
Subject: [krbdev.mit.edu #6604] issues with gss_inquire_context and gss_display_context when using SPNEGO
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.2KiB

Even if gss_init/accept_sec_context reported the actual mechanism it still wouldn’t work.  What I’m seeing is that the top level glue layer is returning fake minor status codes.  What happens is the krb5 function returns an error code, the glue layer which called it calls map_error for that error with the KRB5 mechanism and returns the error code to SPNEGO.  SPNEGO returns the error code.  The top glue layer also calls map_error but this time it calls it for the error with the SPNEGO mechanism.  The mapping function doesn’t find a mapping for the error with SPNEGO but does find one for Kerberos so it makes up a fake error code and returns the fake error code to the caller because error codes are expected to be unique across all mechanisms.  The glue layer for gss_display_status searches the cache of mapped error codes.  It uses the associated mechanism from the cache and ignores the one that was passed in.  The problem with this is that when using SPNEGO you will always pass through the glue layer twice with two different mechanisms with the result that a fake error code will always be returned to the caller.  The concept of fake error codes is fundamentally flawed because they are meaningless to the calling application which therefore cannot respond to them.