zookeeper git commit: ZOOKEEPER-2908: quorum.auth.MiniKdcTest.testKerberosLogin failing with NPE on java 9

[ph...@apache.org]

Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.5 dbd7dc515 -> 5894da317


ZOOKEEPER-2908: quorum.auth.MiniKdcTest.testKerberosLogin failing with NPE on java 9

ZOOKEEPER-2908: quorum.auth.MiniKdcTest.testKerberosLogin failing with NPE on Java 9

Cause:

The NPE exception in the MiniKdcTest.testKerberosLogin() unit test is caused by a duplicate loginContext.logout() call: one logout() call at the end of the test inside the try block and another logout() call in the finally block. When the test finishes, the first logout() call removes the kerbClientPrinc KerberosPrincipal in Krb5LoginModule, so when logout() is called for the second time in the finally block, it tries to remove a null kerbClientPrinc at Krb5LoginModule.java:1193:

subject.getPrincipals().remove(kerbClientPrinc);

where subject is a javax.security.auth.Subject,
getPrincipals() returns Set<Principal>
and the Set implementation is a javax.security.auth.Subject.SecureSet.

In Java 9, SecureSet's remove() method has introduced a new requireNonNull check for its parameter Object o, which fails if someone tries to remove a null from a SecureSet:

Objects.requireNonNull(o,ResourcesMgr.getString(“invalid.null.input.s.”));

Java 8 (and before) did not have this check in the SecureSet.remove() method, and this is the reason why this NPE appeared in Java 9.

Solution:

The unit test was fixed by adding an additional condition before running the logout() call in the finally block: logout() is called only if the Set of Principals is not empty i.e. logout() was not already called inside the try block.

Note: Inside ZK, LoginContext logout() is called only once in the org.apache.zookeeper.Login reLogin() method, when ZK does a re-login after refreshing the Kerberos tickets.

Author: Mark Fenes <mf...@cloudera.com>

Reviewers: Patrick Hunt <ph...@apache.org>

Closes #390 from mfenes/ZOOKEEPER-2908

Change-Id: I018124a578d8a382cac567466407278947705cd6


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/5894da31
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/5894da31
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/5894da31

Branch: refs/heads/branch-3.5
Commit: 5894da317de6f025a172408048e097e89157b73d
Parents: dbd7dc5
Author: Mark Fenes <mf...@cloudera.com>
Authored: Thu Oct 5 08:19:51 2017 -0700
Committer: Patrick Hunt <ph...@apache.org>
Committed: Thu Oct 5 08:19:51 2017 -0700

----------------------------------------------------------------------
 .../test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/5894da31/src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java b/src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java
index a7bbf7f..69dbcd1 100644
--- a/src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java
+++ b/src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java
@@ -175,7 +175,8 @@ public class MiniKdcTest extends KerberosSecurityTestcase {
             loginContext.logout();
 
         } finally {
-            if (loginContext != null) {
+            if (loginContext != null && loginContext.getSubject() != null
+                    && !loginContext.getSubject().getPrincipals().isEmpty()) {
                 loginContext.logout();
             }
         }