Update: http://hg.openjdk.java.net/jdk/sandbox/rev/ae0cd8b2e2c2

Xuelei

On 6/11/2018 6:36 PM, Xuelei Fan wrote:


On 6/11/2018 5:56 PM, Bradford Wetmore wrote:
<...skipped...>
262:  What is the point of the aliases argument in the constructor? Was the idea to provide a mapping between suites we originally created with the SSL_ prefix vs the more current TLS_ prefix we used in the later TLS protocols?  There is only an empty string in every constructor, so this code doesn't do anything.

Added the aliases.

Great, thanks.  Once minor formatting comment which would help comparability/readability.  Take or leave it.

     SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA(
             0x0016, true, "SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA",
             "TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA",
             ProtocolVersion.PROTOCOLS_TO_12,
             K_DHE_RSA, B_3DES, M_SHA, H_SHA256),
->
     SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA(
             0x0016, true, "SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA",
                           "TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA",
             ProtocolVersion.PROTOCOLS_TO_12,
             K_DHE_RSA, B_3DES, M_SHA, H_SHA256),

It looks really nice, and I will take it.  Updated in my local workspace, will push later in my next changeset.

<...skipped...>
840:  Is this else/break needed?

Yes.  It is mostly for performance by ignoring unsupported cipher suites look up.

Ah, because they are coming out of .values() ordered, and the unsupported are at the end.  Can you change the comment:

the following cipher suites are not supported.
->
values() is ordered, remaining cipher suites are not supported.

Updated.


SSLServerSocketImpl.java
------------------------
160:  You should allow multiple changes between server to client, and switch enabled protocols/ciphersuites accordingly.

Yes, multiple changes are allowed.

I didn't see a change here.  If you go from server (default) to client, and then back again, shouldn't you reset to the original server values? And set sslConfig.useClientMode()

e.g.
     sslServerSocket.setUseClientMode(true);
     sslServerSocket.setUseClientMode(false);

The current code won't change back to server.

I may miss something.  I think, it is able to change back.

sslServerSocket.setUseClientMode(true) ->
  if it is server mode, set to use client mode; and if using default protocols, set to use client default protocols; if using default cipher suites, set to use client default cipher suites.  sslConfig is set to use client mode.

sslServerSocket.setUseClientMode(false);
  if it is client mode, set to use server mode; and id using default protocols, set to use server default protocols; if using default cipher suites, set to use server default cipher suites.  sslConfig is set to use server mode.

Thanks,
Xuelei

Reply via email to