RFR: 8061842: Package jurisdiction policy files as something other than JAR

[Seán Coffey]

Hi Brad,

nice to have this going in. Some comments.

1. Bug synopsis, can you edit it perhaps. "Introduce security property 
to control strong crypto" seems more descriptive.

2. Exception handling.

Alot of your new exceptions don't include context. That makes debugging 
more difficult than needs be.

> +            throw new SecurityException(
> +                "Invalid cryptographic jurisdiction policy directory value");
> +        if (!Files.isDirectory(path) || !Files.isReadable(path)) {
> +            throw new Exception("Can't read cryptographic policy directory");

> +                        throw new SecurityException(
> +                            "Unexpected jurisdiction policy filename found");

> +                } catch (Exception e) {
> +                    throw new SecurityException(
> +                        "Couldn't parse jurisdiction policy files");
Please include the exception 'e' in your last exception here.


> +        } catch (DirectoryIteratorException ex) {
> +            // I/O error encountered during the iteration,
> +            // the cause is an IOException
> +            throw new SecurityException(
> +                    "Couldn't iterate through the jurisdiction policy files");
> +        }
Can you add the DirectoryIteratorException to the final exception that 
you're throwing ?

3. Test case.

The TestUnlimited.java testcase seems to be lacking. Do you want to test 
other values for crypto.policy ? 'limited' would be one. Throwing in 
some dummy value would also be good so that the exception handling code 
gets exercised.

It needs to be run in ovm mode since you're setting a Security property.

Regards,
Sean.

On 18/08/2016 05:26, Wang Weijun wrote:
> Before this change, you require default policy in neither export nor import to be empty but do not care about the getMinimum() result. In this change, you make sure the final result is not empty. I assume this is a fix?
>
> 283                     // Did we find a default perms?
>
> What does this line mean?
>
> 296                         // This should never happen
>
> But you can still print out the file name.
>
>
> Can you rename policydir-tbd to something else? I am afraid it will be confused with policy.url.1 etc.
>
> The original README.TXT in unlimited says "are exportable from the United States." and you have "is exportable." now. Is this intended? (IANAL)
>
> TestUnlimited.java:
> 45 "// Use the AES are the test Cipher", you mean "Use AES as the test Cipher"?
> 51 "throw new Exception ("Unlimited policy is NOT active");". No space before "(".
>
> No other comment.
>
> --Max
>
>> On Aug 18, 2016, at 7:22 AM, Bradford Wetmore <bradford.wetmore at oracle.com> wrote:
>>
>> New webrev:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8061842
>> http://cr.openjdk.java.net/~wetmore/8061842/webrev.01/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20160818/5402f941/attachment.htm>