Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v19]

[Anthony Scarpino]

On Wed, 8 Jun 2022 17:49:38 GMT, XenoAmess  wrote:

>> as title.
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   clean up Calendar

I gave a quick look at the security files touched and seems straightforward. I 
didn't see any problems

-

Marked as reviewed by ascarpino (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8302

Re: RFR: 8267319: Use larger default key sizes and algorithms based on CNSA

[Anthony Scarpino]

On Wed, 2 Mar 2022 00:13:41 GMT, Valerie Peng  wrote:

> It's been several years since we increased the default key sizes. Before 
> shifting to PQC, NSA replaced its Suite B cryptography recommendations with 
> the Commercial National Security Algorithm Suite which suggests:
> 
> - SHA-384 for secure hashing
> - AES-256 for symmetric encryption
> - RSA with 3072 bit keys for digital signatures and for key exchange
> - Diffie Hellman (DH) with 3072 bit keys for key exchange
> - Elliptic curve [P-384] for key exchange (ECDH) and for digital signatures 
> (ECDSA)
> 
> So, this proposed changes made the suggested key size and algorithm changes. 
> The changes are mostly in keytool, jarsigner and their regression tests, so 
> @wangweij Could you please take a look?
> 
> Thanks!

Fair point that the keysize would determine the algorithm used and the default 
key sizes for keygen.

-

PR: https://git.openjdk.java.net/jdk/pull/7652

Re: RFR: 8267319: Use larger default key sizes and algorithms based on CNSA

[Anthony Scarpino]

On Wed, 2 Mar 2022 00:13:41 GMT, Valerie Peng  wrote:

> It's been several years since we increased the default key sizes. Before 
> shifting to PQC, NSA replaced its Suite B cryptography recommendations with 
> the Commercial National Security Algorithm Suite which suggests:
> 
> - SHA-384 for secure hashing
> - AES-256 for symmetric encryption
> - RSA with 3072 bit keys for digital signatures and for key exchange
> - Diffie Hellman (DH) with 3072 bit keys for key exchange
> - Elliptic curve [P-384] for key exchange (ECDH) and for digital signatures 
> (ECDSA)
> 
> So, this proposed changes made the suggested key size and algorithm changes. 
> The changes are mostly in keytool, jarsigner and their regression tests, so 
> @wangweij Could you please take a look?
> 
> Thanks!

I have some compatibility concerns about the AES change breaking code that 
expects a SecretKeySpec of 16 bytes. I can see situations where '.getEncoded()' 
returns a byte[32] when user code expects a byte[16]. Also, I'm pretty sure 
passing a 32 byte SecretKeySpec into an AES_128_GCM op will throw an exception. 
 I haven't looked at other modes.

-

PR: https://git.openjdk.java.net/jdk/pull/7652

Re: RFR[M]: Adding MD5 Intrinsic on x86-64

[Anthony Scarpino]

I had looked at the java code changes and are fine with them

Tony

> On Aug 3, 2020, at 10:10 AM, Vladimir Kozlov  
> wrote:
> 
> Hi Ludovic
> 
> This is very professional work!
> 
> CCing to Core-libs because you modified Java code and need review from Java 
> library group.
> 
> Few notes:
> 
> Add tests to verify intrinsic implementation. You can use 
> test/hotspot/jtreg/compiler/intrinsics/sha/ as examples.
> 
> In vm_version_x86.cpp move UseMD5Intrinsics flag setting near UseSHA flag 
> setting.
> 
> In new file macroAssembler_x86_md5.cpp no need empty line after copyright 
> line. There is also typo 'rrdistribute':
> 
> * This code is free software; you can rrdistribute it and/or modify it
> 
> Our validate-headers check failed. See GPL header template: 
> ./make/templates/gpl-header
> 
> Ludovic, it looks like you used only general instructions to implement this 
> code. Can you add comment where the algorithm come from? Or it is just direct 
> translation of Java code?
> 
> Vivek, do we have SSE/AVX instructions which may improve performance of this 
> code? It could be follow up update if we can.
> 
> Did you test it on 32-bit x86? Would be interesting to see result of 
> artificially switching off AVX and SSE: '-XX:UseSSE=0 -XX:UseAVX=0'. It will 
> make sure that only general instructions are needed.
> 
> Thanks,
> Vladimir
> 
>> On 8/3/20 7:39 AM, Ludovic Henry wrote:
>> Hi,
>> A quick follow up on that change. Are you happy with the general approach, 
>> or would rather have it done differently?
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8250902
>> Webrev: http://cr.openjdk.java.net/~luhenry/8250902/webrev.01/
>> Thank you
>> Ludovic

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

[Anthony Scarpino]

I believe we have at least two approvals, Vladimir and me.  Unless there 
is something else to be done, I'll do a final sanity check and push the 
code wednesday or thursday.


Tony

On 12/4/18 5:18 AM, Dmitry Chuyko wrote:

Hello,

AArch64 aes test runs pass in -Dmode=GCM.

-Dmitry

On 12/2/18 3:37 AM, Vladimir Kozlov wrote:

I am fine with Hotspot changes.

But we need to verify changes on all platforms.
I see that aarch64 also supports it in addition to SPARC.

I will run compiler/codegen/aes/ test to make sure it pass on SPARC 
but we don't test aarch64.

I let you know testing results when they are done.

Thanks,
Vladimir


On 11/20/18 6:45 PM, Kamath, Smita wrote:

Hi Tony,

Thanks for your comments.

1)  This intrinsic is also used with solaris-sparc, has there been a 
sanity check by anyone to make sure this does not break the sparc 
intrinsic? It may work as the code is now since the sparc intrinsic 
will only use the first two longs in the subkeyHtbl.
Would it be possible to help with this sanity check?  I don't have 
the required set-up to test this scenario.


