RFR: 8085903: New fix for memory leak in ProtectionDomain cache

2016-01-08 Thread Sean Mullan
Please review this fix for a memory leak in the ProtectionDomain cache (which maps each ProtectionDomain to their granted PermissionCollection). The memory leak only occurs if custom permissions are used in a policy file. http://cr.openjdk.java.net/~mullan/webrevs/8085903/webrev.00/ Custom

Re: RFR 8058778: New APIs for creating certificates and certificate requests

2016-01-08 Thread Sean Mullan
On 01/07/2016 10:38 PM, Wang Weijun wrote: On Jan 8, 2016, at 6:06 AM, Sean Mullan <sean.mul...@oracle.com> wrote: * CertificateFactorySpi Need more details on how inStream is parsed. I thought a "@see CertificateFactory#generateCertificateRequest" is enou

Re: RFR 8058778: New APIs for creating certificates and certificate requests

2016-01-07 Thread Sean Mullan
t at the moment. Builder still using byte[] which is PKCS #10 encoded. Many thanks to Mandy, Larry, and Sean for your comments. Mike, we will add more methods later when they are needed. --Max On Dec 15, 2015, at 11:53 PM, Sean Mullan <sean.mul...@oracle.com> wrote: On 12/03/2015 09:07

Re: Code Review Request, 8133070 Hot lock on BulkCipher.isAvailable

2015-12-22 Thread Sean Mullan
, Sean Mullan wrote: Here are a few other other comments on the code: SSLContextImpl: - I noticed that SSLContext.init does not specify how it handles empty arrays, and you have changed the code so that an empty TrustManager array is treated like they are null - is this change in behavior

Re: Code Review Request, 8133070 Hot lock on BulkCipher.isAvailable

2015-12-18 Thread Sean Mullan
, Sean Mullan wrote: Hi Xuelei, For JDK 9, the EC impl is defined to be in its own module (jdk.crypto.ec). How does it affect this fix if that module is not available/installed? The SunJSSE provider would not support dynamically loading of crypto providers/modules. At present, there are two

Re: Code Review Request, 8146197 SignatureAlgorithms.java after push of JDK-8146192

2015-12-27 Thread Sean Mullan
Looks fine to me. > On Dec 25, 2015, at 10:53 AM, Xuelei Fan wrote: > > Hi, > > Anyone available for a quick review? > > http://cr.openjdk.java.net/~xuelei/8146197/webrev.00/ > > A test failure caused by the removal of BASE64Decoder. Updated to use >

Re: RFR 8141690: JDK-8133151 change to MakeJavaSecurity.java is not complete

2015-11-25 Thread Sean Mullan
The fix looks fine to me. For testing, can you create a test that uses a custom java.security file with "#ifndef solaris-sparc" in it, and check whether the property is used or not depending on what system is being tested? --Sean On 11/24/2015 06:10 AM, Magnus Ihse Bursie wrote: On

Re: Code Review Request, 8136442 Don't tie Certificate signature algorithms to ciphersuites

2015-11-30 Thread Sean Mullan
On 11/30/2015 11:13 AM, Xuelei Fan wrote: You should change the comment above the calls to setupPrivateKeyAndChain >as it still reflects the previous behavior. Oops, forgot the update the comment. Updated: http://cr.openjdk.java.net/~xuelei/8136442/webrev.01/ Looks good. >Also,

Re: Code Review Request, 8136442 Don't tie Certificate signature algorithms to ciphersuites

2015-11-30 Thread Sean Mullan
You should change the comment above the calls to setupPrivateKeyAndChain as it still reflects the previous behavior. Also, should this change only be applicable to TLS 1.2? --Sean On 11/29/2015 08:55 AM, Xuelei Fan wrote: Hi, Please review the fix for JDK-8136442:

Re: RFR 8144107: jdk/security tests not included

2015-11-30 Thread Sean Mullan
Looks good. --Sean On 11/26/2015 02:12 AM, Wang Weijun wrote: Please review the fix at http://cr.openjdk.java.net/~weijun/8144107/webrev.00/ The recent JarSigner API changeset includes some tests in jdk/security but the directory is not included in any test group. This fix adds it into

Re: Code Review Request, 8144313, Test SessionTimeOutTests can be timeout

2015-12-01 Thread Sean Mullan
serverReady doesn't need to be volatile anymore. Looks good otherwise. --Sean On 12/01/2015 05:48 AM, Xuelei Fan wrote: Hi, Please review this test update: http://cr.openjdk.java.net/~xuelei/8144313/webrev.00/ In test/javax/net/ssl/SSLSession/SessionTimeOutTests.java, the update of

Re: RFR 8141457: keytool default cert fingerprint algorithm should be SHA-256

2015-12-01 Thread Sean Mullan
On 11/25/2015 09:39 PM, Wang Weijun wrote: Updated at http://cr.openjdk.java.net/~weijun/8141457/webrev.01/. I was lazy last time. Looks good. --Sean --Max On Nov 24, 2015, at 8:15 PM, Sean Mullan <sean.mul...@oracle.com> wrote: Looks good - although you could replace t

Re: Design review: JEP 273: DRBG-Based SecureRandom Implementations

