Menu
▾
▴
Re: [Opensc-devel] Bad signature generated by pkcs15-crypt ?
|
From: Douglas E E. <dee...@gm...> - 2016-02-24 17:45:03
|
Look OK to me for RSA, but may have issues with EC or GOST. So should only be done for RSA. Others need to comment on this. Can you submit this to github.com/OpenSC/OpenSC as a issue or pull request? https://github.com/OpenSC/OpenSC/issues https://github.com/OpenSC/OpenSC/pulls On 2/24/2016 11:17 AM, Joe...@we... wrote: > Hi again, > > thanks for your suggestions! > I took another look at this, but I did not find a nice solution to pass the information of the modlen > to the lower layers without having to change the API, which would probably be too much of a hazzle for everyone else. > > But what about checking the returned length in sc_pkcs15_compute_signature itself? By this we'd still make the > entire outlen available to the drivers (in case someone needs more than modlen e.g. for temporary data), and would only > alter something if the retured data is less than modlen (but no error code), which to my understanding would > always be unwanted behavior. > > The modified patch looks like this: > > diff --git a/src/libopensc/pkcs15-sec.c b/src/libopensc/pkcs15-sec.c > index 019d8a1..9c78acb > --- a/src/libopensc/pkcs15-sec.c > +++ b/src/libopensc/pkcs15-sec.c > @@ -433,6 +433,14 @@ int sc_pkcs15_compute_signature(struct sc_pkcs15_card *p15card, > > r = use_key(p15card, obj, &senv, sc_compute_signature, tmp, inlen, > out, outlen); > + > + if (r >= 0 && (size_t)r < modlen) // returned size smaller than expected, add leading zeros > + { > + memmove(out + (modlen -r ), out, r); /* overlapping */ > + memset(out, 0, modlen -r ); > + r = modlen; > + } > + > LOG_TEST_RET(ctx, r, "use_key() failed"); > > > This is working fine for me, I tested it with about 10000 different input strings, but obviously just with my card and > with this one use case. > > > > Gesendet: Montag, 22. Februar 2016 um 19:16 Uhr > Von: "Douglas E Engert" <dee...@gm...> > An: ope...@li... > Betreff: Re: [Opensc-devel] Bad signature generated by pkcs15-crypt ? > On 2/22/2016 9:51 AM, Joe...@we... wrote: >> Thanks for providing this patch, with this I got it _almost_ working :-) >> I ran into one real and two minor issues: >> 1) The real issue is that the outlen does not seem to be the expected signature length, >> but the size of the buffer with some extra space. In my case it is 1024 and not the expected >> 512, so this does not work. But I guess it would be possible to compute the expected signature >> length in a general way? > > sc_pkcs15_compute_signature set modlen lines 324-336 from the type of key and its size, then tests if outlen is big enough: > > 339 if (inlen > sizeof(buf) || outlen < modlen) > > But then it passes to lower levels, it passes outlen: > > 434 r = use_key(p15card, obj, &senv, sc_compute_signature, tmp, inlen, > 435 out, outlen); > > In all cases other then the card you have this is not a problem. > > So one possible fix is to set line 435 to: > out, modlen); > then do the memmove stuff if its too short. > > BUT THIS IS A GLOBAL CHANGE, and would need testing for other cards. I don't see why it would be an issue, > but you never know... > > If you try and do an openpgp only fix, it looks like by the time pgp_set_security_env and pgp_compute_signature > are called, they size of the key is not known, just the outlen. Som info cold be saved in the > > Another way: card-openpgp.c only supports RSA. And only 4K, 2K and maybe 1K keys are used. > So if apdu.resplen within 4 bytes of one of these values, assume it is dropped 1, 2, 3 or 4 bytes, > and do the memmove stuff then. (Not perfect, but chance of failure to catch a short signature is 1/2^32) > > There may be more info in the OpenPGP documents that would show how to save the key size internally > in one of the card-openpgp.c internal structures. > > >> 2) Minor techical issues: the apdu was not updated in the end to return the new length, >> and src and dest were mixed up in the memmove > > OK, I never tested the code. good to here you got it working. > >> With this hacked up version of your patch I was able to get a valid signature :-) , but obviously it works only >> for exactly my usecase with at most one leading zero: >> --- a/src/libopensc/card-openpgp.c >> +++ b/src/libopensc/card-openpgp.c >> @@ -1656,6 +1656,13 @@ pgp_compute_signature(sc_card_t *card, const u8 *data, >> r = sc_check_sw(card, apdu.sw1, apdu.sw2); >> LOG_TEST_RET(card->ctx, r, "Card returned error"); >> >> + /* some cards may drop leading 0x00 byte on a signature */ >> + if (apdu.resplen < 512) { >> + memmove(out + 1 , out, apdu.resplen); /* overlaping */ >> + memset(out, 0, 1); >> + apdu.resplen = 512; >> + } >> + >> *Gesendet:* Sonntag, 21. Februar 2016 um 20:54 Uhr >> *Von:* "Douglas E Engert" <dee...@gm...> >> *An:* ope...@li... >> *Betreff:* Re: [Opensc-devel] Bad signature generated by pkcs15-crypt ? >> The patch I sent you has a bug: >> >> memmove(out, out -(outlen - apdu.resplen), apdu.resplen); /* overlaping */ >> should be: >> >> memmove(out, out + (outlen - apdu.resplen), apdu.resplen); /* overlaping */ >> >> >> I have not tried the patch. >> >> On 2/21/2016 7:53 AM, Douglas E Engert wrote: >> >>> Try the attache patch. It is against http:/github.com/OpenSC/OpenSC >>> >> >> >> -- >> >> Douglas E. Engert <DEE...@gm...> >> >> >> ------------------------------------------------------------------------------ >> Site24x7 APM Insight: Get Deep Visibility into Application Performance >> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month >> Monitor end-to-end web transactions and take corrective actions now >> Troubleshoot faster and improve end-user experience. Signup Now! >> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 >> _______________________________________________ >> Opensc-devel mailing list >> Ope...@li... >> https://lists.sourceforge.net/lists/listinfo/opensc-devel[https://lists.sourceforge.net/lists/listinfo/opensc-devel] >> >> >> ------------------------------------------------------------------------------ >> Site24x7 APM Insight: Get Deep Visibility into Application Performance >> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month >> Monitor end-to-end web transactions and take corrective actions now >> Troubleshoot faster and improve end-user experience. Signup Now! >> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140[http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140] >> >> >> >> _______________________________________________ >> Opensc-devel mailing list >> Ope...@li... >> https://lists.sourceforge.net/lists/listinfo/opensc-devel[https://lists.sourceforge.net/lists/listinfo/opensc-devel] >> > > -- > > Douglas E. Engert <DEE...@gm...> > > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > Monitor end-to-end web transactions and take corrective actions now > Troubleshoot faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140[http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140] > _______________________________________________ > Opensc-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/opensc-devel[https://lists.sourceforge.net/lists/listinfo/opensc-devel] > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > Monitor end-to-end web transactions and take corrective actions now > Troubleshoot faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > _______________________________________________ > Opensc-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/opensc-devel > -- Douglas E. Engert <DEE...@gm...> |
