Re: [15] RFR 8172680: Support SHA-3 based Hmac algorithms

[Michael StJohns]

Hi Valerie -

In line

On 4/3/2020 5:32 PM, Valerie Peng wrote:


Hi Mike,

We can update SunPKCS11 provider when the PKCS#11 official header 
files are updated with SHA3 and Hmac w/ SHA3.


I agree with you on the ideal case is to have no lagging in JCA and 
the SunPKCS11 provider.


The main reason for the lagging is that we need to test and make sure 
the added functionality works. I checked NSS which is what existing 
PKCS11 regression tests use and it does not have any SHA3 support. Do 
you know other PKCS11 vendors which supports SHA3 and Hmac w/ SHA3? If 
there are many, it'll help me justifying this when the official 
headers are not updated yet.


I've got an include file from Utimaco dated 27 March 2017 that includes 
the SHA3 assignments from PKCS11 - and their collateral says they 
implement SHA3 (this is all of the message digest, hmac and signature 
mechanisms, and key derivation mechanisms specified for PKCS11 3.0.


Safenet ProtectServer has it 
https://data-protection-updates.gemalto.com/2018/04/27/product-release-safenet-protecttoolkit-5-6/


I can't find anything that says nCipher has it.

That's two out of three of the big ones.

I am not sure if I understand your suggestion of PKCS11 specific 
mechanism naming convention. Is it about duplicating the pending SHA3 
mechanism definitions in SunPKCS11 provider? It's trivial to add the 
SHA3 related mechanism definitions to SunPKCS11 provider, but the 
convention is to add things only after they are official as it may be 
hard to change due to backward compatibility concern.


Something like MessageDigest.getInstance ("SHA3_256", pkcs11provider) 
ends up mapping to an underlying call "CK_MECHANISM m = new CK_MECHANISM 
(CKM_SHA3_256);" where CKM_SHA3_256 is  "public static long CKM_SHA3_256 
= 0x02b0L;"


There are at times a number of proprietary or provider specific 
algorithms that the underlying PKCS11 dll might support, but for which 
the Java PKCS11 provider doesn't have the string (name) to mechanism 
number mapping, but for which the API is the same as for any other 
algorithm of its class.


For example, the Utimaco PKCS11 definitions include

#define CKM_DES3_RETAIL_MAC    0x8135    // Retail-MAC with 
0-Padding


Which is unlikely to be part of any PKCS11 standard, but could be 
accessed by


Mac.getInstance ("PKCS11_MAC_16_8135", pkcs11provider); // 16 is the 
mac length.


So this is an escape mechanism to permit access to provider extensions 
without having to reflect them back into the Java PKCS11 provider.


(When support for EC algorithms were being kept from various software - 
including NSS - due to nervousness about patent claims, I ended up using 
the PKCS11 wrapper classes directly specifically because I couldn't do 
an ECDSA via the PKCS11 provider.  That hasn't been the case for a 
while, but it's always bothered me that the JCA got in the way of the 
underlying provider.)


I don't know that is doable given the current architecture (which 
usually requires an OID for a mechanism to register it for SunPKCS11), 
but something to think about.



Thanks & hope you get enough sleep during this difficult time...

*laugh* I'm doing better thanks.  I wrenched something in my shoulder 
and it kept me awake or woke me up when I was sleeping. Much better now.


Thanks! Mike



Valerie

On 3/31/2020 10:15 AM, Michael StJohns wrote:

Sorry - this one got past me.

For PKCS11 - the assignment of mechanism numbers can happen at any 
time and doesn't necessarily result in a new version of the 
specification.  In this case, the API won't change, so there's no 
reason - since the mechanism numbers have been assigned since last 
May at least - to wait for V3.  Among other things, I would expect 
that various vendors have already implemented these in their 2.xx 
implementations.


One of the reasons I ended up using the SunPKCS11 wrapper classes 
directly quite a while ago was that the PKCS11 spec hadn't been 
updated, but that my PKCS11 provider was supplying various EC 
mechanisms that I needed. Ideally, the JCA and SunPKCS11 provider 
support should *precede* any given underlying PKCS11 device support, 
not trail it by 6-12 months.


The assignment of mechanism numbers is exactly equivalent to the 
assignment of TLS cipher suite numbers - the underlying protocol 
doesn't change, so this is mostly a change to the mapping tables and 
enclosed classes.


In any event, any given PKCS11 implementation may or may not support 
any given set of mechanisms - the provider really ought to be calling 
C_GetMechanismList() and using that as the list of supported JNA 
mechanisms.