2015-11-18 Thread Sean Mullan
/2015 08:43 PM, Wang Weijun wrote: On 2015年11月14日, at 上午1:56, Sean Mullan <sean.mul...@oracle.com> wrote: On 11/12/2015 07:58 PM, Wang Weijun wrote: What happens if configure is called more than once, or simultaneously by more than one thread? The state is reset. The last one

Re: Design review: JEP 273: DRBG-Based SecureRandom Implementations

2015-11-20 Thread Sean Mullan
On 11/19/2015 07:23 PM, Wang Weijun wrote: On Nov 20, 2015, at 1:11 AM, Sean Mullan<sean.mul...@oracle.com> wrote: >> >>However, I cannot get it working, and I found difficulties understanding the EngineDescription inner class inside Provider.java. >> >>1.

Re: RFR 8130132: jarsigner should emit warning if weak algorithms or keysizes are used

2015-11-20 Thread Sean Mullan
This looks good, just a few comments: KeyStoreUtil: 79 if (!ca.getSubjectDN().equals(end.getIssuerDN())) { Use getSubjectX500Principal instead of getSubjectDN as the DN matching algorithm is more precise. Resources: 246 "The %1$s algorithm used as %2$s is considered

Re: JDK 9 RFR of JDK-8143813: Problem list PKCS8Test.java

2015-11-23 Thread Sean Mullan
Looks fine to me. --Sean On 11/23/2015 10:57 AM, joe darcy wrote: Hello, Please review the patch below to problem list sun/security/pkcs/pkcs8/PKCS8Test.java on Solaris until the underlying issues (JDK-8143377) are addressed. Thanks, -Joe diff -r aa0621638103 test/ProblemList.txt

Re: RFR 8141457: keytool default cert fingerprint algorithm should be SHA-256

2015-11-24 Thread Sean Mullan
Looks good - although you could replace the MD5 fingerprints with the SHA256 fingerprints in the test files for some additional testing. --Sean On 11/23/2015 08:00 PM, Wang Weijun wrote: Hi All Please review a code change at http://cr.openjdk.java.net/~weijun/8141457/webrev.00/ SHA-256

Re: Design review: JEP 273: DRBG-Based SecureRandom Implementations

2015-11-19 Thread Sean Mullan
On 11/19/2015 08:41 AM, Wang Weijun wrote: On Nov 18, 2015, at 9:32 PM, Sean Mullan <sean.mul...@oracle.com> wrote: The getInstance methods can now take a SecureRandomParameterSpec object (rather than an AlgorithmParameterSpec). They should throw InvalidAlgorithmParameterExc

Re: RFR 8056174: New APIs for jar signing

2015-11-19 Thread Sean Mullan
On 11/18/2015 09:54 PM, Wang Weijun wrote: In AlgorithmId.getDefaultSigAlgForKey, I think you can remove the last sentence ("Remember ...") - this seems like a ToDo note to yourself which has been done. It's a reminder if we update it again in the future. Ok. It makes me feel a little

Re: RFR: 8085903: New fix for memory leak in ProtectionDomain cache

2016-01-12 Thread Sean Mullan
check for this case in the PDCache.put method, and it instead uses the Map.replace method. Updated webrev: http://cr.openjdk.java.net/~mullan/webrevs/8085903/webrev.01/ --Sean On 01/08/2016 03:06 PM, Sean Mullan wrote: Please review this fix for a memory leak in the ProtectionDomain cache

Re: RFR 8058778: New APIs for creating certificates and certificate requests

2016-01-12 Thread Sean Mullan
A few more comments for now, but I'll need another day or so to finish my review: * General Use @throws instead of @exception * X509Certificate lines 572-585 were removed, but where was it copied? It is not in GeneralName and probably should not be unless we add a toString method. 847

Re: RFR: 8085903: New fix for memory leak in ProtectionDomain cache

2016-01-12 Thread Sean Mullan
Hi Ivan, On 01/12/2016 09:38 AM, Ivan Gerasimov wrote: Hi Sean! On 12.01.2016 16:26, Sean Mullan wrote: I received a private comment that there may be cases where the map's value is reclaimed by the garbage collector, but the key still exists. If that ProtectionDomain is subsequently used

Re: RFR: 8085903: New fix for memory leak in ProtectionDomain cache

2016-01-12 Thread Sean Mullan
()) which should cause a new cache to be created. Let me do some more testing around this, and see if I can check performance and see what is better. Thanks, Sean Xuelei On 1/12/2016 11:01 PM, Sean Mullan wrote: Hi Ivan, On 01/12/2016 09:38 AM, Ivan Gerasimov wrote: Hi Sean! On 12.01.2016 16

Re: RFR: 8085903: New fix for memory leak in ProtectionDomain cache

