JDK-7184246 : Simplify Config.get() of krb5
  • Type: Enhancement
  • Component: security-libs
  • Sub-Component: java.security
  • Affected Version: 8
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2012-07-16
  • Updated: 2020-02-26
  • Resolved: 2012-10-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8 b64Fixed openjdk7uFixed
Related Reports
Relates :  
Relates :  
Description
This is about the internal class sun.security.krb5.Config.

If you want to get a value from inside krb5.conf, you can call getDefault(String). This might be good to get a value from the [libdefaults] section. However, the method was designed to be so smart that it can recursively search for key/value pairs no matter where and how deep it is.

For example, given a krb5.conf

   [s1]
   a=b

   [s2]
   c=d

   [s3]
   e = {
     f = g
   }

getDefault("a") = "b", getDefault("c") = "d", and astonishingly, getDefault("f") = "g".

I don't think this is a good design, for several reasons:

1. It depends on the order of sections if there are key/value pairs with the same key in different sections.

2. It ignores wrong settings. For example, when doing a cross-realm auth, the Realm.getRealmsList(from,to) is used to get a path which should be defined in [capaths]. However, the method simply crawls recursively into any subsection it found and won't notice the [capaths] being mistakenly typed as [capath]

3. It lacks certain features. Because the function always return a String (same with the getDefault(String,String) method), getDefault("e") can only return a null. Therefore there is no way to find out the existence of the subsection e unless we also know it contains a key f.

4. The current Config class needs to know what subsections contains more subsections, and it hardcodes names like [capaths] and [realms].

In short, it's just too smart and becomes unsafe to use. I suggest removing all this smartness and a user must use the full paths to get a value, say,

   kdc = config.get("realms", "SUN.COM", "kdc")

My proposed spec is:

1. The Config class should understand a krb5.conf without knowing any specific section names. All it maintains is a Value, which can be either of

    String
    List<Value>
    TreeMap<String,Value>

Here I use TreeMap to preserve the order (might not be necessary).

2. The basic retrieval method will be

    Value get(String... key)

3. There are simpler methods if you already know what the type in your case is

    String getAsString(String... key)
    List<String> getAsStringList(String... key)

The compatibility risk will be low, and if there really comes a compatibility issue, most likely it will be because the caller had written his krb5.conf wrong.

One of the advantages of the original design is that when a key is provided in both [libdefaults] and a given realm, the method can find it anyway. This will be useful for keys like kdc_timeout, max_retries. However, I think this automatic retrieval is confusing and error-prone, I'd rather manually call the get() method twice.

Comments
The current design is to support duplicate key-value pairs so that calling get(..) returns the last value and getAll(..) returns all values in a space-separated-string. This means the following two cases return the same result: kdc = kdc1 kdc = kdc2 and kdc = kdc1 kdc2 thus compatible with old calls. In fact, the old code only combines secondary values inside [realms] and [capaths], so if you call getAll() for these 2 cases and get() for all other cases, that's 100% the same result. Duplicate section names are not supported, and the last one is used. This is not the same with MIT krb5 where [libdefaults] are merged and first one chosen for the others. I do not intend to use the MIT behavior here.
22-10-2012