hg: jdk8/tl/jdk: 8009636: JARSigner including TimeStamp PolicyID (TSAPolicyID) as defined in RFC3161

2013-04-19 Thread weijun . wang
Changeset: 778b16225d85 Author:weijun Date: 2013-04-19 15:41 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/778b16225d85 8009636: JARSigner including TimeStamp PolicyID (TSAPolicyID) as defined in RFC3161 Reviewed-by: mullan !

Re: Why cannot overwrite a KeyEntry with a TrustCertEntry?

2013-04-21 Thread Weijun Wang
On 4/11/2013 7:47 AM, Sean Mullan wrote: On 04/11/2013 04:36 AM, Weijun Wang wrote: Hi All The KeyStore::setCertificateEntry has * @exception KeyStoreException if the keystore has not been initialized

Re: Code review request: 8005527: [TEST_BUG] Failed Automatically with exit code 1.

2013-04-21 Thread Weijun Wang
Good. It's now kept there. Thanks Max On 4/22/13 11:35 AM, Xuelei Fan wrote: Looks fine to me. A minor comment, would you like to reserve the run tag? It would be help to know how to run the shell script manually. Xuelei On 4/15/2013 10:30 AM, Weijun Wang wrote: Hi All Please take a look

hg: jdk8/tl/jdk: 8005527: [TEST_BUG] console.sh failed Automatically with exit code 1.

2013-04-21 Thread weijun . wang
Changeset: 22a27dfd0510 Author:weijun Date: 2013-04-22 11:39 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/22a27dfd0510 8005527: [TEST_BUG] console.sh failed Automatically with exit code 1. Reviewed-by: xuelei ! test/sun/security/tools/keytool/console.sh

Re: Code Review Request for 7196009

2013-04-22 Thread Weijun Wang
No more comment. Thanks Max On 4/23/13 7:28 AM, Valerie (Yu-Ching) Peng wrote: Yes, I think the original impl which uses parseLine() to process the path values is incorrect since whatever specified should be preserved as a whole. Currently, the parseLine() is only used for parsing

Code review request: 8012615: Realm.getRealmsList returns realms list in wrong

2013-04-24 Thread Weijun Wang
Hi All Please review the fix at http://cr.openjdk.java.net/~weijun/8012615/webrev.00/ Basically, the algorithm to calculate cross-realm authentication path is totally rewritten to be *correct*. This includes both the hierarchy codes and capaths codes. You are welcomed to read this blog

Code review request: 7025699: Policy Tool is not accessible by keyboard

2013-04-24 Thread Weijun Wang
Hi All Please take a look at http://cr.openjdk.java.net/~weijun/7025699/webrev.00/ The bug is about policytool not accessible through a keyboard. Mainly two problems: First menu items have no key accelerators. Second, pressing ENTER on any button is not working. Basically I made these

Re: Code review request: 8012082: SASL auth-conf negotiated, but unencrypted data is accepted, reset to unencrypt

2013-05-01 Thread Weijun Wang
Ping again. On 4/19/13 8:56 AM, Weijun Wang wrote: Resubmitted at http://cr.openjdk.java.net/~weijun/8012082/webrev.01/. Now, when unwrap is called, it does *not* check if the message received matches the QoP. So when auth-conf is negotiated, one side can send an unencrypted token

hg: jdk8/tl/jdk: 8012082: SASL: auth-conf negotiated, but unencrypted data is accepted, reset to unencrypt

2013-05-01 Thread weijun . wang
Changeset: ae4a82e69da2 Author:weijun Date: 2013-05-01 21:05 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ae4a82e69da2 8012082: SASL: auth-conf negotiated, but unencrypted data is accepted, reset to unencrypt Reviewed-by: vinnie !

hg: jdk8/tl/jdk: 8013855: DigestMD5Client has not checked RealmChoiceCallback value

2013-05-02 Thread weijun . wang
Changeset: 81be41c7323f Author:weijun Date: 2013-05-03 10:43 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/81be41c7323f 8013855: DigestMD5Client has not checked RealmChoiceCallback value Reviewed-by: xuelei, mullan !

Re: [8] Code review request for 8005922: TEST_BUG: There is no /tmp directory for windows system.

2013-05-03 Thread Weijun Wang
Hi Jason One question: why is this necessary in i18n.html and ChangeUI.html: -ol +ol start=0 Neither test mentions step #. Otherwise, the changes look fine. Thanks Max On 5/3/13 1:44 PM, Jason Uh wrote: Could I please get a review for this fix for 8005922: Edits to the manual test

Re: [8] Code review request for 8005922: TEST_BUG: There is no /tmp directory for windows system.

2013-05-03 Thread Weijun Wang
On 5/3/13 5:52 PM, Jason Uh wrote: Hi Max, On 05/03/2013 12:33 AM, Weijun Wang wrote: Hi Jason One question: why is this necessary in i18n.html and ChangeUI.html: -ol +ol start=0 Neither test mentions step #. Using a zero-based list was just a stylistic choice, especially as Step 0

Re: Code review request: 8010192: Enable native JGSS provider on Mac

