Code review request, 7188658 Add possibility to disable client initiated renegotiation

Fri Jun 28 00:23:09 UTC 2013

On 6/27/2013 4:59 PM, Xuelei Fan wrote:
> I agree that the property name is pretty confusing now.  We need to
> consolidate the property naming styles, at least in JSSE component. I
> will go back to this topic later.

I've filed:

     JDK-8019346 Reconsider the namespace for JDK-7188658

to track for 8.

Brad

> Xuelei
>
> On 6/28/2013 6:36 AM, Brad Wetmore wrote:
>> Going back to this...
>>
>> On 6/18/2013 9:04 PM, Xuelei Fan wrote:
>>> On 6/19/2013 10:50 AM, Brad Wetmore wrote:
>>>> Sorry for the delay.
>>>>
>>>> Two comments about the property name.
>>>>
>>>> I don't see the jdk.tls *System* Property prefix used anywhere else,
>>>> except as a *Security* properties:
>>>>
>>>>       jdk.tls.rejectClientInitializedRenego
>>>>
>>>> Otherwise, I think we should stick to one of the current namespace.
>>>>
>>>>       jsse.               (my preference since this is expected across
>>>>                            other JDKs)
>>>>       sun.security.ssl.
>>>>
>>> At the beginning, I use the prefix "jsse.*". But it is rejected (CCC)
>>> because the system property has no effect on other providers.
>>
>> ...and...
>>
>>> "jsse" prefix
>>> should not be used in provider level libraries (for example, Oracle
>>> JSSE provider).
>>
>> I'm not following the logic at all here.  I think I'm hearing that:
>>
>>      jsse.*:    used by the javax.net.ssl.* code
>>      jdk.tls.*: used by the sun.security.ssl.* code
>>
>> But that doesn't quite make sense since we already have the following
>> SunJSSE-specific properties:  jsse.enableSNIExtension,
>> jsse.SSLEngine.acceptLargeFragments, jsse.enableCBCProtection.
>>
>> For the javax.net.ssl.* code, we already have been using since very
>> early on:
>>
>>      ssl.*:  used by the javax.net.ssl.* code.  e.g.
>>              KeyManagerFactory, TrustManagerFactory,
>>              ServerSocketFactory.provider, SocketFactory.provider
>>
>> And then there are the other sun.security.ssl.* and javax.net.ssl.* for
>> SunJSSE providers that we expect other providers will use:
>>
>>      sun.security.ssl.*
>>              allowUnsafeRenegotiation, allowLegacyHelloMessages
>>
>>      javax.net.ssl.*
>>              keyStore, keyStorePassword, etc.
>>              trustStore, trustStorePassword, etc.
>>
>> And now we're adding yet another naming scheme?  If it weren't for:
>>
>>
>> http://docs.oracle.com/javase/7/docs/technotes/guides/security/jsse/JSSERefGuide.html#Customization
>>
>>
>> I'd be completely lost.
>>
>> Brad
>>
>>
>>> Now, two
>>> styles are acceptable. "jdk.*" is for JDK properties, and "jsse.*" is
>>> used when other parties also should follow it. "sun.security.ssl" is
>>> deprecated, I think.
>>>
>>>> Regarding rejectClientInitializedRenego, I think the word "Initiated"
>>>> more succinctly captures the intent of this property, rather than
>>>> "Initalized."  And as long as the word "Renegotiation" is, it should
>>>> probably be spelled out completely.  AFAIK, we rarely abbreviate
>>>> external names like this.
>>>>
>>>> Same "initiated" comment about the variable name in your codereview, but
>>>> I don't care much about Renogo.
>>>>
>>> I prefer "Initiated" to "Initalized".  Let's make the update in another
>>> update.
>>
>> Thanks for considering.
>>
>>>> ServerHandshaker.java
>>>> =====================
>>>> 283:  My initial thought was a no_renegotiation(100) warning, but that
>>>> allows the client to decide what to do, rather than the server
>>>> terminating.
>>>>
>>> No, we cannot.  First of all, warning message is not very useful because
>>> in general the sending party cannot know how the receiving party behave.
>>>    Secondly, it is the expected behavior to *reject" client initiated
>>> renegotiation. It is the server who should make the decision, but not
>>> the client.
>>>
>>>> This TLS logic decision is not straightforward, so this needs
>>>> commenting.
>>>>
>>> I think "reject client initialized renegotiation" has say all. ;-) I
>>> will add words about "state != HandshakeMessage.ht_hello_request".
>>>
>>>
>>>> 379:  since you're making changes here.
>>>>
>>>>     response->respond
>>>>
>>> OK.
>>>
>>>> Since you already have CCC approval and are ready to putback, I would
>>>> suggest putting back now, and let's file another CCC to change the name.
>>>>    That should be a no-brainer.
>>>>
>>> OK.
>>>
>>> Thanks,
>>> Xuelei
>>>
>>>> Thanks!
>>>>
>>>> Brad
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 5/29/2013 7:05 PM, Xuelei Fan wrote:
>>>>> Got it. Yes, this fix is addressing a different issue from you
>>>>> mentioned
>>>>> below.
>>>>>
>>>>> Thanks,
>>>>> Xuelei
>>>>>
>>>>> On 5/30/2013 9:53 AM, Bernd Eckenfels wrote:
>>>>>> Am 30.05.2013, 02:18 Uhr, schrieb Xuelei Fan <xuelei.fan at oracle.com>:
>>>>>>>> 2381456
>>>>>>> Would you mind send me the link of the bug, or the code review
>>>>>>> request
>>>>>>> mail?  I may miss some mails about this direction.
>>>>>>
>>>>>> I am afraid I cant sent the link, the Bug is in review state and
>>>>>> therefore not visible for me. It was acknowledged 2012-11-12, see
>>>>>> attached. I guess the link would be
>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=2381456 (not
>>>>>> sure if
>>>>>> the numbers are the same in the new bug tool).
>>>>>>
>>>>>>> Good suggestion.  Oracle provider of JSSE had addressed the TLS
>>>>>>> renegotiation issue in JDK 1.4.2 update 26, JDK 1.5.0 update 24
>>>>>>> and JDK
>>>>>>> 6u 19 around the end of 2009 and the beginning of 2010.  Here is the
>>>>>>> readme of the fix:
>>>>>>> http://www.oracle.com/technetwork/java/javase/documentation/tlsreadme2-176330.html.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Thats a different problem, I was thinking about preventing execessive
>>>>>> client initiated renegotiations. This is for example CVE-2011-1473
>>>>>> from
>>>>>> THC.
>>>>>>
>>>>>>>> You mentioned industry will move to a secure handshake - are you
>>>>>>>> aware of any initiative in that direction?
>>>>>>>>
>>>>>>> See http://www.rfc.org/rfc/rfc5746.txt.  As far as I know, nearly all
>>>>>>> major vendors of SSL protocols has support RFC5746.
>>>>>>
>>>>>> Ok, but thats a different issue. I was expecting 7188658 to address
>>>>>> another point, but I might be wrong.
>>>>>>
>>>>>> I understand that as of Oracle policy we cannot discuss it. Even if
>>>>>> this
>>>>>> is a very well known issue. :-/
>>>>>>
>>>>>> Greetings
>>>>>> Bernd
>>>>>
>>>
>