8042449: Issue for negative byte major record version

Summary: Convert byte to positive integer before making comparison. Also reviewed by Florian Weimer <fweimer@redhat.com>.
Reviewed-by: wetmore

src/share/classes/sun/security/ssl/ByteBufferInputStream.java

 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2014, 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
@@ -60,7 +60,8 @@ class ByteBufferInputStream extends InputStream {
         if (bb.remaining() == 0) {
             return -1;
         }
-        return bb.get();
+
+        return (bb.get() & 0xFF);   // need to be in the range 0 to 255
     }
 
     /**

src/share/classes/sun/security/ssl/EngineInputRecord.java

 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2014, 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
@@ -109,14 +109,8 @@ final class EngineInputRecord extends InputRecord {
             ProtocolVersion recordVersion =
                 ProtocolVersion.valueOf(buf.get(pos + 1), buf.get(pos + 2));
 
-            // Check if too old (currently not possible)
-            // or if the major version does not match.
-            // The actual version negotiation is in the handshaker classes
-            if ((recordVersion.v < ProtocolVersion.MIN.v)
-                    || (recordVersion.major > ProtocolVersion.MAX.major)) {
-                throw new SSLException(
-                    "Unsupported record version " + recordVersion);
-            }
+            // check the record version
+            checkRecordVersion(recordVersion, false);
 
             /*
              * Reasonably sure this is a V3, disable further checks.
@@ -147,18 +141,8 @@ final class EngineInputRecord extends InputRecord {
                 ProtocolVersion recordVersion =
                     ProtocolVersion.valueOf(buf.get(pos + 3), buf.get(pos + 4));
 
-                // Check if too old (currently not possible)
-                // or if the major version does not match.
-                // The actual version negotiation is in the handshaker classes
-                if ((recordVersion.v < ProtocolVersion.MIN.v)
-                        || (recordVersion.major > ProtocolVersion.MAX.major)) {
-
-                    // if it's not SSLv2, we're out of here.
-                    if (recordVersion.v != ProtocolVersion.SSL20Hello.v) {
-                        throw new SSLException(
-                            "Unsupported record version " + recordVersion);
-                    }
-                }
+                // check the record version
+                checkRecordVersion(recordVersion, true);
 
                 /*
                  * Client or Server Hello
@@ -406,14 +390,9 @@ final class EngineInputRecord extends InputRecord {
 
         ProtocolVersion recordVersion = ProtocolVersion.valueOf(
                 srcBB.get(srcPos + 1), srcBB.get(srcPos + 2));
-        // Check if too old (currently not possible)
-        // or if the major version does not match.
-        // The actual version negotiation is in the handshaker classes
-        if ((recordVersion.v < ProtocolVersion.MIN.v)
-                || (recordVersion.major > ProtocolVersion.MAX.major)) {
-            throw new SSLException(
-                "Unsupported record version " + recordVersion);
-        }
+
+        // check the record version
+        checkRecordVersion(recordVersion, false);
 
         /*
          * It's really application data.  How much to consume?

src/share/classes/sun/security/ssl/InputRecord.java

 /*
- * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2014, 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
@@ -533,20 +533,36 @@ class InputRecord extends ByteArrayInputStream implements Record {
         }
     }
 
+    /**
+     * Return true if the specified record protocol version is out of the
+     * range of the possible supported versions.
+     */
+    static void checkRecordVersion(ProtocolVersion version,
+            boolean allowSSL20Hello) throws SSLException {
+        // Check if the record version is too old (currently not possible)
+        // or if the major version does not match.
+        //
+        // The actual version negotiation is in the handshaker classes
+        if ((version.v < ProtocolVersion.MIN.v) ||
+            ((version.major & 0xFF) > (ProtocolVersion.MAX.major & 0xFF))) {
+
+            // if it's not SSLv2, we're out of here.
+            if (!allowSSL20Hello ||
+                    (version.v != ProtocolVersion.SSL20Hello.v)) {
+                throw new SSLException("Unsupported record version " + version);
+            }
+        }
+    }
+
     /**
      * Read a SSL/TLS record. Throw an IOException if the format is invalid.
      */
     private void readV3Record(InputStream s, OutputStream o)
             throws IOException {
         ProtocolVersion recordVersion = ProtocolVersion.valueOf(buf[1], buf[2]);
-        // Check if too old (currently not possible)
-        // or if the major version does not match.
-        // The actual version negotiation is in the handshaker classes
-        if ((recordVersion.v < ProtocolVersion.MIN.v)
-                || (recordVersion.major > ProtocolVersion.MAX.major)) {
-            throw new SSLException(
-                "Unsupported record version " + recordVersion);
-        }
+
+        // check the record version
+        checkRecordVersion(recordVersion, false);
 
         /*
          * Get and check length, then the data.

src/share/classes/sun/security/ssl/ProtocolVersion.java

 /*
- * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2014, 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
@@ -101,7 +101,7 @@ public final class ProtocolVersion implements Comparable<ProtocolVersion> {
         this.v = v;
         this.name = name;
         major = (byte)(v >>> 8);
-        minor = (byte)(v & 0xff);
+        minor = (byte)(v & 0xFF);
     }
 
     // private
@@ -117,8 +117,8 @@ public final class ProtocolVersion implements Comparable<ProtocolVersion> {
         } else if (v == SSL20Hello.v) {
             return SSL20Hello;
         } else {
-            int major = (v >>> 8) & 0xff;
-            int minor = v & 0xff;
+            int major = (v >>> 8) & 0xFF;
+            int minor = v & 0xFF;
             return new ProtocolVersion(v, "Unknown-" + major + "." + minor);
         }
     }
@@ -128,10 +128,7 @@ public final class ProtocolVersion implements Comparable<ProtocolVersion> {
      * numbers. Never throws exceptions.
      */
     public static ProtocolVersion valueOf(int major, int minor) {
-        major &= 0xff;
-        minor &= 0xff;
-        int v = (major << 8) | minor;
-        return valueOf(v);
+        return valueOf(((major & 0xFF) << 8) | (minor & 0xFF));
     }
 
     /**

test/javax/net/ssl/SSLEngine/IllegalRecordVersion.java

+/*
+ * Copyright (c) 2014, 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.
+ */
+
+// This test case relies on updated static security property, no way to re-use
+// security property in samevm/agentvm mode.
+
+/*
+ * @test
+ * @bug 8042449
+ * @summary Issue for negative byte major record version
+ *
+ * @run main/othervm IllegalRecordVersion
+ */
+
+import javax.net.ssl.*;
+import javax.net.ssl.SSLEngineResult.*;
+import java.io.*;
+import java.security.*;
+import java.nio.*;
+
+public class IllegalRecordVersion {
+
+    public static void main(String args[]) throws Exception {
+        SSLContext context = SSLContext.getDefault();
+
+        SSLEngine cliEngine = context.createSSLEngine();
+        cliEngine.setUseClientMode(true);
+        SSLEngine srvEngine = context.createSSLEngine();
+        srvEngine.setUseClientMode(false);
+
+        SSLSession session = cliEngine.getSession();
+        int netBufferMax = session.getPacketBufferSize();
+        int appBufferMax = session.getApplicationBufferSize();
+
+        ByteBuffer cliToSrv = ByteBuffer.allocateDirect(netBufferMax);
+        ByteBuffer srvIBuff = ByteBuffer.allocateDirect(appBufferMax + 50);
+        ByteBuffer cliOBuff = ByteBuffer.wrap("I'm client".getBytes());
+
+
+        System.out.println("client hello (record version(0xa9, 0xa2))");
+        SSLEngineResult cliRes = cliEngine.wrap(cliOBuff, cliToSrv);
+        System.out.println("Client wrap result: " + cliRes);
+        cliToSrv.flip();
+        if (cliToSrv.limit() > 5) {
+            cliToSrv.put(1, (byte)0xa9);
+            cliToSrv.put(2, (byte)0xa2);
+        }
+
+        try {
+            srvEngine.unwrap(cliToSrv, srvIBuff);
+            throw new Exception(
+                "Cannot catch the unsupported record version issue");
+        } catch (SSLException e) {
+            // get the expected exception
+        }
+    }
+}