Re: Code Review Request 6203816 and 6720456

2010-11-19 Thread Weijun Wang
I'm fine with all code changes. Thanks Max On 11/20/2010 08:00 AM, Valerie (Yu-Ching) Peng wrote: On 11/17/10 19:31, Weijun Wang wrote: On 11/18/2010 11:00 AM, Valerie (Yu-Ching) Peng wrote: Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w

hg: jdk7/tl/jdk: 7002036: ktab return code changes on a error case

2010-11-23 Thread weijun . wang
Changeset: de402590e18f Author:weijun Date: 2010-11-24 07:43 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/de402590e18f 7002036: ktab return code changes on a error case Reviewed-by: valeriep ! src/windows/classes/sun/security/krb5/internal/tools/Ktab.java +

code review request: 6894072: always refresh keytab

2010-12-01 Thread Weijun Wang
Hi Valerie The webrev is at -- http://cr.openjdk.java.net/~weijun/6894072/webrev.00/ Changes: 1. New javax..KeyTab, updated sun..KeyTab. As the impl note in javax..KeyTab says: the former is a name with dynamic content, the latter is a snapshot of a file. 2. Now Subject can have private

Fwd: CR 7004035 Updated, P4 java/classes_secu signed jar with only META-INF/* inside is not verifiable

2010-12-03 Thread Weijun Wang
Hi Sean Please review my code changes: http://cr.openjdk.java.net/~weijun/7004035/webrev.00/ After this change, MANIFEST.MF's getSigners() and getCertificates() will be not null. Since every signer of the jar file has a hash of the manifest header, I regard all of them as signers of

hg: jdk7/tl/jdk: 5 new changesets

2010-12-05 Thread weijun . wang
Changeset: b8713c88c060 Author:weijun Date: 2010-12-06 10:46 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/b8713c88c060 7004035: signed jar with only META-INF/* inside is not verifiable Reviewed-by: mullan ! src/share/classes/sun/security/tools/JarSigner.java !

Re: review request for 7008713: diamond conversion of kerberos5 and security tools

2010-12-22 Thread Weijun Wang
Looks fine. BTW, what are we supposed to review? Besides going through the patch and making sure each change is good, the only thing I can think of is looking for lines need coinification but untouched. Made some simple greps and found none yet. I can review the following files you listed

Re: review request for 7005608: diamond conversion of JCA and crypto providers

2010-12-22 Thread Weijun Wang
On 12/23/2010 10:17 AM, Brad Wetmore wrote: You need to update the Copyright updates on these files to include 2010. Not having a lot of experience yet with , the only ones I wasn't sure about were the ones in X509Factor.parseX509orPKCS7Cert. (line 415 425) I assume it just picks up the

Re: Code Review Request: CR 6976118, version number tolerance in the PreMasterSecret

2010-12-29 Thread Weijun Wang
Hi Xuelei Are you sure these 3 files all need to be changed? Hopefully you can change as few as possible. Also, the message name is not PreMasterSecret message. I know it should be ClientKeyExchange for RSAClientKeyExchange.java. and, tolerate is the verb, tolerant is an adjective. Thanks

Re: Code Review Request: CR 6976118, version number tolerance in the PreMasterSecret

2010-12-30 Thread Weijun Wang
On 12/30/2010 06:07 PM, Xuelei Fan wrote: On 12/30/2010 9:39 AM, Weijun Wang wrote: Hi Xuelei Are you sure these 3 files all need to be changed? Hopefully you can change as few as possible. Yes, we need to change all 3 files. As we discussed before, we'd better to check the version number

Re: review request for 7008713, update 1: diamond conversion of kerberos5 and security tools

2011-01-11 Thread Weijun Wang
only one letter. Thanks Max On 01/12/2011 11:04 AM, Stuart Marks wrote: Hi Max, thanks for reviewing the webrev. On 1/11/11 5:25 PM, Weijun Wang wrote: On 01/12/2011 08:06 AM, Stuart Marks wrote: I think there's one in a return statement in there too. You mean this one? public static A,B

Re: review request for 7011998: diamond conversion for jgss and pkcs11

2011-01-13 Thread Weijun Wang
If sunpkcs11.jar includes line number info (which I think yes), then it needs to be updated. Otherwise, line numbers shown in the exception stack info will not match the source code. Max On 01/14/2011 08:15 AM, Stuart Marks wrote: Yes, the byte codes are identical. I compiled with -g:none

code review request: 7012160: read SF file in signed jar in streaming mode

2011-01-14 Thread Weijun Wang
Hi Sean http://cr.openjdk.java.net/~weijun/7012160/webrev.00/ I've made changes to the following classes to enable streaming mode SF file reading: - java/util/jar/JarVerifier.java: 1. New verifyBlock method. 2. Change the constructor from JarVerifier(byte[]) to JarVerifier(byte[],

Re: Request for Comment: adding chain info to keytool -list

2011-01-17 Thread Weijun Wang
cert 1 alias + entity cert 2 alias Andrew On 1/17/2011 4:59 PM, Weijun Wang wrote: Hi All I have a keystore with a bunch of testing root CA, intermediate CA and entity certs, some PrivateKeyEntry and some TrustedCertEntry, and it's quite difficult to know who signs who. Therefore I

7016698: test sun/security/krb5/runNameEquals.sh failed on Ubuntu

2011-02-09 Thread Weijun Wang
Hi Valerie I just looked into to this bug, the reason is that the failed Ubuntu has a libgssapi_krb5.so.2 but no libgssapi_krb5.so. Turns out that a newly installed Ubuntu only has the GSS/krb5 runtime installed, which include the .so.2 file. On the other hand, the .so file (simply a

code review request: 7016698: test sun/security/krb5/runNameEquals.sh failed on Ubuntu

2011-02-10 Thread Weijun Wang
would be closer to your suggestion#2. Thanks, Valerie On 02/09/11 05:47 PM, Weijun Wang wrote: Hi Valerie I just looked into to this bug, the reason is that the failed Ubuntu has a libgssapi_krb5.so.2 but no libgssapi_krb5.so. Turns out that a newly installed Ubuntu only has the GSS/krb5 runtime

hg: jdk7/tl/jdk: 6742654: Code insertion/replacement attacks against signed jars; ...

2011-02-11 Thread weijun . wang
Changeset: 8860e17db3bd Author:weijun Date: 2011-02-12 05:09 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/8860e17db3bd 6742654: Code insertion/replacement attacks against signed jars 6911041: JCK api/signaturetest tests fails for Mixed Code PIT builds (b91) for all trains

hg: jdk7/tl/jdk: 7016698: test sun/security/krb5/runNameEquals.sh failed on Ubuntu

2011-02-11 Thread weijun . wang
Changeset: de923c0ec3c4 Author:weijun Date: 2011-02-12 07:30 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/de923c0ec3c4 7016698: test sun/security/krb5/runNameEquals.sh failed on Ubuntu Reviewed-by: valeriep !

Re: code review request: 7018928: test failure: sun/security/krb5/auto/SSL.java

2011-02-14 Thread Weijun Wang
On 02/15/2011 05:51 AM, Valerie (Yu-Ching) Peng wrote: So, the test failures are intermittent? I think so. The changes look fine. Thanks Max Valerie On 02/13/11 11:49 PM, Weijun Wang wrote: Hi Valerie http://cr.openjdk.java.net/~weijun/7018928/webrev.00/ There was a failure

hg: jdk7/tl/jdk: 7018928: test failure: sun/security/krb5/auto/SSL.java

2011-02-14 Thread weijun . wang
Changeset: 9024288330c4 Author:weijun Date: 2011-02-15 12:11 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/9024288330c4 7018928: test failure: sun/security/krb5/auto/SSL.java Reviewed-by: valeriep ! test/sun/security/krb5/auto/BadKdc1.java !

hg: jdk7/tl/jdk: 7020531: test: java/security/cert/CertificateFactory/openssl/OpenSSLCert.java file not closed after run

2011-03-01 Thread weijun . wang
Changeset: f8bf888edf20 Author:weijun Date: 2011-03-01 16:22 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/f8bf888edf20 7020531: test: java/security/cert/CertificateFactory/openssl/OpenSSLCert.java file not closed after run Reviewed-by: alanb, smarks !

Re: code review request: 7012160: read SF file in signed jar in streaming mode

2011-03-02 Thread Weijun Wang
think that matches the description of the method better. [138]: Same comment as above. I think setSignatureFile is a better name. Also can you add a comment describing what this method does? * JarVerifier [267]: this should be printed in debug only --Sean On 2/10/11 3:08 AM, Weijun Wang wrote

Re: code review request: 7012160: read SF file in signed jar in streaming mode

2011-03-04 Thread Weijun Wang
Small non-functional changes: http://cr.openjdk.java.net/~weijun/7012160/webrev.02/ 1. comments 2. some new language usage, say, operator 3. Vector-List, Hashtable-Map Thanks Max On 03/03/2011 08:32 AM, Weijun Wang wrote: Webrev updated http://cr.openjdk.java.net/~weijun/7012160/webrev

Re: Krb5LoginModule verify TGT?

2011-03-10 Thread Weijun Wang
Hi Christopher I'm not familiar with that function. So it reads the user's secret key from a keytab and try to decrypt the TGT to see if it can successfully get the session key inside? This is a part of the Krb5LoginModule login process: it receives a TGT from the KDC and use either the

Re: Krb5LoginModule verify TGT?

2011-03-10 Thread Weijun Wang
. -Christopher On Thu, Mar 10, 2011 at 6:36 PM, Weijun Wang weijun.w...@oracle.com mailto:weijun.w...@oracle.com wrote: Hi Christopher I'm not familiar with that function. So it reads the user's secret key from a keytab and try to decrypt the TGT to see if it can successfully get the session

hg: jdk7/tl/jdk: 6990848: JGSS/windows security code native code compiler warnings

2011-03-13 Thread weijun . wang
Changeset: d901560d70a7 Author:weijun Date: 2011-03-13 17:09 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/d901560d70a7 6990848: JGSS/windows security code native code compiler warnings Reviewed-by: valeriep ! src/windows/native/sun/security/krb5/NativeCreds.c

Re: code review request: 6894072: always refresh keytab

2011-03-20 Thread Weijun Wang
On 03/19/2011 07:54 AM, Valerie (Yu-Ching) Peng wrote: Max, Krb5AcceptCredential.java 1) you changed it to not extending KerberosKey, potential compatibility concern? Not compatibility concern. I only think that now Krb5AcceptCredential can be something else other than simply KerberosKey.

Re: code review request: 6894072: always refresh keytab

2011-03-23 Thread Weijun Wang
Hi Valerie Updated webrev: http://cr.openjdk.java.net/~weijun/6894072/webrev.02 Changes since last version: 1. A KerberosPrincipal inside javax..KeyTab class. New getInstance() arguments, new getPrincipal() method. It can only be non-null now, but I didn't say anything in the spec. I'm

hg: jdk7/tl/jdk: 7028490: better suggestion for jarsigner when TSA is not accessible

2011-03-23 Thread weijun . wang
Changeset: c43811a602a8 Author:weijun Date: 2011-03-23 18:26 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/c43811a602a8 7028490: better suggestion for jarsigner when TSA is not accessible Reviewed-by: mullan ! src/share/classes/sun/security/tools/JarSigner.java !

code review request: 7023056: NPE from sun.security.util.ManifestEntryVerifier.verify during Maven build

2011-03-24 Thread Weijun Wang
Hi Sean This is a regression made by my former treat-MANIFEST.MF-as-signed code change. Webrev here: http://cr.openjdk.java.net/~weijun/7023056/webrev.00/ For the reason, see the evaluation below. === *Description* Running a Maven build of

Re: Code Review Request for translatability bugs

2011-03-24 Thread Weijun Wang
AuthResources.java: === 1. {expected., expected }, -{.read.end.of.file, , read end of file}, +{expected.expect.read.end.of.file., +expected {0}, read end of file}, The expected. is now useless. At least I grep thru all jdk/src files and

Re: Code Review Request for translatability bugs

2011-03-24 Thread Weijun Wang
Another thing, maybe you can also combine the JarSigner TSA-unavailable warnings like you did for KeyTool integrity-not-checked ones? I've attached a diff and you can directly apply it at jdk level. Thanks Max On 03/25/2011 09:35 AM, Weijun Wang wrote: AuthResources.java

code review request: 7019384: Realm.getRealmsList returns realms list in wrong (reverse) order

2011-03-27 Thread Weijun Wang
Hi Xuelei We fixed an [capaths] bug some time ago: 6789935: cross-realm capath search error http://hg.openjdk.java.net/jdk7/tl/jdk/rev/33bc32405045 Unfortunately, it's still not correct. Here is a new webrev: http://cr.openjdk.java.net/~weijun/7019384/webrev.00/ As described in the

code review request: 7031536: test/sun/security/krb5/auto/HttpNegotiateServer.java should not use static ports

2011-03-28 Thread Weijun Wang
Hi Xuelei This webrev includes 2 kinds of code changes: http://cr.openjdk.java.net/~weijun/7031536/webrev.00/ 1. HttpNegotiateServer: now servers open on port 0. 2. Others: I add run/othervm for all tests in jgss and krb5 that call System.setProperty Thanks Max Original

hg: jdk7/tl/jdk: 2 new changesets

2011-03-28 Thread weijun . wang
Changeset: 86ace035d04d Author:weijun Date: 2011-03-28 18:04 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/86ace035d04d 7019384: Realm.getRealmsList returns realms list in wrong (reverse) order Reviewed-by: xuelei ! src/share/classes/sun/security/krb5/Realm.java !

Re: Exception while processing 'no-addresses' flag in KrbApReq.java

2011-03-28 Thread Weijun Wang
Sorry for the late reply. I suppose your client side program is not in Java? Because in JDK a service ticker's addresses field is always null. Thanks Max On 03/25/2011 07:53 PM, Szabolcs Pota wrote: [+ adding back security-dev] Hi Henry, Thank you for your reply. My answers are below.

Re: Code Review Request for translatability bugs

2011-03-28 Thread Weijun Wang
, Weijun Wang wrote: Another thing, maybe you can also combine the JarSigner TSA-unavailable warnings like you did for KeyTool integrity-not-checked ones? I've attached a diff and you can directly apply it at jdk level. Thanks Max On 03/25/2011 09:35 AM, Weijun Wang wrote: AuthResources.java

Re: Exception while processing 'no-addresses' flag in KrbApReq.java

2011-03-29 Thread Weijun Wang
] at sun.security.jgss.krb5.Krb5Context.acceptSecContext(Krb5Context.java:761) ~[na:na] Regards, Szabolcs On Mon, Mar 28, 2011 at 2:21 PM, Weijun Wang weijun.w...@oracle.com mailto:weijun.w...@oracle.com wrote: Sorry for the late reply. I suppose your client side program is not in Java

Re: Exception while processing 'no-addresses' flag in KrbApReq.java

2011-03-29 Thread Weijun Wang
.Krb5Context.acceptSecContext(Krb5Context.java:761) ~[na:na] Regards, Szabolcs On Mon, Mar 28, 2011 at 2:21 PM, Weijun Wang weijun.w...@oracle.com mailto:weijun.w...@oracle.com wrote: Sorry for the late reply. I suppose your client side program is not in Java? Because in JDK a service ticker's addresses field is always

Re: code review request: 6894072: always refresh keytab

2011-03-31 Thread Weijun Wang
. Valerie On 03/23/11 02:00 AM, Weijun Wang wrote: Hi Valerie Updated webrev: http://cr.openjdk.java.net/~weijun/6894072/webrev.02 Changes since last version: 1. A KerberosPrincipal inside javax..KeyTab class. New getInstance() arguments, new getPrincipal() method. It can only be non-null now

code review request: 7030180: AES 128/256 decrypt exception

2011-03-31 Thread Weijun Wang
Hi Valerie http://cr.openjdk.java.net/~weijun/7030180/webrev.00/ A bug in MIT krb5 1.8 triggers this exception (read evaluation below). They will fix it in 1.8.4 and 1.9. At the mean time, we can check both the session key and the subkey on the acceptor side. I think this does not deserve a

Re: code review request: 6894072: always refresh keytab

2011-04-02 Thread Weijun Wang
. Valerie On 04/01/11 02:14 AM, Weijun Wang wrote: Hi Valerie Updated again: http://cr.openjdk.java.net/~weijun/6894072/webrev.04/ 1. KeyTab can be used by anyone 2. The two compatibility support As for adding keys (from keytab) into private credentials set, I haven't cleaned up old ones. Since

Re: code review request: 6894072: always refresh keytab

2011-04-02 Thread Weijun Wang
On 04/02/2011 05:18 PM, Weijun Wang wrote: Updated again: http://cr.openjdk.java.net/~weijun/6894072/webrev.05/ Changes: 1. New Krb5Util.KeysFromKeyTab as a special kind of KerebrosKey we will add to and remove from private credentials set. Add and remove are only done when

Re: code review request: 6894072: always refresh keytab

2011-04-09 Thread Weijun Wang
it for you I'll remove lines 54-56. Maybe I meant to do that some time ago. Thanks Max I am still looking at the rest of changes, just want to send what I have now, so you don't wait too long. Thanks, Valerie On 04/02/11 02:18 AM, Weijun Wang wrote: Updated again: http

code review request: 7036157: TCP connection does not use kdc_timeout

2011-04-12 Thread Weijun Wang
Hi Valerie http://cr.openjdk.java.net/~weijun/7036157/webrev.00/ There is no regression test because it's not easy to simulate a connection timeout in a regression test. On my home machine, I set KDC to facebook.com. Thanks to the Great Firewall of China that all connections to facebook are

Re: Code review request for 7035115

2011-04-14 Thread Weijun Wang
Changes look fine. Are you going to backport the fix to earlier JDKs? Otherwise, you can simplify cons = clazz.getConstructor(new Class[] {String.class}); Object obj = cons.newInstance(new Object[] {configFile}); to cons = clazz.getConstructor(String.class);

hg: jdk7/tl/jdk: 6894072: always refresh keytab

2011-04-20 Thread weijun . wang
Changeset: f8956ba13b37 Author:weijun Date: 2011-04-20 18:41 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/f8956ba13b37 6894072: always refresh keytab Reviewed-by: valeriep ! src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java +

code review request: 6950929: Failures on Solaris sparc 64bit sun/security/krb5/auto/BadKdc4.java (and linux?)

2011-04-25 Thread Weijun Wang
Hi Valerie Code change for the BadKdc.java test: http://cr.openjdk.java.net/~weijun/6950929/webrev.00/ As described in the lines 44-69 of the new file, the test might fail due to 2 reasons: 1. KDC port opened by some other process (1%) 2. KDC cannot receive the first UDP packet (99%)

hg: jdk7/tl/jdk: 7032354: no-addresses should not be used on acceptor side

2011-04-26 Thread weijun . wang
Changeset: 06c7ee973e05 Author:weijun Date: 2011-04-07 08:51 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/06c7ee973e05 7032354: no-addresses should not be used on acceptor side Reviewed-by: valeriep ! src/share/classes/sun/security/krb5/KrbApReq.java !

Re: code review request: 6894072: always refresh keytab

2011-04-26 Thread Weijun Wang
and refresh them whenever a getKeys() is called. This should be harmless because we don't really use the keys if keytab objects (not keytab files) exist. I can do that. Thanks Max Valerie On 03/31/11 03:41 AM, Weijun Wang wrote: Hi Valerie Sorry for the late reply. I've considered some

hg: jdk7/tl/jdk: 2 new changesets

2011-04-26 Thread weijun . wang
Changeset: 4de90f390a48 Author:weijun Date: 2011-04-11 10:22 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/4de90f390a48 7012160: read SF file in signed jar in streaming mode Reviewed-by: mullan ! src/share/classes/java/util/jar/JarFile.java !

Re: code review request: 6894072: always refresh keytab

2011-04-26 Thread Weijun Wang
of Handshaker and ServerHandsshaker (mentioned in your other mail) Comments inline below: On 04/14/2011 09:45 AM, Valerie (Yu-Ching) Peng wrote: On 04/09/11 03:00 AM, Weijun Wang wrote: src/share/classes/sun/security/jgss/krb5/Krb5Util.java = 1) So, since when do we populate the Subject w

hg: jdk7/tl/jdk: 7036157: TCP connection does not use kdc_timeout

2011-04-26 Thread weijun . wang
Changeset: e9ae2178926a Author:weijun Date: 2011-04-14 12:40 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/e9ae2178926a 7036157: TCP connection does not use kdc_timeout Reviewed-by: valeriep ! src/share/classes/sun/security/krb5/internal/NetClient.java

hg: jdk7/tl/jdk: 6950929: Failures on Solaris sparc 64bit sun/security/krb5/auto/BadKdc4.java (and linux?)

2011-04-27 Thread weijun . wang
Changeset: 0e0db3421e8f Author:weijun Date: 2011-04-27 17:11 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/0e0db3421e8f 6950929: Failures on Solaris sparc 64bit sun/security/krb5/auto/BadKdc4.java (and linux?) Reviewed-by: xuelei ! test/sun/security/krb5/auto/BadKdc.java

code review request: 7040916: DynamicKeyTab test fails on Windows

2011-04-30 Thread Weijun Wang
Hi Andrew http://cr.openjdk.java.net/~weijun/7040916/webrev.00/ The keytab file cannot be removed and the test fails. The file was opened twice and both not closed: 1. once inside the test 2. once inside KeyTab and not closed because it's not a valid keytab Also, I change File.delete()

hg: jdk7/tl/jdk: 7040916: DynamicKeyTab test fails on Windows

2011-05-01 Thread weijun . wang
Changeset: aa7c361144bb Author:weijun Date: 2011-05-01 14:22 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/aa7c361144bb 7040916: DynamicKeyTab test fails on Windows Reviewed-by: xuelei ! src/share/classes/sun/security/krb5/internal/ktab/KeyTab.java !

hg: jdk7/tl/jdk: 7040151: SPNEGO GSS code does not parse tokens in accordance to RFC 2478

2011-05-02 Thread weijun . wang
Changeset: d08d77ad2d7b Author:weijun Date: 2011-05-03 02:48 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/d08d77ad2d7b 7040151: SPNEGO GSS code does not parse tokens in accordance to RFC 2478 Reviewed-by: valeriep !

hg: jdk7/tl/jdk: 7041635: GSSContextSpi.java copyright notice error

2011-05-09 Thread weijun . wang
Changeset: 9f56fbc8b6be Author:weijun Date: 2011-05-10 07:00 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/9f56fbc8b6be 7041635: GSSContextSpi.java copyright notice error Reviewed-by: valeriep ! src/share/classes/sun/security/jgss/spi/GSSContextSpi.java

Re: Code review request: 7043737: klist does not detect non-existing keytab

2011-05-12 Thread Weijun Wang
Valerie On 05/11/11 00:35, Weijun Wang wrote: Hi Valerie http://cr.openjdk.java.net/~weijun/7043737/webrev.00/ Not only a missing keytab is detected, but also an invalid one, where I use a similar error message like the native klist: $ klist -k ASSEMBLY_EXCEPTION Keytab name

hg: jdk8/tl/jdk: 7043737: klist does not detect non-existing keytab

2011-06-08 Thread weijun . wang
Changeset: 9b678733fa51 Author:weijun Date: 2011-06-08 14:01 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9b678733fa51 7043737: klist does not detect non-existing keytab Reviewed-by: valeriep ! src/windows/classes/sun/security/krb5/internal/tools/Klist.java +

code review request: NTLM SASL mech JCK conformance bugs

2011-06-13 Thread Weijun Wang
Hi Vinnie http://cr.openjdk.java.net/~weijun/7043847/webrev.00/ There are 5 bugs, you can see them in the webrev. This fix is for JDK 8. I guess I'll need =2 reviewers for backporting to JDK 7u2. Thanks Max

code review request: 7054428: test/java/security/SecureClassLoader/DefineClassByteBuffer.java error

2011-06-14 Thread Weijun Wang
Hi Alan The last excluded test in jdk_security1: http://cr.openjdk.java.net/~weijun/7054428/webrev.00/ I'm not an NIO expert, but the test looks too wrong. Is there any chance for it to pass in the last 8 years? Thanks Max Original Message *Change Request ID*: 7054428

Re: Code review request for 7041252 Use j.u.Objects.equals in security classes

2011-06-14 Thread Weijun Wang
Code changes look fine. Thanks Max On 06/15/2011 10:28 AM, Joe Darcy wrote: Hello. Please review this change to replace use of private two-argument equals methods with the platform Objects.equals method introduced in JDK 7: 7041252 Use j.u.Objects.equals in security classes

test/java/security/spec/EllipticCurveMatch.java othervm?

2011-06-15 Thread Weijun Wang
Hi Vinnie Why does this test run in /othervm mode? Thanks Max

Re: test/java/security/spec/EllipticCurveMatch.java othervm?

2011-06-15 Thread Weijun Wang
Oh, is it possible to use a non-Secure Random? Thanks Max On 06/15/2011 05:00 PM, Vincent Ryan wrote: On 06/15/11 09:45, Weijun Wang wrote: Hi Vinnie Why does this test run in /othervm mode? Thanks Max It was failing due to SecureRandom problems on some platforms when run in samevm mode

Re: code review request: 7054428: test/java/security/SecureClassLoader/DefineClassByteBuffer.java error

2011-06-15 Thread Weijun Wang
. The reason the original test failed in samevm is that buffers[DIRECT_BUFFER].flip() is not called and its remaining() is zero, and causes the error: java.lang.ClassFormatError: Truncated class file Thanks Max On 06/15/2011 06:56 PM, Alan Bateman wrote: Weijun Wang wrote: Without L104

Re: code review request: 7054428: test/java/security/SecureClassLoader/DefineClassByteBuffer.java error

2011-06-15 Thread Weijun Wang
? Thanks Max On 06/15/2011 07:26 PM, Alan Bateman wrote: Weijun Wang wrote: But the current test passes without closing the stream, even on Windows. I guess it's because the file opened is not in scratch directory and needs not be cleaned up. If we have to close the stream/channel, it seems

hg: jdk8/tl/jdk: 7054428: test/java/security/SecureClassLoader/DefineClassByteBuffer.java error

2011-06-20 Thread weijun . wang
Changeset: 82706552f7a3 Author:weijun Date: 2011-06-20 17:38 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/82706552f7a3 7054428: test/java/security/SecureClassLoader/DefineClassByteBuffer.java error Reviewed-by: alanb ! test/ProblemList.txt !

hg: jdk8/tl/jdk: 7054918: jdk_security1 test target cleanup

2011-06-20 Thread weijun . wang
Changeset: a015dda3bdc6 Author:weijun Date: 2011-06-20 19:17 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a015dda3bdc6 7054918: jdk_security1 test target cleanup Reviewed-by: alanb, smarks, vinnie ! test/ProblemList.txt ! test/java/security/BasicPermission/PermClass.java

Re: Fix for: 6415637: PKCS#12 key stores with empty passwords

2011-06-20 Thread Weijun Wang
Hi Florian Thanks for looking into this. The following bug is for this special purpose: 6879539: enable empty password support for pkcs12 keystore http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6879539 and it's now still in code review mode:

code review request: 6330275: Rework the PaddingTest regression test. (was Re: Fwd: jdk_security2 tests)

2011-06-23 Thread Weijun Wang
http://cr.openjdk.java.net/~weijun/6330275/webrev.00/ Thanks Max On 06/23/2011 08:03 AM, Brad Wetmore wrote: No, feel free to take it. Brad On 6/21/2011 2:24 AM, Weijun Wang wrote: Hi Brad # Timed out, Solaris 10 64bit sparcv9 com/sun/crypto/provider/Cipher/DES/PaddingTest.java generic

Re: code review request: 7061379: [Kerberos] Cross-realm authentication fails, due to nameType problemThe

2011-07-14 Thread Weijun Wang
Ping again. Thanks Max On 07/07/2011 09:10 AM, Weijun Wang wrote: Hi Valerie http://cr.openjdk.java.net/~weijun/7061379/webrev.00/ The bug report says the TGS-REQ asks for a KRB_NT_SRV_INST type whereas the kdc answers with a KRB_NT_PRINCIPAL type. Thus, equalsWithoutRealm function fails

hg: jdk8/tl/jdk: 6330275: Rework the PaddingTest regression test.

2011-07-21 Thread weijun . wang
Changeset: c8dbb9e19355 Author:weijun Date: 2011-07-22 10:25 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c8dbb9e19355 6330275: Rework the PaddingTest regression test. Reviewed-by: wetmore, smarks ! test/ProblemList.txt !

2nd round code review request: 7055363: jdk_security3 cleanup

2011-08-02 Thread Weijun Wang
Hi All http://cr.openjdk.java.net/~weijun/7055363/webrev.01/ *Attention*: please note that the jtreg mode for jdk_security3 is still RunOthervmBatch. Xuelei is now working on the JSSE tests. Once he is ready, he will change the mode to RunSamevmBatch. The code changes: * Still included in

Re: 2nd round code review request: 7055363: jdk_security3 cleanup

2011-08-02 Thread Weijun Wang
On 08/03/2011 12:43 AM, Alan Bateman wrote: Weijun Wang wrote: Hi All http://cr.openjdk.java.net/~weijun/7055363/webrev.01/ I went through the changes to the tests and they look okay to me. It's good to see so many tests removed from the problem list and the tests updated to work in samevm

Re: 2nd round code review request: 7055363: jdk_security3 cleanup

2011-08-03 Thread Weijun Wang
building, but not run the class.main(). I got the strange behavior when I run some tests with build tag with jtreg 4.1 b03. Did you come into the same issue? Thanks, Xuelei On 8/3/2011 11:50 AM, Weijun Wang wrote: On 08/03/2011 12:43 AM, Alan Bateman wrote: Weijun Wang wrote: Hi All http

Re: final inner class not final (was Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror)

2011-08-04 Thread Weijun Wang
. Here it is in _10. Attr(#25) { // InnerClasses [] { // InnerClasses #4 #0 #0 24; } } // end InnerClasses and here's _11: Attr(#25) { // InnerClasses [] { // InnerClasses #4 #0 #0 8; } } // end InnerClasses tom On Aug 3, 2011, at 7:24 PM, Weijun Wang wrote: serialVersionUID warnings for classes

Re: 2nd round code review request: 7055363: jdk_security3 cleanup

2011-08-05 Thread Weijun Wang
java.net.ssl.SSLContext SSLContext.setDefault(SSLContext context) SSLContext.getDefault() Thanks, Xuelei On 8/4/2011 6:03 PM, Weijun Wang wrote: I'll wait then. Plus I can try running the final result. -Max On 08/04/2011 06:01 PM, Xuelei Fan wrote: On 8/4/2011 5:58 PM, Weijun Wang wrote: On 08/04/2011

One new change (was Re: 2nd round code review request: 7055363: jdk_security3 cleanup)

2011-08-06 Thread Weijun Wang
On 08/04/2011 10:38 AM, Xuelei Fan wrote: On 8/4/2011 10:34 AM, Weijun Wang wrote: Oh, that's true. I'll add it back. Why did I remove it at the first place? You may not have to add it back, the library tag may help to build the dependent classes. Maybe, we have some incorrect understand

Re: One new change (was Re: 2nd round code review request: 7055363: jdk_security3 cleanup)

2011-08-07 Thread Weijun Wang
Correct. I'll restore it. Thanks Max On 08/08/2011 06:04 AM, Alan Bateman wrote: Weijun Wang wrote: : + Thread.currentThread().setContextClassLoader( + ResetConfigModule.class.getClassLoader()); Please confirm this is OK. Or, do you have a better solution? Will the thread context class

code review request: 7076415: sun/security/krb5/runNameEquals.sh failed on sles 10

2011-08-10 Thread Weijun Wang
Hi Valerie/Xuelei Webrev at http://cr.openjdk.java.net/~weijun/7076415/webrev.00/ The test fails when running the native JGSS provider on a sles (SUSE Linux Enterprise Server) machine throwing: Exception in thread main GSSException: Invalid name provided (Mechanism level: Hostname

Re: code review request: 7076415: sun/security/krb5/runNameEquals.sh failed on sles 10

2011-08-10 Thread Weijun Wang
On 08/10/2011 07:24 PM, Xuelei Fan wrote: Is it necessary to enable debug log in runNameEquals.sh? Not really, I will remove it. Thanks Max Otherwise, looks fine to me. Xuelei On 8/10/2011 6:00 PM, Weijun Wang wrote: Hi Valerie/Xuelei Webrev at http://cr.openjdk.java.net/~weijun

hg: jdk8/tl/jdk: 7076415: sun/security/krb5/runNameEquals.sh failed on sles 10

2011-08-11 Thread weijun . wang
Changeset: 4574445f35ac Author:weijun Date: 2011-08-12 11:20 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4574445f35ac 7076415: sun/security/krb5/runNameEquals.sh failed on sles 10 Reviewed-by: xuelei ! test/sun/security/krb5/Krb5NameEquals.java

hg: jdk8/tl/jdk: 7055363: jdk_security3 test target cleanup

2011-08-11 Thread weijun . wang
Changeset: cb83fe13af98 Author:weijun Date: 2011-08-12 12:26 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cb83fe13af98 7055363: jdk_security3 test target cleanup Reviewed-by: alanb, xuelei ! test/Makefile ! test/ProblemList.txt !

hg: jdk8/tl/jdk: 7078355: sun/net/www/protocol/file/DirPermissionDenied.sh leaves garbage on some linux systems

2011-08-12 Thread weijun . wang
Changeset: e533c13df9ad Author:weijun Date: 2011-08-12 21:04 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e533c13df9ad 7078355: sun/net/www/protocol/file/DirPermissionDenied.sh leaves garbage on some linux systems Reviewed-by: chegar !

Re: Fwd: Re: Code review request: 7063647, jsse/runtime, To use synchronized map in key manager

2011-08-14 Thread Weijun Wang
a yes for the code review ] -Max On 08/15/2011 11:09 AM, Xuelei Fan wrote: On 8/15/2011 11:02 AM, Xuelei Fan wrote: On 8/15/2011 10:35 AM, Weijun Wang wrote: I'm not sure what action on serverAliasCache you want to protect. For example, 260 aliases = serverAliasCache.get

Re: Code review request: 7064075 Security libraries don't build with javac -Xlint:all, -deprecation -Werror

2011-08-16 Thread Weijun Wang
My 2 cents: @SuppressWarnings(serial) is never a good idea. There will still be a serialver automatically computed and you won't be able to control it. Will the code changes go to jdk8 only? If so, I guess you should use the value calculated from jdk7 codes. If the values on jdk6 and jdk7

Re: 2nd round code review request: 7055363: jdk_security3 cleanup

2011-08-16 Thread Weijun Wang
I'll wait then. Plus I can try running the final result. -Max On 08/04/2011 06:01 PM, Xuelei Fan wrote: On 8/4/2011 5:58 PM, Weijun Wang wrote: On 08/04/2011 05:50 PM, Xuelei Fan wrote: Good point. I will update the test files. Even we have to make update to JSSE tests, I would evaluate

Re: code review request: 7081817, java/classes_secu, test/sun/security/provider/certpath/X509CertPath/IllegalCertiticates.java failing

2011-08-22 Thread Weijun Wang
On 08/22/2011 07:31 PM, Alan Bateman wrote: Xuelei Fan wrote: Simple fix, no webrev. % hg diff src/share/classes/sun/security/provider/certpath/X509CertPath.java --- a/src/share/classes/sun/security/provider/certpath/X509CertPath.java +++

code review request: 7083576 (was Re: no javax/xml/crypto jprt test targets in jdk/test/Makefile)

2011-08-25 Thread Weijun Wang
[ Add security-dev@openjdk.java.net to CC] I think so. javax/xml/crypto is public API and it should go to jdk_security2 (along with javax/crypto). Of course, that is still not included in the default run. I've filed a bug: 7083576: add javax/xml/crypto into jdk_security2 test rule and

Re: code review request: 7083576 (was Re: no javax/xml/crypto jprt test targets in jdk/test/Makefile)

2011-08-26 Thread Weijun Wang
PM, Weijun Wang wrote: [ Add security-dev@openjdk.java.net to CC] I think so. javax/xml/crypto is public API and it should go to jdk_security2 (along with javax/crypto). Of course, that is still not included in the default run. I've filed a bug: 7083576: add javax/xml/crypto

Re: Code review request: 7059542 JNDI name operations should be locale independent

2011-08-29 Thread Weijun Wang
The code changes look fine, and thanks for including the sun/security/{tools,krb5} codes. -Max On 08/28/2011 06:36 PM, Xuelei Fan wrote: Hi Weijun, Would you please review the new update? I also include the changes in security components. webrev: webrev:

hg: jdk8/tl/jdk: 7083576: add javax/xml/crypto into jdk_security2

2011-08-29 Thread weijun . wang
Changeset: 6d6d75421e8a Author:weijun Date: 2011-08-30 10:46 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6d6d75421e8a 7083576: add javax/xml/crypto into jdk_security2 Reviewed-by: mullan ! test/Makefile ! test/javax/xml/crypto/dsig/SecurityManager/XMLDSigWithSecMgr.java

code review request: 7081783: jarsigner error when no $HOME/.keystore

2011-08-29 Thread Weijun Wang
Hi All 7081783: jarsigner error when no $HOME/.keystore Webrev is at -- http://cr.openjdk.java.net/~weijun/7081783/webrev.00/ Description: jarsigner includes a certpath validation check, and shows a warning when the check fails. The CertPathValidator object, unfortunately, is initialized

code review request: 7083664: test hard code of using c:/temp but this dir might not exist

2011-08-30 Thread Weijun Wang
Hi All 7083664: test hard code of using c:/temp but this dir might not exist Webrev is at -- http://cr.openjdk.java.net/~weijun/7083664/webrev.00/ Some of our regression tests set TMP variables on different platforms, and on Windows, it's c:\temp. Unfortunately one of the test machines

code review request: 7081411: Change more keytool -genkeypair to RSA

2011-08-30 Thread Weijun Wang
Hi All 7081411: Change more keytool -genkeypair to RSA http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7081411 Webrev at http://cr.openjdk.java.net/~weijun/7081411/webrev.01/ Because of the Solaris DSA bug described in 7041639, we keep seeing tests generating DSA key pairs failing.

Re: code review request: 7081411: Change more keytool -genkeypair to RSA

2011-08-31 Thread Weijun Wang
to find the potential problems, right? When we change the test to be able to passed on all platform, the test may lost it function to find potential issues partially. Just my very personal view. Thanks, Xuelei On 8/30/2011 8:26 PM, Weijun Wang wrote: Hi All 7081411: Change more keytool

Re: code review request: 7081783: jarsigner error when no $HOME/.keystore

2011-08-31 Thread Weijun Wang
On 08/31/2011 09:17 PM, Xuelei Fan wrote: I understand the code, fine to me. But the loadKeyStore() method looks really ugly and lazy. :-) It's ugly, but not very lazy. Anyway, I'm going to putback this version since you already said fine. Just for your reference in the inline comments.

Re: Code review request for 7049079: NTSYSTEM CLASS IS LEAKING 3 WINDOWS TOKENS

2011-09-02 Thread Weijun Wang
The code change looks fine. Thanks Max On 09/02/2011 11:15 PM, Seán Coffey wrote: This is a forward port to JDK 8 of a fix gone into jdk 5 and jdk6 releases. It'll be ported to 7u shortly after jdk8. Bug id is not public. NTSystem() obtains a native Token instance for each constructor call.

hg: jdk8/tl/jdk: 7081783: jarsigner error when no $HOME/.keystore

2011-09-04 Thread weijun . wang
Changeset: 62c25e4c30a3 Author:weijun Date: 2011-09-05 11:22 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/62c25e4c30a3 7081783: jarsigner error when no $HOME/.keystore Reviewed-by: xuelei ! src/share/classes/sun/security/tools/JarSigner.java

hg: jdk8/tl/jdk: 7081411: DSA keypair generation affected by Solaris bug

2011-09-05 Thread weijun . wang
Changeset: 1d247911e035 Author:weijun Date: 2011-09-05 18:17 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1d247911e035 7081411: DSA keypair generation affected by Solaris bug Reviewed-by: xuelei, mullan, alanb ! test/ProblemList.txt +

<    1   2   3   4   5   6   7   8   9   10   >