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
--- a/jdk/src/share/classes/sun/security/ssl/ByteBufferInputStream.java Tue May 06 10:32:32 2014 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/ByteBufferInputStream.java Wed May 07 02:24:01 2014 +0000
@@ -1,5 +1,5 @@
/*
- * 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 @@
if (bb.remaining() == 0) {
return -1;
}
- return bb.get();
+
+ return (bb.get() & 0xFF); // need to be in the range 0 to 255
}
/**
--- a/jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java Tue May 06 10:32:32 2014 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java Wed May 07 02:24:01 2014 +0000
@@ -1,5 +1,5 @@
/*
- * 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 @@
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 @@
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 @@
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?
--- a/jdk/src/share/classes/sun/security/ssl/InputRecord.java Tue May 06 10:32:32 2014 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/InputRecord.java Wed May 07 02:24:01 2014 +0000
@@ -1,5 +1,5 @@
/*
- * 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
@@ -534,19 +534,35 @@
}
/**
+ * 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.
--- a/jdk/src/share/classes/sun/security/ssl/ProtocolVersion.java Tue May 06 10:32:32 2014 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/ProtocolVersion.java Wed May 07 02:24:01 2014 +0000
@@ -1,5 +1,5 @@
/*
- * 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 @@
this.v = v;
this.name = name;
major = (byte)(v >>> 8);
- minor = (byte)(v & 0xff);
+ minor = (byte)(v & 0xFF);
}
// private
@@ -117,8 +117,8 @@
} 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 @@
* 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));
}
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/javax/net/ssl/SSLEngine/IllegalRecordVersion.java Wed May 07 02:24:01 2014 +0000
@@ -0,0 +1,77 @@
+/*
+ * 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
+ }
+ }
+}