Looks fine to me.
I was wondering, if it is more simple to define PSSParamsHolder as an
enum? Anyway, it is just very minor comment. You can leave it as it is.
Thanks,
Xuelei
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 <xuelei....@oracle.com> wrote:
On 1/3/2019 2:10 AM, Weijun Wang wrote:
On Jan 2, 2019, at 11:56 PM, Xue-Lei Fan <xuelei....@oracle.com> 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
