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

[Weijun Wang]

> On Apr 16, 2020, at 9:19 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
> 
> 
> 
>> 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.

Just take a look. The grammar is simple but we use SimpleDateFormat for debugging messages and Calendar to validate the input.

Maybe we can use 

   LocalDateTime.of(year, month, day, 0, 0).atOffset(ZoneOffset.UTC).toInstant()

to load the date. It takes care of validation and very easy to compare to Instant.now().

I don't have any other comment on the code change.

Thanks,
Max

> 
> --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