[8u] TLSv1.3 RFR: 8245469: Remove DTLS protocol implementation

[Martin Balao]

On 5/21/20 10:23 AM, Alexey Bakhtin wrote:
> Please review changes required to backport TLSv1.3 protocol from JDK11.0.7 to JDK8u
> 

Hi,

A few comments below:

------------------------------------

 * OutputRecord.java, introduced by 8196584
  * d13Encrypt -> Should probably be removed.
  * d10Encrypt -> Should probably be removed.

------------------------------------

 * OutputRecord.java / InputRecord.java

  * Not a big deal but I wonder if we should keep hook methods (such as
initHandshaker, finishHandshaker, launchRetransmission, isEmpty,
expectingFinishFlight) and their calls. Even though there would be no
action because DTLSInputRecord and DTLSOutputRecord were removed, we can
minimize code changes and perhaps avoid work in the future if used by
other OutputRecord/InputRecord subclasses. On the other hand, it would
be 'nop' code currently. I'm not sure, just raised this point to think
loudly. Opinions here are welcomed.

------------------------------------

 * SSLContextImpl.java
  * I understand that the comment "// Used for DTLS in server mode
only." is probably wrong because this is defined in TLS 1.3 (section
4.2.2) and is not DTLS 1.3 specific -even though it makes more sense for
UPD connections than TCP-. Looking into the code, seems to me that it is
not only used in SunJSSE's DTLS 1.3 but could be used in TLS 1.3.
However, that comment should be fixed in main line and then backported.
Otherwise, we would be introducing an improvement that applies to main
line in the update release first. This is for the same reason that we
will keep the "// DTLS cookie exchange manager" related comment.

------------------------------------

 * SSLEngineImpl.java

             deltaDsts -= dsts[i].remaining();
         }

-        return new SSLEngineResult(status, hsStatus, deltaSrcs, deltaDsts,
-                ciphertext != null ? ciphertext.recordSN : -1L);
+        return new SSLEngineResult(status, hsStatus, deltaSrcs, deltaDsts);
     }

     private Ciphertext encode(

You probably decided this change because 8043758 (not in JDK-8) is the
one that introduces the SSLEngineResult public constructor that takes
the sequenceNumber as the last argument. However, TLS 1.3 uses this
constructor so it's not DTLS specific. This is obviously a problem
because a public API is being modified.

I need to understand the consequences of not providing this API (and the
corresponding SSLEngineResult::sequenceNumber function counterpart).
Please let me know of your thoughts on this point.

------------------------------------

 * BaseSSLSocketImpl.java

Why are those methods (introduced by 8040062) removed?

------------------------------------

 * SSLConfiguration.java

I'd only replace enableRetransmissions with 'false' and avoid other
inferences -even if they look correct-.

------------------------------------

Thanks,
Martin.-