From 2be2f20b659fdb22fc1121cfbe7f91c4a00b60c3 Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Tue, 11 Dec 2012 13:14:56 +0800 Subject: [PATCH] 8004488: wrong permissions checked in krb5 Reviewed-by: xuelei --- .../security/auth/module/Krb5LoginModule.java | 4 -- .../sun/security/jgss/krb5/Krb5Util.java | 62 ++++--------------- .../security/krb5/auto/KeyPermissions.java | 56 +++++++++++++++++ .../sun/security/krb5/auto/KeyTabCompat.java | 18 +----- 4 files changed, 72 insertions(+), 68 deletions(-) create mode 100644 jdk/test/sun/security/krb5/auto/KeyPermissions.java diff --git a/jdk/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java b/jdk/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java index 5779da791e5..3d1744ba172 100644 --- a/jdk/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java +++ b/jdk/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; diff --git a/jdk/src/share/classes/sun/security/jgss/krb5/Krb5Util.java b/jdk/src/share/classes/sun/security/jgss/krb5/Krb5Util.java index 97f1a12447c..ec5bc9ca43b 100644 --- a/jdk/src/share/classes/sun/security/jgss/krb5/Krb5Util.java +++ b/jdk/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 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 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 pcs = subj.getPrivateCredentials(); - synchronized (pcs) { - Iterator 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)); - } - } - return keys.toArray(new KerberosKey[keys.size()]); + List 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()]); } public EncryptionKey[] getEKeys() { diff --git a/jdk/test/sun/security/krb5/auto/KeyPermissions.java b/jdk/test/sun/security/krb5/auto/KeyPermissions.java new file mode 100644 index 00000000000..78f0eafc6c5 --- /dev/null +++ b/jdk/test/sun/security/krb5/auto/KeyPermissions.java @@ -0,0 +1,56 @@ +/* + * 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); + } +} + diff --git a/jdk/test/sun/security/krb5/auto/KeyTabCompat.java b/jdk/test/sun/security/krb5/auto/KeyTabCompat.java index f6763510fd2..87a3e7e9c78 100644 --- a/jdk/test/sun/security/krb5/auto/KeyTabCompat.java +++ b/jdk/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"); - } - } }