Re: RFR [15] JDK-8215711, Missing key_share extension for (EC)DHE key exchange should alert missing_extension

[Xuelei Fan]

On 4/5/2020 1:41 PM, Anthony Scarpino wrote:

On 4/4/20 6:11 PM, Xuelei Fan wrote:

Hi,

Could I have the following update reviewed?

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

In the current TLS implementation, if one of "supported_groups" 
extension and "key_share" extension is not present in the ClientHello 
handshake message, the internal_error alter will be used.  Per the 
spec (RFC 8846), the alert should be "missing_extension" alert.


The fuzzing test passed with this update.  No new regression test 
(noreg-external).


Thanks,
Xuelei


The change looks fine. It looks like you implemented what is in section 
9.2, and it looks like the absent methods will be called from the 
consumeOnLoad() as all those extensions can be in a ClientHello msg.



Yes.

Thanks for the review!

Xuelei

Re: RFR [15] JDK-8215711, Missing key_share extension for (EC)DHE key exchange should alert missing_extension

[Anthony Scarpino]

On 4/4/20 6:11 PM, Xuelei Fan wrote:

Hi,

Could I have the following update reviewed?

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

In the current TLS implementation, if one of "supported_groups" 
extension and "key_share" extension is not present in the ClientHello 
handshake message, the internal_error alter will be used.  Per the spec 
(RFC 8846), the alert should be "missing_extension" alert.


The fuzzing test passed with this update.  No new regression test 
(noreg-external).


Thanks,
Xuelei


The change looks fine. It looks like you implemented what is in section 
9.2, and it looks like the absent methods will be called from the 
consumeOnLoad() as all those extensions can be in a ClientHello msg.


thanks

Tony

Re: RFR[15]: 8172404: Tools should warn if weak algorithms are used before restricting them

[Hai-May Chao]

Here is the webrev:

http://cr.openjdk.java.net/~weijun/8172404/webrev.00/

Thanks,
Hai-May


> On Apr 4, 2020, at 11:41 PM, Hai-May Chao  wrote:
> 
> Hi,
> 
> I'd like to request a review for:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8172404 
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8238640 
> 
> 
> It’d be useful to start warning users that certain algorithms and key lengths 
> are becoming weak, so that users could begin transition away from them before 
> they are actually disabled. A new security property named 
> jdk.security.legacyAlgorithms is added to the java.security file to list the 
> legacy algorithms. The keytool and jarsigner tools are enhanced to enforce 
> the new property and to emit the warning messages when legacy algorithms are 
> used.
> 
> Thanks,
> Hai-May

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

[Weijun Wang]

OK, I undertand now:

1. Crypto primitives (Signature/KeyFactory/KeyPairGenerator) would support all 
"EdDSA" and "Ed25519"/"Ed448", and their getAlgorithm() returns what was used 
back in getInstance().

2. Key's getAlgorithm() always returns "EdDSA".

Thanks,
Max

> On Apr 4, 2020, at 6:02 AM, Anthony Scarpino  
> wrote:
> 
> 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
> 

RFR[15]: 8172404: Tools should warn if weak algorithms are used before restricting them

[Hai-May Chao]

Hi,

I'd like to request a review for:

Bug: https://bugs.openjdk.java.net/browse/JDK-8172404 

CSR: https://bugs.openjdk.java.net/browse/JDK-8238640 


It’d be useful to start warning users that certain algorithms and key lengths 
are becoming weak, so that users could begin transition away from them before 
they are actually disabled. A new security property named 
jdk.security.legacyAlgorithms is added to the java.security file to list the 
legacy algorithms. The keytool and jarsigner tools are enhanced to enforce the 
new property and to emit the warning messages when legacy algorithms are used.

Thanks,
Hai-May