RFR 6722928: Support SSPI as a native GSS-API provider

[Weijun Wang]

I've filed https://bugs.openjdk.java.net/browse/JDK-8225687.

> On Jun 11, 2019, at 7:56 AM, Nico Williams <Nico.Williams at twosigma.com> wrote:
> 
> 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 tried but scared away by MS-SPNG's appendix on which version of Windows supports which style of mechListMic. Now that Java only supports one underlying mechanism this field is looks not so crucial.

> 
>   I can't expect you to do that, but can we have a bug opened for this?

There was one at https://bugs.openjdk.java.net/browse/JDK-6773898.

> 
> - src/java.security.jgss/share/classes/sun/security/jgss/wrapper/SunNativeProvider.java
> 
>   Shouldn't the sspi_bridge.dll be in the lib directory?

The DLLs have always been in /bin. Maybe it's more easily loadable by java.exe.

> 
> - 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.

OK.

> 
> - 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.

OK.

> 
>    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.

OK.

> 
>  - 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.

OK.

> 
>  - 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.

Yes, it's called in GSSLibStub.c and those are the functions I must support.

> 
>    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

But I do notice that "Administrator" can also be named "administrator", and "HTTP/host" can also be "http/host".

> 
>  - gss_canonicalize_name() should check that mech_type is Kerberos

OK.

> 
> I'll continue later.  I'm in gss_init_sec_context(), about 58% of the
> way through.

Please.

Thanks,
Max

> 
> Nico
> --