[9] RFC: 8061798: Add support for TLS_FALLBACK_SCSV (RFC 7507)

[Xuelei Fan]

Hi Florian,

I can sponsor you for the specification update approval and changeset
integration if needed.

I still have a few comments about this fix:

1. TLS_FALLBACK_SCSV may still be miss-used as it is defined as a
default enabled cipher suites.

Some applications may just get all supported cipher suites and enable
them all for a certain circumstance.  For example:

    String[] suites = sslSocket.getSupportedCipherSuites();
    sslSocket.setEnabledCipherSuites(suites);

As would result in unexpected behavior as you concerned about this protocol.

What do you think if TLS_FALLBACK_SCSV is not defined as a supported
(enabled) cipher suite?  I mean SSLSocket.getSupportedCipherSuites()
would not return TLS_FALLBACK_SCSV in any cases.

2. SSLEngineImpl
Better to update SSLEngineImpl.java in this fix as what you did for
SSLSocketImpl.java

3. src/java.base/share/classes/javax/net/ssl/SSLParameters.java
------------
 472      * <b>Note:</b> Performing protocol downgrades outside the TLS
 473      * protocol can introduce security vulnerabilities.  The
 474      * built-in version negotiation mechanism of the TLS protocol
 475      * should be used instead of explicit protocol downgrades.
 476      * See section 4 of RFC 7507 for additional advice.

Sounds like it is a part of "implNote".

------------
 478      * @implNote
 479      * The default JSSE provider uses the TLS_FALLBACK_SCSV signaling
 480      * ciphersuite value to indicate protocol fallback, as specified
 481      * in RFC 7507.

As this is a public spec, any JSSE provider should follow this spec.
I'd like to say:
   "A JSSE provider should use the TLS_FALLBACK_SCSV ..."

------------
 491     public final void setIndicateProtocolFallback(
                 boolean indicateFallback) {
 506     public final boolean getIndicateProtocolFallback()

"setIndicate" does not sound like instinctive enough to me.  What do you
think:
        void indicateProtocolFallback(boolean useProtocolFallback)
        boolean isProtocolFallback()
or
        void setUseProtocolFallback(boolean useProtocolFallback)
        boolean getUseProtocolFallback()

Thanks,
Xuelei


On 5/7/2015 2:01 PM, Florian Weimer wrote:
> On 05/07/2015 01:39 AM, Xuelei Fan wrote:
>> On 5/6/2015 9:42 PM, Florian Weimer wrote:
>>> On 05/06/2015 01:42 AM, Xuelei Fan wrote:
>>>> As additional APIs are strongly desired, what do you think to make the
>>>> API more general and easy to use?  For example, using the name:
>>>>
>>>>     SSLParameters.setUseFallbackMode(boolean isFallback)
>>>>     boolean SSLParameters.getuseFallbackMode()
>>>>
>>>> We can implement more for this parameters if need to take care of
>>>> additional more problems during fallback negotiation.  Instinctively,
>>>> developers and code reviewers would not call this APIs unless this is a
>>>> real fallback negotiation, I think.
>>>
>>> Sounds reasonable.  I have add an @implNote mentioning that the default
>>> provider sends TLS_FALLBACK_SCSV.
>>>
>>>   <http://cr.openjdk.java.net/~fweimer/8061798/webrev.01/>
>>>
>> I guess the new webrev may be:
>>    http://cr.openjdk.java.net/~fweimer/8061798/webrev.02/
> 
> Right, sorry.
>