[External] : Re: SSLEngine.unwrap on read-only input ByteBuffer

Norman Maurer norman.maurer at googlemail.com
Thu Mar 24 20:46:52 UTC 2022


I agree with Chris here that doing in-place modification of the source buffer is really surprising even if it’s not read-only. This really sounds like something I would consider a „breaking change“ as I can’t imagine users would expect this at all (nothing was ever in the javadocs that would suggest this behavior)

Bye
Norman 

> Am 24.03.2022 um 21:32 schrieb Bradford Wetmore <bradford.wetmore at oracle.com>:
> 
> Problem easily duplicated, thanks for the reproducer.
> 
> I've updated the bug with the info.
> 
> Brad
> 
> 
>> On 3/24/2022 9:13 AM, Chris Vest wrote:
>> On Wed, Mar 23, 2022 at 10:38 AM Bradford Wetmore <bradford.wetmore at oracle.com <mailto:bradford.wetmore at oracle.com>> wrote:
>>    Offhand, sounds like a bug to me.  I've filed:
>>    https://bugs.openjdk.java.net/browse/JDK-8283577
>>    <https://bugs.openjdk.java.net/browse/JDK-8283577>
>> Thanks. The in-place use of the input buffer might also be unexpected even when the buffer is not read-only.
>>    By chance, do you have a simple reproducer handy?
>> See https://github.com/netty/netty/pull/12213#issuecomment-1077796917 <https://urldefense.com/v3/__https://github.com/netty/netty/pull/12213*issuecomment-1077796917__;Iw!!ACWV5N9M2RV99hQ!fZx3LxRdafSPcHg6-4XPFumXYR6gTlOaQfC14ixjjjwlZK7IbHD4voW9gxXeHFbPRTToQg$>
>>    Brad
>>    On 3/23/2022 9:54 AM, Chris Vest wrote:
>>     > Hi,
>>     >
>>     > In Netty we've been trying to design some safer APIs, and
>>    attempted to
>>     > make more use of read-only ByteBuffers.
>>     >
>>     > We discovered that SSLEngine.unwrap does not like read-only input
>>     > buffers, even though the input buffers should in theory only be read
>>     > from. We obviously make sure that the output buffers are writable.
>>     >
>>     > By my reading of the javadoc, and the code, I believe this was
>>    intended
>>     > to work - or at least not intended to not work - but probably wasn't
>>     > tested directly.
>>     >
>>     > When we try we get this stack trace on adopt-openjdk-11.0.7:
>>     >
>>     >     javax.net.ssl.SSLProtocolException: null
>>     >     at
>>    java.base/sun.security.ssl.Alert.createSSLException(Alert.java:129)
>>     >     at
>>     >         java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:326)
>>     >     at
>>     >         java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:269)
>>     >     at
>>     >         java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:264)
>>     >     at
>>    java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:118)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.decode(SSLEngineImpl.java:668)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.readRecord(SSLEngineImpl.java:623)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:441)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:420)
>>     >     at java.base/javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:674)
>>     >     at
>>    io.netty5.handler.ssl.EngineWrapper.unwrap(EngineWrapper.java:100)
>>     >     at io.netty5.handler.ssl.SslHandler.unwrap(SslHandler.java:1227)
>>     >     at
>>     >         io.netty5.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1105)
>>     >     at io.netty5.handler.ssl.SslHandler.decode(SslHandler.java:1165)
>>     >     at
>>     >         io.netty5.handler.codec.ByteToMessageDecoderForBuffer.decodeRemovalReentryProtection(ByteToMessageDecoderForBuffer.java:384)
>>     >     at
>>     >         io.netty5.handler.codec.ByteToMessageDecoderForBuffer.callDecode(ByteToMessageDecoderForBuffer.java:327)
>>     >     ... 20 common frames omitted
>>     >     Caused by: java.nio.ReadOnlyBufferException: null
>>     >     at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2493)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLCipher$T12GcmReadCipherGenerator$GcmReadCipher.decrypt(SSLCipher.java:1629)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineInputRecord.decodeInputRecord(SSLEngineInputRecord.java:240)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineInputRecord.decode(SSLEngineInputRecord.java:197)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineInputRecord.decode(SSLEngineInputRecord.java:160)
>>     >     at
>>    java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:108)
>>     >     ... 31 common frames omitted
>>     >
>>     >
>>     > I also tried this on a panama-preview snapshot JDK I have, and got a
>>     > similar stack trace:
>>     >
>>     >     % java -version
>>     >     openjdk version "19-internal" 2022-09-20
>>     >     OpenJDK Runtime Environment (fastdebug build
>>     >     19-internal-adhoc.chris.panama-foreign)
>>     >     OpenJDK 64-Bit Server VM (fastdebug build
>>     >     19-internal-adhoc.chris.panama-foreign, mixed mode)
>>     >
>>     >
>>     >     % git show
>>     >     commit 144af9f43cd2d6f88b675b8c85e4034e5b9d6695 (HEAD ->
>>     >     foreign-preview, origin/foreign-preview)
>>     >
>>     >
>>     >     javax.net.ssl.SSLProtocolException: null
>>     >     at
>>    java.base/sun.security.ssl.Alert.createSSLException(Alert.java:129)
>>     >     at
>>     >         java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:371)
>>     >     at
>>     >         java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:314)
>>     >     at
>>     >         java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:309)
>>     >     at
>>    java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:121)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.decode(SSLEngineImpl.java:736)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.readRecord(SSLEngineImpl.java:691)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:506)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:482)
>>     >     at java.base/javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:719)
>>     >     at
>>    io.netty5.handler.ssl.EngineWrapper.unwrap(EngineWrapper.java:100)
>>     >     at io.netty5.handler.ssl.SslHandler.unwrap(SslHandler.java:1227)
>>     >     at
>>     >         io.netty5.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1105)
>>     >     at io.netty5.handler.ssl.SslHandler.decode(SslHandler.java:1165)
>>     >     at
>>     >         io.netty5.handler.codec.ByteToMessageDecoderForBuffer.decodeRemovalReentryProtection(ByteToMessageDecoderForBuffer.java:384)
>>     >     at
>>     >         io.netty5.handler.codec.ByteToMessageDecoderForBuffer.callDecode(ByteToMessageDecoderForBuffer.java:327)
>>     >     ... 20 common frames omitted
>>     >     Caused by: java.nio.ReadOnlyBufferException: null
>>     >     at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2497)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLCipher$T13GcmReadCipherGenerator$GcmReadCipher.decrypt(SSLCipher.java:1933)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineInputRecord.decodeInputRecord(SSLEngineInputRecord.java:239)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineInputRecord.decode(SSLEngineInputRecord.java:196)
>>     >     at
>>     >         java.base/sun.security.ssl.SSLEngineInputRecord.decode(SSLEngineInputRecord.java:159)
>>     >     at
>>    java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:111)
>>     >     ... 31 common frames omitted
>>     >
>>     >
>>     > We can work around this in Netty since we need to support JDK
>>    versions
>>     > that has this issue anyway, but I think it's a bug that should be
>>    fixed
>>     > at some point.
>>     >
>>     > Thanks,
>>     > Chris
>>     >



More information about the security-dev mailing list