Re: RFR: JDK-8209129 :Further improvements to cipher buffer management

2018-08-16 Thread Seán Coffey
was necessary as a result. regards, Sean. On 10/08/2018 15:18, Seán Coffey wrote: Thanks for the review Max - comments inline.. Regards, Sean. On 10/08/18 11:53, Weijun Wang wrote: HmacPKCS12PBESHA1.java:    76 byte[] passwdBytes = key.getEncoded();    77 if ((passwdBytes

Re: RFR: JDK-8209129 :Further improvements to cipher buffer management

2018-08-10 Thread Seán Coffey
l get a new webrev out once that's done. regards, Sean. Thanks Max On Aug 9, 2018, at 6:42 PM, Seán Coffey wrote: Adding RFR to title.. On 09/08/2018 11:37, Seán Coffey wrote: I've been looking further at how private/temporary buffers are used in cipher/keystore management and identified some mo

RFR: JDK-8209129 :Further improvements to cipher buffer management

2018-08-09 Thread Seán Coffey
Adding RFR to title.. On 09/08/2018 11:37, Seán Coffey wrote: I've been looking further at how private/temporary buffers are used in cipher/keystore management and identified some more areas that could benefit with a more aggressive nulling out of contents. I've been testing through use

JDK-8209129 :Further improvements to cipher buffer management

2018-08-09 Thread Seán Coffey
I've been looking further at how private/temporary buffers are used in cipher/keystore management and identified some more areas that could benefit with a more aggressive nulling out of contents. I've been testing through use of stepping through debugging sessions while setting/getting keys

Re: JDK-6782021

2018-08-08 Thread Seán Coffey
Vinnie is not working on security-libs any more and I think the JBS report should be marked as unassigned.  If any contributors want to suggest a patch, then I think it can be reviewed on this list! regards, Sean. On 07/08/2018 06:36, Oddbjørn Kvalsund wrote: Hi, I was just bit by this

RFR: jdk8u : 8208583: Better management of internal KeyStore buffers

2018-08-03 Thread Seán Coffey
Max, I've applied the same patch to jdk8u-dev. It's pretty much a straightforward port. Some small parts didn't apply cleanly since jdk8u doesn't involve use of Finalizer/Cleaner in some areas. i.e. around line 80 : src/share/classes/com/sun/crypto/provider/PBEKey.java and in constructor of

Re: RFR: 8208583: Better management of internal KeyStore buffers

2018-08-03 Thread Seán Coffey
to create a local variable "k" anymore. JavaKeyStore.java: 128 byte[] passwordBytes = null; No need to define so early and assigned null. No other comments. On Aug 3, 2018, at 12:37 AM, Seán Coffey wrote: All valid comments Max. Changes made. Webrev at http://cr.openj

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-03 Thread Seán Coffey
ready to push. Tony On Aug 2, 2018, at 5:15 AM, Seán Coffey wrote: Thanks Tony. I was asked off thread to add some comments to help code maintenance also. webrev updated. Hope we're nearly ready to push now. http://cr.openjdk.java.net/~coffeys/webrev.8207775.v4/webrev/ regards, Sean. On 01

Re: RFR: 8208583: Better management of internal KeyStore buffers

2018-08-02 Thread Seán Coffey
ug 2, 2018, at 7:38 PM, Seán Coffey wrote: No - no problem at all. Some extra exception handling but probably best for the long run. I wonder why DestroyedFailedException was a checked exception, what can we do if it's thrown? Thanks Max http://cr.openjdk.java.net/~coffeys/webrev.8208583

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-02 Thread Seán Coffey
On 08/01/2018 08:39 AM, Seán Coffey wrote: Thanks again for review Tony. I think you raise a good point and should give some performance gain. for line 676: maybe this :   byte[] copy = Arrays.copyOf(output, len);   if (decrypting

Re: RFR: 8208583: Better management of internal KeyStore buffers

2018-08-02 Thread Seán Coffey
No - no problem at all. Some extra exception handling but probably best for the long run. http://cr.openjdk.java.net/~coffeys/webrev.8208583.v3/webrev/index.html regards, Sean. On 02/08/2018 02:13, Weijun Wang wrote: 1. I wasn't able to rename to destroy since that method is reserved for

Re: RFR: 8208583: Better management of internal KeyStore buffers

2018-08-01 Thread Seán Coffey
. The PBEKeyCleanupTest testcase seems to confirm that. http://cr.openjdk.java.net/~coffeys/webrev.8208583.v2/webrev/index.html Regards, Sean. On 01/08/18 09:17, Seán Coffey wrote: Thanks for the comments Max. Comments inline.. On 01/08/2018 07:59, Weijun Wang wrote: Some comments: 1

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-01 Thread Seán Coffey
.net/~coffeys/webrev.8207775.v3/webrev/ regards, Sean. Tony On 07/27/2018 08:29 AM, Seán Coffey wrote: Thanks Tony. If it's alright with you, I'd like to make one more edit for this change. http://cr.openjdk.java.net/~coffeys/webrev.8207775.v2/webrev/ There's a condition where we can null

Re: RFR: 8208583: Better management of internal KeyStore buffers

2018-08-01 Thread Seán Coffey
::clearKey); Sure - I can modify that. regards, Sean. I have never been a lambda expert so forgive me if this is not correct. Thanks Max On Aug 1, 2018, at 3:11 AM, Seán Coffey wrote: Looking to improve management of internal buffers in KeyStore. The com.sun.crypto.provider.KeyProtector

RFR: 8208583: Better management of internal KeyStore buffers

2018-07-31 Thread Seán Coffey
Looking to improve management of internal buffers in KeyStore. The com.sun.crypto.provider.KeyProtector class uses the PBEKey class to protect some keys. Buffers can be freed up earlier if the caller takes responsibility for lifecycle of PBEKey object. (hence no reliance on Cleaner). Some

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-31 Thread Seán Coffey
.8207775.v3/webrev/ regards, Sean. Tony On 07/27/2018 08:29 AM, Seán Coffey wrote: Thanks Tony. If it's alright with you, I'd like to make one more edit for this change. http://cr.openjdk.java.net/~coffeys/webrev.8207775.v2/webrev/ There's a condition where we can null out an array early

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-27 Thread Seán Coffey
, Anthony Scarpino wrote: On 07/26/2018 07:36 AM, Seán Coffey wrote: https://bugs.openjdk.java.net/browse/JDK-8207775 Simple enough fix to null out some internal buffers once they're no longer required. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/ regards, Sean. that looks

RFR: 8207775: Better management of CipherCore buffers

2018-07-26 Thread Seán Coffey
https://bugs.openjdk.java.net/browse/JDK-8207775 Simple enough fix to null out some internal buffers once they're no longer required. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/ regards, Sean.

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-07-10 Thread Seán Coffey
ould handle Logger via its own framework/API in a future JDK release. I've removed the variable names using underscore. Also optimized some variable assignments in X509Impl.commitEvent(..) http://cr.openjdk.java.net/~coffeys/webrev.8148188.v5/webrev/ regards, Sean. On 09/07/2018 18:01, Seán Co

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-07-09 Thread Seán Coffey
Erik, Thanks for reviewing. Comments inline.. On 09/07/18 17:21, Erik Gahlin wrote: Thanks Sean. Some feedback on the code in the security libraries. - We should use camel case naming convention for variables (not underscore). Sure. I see two offending variable names which I'll fix up. -

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-07-09 Thread Seán Coffey
and modifications carried out to clean up the code up further. http://cr.openjdk.java.net/~coffeys/webrev.8148188.v5/webrev/ This enhancement has a dependency on JDK-8203629 Regards, Sean. On 02/07/18 09:49, Erik Gahlin wrote: On 29 Jun 2018, at 17:34, Seán Coffey wrote: I've introduced a new

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-29 Thread Seán Coffey
t;KiB" - not the normal when examining key lengths used in X509Certificates. i.e a 2048 bit key gets displayed as "2 KiB" - I'd prefer to keep the 2048 display version. new webrev at: http://cr.openjdk.java.net/~coffeys/webrev.8148188.v4/webrev/ Regards, Sean. On 28/06/18 17:5

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-28 Thread Seán Coffey
for the update Erik. By default I'm proposing that the new JFR Events and Logger be disabled. As a result the event class shouldn't escape. If performance metrics highlight an issue, we should revisit. regards, Sean. On 27/06/2018 20:57, Erik Gahlin wrote: On 2018-06-27 21:1

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-28 Thread Seán Coffey
new to record events.  I did not get the point to use both. I was initially hoping to concentrate on just JFR events but I got strong feedback to also consider use of Logger (esp. in cases where the jdk.jfr module is not available) regards, Sean. Thanks, Xuelei On 6/26/2018 3:18 PM, Seán Coffey

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-28 Thread Seán Coffey
the above code will create a new instance.  Is the return value of cce.isEnabled() dynamically changed or static? Is there a need to support both logging and JFR?  I'm new to record events.  I did not get the point to use both. Thanks, Xuelei On 6/26/2018 3:18 PM, Seán Coffey wrote: Erik

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-27 Thread Seán Coffey
/26/2018 3:18 PM, Seán Coffey wrote: Erik, I rebased the patch with TLS v1.3 integration today. I hadn't realized how much the handshaker code had changed. Hopefully, I'll get a review from security dev team on that front. Regards the JFR semantics, I believe the edits should match majority of

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-26 Thread Seán Coffey
log/record functionality. I might catch up with you tomorrow to see what the best arrangement would be. http://cr.openjdk.java.net/~coffeys/webrev.8148188.v2/webrev/ regards, Sean. On 25/06/2018 21:22, Seán Coffey wrote: Many thanks for the review comments Erik. Replies inline. On 24/06

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-25 Thread Seán Coffey
Many thanks for the review comments Erik. Replies inline. On 24/06/2018 14:21, Erik Gahlin wrote: Hi Sean, Some of the changes in the webrev belongs to JDK-8203629 and should be removed for clarity. Some initial comments: default.jfc, profile.jfr: The events should not have

RFR: 8148188: Enhance the security libraries to record events of interest

2018-06-22 Thread Seán Coffey
Following on from the recent JDK-8203629 code review, I'd like to propose enhancements on how we can record events in security libs. The introduction of the JFR libraries can give us much better ways of examining JDK actions. For the initial phase, I'm looking to enhance some key security

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-09 Thread Seán Coffey
Some comments on SSLLogger also : formatCaller() uses getStackTrace() to walk the stack. It's probably more expensive than using the newer Stackwalker class. Could it be replaced with something like : return StackWalker.getInstance().walk(s -> s.dropWhile((f ->

Re: [8u-dev] 8193171 : keytool -list displays "JKS" for a PKCS12 keystore

2018-05-01 Thread Seán Coffey
Looks fine to me. regards, Sean. On 30/04/2018 20:16, Ivan Gerasimov wrote: Hello! This is 8u-dev only bug fix. It was noticed that the keytool from the latest JDK 8 update release displays type of PKCS12 keystore as JKS. Would you please help review the patch? BUGURL:

Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

2018-03-14 Thread Seán Coffey
tml> On Wed, Mar 14, 2018 at 12:05 PM, Seán Coffey <sean.cof...@oracle.com <mailto:sean.cof...@oracle.com>> wrote: Hi Martin, Thanks for the 8195607 pointer. I'll get this ported to jdk8u also. I didn't see that actual issue during testing but no harm to port it.

Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

2018-03-14 Thread Seán Coffey
://mail.openjdk.java.net/pipermail/security-dev/2018-February/016776.html ? Kind regards, Martin.- On Wed, Mar 14, 2018 at 11:48 AM, Seán Coffey <sean.cof...@oracle.com <mailto:sean.cof...@oracle.com>> wrote: Looking to backport this fix to jdk8u-dev. Contributed to JDK Project by

RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

2018-03-14 Thread Seán Coffey
Looking to backport this fix to jdk8u-dev. Contributed to JDK Project by Martin Balao. https://bugs.openjdk.java.net/browse/JDK-8165996 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8165996.8u/webrev/ The test/jdk/sun/security/pkcs11/PKCS11Test.java edits didn't seem applicable to

Re: RFR: 8199018: Test crypto provider not registering

2018-03-06 Thread Seán Coffey
like it still needs to be done in jdk/jdk.  Are you planning on doing that? Thanks, Brad On 3/5/2018 6:37 AM, Seán Coffey wrote: Brad pointed out to me off-line that the test case from 8193892 needs correcting. Some late editing to the test broke functionality. I've corrected the testcase

RFR: 8199018: Test crypto provider not registering

2018-03-05 Thread Seán Coffey
Brad pointed out to me off-line that the test case from 8193892 needs correcting. Some late editing to the test broke functionality. I've corrected the testcase and added extra checks to ensure that "MyProvider" is being registered correctly. https://bugs.openjdk.java.net/browse/JDK-8199018

Re: RFR: 8193892: Impact of noncloneable MessageDigest implementation

2018-02-23 Thread Seán Coffey
}     public static final class SHA256 extends DigestBase {     public SHA256() throws Exception {     super("SHA-256", "SUN");     }     } } Thanks, sorry for the delay. Brad On 2/15/2018 7:40 AM, Xuelei Fan wrote: Looks fine to me.  Thanks! Xuelei On 2/15/

