6946669: SSL/Krb5 should not call EncryptedData.reset(data, false)

Reviewed-by: xuelei

src/share/classes/sun/security/krb5/EncryptedData.java

 /*
- * Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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
@@ -336,38 +336,29 @@ public class EncryptedData implements Cloneable {
     }
 
     /**
-     * Reset data stream after decryption, remove redundant bytes.
+     * Reset asn.1 data stream after decryption, remove redundant bytes.
      * @param data the decrypted data from decrypt().
-     * @param encoded true if the encrypted data is ASN1 encoded data,
-     * false if the encrypted data is not ASN1 encoded data.
      * @return the reset byte array which holds exactly one asn1 datum
      * including its tag and length.
      *
      */
-    public byte[] reset(byte[] data, boolean encoded) {
+    public byte[] reset(byte[] data) {
         byte[]  bytes = null;
-        // if it is encoded data, we use length field to
+        // for asn.1 encoded data, we use length field to
         // determine the data length and remove redundant paddings.
-        if (encoded) {
-            if ((data[1] & 0xFF) < 128) {
-                bytes = new byte[data[1] + 2];
-                System.arraycopy(data, 0, bytes, 0, data[1] + 2);
-            } else
-                if ((data[1] & 0xFF) > 128) {
-                    int len = data[1] & (byte)0x7F;
-                    int result = 0;
-                    for (int i = 0; i < len; i++) {
-                        result |= (data[i + 2] & 0xFF) << (8 * (len - i - 1));
-                    }
-                    bytes = new byte[result + len + 2];
-                    System.arraycopy(data, 0, bytes, 0, result + len + 2);
-                }
+        if ((data[1] & 0xFF) < 128) {
+            bytes = new byte[data[1] + 2];
+            System.arraycopy(data, 0, bytes, 0, data[1] + 2);
         } else {
-            // if it is not encoded, which happens in GSS tokens,
-            // we remove padding data according to padding pattern.
-            bytes = new byte[data.length - data[data.length - 1]];
-            System.arraycopy(data, 0, bytes, 0,
-                             data.length - data[data.length - 1]);
+            if ((data[1] & 0xFF) > 128) {
+                int len = data[1] & (byte)0x7F;
+                int result = 0;
+                for (int i = 0; i < len; i++) {
+                    result |= (data[i + 2] & 0xFF) << (8 * (len - i - 1));
+                }
+                bytes = new byte[result + len + 2];
+                System.arraycopy(data, 0, bytes, 0, result + len + 2);
+            }
         }
         return bytes;
     }

src/share/classes/sun/security/krb5/KrbApRep.java

 /*
- * Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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
@@ -130,7 +130,7 @@ public class KrbApRep {
 
         byte[] temp = rep.encPart.decrypt(tgs_creds.key,
             KeyUsage.KU_ENC_AP_REP_PART);
-        byte[] enc_ap_rep_part = rep.encPart.reset(temp, true);
+        byte[] enc_ap_rep_part = rep.encPart.reset(temp);
 
         encoding = new DerValue(enc_ap_rep_part);
         encPart = new EncAPRepPart(encoding);

src/share/classes/sun/security/krb5/KrbApReq.java

 /*
- * Copyright (c) 2000, 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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
@@ -279,14 +279,14 @@ public class KrbApReq {
 
         byte[] bytes = apReqMessg.ticket.encPart.decrypt(dkey,
             KeyUsage.KU_TICKET);
-        byte[] temp = apReqMessg.ticket.encPart.reset(bytes, true);
+        byte[] temp = apReqMessg.ticket.encPart.reset(bytes);
         EncTicketPart enc_ticketPart = new EncTicketPart(temp);
 
         checkPermittedEType(enc_ticketPart.key.getEType());
 
         byte[] bytes2 = apReqMessg.authenticator.decrypt(enc_ticketPart.key,
             KeyUsage.KU_AP_REQ_AUTHENTICATOR);
-        byte[] temp2 = apReqMessg.authenticator.reset(bytes2, true);
+        byte[] temp2 = apReqMessg.authenticator.reset(bytes2);
         authenticator = new Authenticator(temp2);
         ctime = authenticator.ctime;
         cusec = authenticator.cusec;

src/share/classes/sun/security/krb5/KrbAsRep.java

 /*
- * Copyright (c) 2000, 2005, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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
@@ -95,7 +95,7 @@ public class KrbAsRep extends KrbKdcRep {
 
         byte[] enc_as_rep_bytes = rep.encPart.decrypt(dkey,
             KeyUsage.KU_ENC_AS_REP_PART);
-        byte[] enc_as_rep_part = rep.encPart.reset(enc_as_rep_bytes, true);
+        byte[] enc_as_rep_part = rep.encPart.reset(enc_as_rep_bytes);
 
         encoding = new DerValue(enc_as_rep_part);
         EncASRepPart enc_part = new EncASRepPart(encoding);

src/share/classes/sun/security/krb5/KrbCred.java

 /*
- * Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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
@@ -130,7 +130,7 @@ public class KrbCred {
 
         byte[] temp = credMessg.encPart.decrypt(key,
             KeyUsage.KU_ENC_KRB_CRED_PART);
-        byte[] plainText = credMessg.encPart.reset(temp, true);
+        byte[] plainText = credMessg.encPart.reset(temp);
         DerValue encoding = new DerValue(plainText);
         EncKrbCredPart encPart = new EncKrbCredPart(encoding);
 

src/share/classes/sun/security/krb5/KrbPriv.java

 /*
- * Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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 +31,6 @@
 
 package sun.security.krb5;
 
-import sun.security.krb5.EncryptionKey;
 import sun.security.krb5.internal.*;
 import sun.security.krb5.internal.crypto.*;
 import sun.security.util.*;
@@ -159,7 +158,7 @@ class KrbPriv extends KrbAppMessage {
 
                                byte[] bytes = krb_priv.encPart.decrypt(key,
                                    KeyUsage.KU_ENC_KRB_PRIV_PART);
-                               byte[] temp = krb_priv.encPart.reset(bytes, true);
+                               byte[] temp = krb_priv.encPart.reset(bytes);
                                DerValue ref = new DerValue(temp);
                                EncKrbPrivPart enc_part = new EncKrbPrivPart(ref);
 

src/share/classes/sun/security/krb5/KrbTgsRep.java

 /*
- * Copyright (c) 2000, 2005, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2010, 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
@@ -79,7 +79,7 @@ public class KrbTgsRep extends KrbKdcRep {
             tgsReq.usedSubkey() ? KeyUsage.KU_ENC_TGS_REP_PART_SUBKEY :
             KeyUsage.KU_ENC_TGS_REP_PART_SESSKEY);
 
-        byte[] enc_tgs_rep_part = rep.encPart.reset(enc_tgs_rep_bytes, true);
+        byte[] enc_tgs_rep_part = rep.encPart.reset(enc_tgs_rep_bytes);
         ref = new DerValue(enc_tgs_rep_part);
         EncTGSRepPart enc_part = new EncTGSRepPart(ref);
         rep.ticket.sname.setRealm(rep.ticket.realm);

src/share/classes/sun/security/ssl/krb5/KerberosClientKeyExchangeImpl.java

@@ -212,7 +212,7 @@ public final class KerberosClientKeyExchangeImpl
             byte[] bytes = encPart.decrypt(secretKey, KeyUsage.KU_TICKET);
 
             // Reset data stream after decryption, remove redundant bytes
-            byte[] temp = encPart.reset(bytes, true);
+            byte[] temp = encPart.reset(bytes);
             EncTicketPart encTicketPart = new EncTicketPart(temp);
 
             // Record the Kerberos Principals

src/share/classes/sun/security/ssl/krb5/KerberosPreMasterSecret.java

 /*
- * Copyright (c) 2003, 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2010, 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
@@ -27,7 +27,7 @@ package sun.security.ssl.krb5;
 
 import java.io.*;
 import java.security.*;
-import java.security.interfaces.*;
+import java.util.Arrays;
 
 import javax.net.ssl.*;
 
@@ -128,8 +128,8 @@ final class KerberosPreMasterSecret {
                "are not supported for TLS Kerberos cipher suites");
         }
 
-         // Decrypt premaster secret
-         try {
+        // Decrypt premaster secret
+        try {
             EncryptedData data = new EncryptedData(sessionKey.getEType(),
                         null /* optional kvno */, encrypted);
 
@@ -141,8 +141,25 @@ final class KerberosPreMasterSecret {
                  }
             }
 
