Hi Philipp,

Sorry I'm so late giving a response. I've filed

   https://bugs.openjdk.java.net/browse/JDK-8221719
   Jarsigner Fails To Verify Signed By Alias If Alias Given In Wrong Case

The fix is good. Since we don't support identitydb anymore I think we can 
remove the parenthesis around the alias in the storeHash now.

Thanks,
Max

> On Feb 20, 2019, at 8:41 PM, Philipp Kunz <[email protected]> wrote:
> 
> Hi,
> 
> When signing a jarfile the specified alias is converted by JavaKeyStore 
> (storetype JKS) to lower case.
> When verifying the jarfile with the same alias, it is currently not converted 
> by JKS to lower case and jarsigner reports that the jar is not signed by the 
> alias even after having just signed it with that same alias if not all 
> characters are in lower case.
> 
> I found no statement in the documentation that an alias would support only 
> lower case or which characters could be used. It could, however, be worth 
> noting in the documentation of JavaKeyStore or the keytool that the alias can 
> be changed by the keystore in certain circumstances.
> In my opinion, it feels like a bug that aliases are treated differently when 
> signing and verifying with respect to their upper and lower case.
> 
> Find a patch attached. Some explanations, just in case not too obvious anyway:
> 
> - The jarsigner never changed the upper and lower cases of aliases itself and 
> neither does with the patch. That is now delegated to the keystore 
> implementation not only for signing but in addition also for verifying. The 
> fix is therefore not JSK-specific.
> - I assume it is correct that the if (store != null) check and the try catch 
> KeyStoreException can both be moved outside the for loop over the 
> certificates because before as well as now with the patch the result was 
> alway 0 if store was null and KeyStoreException can happen only when loading 
> the keystore, which is certainly true for JKS.
> - storeHash is used only by printCert and inKeyStoreForOneSigner and contains 
> the aliases as values enclosed in parentheses "(" and ")" which is how it is 
> printed by printCert. It would have probably been easier to add the 
> parentheses only later in printCert but then I tried to change as little as 
> possible.
> - Two pairs of "result |= " statements were duplicate. Therefore there are 
> the two methods in the test which show that both actually failed with 
> "ckaliases.contains" being the offending piece of code. In the patch I 
> refactored to recombine them.
> - I really wonder how "result |= SIGNED_BY_ALIAS;" should be set for any 
> certificate c and not only the one an alias in ckaliases points at but I 
> guess that is another story if one at all and might have come from filling 
> storeHash with all certificates and not only the ones in ckaliases or so.
> - At first glance it might look like a mistake that "alias" is not used 
> inside the loop over ckaliases but instead of comparing "alias" in lower case 
> to ckaliases with unspecified case, the certificate c is now compared to the 
> one gotten from the keystore handling the alias case in its own way.
> 
> Would someone sponsor this fix or can/should it be improved?
> 
> Regards,
> Philipp
> <JavaKeyStoreAliasCaseInsensitive.patch>

Reply via email to