On Mon, 21 Sep 2020 08:19:28 GMT, Alexey Bakhtin <abakh...@openjdk.org> wrote:
> Hi, > > Plaese review JDK-8245527 fix which implements LDAP Channel Binding support > for Java GSS/Kerberos. > Initial review is available at core-devs: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html > This version removes "tls-unique" CB type from the list of possible channel > binding types. The only supported type is > "tls-server-end-point" > CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 > > Thank you > Alexey I'm mostly OK with the SASL/JGSS part, except for the small nits in this comment. I'm not an expert on HandshakeCompletedListener. src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Client.java line 156: > 154: if (props != null) { > 155: // TLS Channel Binding > 156: byte[] tlsCB = > (byte[])props.get("jdk.internal.sasl.tlschannelbinding"); You can say the name is defined in another class in another module. If we really want to rename it one day we will know where it's from. src/java.security.jgss/share/classes/sun/security/jgss/krb5/InitialToken.java line 389: > 387: int acceptorAddressType = getAddrType(acceptorAddress, > 388: (channelBinding instanceof TlsChannelBindingImpl)? > 389: > CHANNEL_BINDING_AF_UNSPEC:CHANNEL_BINDING_AF_NULL_ADDR); Normally we put a white space around "?" and ":". src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java line 82: > 80: /** > 81: * Parse value of "com.sun.jndi.ldap.tls.cbtype" property > 82: * @param cbType Please add a `@return` here, esp, about null. src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java line 137: > 135: public TlsChannelBindingType getType() { > 136: return cbType; > 137: } Add a new line here. ------------- PR: https://git.openjdk.java.net/jdk/pull/278