Re: [8u] [RFR] Request for Review of JDK-8196952: Bad primeCertainty value setting in DSAParameterGenerator

2018-02-22 Thread Seán Coffey
Thanks for looking at this one Andrew. I've had it on my to-do list. The patch looks fine. Regards, Sean. On 22/02/18 04:09, Andrew Hughes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8196952 Webrev: http://cr.openjdk.java.net/~andrew/openjdk8/8196952/webrev.01/ Review thread: N/A

Re: RFR 8197518: Kerberos krb5 authentication: AuthList's put method leads to performance issue

2018-02-22 Thread Seán Coffey
This looks good to me Max. I think it'll solve the issue reported. line 109 : I guess this could turn negative if timeLimit was value < 60. I don't think that's possible unless we're dealing with a strange config! For the code comment, may a minor edit : 111 // Only trigger a

Re: RFR: 8193892: Impact of noncloneable MessageDigest implementation

2018-02-15 Thread Seán Coffey
A reminder for this review.. regards, Sean. On 09/02/2018 16:25, Seán Coffey wrote: Looking to push a new test which helps test CloneableDigest code. It's something that arose during JDK-8193683 fix. The test was contributed by Brad Wetmore. I've converted it to use the SSLSocketTemplate

RFR: 8193892: Impact of noncloneable MessageDigest implementation

