Summary

Add an @implNote to javax.security.auth.spi.LoginModule::logout to warn implementers that they should not call remove() on a null value when trying to remove element from either the principals or credentials set of a JAAS Subject.

Problem

Back in JDK-8015081 (https://bugs.openjdk.org/browse/CCC-8015081), we updated the implementation of principals and credentials sets inside a JAAS Subject so that it does not allow null element, and calling the add(), contains(), and remove() methods on null will throw a NullPointerException.

However, it's recently discovered that the logout() methods of various LoginModule implementations included in JDK call the remove() method to remove a principal or a credential object from a subject without any null check. If the object is null (For example, the previous login() method failed) then a NullPointerException is thrown.

Solution

Update the LoginModule implementations to check for the value of an object before trying to remove it. If it's null, then skip the remove() method call. We'll also add an @implNote to the logout() method so all internal and external implementers are fully aware of this (see spec below).

Alternative Solutions

We thought about ignoring null values in the remove() method of the set implementation (inside Subject.java). However, the specification of almost all methods in the base interface Set claims that a NullPointerException should be thrown "if the specified element is null and this set does not permit null elements". The restriction should be applied to all methods including remove(). In fact, we already have JCK tests on JAAS to ensure the exception is thrown when null is removed.

We also thought about skipping the logout() method if the login() method of a LoginModule failed. However, the specification of the LoginContext::logout method claims that "[t]his method invokes the logout method for each LoginModule configured for this LoginContext" and furthermore "[n]ote that this method invokes all LoginModules configured for the application regardless of their respective Configuration flag parameters". In fact, there's another JCK test to ensure all logout methods in the configured LoginModules are called no matter if one succeeds or not.

Specification

In javax.security.auth.spi.LoginModule::logout, add

 * @implNote Implementations should check if a variable is {@code null}
 *      before removing it from the Principals or Credentials set
 *      of a {@code Subject}, otherwise a {@code NullPointerException}
 *      will be thrown as these sets {@linkplain Subject#Subject()
 *      prohibit null elements}. This is especially important if
 *      this method is called after a login failure.

In javax.security.auth.Subject, update

These Sets also prohibit null elements, and attempts to add or query
a null element will result in a {@code NullPointerException}

to

These Sets also prohibit null elements, and attempts to add, query, or remove
a null element will result in a {@code NullPointerException}

in both of its constructors.