Re: RFR (12): 8215318: Amend the Standard Algorithm Names specification to clarify that names can be defined in later versions

2019-01-02 Thread Anthony Scarpino
Looks fine Tony On 1/2/19 12:42 PM, Sean Mullan wrote: Please review this change to the Java Security Standard Algorithm Names specification [1] to clarify that standard names that are defined in later versions of SE are also supported in prior versions, as long as the applicable Security

Re: Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style

2018-12-18 Thread Anthony Scarpino
On 12/17/18 5:26 PM, Xue-Lei Fan wrote: On 12/17/2018 1:17 PM, Anthony Scarpino wrote: It looks like in TransportContext.java:68, you had a mistype that added "fa" to the end of a comment. Oops, I will update it. Also in fatal():267, did you plan to return the exceptio

Re: Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style

2018-12-17 Thread Anthony Scarpino
It looks like in TransportContext.java:68, you had a mistype that added "fa" to the end of a comment. Also in fatal():267, did you plan to return the exception and have the calling method throw the exception? As is, the exception is never return and fatal() continues to throw the exceptions.

Re: Code Review Request, JDK-8214339 : SSLSocketImpl erroneously wraps SocketException

2018-12-15 Thread Anthony Scarpino
Just the complete the thread. Says "fatal() throws exception" is fine. If this all only trying to make the compiler happy, you could just have one "return null" at the bottom of the method. It is not necessary to put a return after each fatal(). I will admit it could be less readable. Or

Re: Code Review Request, JDK-8213782: NullPointerException in sun.security.ssl.OutputRecord.changeWriteCiphers

2018-12-14 Thread Anthony Scarpino
Other than my nit about the “make the compiler happy”, this all looks fine. For KeyUpdate, shouldn’t it never be null given the suite and protocol are already known good? I have not problem with the check to be cautious even if it should never happen. Tony > On Dec 14, 2018, at 9:00 AM,

Re: RFR(S)JDK-8214074: Ghash optimization using AVX instructions

2018-12-07 Thread Anthony Scarpino
://cr.openjdk.java.net/~svkamath/ghash/webrev02/ Thank you very much for your time and assistance. Please let me know if you have any questions. Regards, Smita -Original Message- From: Anthony Scarpino [mailto:anthony.scarp...@oracle.com] Sent: Wednesday, December 5, 2018 10:56 AM To: Kamath

Re: RFR 8208698: Improved ECC Implementation

2018-12-06 Thread Anthony Scarpino
On 11/30/18 12:01 PM, Adam Petcher wrote: JBS: https://bugs.openjdk.java.net/browse/JDK-8208698 webrev: http://cr.openjdk.java.net/~apetcher/8208698/webrev.00/ This is a re-implementation of ECDH and ECDSA that is designed to be resilient against side-channel attacks. The implementation only

Re: RFR(S)JDK-8214074: Ghash optimization using AVX instructions

2018-12-06 Thread Anthony Scarpino
attached. JBS link: https://bugs.openjdk.java.net/browse/JDK-8214074 Webrev link: http://cr.openjdk.java.net/~svkamath/ghash/webrev02/ Thank you very much for your time and assistance. Please let me know if you have any questions. Regards, Smita -Original Message- From: Anthony Scarpino

Re: RFR(S)JDK-8214074: Ghash optimization using AVX instructions

