Code review request: 7195426: kdc_default_options not supported correctly

[Weijun Wang]

Hi Xuelei

The number is not equivalent to the ASN.1 bit string. It's more like a 
simple mapping to an unsigned 32 bit int. Here are some codes copied 
from MIT krb5:

krb5.h:

#define KDC_OPT_FORWARDABLE             0x40000000

get_in_tkt.c:

     if (options&KDC_OPT_FORWARDABLE)
         krb5_get_init_creds_opt_set_forwardable(opt, 1);
     else krb5_get_init_creds_opt_set_forwardable(opt, 0);

I also think 1<<(31-n) is more clear, but since the constants have been 
there for so many years, I believe they were defined for this very 
purpose and directly use them.

Thanks
Max


On 08/31/2012 05:51 PM, Xuelei Fan wrote:
> On 8/31/2012 3:08 PM, Weijun Wang wrote:
>> Please take a look at the change
>>
>>     http://cr.openjdk.java.net/~weijun/7195426/webrev.00
>>
> According to ASN.1 spec, "The leading bit of the bit string is
> identified by the "number" zero, ..." [X.680]
>
>   124     private static final int KDC_OPT_RENEWABLE_OK = 0x00000010;
> The position of renewable-ok is 27. I think the mask is 0x0000,0100.
>
>   125     private static final int KDC_OPT_FORWARDABLE = 0x40000000;
> The position of FORWARDED is 2. I think the mask is 0x2000,0000.
>
> Personally, I would like to use (1<<(31-n)) as the mask. It looks more
> straightforward.
>
> Xuelei
>
>>
>> It seems we confused the mask and the position.
>>
>> Thanks
>> Max
>>
>>
>>
>> -------- Original Message --------
>> 7195426: kdc_default_options not supported correctly
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7195426
>>
>>    Product: java
>>    Category: jgss
>>    Subcategory: krb5plugin
>>
>> === *Description*
>> ============================================================
>> kdc_default_options is a hex number for krb5.conf to define the
>> KDCOptions flags in a single integer where each bit of it represents one
>> of 32 flags.
>>
>> If you want to find out if the n-th flag is turn on, you should check for
>>
>>     kdc_default_options & (1<<(31-n))
>>
>> However, java currently checks for
>>
>>     kdc_default_options & n
>>
>