-            // Reset data stream after decryption, remove redundant bytes
-            preMaster =  data.reset(temp, false);
+            // Remove padding bytes after decryption. Only DES and DES3 have
+            // paddings and we don't support DES3 in TLS (see above)
+
+            if (temp.length == 52 &&
+                    data.getEType() == EncryptedData.ETYPE_DES_CBC_CRC) {
+                // For des-cbc-crc, 4 paddings. Value can be 0x04 or 0x00.
+                if (paddingByteIs(temp, 52, (byte)4) ||
+                        paddingByteIs(temp, 52, (byte)0)) {
+                    temp = Arrays.copyOf(temp, 48);
+                }
+            } else if (temp.length == 56 &&
+                    data.getEType() == EncryptedData.ETYPE_DES_CBC_MD5) {
+                // For des-cbc-md5, 8 paddings with 0x08, or no padding
+                if (paddingByteIs(temp, 56, (byte)8)) {
+                    temp = Arrays.copyOf(temp, 48);
+                }
+            }
+
+            preMaster = temp;
 
             protocolVersion = ProtocolVersion.valueOf(preMaster[0],
                  preMaster[1]);
@@ -191,6 +208,19 @@ final class KerberosPreMasterSecret {
         }
     }
 
+    /**
+     * Checks if all paddings of data are b
+     * @param data the block with padding
+     * @param len length of data, >= 48
+     * @param b expected padding byte
+     */
+    private static boolean paddingByteIs(byte[] data, int len, byte b) {
+        for (int i=48; i<len; i++) {
+            if (data[i] != b) return false;
+        }
+        return true;
+    }
+
     /*
      * Used by server to generate premaster secret in case of
      * problem decoding ticket.

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

@@ -25,6 +25,16 @@
  * @test
  * @bug 6894643 6913636
  * @summary Test JSSE Kerberos ciphersuite
+ * @run main SSL TLS_KRB5_WITH_RC4_128_SHA
+ * @run main SSL TLS_KRB5_WITH_RC4_128_MD5
+ * @run main SSL TLS_KRB5_WITH_3DES_EDE_CBC_SHA
+ * @run main SSL TLS_KRB5_WITH_3DES_EDE_CBC_MD5
+ * @run main SSL TLS_KRB5_WITH_DES_CBC_SHA
+ * @run main SSL TLS_KRB5_WITH_DES_CBC_MD5
+ * @run main SSL TLS_KRB5_EXPORT_WITH_RC4_40_SHA
+ * @run main SSL TLS_KRB5_EXPORT_WITH_RC4_40_MD5
+ * @run main SSL TLS_KRB5_EXPORT_WITH_DES_CBC_40_SHA
+ * @run main SSL TLS_KRB5_EXPORT_WITH_DES_CBC_40_MD5
  */
 import java.io.*;
 import java.net.InetAddress;