2018-12-05 Thread Anthony Scarpino
it is roughly equal. At 16k is the best performance increase from 60k ops/sec to 130 ops/sec Are the AVX instructions slower to setup and therefore affect smaller data sizes? Or maybe the larger array allocation in GHASH.java? Tony On 11/29/18 12:08 PM, Anthony Scarpino wrote: [removed core

8214098: sun.security.ssl.HandshakeHash.T12HandshakeHash constructor check backwards.

2018-12-04 Thread Anthony Scarpino
Hi, I need a code review for the TLS 1.2 check being backwards for HandshakeHash, which showed up with a clonable MessageDigest object. Since TLS 1.3 does not use the same scheme as 1.2, I removed the unnecessary code. http://cr.openjdk.java.net/~ascarpino/8214098/webrev/ Tony

Re: AES ctr benchmark performance

2018-12-03 Thread Anthony Scarpino
of curiosity how do the times look like when you switch NI off? Greetings Bernd -- http://bernd.eckenfels.net *Von: *Anthony Scarpino <mailto:anthony.scarp...@oracle.com> *Gesendet: *Montag, 3. Dezember 2018 21:13 *An: *Kasper Janssens <mailto:kasper.janss...@wdc.com>;

Re: AES ctr benchmark performance

2018-12-03 Thread Anthony Scarpino
Hi, That is how I read it, but when I've done the test it's closer to 55%. But openssl speed and jmh are not comparable. Openssl is only running through it's algorithm, while jmh is running through the VM and java byte code. jmh has to do more work outside of pure crypto ops than openssl.

Re: RFR(S)JDK-8214074: Ghash optimization using AVX instructions

2018-11-29 Thread Anthony Scarpino
. Bug link: https://bugs.openjdk.java.net/browse/JDK-8214074 Webrev link: http://cr.openjdk.java.net/~svkamath/ghash/webrev01/ Thanks and Regards, Smita -Original Message- From: Anthony Scarpino [mailto:anthony.scarp...@oracle.com] Sent: Tuesday, November 20, 2018 2:05 PM To: Kamath, Smita

Re: Code Review Request, JDK-8214321: Misleading code in SSLCipher

2018-11-26 Thread Anthony Scarpino
On 11/26/18 4:14 PM, Xue-Lei Fan wrote: Hi, Please review this code cleanup in SSLCipher.java:    http://cr.openjdk.java.net/~xuelei/8214321/webrev.00/ The code should be fine as readCipherGenerators.length and writeCipherGenerators.length are the same value in the implementation. However,

Re: Problems with AES-GCM native acceleration

2018-11-14 Thread Anthony Scarpino
I agree with Adam that this is more of a tuning issue and not a problem with the crypto. Sending multiple updates is a hack. I've been aware of this bug for a while and I do not understand why this is a significant problem. The stackoverflow comments say it takes 50 seconds to trigger the

RFR[s] 8211339: NPE during SSL handshake caused by HostnameChecker

2018-11-05 Thread Anthony Scarpino
Hi, I'd like to get a code review of this simple change. It's a simple check to make sure the hostname variable is not null and throw a proper exception. http://cr.openjdk.java.net/~ascarpino/8211339/webrev.00/ Tony

Re: Code Review Request, JDK-8212738, Incorrectly named signature scheme ecdsa_secp512r1_sha512

2018-10-29 Thread Anthony Scarpino
Looks good to me Tony On 10/29/2018 10:41 AM, Xuelei Fan wrote: Hi, Please review the update:     http://cr.openjdk.java.net/~xuelei/8212738/webrev.00/ The signature algorithm name should be ""ecdsa_secp521r1_sha512", instead of "ecdsa_secp512r1_sha512". No new regression test.  Trivial

RFR: 8211426: SSL handshake succeeds under JDK 9 (and earlier) but not under JDK 11

2018-10-14 Thread Anthony Scarpino
I’d like a review of this fix for when DSA is the only key available. It’s debatable how realistic this situation is, but it is a regression and key tool uses dsa by default. The fix is to remove tls1.3 from the default protocols. The placement of the code change is to minimize the keymanager

Re: RFR: 8209862:CipherCore performance improvement

2018-10-12 Thread Anthony Scarpino
I'm ok with the changes. Tony On 10/10/2018 09:26 AM, Seán Coffey wrote: Thanks for the review Adam. I've corrected those style issues. Now waiting on 2nd Reviewer. Regards, Sean. On 08/10/18 19:18, Adam Petcher wrote: The organization is better now, thanks. The code looks good to me, but

Re: DSA default algorithm for keytool -genkeypair. Bad choice?

2018-10-10 Thread Anthony Scarpino
don't know what benefit it brings to a user to remove the default. Except from forcing DSA users to add a -keyalg option, RSA and EC users will not gain anything. --Max On Oct 11, 2018, at 5:05 AM, Anthony Scarpino wrote: On 10/10/2018 07:42 AM, Weijun Wang wrote: On Oct 10, 2018, at 7:59 PM

Re: DSA default algorithm for keytool -genkeypair. Bad choice?

2018-10-10 Thread Anthony Scarpino
On 10/10/2018 07:42 AM, Weijun Wang wrote: On Oct 10, 2018, at 7:59 PM, Sean Mullan wrote: There is really no other reason other than DSA keys have been the default keypairs generated by keytool for a long time, so there are some compatibility issues we would have to think through before

Re: RFR: 8209862:CipherCore performance improvement

2018-10-01 Thread Anthony Scarpino
On 10/01/2018 06:11 AM, Seán Coffey wrote: JDK-8207775 introduced some performance regressions in the ciphercore area. Sergey Kuksenko has been looking at this and has contributed the following patch: http://cr.openjdk.java.net/~skuksenko/crypto/8209862/ bug report :

Re: RFR(s): 8208641: SSLSocket should throw an exception when configuring DTLS

2018-09-17 Thread Anthony Scarpino
Thanks.. I updated the copyright.. Tony On 08/29/2018 07:02 AM, Xuelei Fan wrote: Looks fine to me.   Please update the copyright years as well. Thanks, Xuelei On 8/28/2018 9:47 PM, Anthony Scarpino wrote: I need a review of this fix.  Simple change to throw an UnsupportedOperationException

Re: Conceptual feedback on new ECC JEP

2018-09-07 Thread Anthony Scarpino
Adam, I tend to agree with Mike that disallowing import/export of keys using BigInteger is not the value of a branchless implementation. As you point out in the JEP the provider is greatly hindered by this design choice. I feel it would be better to implementing the BigInteger parts and have

Re: Code Review Request, JDK-8210334, TLS 1.3 server fails if ClientHello doesn't have pre_shared_key and psk_key_exchange_modes

2018-09-05 Thread Anthony Scarpino
Looks fine > On Sep 5, 2018, at 11:01 AM, Xuelei Fan wrote: > > Hi, > > Please review: >http://cr.openjdk.java.net/~xuelei/8210334/webrev.00/ > > Simple update, no new regression test. > > Thanks, > Xuelei

Re: Code Review Request JDK-8209965 : The "supported_groups" extension in ServerHellos

2018-08-27 Thread Anthony Scarpino
ded to wait and check for the supported_versions extension if it > presents. As may make the workaround a lot complicated. I would prefer to a > simple change for now. > > Thanks, > Xuelei > >> On 8/26/2018 2:35 PM, Anthony Scarpino wrote: >> The change looks fine but I h

Re: Code Review Request JDK-8209965 : The "supported_groups" extension in ServerHellos

2018-08-26 Thread Anthony Scarpino
The change looks fine but I have a question about restricting it. This sounds like a problem with servers using 1.2 or before, does it make sense to throw an error for 1.3? I don’t like allowing buggy implementation to continue because we will never be able to undo this workaround. It would

Re: RFR11(s): 8207317: SSLEngine negotiation fail exception behavior changed from fail-fast to fail-lazy

2018-08-20 Thread Anthony Scarpino
On 08/20/2018 01:33 PM, Bradford Wetmore wrote: Hi Xuelei, Please review this P1 bug blocking JDK11 RC:     https://bugs.openjdk.java.net/browse/JDK-8207317     http://cr.openjdk.java.net/~wetmore/8207317/webrev.00/ Proposed putback comment is inlined in the webrev. Bug analysis/fix

Re: RFR - 8203614: Java API SSLEngine example code needs correction

2018-08-17 Thread Anthony Scarpino
Looks fine. Tony > On Aug 17, 2018, at 8:45 AM, Jamil Nimeh wrote: > > Hello all, > > This is a simple doc-only one-liner that fixes the sample code in SSLEngine > to use the correct ByteBuffer in the capacity size check. > > webrev:

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-14 Thread Anthony Scarpino
On 08/14/2018 11:27 AM, Sean Mullan wrote: On 8/14/18 1:56 PM, Anthony Scarpino wrote: On 08/13/2018 12:42 PM, Sean Mullan wrote: On 8/10/18 3:49 PM, Anthony Scarpino wrote: On 8/9/2018 4:25 AM, Sean Mullan wrote: On 8/8/18 5:29 PM, Xuelei Fan wrote: The "Default" algorit

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-14 Thread Anthony Scarpino
On 08/13/2018 12:42 PM, Sean Mullan wrote: On 8/10/18 3:49 PM, Anthony Scarpino wrote: On 8/9/2018 4:25 AM, Sean Mullan wrote: On 8/8/18 5:29 PM, Xuelei Fan wrote: The "Default" algorithm defined in the SunJSSE provider is for TLS protocols. What if I set DTLS to be the default,

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-10 Thread Anthony Scarpino
out where SSL[Server]SocketFactory.getDefault() uses a ssl.SocketFactory.provider property set to SunJSSE? If so, can see that as a code review comment, but it seems very obscure for the CSR. Xuelei --Sean Xuelei On 8/8/2018 1:28 PM, Sean Mullan wrote: On 8/8/18 1:58 PM, Anthony Scarpino wrote: I don't se

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-08 Thread Anthony Scarpino
should be Java API since we are changing the behavior of an implementation of a standard API. I asked Joe Darcy this question yesterday, and he agreed. I thought about API, but since it was a behavior change, API didn't sound completely correct. But that's fine if that's what he wants. --Sea

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-08 Thread Anthony Scarpino
this scenario is not supported. I think the Interface Kind should be Java API since we are changing the behavior of an implementation of a standard API. I asked Joe Darcy this question yesterday, and he agreed. --Sean Thanks, Xuelei On 8/7/2018 4:14 PM, Anthony Scarpino wrote: Hi Xuelei,

Re: CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Anthony Scarpino
n Solution section, "Throwing a UnsupportedOperationException when getting a socket from the SSLServerSocketFactory or SSLSocketFactory for DTLS."   I guess you meant, throwing a UOE when calling SSLContext.getServerSocketFactory() and SSLContext.getSocketFactory()? Thanks, Xuelei On 8/7/20

CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

2018-08-07 Thread Anthony Scarpino
I need a review of a CSR for SSLSocket should throw an exception when configuring DTLS. We are targeting this for 12 right now. https://bugs.openjdk.java.net/browse/JDK-8209031 thanks Tony

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-02 Thread Anthony Scarpino
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/08/2018 18:41, Anthony Scarpino wrote: >> That looks fine to me. >> >> Tony &

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-01 Thread Anthony Scarpino
) 0x00); } return copy; == Any other reviews/comments from folks before I submit final build/test & webrev ? Regards, Sean. On 01/08/18 15:55, Anthony Scarpino wrote: My only comment is if it makes sense to have the change at 676 to also only null

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-01 Thread Anthony Scarpino
My only comment is if it makes sense to have the change at 676 to also only null out on decrypt? Otherwise I'm fine with the changes Tony On 07/31/2018 02:04 AM, Seán Coffey wrote: Thanks for review Tony. Comments inline.. On 27/07/18 21:02, Anthony Scarpino wrote: If we are going

