Hi Darwin
Thanks for looking into this file. Most of your words are correct, but
I'd like to explain what was behind those changesets.
See comments below.
On 5/21/2015 3:58 PM, Seán Coffey wrote:
FYI,
concerns from Darwin on the 8078439: 8048194 fixes.
regards,
Sean.
-------- Forwarded Message --------
Subject: 8078439: 8048194: possible bug in commit for these two fixes
Date: Wed, 20 May 2015 16:28:29 -0700
From: Darwin Felix <darwinfe...@yahoo.com>
To: jdk8u-...@openjdk.java.net
CC: darwinfe...@users.sourceforge.net
8078439: 8048194: possible bug in commit for these two fixes
It appears that this commit has a bug:
http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/d777e2918a77/src/share/classes/sun/security/jgss/spnego/SpNegoContext.java
Fundamentally, it appears that this commit desires to honor the
initiator/client's preferred mechanism - first one in the list of mechanisms
the client is offering to the target/server.
However, the commit does not allow for the condition where the server can only
support a different mechanism from the client's offering/list (server not able
to support mechList[0] but is able to support mechList[N]).
In SPNEGO, the client always sends a mechToken using mechList[0]. Before
this change for JDK-8048194, even if the server does not support
mechList[0] and prefer mechList[n], it blindly calls
GSS_acceptSecContext() on the mechToken as if it's for mechList[n],
which will fail immediately. After the fix, server will not touch
mechToken anymore. It sends its own preferred mech (mech_wanted, which
is mechList[n]) back to client and wait for the client to send a
mechToken for this mech in the 2nd round.
In my humble opinion, based on my quick glance of the code, this commit is
broken because the server is no longer allowed to select from one of the other
mechanisms from the client's list.
The server does select one (line 532) and inform the client about it
(line 591).
This next commit also appears to have a bug:
http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/9ab9f20e9bdd/src/share/classes/sun/security/jgss/spnego/SpNegoContext.java
It appears that this commit also desires to honor the initiator/client's
preferred mechanism.
However, this commit also appears to suffer from the condition where the server
can only support a different mechanism from the client's offering/list (server
not able to support mechList[0] but is able to support mechList[N]).
As I described in the bug comment for JDK-8078439, client sends
Microsoft's krb5 OID as mechList[0] but Java only understands the
standard krb5 OID. After the *previous* fix, it sends back the standard
OID and wait for the client to send a mechToken for it. It will be
something like
c: Here is a mechToken for MS krb5 OID
s: Sorry, I don't know that. Please send a token for standard krb5 OID
c: Sure, here it is
s: Good, here is my response token
However, we are seeing clients that only expect one round of
communication and when it cannot see a response token as the 1st reply
it just fails. Thus the 2nd fix:
c: Here is a mechToken for MS krb5 OID
s: Wow, MS krb5 OID, I understand now. Here is my response token
[A happy c]
BTW, before these 2 fixes, it was
c: Here is a mechToken for MS krb5 OID. I also know standard krb5
s: I know about standard krb5. Here is my response token
[A happy c]
Perhaps the above two (2) commits should be reverted?
An alternative approach to supporting/honoring the initiator/client's preferred
mechanism:
Prior to the above two (2) commits, SpNegoContext.java suffered from the
condition where it did not make an attempt/consider to use the
initiator/client's preferred mechanism. Instead, the code would find a
mechanism from the client's list that the server can support. Meaning, the
server's supported/preferred mechanisms is the outer loop and the client's
supported/preferred mechanism is the inner loop (preference specified by its
order in the list).
Exactly, and this is not correct. The reason I didn't touch this is that
Java only supports krb5 now, so whichever is outer or inner loop does
not really make a difference. We will fix it once we support a new mech.
Here's what the code looked like prior to the above two (2) commits:
http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/1a3de3cdc684/src/share/classes/sun/security/jgss/spnego/SpNegoContext.java
Perhaps, if we simply place the client's list as the outer loop and the
server's list as the inner loop, we might be able to honor the client's
preferred mechanism while ensuring that the chosen mechanism is one that the
server can support.
For example, have a look at the implementation of the method named
SpNegoContext.negotiate_mech_type(supported_mechSet, mechList)
Notice that the server's list is chosen as being the outer loop. It might make
more sense to have the client's list be the outer loop.
If possible, I would like to discuss the possibility of reverting the above two
commits and instead implement my proposed change (mechList as the outer loop
and supported_mechSet as the inner loop) to the method named
negotiate_mech_type.
But what JDK-8048194 describes still exists:
c: Here is a mechToken for XYZ mech. BTW, I also support krb5
s: Aha, I also support krb5, let me accept your mechToken...
bang... token not parseable... fail...
[ c still waiting ]
Thanks
Max
thanks,
-darwin
