It seems odd to support both. Once we put that into the code it’s very hard to take out in case someone is depending on it. So for the new properties I would just support one variant.
> On Sep 27, 2018, at 10:23 AM, Weijun Wang <weijun.w...@oracle.com> wrote: > > Oh, I didn't know it was specified in > KeyStore.PasswordProtection.getProtectionAlgorithm(). I'll need to rework on > webrev.04. > > As for pkcs12 or PKCS12, do we really need to treat old and new properties > differently? I would document them in java.security with pkcs12 but support > reading both. > > --Max > >> On Sep 27, 2018, at 9:43 PM, Sean Mullan <sean.mul...@oracle.com> wrote: >> >> On 9/27/18 1:58 AM, Weijun Wang wrote: >>> All others accepted. >>>> 1122 keystore.pkcs12.certProtectionAlgorithm = PBEWithSHA1AndRC2_40 >>>> >>>> Shouldn't this be named certPbeAlgorithm so that it matches >>>> certPbeIterationCount? Same comment about keyProtectionAlgorithm. >>> Unfortunately we already had "keystore.pkcs12.keyProtectionAlgorithm" and >>> it also accepts "PKCS12". >> >> Ah, I see - I forgot about that property. >> >> See >> http://cr.openjdk.java.net/~weijun/8076190/webrev.03/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java-.html: >>> 147 private static final String[] KEY_PROTECTION_ALGORITHM = { >>> 148 "keystore.pkcs12.keyProtectionAlgorithm", >>> 149 "keystore.PKCS12.keyProtectionAlgorithm" >>> 150 }; >>> But you are right the names are not consistent and supporting both "pkcs12" >>> and "PKCS12" is awkward (and cannot make use of the new >>> SecurityProperties::privilegedGetOverridable method). Now I decide to name >>> the new properties as (key|cert)PbeAlgorithm names with only "pkcs12", and >>> only support the old names as a fallback. >> >> Hmm, but this means we have to support 2 properties meaning the same thing, >> and the KeyStore.PasswordProtection.getProtectionAlgorithm() is already >> specified to use the keystore.<type>.keyProtectionAlgorithm property. Based >> on that, I take back my comment, and I think it would be better to retain >> the existing property instead of defining another one with the same meaning. >> I don't like having to try to get the properties with "pkcs12" and "PKCS12", >> but it seems we could live with that since we have been doing it and we only >> need to do this for the existing properties - we could assume "pkcs12" for >> all of the new ones. >> >> --Sean >
