Changeset 233138 in webkit


Ignore:
Timestamp:
Jun 25, 2018 12:05:51 AM (6 years ago)
Author:
zandobersek@gmail.com
Message:

Source/WebCore:
[GCrypt] Zero-prefix (if necessary) output of RSA-based encryption and signing operations
​https://bugs.webkit.org/show_bug.cgi?id=186967

Reviewed by Michael Catanzaro.

Output for RSA-based encryption and signing operations should match the
length of the RSA key. The way we retrieve the MPI data means libgcrypt
can ignore the high-bit zero values and leave us with a valid result
that's shorter in length compared to the RSA key. For instance, if the
output MPI fits into 2040 bits while a 2048-bit key was used we'll end
up with MPI data that will be fitted into a 255-byte Vector, one byte
short of the expected output length.

To avoid this, mpiZeroPrefixedData() is now used when retrieving output
of these RSA operations, and the value of the key size in bytes is
passed to it. This efficiently prepares the output Vector and then
copies the MPI data into it, respecting the MPI data length as well as
the desired length of the output.

No new tests -- relevant tests are now stable (i.e. not sporadically
failing anymore), associated expectations are removed.

  • crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp:

(WebCore::gcryptDerive): Also use mpiZeroPrefixedData().

  • crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp:

(WebCore::gcryptEncrypt):
(WebCore::CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt):

  • crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp:

(WebCore::gcryptSign):
(WebCore::CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign):

  • crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp:

(WebCore::gcryptEncrypt):
(WebCore::CryptoAlgorithmRSA_OAEP::platformEncrypt):

  • crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp:

(WebCore::gcryptSign):
(WebCore::CryptoAlgorithmRSA_PSS::platformSign):

  • crypto/gcrypt/GCryptUtilities.h:

(WebCore::mpiZeroPrefixedData):

LayoutTests:
[GCrypt] Zero-prefix (if necessary) RSA-OAEP encryption, RSA-PSS signing output
​https://bugs.webkit.org/show_bug.cgi?id=186967