2018-02-09 Thread Seán Coffey
Looking to push a new test which helps test CloneableDigest code. It's something that arose during JDK-8193683 fix. The test was contributed by Brad Wetmore. I've converted it to use the SSLSocketTemplate. JBS : https://bugs.openjdk.java.net/browse/JDK-8193892 webrev :

Re: [8u-dev] Request to Review and for Approval to Backport 8192987 : keytool should remember real storetype if it is not provided

2018-01-03 Thread Seán Coffey
Looks fine. Approved for jdk8u-dev. regards, Sean. On 03/01/2018 02:44, Weijun Wang wrote: The code change looks fine to me. Thanks Max On Jan 3, 2018, at 9:16 AM, Ivan Gerasimov wrote: Hello! I'd like to backport this fix to JDK 8u-dev. The patch did NOT

Re: RFR: 8185855: Debug exception stacks should be clearer

2017-12-06 Thread Seán Coffey
StackTrace(); } -e.printStackTrace(); } return null; } Regards, Sean. On 06/12/17 10:20, Seán Coffey wrote: Thanks for reviewing Sean. Not finding the cacerts file has always resulted in the stacktrace going to stderr in the pas

Re: RFR: 8185855: Debug exception stacks should be clearer

2017-12-06 Thread Seán Coffey
ror reading policy " + e); e.printStackTrace(); } // ignore that policy Regards, Sean. On 05/12/17 20:41, Sean Mullan wrote: On 12/5/17 9:27 AM, Seán Coffey wrote: Looking to improve the stacktrace output made when debug mode is enabled for java.security and sun.secur