Sorry - I'm dealing with a bit of lack of sleep here so I may be 
babbling, but I'm wondering if it might not be a bad idea to add some 
sort of PKCS11 specific mechanism naming convention to allow for the 
lag/lead problem?   E.g   PKCS11_DIGEST_02B0 would be PKCS11's 
CKM_SHA3_256 hashing function given



#define CKM_SHA3_2560x02B0

RFR [15] JDK-8242145, New System Properties to configure the TLS signature schemes

[Xuelei Fan]

Hi,

Could I get the following update reviewed?

Webrev: http://cr.openjdk.java.net/~xuelei/8242145/webrev.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8242141
CSR: https://bugs.openjdk.java.net/browse/JDK-8242145
Release-note: https://bugs.openjdk.java.net/browse/JDK-8242147

A third party's TLS implementation may not be able to handle a certain 
signature schemes, and cannot interop with JDK.  Although the 
implementation does not comply to TLS specifications, the impact could 
be significant if an application that uses the implementation is popular.


Thanks,
Xuelei

Re: [RFR] 8166597: Crypto support for the EdDSA Signature Algorithm (JEP 339)

[Anthony Scarpino]

On 4/2/20 8:34 PM, Weijun Wang wrote:

Another question:

Why does getAlgorithm() of PublicKey and PrivateKey return "EdDSA"
instead of "Ed25519" and "Ed448"?

Do we suggest people using "EdDSA" or "Ed25519"/"Ed448" when calling
KeyFactory.getInstance() andKeyPairGenerator.getInstance()?


I don't think the code is suggesting anything. I believe the 
implementation is trying to be consistent with the API and other 
asymmetric keys factories and generators.  Just using EC as an example 
one uses "EC" for the getInstance() and provides the 
AlgorithmParameterSpec to generate the publicKey


kf = KeyFactory.getInstance("EC");
ECParameterSpec.ep = ..
kf.generatePublicKey(ep)

Being consistent for EDDSA, replace "EC" with "EDDSA" and specify a 
NamedParameterSpec to generate the public key; however, it is allowed to 
replace "EC" with ED25519. Similar to how XDH does it. Unfortunately 
generatePublicKey requires an AlgorithmParameterSpec, which is redundant 
in this case:

---
kf = KeyFactory.getInstance("ED25519")
...
kf.generatePublicKey(NamedParameterSpec.ED5519);
---

Since existing code follows the first example we should be consistent 
for apps adding EDDSA.


For KeyPairGenerator, initialize() isn't required to be called with 
getInstance("ED25519") to generate the key, but it will accept an 
initialize() call.  What's different is "EDDSA" will default to ED25519 
and does not require initialize(), but it will accept initialize() to 
change it to ED448.  I believe this is to be consistent with EC and RSA 
that need initialize().


To complete all the EDDSA entries, Signature is different because, the 
key provides the details about the curve.  It's similar to 
KeyPairGenerator, "EDDSA" doesn't lock you into a particular curve, 
allowing the key to specify the curve, which follows the EC/RSA logic. 
Specifying the curve at getInstance() locks you into that curve so at 
least NoSuchAlgorithm will be thrown at getInstance() unlike other 
asymmetric algorithms.


So to wrap all this up, it makes sense for consistency with prior 
behavior that all 3 classes have an EDDSA entry.  And the specific curve 
usage is also consistent with what has already been integrated with XDH.


Tony



Thanks,
Max


On Mar 24, 2020, at 2:53 AM, Anthony Scarpino  
wrote:

On 2/25/20 12:49 PM, Anthony Scarpino wrote:

Hi
I need a code review for the EdDSA support in JEP 339.  The code builds on the 
existing java implemented constant time classes used for XDH and the NIST 
curves.  The change also adds classes to the public API to support EdDSA 
operations.
All information about the JEP is located at:
JEP 339: https://bugs.openjdk.java.net/browse/JDK-8199231
CSR: https://bugs.openjdk.java.net/browse/JDK-8190219
webrev: https://cr.openjdk.java.net/~ascarpino/8166597/webrev/
thanks
Tony



I updated the webrev with some minor updates that were commented previously.

https://cr.openjdk.java.net/~ascarpino/8166597/webrev.01/

Tony

Re: [15] RFR 8172680: Support SHA-3 based Hmac algorithms

[Valerie Peng]

Hi Mike,

We can update SunPKCS11 provider when the PKCS#11 official header files 
are updated with SHA3 and Hmac w/ SHA3.


I agree with you on the ideal case is to have no lagging in JCA and the 
SunPKCS11 provider.


