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....@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....@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.
