Bug 227540 – [MacOS wk1] crypto/workers/subtle/hrsa-postMessage-worker.html is a flaky failure

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

crypto/workers/subtle/hrsa-postMessage-worker.html

History: https://results.webkit.org/?suite=layout-tests&test=crypto%2Fworkers%2Fsubtle%2Fhrsa-postMessage-worker.html

This test appeared to be timing specific in its results reporting

--- /Volumes/Data/worker/bigsur-debug-tests-wk1/build/layout-test-results/crypto/workers/subtle/hrsa-postMessage-worker-expected.txt
+++ /Volumes/Data/worker/bigsur-debug-tests-wk1/build/layout-test-results/crypto/workers/subtle/hrsa-postMessage-worker-actual.txt
@@ -3,6 +3,14 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+PASS All checks passed in worker for public key
+PASS publicKey.type is 'public'
+PASS publicKey.extractable is true
+PASS publicKey.algorithm.name is 'RSASSA-PKCS1-v1_5'
+PASS publicKey.algorithm.modulusLength is 2048
+PASS bytesToHexString(publicKey.algorithm.publicExponent) is '010001'
+PASS publicKey.algorithm.hash.name is 'SHA-256'
+PASS publicKey.usages is ['verify']
 PASS All checks passed in worker for private key
 PASS privateKey.type is 'private'
 PASS privateKey.extractable is false
@@ -11,14 +19,6 @@
 PASS bytesToHexString(privateKey.algorithm.publicExponent) is '010001'
 PASS privateKey.algorithm.hash.name is 'SHA-256'
 PASS privateKey.usages is ['sign']
-PASS All checks passed in worker for public key
-PASS publicKey.type is 'public'
-PASS publicKey.extractable is true
-PASS publicKey.algorithm.name is 'RSASSA-PKCS1-v1_5'
-PASS publicKey.algorithm.modulusLength is 2048
-PASS bytesToHexString(publicKey.algorithm.publicExponent) is '010001'
-PASS publicKey.algorithm.hash.name is 'SHA-256'
-PASS publicKey.usages is ['verify']
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Radar WebKit Bug Importer]

<rdar://problem/79977662>

Created attachment 432624 [details]
Patch

Created attachment 432631 [details]
Patch

Created attachment 432632 [details]
TestExpectations update

[Truitt Savell]

Comment on attachment 432632 [details]
TestExpectations update

Clearing flags on attachment: 432632

Committed r279437 (239294@main): <https://commits.webkit.org/239294@main>

[Chris Dumez]

This is not an issue with the test but rather is really bug in Crypto key deserialization:

38  0x16325d3ad WebCore::ScriptExecutionContext::Task::performTask(WebCore::ScriptExecutionContext&)
39  0x165a32f5d WebCore::WorkerRunLoop::Task::performTask(WebCore::WorkerOrWorkletGlobalScope*)
40  0x165a3249d WebCore::WorkerRunLoop::runInMode(WebCore::WorkerOrWorkletGlobalScope*, WebCore::ModePredicate const&, WebCore::WorkerRunLoop::WaitMode)
41  0x165a3291f WebCore::WorkerRunLoop::runInMode(WebCore::WorkerOrWorkletGlobalScope*, WTF::String const&, WebCore::WorkerRunLoop::WaitMode)
42  0x1659fa04a WebCore::WorkerGlobalScope::unwrapCryptoKey(WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&)
43  0x1635bfd3a WebCore::unwrapCryptoKey(JSC::JSGlobalObject*, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&)
44  0x16359677d WebCore::CloneDeserializer::readTerminal()
45  0x163594179 WebCore::CloneDeserializer::deserialize()

The test sends 2 messages to the worker, the first one with the public key and the second one with the private key. When processing the first message on the worker thread and doing the deserialization of the public key, we call WorkerRunLoop::runInMode() which processes the pending message for the private key.

As a result, the MessageEvents are dispatched to the JS in the wrong order (private key first, then then public key) sometimes.

[Chris Dumez]

The implementation here is wrong:
bool WorkerGlobalScope::unwrapCryptoKey(const Vector<uint8_t>& wrappedKey, Vector<uint8_t>& key)
{
    Ref<WorkerGlobalScope> protectedThis(*this);
    auto resultContainer = CryptoBooleanContainer::create();
    auto doneContainer = CryptoBooleanContainer::create();
    auto keyContainer = CryptoBufferContainer::create();
    thread().workerLoaderProxy().postTaskToLoader([resultContainer, wrappedKey, keyContainer, doneContainer, workerMessagingProxy = makeRef(downcast<WorkerMessagingProxy>(thread().workerLoaderProxy()))](ScriptExecutionContext& context) {
        resultContainer->setBoolean(context.unwrapCryptoKey(wrappedKey, keyContainer->buffer()));
        doneContainer->setBoolean(true);
        workerMessagingProxy->postTaskForModeToWorkerOrWorkletGlobalScope([](ScriptExecutionContext& context) {
            ASSERT_UNUSED(context, context.isWorkerGlobalScope());
        }, WorkerRunLoop::defaultMode());
    });

    auto waitResult = MessageQueueMessageReceived;
    while (!doneContainer->boolean() && waitResult != MessageQueueTerminated)
        waitResult = thread().runLoop().runInMode(this, WorkerRunLoop::defaultMode());

    if (doneContainer->boolean())
        key.swap(keyContainer->buffer());
    return resultContainer->boolean();
}

The runInMode() call means we'll process incoming events while we're in the middle of deserializing a crypto key and this is what can cause messages to get received out of order.

[Chris Dumez]

Created attachment 432815 [details]
WIP Patch

[Chris Dumez]

Created attachment 432816 [details]
Patch

[Chris Dumez]

Created attachment 432820 [details]
Patch

[Chris Dumez]

Created attachment 432821 [details]
Patch

[Geoffrey Garen]

Comment on attachment 432821 [details]
Patch

r=me

[EWS]

Committed r279518 (239369@main): <https://commits.webkit.org/239369@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432821 [details].

[youenn fablet]

*** Bug 229178 has been marked as a duplicate of this bug. ***