RFR: 8185855: Debug exception stacks should be clearer

2017-12-05 Thread Seán Coffey
Looking to improve the stacktrace output made when debug mode is enabled for java.security and sun.security classes. In the past, some of these have led to confusion for end users. Best to add some context when we're printing stacktrace for informational, debug, purposes.

Re: Tls 1.2 support info

2017-12-04 Thread Seán Coffey
Information about the defaults used in the Oracle JDK 6u/7u releases can be found in the JDK Crypto roadmap : https://www.java.com/en/jre-jdk-cryptoroadmap.html Regards, Sean. On 30/11/17 16:53, rgamarra wrote: Thank you for your response Thomas. I'm connecting to a backend that will start

Re: RFR - 8189646: sun/security/ssl/SSLSocketImpl/SSLSocketCloseHang.java failed with "java.net.SocketTimeoutException: Read timed out"

2017-11-20 Thread Seán Coffey
This should help harden the test. Looks fine to me. Regards, Sean. On 20/11/17 16:16, Rob McKenna wrote: Hi folks, This test appears to be timing out. The main culprit looks to be the fact that the 100ms sleep isn't enough for the server to respond (on a busy test machine) so that timeout has

Re: [8u-dev] RFR 8190690: Impact on krb5 test cases in the 8u-CPU nightly

2017-11-13 Thread Seán Coffey
Looks like some test clients can have old configuration files which trigger early loading of InetAddress. Your edits look fine and hardens the testcase against such issues. regards, Sean. On 09/11/2017 09:04, Weijun Wang wrote: Please review the fix at

Re: enable TLS_RSA_WITH_AES_256_CBC_SHA256 in openJDK(build 1.8.0_121-b13)

2017-09-28 Thread Seán Coffey
Check out the Oracle 8u152 early access binaries[1] also. There's a new security property to enable unlimited crypto. No more policy file downloads required! You can simply edit the jre/lib/java.security file to set "crypto.policy" to "unlimited" [2] 8u152 GA had a tentative release date for

Re: RFR: 8170157, 8170245: Enable unlimited cryptographic policy by default in OracleJDK

2017-09-15 Thread Seán Coffey
:04 PM, Seán Coffey wrote: Some modifications to the java.security file(s). Final webrev, I hope : http://cr.openjdk.java.net/~coffeys/webrev.8170157.8u.02/webrev/ regards, Sean. On 01/09/2017 16:04, Seán Coffey wrote: comments inline. On 29/08/17 23:33, Bradford Wetmore wrote: Very minor

Re: RFR: 8170157, 8170245: Enable unlimited cryptographic policy by default in OracleJDK

2017-09-14 Thread Seán Coffey
Some modifications to the java.security file(s). Final webrev, I hope : http://cr.openjdk.java.net/~coffeys/webrev.8170157.8u.02/webrev/ regards, Sean. On 01/09/2017 16:04, Seán Coffey wrote: comments inline. On 29/08/17 23:33, Bradford Wetmore wrote: Very minor comments/tweaks. On 8/18

Re: [10] RFR 8186654 : Poor quality of sun.security.util.Cache.EqualByteArray.hashCode()

