Skip Menu |
 

Subject: issues with SPNEGO
Date: Tue, 22 Dec 2009 18:55:13 -0500
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
Download (untitled) / with headers
text/plain 1.2KiB
I found two problems with SPNEGO and the conversation between the
initiator and the acceptor. One is that if the initiator produces the
final mechanism token it doesn't send it to the acceptor who is waiting
for it. The other is that if the mechanism doesn't set
GSS_C_INTEG_FLAG, the acceptor never sets the state to ACCEPT_COMPLETE.
This fixed both problems for us:


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)
@@ -652,8 +652,9 @@
* mech not finished and mech token missing
*/
ret = GSS_S_DEFECTIVE_TOKEN;
- } else if (sc->mic_reqd &&
- (sc->ctx_flags & GSS_C_INTEG_FLAG)) {
+ } else if (*acc_negState == ACCEPT_INCOMPLETE ||
+ (sc->mic_reqd &&
+ (sc->ctx_flags & GSS_C_INTEG_FLAG))) {
*negState = ACCEPT_INCOMPLETE;
*tokflag = CONT_TOKEN_SEND;
ret = GSS_S_CONTINUE_NEEDED;
@@ -1534,6 +1535,11 @@
sc->mic_reqd = 0;
}
#endif
+
+ if (sc->mic_reqd && !(sc->ctx_flags & GSS_C_INTEG_FLAG))
{
+ sc->mic_reqd = 0;
+ }
+
sc->mech_complete = 1;
if (ret_flags != NULL)
*ret_flags = sc->ctx_flags;
I'm still tracing through the spnego_mech.c logic to evaluate the bug
reports, but I noticed an obvious problem with the patch: the first hunk
dereferences acc_NegState, which is not a pointer.
Download (untitled) / with headers
text/plain 1.8KiB
A little analysis of the reported problems:

1. A final token produced by the initiator is not sent to the acceptor.

I would describe this problem differently: the initiator will not send
more than two tokens to the acceptor unless a MIC is required. The bug
here is that on the second call to init_ctx_cont (the third call to
spnego_gss_init_sec_context), if no MIC is required for the negotiation,
*negState is set to ACCEPT_COMPLETE and *tokflag (aka send_token) is set
to NO_TOKEN_SEND without regard for whether the token exchange has
completed. If the mechanism produces an output token,
init_ctx_call_init will set negState back to ACCEPT_INCOMPLETE, but will
leave send_token as NO_TOKEN_SEND, so spnego_gss_init_sec_context
returns GSS_S_CONTINUE_NEEDED without setting the output token--clearly
invalid behavior.

The proposed fix is to use acc_negState to decide which way to set
*negState and *tokflag. However, RFC 4178 says that acc_negState is
optional after the first reply from the target; if the target doesn't
send it, then acc_negState will have the value ACCEPT_DEFECTIVE_TOKEN
(even though the token isn't defective) and we are supposed to deduce
the negotiation state from the return value of the selected mechanism's
init_sec_context call. The proposed fix does not handle this contingency.

2. If the mechanism doesn't set
GSS_C_INTEG_FLAG, the acceptor never sets the state to ACCEPT_COMPLETE.

This appears to be a failure of the code to check for (sc->ctx_flags &
GSS_C_INTEG_FLAG) in all of the cases where it checks for sc->mic_reqd.
The proposed fix is to set sc->mic_reqd to 0 before a particular check
of sc->mic_reqd in acc_ctx_call_acc if (sc->ctx_flags &
GSS_C_INTEG_FLAG) is false. I don't think that improves the overall
cleanliness of the logic. Consistently using a static inline to check
for (sc->mic_reqd && (sc->ctx_flags & GSS_C_INTEG_FLAG)) might be better.
From: ghudson@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 1.1KiB

Fix two unrelated problems in SPNEGO which don't crop up with the krb5
mechanism.

1. The third call to spnego_init_accept_context uses faulty logic to
determine if the exchange is complete, preventing a third mech token
from being sent to the acceptor if no MIC exchange is required.
Follow the logic used in the second call (in init_ctx_nego), which is
correct.

2. If the acceptor selects a mech other than the optimistic mech, it
sets sc->mic_reqd to 1 whether or not the selected mech supports MICs
(which isn't known until the mech completes). Most code outside of
handle_mic checks sc->mic_reqd along with (sc->ctx_flags &
GSS_C_INTEG_FLAG), but the code in acc_ctx_call_acc neglected to do
so, so it could improperly delegate responsibility for deciding when
the negotiation was finished to handle_mic--which never gets called if
(sc->ctx_flags & GSS_C_INTEG_FLAG) is false. Fix acc_ctx_call_acc to
check sc->ctx_flags so that mechs which don't support integrity
protection can complete if they are selected non-optimistically.


https://github.com/krb5/krb5/commit/d1da896d356bc80e632a9713e5b4246384fb3e77
Commit By: ghudson
Revision: 23742
Changed Files:
U trunk/src/lib/gssapi/spnego/spnego_mech.c
We don't have any GSSAPI mechanisms in the MIT krb5 tree which exercise
either of these bugs, so I was only able to test that my fixes don't
break the single-hop case. I stared at the code for a good long time
and am fairly confident about the changes I made, but testing would be
appreciated.
From: tlyu@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 1.4KiB

pull up r23742 from trunk

------------------------------------------------------------------------
r23742 | ghudson | 2010-02-21 23:52:30 -0500 (Sun, 21 Feb 2010) | 24 lines

ticket: 6603
target_version: 1.8
tags: pullup

Fix two unrelated problems in SPNEGO which don't crop up with the krb5
mechanism.

1. The third call to spnego_init_accept_context uses faulty logic to
determine if the exchange is complete, preventing a third mech token
from being sent to the acceptor if no MIC exchange is required.
Follow the logic used in the second call (in init_ctx_nego), which is
correct.

2. If the acceptor selects a mech other than the optimistic mech, it
sets sc->mic_reqd to 1 whether or not the selected mech supports MICs
(which isn't known until the mech completes). Most code outside of
handle_mic checks sc->mic_reqd along with (sc->ctx_flags &
GSS_C_INTEG_FLAG), but the code in acc_ctx_call_acc neglected to do
so, so it could improperly delegate responsibility for deciding when
the negotiation was finished to handle_mic--which never gets called if
(sc->ctx_flags & GSS_C_INTEG_FLAG) is false. Fix acc_ctx_call_acc to
check sc->ctx_flags so that mechs which don't support integrity
protection can complete if they are selected non-optimistically.

https://github.com/krb5/krb5/commit/39c6d76b5e15540ef8bc60b02e61169377760e71
Commit By: tlyu
Revision: 23748
Changed Files:
U branches/krb5-1-8/src/lib/gssapi/spnego/spnego_mech.c
Subject: [krbdev.mit.edu #6603] issues with SPNEGO
Date: Wed, 24 Feb 2010 17:26:57 -0500
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
RT-Send-Cc:
Our code base is 1.7 with some elements such as S4U pulled in from 1.8.
When generating the diffs for you I took the changes, applied them to a
current copy of your trunk, and then generated the diffs from that as I
wasn't sure how well they would apply for you otherwise. In our code
base acc_NegState is the seventh argument to init_sec_ctx and is an
OM_uint32 * but it looks like that's changed and I didn't catch it.