Reviewed by Michael Catanzaro.

  • platform/gtk/TestExpectations: Remove flaky failures for RSA-OAEP and RSA-PSS tests.
  • platform/wpe/TestExpectations: Ditto.
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r233135 r233138  
     12018-06-25  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        [GCrypt] Zero-prefix (if necessary) RSA-OAEP encryption, RSA-PSS signing output
     4        https://bugs.webkit.org/show_bug.cgi?id=186967
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        * platform/gtk/TestExpectations: Remove flaky failures for RSA-OAEP and RSA-PSS tests.
     9        * platform/wpe/TestExpectations: Ditto.
     10
    1112018-06-24  Simon Fraser  <simon.fraser@apple.com>
    212

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r233080 r233138  
    18481848webkit.org/b/176757 fast/hidpi/filters-multiple.html [ Pass ImageOnlyFailure ]
    18491849
    1850 webkit.org/b/176759 crypto/subtle/rsa-oaep-import-key-wrap-jwk-oct-key.html [ Pass Failure ]
    1851 webkit.org/b/176759 crypto/workers/subtle/rsa-oaep-import-key-encrypt.html [ Pass Failure ]
    1852 webkit.org/b/176759 crypto/subtle/rsa-oaep-import-key-encrypt-label.html [ Pass Failure ]
    1853 
    18541850webkit.org/b/177527 [ Release ] fast/hidpi/filters-blur.html [ Pass ImageOnlyFailure ]
    18551851
    … …  
    18661862
    18671863webkit.org/b/179755 media/video-empty-source.html [ Pass Crash ]
    1868 
    1869 webkit.org/b/180095 imported/w3c/web-platform-tests/WebCryptoAPI/sign_verify/test_rsa_pss.https.html [ Pass Failure ]
    18701864
    18711865webkit.org/b/180369 http/tests/security/mixedContent/insecure-xhr-in-main-frame.html [ Pass Crash ]
    … …  
    18761870webkit.org/b/179948 [ Release ] fast/hidpi/filters-reference.html [ Pass ImageOnlyFailure ]
    18771871webkit.org/b/181031 fast/frames/crash-when-iframe-is-remove-in-eventhandler.html [ Pass Crash ]
    1878 
    1879 webkit.org/b/181139 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/test_rsa_oaep.https.html [ Pass Failure ]
    18801872
    18811873webkit.org/b/181528 http/tests/cache/memory-cache-pruning.html [ Pass Crash ]
    … …  
    19551947webkit.org/b/186638 imported/w3c/web-platform-tests/XMLHttpRequest/send-timeout-events.htm [ Failure Pass ]
    19561948webkit.org/b/186638 imported/w3c/web-platform-tests/IndexedDB/interleaved-cursors-large.html [ Failure Pass ]
    1957 webkit.org/b/186638 imported/w3c/web-platform-tests/WebCryptoAPI/sign_verify/rsa_pss.https.worker.html [ Failure Pass ]
    19581949webkit.org/b/186638 imported/w3c/web-platform-tests/html/webappapis/timers/negative-settimeout.html [ Failure Pass ]
    19591950webkit.org/b/186638 fast/selectors/hover-invalidation-descendant-dynamic.html [ Failure Pass ]
    … …  
    19631954webkit.org/b/186638 compositing/patterns/direct-pattern-compositing.html [ ImageOnlyFailure Pass ]
    19641955webkit.org/b/186638 imported/w3c/web-platform-tests/mathml/relations/html5-tree/color-attributes-1.html [ Timeout Pass ]
    1965 webkit.org/b/186638 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/rsa.https.worker.html [ Failure Pass ]
    19661956
    19671957webkit.org/b/186664 media/video-playsinline.html [ Failure Pass ]

  • trunk/LayoutTests/platform/wpe/TestExpectations

    r233031 r233138  
    10271027
    10281028webkit.org/b/176174 media/event-queue-crash.html [ Pass Failure ]
    1029 
    1030 webkit.org/b/176358 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/rsa.https.worker.html [ Failure Pass ]
    10311029webkit.org/b/177226 imported/w3c/web-platform-tests/WebCryptoAPI/wrapKey_unwrapKey/wrapKey_unwrapKey.https.worker.html [ Pass Failure ]
    10321030
    … …  
    11871185webkit.org/b/182240 svg/animations/svgenum-animation-7.html [ Pass Crash ]
    11881186
    1189 webkit.org/b/181139 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/test_rsa_oaep.https.html [ Pass Failure ]
    1190 
    11911187webkit.org/b/184086 imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html [ Failure ]
    11921188webkit.org/b/184086 imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html [ Failure ]

  • trunk/Source/WebCore/ChangeLog

    r233136 r233138  
     12018-06-25  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        [GCrypt] Zero-prefix (if necessary) output of RSA-based encryption and signing operations
     4        https://bugs.webkit.org/show_bug.cgi?id=186967
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        Output for RSA-based encryption and signing operations should match the
     9        length of the RSA key. The way we retrieve the MPI data means libgcrypt
     10        can ignore the high-bit zero values and leave us with a valid result
     11        that's shorter in length compared to the RSA key. For instance, if the
     12        output MPI fits into 2040 bits while a 2048-bit key was used we'll end
     13        up with MPI data that will be fitted into a 255-byte Vector, one byte
     14        short of the expected output length.
     15
     16        To avoid this, mpiZeroPrefixedData() is now used when retrieving output
     17        of these RSA operations, and the value of the key size in bytes is
     18        passed to it. This efficiently prepares the output Vector and then
     19        copies the MPI data into it, respecting the MPI data length as well as
     20        the desired length of the output.
     21
     22        No new tests -- relevant tests are now stable (i.e. not sporadically
     23        failing anymore), associated expectations are removed.
     24
     25        * crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp:
     26        (WebCore::gcryptDerive): Also use mpiZeroPrefixedData().
     27        * crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp:
     28        (WebCore::gcryptEncrypt):
     29        (WebCore::CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt):
     30        * crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp:
     31        (WebCore::gcryptSign):
     32        (WebCore::CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign):
     33        * crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp:
     34        (WebCore::gcryptEncrypt):
     35        (WebCore::CryptoAlgorithmRSA_OAEP::platformEncrypt):
     36        * crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp:
     37        (WebCore::gcryptSign):
     38        (WebCore::CryptoAlgorithmRSA_PSS::platformSign):
     39        * crypto/gcrypt/GCryptUtilities.h:
     40        (WebCore::mpiZeroPrefixedData):
     41
    1422018-06-24  Simon Fraser  <simon.fraser@apple.com>
    243

  • trunk/Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp

    r224173 r233138  
    9898    }
    9999
    100     auto data = mpiData(xMPI);
    101     if (!data)
    102         return std::nullopt;
    103 
    104     if (data->size() < keySizeInBytes) {
    105         Vector<uint8_t> paddedData(keySizeInBytes - data->size(), 0);
    106         paddedData.appendVector(*data);
    107         *data = WTFMove(paddedData);
    108     }
    109 
    110     return data;
     100    return mpiZeroPrefixedData(xMPI, keySizeInBytes);
    111101}
    112102

  • trunk/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp

    r221665 r233138  
    3434namespace WebCore {
    3535
    36 static std::optional<Vector<uint8_t>> gcryptEncrypt(gcry_sexp_t keySexp, const Vector<uint8_t>& plainText)
     36static std::optional<Vector<uint8_t>> gcryptEncrypt(gcry_sexp_t keySexp, const Vector<uint8_t>& plainText, size_t keySizeInBytes)
    3737{
    3838    // Embed the plain-text data in a `data` s-expression using PKCS#1 padding.
    … …  
    6161        return std::nullopt;
    6262
    63     return mpiData(aSexp);
     63    return mpiZeroPrefixedData(aSexp, keySizeInBytes);
    6464}
    6565
    … …  
    9696ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt(const CryptoKeyRSA& key, const Vector<uint8_t>& plainText)
    9797{
    98     auto output = gcryptEncrypt(key.platformKey(), plainText);
     98    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
     99    auto output = gcryptEncrypt(key.platformKey(), plainText, key.keySizeInBits() / 8);
    99100    if (!output)
    100101        return Exception { OperationError };

  • trunk/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp

    r221665 r233138  
    3636namespace WebCore {
    3737
    38 static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier)
     38static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t keySizeInBytes)
    3939{
    4040    // Perform digest operation with the specified algorithm on the given data.
    … …  
    8484        return std::nullopt;
    8585
    86     return mpiData(sSexp);
     86    return mpiZeroPrefixedData(sSexp, keySizeInBytes);
    8787}
    8888
    … …  
    137137ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign(const CryptoKeyRSA& key, const Vector<uint8_t>& data)
    138138{
    139     auto output = gcryptSign(key.platformKey(), data, key.hashAlgorithmIdentifier());
     139    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
     140    auto output = gcryptSign(key.platformKey(), data, key.hashAlgorithmIdentifier(), key.keySizeInBits() / 8);
    140141    if (!output)
    141142        return Exception { OperationError };

  • trunk/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp

    r221665 r233138  
    3636namespace WebCore {
    3737
    38 static std::optional<Vector<uint8_t>> gcryptEncrypt(CryptoAlgorithmIdentifier hashAlgorithmIdentifier, gcry_sexp_t keySexp, const Vector<uint8_t>& labelVector, const Vector<uint8_t>& plainText)
     38static std::optional<Vector<uint8_t>> gcryptEncrypt(CryptoAlgorithmIdentifier hashAlgorithmIdentifier, gcry_sexp_t keySexp, const Vector<uint8_t>& labelVector, const Vector<uint8_t>& plainText, size_t keySizeInBytes)
    3939{
    4040    // Embed the plain-text data in a data s-expression using OAEP padding.
    … …  
    7171        return std::nullopt;
    7272
    73     return mpiData(aSexp);
     73    return mpiZeroPrefixedData(aSexp, keySizeInBytes);
    7474}
    7575
    … …  
    113113ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSA_OAEP::platformEncrypt(CryptoAlgorithmRsaOaepParams& parameters, const CryptoKeyRSA& key, const Vector<uint8_t>& plainText)
    114114{
    115     auto output = gcryptEncrypt(key.hashAlgorithmIdentifier(), key.platformKey(), parameters.labelVector(), plainText);
     115    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
     116    auto output = gcryptEncrypt(key.hashAlgorithmIdentifier(), key.platformKey(), parameters.labelVector(), plainText, key.keySizeInBits() / 8);
    116117    if (!output)
    117118        return Exception { OperationError };

  • trunk/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp

    r221665 r233138  
    3737namespace WebCore {
    3838
    39 static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, size_t keyLength, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t saltLength)
     39static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t saltLength, size_t keySizeInBytes)
    4040{
    4141    // Perform digest operation with the specified algorithm on the given data.
    … …  
    8585        return std::nullopt;
    8686
    87     // Validate the MPI data size -- it should match the key length in bytes.
    88     auto signature = mpiData(sSexp);
    89     if (!signature || signature->size() != keyLength / 8)
    90         return std::nullopt;
    91 
    92     return signature;
     87    return mpiZeroPrefixedData(sSexp, keySizeInBytes);
    9388}
    9489
    … …  
    143138ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSA_PSS::platformSign(CryptoAlgorithmRsaPssParams& parameters, const CryptoKeyRSA& key, const Vector<uint8_t>& data)
    144139{
    145     auto output = gcryptSign(key.platformKey(), key.keySizeInBits(), data, key.hashAlgorithmIdentifier(), parameters.saltLength);
     140    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
     141    auto output = gcryptSign(key.platformKey(), data, key.hashAlgorithmIdentifier(), parameters.saltLength, key.keySizeInBits() / 8);
    146142    if (!output)
    147143        return Exception { OperationError };

  • trunk/Source/WebCore/crypto/gcrypt/GCryptUtilities.h

    r224189 r233138  
    181181}
    182182
     183static inline std::optional<Vector<uint8_t>> mpiZeroPrefixedData(gcry_mpi_t paramMPI, size_t targetLength)
     184{
     185    // Retrieve the MPI length. Bail if the retrieved length is longer than target length.
     186    auto length = mpiLength(paramMPI);
     187    if (!length || *length > targetLength)
     188        return std::nullopt;
     189
     190    // Fill out the output buffer with zeros. Properly determine the zero prefix length,
     191    // and copy the MPI data into memory area following the prefix (if any).
     192    Vector<uint8_t> output(targetLength, 0);
     193    size_t prefixLength = targetLength - *length;
     194    gcry_error_t error = gcry_mpi_print(GCRYMPI_FMT_USG, output.data() + prefixLength, targetLength, nullptr, paramMPI);
     195    if (error != GPG_ERR_NO_ERROR) {
     196        PAL::GCrypt::logError(error);
     197        return std::nullopt;
     198    }
     199
     200    return output;
     201}
     202
    183203static inline std::optional<Vector<uint8_t>> mpiData(gcry_sexp_t paramSexp)
    184204{
    … …  
    191211}
    192212
     213static inline std::optional<Vector<uint8_t>> mpiZeroPrefixedData(gcry_sexp_t paramSexp, size_t targetLength)
     214{
     215    // Retrieve the MPI value stored in the s-expression: (name mpi-data)
     216    PAL::GCrypt::Handle<gcry_mpi_t> paramMPI(gcry_sexp_nth_mpi(paramSexp, 1, GCRYMPI_FMT_USG));
     217    if (!paramMPI)
     218        return std::nullopt;
     219
     220    return mpiZeroPrefixedData(paramMPI, targetLength);
     221}
     222
    193223static inline std::optional<Vector<uint8_t>> mpiSignedData(gcry_mpi_t mpi)
    194224{
Note: See TracChangeset for help on using the changeset viewer.