On Tue, Nov 20, 2018 at 09:56:36AM +0800, Weijun Wang wrote: > Please take a review at > > https://cr.openjdk.java.net/~weijun/6722928/webrev.01/ > > We ported [1] the native GSS bridge to Windows in JDK 11, but user still have > to download and install a native GSS-API library. This code change provides a > native GSS-API library inside JDK that can be used without setting the > "sun.security.jgss.lib" system property. It is based on Windows SSPI and now > only supports the client side using the default credentials. > > No regression tests included. A Windows Active Directory server is needed.
This is my initial round of comments, and it's mostly high-level: - Right now this looks very basic, and you won't get support for any mechanism other than Kerberos. That's fine though, as NTLM is dead, you're willing to implement SPNEGO yourself, and there are no other GSS mechs on Windows at this time (so far as I know). - You're constructing SPNEGO tokens yourself. I'll have to review that very carefully. It would be better to let Windows provide SPNEGO though... - I've checked the DER-encoded OID constants. They are correct. - The name type OID for GSS_KRB5_NT_PRINCIPAL_NAME is not implemented but is used elsewhere (e.g., in one GSSNameElement constructor). Look for GSSUtil.NT_GSS_KRB5_PRINCIPAL. - gss_canonicalize_name() is not fully implemented. This will be noticeable to callers of GSSNameElement's getKrbName(). In particular, permissions checks will fail (e.g., in GSSCredElement's doServicePermCheck(); similarly in NativeGSSContext). At minimum you absolutely must parse generic GSS name type names into Kerberos-style names (e.g., service@hostname -> service/hostname@). When an MN is displayed, you need to output the Kerberos-style name. (For those following along, "MN" means "mechanism name", which means a GSSName / gss_name_t instance which is "canonical" and whose display form will be mechanism-specific. gss_canonicalize_name() is supposed to output a "canonical" version of its input.) You'll also have to determine a realm for the parsed principal names. You won't need a realm for services. Use an empty realm name for services -- that's the Heimdal/MIT (and Solaris) convention for when you're willing to rely on KDC referrals. (I would also urge you to NOT attempt any DNS canonicalization of hostnames. Let's not further spread that mistake.) For user principals it's trickier, and I think you might need some notion of default realm, but a) maybe you can get away with an empty realm here on Windows, b) I'm not sure where you'd a default realm on Windows. What we do to determine the default realm for user names in our patched version of Martin Rex's GSS->SSPI bridge is call AcquireCredentialsHandle() or QueryCredentialsAttributesW() to get/query the default credential and get its name, and from that the realm, and we use that as the default realm. I'll have to look and see how we handle host-based service names (Martin's original code has a domain_realm table in the registry, but we removed all of that and rely on referrals instead.) - If you can get Legal approval for including / distributing a fork of Martin Rex's bridge, you'll get all of the above functionality and also acceptor functionality. Nico --
