Make WebPageProxy::wrapCryptoKey take CryptoKey instead of serialized key data by szewai · Pull Request #37761 · WebKit/WebKit

szewai · 2024-12-11T07:01:17Z

`22614e8`

Make WebPageProxy::wrapCryptoKey take CryptoKey instead of serialized key data
https://bugs.webkit.org/show_bug.cgi?id=284444
rdar://141265745

Reviewed by Pascoe and Matthew Finkel.

In current implementation of wrapping crypto key, web process serializes key into bytes and sends the bytes to UI
process for encryption. On receiving the bytes, UI process is not able to validate that the bytes actually represent
crypto key, as it does not know the serialization format. To ensure UI process can do validation, now we make web
process send structured crypto key data to UI process, by introducing WebCore::CryptoKeyData and adding IPC
serialization for it. If UI process cannot recreate crypto key from the data, it will reject the request; otherwise it
will do both serialization and encryption.

There should be no user-visible behavior change after this patch.

* Source/WebCore/Headers.cmake:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::serializeAndWrapCryptoKey):
(WebCore::CloneSerializer::serializeCryptoKey):
(WebCore::CloneSerializer::dumpIfTerminal):
(WebCore::SerializedScriptValue::serializeCryptoKey):
(WebCore::wrapCryptoKey): Deleted.
* Source/WebCore/bindings/js/SerializedScriptValue.h:
* Source/WebCore/crypto/CryptoKey.cpp:
(WebCore::CryptoKey::create):
* Source/WebCore/crypto/CryptoKey.h:
(WebCore::CryptoKey::isValid const):
* Source/WebCore/crypto/CryptoKeyData.h: Added.
(WebCore::CryptoKeyData::CryptoKeyData):
(WebCore::CryptoKeyData::isolatedCopy):
* Source/WebCore/crypto/JsonWebKey.h:
(WebCore::JsonWebKey::isolatedCopy):
* Source/WebCore/crypto/RsaOtherPrimesInfo.h:
(WebCore::RsaOtherPrimesInfo::isolatedCopy):
* Source/WebCore/crypto/keys/CryptoKeyAES.cpp:
(WebCore::CryptoKeyAES::exportJwk const):
(WebCore::CryptoKeyAES::data const):
* Source/WebCore/crypto/keys/CryptoKeyAES.h:
* Source/WebCore/crypto/keys/CryptoKeyEC.cpp:
(WebCore::CryptoKeyEC::exportJwk const):
(WebCore::CryptoKeyEC::data const):
* Source/WebCore/crypto/keys/CryptoKeyEC.h:
* Source/WebCore/crypto/keys/CryptoKeyHMAC.cpp:
(WebCore::CryptoKeyHMAC::exportJwk const):
(WebCore::CryptoKeyHMAC::data const):
* Source/WebCore/crypto/keys/CryptoKeyHMAC.h:
* Source/WebCore/crypto/keys/CryptoKeyOKP.cpp:
(WebCore::CryptoKeyOKP::exportJwk const):
(WebCore::CryptoKeyOKP::namedCurveFromString):
(WebCore::CryptoKeyOKP::data const):
* Source/WebCore/crypto/keys/CryptoKeyOKP.h:
* Source/WebCore/crypto/keys/CryptoKeyRSA.cpp:
(WebCore::CryptoKeyRSA::exportJwk const):
(WebCore::CryptoKeyRSA::data const):
* Source/WebCore/crypto/keys/CryptoKeyRSA.h:
* Source/WebCore/crypto/keys/CryptoKeyRaw.cpp:
(WebCore::CryptoKeyRaw::data const):
* Source/WebCore/crypto/keys/CryptoKeyRaw.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::serializeAndWrapCryptoKey):
* Source/WebCore/dom/Document.h:
* Source/WebCore/dom/EmptyScriptExecutionContext.h:
* Source/WebCore/dom/ScriptExecutionContext.h:
* Source/WebCore/page/CryptoClient.h:
(WebCore::CryptoClient::serializeAndWrapCryptoKey const):
* Source/WebCore/workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::serializeAndWrapCryptoKey):
* Source/WebCore/workers/WorkerGlobalScope.h:
* Source/WebCore/worklets/WorkletGlobalScope.h:
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/API/APISerializedScriptValue.cpp:
(API::SerializedScriptValue::serializeCryptoKey):
* Source/WebKit/Shared/API/APISerializedScriptValue.h:
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::serializeAndWrapCryptoKey):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::serializeAndWrapCryptoKey):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.messages.in:
* Source/WebKit/WebProcess/WebCoreSupport/WebCryptoClient.cpp:
(WebKit::WebCryptoClient::serializeAndWrapCryptoKey const):
* Source/WebKit/WebProcess/WebCoreSupport/WebCryptoClient.h:
* Source/WebKitLegacy/WebCoreSupport/WebCryptoClient.h:
* Source/WebKitLegacy/WebCoreSupport/WebCryptoClient.mm:
(WebCryptoClient::serializeAndWrapCryptoKey const):

