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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@233138 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 15a0b6b..7ed3096 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2018-06-25 Zan Dobersek <zdobersek@igalia.com>
+
+ [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.
+
2018-06-24 Simon Fraser <simon.fraser@apple.com>
Fix the composition underline to be transformed by -apple-color-filter
diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations
index daf2275..9ab3e66 100644
--- a/LayoutTests/platform/gtk/TestExpectations
+++ b/LayoutTests/platform/gtk/TestExpectations
@@ -1847,10 +1847,6 @@
webkit.org/b/176757 fast/hidpi/filters-invert.html [ Pass ImageOnlyFailure ]
webkit.org/b/176757 fast/hidpi/filters-multiple.html [ Pass ImageOnlyFailure ]
-webkit.org/b/176759 crypto/subtle/rsa-oaep-import-key-wrap-jwk-oct-key.html [ Pass Failure ]
-webkit.org/b/176759 crypto/workers/subtle/rsa-oaep-import-key-encrypt.html [ Pass Failure ]
-webkit.org/b/176759 crypto/subtle/rsa-oaep-import-key-encrypt-label.html [ Pass Failure ]
-
webkit.org/b/177527 [ Release ] fast/hidpi/filters-blur.html [ Pass ImageOnlyFailure ]
webkit.org/b/177530 imported/w3c/web-platform-tests/XMLHttpRequest/setrequestheader-header-allowed.htm [ Pass Failure ]
@@ -1866,8 +1862,6 @@
webkit.org/b/179755 media/video-empty-source.html [ Pass Crash ]
-webkit.org/b/180095 imported/w3c/web-platform-tests/WebCryptoAPI/sign_verify/test_rsa_pss.https.html [ Pass Failure ]
-
webkit.org/b/180369 http/tests/security/mixedContent/insecure-xhr-in-main-frame.html [ Pass Crash ]
webkit.org/b/179174 inspector/console/webcore-logging.html [ Failure Crash ]
@@ -1876,8 +1870,6 @@
webkit.org/b/179948 [ Release ] fast/hidpi/filters-reference.html [ Pass ImageOnlyFailure ]
webkit.org/b/181031 fast/frames/crash-when-iframe-is-remove-in-eventhandler.html [ Pass Crash ]
-webkit.org/b/181139 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/test_rsa_oaep.https.html [ Pass Failure ]
-
webkit.org/b/181528 http/tests/cache/memory-cache-pruning.html [ Pass Crash ]
webkit.org/b/181529 imported/blink/http/tests/webfont/animation-assert.html [ Pass Crash ]
@@ -1954,7 +1946,6 @@
webkit.org/b/186638 imported/w3c/web-platform-tests/WebCryptoAPI/wrapKey_unwrapKey/wrapKey_unwrapKey.https.worker.html [ Failure Pass ]
webkit.org/b/186638 imported/w3c/web-platform-tests/XMLHttpRequest/send-timeout-events.htm [ Failure Pass ]
webkit.org/b/186638 imported/w3c/web-platform-tests/IndexedDB/interleaved-cursors-large.html [ Failure Pass ]
-webkit.org/b/186638 imported/w3c/web-platform-tests/WebCryptoAPI/sign_verify/rsa_pss.https.worker.html [ Failure Pass ]
webkit.org/b/186638 imported/w3c/web-platform-tests/html/webappapis/timers/negative-settimeout.html [ Failure Pass ]
webkit.org/b/186638 fast/selectors/hover-invalidation-descendant-dynamic.html [ Failure Pass ]
webkit.org/b/186638 fast/selectors/hover-quirks.html [ Failure Pass ]
@@ -1962,7 +1953,6 @@
webkit.org/b/186638 fast/selectors/unqualified-hover-strict.html [ Failure Pass ]
webkit.org/b/186638 compositing/patterns/direct-pattern-compositing.html [ ImageOnlyFailure Pass ]
webkit.org/b/186638 imported/w3c/web-platform-tests/mathml/relations/html5-tree/color-attributes-1.html [ Timeout Pass ]
-webkit.org/b/186638 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/rsa.https.worker.html [ Failure Pass ]
webkit.org/b/186664 media/video-playsinline.html [ Failure Pass ]
diff --git a/LayoutTests/platform/wpe/TestExpectations b/LayoutTests/platform/wpe/TestExpectations
index 7effaee..3132d0e 100644
--- a/LayoutTests/platform/wpe/TestExpectations
+++ b/LayoutTests/platform/wpe/TestExpectations
@@ -1026,8 +1026,6 @@
webkit.org/b/172815 imported/w3c/i18n/bidi/bidi-plaintext-011.html [ Pass ImageOnlyFailure ]
webkit.org/b/176174 media/event-queue-crash.html [ Pass Failure ]
-
-webkit.org/b/176358 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/rsa.https.worker.html [ Failure Pass ]
webkit.org/b/177226 imported/w3c/web-platform-tests/WebCryptoAPI/wrapKey_unwrapKey/wrapKey_unwrapKey.https.worker.html [ Pass Failure ]
# Apparently fails on Mac, but I don't know for what reasons. It passes on GTK/WPE.
@@ -1186,8 +1184,6 @@
webkit.org/b/182240 svg/animations/svgenum-animation-7.html [ Pass Crash ]
-webkit.org/b/181139 imported/w3c/web-platform-tests/WebCryptoAPI/encrypt_decrypt/test_rsa_oaep.https.html [ Pass Failure ]
-
webkit.org/b/184086 imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html [ Failure ]
webkit.org/b/184086 imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html [ Failure ]
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index dd86d6f..ef69048 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,44 @@
+2018-06-25 Zan Dobersek <zdobersek@igalia.com>
+
+ [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):
+
2018-06-24 Simon Fraser <simon.fraser@apple.com>
Fix the DUMP_NODE_STATISTICS code so that it compiles
diff --git a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp
index 13dcf96..95aca92 100644
--- a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp
+++ b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmECDHGCrypt.cpp
@@ -97,17 +97,7 @@
gcry_mpi_point_snatch_get(xMPI, nullptr, nullptr, point.release());
}
- auto data = mpiData(xMPI);
- if (!data)
- return std::nullopt;
-
- if (data->size() < keySizeInBytes) {
- Vector<uint8_t> paddedData(keySizeInBytes - data->size(), 0);
- paddedData.appendVector(*data);
- *data = WTFMove(paddedData);
- }
-
- return data;
+ return mpiZeroPrefixedData(xMPI, keySizeInBytes);
}
std::optional<Vector<uint8_t>> CryptoAlgorithmECDH::platformDeriveBits(const CryptoKeyEC& baseKey, const CryptoKeyEC& publicKey)
diff --git a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp
index 3044ace..15ab8e0 100644
--- a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp
+++ b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSAES_PKCS1_v1_5GCrypt.cpp
@@ -33,7 +33,7 @@
namespace WebCore {
-static std::optional<Vector<uint8_t>> gcryptEncrypt(gcry_sexp_t keySexp, const Vector<uint8_t>& plainText)
+static std::optional<Vector<uint8_t>> gcryptEncrypt(gcry_sexp_t keySexp, const Vector<uint8_t>& plainText, size_t keySizeInBytes)
{
// Embed the plain-text data in a `data` s-expression using PKCS#1 padding.
PAL::GCrypt::Handle<gcry_sexp_t> dataSexp;
@@ -60,7 +60,7 @@
if (!aSexp)
return std::nullopt;
- return mpiData(aSexp);
+ return mpiZeroPrefixedData(aSexp, keySizeInBytes);
}
static std::optional<Vector<uint8_t>> gcryptDecrypt(gcry_sexp_t keySexp, const Vector<uint8_t>& cipherText)
@@ -95,7 +95,8 @@
ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt(const CryptoKeyRSA& key, const Vector<uint8_t>& plainText)
{
- auto output = gcryptEncrypt(key.platformKey(), plainText);
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
+ auto output = gcryptEncrypt(key.platformKey(), plainText, key.keySizeInBits() / 8);
if (!output)
return Exception { OperationError };
return WTFMove(*output);
diff --git a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp
index 65b5981..ec3b61a 100644
--- a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp
+++ b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSASSA_PKCS1_v1_5GCrypt.cpp
@@ -35,7 +35,7 @@
namespace WebCore {
-static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier)
+static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t keySizeInBytes)
{
// Perform digest operation with the specified algorithm on the given data.
Vector<uint8_t> dataHash;
@@ -83,7 +83,7 @@
if (!sSexp)
return std::nullopt;
- return mpiData(sSexp);
+ return mpiZeroPrefixedData(sSexp, keySizeInBytes);
}
static std::optional<bool> gcryptVerify(gcry_sexp_t keySexp, const Vector<uint8_t>& signature, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier)
@@ -136,7 +136,8 @@
ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign(const CryptoKeyRSA& key, const Vector<uint8_t>& data)
{
- auto output = gcryptSign(key.platformKey(), data, key.hashAlgorithmIdentifier());
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
+ auto output = gcryptSign(key.platformKey(), data, key.hashAlgorithmIdentifier(), key.keySizeInBits() / 8);
if (!output)
return Exception { OperationError };
return WTFMove(*output);
diff --git a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp
index 0c066b7..1a668f2 100644
--- a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp
+++ b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_OAEPGCrypt.cpp
@@ -35,7 +35,7 @@
namespace WebCore {
-static std::optional<Vector<uint8_t>> gcryptEncrypt(CryptoAlgorithmIdentifier hashAlgorithmIdentifier, gcry_sexp_t keySexp, const Vector<uint8_t>& labelVector, const Vector<uint8_t>& plainText)
+static 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)
{
// Embed the plain-text data in a data s-expression using OAEP padding.
// Empty label data is properly handled by gcry_sexp_build().
@@ -70,7 +70,7 @@
if (!aSexp)
return std::nullopt;
- return mpiData(aSexp);
+ return mpiZeroPrefixedData(aSexp, keySizeInBytes);
}
static std::optional<Vector<uint8_t>> gcryptDecrypt(CryptoAlgorithmIdentifier hashAlgorithmIdentifier, gcry_sexp_t keySexp, const Vector<uint8_t>& labelVector, const Vector<uint8_t>& cipherText)
@@ -112,7 +112,8 @@
ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSA_OAEP::platformEncrypt(CryptoAlgorithmRsaOaepParams& parameters, const CryptoKeyRSA& key, const Vector<uint8_t>& plainText)
{
- auto output = gcryptEncrypt(key.hashAlgorithmIdentifier(), key.platformKey(), parameters.labelVector(), plainText);
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
+ auto output = gcryptEncrypt(key.hashAlgorithmIdentifier(), key.platformKey(), parameters.labelVector(), plainText, key.keySizeInBits() / 8);
if (!output)
return Exception { OperationError };
return WTFMove(*output);
diff --git a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp
index 95bfc40..922cc9b 100644
--- a/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp
+++ b/Source/WebCore/crypto/gcrypt/CryptoAlgorithmRSA_PSSGCrypt.cpp
@@ -36,7 +36,7 @@
namespace WebCore {
-static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, size_t keyLength, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t saltLength)
+static std::optional<Vector<uint8_t>> gcryptSign(gcry_sexp_t keySexp, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t saltLength, size_t keySizeInBytes)
{
// Perform digest operation with the specified algorithm on the given data.
Vector<uint8_t> dataHash;
@@ -84,12 +84,7 @@
if (!sSexp)
return std::nullopt;
- // Validate the MPI data size -- it should match the key length in bytes.
- auto signature = mpiData(sSexp);
- if (!signature || signature->size() != keyLength / 8)
- return std::nullopt;
-
- return signature;
+ return mpiZeroPrefixedData(sSexp, keySizeInBytes);
}
static std::optional<bool> gcryptVerify(gcry_sexp_t keySexp, const Vector<uint8_t>& signature, const Vector<uint8_t>& data, CryptoAlgorithmIdentifier hashAlgorithmIdentifier, size_t saltLength)
@@ -142,7 +137,8 @@
ExceptionOr<Vector<uint8_t>> CryptoAlgorithmRSA_PSS::platformSign(CryptoAlgorithmRsaPssParams& parameters, const CryptoKeyRSA& key, const Vector<uint8_t>& data)
{
- auto output = gcryptSign(key.platformKey(), key.keySizeInBits(), data, key.hashAlgorithmIdentifier(), parameters.saltLength);
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!(key.keySizeInBits() % 8));
+ auto output = gcryptSign(key.platformKey(), data, key.hashAlgorithmIdentifier(), parameters.saltLength, key.keySizeInBits() / 8);
if (!output)
return Exception { OperationError };
return WTFMove(*output);
diff --git a/Source/WebCore/crypto/gcrypt/GCryptUtilities.h b/Source/WebCore/crypto/gcrypt/GCryptUtilities.h
index 6bd1e15..d9a67c6 100644
--- a/Source/WebCore/crypto/gcrypt/GCryptUtilities.h
+++ b/Source/WebCore/crypto/gcrypt/GCryptUtilities.h
@@ -180,6 +180,26 @@
return output;
}
+static inline std::optional<Vector<uint8_t>> mpiZeroPrefixedData(gcry_mpi_t paramMPI, size_t targetLength)
+{
+ // Retrieve the MPI length. Bail if the retrieved length is longer than target length.
+ auto length = mpiLength(paramMPI);
+ if (!length || *length > targetLength)
+ return std::nullopt;
+
+ // Fill out the output buffer with zeros. Properly determine the zero prefix length,
+ // and copy the MPI data into memory area following the prefix (if any).
+ Vector<uint8_t> output(targetLength, 0);
+ size_t prefixLength = targetLength - *length;
+ gcry_error_t error = gcry_mpi_print(GCRYMPI_FMT_USG, output.data() + prefixLength, targetLength, nullptr, paramMPI);
+ if (error != GPG_ERR_NO_ERROR) {
+ PAL::GCrypt::logError(error);
+ return std::nullopt;
+ }
+
+ return output;
+}
+
static inline std::optional<Vector<uint8_t>> mpiData(gcry_sexp_t paramSexp)
{
// Retrieve the MPI value stored in the s-expression: (name mpi-data)
@@ -190,6 +210,16 @@
return mpiData(paramMPI);
}
+static inline std::optional<Vector<uint8_t>> mpiZeroPrefixedData(gcry_sexp_t paramSexp, size_t targetLength)
+{
+ // Retrieve the MPI value stored in the s-expression: (name mpi-data)
+ PAL::GCrypt::Handle<gcry_mpi_t> paramMPI(gcry_sexp_nth_mpi(paramSexp, 1, GCRYMPI_FMT_USG));
+ if (!paramMPI)
+ return std::nullopt;
+
+ return mpiZeroPrefixedData(paramMPI, targetLength);
+}
+
static inline std::optional<Vector<uint8_t>> mpiSignedData(gcry_mpi_t mpi)
{
auto data = mpiData(mpi);