2013-05-03 Thread Weijun Wang
to make native provider work. Thanks Max Otherwise, changes look ok to me. Valerie On 03/18/13 05:38, Erik Joelsson wrote: The build changes look ok. /Erik On 2013-03-18 12:00, Weijun Wang wrote: Please take a look at http://cr.openjdk.java.net/~weijun/8010192/webrev.00/ The Mac native

Code review request: 8012679: Let allow_weak_crypto default to false

2013-05-06 Thread Weijun Wang
Webrev at http://cr.openjdk.java.net/~weijun/8012679/webrev.00/ Thanks Max

hg: jdk8/tl/jdk: 8010192: Enable native JGSS provider on Mac

2013-05-06 Thread weijun . wang
Changeset: 814dcc08df52 Author:weijun Date: 2013-05-07 12:30 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/814dcc08df52 8010192: Enable native JGSS provider on Mac Reviewed-by: valeriep ! make/sun/security/Makefile ! makefiles/CompileNativeLibraries.gmk !

Re: Code review request, JDK-8005535: SSLSessionImpl should have protected finalize()

2013-05-09 Thread Weijun Wang
There is no need to add final there. -Max On 5/9/13 10:39 PM, Xuelei Fan wrote: Hi Max, Please review the simple fix to properly override the finalize() method. http://cr.openjdk.java.net/~xuelei/8005535/webrev.00/ Trivial update, no new regression test. Thanks, Xuelei

doPrivileged with lambda

2013-05-10 Thread Weijun Wang
So we have been writing x = AccessController.doPrivileged( new PrivilegedActionObject() { public Object run() { return some(); } } ); Using jdk8 lambda, it seems we can write x =

RFR: 8012261: update policytool to support java.net.HttpURLPermission

2013-05-16 Thread Weijun Wang
Hi All Please take a look at http://cr.openjdk.java.net/~weijun/8012261/webrev.00/ which supports the new HttpURLPermission type introduced in 8010464: Evolve java networking same origin policy http://hg.openjdk.java.net/jdk8/tl/jdk/rev/93a268759ec3 Noreg-trivial. Thanks Max

Re: RFR: 8012261: update policytool to support java.net.HttpURLPermission

2013-05-17 Thread Weijun Wang
? Regards, Christophe. Weijun Wang mailto:weijun.w...@oracle.com May 16, 2013 7:17 PM Hi All Please take a look at http://cr.openjdk.java.net/~weijun/8012261/webrev.00/ which supports the new HttpURLPermission type introduced in 8010464: Evolve java networking same origin policy http

hg: jdk8/tl/jdk: 8012261: update policytool to support java.net.HttpURLPermission

2013-05-17 Thread weijun . wang
Changeset: 0f7aaabed25f Author:weijun Date: 2013-05-18 10:15 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0f7aaabed25f 8012261: update policytool to support java.net.HttpURLPermission Reviewed-by: mullan ! src/share/classes/sun/security/tools/policytool/PolicyTool.java !

Code review request: 8014310: JAAS/Krb5LoginModule using des encytypes failure with NPE after JDK-8012679

2013-05-26 Thread Weijun Wang
Please review the code changes at http://cr.openjdk.java.net/~weijun/8014310/webrev.00/ The reason is that since we set allow_weak_crypto to false, if the user only had DES keys or only has DES-related etypes enabled, there will be no working etype at all. Soon or later, an NPE is thrown.

Code review request: 8001326: Improve Kerberos replay caching

2013-05-28 Thread Weijun Wang
Please review the code changes at http://cr.openjdk.java.net/~weijun/8001326/webrev.00/ Two new system properties are introduced. sun.security.krb5.rcache to control what rcache type should be used. Besides the original one (which does not need this system property to be set), we support

Code review request: 8015274 and 8015276: policytool manual test problems

2013-05-30 Thread Weijun Wang
Please review the fix at http://cr.openjdk.java.net/~weijun/8015274/webrev.00/ It's for two bugs: 8015274: TEST_BUG: Step2: After selecting 'View Warning Log', it is empty instead of FileNotFound. 8015276: TEST_BUG: The 'ptool.test' can't be saved in the 'tmp' folder. Thanks Max

Comparing name-type in KerberosPrincipal

2013-06-06 Thread Weijun Wang
Hi All Recently we fixed a bug in OpenJDK: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7061379 http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e68db408d08c Here name-type equality is not checked anymore in the PrincipalName::equals() method. Since RFC 4120 6.2 says ... The

Re: Code review request: 8014310: JAAS/Krb5LoginModule using des encytypes failure with NPE after JDK-8012679

2013-06-07 Thread Weijun Wang
Ding dong. On 5/27/13 10:06 AM, Weijun Wang wrote: Please review the code changes at http://cr.openjdk.java.net/~weijun/8014310/webrev.00/ The reason is that since we set allow_weak_crypto to false, if the user only had DES keys or only has DES-related etypes enabled

A fundamental JGSS-API bug?

2013-06-07 Thread Weijun Wang
Hi All The GSSContext::initSecContext() method could either return a token (possibly null if no more token is needed) when the call succeeds or throw a GSSException if there is a failure, but not *both*. The same applies to acceptSecContext(). However, according to RFC 2743 2.2.1 [1], the C