Canonical link: https://commits.webkit.org/287927@main

52ae166

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	~~🧪 win-tests~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	~~🛠 mac-safer-cpp~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-12-11T07:01:23Z

EWS run on previous version of this PR (hash 3529f7a)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	❌ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	✅ 🛠 tv	❌ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-12-11T23:49:13Z

EWS run on previous version of this PR (hash 1e4a1f7)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	❌ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	❌ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	✅ 🛠 tv	~~🛠 mac-safer-cpp~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-12-12T02:40:21Z

EWS run on previous version of this PR (hash 677309d)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	❌ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	✅ 🛠 tv	~~🛠 mac-safer-cpp~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-12-12T06:22:36Z

EWS run on previous version of this PR (hash fc71371)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-12-13T04:08:43Z

EWS run on previous version of this PR (hash 7f92321)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	⏳ 🧪 ios-wk2-wpt	~~🧪 mac-wk1~~	❌ 🛠 wpe-cairo
	⏳ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	~~🧪 mac-intel-wk2~~
	✅ 🛠 tv	~~🛠 mac-safer-cpp~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-12-13T05:24:37Z

EWS run on previous version of this PR (hash aa7c37d)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
❌ 🧪 bindings	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
❌ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
❌ 🧪 webkitpy	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
	❌ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	❌ 🛠 vision-sim	❌ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	⏳ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	❌ 🛠 tv	❌ 🛠 mac-safer-cpp
	❌ 🛠 tv-sim
	❌ 🛠 watch
	❌ 🛠 watch-sim

webkit-early-warning-system · 2024-12-13T06:11:40Z

EWS run on previous version of this PR (hash d61c322)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
	❌ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	❌ 🛠 vision-sim	❌ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	⏳ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	❌ 🛠 tv	❌ 🛠 mac-safer-cpp
	❌ 🛠 tv-sim
	❌ 🛠 watch
	❌ 🛠 watch-sim

webkit-early-warning-system · 2024-12-13T07:39:49Z

EWS run on previous version of this PR (hash 08c79f8)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	❌ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
✅ 🧪 webkitpy	⏳ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

nmahendru · 2024-12-13T22:28:03Z

Source/WebCore/crypto/CryptoKeyData.h

Why do you need both jwk and Vector together.. wouldn't jwk work everywhere ?
and if it's either then std::variant<jwk,Vector> seems like a natural choice.

The algorithm Identifier should probably also not really need optional ? it should always be there.

CryptoKeyType also.

Or is it something to do with IPC serialization that all items are optional ?

Why do you need both jwk and Vector together.. wouldn't jwk work everywhere ?

CryptoKeyRaw cannot export jwk

The algorithm Identifier should probably also not really need optional ? it should always be there.

Algorithm identifier is not optional, hash algorithm identifier is; see members of CryptoKeyData starting at line 73 of this file.

CryptoKeyType also.

CryptoKeyType is optional because some importJwk functions do not require that.
When a parameter is not required for all functions, I make it optional.

Or is it something to do with IPC serialization that all items are optional ?