2017-09-07 Thread Seán Coffey
Looks good to me Ivan. regards, Sean. On 23/08/2017 21:54, Ivan Gerasimov wrote: Hello! An auxiliary class EqualByteArray implements hashCode() in a way that small changes to the content do not change the hash value. It is proposed to reuse Arrays.hashCode(byte[]) for the hash code

Re: RFR: 8170157, 8170245: Enable unlimited cryptographic policy by default in OracleJDK

2017-09-01 Thread Seán Coffey
comments inline. On 29/08/17 23:33, Bradford Wetmore wrote: Very minor comments/tweaks. On 8/18/2017 7:01 AM, Seán Coffey wrote: Looking to backport 8170157 to jdk8u-dev. The 8170245 test bug also gets pulled in for this port since some tests need cleaning up to deal with unlimited crypto

Re: RFR : 8159035: com/sun/crypto/provider/Cipher/CTS/CTSMode.java test crashed due to unhandled case of cipher length value as 0

2017-08-25 Thread Seán Coffey
ards, Sean. On 16/08/17 13:14, Seán Coffey wrote: Looking to backport this fix to jdk8u-dev. The bug report is not public but the JDK 9 review thread captures the details. The fix is similar. Some extra intrinsics work in jdk9 wasn't applicable for the jdk8u-dev port. http://mail.openjdk.

RFR: 8170157, 8170245: Enable unlimited cryptographic policy by default in OracleJDK

2017-08-18 Thread Seán Coffey
Looking to backport 8170157 to jdk8u-dev. The 8170245 test bug also gets pulled in for this port since some tests need cleaning up to deal with unlimited crypto environment. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8170157.8u.01/webrev/index.html -- Regards, Sean.

RFR : 8159035: com/sun/crypto/provider/Cipher/CTS/CTSMode.java test crashed due to unhandled case of cipher length value as 0

2017-08-16 Thread Seán Coffey
Looking to backport this fix to jdk8u-dev. The bug report is not public but the JDK 9 review thread captures the details. The fix is similar. Some extra intrinsics work in jdk9 wasn't applicable for the jdk8u-dev port.

Re: [RFR] 8174849: Change SHA1 certpath restrictions - issue with 3rd party JCE provider

2017-07-14 Thread Seán Coffey
Tony, I think we should log a JDK 8u bug for this issue if one doesn't already exist. If the buggy SigAlgName was allowed in 8u updates already, then it should be continued to be allowed for compatibility reasons IMO. There might be time to revert that change in 8u152. For 9, then maybe we

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Seán Coffey
you should always get a useful string. At the moment (with SunEC AlgorithmParameters), the string prints the friendly name followed by the OID: Unsupported curve: brainpoolP256r1 (1.3.36.3.3.2.8.1.1.7) On 7/7/2017 4:12 PM, Seán Coffey wrote: Adam, would it be useful to get the curve name

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-07 Thread Seán Coffey
Adam, would it be useful to get the curve name in the new exception ? I think it would help with future debugging. Line 96 already gets the curve name if we're dealing with ECGenParameterSpec instance. I think the same approach could be applied to your new code. Regards, Sean. On

Re: Support for CFRG (curve25519 etc) in Java/JCE

2017-06-21 Thread Seán Coffey
This appears to be tracked via https://bugs.openjdk.java.net/browse/JDK-8171277 Regards, Sean. On 20/06/17 21:32, Anders Rundgren wrote: Hi List, I'm an long time user of Java and JCE. I've just begun looking into the recently standardized curve25519 crypto. Since I didn't find any JEP or

Re: How do I know which granted permission is not needed?

2017-06-21 Thread Seán Coffey
you're mostly likely aware of this debug option but the java.security.debug option allows 'access' which should give you alot more information about each permission check that's been made. Maybe it's a case of scanning the output for permissions not checked and seeing if they're really

Re: RFR 8181975: Run sun/security/pkcs11 tests on Mac

2017-06-21 Thread Seán Coffey
Looks fine to me. Regards, Sean. On 21/06/17 12:27, Bhanu Gopularam wrote: Hi all, Please review fix for following test bug: Bug Id - https://bugs.openjdk.java.net/browse/JDK-8181975 In test/sun/security/pkcs11/PKCS11Test.java updated path for nss-libs on MacOSX platform. Webrev -

Re: RFR : 8181205:JRE fails to load/register security providers when started from UNC pathname

2017-06-06 Thread Seán Coffey
/lib/ext/access-bridge-32.jar"); The Paths.get(URI) method handles the UNC path in the correct fashion. I'll add this comment to the code : // Use the Paths.get(uri) call in order to handle UNC based file name conversion correctly. regards, Sean. Thanks, Xuelei On 6/6/2017 1:40 AM, Seán

Re: RFR : 8181205:JRE fails to load/register security providers when started from UNC pathname