Re: A fundamental JGSS-API bug?

2013-06-07 Thread Weijun Wang
On 6/7/13 11:49 PM, Sean Mullan wrote: try { send(initSecContext(inToken)); } catch (GSSException e) { if (e.getResidue() != null) { send(e.getResidue()); } throw e; } That doesn't seem too

Re: A fundamental JGSS-API bug?

2013-06-08 Thread Weijun Wang
On 6/8/13 12:20 PM, Xuelei Fan wrote: The recommended way was while (!context.isEstablished()) { context.initSecContext(is, os); os.flush(); } When I say easier, I mean it looks like there is no need to make any application change and this method can just write KRB_ERROR

Re: Code review request: 8015274, 8015276 and 8016158: policytool manual test problems

2013-06-10 Thread Weijun Wang
changes in i18n.html, keystore URLs, some warning now does not show. I personally run i18n.sh on Mac and Windows. Every step looks fine now. Thanks Max On 5/31/13 9:41 AM, Weijun Wang wrote: Please review the fix at http://cr.openjdk.java.net/~weijun/8015274/webrev.00/ It's for two bugs

Re: Code review request: 8015274, 8015276 and 8016158: policytool manual test problems

2013-06-11 Thread Weijun Wang
Mullan wrote: Why did you need to check for a $HOME/.java.policy file in i18n.sh? Can you explain that? Otherwise, changes look good to me. --Sean On 06/08/2013 09:32 PM, Weijun Wang wrote: Updated at http://cr.openjdk.java.net/~weijun/8015274/webrev.01/ A new bug 8016158 is added

Re: Code Review Request for 7196805: DH Key interoperability testing between SunJCE and JsafeJCE not successful

2013-06-12 Thread Weijun Wang
Hi Valerie Not sure if we really need to fix the equals() method here. Is there a spec saying the equality is based on internal fields instead of encoded bytes? I am saying this because it's not easy to get equals() correct here. In DHPrivateKey.java, the new equals() requires the other

hg: jdk8/tl/jdk: 2 new changesets

2013-06-12 Thread weijun . wang
Changeset: 021fdd093cd9 Author:weijun Date: 2013-06-13 09:59 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/021fdd093cd9 8014310: JAAS/Krb5LoginModule using des encytypes failure with NPE after JDK-8012679 Reviewed-by: valeriep !

Code review request: 6727821: javax.security.auth.login.Configuration keeps reference to context ClassLoader

2013-06-13 Thread Weijun Wang
Hi All Please review this code change http://cr.openjdk.java.net/~weijun/6727821/webrev.00/ Storing a thread-related variable into a static field is not sensible. A similar problem in javax.security.auth.Policy, but the class is already @Deprecated. If you believe it had better be fixed,

Re: Code review request, 7188658 Add possibility to disable client initiated renegotiation

2013-06-13 Thread Weijun Wang
I see. The code change looks fine then. Thanks Max On 6/14/13 9:45 AM, Xuelei Fan wrote: On 6/14/2013 9:39 AM, Weijun Wang wrote: What is this for? state != HandshakeMessage.ht_hello_request It is to allow server initialized renegotiation. If server want a renegotiation, it may send

hg: jdk8/tl/jdk: 8001326: Improve Kerberos caching

2013-06-21 Thread weijun . wang
Changeset: 4503e04141f7 Author:weijun Date: 2013-06-21 18:26 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4503e04141f7 8001326: Improve Kerberos caching Reviewed-by: valeriep ! src/share/classes/sun/security/jgss/krb5/AcceptSecContextToken.java !

Re: Code review request: 8017453: ReplayCache tests fail on multiple platforms (was Re: Please be prepared for possible test failures (was Fwd: [JBS] (JDK-8001326) Improve Kerberos replay caching))

2013-06-23 Thread Weijun Wang
the system property os.name should always have a value, it makes me comfort not thinking of possible NPE. Thanks Max Xuelei On 6/24/2013 12:36 AM, Wang Weijun wrote: Send again. And BTW, JPRT runs fine. 在 Jun 23, 2013,6:29 PM,Weijun Wang weijun.w...@oracle.com 写道: The macosx problem found

Re: Code review request: 8017453: ReplayCache tests fail on multiple platforms (was Re: Please be prepared for possible test failures (was Fwd: [JBS] (JDK-8001326) Improve Kerberos replay caching))

2013-06-23 Thread Weijun Wang
How about if (!os.startsWith(Solaris) || !os.startsWith(Linux)) { mode = -1; } Thanks Max On 6/24/13 10:35 AM, Xuelei Fan wrote: On 6/24/2013 10:01 AM, Weijun Wang wrote: On 6/24/13 9:49 AM, Xuelei Fan wrote: ReplayCacheTestProc.java

Re: Code review request: 8017453: ReplayCache tests fail on multiple platforms (was Re: Please be prepared for possible test failures (was Fwd: [JBS] (JDK-8001326) Improve Kerberos replay caching))

