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....@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....@oracle.com 
>>> <mailto:xuelei....@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.

Reply via email to