8186831: Kerberos ignores PA-DATA with a non-null s2kparams

Reviewed-by: xuelei

5 files changed, 140 insertions, 15 deletions

diff --git a/src/share/classes/sun/security/jgss/krb5/CipherHelper.java b/src/share/classes/sun/security/jgss/krb5/CipherHelper.java
index c637b0169d..0b8adfb611 100644
--- a/src/share/classes/sun/security/jgss/krb5/CipherHelper.java
+++ b/src/share/classes/sun/security/jgss/krb5/CipherHelper.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2017, 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
@@ -44,6 +44,7 @@ import sun.security.krb5.internal.crypto.Des3;
 import sun.security.krb5.internal.crypto.Aes128;
 import sun.security.krb5.internal.crypto.Aes256;
 import sun.security.krb5.internal.crypto.ArcFourHmac;
+import sun.security.krb5.internal.crypto.EType;
 
 class CipherHelper {
 
@@ -77,10 +78,6 @@ class CipherHelper {
     private int sgnAlg, sealAlg;
     private byte[] keybytes;
 
-    // new token format from draft-ietf-krb-wg-gssapi-cfx-07
-    // proto is used to determine new GSS token format for "newer" etypes
-    private int proto = 0;
-
     CipherHelper(EncryptionKey key) throws GSSException {
         etype = key.getEType();
         keybytes = key.getBytes();
@@ -106,7 +103,6 @@ class CipherHelper {
         case EncryptedData.ETYPE_AES256_CTS_HMAC_SHA1_96:
             sgnAlg = -1;
             sealAlg = -1;
-            proto = 1;
             break;
 
         default:
@@ -123,8 +119,10 @@ class CipherHelper {
         return sealAlg;
     }
 
+    // new token format from draft-ietf-krb-wg-gssapi-cfx-07
+    // proto is used to determine new GSS token format for "newer" etypes
     int getProto() {
-        return proto;
+        return EType.isNewer(etype) ? 1 : 0;
     }
 
     int getEType() {
diff --git a/src/share/classes/sun/security/krb5/internal/PAData.java b/src/share/classes/sun/security/krb5/internal/PAData.java
index 96db24f4ef..fd1429a4c3 100644
--- a/src/share/classes/sun/security/krb5/internal/PAData.java
+++ b/src/share/classes/sun/security/krb5/internal/PAData.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -31,7 +32,7 @@
 
 package sun.security.krb5.internal;
 
-import sun.security.krb5.KrbException;
+import sun.security.krb5.internal.crypto.EType;
 import sun.security.util.*;
 import sun.security.krb5.Asn1Exception;
 import java.io.IOException;
@@ -207,8 +208,8 @@ public class PAData {
             while (d2.data.available() > 0) {
                 DerValue value = d2.data.getDerValue();
                 ETypeInfo2 tmp = new ETypeInfo2(value);
-                if (tmp.getParams() == null) {
-                    // we don't support non-null s2kparams
+                if (EType.isNewer(tmp.getEType()) || tmp.getParams() == null) {
+                    // we don't support non-null s2kparams for old etypes
                     return tmp.getEType();
                 }
             }
@@ -274,8 +275,9 @@ public class PAData {
             while (d2.data.available() > 0) {
                 DerValue value = d2.data.getDerValue();
                 ETypeInfo2 tmp = new ETypeInfo2(value);
-                if (tmp.getParams() == null && tmp.getEType() == eType) {
-                    // we don't support non-null s2kparams
+                if (tmp.getEType() == eType &&
+                        (EType.isNewer(eType) || tmp.getParams() == null)) {
+                    // we don't support non-null s2kparams for old etypes
                     return new SaltAndParams(tmp.getSalt(), tmp.getParams());
                 }
             }
diff --git a/src/share/classes/sun/security/krb5/internal/crypto/EType.java b/src/share/classes/sun/security/krb5/internal/crypto/EType.java
index feed5d8b96..ee59d21507 100644
--- a/src/share/classes/sun/security/krb5/internal/crypto/EType.java
+++ b/src/share/classes/sun/security/krb5/internal/crypto/EType.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2017, 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
@@ -301,6 +301,26 @@ public abstract class EType {
         return isSupported(eTypeConst, enabledETypes);
     }
 
+    /**
+     * https://tools.ietf.org/html/rfc4120#section-3.1.3:
+     *
+     *                 A "newer" enctype is any enctype first officially
+     * specified concurrently with or subsequent to the issue of this RFC.
+     * The enctypes DES, 3DES, or RC4 and any defined in [RFC1510] are not
+     * "newer" enctypes.
+     *
+     * @param eTypeConst the encryption type
+     * @return true if "newer"
+     */
+    public static boolean isNewer(int eTypeConst) {
+        return eTypeConst != EncryptedData.ETYPE_DES_CBC_CRC &&
+                eTypeConst != EncryptedData.ETYPE_DES_CBC_MD4 &&
+                eTypeConst != EncryptedData.ETYPE_DES_CBC_MD5 &&
+                eTypeConst != EncryptedData.ETYPE_DES3_CBC_HMAC_SHA1_KD &&
+                eTypeConst != EncryptedData.ETYPE_ARCFOUR_HMAC &&
+                eTypeConst != EncryptedData.ETYPE_ARCFOUR_HMAC_EXP;
+    }
+
     public static String toString(int type) {
         switch (type) {
         case 0:
diff --git a/test/sun/security/krb5/auto/DiffSaltParams.java b/test/sun/security/krb5/auto/DiffSaltParams.java
new file mode 100644
index 0000000000..275b827098
--- /dev/null
+++ b/test/sun/security/krb5/auto/DiffSaltParams.java
@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2017, 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 8186831
+ * @summary Kerberos ignores PA-DATA with a non-null s2kparams
+ * @compile -XDignore.symbol.file DiffSaltParams.java
+ * @run main/othervm -Dsun.security.krb5.debug=true -Dsun.net.spi.nameservice.provider.1=ns,mock DiffSaltParams
+ */
+
+public class DiffSaltParams {
+
+    public static void main(String[] args) throws Exception {
+
+        OneKDC kdc = new OneKDC(null).writeJAASConf();
+        kdc.addPrincipal("user1", "user1pass".toCharArray(),
+                "hello", new byte[]{0, 0, 1, 0});
+        kdc.addPrincipal("user2", "user2pass".toCharArray(),
+                "hello", null);
+        kdc.addPrincipal("user3", "user3pass".toCharArray(),
+                null, new byte[]{0, 0, 1, 0});
+        kdc.addPrincipal("user4", "user4pass".toCharArray());
+
+        Context.fromUserPass("user1", "user1pass".toCharArray(), true);
+        Context.fromUserPass("user2", "user2pass".toCharArray(), true);
+        Context.fromUserPass("user3", "user3pass".toCharArray(), true);
+        Context.fromUserPass("user4", "user4pass".toCharArray(), true);
+    }
+}
diff --git a/test/sun/security/krb5/auto/KDC.java b/test/sun/security/krb5/auto/KDC.java
index 696ae3973d..f81582d5ae 100644
--- a/test/sun/security/krb5/auto/KDC.java
+++ b/test/sun/security/krb5/auto/KDC.java
@@ -149,6 +149,17 @@ public class KDC {
     private TreeMap<String,PrincipalName> alias2Principals = new TreeMap<>
             (String.CASE_INSENSITIVE_ORDER);
 
+    // Non default salts. Precisely, there should be different salts for
+    // different etypes, pretend they are the same at the moment.
+    private TreeMap<String,String> salts = new TreeMap<>
+            (String.CASE_INSENSITIVE_ORDER);
+
+    // Non default s2kparams for newer etypes. Precisely, there should be
+    // different s2kparams for different etypes, pretend they are the same
+    // at the moment.
+    private TreeMap<String,byte[]> s2kparamses = new TreeMap<>
+            (String.CASE_INSENSITIVE_ORDER);
+
     // Realm name
     private String realm;
     // KDC
@@ -370,10 +381,28 @@ public class KDC {
      * @param pass the password for the principal
      */
     public void addPrincipal(String user, char[] pass) {
+        addPrincipal(user, pass, null, null);
+    }
+
+    /**
+     * Adds a new principal to this realm with a given password.
+     * @param user the principal's name. For a service principal, use the
+     *        form of host/f.q.d.n
+     * @param pass the password for the principal
+     * @param salt the salt, or null if a default value will be used
+     * @param s2kparams the s2kparams, or null if a default value will be used
+     */
+    public void addPrincipal(String user, char[] pass, String salt, byte[] s2kparams) {
         if (user.indexOf('@') < 0) {
             user = user + "@" + realm;
         }
         passwords.put(user, pass);
+        if (salt != null) {
+            salts.put(user, salt);
+        }
+        if (s2kparams != null) {
+            s2kparamses.put(user, s2kparams);
+        }
     }
 
     /**
@@ -611,6 +640,9 @@ public class KDC {
         if (p.getRealmString() == null) {
             pn = pn + "@" + getRealm();
         }
+        if (salts.containsKey(pn)) {
+            return salts.get(pn);
+        }
         if (passwords.containsKey(pn)) {
             try {
                 // Find the principal name with correct case.
@@ -628,6 +660,29 @@ public class KDC {
     }
 
     /**
+     * Returns the s2kparams for the principal given the etype.
+     * @param p principal
+     * @param etype encryption type
+     * @return the s2kparams, might be null
+     */
+    protected byte[] getParams(PrincipalName p, int etype) {
+        switch (etype) {
+            case EncryptedData.ETYPE_AES128_CTS_HMAC_SHA1_96:
+            case EncryptedData.ETYPE_AES256_CTS_HMAC_SHA1_96:
+                String pn = p.toString();
+                if (p.getRealmString() == null) {
+                    pn = pn + "@" + getRealm();
+                }
+                if (s2kparamses.containsKey(pn)) {
+                    return s2kparamses.get(pn);
+                }
+                return new byte[] {0, 0, 0x10, 0};
+            default:
+                return null;
+        }
+    }
+
+    /**
      * Returns the key for a given principal of the given encryption type
      * @param p the principal
      * @param etype the encryption type
@@ -650,7 +705,7 @@ public class KDC {
                 }
             }
             return new EncryptionKey(EncryptionKeyDotStringToKey(
-                    getPassword(p, server), getSalt(p), null, etype),
+                    getPassword(p, server), getSalt(p), getParams(p, etype), etype),
                     etype, kvno);
         } catch (KrbException ke) {
             throw ke;
@@ -1116,7 +1171,7 @@ public class KDC {
                             epas[i],
                             epas[i] == EncryptedData.ETYPE_ARCFOUR_HMAC ?
                                 null : getSalt(body.cname),
-                            null).asn1Encode());
+                            getParams(body.cname, epas[i])).asn1Encode());
                 }
                 boolean allOld = true;
                 for (int i: eTypes) {