> On Apr 16, 2020, at 9:19 AM, Weijun Wang <[email protected]> wrote:
>
>
>
>> On Apr 15, 2020, at 11:29 PM, Sean Mullan <[email protected]> wrote:
>>
>> On 4/15/20 3:30 AM, Weijun Wang wrote:
>>>> On Apr 14, 2020, at 1:00 AM, Sean Mullan <[email protected]> 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
