RFR 8233222: Clarify system property usage in KerberosPrincipal instantiation

Weijun Wang weijun.wang at oracle.com
Fri Nov 22 01:27:54 UTC 2019


I rewrite the change into

+     * <p>Note that when this class or any other Kerberos-related class is
+     * initially loaded and initialized, it may read and cache the default
+     * realm from the Kerberos configuration file or via the
+     * java.security.krb5.realm system property (the value will be empty if
+     * no default realm is specified), such that any subsequent calls to set
+     * or change the default realm by setting the java.security.krb5.realm
+     * system property may be ignored.

I kept the "via" word because it's used in all other places. The "the value will be empty" description might be still not precise but IMO it should be clear.

I'll dup the same paragraph in the other constructor.

Here is the webrev:

   https://cr.openjdk.java.net/~weijun/8233222/webrev.00

Thanks,
Max

> On Nov 22, 2019, at 5:29 AM, Sean Mullan <sean.mullan at oracle.com> wrote:
> 
> On 11/14/19 10:41 PM, Weijun Wang wrote:
>> Please review the change below:
>> *diff --git a/src/java.security.jgss/share/classes/javax/security/auth/kerberos/KerberosPrincipal.java b/src/java.security.jgss/share/classes/javax/security/auth/kerberos/KerberosPrincipal.java*
>> *--- a/src/java.security.jgss/share/classes/javax/security/auth/kerberos/KerberosPrincipal.java*
>> *+++ b/src/java.security.jgss/share/classes/javax/security/auth/kerberos/KerberosPrincipal.java*
>> @@ -106,10 +106,19 @@
>>       *
>>       * <p>If the input name does not contain a realm, the default realm
>>       * is used. The default realm can be specified either in a Kerberos
>> -     * configuration file or via the java.security.krb5.realm
>> +     * configuration file or via the {@systemproperty java.security.krb5.realm}
>>       * system property. For more information, see the
>>       * {@extLink security_guide_jgss_tutorial Kerberos Requirements}.
>> -     * Additionally, if a security manager is
>> +     *
>> +     * <p>Please note that when this class or any other Kerberos-related class
>> +     * is initially loaded and initialized, it might read the default realm
>> +     * from the Kerberos configuration file or via the
>> +     * java.security.krb5.realm system property. The default realm is cached
>> +     * (even if there is none) and any calls to subsequently set or change
>> +     * the default realm by setting the java.security.krb5.realm system
>> +     * property might be ignored.
> 
> I would probably remove "Please" and use "may" instead of "might". I also think you can remove "via". The last sentence sounds like caching is a requirement, so maybe reword as:
> 
> "Note that when this class or any other Kerberos-related class is initially loaded and initialized, it may read and cache the default realm (even if there is none) from the Kerberos configuration file or the java.security.krb5.realm system property, such that any subsequent calls to set or change the default realm by setting the java.security.krb5.realm system property may be ignored."
> 
> A user may not know what you mean by "even if there is none" so I think you may need to explain that in more detail, perhaps in an additional sentence.
> 
>> +     *
>> +     * <p>Additionally, if a security manager is
>>       * installed, a {@link ServicePermission} must be granted and the service
>>       * principal of the permission must minimally be inside the
>>       * {@code KerberosPrincipal}'s realm. For example, if the result of
>> Here, the "Kerberos-related" class could be KeyTab as shown in this bug or something else like JAAS login configured with a Krb5LoginModule, or a JGSS call that touched the Kerberos 5 mechanism.
>> I used several "might" because this is just a hint but not a specified behavior and I don't want to restrict any evolution of the underlying implementation. For the same reason there is no test, although it's quite easy to trigger such a case. I've added a noreg-doc label.
>> Also, I assume there is no need for a CSR. This is not about compatibility or any new specification.
> 
> I would probably double-check with Joe. This could be considered a bit more than just a specification clarification.
> 
> Also, I assume you would make the same change to both of the KerberosPrincipal ctors?
> 
> Lastly, I think you should change the title of the bug to better reflect what fix you are proposing, maybe "Clarify KerberosPrincipal behavior when java.security.krb5 system property is set or changed."
> 
> --Sean




More information about the security-dev mailing list