The logic is we extract CryptoKeyData from CryptoKey (with exportJwk and other property accessor function) on the web process side, send CryptoKeyData to UI process, and recreate CryptoKey from CryptoKeyData (with CryptoKey::create(CryptoKeyData&&))on the UI process side.

nmahendru · 2024-12-13T22:44:02Z

Source/WebCore/dom/Document.cpp

This looks like a new API. Who is calling it ?

auto wrappedKey = serializeAndWrapCryptoKey(m_lexicalGlobalObject, key->data()); in Source/WebCore/bindings/js/SerializedScriptValue.cpp

nmahendru · 2024-12-13T22:46:30Z

This seems like an additive change and the title of the PR says wrapCryptoKey is changing but the commit diff does not show that it's changing. Am I missing something ?

nmahendru · 2024-12-13T22:47:32Z

Source/WebCore/crypto/CryptoKeyData.h

Today I learnt that this is valid. thanks!

szewai · 2024-12-13T22:50:00Z

This seems like an additive change and the title of the PR says wrapCryptoKey is changing but the commit diff does not show that it's changing. Am I missing something ?

Because I directly replace the usage of wrapCryptoKey with serializeAndWrapCryptoKey, since we are doing both serialization and wrapping within the same message now

nmahendru · 2024-12-13T23:14:16Z

This seems like an additive change and the title of the PR says wrapCryptoKey is changing but the commit diff does not show that it's changing. Am I missing something ?

Because I directly replace the usage of wrapCryptoKey with serializeAndWrapCryptoKey, since we are doing both serialization and wrapping within the same message now

Looks good to me.
We could use a std::variant for jwk/vs Vector in CryptoData but that's stylistic and not causing breakage as it's all internal to WebKit.

nmahendru · 2024-12-13T23:16:22Z

This seems like an additive change and the title of the PR says wrapCryptoKey is changing but the commit diff does not show that it's changing. Am I missing something ?

Because I directly replace the usage of wrapCryptoKey with serializeAndWrapCryptoKey, since we are doing both serialization and wrapping within the same message now

Looks good to me. We could use a std::variant for jwk/vs Vector in CryptoData but that's stylistic and not causing breakage as it's all internal to WebKit.

and yea the build needs to be green!

webkit-early-warning-system · 2024-12-14T05:22:05Z

EWS run on previous version of this PR (hash 7d9bbe5)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

sysrqb

Mostly rubberstamped based on Nitin's review

sysrqb · 2024-12-16T21:58:31Z

Source/WebCore/crypto/keys/CryptoKeyHMAC.cpp

I assume this is because the key size is in bytes, and we need bits? Please make that more explicit.

Yes; will update

achristensen07 · 2024-12-16T21:55:55Z

Source/WebKitLegacy/WebCoreSupport/WebCryptoClient.h

This looks like it can be a forward declaration instead of including a header from another header.

achristensen07 · 2024-12-16T21:57:15Z

Source/WebCore/crypto/keys/CryptoKeyRaw.h

This looks unused.

Will remove.

achristensen07 · 2024-12-16T21:59:20Z

Source/WebCore/crypto/keys/CryptoKeyOKP.cpp

I'm not sure if it would be better to have this return std::optional and have CryptoKey::create return nullptr if it's not one of the two supported string, or if this actually is unreachable.

Yes, might be better to return std::optional here.

achristensen07 · 2024-12-16T21:59:47Z

Source/WebCore/crypto/keys/CryptoKeyHMAC.cpp

Will update with CHAR_BIT.

achristensen07 · 2024-12-16T22:00:56Z

Source/WebCore/crypto/keys/CryptoKeyRSA.cpp

Can this call a JsonWebKey constructor with members rather than calling the empty constructor then populating?

I think it can, but JsonWebKey currently doesn't have constructors and I tried to use what's available to keep change size small.
May it do as followup.

webkit-early-warning-system · 2024-12-17T04:08:39Z

EWS run on current version of this PR (hash 52ae166)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	~~🧪 win-tests~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
✅ 🧪 webkitpy	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	~~🛠 mac-safer-cpp~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