2) I have changed the code so that subkeyH corresponds to the first 
two entries of subkeyHtbl.  Please find the updated webrev link.


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 ; 'Vladimir Kozlov' 

Cc: Viswanathan, Sandhya ; 
core-libs-dev@openjdk.java.net; hotspot compiler 

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


On 11/19/18 12:50 PM, Kamath, Smita wrote:

Hi Vladimir,

I'd like to contribute an optimization for GHASH Algorithm using AVX
Instructions. I have tested this optimization on SKX x86_64 platform
and it shows ~20-30% performance improvement for larger message sizes
(for example 8k).

I, smita.kam...@intel.com <mailto:smita.kam...@intel.com> , Shay
Gueuron, (shay.gue...@intel.com <mailto:shay.gue...@intel.com>)and
Regev Shemy (regev.sh...@intel.com <mailto:regev.sh...@intel.com>) are
contributors to this code.

Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8214074

Link to webrev: http://cr.openjdk.java.net/~svkamath/ghash/webrev/

For testing the implementation, I have executed TestAESMain.java. I
have executed Jtreg tests and tested this code on 64 bit Windows and
Linux platforms.

Please review and let me know if you have any comments.

Thanks and Regards,

Smita



Hi,

This intrinsic is also used with solaris-sparc, has there been a 
sanity check by anyone to make sure this does not break the sparc 
intrinsic?
It may work as the code is now since the sparc intrinsic will only 
use the first two longs in the subkeyHtbl.


In that same idea, have you consider combining the subkeyH to be the 
first two of subkeyHtbl for the non-avx operations?  It would 
eliminate an extra two longs per instance.


Tony

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

[Anthony Scarpino]

On 11/19/18 12:50 PM, Kamath, Smita wrote:

Hi Vladimir,

I’d like to contribute an optimization for GHASH Algorithm using AVX 
Instructions. I have tested this optimization on SKX x86_64 platform and 
it shows ~20–30% performance improvement for larger message sizes (for 
example 8k).


I, smita.kam...@intel.com  , Shay 
Gueuron, (shay.gue...@intel.com )and Regev 
Shemy (regev.sh...@intel.com ) are 
contributors to this code.


Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8214074

Link to webrev: http://cr.openjdk.java.net/~svkamath/ghash/webrev/