2017-06-06 Thread Seán Coffey
ping. Can I get a review for this please ? regards, Sean. On 01/06/2017 17:23, Seán Coffey wrote: The recent JDK-8163528 fix caused a regression for JDK binaries launched with a UNC pathname. We can use the Paths class to create the required File. I managed to put a test together which

RFR : 8181205:JRE fails to load/register security providers when started from UNC pathname

2017-06-01 Thread Seán Coffey
The recent JDK-8163528 fix caused a regression for JDK binaries launched with a UNC pathname. We can use the Paths class to create the required File. I managed to put a test together which should test this code path. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8181205/webrev/ JBS

Re: [jdk8u-dev] Review Request and Approval to Backport: 8140436: Support the FFDHE TLS extension

2017-05-22 Thread Seán Coffey
Ivan, this is an enhancement backport request for jdk8u. As a result, it's subject to extra steps before push approval can be considered. See Rule 3 : http://openjdk.java.net/projects/jdk8u/groundrules.html regards, Sean. On 19/05/2017 17:25, Ivan Gerasimov wrote: Hello! The backport

Re: RFR[8u] JDK-8157035: Use stronger algorithms and keys for JSSE testing

2017-04-10 Thread Seán Coffey
Looks good to me Prasad. Thanks for fixing this. Regards, Sean. On 10/04/17 07:02, Prasadrao Koppula wrote: Hi, Please review this patch for “JDK-8157035: Use stronger algorithms and keys for JSSE testing” Issue: https://bugs.openjdk.java.net/browse/JDK-8157035 Webrev:

Re: RFR 8171319: keytool should print out warnings when reading or generating cert/cert req using weak algorithms

2017-03-07 Thread Seán Coffey
/webrev.02/ Changes: 1. Combined printWeakWarningsWithoutNewLine() and printWeakWarnings() to one method. 2. New X509CRLImpl.toStringWithAlgName(String name). Nothing more. Thanks Max On 02/15/2017 11:04 PM, Seán Coffey wrote: Hi Weijun, That's looks good to me and will be a big help

Re: RFR 8171319: keytool should print out warnings when reading or generating cert/cert req using weak algorithms

2017-02-23 Thread Seán Coffey
(String name). Nothing more. Thanks Max On 02/15/2017 11:04 PM, Seán Coffey wrote: Hi Weijun, That's looks good to me and will be a big help for keytool usability. some thoughts : Main.java : in your printCRL method, would you consider editing the X509CRLImpl class to print with a customized

Re: RFR 8171319: keytool should print out warnings when reading or generating cert/cert req using weak algorithms

2017-02-15 Thread Seán Coffey
Hi Weijun, That's looks good to me and will be a big help for keytool usability. some thoughts : Main.java : in your printCRL method, would you consider editing the X509CRLImpl class to print with a customized string ? It'll make the code more resilient to future changes in this area i.e.

Re: TlsRsaPremasterSecretParameterSpec

2017-02-09 Thread Seán Coffey
Jonathan wrote: Hi Sean, I am using 8u121 and I have raised a bug at http://bugreport.java.com/. I haven’t received any response but the internal review ID was: 9047607. Regards, Jonathan Patchell Senior Software Developer Gemalto *From:*Seán Coffey [mailto:sean.cof...@oracle.com] *Sent

Re: TlsRsaPremasterSecretParameterSpec

2017-02-08 Thread Seán Coffey
What version of JDK 8u are you running with ? There's been a few tweaks in this code area which might help you. https://bugs.openjdk.java.net/browse/JDK-8149017 https://bugs.openjdk.java.net/browse/JDK-8158111 If you can reproduce with 8u121, please log an issue via http://bugreport.java.com/

Re: RFR : 8173783: IllegalArgumentException: jdk.tls.namedGroups

2017-02-07 Thread Seán Coffey
"no available elliptic curves." + (property.exists() ? " " + property : "") where [jdk.tls.namedGroups|default] is the branch you took to arrive at the list value. Brad On 2/7/2017 7:25 AM, Seán Coffey wrote: The recent JDK-8148516 enhancement causes issue for J

RFR : 8173783: IllegalArgumentException: jdk.tls.namedGroups

2017-02-07 Thread Seán Coffey
The recent JDK-8148516 enhancement causes issue for JDKs without EC support. It's primarily an issue for JDK 6u which doesn't have SunEC but this still needs to be fixed in all release families. bug report : https://bugs.openjdk.java.net/browse/JDK-8173783 webrev :

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-31 Thread Seán Coffey
. thanks Tony On 01/30/2017 03:31 AM, Seán Coffey wrote: src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java CertPathValidatorException is caught 3 times in new code but we're not printing out the exact algorithm that caused the exception. AFAIK, that should be in the excepti

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-30 Thread Seán Coffey
src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java CertPathValidatorException is caught 3 times in new code but we're not printing out the exact algorithm that caused the exception. AFAIK, that should be in the exception message. Would it be possible to use something