2016-01-12 Thread Sean Mullan
-8055753 On 01/12/2016 11:02 AM, Sean Mullan wrote: On 01/12/2016 10:26 AM, Xuelei Fan wrote: I think hashMap.put() may be similar or more effective than hashMap.putIfAbsent(). Ok, I think I see what you are trying to say. The common case is that the putIfAbsent method is going to succeed (i.e

Re: [9] RFR:JDK-8143305:sun/security/ec/TestEC.java fails intermittently

2016-06-02 Thread Sean Mullan
On 06/02/2016 06:16 AM, Tim Du wrote: Hi, Please help to review the path for JDK-8143305 TestEC.java test for ECC algorithms in SSL/TLS with different cipher suites. It failed with javax.net.ssl.SSLException: Unsupported or unrecognized SSL message intermittently. The exception was threw at

Re: RFR 8158442: SecureRandomParameters missing "@since 9"

2016-06-06 Thread Sean Mullan
Looks good. --Sean On 06/06/2016 02:22 AM, Wang Weijun wrote: I forgot to add "@since 9" in a new interface, please review this trivial change diff --git a/src/java.base/share/classes/java/security/SecureRandomParameters.java

Re: RFR 8062758: Update java/security/Security/ClassLoaderDeadlock/Deadlock2.sh with the removal of -Djava.ext.dirs

2016-06-06 Thread Sean Mullan
Looks fine to me. Also copying Mandy to see if she is ok with this. --Sean On 06/06/2016 01:40 AM, Bhanu Gopularam wrote: Hi all, Please review fix for following bug : Bug - https://bugs.openjdk.java.net/browse/JDK-8062758 Issue - Test java/security/Security/ClassLoaderDeadlock/Deadlock2.sh

Re: RFR 8157308: DRBG serialization fix

2016-06-03 Thread Sean Mullan
On 06/03/2016 10:13 AM, Wang Weijun wrote: On Jun 3, 2016, at 10:02 PM, Sean Mullan <sean.mul...@oracle.com> wrote: On 06/03/2016 02:12 AM, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/8157308/webrev.01/ AbstractDrbg is an internal class so I have to

Re: [9] RFR:JDK-8143305:sun/security/ec/TestEC.java fails intermittently

2016-06-03 Thread Sean Mullan
The codes were also updated follow Sean's suggestion at here: http://cr.openjdk.java.net/~tidu/8158620/webrev.00/ Please help to review again.Thanks. Regards Tim On 2016/6/3 4:38, Sean Mullan wrote: On 06/02/2016 06:16 AM, Tim Du wrote: Hi, Please help to review the path for JDK-8143305 TestEC.java

Re: RFR 8154005: Add algorithm constraint that specifies the restriction date

2016-05-26 Thread Sean Mullan
On 05/11/2016 06:46 PM, Anthony Scarpino wrote: Please review the changes related to 8154005. This is a continuation JEP-288. It adds a denyAfter constraint the stops PKIX algorithm support at a specified date. http://cr.openjdk.java.net/~ascarpino/8154005/webrev/ A few minor comments on

Re: [9] RFR 8154191: Deprivilege java.smartcardio module

2016-06-15 Thread Sean Mullan
On 6/14/2016 2:28 PM, Sean Mullan wrote: I don't think you need to make changes to test/javax/smartcardio/policy. Instead you can change the tests that use this policy file to use the new jtreg @run /java.security.policy=policy which extends the default system policy. --Sean On 06/13/2016 08:10

Re: [9] RFR 8157881: security.provider property description needs to be updated for modules

2016-06-13 Thread Sean Mullan
On 06/10/2016 04:07 PM, Valerie Peng wrote: Sounds good. Webrev updated at: http://cr.openjdk.java.net/~valeriep/8157881/webrev.01/ Looks good. --Sean

Re: RFR 8157308: DRBG serialization fix

2016-06-03 Thread Sean Mullan
Max On Jun 3, 2016, at 12:09 AM, Sean Mullan <sean.mul...@oracle.com> wrote: For the test/sun/security/provider/SecureRandom/AbstractDrbgSpec.java that was removed, are you still getting adequate test coverage somewhere else on the SecureRandom API tests this test was checking? Oth

Re: RFR 8157308: DRBG serialization fix

2016-06-02 Thread Sean Mullan
For the test/sun/security/provider/SecureRandom/AbstractDrbgSpec.java that was removed, are you still getting adequate test coverage somewhere else on the SecureRandom API tests this test was checking? Otherwise this looks good, though may I suggest you adjust the bug synopsis to be less

Re: RFR: 8154009: Some methods of java.security.Security require more permissions, than necessary

2016-06-02 Thread Sean Mullan
proposed. 2) I removed 2 tests from *test/ProblemList.txt*, which were marked as failed due to JDK-8154009 (current fix). Best regards, Artem Kosarev. ** On 01.06.2016 17:03, Sean Mullan wrote: I think it would be helpful to add a comment to EmptyPolicy.policy so it contains something, ex: // empty

Re: RFR: 8154009: Some methods of java.security.Security require more permissions, than necessary

2016-06-01 Thread Sean Mullan
I think it would be helpful to add a comment to EmptyPolicy.policy so it contains something, ex: // empty policy file for testing Otherwise, looks fine. --Sean On 05/30/2016 09:03 AM, Artem Kosarev wrote: Hello. Could you please review the proposed fix issue which is NOT applicable for JDK

Re: RFR 8152108: Correct jarsigner warning message about missing timestamp

2016-05-31 Thread Sean Mullan
Looks fine. Bug should have a noreg label. --Sean On 05/31/2016 09:33 AM, Wang Weijun wrote: Please review this string change for https://bugs.openjdk.java.net/browse/JDK-8152108 Correct jarsigner warning message about missing timestamp ---

