Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some cards may return RSA signatures without leading zero bytes Fixes #1283 #1319

Merged
merged 1 commit into from
Jul 11, 2018

Conversation

dengert
Copy link
Member

@dengert dengert commented Apr 9, 2018

Add leading zero bytes in a signature so it is the size of modulus.
Fixes #1283

On branch short-signatures
Changes to be committed:
modified: libopensc/pkcs15-sec.c

Checklist
  • Documentation is added or updated
  • New files have a LGPL 2.1 license statement
  • Tested with the following card:
    • tested PKCS#11
    • tested Windows Minidriver
    • tested macOS Tokend

@mouse07410
Copy link
Contributor

Very preliminary test shows that this PR works with OpenPGP 2.0 (YubiKey NEO) and 2.1 (YubiKey 4).

@dengert
Copy link
Member Author

dengert commented Apr 9, 2018

The fix for #1283 did not return the new length. 0674dfe adds r = modlen;. This needs to be tested with a card that fails, which I do not have.

I can squash after https://github.com/MSoegtrop tests with failing card.

Copy link
Member

@frankmorgner frankmorgner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

performing this check on the lower layer is a bit more tricky, so I'll accept this in the PKCS#15 layer.

Please have a look at the inline comment.

/* Some cards may return RSA signature as integer without leading zero bytes */
/* Already know outlen >= modlen and r >= 0 */
tmpoutlen = r;
if (obj->type == SC_PKCS15_TYPE_PRKEY_RSA && tmpoutlen < modlen) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix the indenting and rewrite the code so that tmpoutlen is not needed.

@frankmorgner
Copy link
Member

What's the testing status, @MSoegtrop?

Add leading zeros to RSA signature so it is the size of modulus.
Return modulus length.

 Changes to be committed:
	modified:   src/libopensc/pkcs15-sec.c
@dengert
Copy link
Member Author

dengert commented Jul 5, 2018

Removed temoutlen as requested and rebased into one commit. dde41bb
Tested with pkcs11-tool with card that does not have this problem.

@MSoegtrop This still needs to be tested using a card that has this problem.

@frankmorgner frankmorgner merged commit fbc9ff8 into OpenSC:master Jul 11, 2018
@frankmorgner
Copy link
Member

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants