[PATCH] Truncate hash values for ECDSA signature scheme

[Dmitry Eremin-Solenikov]

On Tue, Dec 17, 2013 at 12:52 PM, Werner Koch <wk at gnupg.org> wrote:
> On Tue, 17 Dec 2013 08:37, nmav at gnutls.org said:
>
>> My understanding is that truncation applies to both DSA and ECDSA (I'm
>
> Right.
>
>> not aware of the difference in opaque-mpis and normal ones though). It
>> is more interesting that truncation should also apply on the bit-level
>> (i.e., on a curve of 255 bits, the truncation of SHA256 should be done
>> by a single bit), but I don't think any implementation does that.
>
> Libgcrypt does that; itwas not readily accesible by my brain, given that
> I mostly did EdDSA stuff the last weeks:
>
>   /* Convert the INPUT into an MPI if needed.  */
>   if (mpi_is_opaque (input))
>     {
>       abuf = mpi_get_opaque (input, &abits);
>       rc = _gcry_mpi_scan (&hash, GCRYMPI_FMT_USG, abuf, (abits+7)/8, NULL);
>       if (rc)
>         return rc;
>       if (abits > qbits)
>         mpi_rshift (hash, hash, abits - qbits);
>     }
>   else
>     hash = input;

I was looking onto this code. If mpi is opaque, it will be shifted. Hopefully.
If not, it will be used as is - not shifted! I think this code should
be reimplemented as following
(more or less)

if (mpi_is_opaque(input))
  {
      abuf = mpi_get_opaque (input, &abits);
      rc = _gcry_mpi_scan (&hash, GCRYMPI_FMT_USG, abuf, (abits+7)/8, NULL);
  }
  else
  {
    hash = mpi_copy(input)
    abits = mpi_get_nbits(input);
  }
  if (abits > qbits)
    mpi_rshift (hash, hash, abits - qbits);

This would be more correct, isn't it?

> I am not 100% that the conversion to an unsigned integer and then
> shifting the MPI is the right solution.  However, given that the same
> code is in the DSA code and that passed the FIPS validation, it should
> be okay.
>
> In general I prefer to use opaque MPIs for hash values because that
> avoids the leading zero problems and is anyway better for data which is
> not a number.  For historic reasons an MPI is sometimes to passed to the
> fucntions and thus we need to implement the two cases.

This makes sense.

-- 
With best wishes
Dmitry