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