@@ -37,7 +47,7 @@ import sun.security.krb5.internal.ktab.KeyTab;
 
 public class SSL {
 
-    private static final String KRB5_CIPHER = "TLS_KRB5_WITH_3DES_EDE_CBC_SHA";
+    private static String krb5Cipher;
     private static final int LOOP_LIMIT = 1;
     private static int loopCount = 0;
     private static volatile String server;
@@ -45,6 +55,8 @@ public class SSL {
 
     public static void main(String[] args) throws Exception {
 
+        krb5Cipher = args[0];
+
         KDC kdc = KDC.create(OneKDC.REALM);
         // Run this after KDC, so our own DNS service can be started
         try {
@@ -117,7 +129,7 @@ public class SSL {
             SSLSocket sslSocket = (SSLSocket) sslsf.createSocket(server, port);
 
             // Enable only a KRB5 cipher suite.
-            String enabledSuites[] = {KRB5_CIPHER};
+            String enabledSuites[] = {krb5Cipher};
             sslSocket.setEnabledCipherSuites(enabledSuites);
             // Should check for exception if enabledSuites is not supported
 
@@ -155,7 +167,7 @@ public class SSL {
             port = sslServerSocket.getLocalPort();
 
             // Enable only a KRB5 cipher suite.
-            String enabledSuites[] = {KRB5_CIPHER};
+            String enabledSuites[] = {krb5Cipher};
             sslServerSocket.setEnabledCipherSuites(enabledSuites);
             // Should check for exception if enabledSuites is not supported