RFR 8192987: keytool should remember real storetype if it is not provided

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 8192987: keytool should remember real storetype if it is not provided

Weijun Wang
Hi All

Please take a look at

   http://cr.openjdk.java.net/~weijun/8192987/webrev.00/

The fix delays the assignment of storetype and srcstoretype (when they are not provided on the command line) to where the actual keystore file is loaded.

I could have retained lines 711-719 because we had hasStoretypeOption and hasSrcStoretypeOption flags indicating whether they are provided by the user or set on those lines, but leaving them to be null as long as possible helps exposing any unintended usages. Now they are only used in pkcs11 and MSCAPI type checks which are null safe, and hasStoretypeOption and hasSrcStoretypeOption are now useless and removed.

Thanks
Max

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192987: keytool should remember real storetype if it is not provided

Sean Mullan
Update the copyright date on KeyStoreUtil.java. Otherwise looks fine to me.

--Sean

On 12/7/17 3:53 AM, Weijun Wang wrote:

> Hi All
>
> Please take a look at
>
>     http://cr.openjdk.java.net/~weijun/8192987/webrev.00/
>
> The fix delays the assignment of storetype and srcstoretype (when they are not provided on the command line) to where the actual keystore file is loaded.
>
> I could have retained lines 711-719 because we had hasStoretypeOption and hasSrcStoretypeOption flags indicating whether they are provided by the user or set on those lines, but leaving them to be null as long as possible helps exposing any unintended usages. Now they are only used in pkcs11 and MSCAPI type checks which are null safe, and hasStoretypeOption and hasSrcStoretypeOption are now useless and removed.
>
> Thanks
> Max
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192987: keytool should remember real storetype if it is not provided

Weijun Wang
I think my previous fix introduced a behavior change. Here is an update:

   http://cr.openjdk.java.net/~weijun/8192987/webrev.01/

Precisely, the "hasStoretypeOption == false" is equivalent to "storetype == null" but should not be removed. Although we can probe storetype of pkcs12/jks/jceks, that doesn't mean we can probe a 3rd party storetype.

The test is updated so that when a wrong storetype is provided, it will be used and the loading of keystore would fail.

Thanks
Max

> On Dec 7, 2017, at 9:06 PM, Sean Mullan <[hidden email]> wrote:
>
> Update the copyright date on KeyStoreUtil.java. Otherwise looks fine to me.
>
> --Sean
>
> On 12/7/17 3:53 AM, Weijun Wang wrote:
>> Hi All
>> Please take a look at
>>    http://cr.openjdk.java.net/~weijun/8192987/webrev.00/
>> The fix delays the assignment of storetype and srcstoretype (when they are not provided on the command line) to where the actual keystore file is loaded.
>> I could have retained lines 711-719 because we had hasStoretypeOption and hasSrcStoretypeOption flags indicating whether they are provided by the user or set on those lines, but leaving them to be null as long as possible helps exposing any unintended usages. Now they are only used in pkcs11 and MSCAPI type checks which are null safe, and hasStoretypeOption and hasSrcStoretypeOption are now useless and removed.
>> Thanks
>> Max

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8192987: keytool should remember real storetype if it is not provided

Sean Mullan
Looks good.

--Sean

On 12/7/17 9:39 AM, Weijun Wang wrote:

> I think my previous fix introduced a behavior change. Here is an update:
>
>     http://cr.openjdk.java.net/~weijun/8192987/webrev.01/
>
> Precisely, the "hasStoretypeOption == false" is equivalent to "storetype == null" but should not be removed. Although we can probe storetype of pkcs12/jks/jceks, that doesn't mean we can probe a 3rd party storetype.
>
> The test is updated so that when a wrong storetype is provided, it will be used and the loading of keystore would fail.
>
> Thanks
> Max
>
>> On Dec 7, 2017, at 9:06 PM, Sean Mullan <[hidden email]> wrote:
>>
>> Update the copyright date on KeyStoreUtil.java. Otherwise looks fine to me.
>>
>> --Sean
>>
>> On 12/7/17 3:53 AM, Weijun Wang wrote:
>>> Hi All
>>> Please take a look at
>>>     http://cr.openjdk.java.net/~weijun/8192987/webrev.00/
>>> The fix delays the assignment of storetype and srcstoretype (when they are not provided on the command line) to where the actual keystore file is loaded.
>>> I could have retained lines 711-719 because we had hasStoretypeOption and hasSrcStoretypeOption flags indicating whether they are provided by the user or set on those lines, but leaving them to be null as long as possible helps exposing any unintended usages. Now they are only used in pkcs11 and MSCAPI type checks which are null safe, and hasStoretypeOption and hasSrcStoretypeOption are now useless and removed.
>>> Thanks
>>> Max
>