Re: CFV: New Security Group Member: Adam Petcher

2018-07-31 Thread Anthony Scarpino
Vote: Yes On 07/31/2018 08:58 AM, Sean Mullan wrote: I hereby nominate Adam Petcher to Membership in the Security Group. Adam is a member of the Security Libraries Team at Oracle and has a strong background in cryptography. He has been working on security for the JDK since December 2016.

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-27 Thread Anthony Scarpino
. On 26/07/18 17:42, 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

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-26 Thread Anthony Scarpino
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 fine.. Tony

RFR(xs) 8206968: java/net/httpclient/CancelledResponse.java fails after TLS1.3 changeset

2018-07-20 Thread Anthony Scarpino
Below is a simple fix that the test intermittently fails on. The problem is believed to be that the read thread sees a close_notify which cause the read thread to do a write operation. That write operation conflicts with the on-going write thread usage of the Cipher object doing the

RFR(s): 8204196: integer cleanup

2018-07-19 Thread Anthony Scarpino
I need a review of some integer check cleanups. http://cr.openjdk.java.net/~ascarpino/int/webrev/ thanks Tony

Re: RFR[12] JDK-8179098 "Crypto AES/ECB encryption/decryption performance regression (introduced in jdk9b73)"

2018-07-10 Thread Anthony Scarpino
/9/2018 3:14 PM, Anthony Scarpino wrote: On 07/03/2018 02:03 PM, Valerie Peng wrote: Hi Tony, Would you have time to review this? Instead of doing the bounds check per block, the changes move the bounds check up one level. Bug: https://bugs.openjdk.java.net/browse/JDK-8179098 Webrev: http

