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

[Valerie Peng]

Yes, it can be quite a confusing scenario. Some algorithms have multiple 
oids (say X) and multiple algorithm names (say Y). We need to be able to 
handle all of them (X+Y). However, the separation of alias definition 
for OidString and SecurityProviderConstants are intentional and I have 
them separated with the reason below:

The aliases in OidString can be viewed as internal, e.g. from JCA 
classes and used by AlgorithmId class and its regression tests. All of 
the supported oids (X of them) have to have an OidString enum type as 
their usage may be present in the DER encoding and all internal classes 
need to handle it. As for the multiple algorithm names, supposedly, 
there is only one standard name and the rest are all aliases. Aliases 
can be used to request the impl from provider, but that's all. Internal 
classes do not need to understand these external/provider-related 
aliases. By separating these external aliases inside 
SecurityProviderConstants, it is clear that these aliases are for 
providers only.

StdName itself should not be its aliases. Which line in 
SecurityProviderConstants class has this problem?

Thanks,

Valerie

On 4/25/2020 7:46 AM, Weijun Wang wrote:
> And I am confused by aliases in SecurityProviderConstants, together with the duplicated stdName in OidString, it seems to show that one OID could have multiple names, and one name could have multiple OIDs. Can we consolidate aliases inside OidString also?
>
> Also, looking at the store() calls in SecurityProviderConstants, it looks like a stdName itself is one of its aliases. Is this really useful? From what I read in the provider classes, you are adding ServiceKeys based on these aliases to a serviceMap which already has a ServiceKey based on stdName.
>
> Thanks,
> Max
>
>> On Apr 25, 2020, at 6:28 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>
>> OidString.java
>> ==============
>>
>> 1. ExtendedKeyUsage names: used to be "serverAuth", now "ServerAuth". First letter capitalized, is this necessary?
>>
>> 2. Can we move name2oidStr() from OidString to AlgorithmId? The computeOidTable process looks like an alien.
>>
>> 3. Two questions on the following lines:
>>
>> 415         // set extra alias mappings or specify the preferred one when
>> 416         // one standard name maps to multiple enums
>> 417         // NOTE: key must use UPPER CASE
>> 418         name2enum.put("SHA1", SHA_1);
>> 419         name2enum.put("SHA", SHA_1);
>> 420         name2enum.put("SHA224", SHA_224);
>> 421         name2enum.put("SHA256", SHA_256);
>> 422         name2enum.put("SHA384", SHA_384);
>> 423         name2enum.put("SHA512", SHA_512);
>> 424         name2enum.put("SHA512/224", SHA_512$224);
>> 425         name2enum.put("SHA512/256", SHA_512$256);
>> 426         name2enum.put("DH", DiffieHellman);
>> 427         name2enum.put("DSS", SHA1withDSA);
>> 428         name2enum.put("RSA", RSA);
>>
>>    a) For line 428, is this because both RSA and ITUX509_RSA have the same stdName and you are setting the preferred one? However, I can see that "DiffieHellman", "DSA", and "SHA1withDSA" also appear in multiple places. Do they need special attention?
>>
>>    b) For the other lines, can we make this info somewhere inside the constructor? After all our goal is to consolidate all info in one single place, and a single line is better than a single file, esp, a very big file.
>>
>> 4. Are you sure the OID <-> name mapping is always the same everywhere (for all primitives and in all providers)? I mean for a stdName, if one OID alias is added in one place, should it always be added the same way in another? Have you compared the aliases map after this change?
>>
>> 5. I found KnownOIDs to be a better class name.
>>
>> AlgorithmId.java
>> ================
>>
>> There are still many lines like
>>
>>     public static final ObjectIdentifier MD2_oid = algOID(OidString.MD2);
>>
>> here. Can they be eliminated? I use IntelliJ IDEA to find their usages and most are used in only one place and some are not used at all.
>>
>>
>> I haven't read other files yet. Will send more comment later.
>>
>> Thanks,
>> Max
>>
>>
>>> On Apr 24, 2020, at 7:11 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>>
>>> Hi Max,
>>>
>>> Would you have time to review this change? The current webrev attempts to cover all security classes where hard-coded oid strings and consolidate these known oid string values into a single enum type. The changes are quite extensive, I can trim back and only cover the provider algorithm oids if you prefer. There are pros and cons for both approach.
>>>
>>> I know that the naming convention is to use all upper case for enum constants, but choose to use the same naming convention as standard names to simplify the code. SecurityProviderConstants class defines the common mappings which are general to providers. Provider-specific alias mappings are handled in specific provider class, e.g. SunJSSE class.
>>>
>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8242151
>>>
>>> Webrev: http://cr.openjdk.java.net/~valeriep/8242151/webrev.00/
>>>
>>> Mach5 runs clean.
>>>
>>> Valerie
>>>