The main reason for the lagging is that we need to test and make sure 
the added functionality works. I checked NSS which is what existing 
PKCS11 regression tests use and it does not have any SHA3 support. Do 
you know other PKCS11 vendors which supports SHA3 and Hmac w/ SHA3? If 
there are many, it'll help me justifying this when the official headers 
are not updated yet.


I am not sure if I understand your suggestion of PKCS11 specific 
mechanism naming convention. Is it about duplicating the pending SHA3 
mechanism definitions in SunPKCS11 provider? It's trivial to add the 
SHA3 related mechanism definitions to SunPKCS11 provider, but the 
convention is to add things only after they are official as it may be 
hard to change due to backward compatibility concern.


Thanks & hope you get enough sleep during this difficult time...

Valerie

On 3/31/2020 10:15 AM, Michael StJohns wrote:

Sorry - this one got past me.

For PKCS11 - the assignment of mechanism numbers can happen at any 
time and doesn't necessarily result in a new version of the 
specification.  In this case, the API won't change, so there's no 
reason - since the mechanism numbers have been assigned since last May 
at least - to wait for V3.  Among other things, I would expect that 
various vendors have already implemented these in their 2.xx 
implementations.


One of the reasons I ended up using the SunPKCS11 wrapper classes 
directly quite a while ago was that the PKCS11 spec hadn't been 
updated, but that my PKCS11 provider was supplying various EC 
mechanisms that I needed. Ideally, the JCA and SunPKCS11 provider 
support should *precede* any given underlying PKCS11 device support, 
not trail it by 6-12 months.


The assignment of mechanism numbers is exactly equivalent to the 
assignment of TLS cipher suite numbers - the underlying protocol 
doesn't change, so this is mostly a change to the mapping tables and 
enclosed classes.


In any event, any given PKCS11 implementation may or may not support 
any given set of mechanisms - the provider really ought to be calling 
C_GetMechanismList() and using that as the list of supported JNA 
mechanisms.


Sorry - I'm dealing with a bit of lack of sleep here so I may be 
babbling, but I'm wondering if it might not be a bad idea to add some 
sort of PKCS11 specific mechanism naming convention to allow for the 
lag/lead problem? E.g   PKCS11_DIGEST_02B0 would be PKCS11's 
CKM_SHA3_256 hashing function given



#define CKM_SHA3_2560x02B0


Just a thought.

Thanks - Mike


On 3/19/2020 5:27 PM, Valerie Peng wrote:

Hi Mike,

Thanks for heads up. From what I can gather, SHA3 inclusion is part 
of PKCS#11 v3. Is this the next release which you are referring to? 
Or will there be an update to v2.40? Is there any schedule info for 
these update/release do you know?


Following the convention, we normally don't add something which the 
underlying library has no support yet. With the new 6-month JDK 
release cycle, it's much faster for the added functionality to be 
available. So, I'd still prefer to update SunPKCS11 provider with 
SHA-3 once they are officially included.


Valerie

On 3/18/2020 4:07 PM, Michael StJohns wrote:

On 3/18/2020 6:57 PM, Valerie Peng wrote:


Anyone has time to help review this straight forward RFE? It's to 
add SHA-3 support to Hmac.


RFE: https://bugs.openjdk.java.net/browse/JDK-8172680

Webrev: http://cr.openjdk.java.net/~valeriep/8172680/webrev.00/

Mach5 run is clean.

Thanks,
Valerie


Valerie -

I know the RFE says no PKCS11 because 2.40 doesn't include those 
items, but OASIS PKCS11 has proposed  SHA3 identifiers at 
https://github.com/oasis-tcs/pkcs11/blob/master/working/identifier_db/sha3.result 
- maybe reach out and ask if these have been allocated pending the 
next release?


Mike


#define CKM_SHA3_256  0x02b0UL
 #define CKM_SHA3_256_HMAC 0x02b1UL
 #define CKM_SHA3_256_HMAC_GENERAL 0x02b2UL
 #define CKM_SHA3_224  0x02b5UL
 #define CKM_SHA3_224_HMAC 0x02b6UL
 #define CKM_SHA3_224_HMAC_GENERAL 0x02b7UL
 #define CKM_SHA3_384  0x02c0UL
 #define CKM_SHA3_384_HMAC 0x02c1UL
 #define CKM_SHA3_384_HMAC_GENERAL 0x02c2UL
 #define CKM_SHA3_512  0x02d0UL
 #define CKM_SHA3_512_HMAC 0x02d1UL
 #define CKM_SHA3_512_HMAC_GENERAL 0x02d2UL

Re: RFR 8241960: The SHA3 message digests are not thread safe when cloned

[Valerie Peng]

The updated webrev looks good. I will proceed with pre-integration 
testing and push the changes on your behalf once the testing is done.


