8004488: wrong permissions checked in krb5

Reviewed-by: xuelei

src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java

@@ -1067,10 +1067,6 @@ public class Krb5LoginModule implements LoginModule {
                     if (ktab != null) {
                         if (!privCredSet.contains(ktab)) {
                             privCredSet.add(ktab);
-                            // Compatibility; also add keys to privCredSet
-                            for (KerberosKey key: ktab.getKeys(kerbClientPrinc)) {
-                                privCredSet.add(new Krb5Util.KeysFromKeyTab(key));
-                            }
                         }
                     } else {
                         succeeded = false;

src/share/classes/sun/security/jgss/krb5/Krb5Util.java

@@ -40,10 +40,7 @@ import sun.security.krb5.EncryptionKey;
 import sun.security.krb5.KrbException;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
-import java.util.Objects;
-import java.util.Set;
 import sun.security.krb5.KerberosSecrets;
 import sun.security.krb5.PrincipalName;
 /**
@@ -189,18 +186,6 @@ public class Krb5Util {
         return subject;
     }
 
-    // A special KerberosKey, used as keys read from a KeyTab object.
-    // Each time new keys are read from KeyTab objects in the private
-    // credentials set, old ones are removed and new ones added.
-    public static class KeysFromKeyTab extends KerberosKey {
-        private static final long serialVersionUID = 8238092170252746927L;
-
-        public KeysFromKeyTab(KerberosKey key) {
-            super(key.getPrincipal(), key.getEncoded(),
-                    key.getKeyType(), key.getVersionNumber());
-        }
-    }
-
     /**
      * Credentials of a service, the private secret to authenticate its
      * identity, which can be:
@@ -239,7 +224,7 @@ public class Krb5Util {
                 // Compatibility with old behavior: even when there is no
                 // KerberosPrincipal, we can find one from KerberosKeys
                 List<KerberosKey> keys = SubjectComber.findMany(
-                        subj, null, null, KerberosKey.class);
+                        subj, serverPrincipal, null, KerberosKey.class);
                 if (!keys.isEmpty()) {
                     sc.kp = keys.get(0).getPrincipal();
                     serverPrincipal = sc.kp.getName();
@@ -255,9 +240,9 @@ public class Krb5Util {
                         subj, null, null, KeyTab.class);
             sc.kk = SubjectComber.findMany(
                         subj, serverPrincipal, null, KerberosKey.class);
-            sc.tgt = SubjectComber.find(subj, null, null, KerberosTicket.class);
-
-            if (sc.ktabs.isEmpty() && sc.kk.isEmpty()) {
+            sc.tgt = SubjectComber.find(
+                    subj, null, serverPrincipal, KerberosTicket.class);
+            if (sc.ktabs.isEmpty() && sc.kk.isEmpty() && sc.tgt == null) {
                 return null;
             }
             return sc;
@@ -268,37 +253,16 @@ public class Krb5Util {
         }
 
         public KerberosKey[] getKKeys() {
-            if (ktabs.isEmpty()) {
-                return kk.toArray(new KerberosKey[kk.size()]);
-            } else {
-                List<KerberosKey> keys = new ArrayList<>();
-                for (KeyTab ktab: ktabs) {
-                    for (KerberosKey k: ktab.getKeys(kp)) {
-                        keys.add(k);
-                    }
-                }
-                // Compatibility: also add keys to privCredSet. Remove old
-                // ones first, only remove those from keytab.
-                if (!subj.isReadOnly()) {
-                    Set<Object> pcs = subj.getPrivateCredentials();
-                    synchronized (pcs) {
-                        Iterator<Object> iterator = pcs.iterator();
-                        while (iterator.hasNext()) {
-                            Object obj = iterator.next();
-                            if (obj instanceof KeysFromKeyTab) {
-                                KerberosKey key = (KerberosKey)obj;
-                                if (Objects.equals(key.getPrincipal(), kp)) {
-                                    iterator.remove();
-                                }
-                            }
-                        }
-                    }
-                    for (KerberosKey key: keys) {
-                        subj.getPrivateCredentials().add(new KeysFromKeyTab(key));
-                    }
+            List<KerberosKey> keys = new ArrayList<>();
+            for (KerberosKey k: kk) {
+                keys.add(k);
+            }
+            for (KeyTab ktab: ktabs) {
+                for (KerberosKey k: ktab.getKeys(kp)) {
+                    keys.add(k);
                 }
-                return keys.toArray(new KerberosKey[keys.size()]);
             }
+            return keys.toArray(new KerberosKey[keys.size()]);
         }
 
         public EncryptionKey[] getEKeys() {

test/sun/security/krb5/auto/KeyPermissions.java

+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8004488
+ * @summary wrong permissions checked in krb5
+ * @compile -XDignore.symbol.file KeyPermissions.java
+ * @run main/othervm KeyPermissions
+ */
+
+import java.security.AccessControlException;
+import java.security.Permission;
+import javax.security.auth.PrivateCredentialPermission;
+import sun.security.jgss.GSSUtil;
+
+public class KeyPermissions extends SecurityManager {
+
+    @Override
+    public void checkPermission(Permission perm) {
+        if (perm instanceof PrivateCredentialPermission) {
+            if (!perm.getName().startsWith("javax.security.auth.kerberos.")) {
+                throw new AccessControlException(
+                        "I don't like this", perm);
+            }
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        System.setSecurityManager(new KeyPermissions());
+        new OneKDC(null).writeJAASConf();
+        Context s = Context.fromJAAS("server");
+        s.startAsServer(GSSUtil.GSS_KRB5_MECH_OID);
+    }
+}
+

test/sun/security/krb5/auto/KeyTabCompat.java

@@ -24,6 +24,7 @@
 /*
  * @test
  * @bug 6894072
+ * @bug 8004488
  * @compile -XDignore.symbol.file KeyTabCompat.java
  * @run main/othervm KeyTabCompat
  * @summary always refresh keytab
@@ -70,21 +71,8 @@ public class KeyTabCompat {
         s.startAsServer(GSSUtil.GSS_KRB5_MECH_OID);
         s.status();
 
-        if (s.s().getPrivateCredentials(KerberosKey.class).size() != 1) {
-            throw new Exception("There should be one KerberosKey");
+        if (s.s().getPrivateCredentials(KerberosKey.class).size() != 0) {
+            throw new Exception("There should be no KerberosKey");
         }
-
-        Thread.sleep(2000);     // make sure ktab timestamp is different
-
-        kdc.addPrincipal(OneKDC.SERVER, "pass2".toCharArray());
-        kdc.writeKtab(OneKDC.KTAB);
-
-        Context.handshake(c, s);
-        s.status();
-
-        if (s.s().getPrivateCredentials(KerberosKey.class).size() != 1) {
-            throw new Exception("There should be only one KerberosKey");
-        }
-
     }
 }