[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-security-dev
Subject:    Re: Code Review Request, JDK-8178728 Check the AlgorithmParameters in algorithm constraints
From:       Xuelei Fan <xuelei.fan () oracle ! com>
Date:       2017-06-07 3:13:44
Message-ID: cac94f28-b872-2198-2959-46e2469793df () oracle ! com
[Download RAW message or body]

On 6/6/2017 6:03 PM, Anthony Scarpino wrote:
> On 06/06/2017 04:04 PM, Xuelei Fan wrote:
>> New webrev:
>>     http://cr.openjdk.java.net/~xuelei/8178728/webrev.01/
>>
>> On 6/6/2017 1:45 PM, Anthony Scarpino wrote:
>>> On 06/05/2017 02:15 PM, Xuelei Fan wrote:
>>>> Hi,
>>>>
>>>> Please review the JDK 10 update:
>>>>     http://cr.openjdk.java.net/~xuelei/8178728/webrev.00/
>>>>
>>>> This update extends the DisabledAlgorithmConstraints implementation by
>>>> checking the AlgorithmParameters, which is ignored at present.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>
>>> I'm find with the change, but I have an organizational requests
>>>
>>> DisabledAlgorithmConstraints.java:253-264:
>>> Can you move DH/DiffieHellman string value checking into a method in 
>>> AlgorithmDecomposer?  All the algorithm name details are handling in 
>>> there.  Just to be consistent in keeping them in one place.
>>>
>> Good points.  Updated accordingly.
>>
>> I'm not very sure of the impact to decompose the general algorithm 
>> names yet.  So I just add a more method (getAliases()), and not touch 
>> on the decomposes() method.
> 
> While I was review this earlier today, I was thinking about changes to 
> aliases, including the hashes, that could make this faster.
> 
I thought of the option when I made the update.  It's a better position. 
But I'm not confidential with my update.  So let's do it in JDK 10 
later.  May backport this update, I would like to keep the impact as 
minimal as possible.

> The changes look fine.
> 
Thanks for the view!

Xuelei
[prev in list] [next in list] [prev in thread] [next in thread]