… key data https://bugs.webkit.org/show_bug.cgi?id=284444 rdar://141265745 Reviewed by Pascoe and Matthew Finkel. In current implementation of wrapping crypto key, web process serializes key into bytes and sends the bytes to UI process for encryption. On receiving the bytes, UI process is not able to validate that the bytes actually represent crypto key, as it does not know the serialization format. To ensure UI process can do validation, now we make web process send structured crypto key data to UI process, by introducing WebCore::CryptoKeyData and adding IPC serialization for it. If UI process cannot recreate crypto key from the data, it will reject the request; otherwise it will do both serialization and encryption. There should be no user-visible behavior change after this patch. * Source/WebCore/Headers.cmake: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/bindings/js/SerializedScriptValue.cpp: (WebCore::serializeAndWrapCryptoKey): (WebCore::CloneSerializer::serializeCryptoKey): (WebCore::CloneSerializer::dumpIfTerminal): (WebCore::SerializedScriptValue::serializeCryptoKey): (WebCore::wrapCryptoKey): Deleted. * Source/WebCore/bindings/js/SerializedScriptValue.h: * Source/WebCore/crypto/CryptoKey.cpp: (WebCore::CryptoKey::create): * Source/WebCore/crypto/CryptoKey.h: (WebCore::CryptoKey::isValid const): * Source/WebCore/crypto/CryptoKeyData.h: Added. (WebCore::CryptoKeyData::CryptoKeyData): (WebCore::CryptoKeyData::isolatedCopy): * Source/WebCore/crypto/JsonWebKey.h: (WebCore::JsonWebKey::isolatedCopy): * Source/WebCore/crypto/RsaOtherPrimesInfo.h: (WebCore::RsaOtherPrimesInfo::isolatedCopy): * Source/WebCore/crypto/keys/CryptoKeyAES.cpp: (WebCore::CryptoKeyAES::exportJwk const): (WebCore::CryptoKeyAES::data const): * Source/WebCore/crypto/keys/CryptoKeyAES.h: * Source/WebCore/crypto/keys/CryptoKeyEC.cpp: (WebCore::CryptoKeyEC::exportJwk const): (WebCore::CryptoKeyEC::data const): * Source/WebCore/crypto/keys/CryptoKeyEC.h: * Source/WebCore/crypto/keys/CryptoKeyHMAC.cpp: (WebCore::CryptoKeyHMAC::exportJwk const): (WebCore::CryptoKeyHMAC::data const): * Source/WebCore/crypto/keys/CryptoKeyHMAC.h: * Source/WebCore/crypto/keys/CryptoKeyOKP.cpp: (WebCore::CryptoKeyOKP::exportJwk const): (WebCore::CryptoKeyOKP::namedCurveFromString): (WebCore::CryptoKeyOKP::data const): * Source/WebCore/crypto/keys/CryptoKeyOKP.h: * Source/WebCore/crypto/keys/CryptoKeyRSA.cpp: (WebCore::CryptoKeyRSA::exportJwk const): (WebCore::CryptoKeyRSA::data const): * Source/WebCore/crypto/keys/CryptoKeyRSA.h: * Source/WebCore/crypto/keys/CryptoKeyRaw.cpp: (WebCore::CryptoKeyRaw::data const): * Source/WebCore/crypto/keys/CryptoKeyRaw.h: * Source/WebCore/dom/Document.cpp: (WebCore::Document::serializeAndWrapCryptoKey): * Source/WebCore/dom/Document.h: * Source/WebCore/dom/EmptyScriptExecutionContext.h: * Source/WebCore/dom/ScriptExecutionContext.h: * Source/WebCore/page/CryptoClient.h: (WebCore::CryptoClient::serializeAndWrapCryptoKey const): * Source/WebCore/workers/WorkerGlobalScope.cpp: (WebCore::WorkerGlobalScope::serializeAndWrapCryptoKey): * Source/WebCore/workers/WorkerGlobalScope.h: * Source/WebCore/worklets/WorkletGlobalScope.h: * Source/WebKit/Scripts/webkit/messages.py: (headers_for_type): * Source/WebKit/Shared/API/APISerializedScriptValue.cpp: (API::SerializedScriptValue::serializeCryptoKey): * Source/WebKit/Shared/API/APISerializedScriptValue.h: * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in: * Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp: * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::serializeAndWrapCryptoKey): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/UIProcess/WebPageProxy.messages.in: * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::serializeAndWrapCryptoKey): * Source/WebKit/UIProcess/WebProcessProxy.h: * Source/WebKit/UIProcess/WebProcessProxy.messages.in: * Source/WebKit/WebProcess/WebCoreSupport/WebCryptoClient.cpp: (WebKit::WebCryptoClient::serializeAndWrapCryptoKey const): * Source/WebKit/WebProcess/WebCoreSupport/WebCryptoClient.h: * Source/WebKitLegacy/WebCoreSupport/WebCryptoClient.h: * Source/WebKitLegacy/WebCoreSupport/WebCryptoClient.mm: (WebCryptoClient::serializeAndWrapCryptoKey const): Canonical link: https://commits.webkit.org/287927@main

