> On Apr 30, 2019, at 1:56 PM, Philipp Kunz <[email protected]> wrote:
>
> 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?
No. It's hardcoded and not localized in printCert.
> 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?
Probably not. I believe storeHash was introduced to avoid too many calls to
store.getCertificateAlias when there are hundreds of classes in a signed jar.
--Max
>
> 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 <
>>> [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>
>>>
>>
>>
>>