Re: RFR[12] JDK-8179098 "Crypto AES/ECB encryption/decryption performance regression (introduced in jdk9b73)"

2018-07-09 Thread Anthony Scarpino
On 07/03/2018 02:03 PM, Valerie Peng wrote: Hi Tony, Would you have time to review this? Instead of doing the bounds check per block, the changes move the bounds check up one level. Bug: https://bugs.openjdk.java.net/browse/JDK-8179098 Webrev:

Re: RFR 8203228: Branch-free output conversion for X25519 and X448

2018-06-25 Thread Anthony Scarpino
The change looks fine to me Tony On 06/25/2018 05:49 AM, Adam Petcher wrote: It would be nice to get this X25519/X448 enhancement into JDK 11. If anyone has some time to review this in the next day or so, I would appreciate it. On 5/15/2018 2:42 PM, Adam Petcher wrote: Webrev:

Re: Code Review Request: TLS 1.3 full handshake (JDK-8196584)

2018-06-19 Thread Anthony Scarpino
Read side key limit change at: http://hg.openjdk.java.net/jdk/sandbox/rev/6210466cf1ac Tony

Re: Code Review Request: TLS 1.3 Implementation

2018-06-15 Thread Anthony Scarpino
On 06/15/2018 06:53 AM, Xuelei Fan wrote: SSLCipher.java -- In the implementation, the key usage impacts the write side only.   I think the read side should also limit the key usage. I remember it being discussed many many months ago we at first planned to do the read side, but

Re: Code Review Request: TLS 1.3 full handshake (JDK-8196584)

2018-06-13 Thread Anthony Scarpino
DTLSRecord.java & SSLRecord.java The two variables below not used. They weren't used before the code restructuring either. maxDataSizeMinusOneByteRecord maxAlertRecordSize Tony On 06/13/2018 02:21 PM, Anthony Scarpino wrote: I found some commented out code that I will re

Re: Code Review Request: TLS 1.3 full handshake (JDK-8196584)

