[15] RFR JDK-8242151 Improve OID mapping and reuse among JDK security providers for aliases registration

[Valerie Peng]

Hmm, I took a shot at keytool/Main.java and used 
KnownOIDs.findMatch(...) to construct the oid.

They will be included in webrev.02.

Thanks,
Valerie
On 5/1/2020 3:29 PM, Valerie Peng wrote:
>
> These two BASE ones are simply used to get rid of the hardcoded oid 
> string code in keytool/Main.java.
>
> I can remove them (in webrev.02) and maybe you can update 
> keytool/Main.java later to use the right KnownOIDs enum for oid 
> construction? There are a few places in keytool/Main.java which can be 
> updated to use the enum KnownOIDs, but that's a bit far from the main 
> purpose of this RFE.
>
> Thanks,
> Valerie
> On 5/1/2020 6:16 AM, Weijun Wang wrote:
>> One more thing:
>>
>> In KnownOIDs.java, I found these 2 lines:
>>
>>      PKIX_KP_BASE("1.3.6.1.5.5.7.3."),
>>      PKIX_OCSP_BASE("1.3.6.1.5.5.7.48."),
>>
>> IMHO, they should not belong here, at least, we should remove the dot 
>> at the end and make them real OIDs.
>>
>> I was testing the ObjectIdentifier generation and notice these two.
>>
>> Thanks,
>> Max
>>
>>
>>> On May 1, 2020, at 5:45 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>>
>>> ObjectIdentifier.java
>>> ---------------------
>>>
>>> Have you thought about storing the ObjectIdentifier object 
>>> somewhere? ObjectIdentifier.of() creates a new object each time and 
>>> the conversion of string to byte[] might be a performance issue. We 
>>> used to have a lot of ObjectIdentifier objects in AlgorithmId but 
>>> now we only have KnownOIDs.
>>>
>>> I had a talk with Stuart and he has a suggestion that we can stuff 
>>> all pre-calculated OID DER encodings in a long byte array in a 
>>> resource file, and in KnownOIDs each element has an offset/length 
>>> pair that point to its DER encoding. Also, whatever cache mechanism 
>>> we use in the future, I suggest making "new 
>>> ObjectIdentifier(String)" private and keep "ObjectIdentifier 
>>> of(String)".
>>>
>>> SecurityProviderConstants.java
>>> ------------------------------
>>>
>>>     public static List<String> alias(String ... aliases) {
>>>         return Arrays.asList(aliases);
>>>     }
>>>
>>> Probably not necessary, you can simply call List.of(...) everywhere.
>>>
>>> SunMSCAPI.java
>>> --------------
>>>
>>> Why not call getAliases() inside "new ProviderService" like in the 
>>> other providers? Same in UCrypto.
>>>
>>> AlgorithmId.java
>>> ----------------
>>>
>>> algOID(String). You don't check "if (name.indexOf('.') != -1)" at 
>>> the beginning anymore. Is there an algorithm name containing "."?
>>>
>>> It's a pity we have to collect OIDs from other providers. Maybe it 
>>> should only be necessary when we use that provider, for example, 
>>> when encoding a signature, we should ask the provider about the OID. 
>>> I wish there were a Signature::getAlgorithmId, but if not, maybe we 
>>> can rename Algorithm::alfOID(name) to Algorithm::alfOID(name, 
>>> provider).
>>>
>>> Do you know a bad case if we don't collect those OIDs? It must be 
>>> some algorithm that is not in the Standard Names.
>>>
>>> Overall I think the change looks great, and we have a single place 
>>> to store all OIDs. The mapping among the OID string, KnownOIDs, and 
>>> ObjectIdentifier could be enhanced. Do you have a benchmark?
>>>
>>> Thanks,
>>> Max
>>>
>>>
>>>> On Apr 30, 2020, at 6:59 AM, Valerie Peng <valerie.peng at oracle.com> 
>>>> wrote:
>>>>
>>>> Here is the updated webrev 
>>>> http://cr.openjdk.java.net/~valeriep/8242151/webrev.01/