On 10/17/2017 11:45 AM, Martin Balao wrote:
http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-8148421/webrev.03/
TlsMasterSecretParameterSpec.java
---------------------------------
This spec update impacts the PKCS11 implementation too. Please update
jdk/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11TlsMasterSecretGenerator.java
as well.
ClientHandshaker.java:
----------------------
Per RFC 7627:
1. For full handshaking, a client MUST send the "extended_master_secret"
extension.
2. A client SHOULD NOT offer an abbreviated handshake to resume a
session that that does not use an extended master secret. Instead, it
SHOULD offer a full handshake.
3. When offering an abbreviated handshake, the client MUST send the
"extended_master_secret" extension in its ClientHello.
4. For abbreviated handshake, if the original session did not use the
"extended_master_secret" extension but the new ServerHello contains the
extension, the client MUST abort the handshake.
5. For abbreviated handshake, if the original session used the extension
but the new ServerHello does not contain the extension, the client MUST
abort the handshake.
If I'm reading correct, the update in ClientHandshaker.java implements
#1, but missing #2, #4 and #5, and a conditional support of #3.
For the missing of #4 and #5, it might be mainly caused by that the
800-828 lines are put in a place where full handshaking happens.
Abbreviated handshake return at line 756 and cannot reach line 800.
For #2, I may suggest combine the extension together with System
property "jdk.tls.allowUnsafeServerCertChange". If not using extended
master secret, and not allowUnsafeServerCertChange, and
useExtendedMasterSecretExtension, do not offer abbreviated handshake.
Using full handshake instead for TLS 1.0+. Besides, if using the
extension, don't use the server certificate change checking any more.
See allowUnsafeServerCertChange comments in ClientHandshaker.java.
For #3, I may always send the "extended_master_secret" extension, the
server side can handle it property, no matter the original session use
the extension or not.
SSLSessionImpl.java
-------------------
94 private boolean useExtendedMasterSecret;
200 void setUseExtendedMasterSecret() {
211 boolean getUseExtendedMasterSecret() {
I may suggest use "final" useExtendedMasterSecret (set during
construction), so that the set/get methods do not compete against each
other. Using "final" may need to adjust some source code. Looks like
it is doable.
ServerHandshaker.java
---------------------
For safer, as there is no compatibility impact as if the client request
for the extension, I think we may want to always enable the extension in
server side. It means the system property
"jsse.useExtendedMasterSecret" disables the extension in client side
only. And the property cannot be used to disable server acceptance of
the extension.
Per RFC 7627:
A. For full handshaking, if a server implementing this document receives
the "extended_master_secret" extension, it MUST include the extension in
its ServerHello message.
B. For abbreviated handshake request, If the original session did not
use the "extended_master_secret" extension but the new ClientHello
contains the extension, then the server MUST NOT perform the abbreviated
handshake. Instead, it SHOULD continue with a full handshake.
C. For abbreviated handshake request, if the original session used the
"extended_master_secret" extension but the new ClientHello does not
contain it, the server MUST abort the abbreviated handshake.
D. For abbreviated handshake request, if neither the original session
nor the new ClientHello uses the extension, the server SHOULD abort the
handshake.
E. For abbreviated handshake request, if the new ClientHello contains
the extension and the server chooses to continue the handshake, then the
server MUST include the "extended_master_secret" extension in its
ServerHello message.
If I'm reading correct, the update in ServerHandshaker.java implements
#A, #C and #E, but missing #B and #C.
For #B, I think it should be fine to follow the spec: continue with a
full handshake.
For #C, I was wondering we may need a new system property
(jsse.allowLegacyResumption?) to turn on/off this behavior. If
application want a strict mode, the server abort the abbreviated
handshake for case #C. Otherwise, the server can continue with an
abbreviated handshake in order to support legacy resumption.
Hope it helps!
Regards,
Xuelei