2018-06-13 Thread Anthony Scarpino
I found some commented out code that I will remove in CertificateMessage, lines 1300-1319 on my next push unless this should be uncommented. In SupportedVersionsExtension.java, HRRSupportedVersionsProducer and HRRSupportedVersionReproducer could be merged with a boolean in the constructor to

Re: Code Review Request: TLS 1.3 Implementation

2018-06-09 Thread Anthony Scarpino
On 06/08/2018 07:55 PM, Bradford Wetmore wrote: [...] TransportContext.java - 90:  Any chance of combining these 3 constructors into 1?  Almost identical duplicate code here. I have some code to change this already. Tony

Re: RFR JDK-8178374: Problematic ByteBuffer handling in CipherSpi.bufferCrypt method

2018-06-08 Thread Anthony Scarpino
On 06/06/2018 02:16 PM, Valerie Peng wrote: Hi Tony, Can you please help reviewing this fix? The original impl uses buffering for both input and output and is broken in several places. I ended up re-writing most of them. The buffering is done only on input. The new regression test covers all

Re: Code Review Request: TLS 1.3 full handshake (JDK-8196584)

2018-06-07 Thread Anthony Scarpino
Xuelei, I'll push updates if you're are ok with the changes. Tony --- ServerHandshakeContext.java ServerHello.java - spelling nits only HandshakeContext.java - Could getActiveCipherSuites() compile a list of cipher suites once per ProtocolVersion instead doing it for each instance of

Re: Code Review Request: TLS 1.3 Implementation

2018-06-06 Thread Anthony Scarpino
I can make the below changes if they are accepted. Tony -- InputRecord.java - Optimize Imports - fragmentSize appears not to be used. It is constructed, it can be set by changeFragmentSize. MaxFragExtension even calls these methods, but no where can I find that uses the fragmentSize. It

Re: Code Review Request: TLS 1.3 Implementation

2018-06-04 Thread Anthony Scarpino
On 06/04/2018 08:50 AM, Xuelei Fan wrote: On 6/4/2018 8:36 AM, Anthony Scarpino wrote: Here are questions I have about the code before I or someone makes a change: ChangeCipherSpec.java - 69 & 200:  Given this is legacy data on older version of TLS, is the exception better wo

Re: Code Review Request: TLS 1.3 Implementation

2018-06-04 Thread Anthony Scarpino
Here are questions I have about the code before I or someone makes a change: ChangeCipherSpec.java - 69 & 200: Given this is legacy data on older version of TLS, is the exception better worded "Not supported"? "yet" implies future additions. - ChangeCipherSpec is removed in 1.3, why is there

Re: Code Review Request: TLS 1.3 Implementation

2018-06-03 Thread Anthony Scarpino
Xuelei, From looking at your code change at 151 and 219 when a single KeyUpdate handshake is sent, it appears you are changing all 4 keys, both send and receive from the client and send and receive from the server. It was my interpretation of the spec that only the send and receive keys

Re: JNLP launched legacy app needs to override jdk.tls.disabledAlgorithms

2018-05-21 Thread Anthony Scarpino
The property can get loaded very early and it can be hard to get ahead of it. The only suggestion I can think of is your option 1 isn't doing what you expect. To append to the existing java.security properties, you use one equals, “=“. To override you use 2, “==‘. So you may want something

Re: RFR[11] JDK-8146293 "Add Support for RSA-PSS Signature Algorithm as in PKCS#1 v2.2"

2018-04-27 Thread Anthony Scarpino
I'm fine with 1 or 2. Maybe leaning more toward 2 given what Xuelei said about the RFCs. Tony On 04/27/2018 04:41 PM, Valerie Peng wrote: I'd also strongly prefer to pick one as standard name for RSA PSS signature and use it consistently. Here are the possible choices for RSA PSS

Re: [11] RFR: JDK-8197441: Signature#initSign/initVerify for an invalid private/public key fails with ClassCastException for SunPKCS11 provider

2018-02-15 Thread Anthony Scarpino
On 02/15/2018 04:40 PM, Valerie Peng wrote: Anyone can help review this trivial fix? It'll just take a minute or so. Essentially, the fix is to re-throw with InvalidKeyException when the casting failed. Bug: https://bugs.openjdk.java.net/browse/JDK-8197441 Webrev:

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

2017-12-05 Thread Anthony Scarpino
On 12/05/2017 06:27 AM, Seán Coffey wrote: 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

Re: KDF API review, round 2

