On Mon, Jun 10, 2019 at 10:30:50AM +0800, Weijun Wang wrote:
> Updated webrev at
>
> http://cr.openjdk.java.net/~weijun/6722928/webrev.08/
-
src/java.security.jgss/share/classes/sun/security/jgss/spnego/NegTokenTarg.java
Ugh, I never noticed GSSUtil.useMSInterop(). This really should not
be needed. I believe a proper implementation of RFC4178 wouldn't
need it.
I can't expect you to do that, but can we have a bug opened for this?
-
src/java.security.jgss/share/classes/sun/security/jgss/wrapper/SunNativeProvider.java
Shouldn't the sspi_bridge.dll be in the lib directory?
- src/java.security.jgss/share/native/libj2gss/NativeFunc.h
- src/java.security.jgss/share/native/libj2gss/gssapi.h
These continue to have improper constification.
There should be no function arguments of type "const gss_...".
I've a PR for Heimdal to fix this in Heimdal. It was very
mechanical: search the prototypes and function definitions for
\<const[^I ]*gss_
then change that to gss_const_... then fix all the warnings and
errors that came up when building the result.
- All remaining commentary will be about
src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp
- CHECK_*() macros
Macro bodies should not end in a semi-colon.
If these were public and since these macro bodies are all if
statements, you should wrap them in do { } while (0), but since
they're private we can make sure that all uses are correct.
- gss_import_name() doesn't check that the first two bytes of the
input buffer are the expected token ID when the name-type is
GSS_C_NT_EXPORTED_NAME.
- gss_import_name() doesn't check that the third byte of the input
buffer is 0 when the name-type is GSS_C_NT_EXPORTED_NAME.
- gss_compare_name(), this code will not distinguish a name of the
form 'foo@' from 'foo\@'
434 if (l1 < l2 && n2[l1] != L'@'
435 || l2 < l1 && n1[l2] != L'@') {
436 return GSS_S_COMPLETE; // different
437 }
Honestly, this is not the most serious problem because nothing
really should be using gss_compare_name(), but you do use it... and
anyways, it's wrong.
Perhaps the gss_name_struct struct should have a length of realm or
length-of-not-realm-part field to make this check easier.
- gss_compare_name(), do not use NORM_IGNORECASE
- gss_canonicalize_name() should check that mech_type is Kerberos
I'll continue later. I'm in gss_init_sec_context(), about 58% of the
way through.
Nico
--
