# HG changeset patch # User xuelei # Date 1399429441 0 # Node ID f95477ce56e4d27ccf2c295c483f1ea5487a339d # Parent 78d04b441327466d69d6134f70854ea7838ee149 8042449: Issue for negative byte major record version Summary: Convert byte to positive integer before making comparison. Also reviewed by Florian Weimer . Reviewed-by: wetmore diff -r 78d04b441327 -r f95477ce56e4 jdk/src/share/classes/sun/security/ssl/ByteBufferInputStream.java --- 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 } /** diff -r 78d04b441327 -r f95477ce56e4 jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java --- 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? diff -r 78d04b441327 -r f95477ce56e4 jdk/src/share/classes/sun/security/ssl/InputRecord.java --- 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. diff -r 78d04b441327 -r f95477ce56e4 jdk/src/share/classes/sun/security/ssl/ProtocolVersion.java --- 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)); } /** diff -r 78d04b441327 -r f95477ce56e4 jdk/test/javax/net/ssl/SSLEngine/IllegalRecordVersion.java --- /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 + } + } +}