Re: Code Review Request, JDK-8173066 More verbose debug output for selection of X509 certs

2017-01-19 Thread Seán Coffey
Looks good. Thanks for the quick turn around. regards, Sean. On 19/01/2017 17:37, Xuelei Fan wrote: Hi Sean, Would you please review this debug log update for JSSE key manager implementation: http://cr.openjdk.java.net/~xuelei/8173066/webrev.00/ Trivial update, no new regression test.

Re: [8u] RFA for backport of JDK-8157665: ProblemList.txt needs to be updated as 7041639 closed

2017-01-04 Thread Seán Coffey
The changes look fine. Please ensure that the tests pass on JPRT solaris platforms before pushing. Approved. Regards, Sean. On 04/01/17 07:02, Nikita Jain wrote: Sorry, I made a mistake. The trivial backport requires a code review. JDK8 webrev link is mentioned in previous mail. The

Re: [9] RFR:8168769: javax/net/ssl/TLSv12/DisabledShortRSAKeys.java timed out

2016-12-22 Thread Seán Coffey
Looks good. regards, Sean. On 22/12/2016 09:20, Tim Du wrote: Hi All: Would like help to review the code change for javax/net/ssl/TLSv12/DisabledShortRSAKeys.java? The test case failed with synchronization test issue between client and server intermittently, use advantage of

Re: An Unexpected I/O exception with AsyncHttpClient while performing SSL requests

2016-12-22 Thread Seán Coffey
Your JDK version is quite old. Try updating to the latest JDK 8u release. this might be a factor and was fixed in 8u51. http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/445debb5c61d Sean. On 21/12/2016 19:43, Shlomi Abramoviz wrote: Hi everyone, I was referred to this group and I hope this is

Re: RFR: 8170157/8169335: Unlimited Cryptography Policy Changes

2016-12-05 Thread Seán Coffey
looks good. You'll need to run the new CryptoPolicyFallback.java testcase in othervm mode. Regards, Sean. On 02/12/16 23:50, Bradford Wetmore wrote: Hi, I need reviewers for these related bugs: https://bugs.openjdk.java.net/browse/JDK-8170157 Enable unlimited cryptographic policy by

Re: [8u-dev] RFR 8170278 : ticket renewal won't happen with debugging turned on

2016-11-29 Thread Seán Coffey
Looks good. regards, Sean. On 29/11/2016 09:08, Ivan Gerasimov wrote: Hello! It was reported that there's broken behavior exposed when the debug mode is turned on, which is due to KerberosTicket.toString() throwing an exception, if the ticket was already destroyed. The proposed fix is in

Re: RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

2016-11-17 Thread Seán Coffey
On 17/11/2016 17:46, Bradford Wetmore wrote: On 11/17/2016 7:19 AM, Seán Coffey wrote: The /policy= jtreg tag was also another possible solution. That's a policy file, not a java.security file. Ah, of course. Wasn't thinking right. regards, Sean. SeanM pointed out that we could do

Re: RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

2016-11-17 Thread Seán Coffey
out = new PrintWriter(FILENAME)) { Files.lines(path) .filter(x -> !x.trim().startsWith("crypto.policy")) .forEach(out::println); } Latest at: http://cr.openjdk.java.net/~wetmore/8169335/webrev.02 Brad --Max 4.

Re: RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

2016-11-16 Thread Seán Coffey
In the recent jdk8u-dev edits of this file for 8157561, we introduced a debug field based on this key : Debug.getInstance("jca", "Cipher"); Can we continue to use 'jca' to be consistent for people upgrading ? for the testcase, I guess you can edit

RFR : (XS) 8166432:Bad 8u112 merge of sun/security/tools/jarsigner/warnings/Test.java

2016-11-09 Thread Seán Coffey
A bad test code merge occurred when 8u112 and CPU code was being merged a few weeks ago. Some extra functionality added to a helper test was lost. This fix restores it. 8u applicable only. diff --git a/test/sun/security/tools/jarsigner/warnings/Test.java

Re: RFR: 8157561 :Ship the unlimited policy files in JDK Updates

2016-11-07 Thread Seán Coffey
on problem. Looks ok otherwise. Thanks, Brad On 11/4/2016 7:16 AM, Erik Joelsson wrote: Build changes look ok to me. /Erik On 2016-11-04 14:42, Seán Coffey wrote: Looking to push this enhancement to jdk8u. The change introduces the new Security property which was brought into JDK 9 via J

RFR: 8157561 :Ship the unlimited policy files in JDK Updates

2016-11-04 Thread Seán Coffey
Looking to push this enhancement to jdk8u. The change introduces the new Security property which was brought into JDK 9 via JDK-8061842. The code differs in that jar files continue to be used and backwards compatibility is maintained. If the new security property is not defined and the policy