In the mean time, if you have any particular wording for the commit 
message, please send it my way.


Thanks,
Valerie
On 4/3/2020 5:40 AM, Alexey Bakhtin wrote:

Hello Valeri,

Thank you for review. Please, have a look at updated patch at : 
https://cr.openjdk.java.net/~abakhtin/8241960/webrev.v1/


I’ve fixed your findings and renamed test class to ThreadSafetyTest. 
Jtreg already has ThreadSafeTest class for java.text.Normalizer 
functionality.


Thank you
Alexey

On 3 Apr 2020, at 02:51, Valerie Peng > wrote:


Hi Alexey,

In general looks pretty good, just some comments on the regression test:

- line 28: The duration value 10 may be lowered to shorten the 
execution time. On a Linux machine, with threadFactor=5, each digest 
algo takes about (2xduration+2) sec and overall takes ~283sec. 
Cutting the duration value drastically to 2, I still can reproduce 
the bug. Maybe 4 would be a better default value considering the 
execution time.


- line 43: add "SHA-512/224", "SHA-512/256" to the ALGORITHM_ARRAY?

- line 50: instead of hardcoding 5 here, why not just use 
threadsFactor (assigned with default value 5 on line 48)?


- line 52-55: I think you meant to use 5 (instead of the value 180) 
as the default min duration on line 52. Then, use duration variable 
instead of the hardcoded  5 on line54. For testing purpose, why not 
just use the supplied value? You already have default values if none 
are supplied.


- Consider iterating through existing providers and remove 
isSHA3supported() method. This way all providers which supports the 
digest algorithm are tested and no provider-specific knowledge is 
needed. For example:


     Provider[] provs = Security.getProviders();
     for (Provider p : provs) {
     System.out.println("Testing provider " + p.getName() + "...");
     for (String alg: ALGORITHM_ARRAY) {
     try {
     MessageDigest md = MessageDigest.getInstance(alg, p);
     testThreadSafe(md, input, nTasks, duration, false);
                 

- line 76: missing space char between } and catch.

Thanks,
Valerie

Re: RFR 8241960: The SHA3 message digests are not thread safe when cloned

[Alexey Bakhtin]

Hello Valeri,

Thank you for review. Please, have a look at updated patch at :  
https://cr.openjdk.java.net/~abakhtin/8241960/webrev.v1/

I’ve fixed your findings and renamed test class to ThreadSafetyTest. Jtreg 
already has ThreadSafeTest class for java.text.Normalizer functionality.

Thank you
Alexey

On 3 Apr 2020, at 02:51, Valerie Peng 
mailto:valerie.p...@oracle.com>> wrote:


Hi Alexey,

In general looks pretty good, just some comments on the regression test:

- line 28: The duration value 10 may be lowered to shorten the execution time. 
On a Linux machine, with threadFactor=5, each digest algo takes about 
(2xduration+2) sec and overall takes ~283sec. Cutting the duration value 
drastically to 2, I still can reproduce the bug. Maybe 4 would be a better 
default value considering the execution time.

- line 43: add "SHA-512/224", "SHA-512/256" to the ALGORITHM_ARRAY?

- line 50: instead of hardcoding 5 here, why not just use threadsFactor 
(assigned with default value 5 on line 48)?

- line 52-55: I think you meant to use 5 (instead of the value 180) as the 
default min duration on line 52. Then, use duration variable instead of the 
hardcoded  5 on line54. For testing purpose, why not just use the supplied 
value? You already have default values if none are supplied.

- Consider iterating through existing providers and remove isSHA3supported() 
method. This way all providers which supports the digest algorithm are tested 
and no provider-specific knowledge is needed. For example:

Provider[] provs = Security.getProviders();

for (Provider p : provs) {

System.out.println("Testing provider " + p.getName() + "...");

for (String alg: ALGORITHM_ARRAY) {

try {

MessageDigest md = MessageDigest.getInstance(alg, p);

testThreadSafe(md, input, nTasks, duration, false);



- line 76: missing space char between } and catch.

Thanks,
Valerie

Re: RFR 8240848: ArrayIndexOutOfBoundsException buf for TextCallbackHandler

[Weijun Wang]

Ping again.

> On Mar 12, 2020, at 9:21 PM, Weijun Wang  wrote:
> 
> Please take a review at
> 
>   http://cr.openjdk.java.net/~weijun/8240848/webrev.00/
> 
> The problem is that selection has a different meaning for a specified 
> optionType (the option value) and an unspecified one (the option index).
> 
> I also take this chance to make ConfirmationCallback more robust.
> 
> Thanks,
> Max
>