webkit-commit-queue · 2024-12-17T06:09:11Z

Committed 287927@main (22614e8): https://commits.webkit.org/287927@main

Reviewed commits have been landed. Closing PR #37761 and removing active labels.

szewai self-assigned this Dec 11, 2024

szewai added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Dec 11, 2024

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 11, 2024

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 11, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from 3529f7a to 1e4a1f7 Compare December 11, 2024 23:49

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 12, 2024

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 12, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from 1e4a1f7 to 677309d Compare December 12, 2024 02:40

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 12, 2024

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 12, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from 677309d to fc71371 Compare December 12, 2024 06:22

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 12, 2024

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from fc71371 to 7f92321 Compare December 13, 2024 04:08

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from 7f92321 to aa7c37d Compare December 13, 2024 05:24

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from aa7c37d to d61c322 Compare December 13, 2024 06:11

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from d61c322 to 08c79f8 Compare December 13, 2024 07:39

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

szewai requested review from cdumez, rniwa and zdobersek as code owners December 13, 2024 21:02

szewai requested review from nmahendru and pascoej December 13, 2024 21:02

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 13, 2024

nmahendru reviewed Dec 13, 2024

View reviewed changes

Source/WebCore/crypto/CryptoKeyData.h Outdated

Copy link

Contributor

nmahendru Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today I learnt that this is valid. thanks!

szewai removed the merging-blocked Applied to prevent a change from being merged label Dec 14, 2024

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from e57f567 to 7d9bbe5 Compare December 14, 2024 05:21

szewai requested review from achristensen07 and sysrqb December 16, 2024 20:17

pascoej approved these changes Dec 16, 2024

View reviewed changes

sysrqb approved these changes Dec 16, 2024

View reviewed changes

achristensen07 reviewed Dec 16, 2024

View reviewed changes

szewai force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from 7d9bbe5 to 52ae166 Compare December 17, 2024 04:08

szewai added the merge-queue Applied to send a pull request to merge-queue label Dec 17, 2024

webkit-commit-queue force-pushed the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch from 52ae166 to 22614e8 Compare December 17, 2024 06:09

webkit-commit-queue merged commit 22614e8 into WebKit:main Dec 17, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 17, 2024

szewai deleted the eng/Make-WebPageProxy-wrapCryptoKey-take-CryptoKey-instead-of-serialized-key-data branch January 2, 2026 18:11

Conversation

szewai commented Dec 11, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szewai Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nmahendru commented Dec 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szewai commented Dec 13, 2024

Uh oh!

nmahendru commented Dec 13, 2024

Uh oh!

nmahendru commented Dec 13, 2024

Uh oh!

webkit-early-warning-system commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sysrqb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szewai commented Dec 11, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Dec 11, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 11, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 12, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 12, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 13, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 13, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 13, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 13, 2024 •

edited

Loading

szewai Dec 13, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 14, 2024 •

edited

Loading

webkit-early-warning-system commented Dec 17, 2024 •

edited

Loading