Code review request, CR 7174244, NPE in Krb5ProxyImpl.getServerKeys()

Weijun Wang weijun.wang at oracle.com
Wed Jun 6 06:34:55 PDT 2012


Then the fix is fine. Please add the CR that causes this regression to 
the see also field.

-Max

On 06/06/2012 09:22 PM, Xuelei Fan wrote:
> On 6/6/2012 9:11 PM, Weijun Wang wrote:
>> Hi Xuelei
>>
>> I don't quite understand the bug report. Certainly it's good to prevent
>> that NPE, but why must the cipher suites be ordered? It seems it's not
>> ordered before the new HashMap.
> No, it was not ordered because of a regression.
>
>> Is this a regression or an RFE?
> It's a regression. We used to use TreeSet to sort the cipher suites. But
> later it is miss-updated to use ArrayList rather then TreeSet in a bug
> fix. Then the order was lost.
>
> We need to sort the cipher suites because the order is the preference
> when TLS handshaking selects a proper cipher suite. Please refer to TLS
> specification about the order, page 41, RFC5246:
>
>     cipher_suites
>        This is a list of the cryptographic options supported by the
>        client, with the client's first preference first.
>
>> You'd
>> better add some explanation in the bug report.
>>
> I will.
>
> Thanks,
> Xuelei
>
>> Thanks
>> Max
>>
>> On 06/06/2012 08:27 PM, Xuelei Fan wrote:
>>> Hi Weijun,
>>>
>>> Would you please review the fix for CR 7174244.
>>>       http://cr.openjdk.java.net./~xuelei/7174244/webrev.00/
>>>
>>> There are mainly two updates in the fix. One is to have
>>> Krb5ProxyImpl.getServerKeys()  to check null return value of
>>> Krb5Util.getServiceCreds(). The other one is to have cipher suite order
>>> to be independent from HashMap implementation.  TreeSet is used to
>>> replace ArrayList so that the cipher suites order are sorted
>>> automatically.
>>>
>>> Thanks,
>>> Xuelei
>



More information about the security-dev mailing list