Code review request: 7184815 (was Re: OpenJDK krb5 ignore /etc/krb5.conf?)

[Weijun Wang]

On 08/09/2012 10:10 AM, Valerie (Yu-Ching) Peng wrote:
> Max,
>
> The new ordering for reading the config files sounds reasonable.
> However, I have questions on scenarios where the various config files
> don't exist and the corresponding handling.
> The new impl of getJavaFileName()/getNativeFileName() return file names
> even when the file doesn't exist.
> This will lead to an IOException when loadConfigFile(String) is called.
> Is this intended?

Yes. But this IOException will be ignored at the end of the private 
constructor. This is the also the old behavior. The main purpose is that 
even there is no (any kind of) krb5.conf there is still a chance to read 
config from DNS etc.

>
> Also, the toStringIndented(...) method removed several calls which
> append the 'prefix' and made various adjustments.
> Do you have sample output using the new impl? Reading through the code,
> some don't look quite right.

There are 2 major changes:

1. A string value does not starts from a newline
2. Vectors are inside square brackets.

For example, for a very typical krb5.conf

[libdefaults]
default_realm = R
[realms]
R = {
   kdc = k1
   kdc = k2
}

the old toString is

libdefaults = {
     default_realm = {
         R
     }
}
realms = {
     R = {
         kdc = {
                 k1
                 k2
         }
     }
}

and the new output is

{
     libdefaults = {
         default_realm = R
     }
     realms = {
         R = {
             kdc = [k1,k2,]
         }
     }
}

Thanks
Max


> Thanks,
> Valerie
>
> On 08/02/12 06:41, Weijun Wang wrote:
>> Ping again.
>>
>> On 07/18/2012 02:29 PM, Weijun Wang wrote:
>>> 7184815: [macosx] Need to read Kerberos config in files
>>>
>>> Please take a review:
>>>
>>>     http://cr.openjdk.java.net/~weijun/7184815/webrev.00/
>>>
>>> I break the config setting to Java setting and native setting, and
>>> insert the reading of SCDynamicStoreConfig between the two. This should
>>> preserve the 6u behavior and add a fallback to legacy config files.
>>>
>>> No new regression test, because of SCDynamicStoreConfig and system
>>> config files, will ask SQE to create a manual test.
>>>
>>> Thanks
>>> Max
>>>
>>>
>>> On 07/18/2012 08:26 AM, Weijun Wang wrote:
>>>> I'm not familiar with how Mac does it, but normally there are two
>>>> ways a
>>>> Kerberos authentication is performed, through the initial login and
>>>> through kinit. The former is integrated into the system (a pam module?)
>>>> and I guess in this case the config is inside SCDynamicStoreConfig. For
>>>> the latter, the Kerberos clients are regarded as standalone tools and a
>>>> /etc/krb5.conf is needed.
>>>>
>>>> Java works in both ways, if there is already a credentials cache it
>>>> will
>>>> happily use it. On the other hand, it also includes the Krb5LoginModule
>>>> that does all the login itself. Therefore, it should read both
>>>> styles of
>>>> config on a Mac.
>>>>
>>>> I've filed a new bug, It will appear soon at
>>>>
>>>>     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7184815
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>
>>>> On 07/17/2012 10:35 PM, Mike Swingler wrote:
>>>>> On Jul 16, 2012, at 8:32 PM, Weijun Wang <weijun.wang at oracle.com>
>>>>> wrote:
>>>>>
>>>>>> Ping again.
>>>>>>
>>>>>> On 07/05/2012 04:34 PM, Weijun Wang wrote:
>>>>>>> Hi Scott
>>>>>>>
>>>>>>> On Mac since Lion, sun.security.krb5.Config tries to locate the
>>>>>>> config
>>>>>>> info in this order:
>>>>>>>
>>>>>>> 1. java.security.krb5.conf system property
>>>>>>> 2. ${jre}/lib/security/krb5.conf
>>>>>>> 3. SCDynamicStoreConfig
>>>>>>>
>>>>>>> The main difference from other platforms is that it will not try
>>>>>>> config
>>>>>>> files, say, /Library/Preferences/edu.mit.Kerberos or /etc/krb5.conf.
>>>>>>>
>>>>>>> On the other hand, even /usr/bin/kinit comes with Lion reads the
>>>>>>> config
>>>>>>> file (if there is no SCDynamicStoreConfig setting).
>>>>>>>
>>>>>>> Is there a special reason for the current Java behavior? I do notice
>>>>>>> that the Apple 6u33 already does this.
>>>>>
>>>>> No special reason I can think of, beyond simply swapping the
>>>>> implementation to read from the SCDynamicStoreConfig. Java SE 6 had
>>>>> previously had been relying on the system to write out a
>>>>> /Library/Preferences/edu.mit.Kerberos file, but that went away with OS
>>>>> X 10.7, so we didn't see much point in reading the file, since little
>>>>> else on the system would be paying attention to it either for the
>>>>> purposes of SSO.
>>>>>
>>>>> It seems perfectly reasonable that if there are no
>>>>> SCDynamicStoreConfig entries, falling back to reading the legacy
>>>>> config files may be a valid option. I'm actually somewhat surprised
>>>>> that they are consulted by kinit.
>>>>>
>>>>> Regards,
>>>>> Mike Swingler
>>>>> Apple Inc.
>>>>>
>>>>
>>>
>