2013-06-23 Thread Weijun Wang
Oh, no if (!os.startsWith(Solaris) !os.startsWith(Linux)) { --Max On 6/24/13 11:01 AM, Weijun Wang wrote: How about if (!os.startsWith(Solaris) || !os.startsWith(Linux)) { mode = -1; } Thanks Max On 6/24/13 10:35 AM, Xuelei Fan wrote

Re: Code review request: 8017453: ReplayCache tests fail on multiple platforms (was Re: Please be prepared for possible test failures (was Fwd: [JBS] (JDK-8001326) Improve Kerberos replay caching))

2013-06-23 Thread Weijun Wang
Oh, actually it should be SunOS. On 6/24/13 11:28 AM, Xuelei Fan wrote: On 6/24/2013 11:04 AM, Weijun Wang wrote: Oh, no if (!os.startsWith(Solaris) !os.startsWith(Linux)) { Good. Xuelei --Max On 6/24/13 11:01 AM, Weijun Wang wrote: How about if (!os.startsWith

Re: Code review request: 8017453: ReplayCache tests fail on multiple platforms (was Re: Please be prepared for possible test failures (was Fwd: [JBS] (JDK-8001326) Improve Kerberos replay caching))

2013-06-23 Thread Weijun Wang
I would stick to SunOS since that's how the JGSS native provider detects the OS. Maybe do a detailed check later. On 6/24/13 12:36 PM, Xuelei Fan wrote: On 6/24/2013 12:29 PM, Weijun Wang wrote: Oh, actually it should be SunOS. Really? Then I would suggest you also check Solaris. http

hg: jdk8/tl/jdk: 8017453: ReplayCache tests fail on multiple platforms

2013-06-24 Thread weijun . wang
Changeset: 1bf060029a5d Author:weijun Date: 2013-06-24 16:25 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1bf060029a5d 8017453: ReplayCache tests fail on multiple platforms Reviewed-by: xuelei ! test/sun/security/krb5/auto/ReplayCacheExpunge.java !

Re: Code Review Request for 7196805: DH Key interoperability testing between SunJCE and JsafeJCE not successful

2013-06-24 Thread Weijun Wang
Code change looks fine. No regression test? --Max On 6/20/13 4:45 AM, Valerie (Yu-Ching) Peng wrote: I have updated the webrev to address your comments: http://cr.openjdk.java.net/~valeriep/7196805/webrev.01/ As for using JDK 8 default method feature, I think maybe it's better to delay the

Mac awt regression? (was JDK-8017189: [macosx]Step9: After openning the File menu, the items are disabled; regression since b94)

2013-06-24 Thread Weijun Wang
Hi All Sorry to write in HTML. I need to add a table. The bug is about after several clicks on policytool a menu item is disabled. I've simplified the tool to a small program (attached) with the same behavior. Running the program in JDK 7 and 8 on Mac shows these differences: JDK 7

Re: Code review request: 8016051: Possible ClassCastException in KdcComm

2013-06-25 Thread Weijun Wang
Hi Artem Line 221 is a little too long, you may want to break it. Otherwise, fine. Have you tried running JPRT with at least the jdk_security test? Thanks Max On 6/25/2013 1:35 PM, Artem Smotrakov wrote: Hi, please review this fix for 8:

hg: jdk8/tl/jdk: 8016051: Possible ClassCastException in KdcComm

2013-06-25 Thread weijun . wang
Changeset: 89631a384ee6 Author:weijun Date: 2013-06-25 21:51 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/89631a384ee6 8016051: Possible ClassCastException in KdcComm Reviewed-by: weijun Contributed-by: Artem Smotrakov artem.smotra...@oracle.com !

Re: Code review request: 8016051: Possible ClassCastException in KdcComm

2013-06-25 Thread Weijun Wang
-builds.html) Artem On 25.06.2013 16:04, Weijun Wang wrote: Hi Artem Line 221 is a little too long, you may want to break it. Otherwise, fine. Have you tried running JPRT with at least the jdk_security test? Thanks Max On 6/25/2013 1:35 PM, Artem Smotrakov wrote: Hi, please review this fix

Re: Smart Cards in Java Kerberos

2013-06-25 Thread Weijun Wang
Java (at least Oracle JDK) does not support PKINIT. Yes, you can do it outside, create a KerberosTicket and a KerberosPrincipal, create a JAAS Subject containing them, and call Subject.doAs() later. It should work. On Windows, if you manage to use Windows' own login and have the ticket

On 8017264: Java app crash on it's startup after Java updated to 7u25 from 7u21

2013-06-25 Thread Weijun Wang
Hi, Hotspot guys We (SE security) received a bug report on a new crash for 7u25 and need some help from you: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8017264 Here the top frames look like: C [msvcr100.dll+0x10b3b] wcspbrk+0x12d V [jvm.dll+0xa9b63] C

Re: On 8017264: Java app crash on it's startup after Java updated to 7u25 from 7u21

2013-06-26 Thread Weijun Wang
in the stacktrace is reflecting closest available symbol rather than actual symbol. As you say the sequence of calls don't really make sense. David On 26/06/2013 11:23 AM, Weijun Wang wrote: Hi, Hotspot guys We (SE security) received a bug report on a new crash for 7u25 and need some help from you

Re: Smart Cards in Java Kerberos

2013-07-01 Thread Weijun Wang
your app on a Windows box and it automatically works. Expect for this, I don't think the solution is not cross-platform. I'll look into the option, provided by Weijun Wang and create KerberosTicket/KerberosPrincipal. I hope it would do the job. You need to get the ticket anyway. Either from

Request for review: 8019267: NPE in AbstractSaslImpl when trace level = FINER in KRB5

2013-07-09 Thread Weijun Wang
Hi All An NPE was suppressed in jdk8 with a simple if. This new fix tries to output useful info even for a null argument. Please review the code changes at http://cr.openjdk.java.net/~weijun/8019267/webrev.01/ The added test makes sure logging does not crash and the output is

hg: jdk8/tl/jdk: 8019267: NPE in AbstractSaslImpl when trace level = FINER in KRB5

2013-07-10 Thread weijun . wang
Changeset: 780a64979c8d Author:weijun Date: 2013-07-10 15:11 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/780a64979c8d 8019267: NPE in AbstractSaslImpl when trace level = FINER in KRB5 Reviewed-by: mullan !

Re: Request for review: 8019267: NPE in AbstractSaslImpl when trace level = FINER in KRB5

2013-07-10 Thread Weijun Wang
On 07/09/2013 03:32 AM, Weijun Wang wrote: Hi All An NPE was suppressed in jdk8 with a simple if. This new fix tries to output useful info even for a null argument. Please review the code changes at http://cr.openjdk.java.net/~weijun/8019267/webrev.01/ The added test makes sure logging does

Code review reqest: 8019410: sun/security/krb5/auto/ReplayCacheTestProc.java

2013-07-12 Thread Weijun Wang
Please take a look at http://cr.openjdk.java.net/~weijun/8019410/webrev.00/ The ReplayCacheTestProc test fails on linux-i586 with MIT krb5 1.6.1 installed. The reason is that the acceptor does not start at all using native JGSS. This fix adds a sanity check on it and if the check does not

Suggested update to RFC 5653 JGSS-API: Provide a way to return a token when context establishment fails

2013-07-14 Thread Weijun Wang
Hi All I am Weijun Wang from the Java SE Seurity Team at Oracle, and this mail is about a design flaw in the initSecContext and acceptSecContext methods of the GSSContext class defined in RFC 5653 7.4.3 [1] and 7.4.9 [2]. The GSSContext::initSecContext() method could either return a token

Code review request: 8016594: Native Windows ccache still reads DES tickets

2013-07-15 Thread Weijun Wang
Please take a look at http://cr.openjdk.java.net/~weijun/8016594/webrev.00/ Instead of always reading tickets with session key of DES/RC4 etypes, it should accept any ticket in the default_tkt_enctypes setting. No reg test, needs a SQE test running with Windows 2008 as server. Thanks Max

Re: [kitten] Suggested update to RFC 5653 JGSS-API: Provide a way to return a token when context establishment fails

2013-07-19 Thread Weijun Wang
...@ietf.org mailto:kitten-requ...@ietf.org Date: Mon, Jul 15, 2013 at 12:01 PM Message: 4 Date: Mon, 15 Jul 2013 11:58:47 +0800 From: Weijun Wang weijun.w...@oracle.com mailto:weijun.w...@oracle.com To: kit...@ietf.org mailto:kit...@ietf.org, OpenJDK security-dev@openjdk.java.net mailto:security-dev

Code review request: 8021789: jarsigner parses alias as command line option (depending on locale)

2013-07-31 Thread Weijun Wang
Hi All Please review the fix at http://cr.openjdk.java.net/~weijun/8021789/webrev.00/ The problem is that jarsigner uses Collator::compare to check for command line options, and if that Collator uses Collator.PRIMARY as strength it treats -debug and debug the same (in some locales?). I

hg: jdk8/tl/jdk: 8021789: jarsigner parses alias as command line option (depending on locale)

2013-08-01 Thread weijun . wang
Changeset: 29f153e11683 Author:weijun Date: 2013-08-02 08:59 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/29f153e11683 8021789: jarsigner parses alias as command line option (depending on locale) Reviewed-by: vinnie !

Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot

2013-08-06 Thread Weijun Wang
I am not sure if IDN.java is the correct place to change. At least I've seen trailing dots in DNS entries. So maybe it's not so illegal. --Max On 8/6/13 7:44 PM, Xuelei Fan wrote: Hi, Please review the bug fix to strict the illegal input checking in IDN. webrev:

Re: [8] Request for Review: 8022461: Fix lint warnings in sun.security.{provider, rsa, x509}

2013-08-06 Thread Weijun Wang
Change looks fine. Minor issue: Extra parenthesis in src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java: +byte[] keyArray = +(new DerValue(DerValue.tag_Sequence, + out.toByteArray())).toByteArray(); One question: In

Re: Code review request: 8016594: Native Windows ccache still reads DES tickets

2013-08-07 Thread Weijun Wang
KERB_ETYPE_RC4_MD4 is a MS-private etype we don't support. Now that we pass in default_tkt_enctypes explicitly as an argument it seems no need to hardcode or comment on anything. Thanks Max -Dmitry On 2013-07-15 15:02, Weijun Wang wrote: Please take a look at http

hg: jdk8/tl/jdk: 7151062: [macosx] SCDynamicStore prints error messages to stderr

2013-08-07 Thread weijun . wang
Changeset: 906dd23334c1 Author:weijun Date: 2013-08-07 19:06 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/906dd23334c1 7151062: [macosx] SCDynamicStore prints error messages to stderr Reviewed-by: xuelei ! src/macosx/native/java/util/SCDynamicStoreConfig.m

Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot

2013-08-11 Thread Weijun Wang
if (q input.length()) { // Ah, a dot! out.append('.'); } p = q + 1; Using if (q != input.length()) should be even better. The searchDots method clearly specifies that or if there is no dots, return the length of input string. --Max

A 8021788 regression? 8022761: SQE test regression on wrongly signed indexed jar file

2013-08-11 Thread Weijun Wang
Hi Sherman SQE observes a regression in their test suite and the reason is my recent fix for 8021788 at http://hg.openjdk.java.net/jdk8/tl/jdk/rev/758e3117899c The jar file mentioned contains 66 Mon Jun 04 15:42:18 CST 2007 META-INF/INDEX.LIST 323 Sat Apr 01 15:47:28 CST 2000

Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot

2013-08-12 Thread Weijun Wang
I say 8a and 8b. --Max Thanks, Xuelei On 8/12/2013 11:11 AM, Weijun Wang wrote: I think the fix is adequate and necessary. One problem: lines 367-373 adds a new IAE to ToUnicode but the method should not fail forever. And some small comments on styles etc. On 8/12/13 9:09 AM, Xuelei Fan

Re: [JDK 8]Code review request 8022228, Intermittent test failures in sun/security/ssl/javax/net/ssl/NewAPIs

2013-08-21 Thread Weijun Wang
SessionTimeOutTests.java: 411 local.initCause(remote); What if local already has a cause? Will this overwrite it? 427 exception.addSuppressed(startException); Is it possible startException is null? Same for the other test. Also, if you have to do all these checks

Re: Code review request: 8012615: Realm.getRealmsList returns realms list in wrong

2013-08-22 Thread Weijun Wang
, Valerie On 07/29/13 02:11, Weijun Wang wrote: Hi Valerie Please review the capaths code change at http://cr.openjdk.java.net/~weijun/8012615/webrev.01/ It includes: Config.java === Add method to check if a sub-stanza exists. Realm.java == Rewrite reading cross-realm path

RFR: 8015669: KerberosPrincipal::equals should ignore name-type

2013-08-25 Thread Weijun Wang
Hi Sean Please look at the webrev at http://cr.openjdk.java.net/~weijun/8015669/webrev.00/ A new regression test is added. Thanks Max

Re: RFR 8022761: SQE test regression on wrongly signed indexed jar file

2013-08-26 Thread Weijun Wang
Ping again. On 8/19/13 9:11 PM, Weijun Wang wrote: Hi Sherman I try out jar i after signing and it puts INDEX.LIST at the very beginning of the file. Does this mean INDEX.LIST was actually an exception? Or it just jari bug? Anyway, I think I should update the fix for 8021788 and here

hg: jdk8/tl/jdk: 2 new changesets

2013-08-27 Thread weijun . wang
Changeset: ca53110f1c74 Author:weijun Date: 2013-08-27 17:50 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ca53110f1c74 8015669: KerberosPrincipal::equals should ignore name-type Reviewed-by: mullan ! src/share/classes/javax/security/auth/kerberos/KerberosPrincipal.java +

Re: Code review request, JKD-8024068 sun/security/ssl/javax/net/ssl/ServerName/IllegalSNIName.java fails

2013-09-01 Thread Weijun Wang
Code change looks fine. --Max On 9/2/13 10:41 AM, Xuelei Fan wrote: On 9/2/2013 10:38 AM, Xuelei Fan wrote: Hi Weijun, Please review this simple test fix. There is a typo in the test. The issue is exposed after the fix of JDK-8023881. webrev:

RFR 8024046,Test sun/security/krb5/runNameEquals.sh failed on 7u45 Embedded linux-ppc*

2013-09-06 Thread Weijun Wang
Please review the fix at http://cr.openjdk.java.net/~weijun/8024046/webrev.00/ We used to believe every Solaris/Linux/Mac should have native GSS libs, but this is not always correct. However, we do notice that for every system with native GSS lib, it also has the krb5-config utility. Most

Code review request: 8011402: Move blacklisting certificate logic from hard code to data

2013-09-06 Thread Weijun Wang
Hi Sean Please review the code changes at 8011402: Move blacklisting certificate logic from hard code to data Hard coded blacklisted certificates are moved out of the class file and now inside a data file. Furthermore, only their fingerprints are released in the JRE. The makefile covers

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

2013-09-06 Thread Weijun Wang
On 9/6/13 10:07 PM, Erik Joelsson wrote: Hello Max, I couldn't find the link to the review but I'm guessing this is the one: http://cr.openjdk.java.net/~weijun/8011402/webrev.00/ Correct, sorry about that. 3. Most important: it only works if both $(BLACKLISTED_CERTS_SRC_OPEN) and

hg: jdk8/tl/jdk: 8024046: Test sun/security/krb5/runNameEquals.sh failed on 7u45 Embedded linux-ppc*

2013-09-08 Thread weijun . wang
Changeset: 6bfabb71ae1e Author:weijun Date: 2013-09-09 11:08 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6bfabb71ae1e 8024046: Test sun/security/krb5/runNameEquals.sh failed on 7u45 Embedded linux-ppc* Reviewed-by: xuelei ! test/sun/security/krb5/runNameEquals.sh

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

2013-09-09 Thread Weijun Wang
On 9/7/13 1:26 AM, Sean Mullan wrote: 8011402: Move blacklisting certificate logic from hard code to data X509CertImpl: Might it not be better to store the fingerprints in UntrustedCertificates in a WeakHashMap (using the Certificate as a key)? This way we don't need to add public

Re: Code review request, 8024501, sun.security.mscapi.Key has no definition of serialVersionUID

2013-09-10 Thread Weijun Wang
Looks good. --Max On 9/11/13 10:30 AM, Xuelei Fan wrote: Hi, Please review a fix of a warning issue that sun.security.mscapi.Key has no definition of serialVersionUID. As -Werror specified, this warning cause jdk building error on windows platform. webrev:

webrev.01 of 8011402: Move blacklisting certificate logic from hard code to data

2013-09-11 Thread Weijun Wang
to UntrustedCertificates - Cache hash for Certificate - log blacklist parsing error in UntrustedCertificates - A new test Thanks Max On 9/6/13 9:30 PM, Weijun Wang wrote: Hi Sean Please review the code changes at 8011402: Move blacklisting certificate logic from hard code to data http

Re: webrev.01 of 8011402: Move blacklisting certificate logic from hard code to data

2013-09-11 Thread Weijun Wang
Slightly updated at the same location. Added different algorithm check in the 2 makefiles. Thanks Max On 9/11/13 3:57 PM, Weijun Wang wrote: Hi Sean and Erik An updated webrev is at http://cr.openjdk.java.net/~weijun/8011402/webrev.01/ Changes since the last webrev: - Some makefile

Re: webrev.01 of 8011402: Move blacklisting certificate logic from hard code to data

2013-09-11 Thread Weijun Wang
Yes, you're right. I need to try run it again with OPENJDK set. Thanks Max On 9/11/13 9:45 PM, Erik Joelsson wrote: On 2013-09-11 15:32, Weijun Wang wrote: Slightly updated at the same location. Added different algorithm check in the 2 makefiles. I should have noticed this earlier

Re: webrev.01 of 8011402: Move blacklisting certificate logic from hard code to data

2013-09-12 Thread Weijun Wang
. Certainly. You should also check that the files have the same number of entries. In my assumption, open and closed could have dups. I'll compare the Set size then. UntrustedCertificates: [84] nit: insert spaces around the = and OK. Thanks Max --Sean On 09/11/2013 09:32 AM, Weijun Wang

Re: Code Review Request: 7122707 update Security Providers to JDK8

2013-09-16 Thread Weijun Wang
On 9/17/13 9:27 AM, Xuelei Fan wrote: src/share/classes/sun/security/jgss/wrapper/SunNativeProvider.java == As above, I'm not sure native JGSS provider is using JDK version as the provider version. -super(NAME, 1.0, INFO);

Re: webrev.01 of 8011402: Move blacklisting certificate logic from hard code to data

2013-09-17 Thread Weijun Wang
hash field to volatile. Thanks Max On 9/13/13 8:29 AM, Weijun Wang wrote: On 9/13/13 5:15 AM, Sean Mullan wrote: Ok, I suggested you use a WeakHashMap but now I'm a little concerned this could become a bottleneck if every certificate check has to lock the map. Hmm. Maybe we should go back

hg: jdk8/tl/jdk: 8012615: Realm.getRealmsList returns realms list in wrong

2013-09-18 Thread weijun . wang
Changeset: ee8b292ee568 Author:weijun Date: 2013-09-18 18:22 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ee8b292ee568 8012615: Realm.getRealmsList returns realms list in wrong Reviewed-by: valeriep, xuelei ! src/share/classes/sun/security/krb5/Config.java !

Re: webrev.01 of 8011402: Move blacklisting certificate logic from hard code to data

2013-09-18 Thread Weijun Wang
On 9/18/13 9:20 PM, Sean Mullan wrote: I think it may be worth adding (maybe not for JDK 8 but JDK 9) a new method to the Certificate class called getFingerPrint(String alg) ... This way the fingerprint would not have to be calculated every time when using 3rd party providers for

hg: jdk8/tl/jdk: 8011402: Move blacklisting certificate logic from hard code to data

2013-09-18 Thread weijun . wang
Changeset: 07d73060e0da Author:weijun Date: 2013-09-18 21:37 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/07d73060e0da 8011402: Move blacklisting certificate logic from hard code to data Reviewed-by: erikj, mullan ! make/java/security/Makefile ! makefiles/CopyFiles.gmk !

Code review request: 8012615: backport of capaths fix to 7u60

2013-09-18 Thread Weijun Wang
Hi Valerie I've backported 8012615 to 7u60. Please take a review http://cr.openjdk.java.net/~weijun/8012615/7u-dev/webrev.00/ Changes include: 1. Direct copy of parseCapaths and parseHierarchy from jdk8, except for a) use old Config methods b) parseCapaths returns null instead of

Re: [8] Request for review: 8008296: keytool utility doesn't support '-importpassword' command

2013-09-18 Thread Weijun Wang
Hi Vinnie Mostly good, but do you need to add a help line for it? You reuse the GENSECKEY command so keytool -importpass -help looks strange. Also, the command name is -importpassword but the prompt is Enter the passphrase to be stored. Feel a little uncomfortable. Thanks Max On 9/14/13

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

2013-09-24 Thread Weijun Wang
Hi Xuelei Comments below. On 9/22/13 11:15 AM, Xuelei Fan wrote: Hi Weijun, Are you available to review this update? webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.00/ This is an enhancement to support stronger ephemeral DH keys during TLS handshaking. A new system property is

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

2013-09-25 Thread Weijun Wang
Please also update the CCC. On 9/24/13 6:42 PM, Xuelei Fan wrote: new webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.01/ ServerHandshaker.java: 1298: Should be system property not defined. 1311: customize 1319: Read below Overall, I think the comment is too long. :) ... Why

Code review request: 8024861: Incomplete token triggers GSS-API NullPointerException

2013-09-26 Thread Weijun Wang
Hi All Please take a review at http://cr.openjdk.java.net/~weijun/8024861/webrev.00/ When the first NegTokenInit does not include the mechToken, Java throws an NPE. This code change checks it and throw a GSSException instead. Precisely, the mechToken can be missing and the initiator will

hg: jdk8/tl/jdk: 8024861: Incomplete token triggers GSS-API NullPointerException

2013-09-27 Thread weijun . wang
Changeset: eb2c81533876 Author:weijun Date: 2013-09-27 15:25 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/eb2c81533876 8024861: Incomplete token triggers GSS-API NullPointerException Reviewed-by: mullan ! src/share/classes/sun/security/jgss/spnego/SpNegoContext.java +

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

2013-09-30 Thread Weijun Wang
Now that we only check localhost and localhost.localdomain, the following hack in the SSL.java test is not necessary any more: 92 // Run this after KDC, so our own DNS service can be started 93 try { 94 server =

Re: [8] Request for review: 8008296: keytool utility doesn't support '-importpassword' command

2013-10-03 Thread Weijun Wang
the prompt to its previous value: 'Enter the password to be stored'. Let me know if there are any other issues. Otherwise I'd like to push this today. On 19 Sep 2013, at 00:51, Weijun Wang wrote: Hi Vinnie Mostly good, but do you need to add a help line for it? You reuse the GENSECKEY command so

Re: [8] 8008171: Refactor KeyStore.DomainLoadStoreParameter as a standalone class

2013-10-08 Thread Weijun Wang
The code change looks fine. Do we need a CCC for it? Thanks Max p.s. I would use the bugs.openjdk.java.net URL. On 10/9/13 2:14 AM, Vincent Ryan wrote: Please review the following change - it's a simple re-factoring to promote a nested class to a stand-alone class: Bug:

Code review request: 8026235: keytool NSS test should use 64 bit lib on Solaris

2013-10-09 Thread Weijun Wang
Hi Vinnie Please take a review at http://cr.openjdk.java.net/~weijun/8026235/webrev.00/ Thanks Max

Re: [8] Request for Review: 8026052: cannot find symbol Proc

2013-10-09 Thread Weijun Wang
This looks harmless. But it looks more like a jtreg issue if it cannot deal with that slash. I haven't noticed it failing on Windows before. Can you point me to a jtr file. Thanks Max On 10/10/13 8:56 AM, Jason Uh wrote: Please review this fix. This changeset removes trailing slashes at

hg: jdk8/tl/jdk: 8026235: keytool NSS test should use 64 bit lib on Solaris

2013-10-10 Thread weijun . wang
Changeset: 74b4d20769fd Author:weijun Date: 2013-10-10 15:24 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/74b4d20769fd 8026235: keytool NSS test should use 64 bit lib on Solaris Reviewed-by: vinnie ! test/sun/security/tools/keytool/autotest.sh

Code review request: 7025699: Policy Tool is not accessible by keyboard

2013-10-11 Thread Weijun Wang
Hi All Please review the fix at http://cr.openjdk.java.net/~weijun/7025699/webrev.00/ The fix includes porting PolicyTool from AWT to Swing, defining mnemonics for menu items and buttons, and adding keyboard shortcuts for the File - New/Open/Save items. Several tests are updated also.

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