2017-11-27 Thread Anthony Scarpino
On 11/27/2017 11:16 AM, Jamil Nimeh wrote: I thought that we had ditched setParameter in favor of putting these parameters in getInstance.  IIRC we were headed toward an algorithm naming convention of /, plus APS in the getInstance (which may be null (and might be for most KDFs that we start

Re: [10] XXS RFR 8187023: Cannot read pkcs11 config file in UTF-16 environment

2017-08-31 Thread Anthony Scarpino
On 08/31/2017 02:19 PM, Ivan Gerasimov wrote: On 8/31/17 1:44 PM, Anthony Scarpino wrote: On 08/31/2017 01:10 PM, Ivan Gerasimov wrote: Hello! Currently, when reading the pkcs11 config file, the default encoding is assumed. This causes errors when the encoding is set to UTF-16. Would

Re: [10] XXS RFR 8187023: Cannot read pkcs11 config file in UTF-16 environment

2017-08-31 Thread Anthony Scarpino
On 08/31/2017 01:10 PM, Ivan Gerasimov wrote: Hello! Currently, when reading the pkcs11 config file, the default encoding is assumed. This causes errors when the encoding is set to UTF-16. Would you please help review the trivial fix? Bug: https://bugs.openjdk.java.net/browse/JDK-8187023

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

2017-07-16 Thread Anthony Scarpino
> On Jul 14, 2017, at 12:01 PM, Langer, Christoph <christoph.lan...@sap.com> > wrote: > > Hi, > >> From: Anthony Scarpino [mailto:anthony.scarp...@oracle.com] > >> I'm working on a test so we avoid this in the future. > > OK, so, shall I submit the f

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

2017-07-14 Thread Anthony Scarpino
On 7/14/17 12:56 PM, Anthony Scarpino wrote: On 07/14/2017 08:37 AM, Langer, Christoph wrote: Hi, after the discussion in thread http://mail.openjdk.java.net/pipermail/security-dev/2017-July/016068.html, please review my proposed change: Bug: https://bugs.openjdk.java.net/browse/JDK-8184673

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

2017-07-14 Thread Anthony Scarpino
On 07/14/2017 08:37 AM, Langer, Christoph wrote: Hi, after the discussion in thread http://mail.openjdk.java.net/pipermail/security-dev/2017-July/016068.html, please review my proposed change: Bug: https://bugs.openjdk.java.net/browse/JDK-8184673 Change: *diff -r 76fca9438ee9 -r

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

2017-07-13 Thread Anthony Scarpino
On 07/12/2017 07:45 AM, Sean Mullan wrote: On 7/11/17 3:10 PM, Langer, Christoph wrote: In any case, from what you are saying, I take that I can safely patch our JDK distribution with this change without doing a bad thing to security in general, wouldn't you agree? Yes, I agree. Also,

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

2017-07-13 Thread Anthony Scarpino
On 07/13/2017 11:26 AM, Anthony Scarpino wrote: On 07/12/2017 11:59 PM, Langer, Christoph wrote: I then suggest to also revert JDK10 and 9 to use X509CertImpl.getSigAlgName() forthe time being until some better check to go for the encoded AlgorithmId. Would you be fine with that Looking back

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

2017-07-13 Thread Anthony Scarpino
On 07/12/2017 11:59 PM, Langer, Christoph wrote: Hi Sean, So, I guess I would be fine if this could at least be changed for JDKs <= 8 for compatibility reasons. I can understand if for JDK >= 9 we say this is a new release and the standard algorithm names shall be enforced. Wouldn't that be a

Re: Code Review Request, JDK-8178728 Check the AlgorithmParameters in algorithm constraints

2017-06-06 Thread Anthony Scarpino
On 06/06/2017 04:04 PM, Xuelei Fan wrote: New webrev: http://cr.openjdk.java.net/~xuelei/8178728/webrev.01/ On 6/6/2017 1:45 PM, Anthony Scarpino wrote: On 06/05/2017 02:15 PM, Xuelei Fan wrote: Hi, Please review the JDK 10 update: http://cr.openjdk.java.net/~xuelei/8178728/webrev.00

Re: [10] RFR 8166222: Don't treat signed jars with invalid timestamps as unsigned

2017-05-11 Thread Anthony Scarpino
I think your code looks good. You should check the CertPathValidator tests, I think one of them might fail after this change. Tony On 05/10/2017 04:36 PM, Weijun Wang wrote: Ping again. On 04/12/2017 11:52 PM, Weijun Wang wrote: Please take a review at

RFR 8176457: Add verbose option to java.security.debug

2017-05-02 Thread Anthony Scarpino
I need a review for adding the verbose option to debugging so that the output is not overwhelmed with unnecessary info. http://cr.openjdk.java.net/~ascarpino/8176457/webrev/ Tony

Re: RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-11 Thread Anthony Scarpino
On 04/10/2017 12:18 PM, Sean Mullan wrote: On 4/6/17 4:39 PM, Anthony Scarpino wrote: I'd like to get a review for this performance change to use the existing CounterMode parallelized intrinsic instead of GCTR's own version. The two classes were nearly identical except for the doFinal() method

Re: RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-11 Thread Anthony Scarpino
On 04/10/2017 10:27 AM, Paul Sandoz wrote: On 7 Apr 2017, at 12:47, Anthony Scarpino <anthony.scarp...@oracle.com> wrote: On 04/07/2017 06:58 AM, Chris Hegarty wrote: On 06/04/17 21:39, Anthony Scarpino wrote: I'd like to get a review for this performance change to use the ex

Re: RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-07 Thread Anthony Scarpino
On 04/07/2017 06:58 AM, Chris Hegarty wrote: On 06/04/17 21:39, Anthony Scarpino wrote: I'd like to get a review for this performance change to use the existing CounterMode parallelized intrinsic instead of GCTR's own version. The two classes were nearly identical except for the doFinal

RFR 8177784 Use CounterMode intrinsic for AES/GCM

2017-04-06 Thread Anthony Scarpino
I'd like to get a review for this performance change to use the existing CounterMode parallelized intrinsic instead of GCTR's own version. The two classes were nearly identical except for the doFinal() method which doesn't belong in CounterMode.java. I could have been more aggressive with

Re: RFR[9] 8165367: Additional tests for JEP 288: Disable SHA-1 Certificates

2017-04-04 Thread Anthony Scarpino
On 03/30/2017 07:41 AM, John Jiang wrote: Hi, This patch includes a set of new test cases for JEP 288. The cases just focus on the usage constraints TLSSever and TLSClient with TLS communication. Issue: https://bugs.openjdk.java.net/browse/JDK-8165367 Webrev:

Re: RFR: 3 security-libs release notes on keytool/krb5/etc

2017-03-24 Thread Anthony Scarpino
I'd agree with Sean, "weak" implies anything risk, the weakness isn't necessarily key length related. Tony On 03/24/2017 11:56 AM, Sean Mullan wrote: On 3/24/17 7:01 AM, Weijun Wang wrote: I'll use "short key". I prefer the term "weak" which implies it is a risk. We already use that term

Re: RFR 8176536: Backport weak algorithms checking

2017-03-20 Thread Anthony Scarpino
Hi Sean, I updated the webrev with your recent change. One you ok this, I'll request approval for backport. http://cr.openjdk.java.net/~ascarpino/8176536/webrev.01/ thanks Tony On 03/17/2017 01:25 PM, Anthony Scarpino wrote: Oh yeah. I had forgot to resync to get your change. Thanks

Re: RFR 8176536: Backport weak algorithms checking

2017-03-17 Thread Anthony Scarpino
backport: > https://bugs.openjdk.java.net/browse/JDK-8176503 > > --Sean > >> On 3/16/17 1:04 AM, Anthony Scarpino wrote: >> Hi, >> >> I need a review of this large backport of the weak algorithm checking >> code to jdk8. >> >> In mosts cases the

RFR 8176536: Backport weak algorithms checking

2017-03-15 Thread Anthony Scarpino
Hi, I need a review of this large backport of the weak algorithm checking code to jdk8. In mosts cases the changes are either identical or 95% of what is in jdk9, the below two files deviate the most from jdk9 because of other jdk9 features:

Re: RFR 8176350: Usage constraints don't take effect when using PKIX

2017-03-08 Thread Anthony Scarpino
dumping the log when the usage is not allowed? Xuelei On 3/8/2017 1:15 PM, Anthony Scarpino wrote: Hi, I need a code review of this small change.. The PKIX path for usage checking didn't pass the variant to the checkers because of a previous needed check for plugins. http://cr.openjdk.java.net

RFR 8176350: Usage constraints don't take effect when using PKIX

2017-03-08 Thread Anthony Scarpino
Hi, I need a code review of this small change.. The PKIX path for usage checking didn't pass the variant to the checkers because of a previous needed check for plugins. http://cr.openjdk.java.net/~ascarpino/8176350/webrev/ thanks Tony

RFR 8175250: Manifest checking throws exception with no entry

2017-02-22 Thread Anthony Scarpino
Hi, I need a review of this small change that wasn't caught in normal testing of manifest entries. http://cr.openjdk.java.net/~ascarpino/8175250/webrev/ thanks Tony

[RFR] 8174849: Change SHA1 certpath restrictions

2017-02-13 Thread Anthony Scarpino
Hi, I need a quick review on a simple certpath config change. http://cr.openjdk.java.net/~ascarpino/8174849/webrev/ thanks Tony

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

2017-02-07 Thread Anthony Scarpino
(!permits(primitives, key.getAlgorithm(), null)) { -return false; -} This block cannot be removed as the standard permits() (seel line 130) still need to this check. Otherwise, looks fine to me. Xuelei On 1/23/2017 3:27 PM, Anthony Scarpino wrote: Hi, I need a c

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

2017-02-07 Thread Anthony Scarpino
more than a timestamp. --Sean On 1/30/17 6:57 PM, Anthony Scarpino wrote: On 01/23/2017 03:27 PM, Anthony Scarpino wrote: Hi, I need a code review of this change that brings more detail constraints checking and control to certpath and jar disabled algorithm Security properties. http://

Re: RFR 8173410: Add commented config line for jdk.security.provider.preferred

2017-02-07 Thread Anthony Scarpino
formating consistent. Why did you move the RMI Registry Serial Filter? Offhand, this seems gratuitous, and will make ift more difficult for folks trying to maintain a single or slightly modified java.security across release families. Looks good otherwise. Brad On 2/6/2017 11:26 AM, Anthony

RFR 8173410: Add commented config line for jdk.security.provider.preferred

2017-02-06 Thread Anthony Scarpino
Please review this change for Solaris SPARC configurations to add an optional configuration line. There are a few minor changes which didn't change any content. Changing some "#"'s for a bit more consistency and readability across the file. Also moving an RMI entry to the end that was merged

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

2017-01-31 Thread Anthony Scarpino
that will certainly benefit logging. regards, Sean. On 30/01/2017 18:21, Anthony Scarpino wrote: Hi Sean, Actually Sean M and I were talking about that offline on thursday. That file is changing a lot. The three sections you mention have changed a lot, but the general idea is the disabled alg

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

2017-01-30 Thread Anthony Scarpino
On 01/23/2017 03:27 PM, Anthony Scarpino wrote: Hi, I need a code review of this change that brings more detail constraints checking and control to certpath and jar disabled algorithm Security properties. http://cr.openjdk.java.net/~ascarpino/8160655/webrev/ thanks Tony Updated review

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

2017-01-30 Thread Anthony Scarpino
() instead * SignatureFileVerifier.java 294 Timestamp[] timestamp = new Timestamp[newSigners.length]; "timestamps" would be more clear as a variable name 299 System.out.println("Timestamp[" + (i - 1) + "] = " + debug.println --Sean On 1/23

Re: Code Review Request, JDK-8172869 4096 is not supported yet for the DH Parameter Generator

2017-01-24 Thread Anthony Scarpino
On 01/24/2017 11:24 AM, Xuelei Fan wrote: Hi, Please review this spec documentation update: http://cr.openjdk.java.net/~xuelei/8172869/webrev.00/ In the java.security.AlgorithmParameterGenerator specification, 4096 bits DH parameter generator is specified as a required feature. However,

Re: RFR 8172527: Rename jdk.crypto.token to jdk.crypto.cryptoki

2017-01-20 Thread Anthony Scarpino
On 01/19/2017 11:50 AM, Mandy Chung wrote: On Jan 19, 2017, at 11:39 AM, Anthony Scarpino <anthony.scarp...@oracle.com> wrote: Hi, I need a review to rename the jdk.crypto.token to jdk.crypto.cryptoki. This is to change what 8171202 had done to the original jdk.crypto.pkcs11

RFR 8172527: Rename jdk.crypto.token to jdk.crypto.cryptoki

2017-01-19 Thread Anthony Scarpino
Hi, I need a review to rename the jdk.crypto.token to jdk.crypto.cryptoki. This is to change what 8171202 had done to the original jdk.crypto.pkcs11 module. For those not familiar with discussions elsewhere, the term "token" is confusing and unclear as it can mean many things

Re: Code Review Request, JDK-8171003, A couple of JSSE tests have been failing after JDK-8170329

2016-12-09 Thread Anthony Scarpino
Looks good. Tony On 12/09/2016 12:54 PM, Xuelei Fan wrote: Hi, Please review this typo issue introduced in JDK-8170329. $ hg diff test/sun/net/www/protocol/https/HttpsClient/ProxyAuthTest.java @Override -protected boolean isCustomizeClientConnection() { +protected boolean

Re: [9] RFR 8079898: Resolve disabled warnings for libj2ucrypto

2016-12-07 Thread Anthony Scarpino
On 12/07/2016 03:21 PM, Valerie Peng wrote: Anyone can help reviewing this? The fix is straight forward, just renamed the DEBUG to J2UC_DEBUG to address the E_MACRO_REDEFINED warning. In addition, I also updated the nativeCrypto.h to remove the workaround for a Solaris12-specific issue which

Re: RFR: 8170131: Certificates not being blocked by jdk.tls.disabledAlgorithms property

2016-12-02 Thread Anthony Scarpino
disabled > via the jdk.tls.disabledAlgorithms or the jdk.certpath.disabledAlgorithms > property. Please take another look and let me know if you are ok with it: > > http://cr.openjdk.java.net/~mullan/webrevs/8170131/webrev.01/ > > Thanks, > Sean > >> On 11/22/16 11:00 AM, Antho

<    1   2   3   4   5   6   7   >