Code review request: 8016594: Native Windows ccache still reads DES tickets

Xuelei Fan Xuelei.Fan at Oracle.Com
Thu Aug 8 03:54:04 PDT 2013


Fine to go to me if you considered Dimitry's comments.

Xuelei

On Aug 8, 2013, at 18:09, Weijun Wang <weijun.wang at oracle.com> wrote:

> Can I consider the fix reviewed and it's fine?
> 
> Thanks
> Max
> 
> On 8/7/13 9:32 PM, Xuelei Fan wrote:
>> On 8/7/2013 9:30 PM, Weijun Wang wrote:
>>> First, thanks for your feedbacks.
>>> 
>>> I only intended to fix etypes in this bug and since I don't have a lot
>>> of experience on native kerberos on Mac (it is the Heimdal impl instead
>>> of MIT's) I didn't want to touch a lot.
>>> 
>>> Precisely, comparing only "krbtgt" is not enough. When doing cross-realm
>>> auth from R1 to R2, it's likely to have "krbtgt/R2 at R1" in ccache and it
>>> should not used as initial TGT.
>>> 
>>> Shall we fix this in another bug when I (or QE) are more familiar with
>>> native krb5 on Mac?
>> OK to me.
>> 
>> Xuelei
>> 
>>> Thanks
>>> Max
>>> 
>>> On 8/7/13 9:09 PM, Xuelei Fan wrote:
>>>> On 8/7/2013 7:53 PM, Dmitry Samersoff wrote:
>>>>> Xuelei,
>>>>> 
>>>>> 1. strncmp calls strlen at first, so explicit call to strlen is not
>>>>> necessary.
>>>> I was wondering to make the comparing when the length of serverName is
>>>> bigger than strlen("krbtgt").  For example, "krbtgt_extra".  Mine
>>>> suggested code is incorrect, as the output name of krb5_unparse_name may
>>>> be "krbtgt_extra/h.o.s.t at realm", but not "krbtgt_extra".
>>>> 
>>>> It's a little problem, but we might want to make the comparing more
>>>> precisely.
>>>> 
>>>>> 2. strlen("krbtgt") == sizeof("krbtgt")-1
>>>>> as sizeof count terminating 0.
>>>> You are right.
>>>> 
>>>> Xuelei
>>>> 
>>>>> -Dmitry
>>>>> 
>>>>> 
>>>>> On 2013-08-07 15:31, Xuelei Fan wrote:
>>>>>> On 8/7/2013 6:58 PM, Weijun Wang wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> On 8/7/13 5:23 PM, Dmitry Samersoff wrote:
>>>>>>>> Weijun,
>>>>>>>> 
>>>>>>>> nativeccache.c:
>>>>>>>> 
>>>>>>>> 322: Could you change strlen("krbtgt") to sizeof("krbtgt")-1 to
>>>>>>>> save a
>>>>>>>> bit of computer power?
>>>>>>> 
>>>>>>> Sure.
>>>>>> 
>>>>>> strncmp() is normally work with strlen() while comparing two
>>>>>> strings, in
>>>>>> case the length of the two string are not equal.
>>>>>> 
>>>>>> - 322  if (strncmp (serverName, "krbtgt", strlen("krbtgt")) == 0 &&
>>>>>> + 322  if (strlen(serverName) == sizeof("krbtgt") &&
>>>>>> +        strncmp (serverName, "krbtgt", sizeof("krbtgt")) == 0 &&
>>>>>> 
>>>>>> BTW, as it is a local function, would you like to add a "static"
>>>>>> keyword
>>>>>> to isIn() function?
>>>>>> 
>>>>>> Xuelei
>> 


More information about the security-dev mailing list