The code change looks fine to me.

One small comment: I notice that "KeyType type" and "AlgorithmParameterSpec 
keyParams" always go hand in hand, either as fields inside a class, or in the 
argument list of a method. Does it make sense to combine these 2 guys into a 
single class? It will look like a child class of AlgorithmId which understand 
what its alg and params mean. Of course you won't be able to make KeyType an 
enum anymore.

All up to you.

Thanks,
Max


> On May 30, 2020, at 5:07 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
> 
> Re-worked the fix a bit to take in Max's feedback:
> 
> Webrev updated athttp://cr.openjdk.java.net/~valeriep/8242897/webrev.01/
> 
> Instead of completely reverting to the pre-PSS behavior of ignoring the OID 
> in the DER encoding, this updated webrev has special handling to work with 
> the scenario in the bug report. Most changes are code refactoring driven by 
> the argument type change...
> 
> Thanks,
> Valerie
> 
> 
> On 5/7/2020 4:17 PM, Valerie Peng wrote:
>> Ping?
>> 
>> On 4/23/2020 3:28 PM, Valerie Peng wrote:
>>> Anyone has time to help review this fix? After the support for RSASSA-PSS 
>>> keys is added, when parsing the DER encoding, the key algorithm is based on 
>>> the parsed algorithm oid/name. However, an exception is thrown if the 
>>> parsed algorithm oid/name is neither RSA nor RSASSA-PSS. For this 
>>> particular report, the algorithm oid is 1.3.14.3.2.15 which is 
>>> unsupport/unrecognized by JDK. In earlier releases, the bytes are parsed 
>>> but key algorithm is always "RSA".
>>> 
>>> To maintain this backward compatibility behavior, I changed the current 
>>> impl to set the key algorithm upon key construction time w/ a KeyType 
>>> argument (RSA or RSASSA-PSS) even when DER encoding is given. After parsing 
>>> the DER encoding, for non-RSA keys, the parsed algorithm oid/name should 
>>> match the given key type, otherwise an exception is thrown.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8242897
>>> 
>>> Webrev: http://cr.openjdk.java.net/~valeriep/8242897/webrev.00/
>>> 
>>> Mach5 run is clean.
>>> 
>>> Thanks,
>>> Valerie

Reply via email to