RFR: 8242565: Policy initialization issues when the denyAfter constraint is enabled

[Weijun Wang]

> On Apr 15, 2020, at 11:29 PM, Sean Mullan <sean.mullan at oracle.com> wrote:
> 
> On 4/15/20 3:30 AM, Weijun Wang wrote:
>>> On Apr 14, 2020, at 1:00 AM, Sean Mullan <sean.mullan at oracle.com> wrote:
>>> 
>>> When a SecurityManager is enabled, early code paths that involve ServiceLoader (SL) can trigger permission checks that cause parsing of a custom policy file to fail due to recursive processing of the policy file.
>>> 
>>> I have fixed two of these issues which can occur under the following conditions:
>>> 
>>> 1. SecurityManager enabled
>>> 2. Signed JAR on the classpath
>>> 3. Policy file granting permission based on who signed jar and a keystore entry containing the alias/key
>>> 4. Code triggering a permission check based on that grant
>>> 5. A SHA-1 denyAfter constraint set in the jdk.jar.disabledAlgorithms property in the java.security file
>>> 
>>> Parsing of the denyAfter constraint is required to verify the signed JAR, which happens very early. This causes SL to search for a LocaleProvider to parse the denyAfter date field which triggers a permission check, causes the policy machinery to bootstrap, and which triggers further permission checks, etc.

Do we really need a LocaleProvider here? Or, is the grammar really locale-dependent? This does not look user-friendly to me.

--Max

>>> 
>>> The first issue is that PKCS12KeyStore is unable to verify the integrity of the keystore entry in the custom policy file because it cannot find a "PBE" AlgorithmParameters implementation. I fixed this by adding "SunJCE" to the list of providers that are automatically installed during signed JAR verification.
>>> 
>>> The second issue is also in PKCS12 KeyStore where it tries to instantiate a java.text.Collator which also uses SL and thus triggers a recursive permission check. This was fixed by using a lazy initialization Holder pattern.
>> Can we just add a "collator" argument to the getPassWithModifier() method? This looks ugly but we already have "rb".
> 
> I think that is a fine suggestion, especially since keytool and jarsigner (the only code that calls KeyStoreUtil.getPassWithModifier()) already cache a static Collator instance. New webrev:
> 
> https://cr.openjdk.java.net/~mullan/webrevs/8242565/webrev.01/
> 
> --Sean