Re: RFR 8186884: Test native KDC, Java krb5 lib, and native krb5 lib in one test

[Artem Smotrakov]

Looks good to me.

Artem


On 09/11/2017 11:07 AM, Weijun Wang wrote:

Sorry to update again. Might be because jdk10 is frozen now.

    http://cr.openjdk.java.net/~weijun/8186884/webrev.03

But changes are real:

1. kdc.supported.enctypes supported by native KDCs correctly. In fact, I am now 
able to set it to aes256-sha2 (unsupported in Java yet) and test for interop 
between native libs with a native KDC.

2. A KDC::kinit method is introduced which calls native kinit when KDC is 
native. This makes the test above possible. Otherwise, Java will not be able to 
generate a aes256-sha2 ccache file.

Thanks
Max

On Sep 8, 2017, at 8:38 PM, Artem Smotrakov <artem.smotra...@oracle.com> wrote:

Hi Max,

Looks good to me. Below are a couple of minor comments you may want to address. 
No need a new webrev.

Thanks!

1. Proc.java, better to use braces

http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449

2. If you already updated KDC.java, it may be good to use try-with-resources 
for streams in a couple of places.

Artem


On 09/08/2017 06:19 AM, Weijun Wang wrote:

Small update on http://cr.openjdk.java.net/~weijun/8186884/webrev.02. All files 
belong to a single Proc now have the same prefix so they appear together in 
file list.

Thanks
Max

On Sep 7, 2017, at 8:39 PM, Weijun Wang <weijun.w...@oracle.com> wrote:

Updated at

  http://cr.openjdk.java.net/~weijun/8186884/webrev.01/

Now the libraries can be more freely combined, so you can test interop between 
one native library and another one:

   jtreg -Dnative.krb5.libs=j=,n=,m=lib1.so,h=lib2.so BasicProc.java


More comments inline below.

On Sep 7, 2017, at 3:29 PM, Artem Smotrakov <artem.smotra...@oracle.com> wrote:

Hi Max,

In general, looks fine to me. Below are a couple of comments you might want to 
address.

1. BasicProc.java, it might be better to use named constants for parameters for 
once() method. That would make it easier to understand what each particular 
onse() call does

I am passing in label and library names now.

2. BasicProc.java, could you please add an exception message?

+                if (!Arrays.equals(msg, msg2)) {
+                    throw new Exception();
+                }
+                break;

Fixed.

3. BasicProc.java, should the test do some cleanup then?

+            Files.copy(Paths.get("ccache.base"), Paths.get("ccache." + label));

Nowadays I prefer to let jtreg do the cleanup/retain. In fact, I am able to 
find a KDC.java bug by saving the ccache, where the incoming service ticket is 
invalid and not saved into the ccache.

Thanks
Max

Artem

On 09/07/2017 03:07 AM, Weijun Wang wrote:

Please take a review at

   http://cr.openjdk.java.net/~weijun/8186884/webrev.00/

BasicProc.java is enhanced to use a native JGSS provider, and KDC.java is 
enhanced to start (not use) a native KDC. For example, you would be able to 
test interop among Java JGSS, native JGSS (with MIT krb5) and Heimdal KDC with

    jtreg -Dnative.krb5.lib=/usr/local/krb5/lib/libgssapi_krb5.so \
          -Dnative.kdc.path=/usr/local/heimdal \
          test/sun/security/krb5/auto/BasicProc.java

Without those 2 new system properties, it behaves like before, i.e. Java GSS on 
the embedded KDC.

Another change in Context.java. Instead of using shared states to provide 
username and password when doing a krb5 login, a callback handler is used. This 
is considered more common. An extra permission is needed to read the default 
username (though I think this can coded as optional).

Thanks
Max