Re: [9] RFR 8154191: Deprivilege java.smartcardio module

2016-06-14 Thread Sean Mullan
I don't think you need to make changes to test/javax/smartcardio/policy. Instead you can change the tests that use this policy file to use the new jtreg @run /java.security.policy=policy which extends the default system policy. --Sean On 06/13/2016 08:10 PM, Valerie Peng wrote: Sean, Can

Re: RFR 8159805: sun/security/tools/jarsigner/warnings/NoTimestampTest.java fails after JDK-8027781

2016-06-17 Thread Sean Mullan
Looks fine. --Sean On 06/17/2016 08:40 AM, Wang Weijun wrote: I forgot to update the test as well. Please review this patch: diff --git a/test/sun/security/tools/jarsigner/warnings/Test.java b/test/sun/security/tools/jarsigner/warnings/Test.java ---

Re: RFR 8157730: Mark deprecated java.security.{Identity, IdentityScope, Signer} APIs with forRemoval=true

2016-06-17 Thread Sean Mullan
Looks fine to me. --Sean On 06/17/2016 08:52 AM, Vincent Ryan wrote: Three Identity-related classes were deprecated in JDK 1.2. Please review the patch below that marks them as candidates for removal in a future JDK release. See http://openjdk.java.net/jeps/277 for details of the enhanced

Re: RFR 8027781: New jarsigner timestamp warning is grammatically incorrect

2016-06-16 Thread Sean Mullan
On 06/16/2016 12:17 AM, Wang Weijun wrote: Hi Sean Please review a resource string change diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java ---

Re: RFR: 8133085- Remove old style (pre-JDK 1.4) "new SunJCE()" provider calls in tests. Fails to compile.

2016-01-08 Thread Sean Mullan
Please update copyrights to include 2016. Otherwise, looks fine to me. --Sean On 01/08/2016 02:54 AM, Bhanu Gopularam wrote: Hi all, Please review a fix for following bug: Bug Id -https://bugs.openjdk.java.net/browse/JDK-8133085 Issue – Some security regression tests are directly

Re: RFR 8058778: New APIs for creating certificates and certificate requests

2016-01-13 Thread Sean Mullan
addAuthorityKeyIdentifierExtension(); X509Certificate.Builder addSubjectKeyIdentifierExtension(); 2. X509Extension getRawExtensionValue(String oid) 3. Spec changes we discussed. Still one TODO in X509Certificate.Builder subject(String name). Some comments below in line. On Jan 13, 2016, at 5:58 AM, Sean Mullan

Re: Code Review Request 8146669 Test SessionTimeOutTests fails intermittently

2016-01-15 Thread Sean Mullan
It seems like it would be cleaner to use AtomicBoolean for serverReady since you are just checking if it is on or off. Otherwise looks fine. --Sean On 01/13/2016 04:23 AM, Xuelei Fan wrote: Hi, Please review the intermittently test failure fix.

Re: RFR [9] 7067728: Remove stopThread default java.policy

2016-01-14 Thread Sean Mullan
Looks good to me. --Sean On 01/14/2016 05:05 AM, Chris Hegarty wrote: The "stopThread” RuntimePermission is granted by default. The Thread.stop methods have been deprecated for more than 15 years. It seems reasonable, in a major release, to remove the default grant of stopThread. diff --git

Re: RFR 8158633: BASE64 encoded cert not correctly parsed

2016-06-17 Thread Sean Mullan
Looks fine to me, but can you add a comment to the bug report explaining what the issue was? Thanks, Sean On 06/17/2016 06:57 AM, Wang Weijun wrote: Please review the fix at http://cr.openjdk.java.net/~weijun/8158633/webrev.00/ *Jon*: If I write this line inside a .java test: @test

Re: RFR 8059212: Modify sun/security/smartcardio manual regression tests so that they do not just fail if no cardreader found

2016-02-11 Thread Sean Mullan
This fix looks fine to me. --Sean On 01/29/2016 10:40 AM, Ivan Gerasimov wrote: Hello! We've got a request to modify the smartcardio-related regtests so that they'll be more convenient to use. Konstantin, could you please confirm this will work for you? BUGURL:

Re: SHA1 deprecation for codesigning? (Was: RFR : 8038837:Add support to jarsigner for specifying timestamp hash algorithm)

2016-01-27 Thread Sean Mullan
On 01/27/2016 11:43 AM, e...@zusammenkunft.net wrote: BTW, is there any SHA1 deprecation planned/expected for JNLP code signing? Yes. We are working on a plan for restricting certificates signed with SHA-1 and other use cases, but don't have any dates to share yet. Thanks, Sean

Re: JEP Review Request: SHA-3 Hash Algorithm

2016-02-23 Thread Sean Mullan
This looks good. One suggestion, in the Description section, I think we should state what the standard algorithm names are for the 4 new MessageDigest algorithms. It is sort of implied, but I think we should be more specific. Perhaps after the "These can be implemented ..." sentence, add:

RFR 8138653: Default key sizes for the AlgorithmParameterGenerator and KeyPairGenerator implementations should be upgraded

