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

Xuelei Fan xuelei.fan at oracle.com
Thu Jun 27 17:43:14 PDT 2013


Thanks!  But it may not apply to JDK-7188658.  It is a naming style
applies to new names in the future.

Xuelei

On 6/28/2013 8:23 AM, Brad Wetmore wrote:
> 
> 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
>>>>>>
>>>>
>>



More information about the security-dev mailing list