Hi Max,
On 08/05/2015 05:40 PM, Weijun Wang wrote:
Hi Artem
First, you shouldn't need any @modules here, the
sun/security/krb5/auto already contains a TEST.properties file
covering everything. For the same reason, you should place
KinitConfPlusProps.java somewhere inside auto. If you think a subdir
is better, put it into auto/tools.
I moved the tests to sun/security/krb5/auto
Back to the tests themselves:
I don't see how KrbTicket.java is related to the bug description.
There is no system properties mentioned.
Right, this test doesn't use any krb5-related system properties. It only
uses krb5 config file, and check if a krb ticket has appropriate state.
I didn't find any test that checks it (at least, explicitly). The bug
subject/description is confusing a little bit, I updated it.
KinitConfPlusProps.java does not seems to prove that both conf file
and system properties are used. Why should the 1st kinit call fail?
The test starts a local KDC instance on a random free port. Next,
hostname and port number are written to krb5 config file.
"java.security.krb5.kdc" specifies only a host without port. Since the
property is preferable, kinit tries to connect to a local KDC on default
port, and connection fails.
By the way, as far as I know, currently it is not possible to specify a
port number in "java.security.krb5.kdc".
http://hg.openjdk.java.net/jdk9/dev/jdk/file/66e2bdad38a8/src/java.security.jgss/share/classes/sun/security/krb5/Config.java
It may be better to make it possible. I am not sure that ":" symbol can
be used here because of compatibility risks. Currently it is used to
separate hostnames:
String tmp = getProperty("java.security.krb5.kdc");
if (tmp != null) {
// The user can specify a list of kdc hosts separated by ":"
defaultKDC = tmp.replace(':', ' ');
} else {
defaultKDC = null;
}
What do you think?
Doesn't the principal name also contain the realm here?
Yes, it does. I think it would be better to use a principal name without
realm here since krb config contain it. I updated the test.
The conf file only contains realm and kdc and nothing else. If both
conf file and system properties are provided, how do you prove the
conf file is also read and not ignored?
The test doesn't check it. I see the following ways to check it:
- Corrupt krb5 conf, and run kinit with it. I suppose it should fail.
- Add some extra parameters to krb5, run kinit, and then try to use
obtained data, and check that those extra parameters were used (I am not
sure about details right now, need to do some experiments)
What do you think?
Thanks for review. Please take a look at updated webrev
http://cr.openjdk.java.net/~asmotrak/8075299/webrev.01/
Artem
Thanks
Max
On 08/05/2015 07:01 PM, Artem Smotrakov wrote:
Hello,
Please review a couple of new tests which checks if krb5 settings are
read correctly from conf file and system properties.
Bug: https://bugs.openjdk.java.net/browse/JDK-8075299
Webrev: http://cr.openjdk.java.net/~asmotrak/8075299/webrev.00/
Artem
