On Wed, 15 Oct 2025 17:34:55 GMT, Weibing Xiao <[email protected]> wrote:
>> [webrev.zip](https://github.com/user-attachments/files/22605072/webrev.zip) >> NPE thrown from SASL GSSAPI impl when TLS is used with QOP auth-int against >> Active Directory. >> >> When the exception is triggered, LDAP Connection will do "clean-up" >> operation and output stream get flushed and closed the context while >> GssKrb5Client is still wrapping the message, and tried to send the abandoned >> info to the client at line >> https://github.com/openjdk/jdk/blob/master/src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Base.java#L140. >> That's the reason to throw NPE. >> >> The change is going to close socket and output stream in LdapClient.java. It >> would allow SASL client code to send the abandoned request to client; then >> dispose GSS context. This will avoid NPE to thrown at line 140 of >> GssKrb5Base.java. >> >> No test file is attached for this MR since it needs Sasl LDAP server with >> security setup. Attached the updated webrev for the reference. > > Weibing Xiao has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 26 additional > commits since the last revision: > > - update the code > - Merge branch 'master' of github.com:openjdk/jdk into JDK-8362268 > - Merge branch 'master' into JDK-8362268 > - Merge branch 'master' of github.com:weibxiao/jdk > - Merge branch 'openjdk:master' into master > - Merge branch 'openjdk:master' into master > - Merge branch 'master' of github.com:openjdk/jdk > - Merge branch 'openjdk:master' into master > - Merge branch 'master' of github.com:openjdk/jdk > - Merge branch 'master' of github.com:openjdk/jdk > - ... and 16 more: https://git.openjdk.org/jdk/compare/340509e1...4dd20668 Updated the code to address the review comments. Add some of my extra analysis as below, In JDK, at the line https://github.com/openjdk/jdk/blob/1bd814c3b24eb7ef5633ee34bb418e0981ca1708/src/java.naming/share/classes/com/sun/jndi/ldap/sasl/SaslInputStream.java#L127, the buffer length was larger than default value 65536, it would trigger the exception. At the line of 451 of Connnection.java, the exception was caught. Connection::cleanup got called. At line https://github.com/openjdk/jdk/blob/1bd814c3b24eb7ef5633ee34bb418e0981ca1708/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#L657, the method of closeOpenedSocket will close the output stream. For Sasl implementation, this output stream is SaslOutputStream.java. So that SaslOutputStream::close is used to close the stream. Inside this method, GssKrb5Client::dispose was called, its implementation is base class GssKrb5Base::dispose, which is disposing the GSS Context and set it to be null. At mean time, Sasl output stream was till in the middle to handle the buffer by using GssKrb5Base::wrap. Since Gss Con text already set to be null, it caused the NPE at line of https://github.com/openjdk/jdk/blob/1bd814c3b24eb7ef5633ee34bb418e0981ca1708/src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Base.java#L140. JNDI connection was created inside the constructor o LdapClient.java. When Connection object was created, the output stream and input stream was created also. For Sasl implementation, it was SaslnputStream and SaslOutputStream. Both resources were created inside LdapClient when Connection object was created. It seems ok to close them in LdapClient.java. Consider locking the connection, the change was called inside LdapClient::close, which was using ReentrantLock to control the access. Hope to get more info to refine the code here. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26566#issuecomment-3407777489
