Re: Signature::Decode bug?

[Bruce Stephens <bruce.stephens@xxxxxxxxx>]

"Nicholas, Richard  (US SSA)" <Richard.Nicholas@xxxxxxxxxxxxxxxxx> writes:

> Bruce,
>
> You are correct, the signature decoding logic in
> cml/cmapi/src/CM_Signature.cpp, lines 363-420, is wrong.  The DSA code
> should simply use 20 bytes as the length of both the r and s values
> (since only SHA-1 is supported).
>
> For ECDSA, the length of r and s should both be nLen, which is not known
> to the Signature::Decode() function.  Therefore, for ECDSA, the code
> should simply copy the r and s values from the decoded structure without
> padding.  (The correct padding will need to be performed prior to
> performing signature verification.)

Good, I feared something more complex was involved.  (I wasn't aware
of the cml list, which is obviously more appropriate than the sfl
list.)

The change I made just uses snaccSig.r.length() and
snaccSig.s.length(), producing a buffer of length 2*max(r.length(),
s.length()) in the obvious way.  I believe that's sufficient for ECDSA
(the spec says that the buffer is "at most" 2 nLen, so shorter is OK).
And that's straightforward enough.

Ah, I guess if both high order octets are >= 0x80 then r.length() and
s.length() might be too big (including the zero required for
encoding).  In that case the code I did is (at least technically)
wrong for PKCS#11.  Bother.  However, that's easy enough to fix.

(Anyway, it doesn't affect Isode's use (I wrote an OpenSSL_Handle, and
the ECDSA code won't fail on a buffer with that unnecessary padding).)

[...]