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

Weijun Wang at
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.


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.
>>> 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