2016-02-24 Thread Sean Mullan
Please review this fix to improve security defaults by increasing the default keysize of the RSA, DSA, and DiffieHellman implementations of AlgorithmParameterGenerator and KeyPairGenerator from 1024 to 2048 bits: http://cr.openjdk.java.net/~mullan/webrevs/8138653/webrev.00/ Thanks, Sean

Re: RFR 8138653: Default key sizes for the AlgorithmParameterGenerator and KeyPairGenerator implementations should be upgraded

2016-02-24 Thread Sean Mullan
|parameters. I think we also cache 2048 bit values. Maybe you can modify. This is true -- I will add 2048 to the above sentence. --Sean Regards, Sean. On 24/02/16 14:54, Sean Mullan wrote: Please review this fix to improve security defaults by increasing the default keysize of the RSA, DSA

Re: Code Review Request 8149417 Use final restricted flag

2016-02-23 Thread Sean Mullan
On 02/17/2016 08:28 PM, Xuelei Fan wrote: Hi, A new test case was added. Please review the update: http://cr.openjdk.java.net/~xuelei/8149417/webrev.01/ This looks fine to me. --Sean

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

2016-02-25 Thread Sean Mullan
On 02/18/2016 03:10 AM, Weijun Wang wrote: There is another compatibility issue which is more important: Today, we tell users to load their own PKCS11 provider with -providerClass sun.security.pkcs11.SunPKCS11 -providerArg some.cfg and seems the new options should be -provider

Re: RFR: 8133085- Remove old style (pre-JDK 1.4) "new SunJCE()" provider calls in tests. Fails to compile.

2016-01-19 Thread Sean Mullan
- From: Bradford Wetmore Sent: Saturday, January 09, 2016 12:31 AM To: Sean Mullan; Bhanu Gopularam; security-dev@openjdk.java.net Subject: Re: RFR: 8133085- Remove old style (pre-JDK 1.4) "new SunJCE()" provider calls in tests. Fails to compile. (Xuelei, see below) Bhanu, Th

Re: RFR 8147772: Some KerberosTicket methods throw NPE after being destroyed

2016-01-20 Thread Sean Mullan
On 01/19/2016 10:32 PM, Wang Weijun wrote: http://cr.openjdk.java.net/~weijun/8147772/webrev.00/ No spec change, just safer return value. I think it would be useful to update the specification to indicate how these methods behave when the object has been destroyed. I also noticed that many

Re: RFR 8147772: Some KerberosTicket methods throw NPE after being destroyed

2016-01-20 Thread Sean Mullan
On 01/20/2016 09:13 AM, Wang Weijun wrote: On Jan 20, 2016, at 10:14 PM, Sean Mullan <sean.mul...@oracle.com> wrote: On 01/20/2016 08:52 AM, Wang Weijun wrote: You mean let them throw an ISE after destroyed? Not sure if it is backportable. No, I am just talking about documenting ex

Re: RFR 8147772: Some KerberosTicket methods throw NPE after being destroyed

2016-01-20 Thread Sean Mullan
methods that already throw ISE, I would add: @throws IllegalStateException if the ticket has been destroyed --Sean The problem is reported by customers using an old JRE. --Max On Jan 20, 2016, at 9:36 PM, Sean Mullan <sean.mul...@oracle.com> wrote: On 01/19/2016 10:32 PM, Wang Weijun

Re: RFR 8147400: Deprecate policytool

2016-01-27 Thread Sean Mullan
Looks good to me. --Sean On 01/26/2016 04:56 AM, Wang Weijun wrote: Hi All Please review the patch below. Every change after line 873 is adding "@SuppressWarnings("deprecation")" to a top-level class that references the PolicyTool class. I wish they were static inner classes. We also

Re: Code Review Request 8149417 Use final restricted flag

2016-02-15 Thread Sean Mullan
Looks good. --Sean On 02/15/2016 01:33 AM, Xuelei Fan wrote: Hi, Please review this code cleanup: http://cr.openjdk.java.net/~xuelei/8149417/webrev.00/ Simple fix, adding final key word to a static flag. Thanks, Xuelei

Re: Code Review Request, 8148500: [Spec] Enabled SSL Protocols may not be used

2016-02-15 Thread Sean Mullan
On lines 282-5 of SSLSocket, I think you should use similar language to be consistent: "Note that even if a suite has been enabled, it may never be used. This can occur if the peer does not support it, the requisite certificates (and private keys) for the suite are not available, or an

Re: RFR 8147772: Some KerberosTicket methods throw NPE after being destroyed

2016-02-16 Thread Sean Mullan
, 2016, at 10:18 PM, Sean Mullan <sean.mul...@oracle.com> wrote: On 01/20/2016 09:13 AM, Wang Weijun wrote: On Jan 20, 2016, at 10:14 PM, Sean Mullan <sean.mul...@oracle.com> wrote: On 01/20/2016 08:52 AM, Wang Weijun wrote: You mean let them throw an ISE after destroyed? Not sure if

Re: Code Review Request 8139565 Restrict certificates with DSA keys less than 1024 bits

2016-02-16 Thread Sean Mullan
Looks good. --Sean On 02/16/2016 12:16 AM, Xuelei Fan wrote: Added a new regression test: http://cr.openjdk.java.net/~xuelei/8139565/webrev.01/ Thanks, Xuelei On 2/15/2016 8:23 AM, Xuelei Fan wrote: Hi, Please review this security crypto constraints update:

