code review request: 7047200: keytool safe store (was Misleading error message)

[Xuelei Fan]

Thanks for the answer. The fix looks fine to me.

Thanks,
Xuelei

On 9/9/2011 9:38 AM, Weijun Wang wrote:
> 
> On 09/09/2011 08:52 AM, Xuelei Fan wrote:
>> KeyStore.store() will damage the key store/output stream because of
>> java.lang.IllegalArgumentException: password can't be null, is it right?
> 
> Yes.
> 
>>
>> It seems that before the exception, there is nothing write to the output
>> stream. I'm not sure why the key store will be damaged. What's you
>> evaluation?
> 
> In the bug report (as well as my regression test), the user mistakenly
> adds -protected to the command line so there is no prompt for password,
> and keytool finally goes to "KeyStore.store(outStream, pass)" with
> pass==null. Here, the outStream is opened as a FileOutputStream to the
> keystore file, but since the store method fails, nothing is really
> written out and the file becomes empty.
> 
> -Max
> 
>>
>> Thanks,
>> Xuelei
>>
>> On 9/8/2011 5:13 PM, Weijun Wang wrote:
>>>
>>> Bug weblink: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7047200
>>> Webrev: http://cr.openjdk.java.net/~weijun/7047200/webrev.00/
>>>
>>> The original bug report is a false report. However, because of a simple
>>> input error, the keystore file is damaged permanently. This is
>>> definitely not a nice user experience.
>>>
>>> The fix stores the keystore content to a byte array first before writing
>>> it to a file. An alternative way would be store the content to a new
>>> file name and then do a remove-and-rename, but since keystore files are
>>> normally small, it's not worth trying.
>>>
>>> Thanks
>>> Max
>>>
>>> On 06/29/2011 08:50 AM, weijun.wang at oracle.com wrote:
>>>> 7047200: keytool safe store (was Misleading error message)
>>>>
>>>>
>>>> === *Description*
>>>> ============================================================
>>>> FULL PRODUCT VERSION :
>>>> java version "1.6.0_25"
>>>> Java(TM) SE Runtime Environment (build 1.6.0_25-b06)
>>>> Java HotSpot(TM) Client VM (build 20.0-b11, mixed mode, sharing)
>>>>
>>>> ADDITIONAL OS VERSION INFORMATION :
>>>> Microsoft Windows XP [Version 5.1.2600]
>>>> (C) Copyright 1985-2001 Microsoft Corp.
>>>>
>>>> A DESCRIPTION OF THE PROBLEM :
>>>> Why is an error being generated after I key in the password twice?
>>>>
>>>> REGRESSION.  Last worked in version 6u25
>>>>
>>>> STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
>>>> Command Line
>>>> Microsoft Windows XP [Version 5.1.2600]
>>>> (C) Copyright 1985-2001 Microsoft Corp.
>>>>
>>>> C:\Documents and Settings\Jon>keytool -genkeypair -v -protected -alias
>>>> jon -file
>>>> certif.file
>>>> What is your first and last name?
>>>> [Unknown]: Jon C.
>>>> What is the name of your organizational unit?
>>>> [Unknown]: @Jon's
>>>> What is the name of your organization?
>>>> [Unknown]: @Jon's
>>>> What is the name of your City or Locality?
>>>> [Unknown]: Birkirkara
>>>> What is the name of your State or Province?
>>>> [Unknown]: Malta(EU)
>>>> What is the two-letter country code for this unit?
>>>> [Unknown]: MT
>>>> Is CN=Jon C., OU=@Jon's, O=@Jon's, L=Birkirkara, ST=Malta(EU), C=MT
>>>> correct?
>>>> [no]: yes
>>>>
>>>> Generating 1,024 bit DSA key pair and self-signed certificate
>>>> (SHA1withDSA) with
>>>> a validity of 90 days
>>>> for: CN=Jon C., OU=@Jon's, O=@Jon's, L=Birkirkara, ST=Malta(EU), C=MT
>>>> Enter key password for<jon>
>>>> (RETURN if same as keystore password):
>>>> Re-enter new password:
>>>> [Storing C:\Documents and Settings\Jon\.keystore]
>>>>
>>>>
>>>> EXPECTED VERSUS ACTUAL BEHAVIOR :
>>>> EXPECTED -
>>>> PKI should be generated
>>>> ACTUAL -
>>>> Error message is displayed.
>>>>
>>>> ERROR MESSAGES/STACK TRACES THAT OCCUR :
>>>> keytool error: java.lang.IllegalArgumentException: password can't be
>>>> null
>>>> java.lang.IllegalArgumentException: password can't be null
>>>> at
>>>> sun.security.provider.JavaKeyStore.engineStore(JavaKeyStore.java:508)
>>>>
>>>> at
>>>> sun.security.provider.JavaKeyStore$JKS.engineStore(JavaKeyStore.java:
>>>> 38)
>>>> at java.security.KeyStore.store(KeyStore.java:1117)
>>>> at sun.security.tools.KeyTool.doCommands(KeyTool.java:901)
>>>> at sun.security.tools.KeyTool.run(KeyTool.java:171)
>>>> at sun.security.tools.KeyTool.main(KeyTool.java:165)
>>>>
>>>> REPRODUCIBILITY :
>>>> This bug can be reproduced always.
>>>>
>>>> === *Evaluation*
>>>> =============================================================
>>>> This is mainly a user error:
>>>>
>>>>> keytool -genkeypair -v -protected -alias jon -file certif.file
>>>>
>>>> 1. Does the user intent to create a new keystore certif.file? If so,
>>>> please use "-keystore certif.file".
>>>>
>>>> 2. The default keystore type at the moment, JKS, is file-based. So, do
>>>> not specify "-protected". This option is for token-based keystores
>>>> which has their own special protection mechanism.
>>>>
>>>> Having said that, we can enhance keytool to deal with this user input
>>>> error more friendly.
>>