smimesign icon indicating copy to clipboard operation
smimesign copied to clipboard

Published 20 hours ago • verified publisher icon github

Use appropriate digest algorithm during signature creation

Open oncilla opened this issue 3 years ago • 4 comments

Pass the public key instead of the marshalled public key to digestAlgorithmForPublicKey in SignedData.AddSignerInfo.

Previously, the marshalled public key was passed instead of the actual public key. The result is that always SHA256 was being selected, even for ECDSA where the hash algorithm should be selected based on the curve.

oncilla avatar Oct 30 '21 22:10 oncilla

Hi @lgarron

What can I do to move this forward?

oncilla avatar Nov 13 '21 08:11 oncilla

I'm not qualified to review this, but @vcsjones may be!

lgarron avatar Nov 14 '21 00:11 lgarron

I see:

https://github.com/github/smimesign/blob/e650daf6eaadf85c763fa06dcfb0e4d794d293dc/ietf-cms/protocol/protocol.go#L755-L766

Since we're passing in something that is not an ecdsa.PublicKey, none of the if checks match and it defaults to SHA256.

I think this change looks good, but it would be great to get some test coverage that indeed the right digest algorithm is used for the curve. Would you be able to add some test coverage for this, @oncilla?

vcsjones avatar Nov 16 '21 22:11 vcsjones

@vcsjones sure. I will have look tomorrow

oncilla avatar Nov 16 '21 22:11 oncilla