Re: Code Review Request, 8148500: [Spec] Enabled SSL Protocols may not be used

2016-02-16 Thread Sean Mullan
Looks good. --Sean On 02/15/2016 09:14 PM, Xuelei Fan wrote: It's nice. Here is the updated webrev: http://cr.openjdk.java.net/~xuelei/8148500/webrev/ Thanks, Xuelei On 2/16/2016 12:05 AM, Sean Mullan wrote: On lines 282-5 of SSLSocket, I think you should use similar language

Re: Code Review Request 8139565 Restrict certificates with DSA keys less than 1024 bits

2016-02-17 Thread Sean Mullan
in a X.509 certificate. The return value of DSAKey.getParams() may be null. This special case now is considered in the KeyUtil implementation. Thanks, Xuelei On 2/17/2016 4:22 AM, Sean Mullan wrote: Looks good. --Sean On 02/16/2016 12:16 AM, Xuelei Fan wrote: Added a new regression test

Re: RFR 8138653: Default key sizes for the AlgorithmParameterGenerator and KeyPairGenerator implementations should be upgraded

2016-03-01 Thread Sean Mullan
than 1024 may be incompatible and rejected). We will wait on this one for now. - The SunPKCS11 default size for RSA keys has been increased to 2048. - A bug in the PKCS11 tests was fixed which caused the version of newer NSS libraries to be unrecognized. --Sean On 02/24/2016 09:54 AM, Sean

Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list

2016-03-14 Thread Sean Mullan
Looks good to me. --Sean On 03/11/2016 10:28 PM, joe darcy wrote: Hello, As Jon Gibbons has noted off-list, the problem list entries can directly include the bug number associated with the test in question, enabling better reporting. This format should be used rather than the current

Re: RFR 8140422: Add mechanism to allow non default root CAs to be not subject to algorithm restrictions

2016-03-08 Thread Sean Mullan
You should also copy build-dev on the next iteration since there are a few Makefile changes. On 02/29/2016 11:55 AM, Anthony Scarpino wrote: I need a code review of this change: http://cr.openjdk.java.net/~ascarpino/8140422/webrev/ Currently CertPath algorithm restrictions allow or deny all

Re: RFR 8140422: Add mechanism to allow non default root CAs to be not subject to algorithm restrictions

2016-04-12 Thread Sean Mullan
On 04/11/2016 11:59 AM, Anthony Scarpino wrote: I believe I have addressed all previous comments and some changes were made to rename cacerts to jdkCA and how it works AnchorCertificates.java http://cr.openjdk.java.net/~ascarpino/8140422/webrev.03/ * CertConstraintParameters 31 * This

New JEP Draft for review: "Disable SHA-1 Certificates"

2016-04-05 Thread Sean Mullan
We are seeking feedback on a new JEP Draft ("Disable SHA-1 Certificates) that is initially targeted to JDK 9: http://openjdk.java.net/jeps/8149555 The goal of the JEP is to improve the default security configuration of the JDK by disabling X.509 certificate chains with SHA-1 based

Re: RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

2016-03-31 Thread Sean Mullan
Just a few comments: - SunJCE 707 // TODO: aliases with OIDs leftover TODO. - SecureRandom 604 * @implSpec The default implementation returns {@code null}. Technically, I don't think that is correct, since it is really dependent on what the underlying Spi is doing.

CFV: New Security Group Member: Jamil Nimeh

2016-04-29 Thread Sean Mullan
for JDK 9. Votes are due by May 13, 2016, 3:00 PM UTC. Only current Members of the Security Group [1] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list. For Lazy Consensus voting instructions, see [2]. Sean Mullan [1] http://openjd

Re: CFV: New Security Group Member: Jamil Nimeh

2016-04-29 Thread Sean Mullan
Vote: yes On 04/29/2016 10:40 AM, Sean Mullan wrote: I hereby nominate Jamil Nimeh to Membership in the Security Group. Jamil is a member of the Java Security Libraries team at Oracle and has been an active contributor to the OpenJDK Security Group for approximately two years. Jamil was voted

Re: RFR(XXS): 8155036: Remove sun.security.action.GetBooleanSecurityPropertyAction

2016-04-26 Thread Sean Mullan
On 04/25/2016 12:47 PM, Claes Redestad wrote: Hi, since JEP-260 encapsulates internal APIs, we could remove unused classes like this one hg rm ./src/java.base/share/classes/sun/security/action/GetBooleanSecurityPropertyAction.java The removal of this class is ok with me. There also doesn't

Re: RFR: 9: 8156035: Remove intermittent key from sun/security/rsa/SpecTest.java

2016-05-18 Thread Sean Mullan
Looks fine to me. --Sean On 05/18/2016 01:41 PM, Rajan Halade wrote: Please review this small change to remove intermittent key from this test. JDK-8137231 addressed intermittent failure and we haven't seen test failure for a long time and with frequent runs. Bug:

Re: Code Review Request JDK-8151856, Note that disabledAlgorithms override the same algorithms of legacyAlgorithm

2016-05-18 Thread Sean Mullan
719 # javax.net.ssl.SSLParameters.getAlgorithmConstraints()), I think it should be setAlgorithmConstraints. Otherwise, looks fine to me. --Sean On 05/18/2016 11:19 AM, Xuelei Fan wrote: Hi, Please review this spec update of the "jdk.tls.legacyAlgorithms" Security Property:

Re: RFR: regex changes -- sun.security.util.Debug issue

2016-05-10 Thread Sean Mullan
Hi Xueming, It looks ok to me, but I'm curious if there may be other security classes that are initialized quite early and write to debug and may run into similar issues. Did you run all of the security regression tests in the jprt job? --Sean On 5/10/16 11:57 AM, Xueming Shen wrote:

Re: Support version 1 cert generation

2016-05-17 Thread Sean Mullan
Hi Xuelei, Can you elaborate under what circumstances this is useful for testing? X.509 v3 was first published in 1996, and v1 certificates should be pretty much non-existent these days (although there are some root certs that are still v1). v1 certificates do not support extensions. Adding

Result: New Security Group Member: Jamil Nimeh

2016-05-16 Thread Sean Mullan
The vote for Jamil Nimeh [1] is now closed. Yes: 7 Veto: 0 Abstain: 0 According to the Bylaws definition of Lazy Consensus, this is sufficient to approve the nomination. Sean Mullan [1] http://mail.openjdk.java.net/pipermail/security-dev/2016-April/013781.html

Re: Support version 1 cert generation

2016-05-18 Thread Sean Mullan
certificate can also be used as a self-signed cert. --Sean It may be something that we only want to consider for self-signed cert on request. Thanks, Xuelei On May 17, 2016, at 7:45 PM, Sean Mullan <sean.mul...@oracle.com> wrote: Hi Xuelei, Can you elaborate under what circums

Re: RFR 8138766: New default -sigalg for keytool

2016-05-18 Thread Sean Mullan
Looks good to me. Please create a release-note subtask so that this change in keytool defaults gets documented in the JDK 9 release notes. (There should also be one for the jarsigner tool). Thanks, Sean On 05/16/2016 10:06 PM, Wang Weijun wrote: Please take a look at

Re: JDK 9 RFR of JDK-8156897: Problem list ShortRSAKey1024.sh on windows

2016-05-12 Thread Sean Mullan
Looks fine to me. --Sean On 5/12/16 3:11 PM, joe darcy wrote: Hello, The test sun/security/mscapi/ShortRSAKey1024.sh has been seen to fail with some frequency on windows. Until JDK-8153948 is addressed, the test should be problem listed. Please review the patch below which does this.

Review Request: 8150468: ClassCircularityError on error in security policy file

2016-05-06 Thread Sean Mullan
Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8150468: http://cr.openjdk.java.net/~mullan/webrevs/8150468/webrev.00/ The fix is to record bad policy files as they are parsed and ignore them during any subsequent permission checks. Thanks, Sean

Re: RFR: 8155775: Re-examine naming of privileged methods to access System properties

2016-05-02 Thread Sean Mullan
On 05/02/2016 10:15 AM, Sean Mullan wrote: This looks good. Just a couple of comments: * src/java.base/share/classes/java/util/TimeZone.java 698 props.setProperty("user.timezone", id); This will only change the local copy of the property. I think you want to keep the ori

Re: RFR: 8155775: Re-examine naming of privileged methods to access System properties

2016-05-02 Thread Sean Mullan
This looks good. Just a couple of comments: * src/java.base/share/classes/java/util/TimeZone.java 698 props.setProperty("user.timezone", id); This will only change the local copy of the property. I think you want to keep the original code which does a System.setProperty. *

Re: RFR: 8155775: Re-examine naming of privileged methods to access System properties

2016-05-10 Thread Sean Mullan
On 5/9/16 11:44 PM, Wang Weijun wrote: I have a question related. There are some places in JDK that use doPrivileged to read "os.name" etc. This system property is in the default java.policy file On May 2, 2016, at 10:15 PM, Sean Mullan <sean.mul...@oracle.com> wrote: This

Re: RFR: regex changes -- sun.security.util.Debug issue

2016-05-10 Thread Sean Mullan
On 5/10/16 1:30 AM, Alan Bateman wrote: On 10/05/2016 06:36, Xueming Shen wrote: Hi, While testing for the attached regex changes, a fatal vm init error was triggered for test case with -Djava.security.debug=xyz turned on, as showed in following stacktrace. It appears

Re: Review Request: 8150468: ClassCircularityError on error in security policy file

2016-05-10 Thread Sean Mullan
On 5/9/16 6:20 PM, Mandy Chung wrote: On May 6, 2016, at 11:43 AM, Sean Mullan <sean.mul...@oracle.com> wrote: Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8150468: http://cr.openjdk.java.net/~mullan/webrevs/8150468/webrev.00/ The fix is to record bad policy

Re: RFR 8077360: Lower the number of providers created when using ServiceLoader

2016-04-18 Thread Sean Mullan
Looks good to me. --Sean On 04/15/2016 08:09 PM, Valerie Peng wrote: Hi Sean, Not sure if you have reviewed this or not as you were on vacation while Mandy and Alan reviewed this prototype fix for Jigsaw workspace earlier. Now that the relevant Jigsaw code have been merged into JDK, I plan to

Re: Code Review Request 8144566, Custom HostnameVerifier disables SNI extension

2016-04-20 Thread Sean Mullan
* SSLSocketImpl.java 2100 // ONLY used by ClientHandshaker for the server hostname during handshaling typo: handshaking 2114 synchronized private void useImplicitHost(boolean noSniUpdate) { the modifier order should be "private synchronized ..." See:

Re: Code Review Request 8144566, Custom HostnameVerifier disables SNI extension

2016-04-21 Thread Sean Mullan
Updated webrev looks fine to me. --Sean On 04/20/2016 11:18 AM, Xuelei Fan wrote: Thanks for the comments, all looks reasonable to me. Updated webrev: http://cr.openjdk.java.net/~xuelei/8144566/webrev.02/ Thanks, Xuelei On 4/20/2016 9:10 PM, Sean Mullan wrote: * SSLSocketImpl.java 2100

RFR: 8159752: Grant de-privileged module permissions by default with java.security.policy override option

2016-07-14 Thread Sean Mullan
Please review this change to the default Policy provider implementation to grant de-privileged module permissions by default even when the java.security.policy override option is specified or when the Policy.getInstance API is used:

Re: RFR: 8159752: Grant de-privileged module permissions by default with java.security.policy override option

2016-07-14 Thread Sean Mullan
On 07/14/2016 04:38 PM, Chris Hegarty wrote: The default.policy file is now always loaded by the default Policy provider implementation (sun/security/provider/PolicyFile). It is loaded if the java.security.policy '=' or '==' option is specified, and also if the application uses the

Re: RFR 8161354: policytool fails if it needs to show an error dialog before the main window appears

2016-07-14 Thread Sean Mullan
Looks ok to me. Make sure you add a noreg label to the bug. --Sean On 07/14/2016 04:30 AM, Weijun Wang wrote: Please review the patch below: diff --git a/src/jdk.policytool/share/classes/sun/security/tools/policytool/PolicyTool.java

Re: [9] RFR 8159488 "Deprivilege java.xml.crypto" and 8161171 "Missed the make/common/Modules.gmk file when integrating JDK-8154191"

2016-07-25 Thread Sean Mullan
of them are covered though. I will double check and add test if necessary... Thanks, Valerie On 7/21/2016 12:08 PM, Sean Mullan wrote: On 07/20/2016 07:57 PM, Valerie Peng wrote: Sean, I have updated webrev to rid the dependency on sun.security.jca packages. Also, I found one additional

Re: Code Review Request JDK-8161898 Mark the use of deprecated javax.security.cert APIs with forRemoval=true

2016-07-25 Thread Sean Mullan
Looks fine to me. --Sean On 07/20/2016 10:32 PM, Xuelei Fan wrote: Hi, javax.security.cert was deprecated and marked with forRemoval=true in JDK 9. The use of javax.security.cert APIs should be marked with forRemoval=true too. Please review the update:

Re: Code Review Request: JDK-8161506 : Deprecate pre-1.2 SecurityManager methods and fields with forRemoval=true

2016-07-27 Thread Sean Mullan
On 07/27/2016 12:45 PM, Coleen Phillimore wrote: This looks great. Thank you! Should we have another bug to actually remove the code and hotspot code for jdk10 Yes we should. We need to also do that for all of the other APIs that we have added forRemoval=true to for 9, so I was thinking

Re: RFR 8162752: keytool -importkeystore should probe srcstoretype if not specified

2016-07-29 Thread Sean Mullan
In the test: 37 // When there is no -srckeystore, it should be probed from the file did you mean to say "no -srcstoretype"? Looks fine otherwise. --Sean On 07/28/2016 10:11 PM, Weijun Wang wrote: Hi All Please review the fix at

Code Review Request: JDK-8161506 : Deprecate pre-1.2 SecurityManager methods and fields with forRemoval=true

2016-07-27 Thread Sean Mullan
Please review this change to mark the pre-1.2 deprecated SecurityManager methods (and 1 field) with forRemoval=true. These methods are no longer necessary and are known to be error-prone and have been deprecated since JDK 1.2. The intention is to remove them in a subsequent release of Java SE.

Re: [9]RFR 8163435: Update issue number for SupportedDHKeys.java and UnsupportedDHKeys.java in ProblemList

2016-08-10 Thread Sean Mullan
Looks fine to me. Please add a noreg-trivial or noreg-cleanup label to the bug. --Sean On 08/09/2016 02:35 AM, John Jiang wrote: Hi, In ProblemList.txt, the tests sun/security/pkcs11/KeyAgreement/SupportedDHKeys.java and sun/security/pkcs11/KeyAgreement/UnsupportedDHKeys.java are tracked by

Re: RFR: 8061842: Package jurisdiction policy files as something other than JAR

2016-08-10 Thread Sean Mullan
Hi Brad, Looks pretty good. You should also send this to build-dev to review the Makefile changes. Just a few comments: - src/java.base/share/conf/security/policy/README.txt 17 contain no restrictions on cryptographic strengths, but they must s/must/must be/ 18 specifically activated by

<    3   4   5   6   7   8   9   10   11   12   >