RFR (S): JDK-8149169 - SSLSocketInputRecord.decodeInputRecord buffer overflow

[Langer, Christoph]

Hi,



I think I've found a way to fix the issue which looks quite reasonable to me. Would you please comment/review it? I've also included a test to reproduce the issue.



Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8149169.1/

Bug: https://bugs.openjdk.java.net/browse/JDK-8149169



Thanks and best regards

Christoph



From: Langer, Christoph
Sent: Dienstag, 15. März 2016 23:00
To: security-dev at openjdk.java.net
Subject: Regarding JDK-8149169 - SSLSocketInputRecord.decodeInputRecord buffer overflow



Hi there,



today I did some debugging regarding the TLS exception I've seen and reported in JDK-8149169:



javax.net.ssl.SSLException: java.nio.BufferOverflowException
        at sun.security.ssl.Alerts.getSSLException(Alerts.java:214)
        at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1948)
        at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1900)
        at sun.security.ssl.SSLSocketImpl.handleException(SSLSocketImpl.java:1883)
        at sun.security.ssl.SSLSocketImpl.handleException(SSLSocketImpl.java:1809)
        at sun.security.ssl.AppInputStream.read(AppInputStream.java:173)
        at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
        at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
        at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
        at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:704)<http://www.http.HttpClient.parseHTTPHeader%28HttpClient.java:704%29>
        at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:647)<http://www.http.HttpClient.parseHTTP%28HttpClient.java:647%29>
        at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:675)<http://www.http.HttpClient.parseHTTP%28HttpClient.java:675%29>
        at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1534)<http://www.protocol.http.HttpURLConnection.getInputStream0%28HttpURLConnection.java:1534%29>
        at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1439)<http://www.protocol.http.HttpURLConnection.getInputStream%28HttpURLConnection.java:1439%29>
        at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:480)
        at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:319)<http://www.protocol.https.HttpsURLConnectionImpl.getResponseCode%28HttpsURLConnectionImpl.java:319%29>
        at com.sap.cl.HttpsURLConnectionTest.sendGETRequest(HttpsURLConnectionTest.java:42)
        at com.sap.cl.HttpsURLConnectionTest.main(HttpsURLConnectionTest.java:63)
Caused by: java.nio.BufferOverflowException
        at java.nio.HeapByteBuffer.put(HeapByteBuffer.java:206)
        at sun.security.ssl.SSLSocketInputRecord.decodeInputRecord(SSLSocketInputRecord.java:226)
        at sun.security.ssl.SSLSocketInputRecord.decode(SSLSocketInputRecord.java:178)
        at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1012)
        at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:957)
        at sun.security.ssl.AppInputStream.read(AppInputStream.java:159)
        ... 12 more



I think the problem is with the logic in sun.security.ssl.AppInputStream. read(byte[] b, int off, int len). The read method calls the readRecord(buffer) method of the socket (SSLSocketImpl) and hands it the buffer to be eventually filled by SSLSocketInputRecord.decodeInputRecord(). The buffer is initialized with 4K and before readRecord is called, the packet length is verified (in line 144: int packetLen = socket.bytesInCompletePacket();) and the buffer reallocated if the incoming package would be larger than the buffer. However, in my case, the incoming package is a handshake package of a small size, so the buffer won't be adjusted. Then, after the handshake is done, the real data packet is read, still within SSLSocketImpl.readRecord() (e.g. line 1012 of SSLSocketImpl) and this one has a length of more than 4K. So the buffer will be too small in decodeInputRecord and hence the exception is thrown.



So, basically the issue will appear if the TLS data package following immediately after a server initiated handshake will be larger than the buffer of AppInputStream. I guess that should be easily recreatable in a small test case.



Now the question how to fix? I can see 3 options:

a)      Just allocate the ByteBuffer in AppInputStream to SSLRecord.maxLargeRecordSize (about 32K) - easiest fix and removing the need to check the length for each record. But I guess this is not desired as the buffer is unnecessarily large for most cases?

b)      Extend SSLSocketInputRecord somehow to be able to not only read the length of the incoming packet but also the type, e.g. if it is a handshake. In that case the buffer would need to be extended to SSLRecord.maxLargeRecordSize. But why not do a) then??

c)       Check the volume bytes returned from readRecord and redo the read in case the volume is larger than the buffer capacity



Which way should I pursue? Do you see another option? Or am I getting something completely wrong running into an illegal case?



Thanks in advance for your feedback,

Christoph



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/security-dev/attachments/20160316/07d7c510/attachment.html>