Changeset 217969 in webkit


Ignore:
Timestamp:
Jun 9, 2017 1:47:43 AM (7 years ago)
Author:
zandobersek@gmail.com
Message:

[GCrypt] ECDSA signing results can be smaller than the EC key size
https://bugs.webkit.org/show_bug.cgi?id=171535

Reviewed by Jiewen Tan.

The libgcrypt-based implementation of the ECDSA signing operation does not
properly address the resulting r and s integers that do not potentially
match the EC key in terms of size.

To address that, the retrieved MPI data for both integers is handled depending
on the size of said data. Strictly requiring an amount of bytes that matches
the key data size N, we simply take the last N bytes if the MPI data is equal
or larger than N in size. If smaller, we first append enough zero bytes to the
output Vector object before attaching the MPI data in whole so that the amount
of appended bytes matches N.

No new tests -- covers an implementation detail that is not trivial to test,
but can rely sufficiently on existing tests.

  • crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:

(WebCore::extractECDSASignatureInteger):
(WebCore::gcryptSign):

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r217968 r217969  
     12017-06-09  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        [GCrypt] ECDSA signing results can be smaller than the EC key size
     4        https://bugs.webkit.org/show_bug.cgi?id=171535
     5
     6        Reviewed by Jiewen Tan.
     7
     8        The libgcrypt-based implementation of the ECDSA signing operation does not
     9        properly address the resulting `r` and `s` integers that do not potentially
     10        match the EC key in terms of size.
     11
     12        To address that, the retrieved MPI data for both integers is handled depending
     13        on the size of said data. Strictly requiring an amount of bytes that matches
     14        the key data size N, we simply take the last N bytes if the MPI data is equal
     15        or larger than N in size. If smaller, we first append enough zero bytes to the
     16        output Vector object before attaching the MPI data in whole so that the amount
     17        of appended bytes matches N.
     18
     19        No new tests -- covers an implementation detail that is not trivial to test,
     20        but can rely sufficiently on existing tests.
     21
     22        * crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp:
     23        (WebCore::extractECDSASignatureInteger):
     24        (WebCore::gcryptSign):
     25
    1262017-06-09  Daewoong Jang  <daewoong.jang@navercorp.com>
    227
  • trunk/Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDSAGCrypt.cpp

    r217754 r217969  
    4040namespace WebCore {
    4141
     42static bool extractECDSASignatureInteger(Vector<uint8_t>& signature, gcry_sexp_t signatureSexp, const char* integerName, size_t keySizeInBytes)
     43{
     44    // Retrieve byte data of the specified integer.
     45    PAL::GCrypt::Handle<gcry_sexp_t> integerSexp(gcry_sexp_find_token(signatureSexp, integerName, 0));
     46    if (!integerSexp)
     47        return false;
     48
     49    auto integerData = mpiData(integerSexp);
     50    if (!integerData)
     51        return false;
     52
     53    size_t dataSize = integerData->size();
     54    if (dataSize >= keySizeInBytes) {
     55        // Append the last `keySizeInBytes` bytes of the data Vector, if available.
     56        signature.append(&integerData->at(dataSize - keySizeInBytes), keySizeInBytes);
     57    } else {
     58        // If not, prefix the binary data with zero bytes.
     59        for (size_t paddingSize = keySizeInBytes - dataSize; paddingSize > 0; --paddingSize)
     60            signature.uncheckedAppend(0x00);
     61        signature.appendVector(*integerData);
     62    }
     63
     64    return true;
     65}
     66
    4267static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t keySizeInBytes)
    4368{
     
    85110
    86111    // Retrieve MPI data of the resulting r and s integers. They are concatenated into
    87     // a single buffer after checking that the data length matches the key size.
    88     // FIXME: But r and s integers can still be valid even if they're of shorter size.
    89     // https://bugs.webkit.org/show_bug.cgi?id=171535
     112    // a single buffer, properly accounting for integers that don't match the key in size.
    90113    Vector<uint8_t> signature;
    91     {
    92         PAL::GCrypt::Handle<gcry_sexp_t> rSexp(gcry_sexp_find_token(signatureSexp, "r", 0));
    93         if (!rSexp)
    94             return std::nullopt;
    95 
    96         auto rData = mpiData(rSexp);
    97         if (!rData || rData->size() != keySizeInBytes)
    98             return std::nullopt;
    99 
    100         signature.appendVector(*rData);
    101     }
    102     {
    103         PAL::GCrypt::Handle<gcry_sexp_t> sSexp(gcry_sexp_find_token(signatureSexp, "s", 0));
    104         if (!sSexp)
    105             return std::nullopt;
    106 
    107         auto sData = mpiData(sSexp);
    108         if (!sData || sData->size() != keySizeInBytes)
    109             return std::nullopt;
    110 
    111         signature.appendVector(*sData);
    112     }
     114    signature.reserveInitialCapacity(keySizeInBytes * 2);
     115
     116    if (!extractECDSASignatureInteger(signature, signatureSexp, "r", keySizeInBytes)
     117        || !extractECDSASignatureInteger(signature, signatureSexp, "s", keySizeInBytes))
     118        return std::nullopt;
    113119
    114120    return signature;
Note: See TracChangeset for help on using the changeset viewer.