On 08/30/2011 09:52 PM, Xuelei Fan wrote:
1. Do you want to add more debug info? line 1509-1511:
+ if (debug) {
+ e.printStackTrace();
+ }
Yes. The -debug option is used to show exception full stack when an
error occurs. I want the exception printed for warnings also.
2. It looks a little strange to me that even if there is Runtime
exception, we still do some additional work in final block.
Well, the handling of exception at signing and verification are
different (compare line 212 and 227). There must be a keystore at
signing time but not necessary at verification. Therefore, the exception
handling is outside of this method.
3. I try hard to understand the code how to solve the issue. It is not
easy for me to understand why the update works.
It's because for verification the exception is ignored in the test case
(line 214-218), so even if ~/.keystore does not exist, the new finally
block makes sure the CertPathValidator is properly initialized. Before
this fix, although the exception is also ignored, the CertPathValidator
lines are totally ignored, and the CertPath validation shows an NPE and
a warning is shown.
I'm thinking, is it
possible to ignore user_home/.keystore instead of try to load it when
the file does not exist (File.exists() or catch FileNotFoundException, etc)?
The current logic is, "loading keystore from user_home/.keystore".
Can we change to use a refined logic, "if user_home/.keystore exists, we
load the keysore; otherwise, ignore it". I'm not sure the new logic get
the code more complicated, or more intuitive.
Well, it looks more correct, but is complicated in 2 senses:
1. ~/.keystore and user-specified -keystore are not treated the same.
You can ignore ~/.keystore, but if a user-specified -keystore does not
exist, it's an error.
2. signing and verification have different behaviors on exception
handling. See above.
Therefore I prefer my current code changes more. It's focused on only
one place where the bug is: make sure CertPathValidator is always
initialized, and leave all old logic unchanged.
Thanks
Max
Xuelei
On 8/30/2011 12:56 PM, Weijun Wang wrote:
Hi All
7081783: jarsigner error when no $HOME/.keystore
Webrev is at --
http://cr.openjdk.java.net/~weijun/7081783/webrev.00/
Description:
jarsigner includes a certpath validation check, and shows a warning when
the check fails. The CertPathValidator object, unfortunately, is
initialized in a method that can only be executed if a local keystore is
found (either ~/.keystore or specified by -keystore). Therefore, if
there is no local keystore but the jarfile's signer can be directly
verified by a cert in cacerts, we still see:
Warning:
This jar contains entries whose certificate chain is not validated.
The code changes make sure the CertPathValidator object is always
initialized.
For reg test, it's a simple call --
${TESTJAVA}${FS}bin${FS}jarsigner \
-J-Duser.home=. \
-verify -strict ${TESTSRC}${FS}bootstrap.jar
Here I override user.home so that even if the test machine has a
./keystore, it won't be affected. The bootstrap.jar file is a small
signed jar that is signed by a real CA that can be chained into an item
in cacerts.
Thanks
Max
