Jarsigner Fails To Verify Signed By Alias If Alias Given In Wrong Case

[Philipp Kunz]

Hi Max,
I agree that the parentheses are superfluous, however, I would slightly
prefer going small steps.
Would the parentheses have to come from a resource bundle as well like
SPACE or COMMA when removed from storeHash values?Now that with the
patch, storeHash is not any longer useful to check whether it contains
ckaliases in inKeyStoreForOneSigner (but currently still useful to save
another call to store.getCertificateAlias in printCert lateron), would
it be a worth a consideration to rearrange the code populating
storeHash closer to printCert in some way or even just completely
remove storeHash and replace it with store.getCertificateAlias in
printCert or anything similar?
Regards,Philipp




On Sat, 2019-03-30 at 23:01 +0800, Weijun Wang wrote:
> 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 <philipp.kunz at paratix.ch>
> > 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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190430/2e85243d/attachment.htm>