Inconsistent SSLEngine behavior for closing outbound while in handshake in 11ea22
Xuelei Fan
xuelei.fan at oracle.com
Tue Aug 7 14:54:25 UTC 2018
Hi Tim,
Thank you very much for the test code. I can play with it.
I made an update accordingly in the new webrev patch:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.03/
Would you mind test if the patch works for you?
Thanks & Regards,
Xuelei
On 8/6/2018 5:01 PM, Tim Brooks wrote:
> Hi Xuelei,
>
> I’ve attached a java file with a method that can be used as a test case. The caller will need to provide SSLEngines. I assume that is something you can do? As I said this is with TLS 1.2.
>
> I catch the exceptions I mentioned in my last email. Although there are comments that I do not think that these exceptions make sense. Additionally, the failing assertions are currently wrapped in if(false) {} so that the method will complete.
>
> Let me know if you have any questions.
>
> - Tim=
>
>
>
>
>
>> On Aug 3, 2018, at 6:47 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>
>> Hi Tim,
>>
>> Your concerns make sense to me. Did you have a test case that I can use to reproduce the issues?
>>
>> Thanks,
>> Xuelei
>>
>> On 8/3/2018 5:04 PM, Tim Brooks wrote:
>>> Hi Xuelei
>>> I pulled the JDK 11 source today and applied your patch. And then I built the jdk to run some tests. I hope that is the correct approach.
>>> It appears that this change resolved some of my prior issues. But I notice some other issues. This is tested with TLS 1.2. I have not really setup my environment yet for TLS 1.3, so I was not able to test that code path. I have -Djavax.net.debug=all enabled. However, I have pruned some of messages from this email to reduce text. If you need more let me know.
>>> Client:
>>> 1. ClientHello.java:633|Produced ClientHello handshake message
>>> 2. SSLEngineOutputRecord.java:465|WRITE: TLS12 handshake, length = 204
>>> Server:
>>> 3. SSLEngineInputRecord.java:214|READ: TLSv1.2 handshake, length = 204
>>> 4. ClientHello.java:788|Consuming ClientHello handshake message
>>> 5. ClientHello.java:818|Negotiated protocol version: TLSv1.2
>>> 6. ServerHello.java:361|Produced ServerHello handshake message
>>> 7. CertificateMessage.java:263|Produced server Certificate handshake message
>>> 8. ServerHelloDone.java:97|Produced ServerHelloDone handshake message
>>> 9. SSLEngineOutputRecord.java:465|WRITE: TLS12 handshake, length = 1086
>>> Client:
>>> 10. SSLEngineInputRecord.java:214|READ: TLSv1.2 handshake, length = 1086
>>> 11. ServerHello.java:862|Consuming ServerHello handshake message
>>> 12. ServerHello.java:958|Negotiated protocol version: TLSv1.2
>>> 13. CertificateMessage.java:358|Consuming server Certificate handshake message
>>> 14. ServerHelloDone.java:142|Consuming ServerHelloDone handshake message
>>> 15. RSAClientKeyExchange.java:195|Produced RSA ClientKeyExchange handshake message
>>> 16. ChangeCipherSpec.java:109|Produced ChangeCipherSpec message
>>> 17. ChangeCipherSpec.java:109|Produced ChangeCipherSpec message
>>> 18. SSLEngineOutputRecord.java:465|WRITE: TLS12 handshake, length = 262
>>> 19. SSLEngineOutputRecord.java:465|WRITE: TLS12 change_cipher_spec, length = 1
>>> 20. SSLEngineOutputRecord.java:465|WRITE: TLS12 handshake, length = 16
>>> At this point I do not send that data yet to the server. The client has produced the client key exchange and client finished messages. But they are still in transit (in this scenario).
>>> Server (we manually call closeOutboud() because we have decided we need to close this socket):
>>> 21. SSLEngineImpl.java:745|Closing outbound of SSLEngine
>>> 22. SSLEngineOutputRecord.java:465|WRITE: TLS12 alert, length = 2
>>> 23. SSLEngineOutputRecord.java:465|Raw write (0000: 15 03 03 00 02 01 5A ......Z)
>>> - I believe this is user_canceled alert
>>> 24. SSLEngineOutputRecord.java:465|Raw write (0000: 15 03 03 00 02 01 00 .......)
>>> - I believe this is close_notify
>>> At this point:
>>> Server engine.isOutboundDone() = true
>>> Server engine.isInboundDone() = false
>>> Client:
>>> 24. SSLEngineInputRecord.java:214|READ: TLSv1.2 alert, length = 2
>>> 25. Alert.java:232|Received alert message ("Alert": {"level" : "warning", "description": "user_canceled"})
>>> 26. SSLEngineInputRecord.java:214|READ: TLSv1.2 alert, length = 2
>>> 27. Alert.java:232|Received alert message ("Alert": {"level": "warning", "description": "close_notify"})
>>> 28. Fatal (UNEXPECTED_MESSAGE): Received close_notify during handshake (
>>> "throwable" : {
>>> javax.net.ssl.SSLProtocolException: Received close_notify during handshake
>>> at java.base/sun.security.ssl.Alert.createSSLException(Alert.java:126)
>>> at java.base/sun.security.ssl.Alert.createSSLException(Alert.java:117)
>>> at java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:307)
>>> at java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:263)
>>> At this point I want to note that it feels weird that this produces an exception. The other side said "user_canceled" and then sent "close_notify". That seems correct to me? So it feels weird that consumers of the SSlEngine have to handle this exception and then continue using the engine for a proper close. That's just a side note. I (think) this behavior is not new. But it seems odd.
>>> At this point:
>>> Client engine.isOutboundDone() = false
>>> Client engine.isInboundDone() = true
>>> Still Client:
>>> 29. SSLEngineOutputRecord.java:465|WRITE: TLS12 alert, length = 2
>>> 30. SSLCipher.java:1726|Plaintext before ENCRYPTION (0000: 02 0A ..)
>>> 31. 2018-08-04 05:16:34.300 KGT|SSLEngineOutputRecord.java:483|Raw write (
>>> 0000: 15 03 03 00 1A 00 00 00 00 00 00 00 01 E2 36 F9 ..............6.
>>> 0010: 09 D2 20 65 B5 C9 04 CB D3 47 8B E2 CA 0B B2 .. e.....G.....
>>> )
>>> - I believe this is unexpected_message. And I believe it is encrypted at this point as the client has sent the client finished message. This alert feels incorrect to me. The server sent "user_canceled". I feel like once that is sent, we should be able to receive "close_notify" without the client engine identifying this as an unexpected message.
>>> At this point:
>>> Client engine.isOutboundDone() = true
>>> Client engine.isInboundDone() = true
>>> That seems correct to me. We need to flush the last alert for the client and then we are done with the engine (although I believe the alert should be close_notify and not unexpected_message).
>>> At this point the server starts receiving all the handshake messages from the client.
>>> Server:
>>> 32. SSLEngineInputRecord.java:214|READ: TLSv1.2 handshake, length = 262
>>> 33. RSAClientKeyExchange.java:279|Consuming RSA ClientKeyExchange handshake message
>>> 34. SSLEngineInputRecord.java:214|READ: TLSv1.2 change_cipher_spec, length = 1
>>> 35. ChangeCipherSpec.java:143|Consuming ChangeCipherSpec message
>>> 36. SSLEngineInputRecord.java:214|READ: TLSv1.2 handshake, length = 40
>>> 37. Finished.java:581|Consuming client Finished handshake message.
>>> At this point:
>>> Server engine.isOutboundDone() = false
>>> Server engine.isInboundDone() = false
>>> That seems completely wrong to me. We manually told the server outbound was done. But receiving the final handshake messages has put it back to outbound not being done. Additionally, at this point the server's handshake status is NEED_WRAP and NEED_TASK. It looks like it is trying to continue handshaking.
>>> Still Server:
>>> 38. ChangeCipherSpec.java:109|Produced ChangeCipherSpec message
>>> 39. Finished.java:450|Produced server Finished handshake message
>>> 40. SSLEngineInputRecord.java:214|READ: TLSv1.2 alert, length = 26
>>> 41. Alert.java:232|Received alert message
>>> 42. TransportContext.java:312|Fatal (UNEXPECTED_MESSAGE): Received fatal alert: unexpected_message (
>>> "throwable" : {
>>> javax.net.ssl.SSLProtocolException: Received fatal alert: unexpected_message
>>> at java.base/sun.security.ssl.Alert.createSSLException(Alert.java:126)
>>> at java.base/sun.security.ssl.Alert.createSSLException(Alert.java:117)
>>> at java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:307)
>>> at java.base/sun.security.ssl.Alert$AlertConsumer.consume(Alert.java:281)
>>> At this point the server engine has received the alert. That throws an exception. And the engine still seems to be in an incorrect "done" state.
>>> At this point:
>>> Server engine.isOutboundDone() = false
>>> Server engine.isInboundDone() = true
>>> Let me know if you have any questions. The particular point where we send close_notify from the server to the client is just incidental from how our "close during handshake" test works. Sending it at other points in the handshake process may produce different outcomes.
>>> Thanks,
>>> Tim
>>>> On Jul 30, 2018, at 12:13 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>>>
>>>> Hi Tim,
>>>>
>>>> Would you mind look at the code I posted in the following thread:
>>>> http://mail.openjdk.java.net/pipermail/security-dev/2018-July/017708.html
>>>>
>>>> In the update, we are trying make the synchronization more simple and robust. I appreciate if you could comment by the end of this week.
>>>>
>>>> Note that with this update, a complete TLS connection should close both inbound and outbound explicitly. However, existing applications may not did this way because TLS 1.2 and prior version can work around it. But for TLS 1.3, it is possible to hang the application if the connection is not closed. If the source code update is not available, please consider to use the "jdk.tls.acknowledgeCloseNotify" System Property as a workaround.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 7/18/2018 11:51 AM, Tim Brooks wrote:
>>>>> Yes. I can test once there is a patch. My inquiry was motivated by some work on Elasticsearch fyi. I can test a patch against that work.
>>>>> https://github.com/elastic/elasticsearch/issues/32144
>>>>> - Tim
>>>>>> On Jul 17, 2018, at 8:40 PM, Xuelei Fan <xuelei.fan at oracle.com <mailto:xuelei.fan at oracle.com>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> We are working on the JDK 11 close issue.
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8207009
>>>>>>
>>>>>> I appreciate if you can help test if we have a patch.
>>>>>>
>>>>>> Thanks,
>>>>>> Xuelei
>>>>>>
>>>>>> On 7/17/2018 4:26 PM, Tim Brooks wrote:
>>>>>>> My understanding is that when you are interested in closing the underlying socket when using the SSLEngine, you must call closeOutbound() and WRAP and UNWRAP until both isInboundDone() and isOutboundDone() return true.
>>>>>>> One edge case of this is if you are interested in closing the socket prior to the completion of a handshake. In JDK 10.0.1 (and I believe prior JDKs) this was the behavior for one way in which this arises:
>>>>>>> 1. Initiate handshake
>>>>>>> 2. UNWRAP data from client
>>>>>>> 3. WRAP data to send to client. Handshake status is "NEED_UNWRAP"
>>>>>>> 4. Call closeOutbound() (perhaps the server is shutting down and you want to close the connection).
>>>>>>> 5. Handshake status now returns "NEED_WRAP"
>>>>>>> JDK10:
>>>>>>> isInboundDone() - returns false
>>>>>>> isOutboundDone() - returns false
>>>>>>> A call to wrap() produces 7 bytes and status = CLOSED. Handshake status is now NEED_UNWRAP.
>>>>>>> isInboundDone() - returns false
>>>>>>> isOutboundDone() - returns true
>>>>>>> JDK11:
>>>>>>> isInboundDone() - returns true
>>>>>>> isOutboundDone() - returns false
>>>>>>> A call to wrap() throws the following exception:
>>>>>>> javax.net.ssl.SSLException: Cannot kickstart, the connection is broken or closed
>>>>>>> at java.base/sun.security.ssl.TransportContext.kickstart(TransportContext.java:205)
>>>>>>> at java.base/sun.security.ssl.SSLEngineImpl.writeRecord(SSLEngineImpl.java:167)
>>>>>>> at java.base/sun.security.ssl.SSLEngineImpl.wrap(SSLEngineImpl.java:138)
>>>>>>> at java.base/sun.security.ssl.SSLEngineImpl.wrap(SSLEngineImpl.java:116)
>>>>>>> at java.base/javax.net.ssl.SSLEngine.wrap(SSLEngine.java:471)
>>>>>>> I’m not sure what the procedure for closing a connection prior to handshake completion is for TLS. But obviously this is a scenario that can arise. It seems wrong to me that the state transitions for the SSLEngine do not handle this. The fact that “isOutboundDone()” returns false, but I cannot WRAP seems to be an issue.
More information about the security-dev
mailing list