Continue for the rest of the update.
On 6/3/2019 5:42 PM, Anthony Scarpino wrote:
http://cr.openjdk.java.net/~ascarpino/stateless/webrev.02
Finished.java
-------------
447 NewSessionTicket.handshake12Producer.produce(shc,
message);
624 SSLHandshake[] probableHandshakeMessages = new
SSLHandshake[] {
625 SSLHandshake.NEW_SESSION_TICKET,
626 SSLHandshake.FINISHED
627 };
628
629 for (SSLHandshake hs : probableHandshakeMessages) {
630 HandshakeProducer handshakeProducer =
631 shc.handshakeProducers.remove(hs.id);
632 if (handshakeProducer != null) {
633 handshakeProducer.produce(shc, fm);
634 }
635 }
If reading the two blocks together, looks like NewSessionTicket producer
is called twice at line 447 and line 633.
SSLSessionContextImpl.java
--------------------------
73 private boolean statelessSession = true;
May be able to declare as 'final' field.
230 private int getDefaultCacheLimit(boolean server) {
Except to get the default cache limit, this method also trying to get
more configuration
(jdk.tls.server.enableSessionTicketExtension/jdk.tls.client.enableSessionTicketExtension,
etc). To be more clear, it may worthy to update the method name or
using different method for different purpose.
SSLSessionImpl.java
-------------------
304 SSLSessionImpl(HandshakeContext hc, ByteBuffer buf) throws
IOException {
...
308 this.host = hc.conContext.transport.getPeerHost();
309 this.port = hc.conContext.transport.getPeerPort();
310 this.localSupportedSignAlgs = new ArrayList<>();
The host, port and localSupportedSignAlgs are using the current
transport host and port, but not from the previous session. There might
be some potential security problems if the current
host/port/localSupportedSignAlgs different from the precious session.
-------------
320 // The CH session id may reset this if it's provided
321 this.sessionId = new SessionId(true,
322 hc.sslContext.getSecureRandom());
New session ID are generated for resumption session. There are might be
compatibility issues because of the current SSLSession.getId() spec.
-------------
312 boundValues = null;
Applications configured boundValues get lost.
Basically, I was wondering if the session data could be used to
construct a SSLSession object that comply to the public APIs of
SSLSession and ExtendedSSLSession. Please make sure the following
information is not missed (I may miss something, please double check the
public API for sure):
Session ID
SSLSessionContext
getLastAccessedTime
How to invalidate? (see bellow question)
boundValues
peer principle (via cert)
peer host
peer port
getPacketBufferSize/getApplicationBufferSize
getPeerSupportedSignatureAlgorithms
getStatusResponses
As John had reviewed the test code, I will skip that part.
Okay, I'm done with the code review. No major concerns to me. I'm fine
if you want to address my concerns after RPD1.
Thanks,
Xuelei
