Re: [12] RFR 8215694: keytool cannot generate RSASSA-PSS certificates
Webrev updated again at
https://cr.openjdk.java.net/~weijun/8215694/webrev.02
The major change this time is in PSSParameters.java. Instead of containing
fields for the components of a PSSParameterSpec, it now contains the
PSSParameterSpec itself directly. A new public static getEncoded() method is
added so it can be called in AlgorithmId.java.
The new getEncoded() method only writes non-default values into the encoding
which conforms to the DER rule. I haven't enforced the same check on the read
side (i.e. reject the input if a default value is encoded) for interoperability.
Thanks,
Max
> On Jan 7, 2019, at 8:56 AM, Weijun Wang wrote:
>
> Hi Xuelei,
>
> Webrev updated at
>
> https://cr.openjdk.java.net/~weijun/8215694/webrev.01
>
> A new method AlgorithmId::getWithParameterSpec is added. I also cached 6
> constants in AlgorithmId $PSSParamsHolder, although the static block inside
> it to generate the AlgorithmIds are a little heavy. We can extract the
> encoding lines inside PSSParameters::engineGetEncoded to create something
> like PSSParameterSpec::getEncoded(), but that will be an RFE.
>
> Thanks,
> Max
>
>> On Jan 3, 2019, at 11:27 PM, Xue-Lei Fan wrote:
>>
>> On 1/3/2019 2:10 AM, Weijun Wang wrote:
On Jan 2, 2019, at 11:56 PM, Xue-Lei Fan wrote:
sigAlg.equalsIgnoreCase("RSASSA-PSS"):
Do you really want to ignore the case? I used to think that an algorithm
name is case sensitive.
>>> getInstance(alg) is always case-insensitive.
>> Hm, I missed it.
>>
Main.java:1445 minor, 4 more indent?
>>> Then it's longer than 80 chars. How about I un-indent lines 1443 and 1444?
>> maybe, use 4 white spaces? See also the following comment.
>>
AlgorithmId.java:1073-1091:
I may prefer to use cached parameters (for both AlgorithmParameters and
AlgorithmParameterSpec) for each size, for performance.
>>> OK for AlgorithmParameterSpec. Which AlgorithmParameters do you mean? The
>>> one in SignatureUtil?
>> Yes, replacing SignatureUtil.createAlgorithmParameters(). Then we don't
>> need to worry about the indents above.
>>
>> Thanks,
>> Xuelei
>>
>>> Thanks,
>>> Max
Xuelei
On 12/21/2018 1:44 AM, Weijun Wang wrote:
> Please take a review at
> https://cr.openjdk.java.net/~weijun/8215694/webrev.00/
> This bug reveals several issues:
> 1. Encoding of the RSASSA-PSS signature algorithm in PKCS10 and
> X509CertImpl.
> 2. The missing of setParameter() call for PKCS10 and X509CertImpl.
> 3. All keytool commands of -genkeypair, -certreq, -gencert, -selfcert are
> affected.
> 4. Wrong NULL after encoding of RSASSA-PSS key algorithm.
> Please confirm this is safe to be fixed in JDK 12.
> Thanks,
> Max
>
Re: Code Review Request, JDK-8214418 HttpClient falls in running with 100% cpu usage after an error signalled on channel
Hi Xuelei, On 22/12/2018 17:20, Xue-Lei Fan wrote: The issue is caused by the handshake status "NEED_WRAP" while the connection is half-closed. An application may just call wrap() when the handshake status is "NEED_WRAP". For compatibility, I changed the handshake status from NEED_WRAP back to NOT_HANDSHAKING for inbound half-closed connection. An application can use SSLEngine.isOutboundDone() for the determination if SSLEngine.wrap() should be called. For clarification: the logic for the application would be to call SSLEngine.isOutboundDone(), and if that is false, to call SSLEngine.wrap() in order to trigger the sending of the close notify response. Is my understanding correct? best regards, -- daniel
Re: Code Review Request, JDK-8214418 HttpClient falls in running with 100% cpu usage after an error signalled on channel
Code changes look good to me. --Jamil On 1/8/2019 3:00 PM, Xue-Lei Fan wrote: ping ... Xuelei On 12/22/2018 9:20 AM, Xue-Lei Fan wrote: Hi, Could I get the update reviewed? http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/ The reproducing testing case passed with the update. The issue is caused by the handshake status "NEED_WRAP" while the connection is half-closed. An application may just call wrap() when the handshake status is "NEED_WRAP". For compatibility, I changed the handshake status from NEED_WRAP back to NOT_HANDSHAKING for inbound half-closed connection. An application can use SSLEngine.isOutboundDone() for the determination if SSLEngine.wrap() should be called. Thanks, Xuelei
Re: Code Review Request, JDK-8214418 half-closed SSLEnigne status may cause application dead loop
On 1/9/2019 6:10 AM, Chris Hegarty wrote: Xuelei, Is it possible to update the synopsis of this bug to better reflect the underlying issue ( rather than one particular symptom )? Updated. Also, it is possible to construct a small, non-HTTP related, targeted test that verifies the fix? There was an test update that covered the fix. Thanks, Xuelei -Chris. On 08/01/2019 23:00, Xue-Lei Fan wrote: ping ... Xuelei On 12/22/2018 9:20 AM, Xue-Lei Fan wrote: Hi, Could I get the update reviewed? http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/ The reproducing testing case passed with the update. The issue is caused by the handshake status "NEED_WRAP" while the connection is half-closed. An application may just call wrap() when the handshake status is "NEED_WRAP". For compatibility, I changed the handshake status from NEED_WRAP back to NOT_HANDSHAKING for inbound half-closed connection. An application can use SSLEngine.isOutboundDone() for the determination if SSLEngine.wrap() should be called. Thanks, Xuelei
RFR 8215776: Keytool importkeystore may mix up certificate chain entries when DNs conflict
Please take a review at https://cr.openjdk.java.net/~weijun/8215776/webrev.00/ PKCS12KeyStore now can find certificate issuers more precisely using SubjectKeyIdentifier and AuthorityKeyIdentifier. I thought about using CertPath builder or checking signatures but those changes are too much. Thanks, Max
Re: Code Review Request, JDK-8214418 HttpClient falls in running with 100% cpu usage after an error signalled on channel
Xuelei, Is it possible to update the synopsis of this bug to better reflect the underlying issue ( rather than one particular symptom )? Also, it is possible to construct a small, non-HTTP related, targeted test that verifies the fix? -Chris. On 08/01/2019 23:00, Xue-Lei Fan wrote: ping ... Xuelei On 12/22/2018 9:20 AM, Xue-Lei Fan wrote: Hi, Could I get the update reviewed? http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/ The reproducing testing case passed with the update. The issue is caused by the handshake status "NEED_WRAP" while the connection is half-closed. An application may just call wrap() when the handshake status is "NEED_WRAP". For compatibility, I changed the handshake status from NEED_WRAP back to NOT_HANDSHAKING for inbound half-closed connection. An application can use SSLEngine.isOutboundDone() for the determination if SSLEngine.wrap() should be called. Thanks, Xuelei
