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

[Weijun Wang]

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.
>