Re: [8u-dev] Request for Review & Approval - 8163304: jarsigner -verbose -verify should print the algorithms used to sign the jar

2016-10-20 Thread Seán Coffey
Approved. Regards, Sean. On 21 October 2016 03:38:20 GMT+01:00, Wang Weijun wrote: > > >On Oct 21, 2016, at 10:31 AM, Rob McKenna >wrote: > >Hi folks, > >Looking for a codereview and push approval for the following: > >bug:

Re: [8u-dev] Request for Review and Approval to backport: 8167591: Add MD5 to signed JAR restrictions

2016-10-20 Thread Seán Coffey
Approved. Regards, Sean. On 20/10/2016 22:06, Anthony Scarpino wrote: On 10/20/2016 11:15 AM, Ivan Gerasimov wrote: Hello! Would you please help review and give the approval to backport this fix? The changes in the backport, comparing to the fix in 9, are due to different file structure and

Re: [9]RFR 8136355: CKM_SSL3_KEY_AND_MAC_DERIVE no longer available by default on Solaris 12

2016-09-22 Thread Seán Coffey
has to be removed now that the version check has been >corrected. >http://cr.openjdk.java.net/~valeriep/8136355/webrev.02/ > >Thanks, >Valerie > >On 9/21/2016 10:49 AM, Seán Coffey wrote: >> Hey Valerie, >> >> There are a few calls in this code where an except

Re: [9]RFR 8136355: CKM_SSL3_KEY_AND_MAC_DERIVE no longer available by default on Solaris 12

2016-09-21 Thread Seán Coffey
Hey Valerie, There are a few calls in this code where an exception is thrown if a bad version is received. It's code that already existed, but would you mind enhancing the exceptions to print the version while editing the code there ? e.g. P11TlsKeyMaterialGenerator.java + throw

RFR: 8164846: CertificateException missing cause of underlying exception

2016-08-31 Thread Seán Coffey
bug ID : https://bugs.openjdk.java.net/browse/JDK-8164846 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8164846.jdk9/webrev/ A CertificateException in SSLContextImpl doesn't include information about the cause of the exception. This can often contain a valuable snippet of information

Re: RFR: 8150530:Improve javax.crypto.BadPaddingException messages

2016-08-24 Thread Seán Coffey
"is used during decryption."); Regards, Sean. On 24/08/16 15:55, Xuelei Fan wrote: On 8/24/2016 7:02 PM, Seán Coffey wrote: On 22/08/16 11:06, Xuelei Fan wrote: Minor comments: CipherCore.java --- "... could arise if a bad key or password is used during decry

Re: RFR: 8150530:Improve javax.crypto.BadPaddingException messages

2016-08-24 Thread Seán Coffey
934: "Insufficient buffer for AEAD cipher fragment, needs more than (recordIvSize + tagSize) bytes, but only (bb.remaining()) remains in the buffer" P11RSACipher.java - 360: "The output buffer (outLen bytes) is too small to hold the produced data (tmpBuff

RFR: 8150530:Improve javax.crypto.BadPaddingException messages

2016-08-22 Thread Seán Coffey
Looking to improve some of the messages used in generation of BadPaddingException messages. The 'Given final block not properly padded' one in particular has caused confusion for some users in the past. JBS report : https://bugs.openjdk.java.net/browse/JDK-8150530 webrev :

RFR : 8162362: Introduce system property to control enabled ciphersuites

2016-08-21 Thread Seán Coffey
I'd like to backport this to jdk8u-dev. The new jdk.tls.client.cipherSuites and jdk.tls.server.cipherSuites system properties will allows users to control what ciphersuites are enabled on SSL Sockets/Engines. One extra edit for jdk8u in SSLEngineImpl/SSLSocketImpl where ciphersuites was not

Re: RFR: (XS) 8162916:Test sun/security/krb5/auto/UnboundSSL.java fails

2016-08-18 Thread Seán Coffey
Thanks for the tip Artem, Max. No need to modify the policy file then. Below is the new suggested patch for jdk8u-dev. JPRT results are good. diff --git a/test/sun/security/krb5/auto/UnboundSSL.java b/test/sun/security/krb5/auto/UnboundSSL.java ---

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

2016-08-18 Thread Seán Coffey
Hi Brad, nice to have this going in. Some comments. 1. Bug synopsis, can you edit it perhaps. "Introduce security property to control strong crypto" seems more descriptive. 2. Exception handling. Alot of your new exceptions don't include context. That makes debugging more difficult than

Re: [8u-dev] Request for Approval: Backport of 8144566: Custom HostnameVerifier disables SNI extension

2016-08-18 Thread Seán Coffey
Changes look good Ramanand. Reviewed. Regards, Sean. On 18/08/2016 07:03, David Buck wrote: Hi Ramanand! As there are (minor) changes between the two change sets, you will need to get a code review of the backported changes. I have included the security-dev alias in the CC list. Cheers,

<    1   2   3   >