For testing the implementation, I have executed TestAESMain.java. I have 
executed Jtreg tests and tested this code on 64 bit Windows and Linux 
platforms.


Please review and let me know if you have any comments.

Thanks and Regards,

Smita



Hi,

This intrinsic is also used with solaris-sparc, has there been a sanity 
check by anyone to make sure this does not break the sparc intrinsic? 
It may work as the code is now since the sparc intrinsic will only use 
the first two longs in the subkeyHtbl.


In that same idea, have you consider combining the subkeyH to be the 
first two of subkeyHtbl for the non-avx operations?  It would eliminate 
an extra two longs per instance.


Tony

Re: RFR(M): 8152172: PPC64: Support AES intrinsics

[Anthony Scarpino]

[Switching to security-dev, core-lib-dev was bcc'ed]

I don't see any problem changing it to int[][].  Being an Object[] is 
odd in my opinion.


Tony

On 03/25/2016 04:00 PM, Vladimir Kozlov wrote:

This question is for core libs.

For JIT to generate optimal code we want to change type of
AESCrypt::sessionK from Object[] to int[][].
Otherwise JIT have to generate checkcast code to make sure that elements
of sessionK array are int[].
I see that sessionK field is private so we are not changing public API.

Thanks,
Vladimir

On 3/24/16 6:17 PM, Hiroshi H Horii wrote:

Hi Vladimir,

Thank you for your reviewing.

Is it possible to modify the type of sessionK from Object[] to int[][]?

We can guess, the type was designed for flexibility. However, only
int[] is used
to store elements of sessionK, so far. In addition, this field is
private.
Though adding checkcast is another solution, it produces overheads.

I attached an additional patch (generated with hg diff -g) to jdk
directory.
I would like to ask thoughts about this change of java code.



Regards,
Hiroshi
---
Hiroshi Horii,
IBM Research - Tokyo


Vladimir Kozlov  wrote on 03/24/2016
09:09:11:

 > From: Vladimir Kozlov 
 > To: Hiroshi H Horii/Japan/IBM@IBMJP, "hotspot-compiler-
 > d...@openjdk.java.net" 
 > Cc: "Doerr, Martin" , Tim Ellison
 > , "Simonis, Volker" 
 > Date: 03/24/2016 09:19
 > Subject: Re: RFR(M): 8152172: PPC64: Support AES intrinsics
 >
 > I think we need to add gen_checkcast for objAESCryptKey node since
 > java code has no guarantee that sessionK array elements are
 > initialized with int[]. Or we should modify java code to declare
sessionK
 > as int[][].
 >
 > Thanks,
 > Vladimir
 >
 > Note, intrinsics are correctly handle case when
 > On 3/22/16 8:47 AM, Hiroshi H Horii wrote:
 > > Dear all:
 > >
 > > Can I please request reviews for the following change?
 > > This change was created for JDK 9 to enable POWER8's AES
 > > instructions for AES calculation.
 > >
 > > This request follows this discussion.
 > > http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-
 > March/021926.html
 > >
 > > Description:
 > > This change adds stub routines support for single-block AES
 > > encryption and decryption operations on the POWER8 platform.
 > > They are available only when the application is configured to
 > > use SunJCE crypto provider on little endian.
 > >
 > > These stubs make use of efficient hardware AES instructions
 > > and thus offer significant performance improvements over
 > > JITed code on POWER8 as on x86 and SPARC. AES stub routines
 > > are enabled by default on POWER8 platforms that support AES
 > > instructions (vcipher). They can be explicitly enabled or
 > > disabled on the command-line using UseAES and
 > > UseAESIntrinsics JVM flags. Unlike x86 and SPARC, vcipher
 > > and vnchiper of POWER8 need the same round keys of AES.
 > > Therefore, inline_aescrypt_Block in library_call.cpp calls the
 > > stub with AESCrypt.sessionK[0] as round keys.
 > >
 > > Summary of source code changes:
 > >
 > >   *src/cpu/ppc/vm/assembler_ppc.hpp
 > >   *src/cpu/ppc/vm/assembler_ppc.inline.hpp
 > > - Adds support for vrld instruction to rotate vector register
values
 > >with left doubleword.
 > >
 > >   *src/cpu/ppc/vm/stubGenerator_ppc.cpp
 > > - Defines stubs for single-block AES encryption and decryption
 > >routines supporting all key sizes (128, 192 and 256-bit).
 > > - Current POWER AES decryption instructions are not compatible
 > >with SunJCE expanded decryption key format. Thus decryption
 > >stubs read the expanded encryption keys (sessionK[0]) with
 > >descendant order.
 > > - Encryption stubs use SunJCE expanded encryption key as their
 > >is no incompatibility issue between POWER8 AES encryption
 > >instructions and SunJCE expanded encryption keys.
 > >
 > >   *src/cpu/ppc/vm/vm_version_ppc.cpp
 > > - Detects AES capabilities of the underlying CPU by using
 > >has_vcipher().
 > > - Enables UseAES and UseAESIntrinsics flags if the underlying
 > >CPU supports AES instructions and neither of them is
explicitly
 > >disabled on the command-line. Generate warning message if
 > >either of these flags are enabled on the command-line
 > >whereas the underlying CPU does not support AES instructions.
 > >
 > >   *src/share/vm/opto/library_call.cpp
 > >  - Passes the first input parameter, reference to sessionK[0]
to the
 > > AES stubs only on the POWER platform.
 > >
 > >*src/share/vm/opto/graphKit.cpp
 > >  - Supports T_NARROWOOP type for GraphKit::load_array_element.
 > >
 > > Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
 > > Webrev:
http://cr.openjdk.java.net/~mdoerr/8152172_ppc64le_aes/webrev.00/
 > >
 > >
 > > Regards,
 > > Hiroshi
 > > ---
 > > Hiroshi Horii,
 > > IBM Research - Tokyo
 > >
 >

Re: RFR (S) 8035974: Refactor DigestBase.engineUpdate() method for better code generation by JIT compiler

[Anthony Scarpino]

This looks fine to me

Tony

> On May 21, 2014, at 4:34 PM, Vladimir Kozlov  
> wrote:
> 
> http://cr.openjdk.java.net/~kvn/8035974/webrev
> https://bugs.openjdk.java.net/browse/JDK-8035974
> 
> Contributed by James Cheng and modified by me.
> 
> To use intrinsics to accelerate SHA operations on multiple blocks [1], it is 
> needed to pull a loop out of DigestBase.engineUpdate() and make a new method 
> implCompressMultiBlock() which contains only the loop and can be intrinsified.
> 
> On platforms which does not use intrinsic implCompressMultiBlock() method 
> will be inlined by JIT and the same code will be generated as before. So no 
> performance regression with the pure Java SUN provider is expected.
> 
> About arithmetic change. limit = ofs + len will not overflow integer because 
> ofs <= b.length - len (there is check).
> 
> Tested with jdk jtreg tests and new hotspot jtreg test James wrote for 
> 8035968.
> 
> Thanks,
> Vladimir
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8035968

hg: jdk8/tl/jdk: 8029550: javadoc since tag for recent Hashtable updates

[anthony . scarpino]

Changeset: 014c04fd7460
Author:ascarpino
Date:  2013-12-04 17:37 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/014c04fd7460

8029550: javadoc since tag for recent Hashtable updates
Reviewed-by: mullan

! src/share/classes/java/security/Provider.java

hg: jdk8/tl/jdk: 8027218: TEST_BUG: sun/security/pkcs11/ec tests fail because of ever-changing key size restrictions

[anthony . scarpino]

Changeset: 6deabb82f72b
Author:ascarpino
Date:  2013-12-04 10:59 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6deabb82f72b

8027218: TEST_BUG: sun/security/pkcs11/ec tests fail because of ever-changing 
key size restrictions
Reviewed-by: vinnie

! test/sun/security/pkcs11/PKCS11Test.java
! test/sun/security/pkcs11/ec/ReadCertificates.java
! test/sun/security/pkcs11/ec/TestCurves.java