LibCrypto/LibWeb: Merge pull requests from Ladybird - #26978
Conversation
Will be used by an upcoming JS prototype Co-authored-by: Jamie Mansfield <jmansfield@cadixdev.org>
(cherry picked from commit 81a0aa5)
(cherry picked from commit 90a2dcf)
(cherry picked from commit 92d4cb7)
This alone lets us pass around 40 WPT tests: WebCryptoAPI/import_export/symmetric_importKey.https.any (cherry picked from commit 6f88376)
This lets us pass an additional (roughly) 40 WPT tests: WebCryptoAPI/import_export/symmetric_importKey.https.any (cherry picked from commit 9255a1a)
This is progress towards passing more WPT tests, although none of them gets green due to this commit. (cherry picked from commit d86dcac)
This lets us pass an additional (roughly) 20 WPT tests: WebCryptoAPI/encrypt_decrypt/aes_cbc.https.any (cherry picked from commit eb19325)
This lets us pass an additional (roughly) 15 WPT tests: WebCryptoAPI/encrypt_decrypt/aes_cbc.https.any (cherry picked from commit b105612)
(cherry picked from commit ff3d78f)
(cherry picked from commit 3553861)
(cherry picked from commit 0864436)
(cherry picked from commit 030cbef)
(cherry picked from commit 4b2120d)
(cherry picked from commit 55ef1c7)
(cherry picked from commit 74403d7)
(cherry picked from commit af3383d)
(cherry picked from commit 8c5e5ad)
(cherry picked from commit 944f00c)
(cherry picked from commit 5e98c3f)
(cherry picked from commit b281fa2)
(cherry picked from commit f9b511a)
(cherry picked from commit 80d37a6)
(cherry picked from commit e53b3f5)
(cherry picked from commit 3dd80d2)
(cherry picked from commit 2672acf)
(cherry picked from commit 196d993)
(cherry picked from commit 171af8d)
Previously, callers were passing the size in bytes, but the method expected bits. This caused a crash in LibCrypto when verifying the key size later on. Also make the naming of local variables and parameters a little more clear between the different AES algorithms :^) (cherry picked from commit 0db171c)
(cherry picked from commit f73a434)
|
As far as I know, we don't have any wpt-import tests in tree at the moment. Do you think that's something we want to do? Always seemed a bit strange to me, given that you want to run actual WPT eventually anyways. Two ways forward:
|
|
I can strip them if they're not wanted here. The groundwork for importing wpt tests was done prior to the Ladybird fork, so there is some stuff (https://github.com/SerenityOS/serenity/tree/master/Tests/LibWeb/Text/input/wpt-import). I certainly think it's worthwhile for Ladybird to have them in-tree - maybe less so for us? |
|
I don't have a strong opinion on this. I'm happy to go with what you think makes sense, or you could ask on Discord for more input. |
(These files were just added in the cherry-picks from nico (see the commit messages), after the Ladybird fork.) |
Ah, I assumed given the lack of actual wpt tests - it was done prior to the fork, I didn't really look beyond that. |
And here's the wild part: instead of cloning WPT tests, import the relevant WPT tests that this fixes into our own test suite. This works by adding a small Ladybird-specific callback in resources/testharnessreport.js (which is what that file is meant for!) Note that these run as text tests, and so they must signal the runner when they are done. Tests using the "usual" WPT harness should just work, but tests that do something more freestyle will need manual signaling if they are to be imported. I've also increased the test timeout here from 30 to 60 seconds, to accommodate the larger WPT-style tests. (cherry picked from commit ec0838b)
(cherry picked from commit 329cd94)
(cherry picked from commit 19ee8dd)
(cherry picked from commit 5b67f17)
Let's try to keep algorithm implementations together. No functional changes. (cherry picked from commit b290c18)
(cherry picked from commit f8c8537)
None of the algorithms actually set the `extractable` internal slot in their implementations, and looking at `SubtleCrypto::import_key()` it seems likely that a step is missing here. (cherry picked from commit f799349)
(cherry picked from commit 009f328)
(cherry picked from commit e0def9d)
This makes the X25519 importKey tests from WPT actually run. (cherry picked from commit 8cb371b)
(cherry picked from commit 4d25369)
(cherry picked from commit ed7b1ca)
(cherry picked from commit fe98cbc)
(cherry picked from commit 7b678d3)
This is required for the following commits to avoid a conflict with the `Certificate` namespace. (cherry picked from commit 07ced0c)
The declaration of `DefaultRootCACertificates` was in `Certificate.h` and its implementation in `TLSv12.cpp`. It has been moved over to `TLSv12.h` for consistency. This is in preparation of the next commits to split the changes. (cherry picked from commit e42410a)
The implementation of `Certificate::is_valid` and `Certificate::is_self_signed` were in `TLSv12.cpp` and they have been moved to `Certificate.cpp`. This is in preparation of the next commits to split the changes. (cherry picked from commit 32a90a7)
This is in preparation of the next commits to split the changes. (cherry picked from commit fcdcba5)
2eaec0f to
f86be6b
Compare
|
Okay, I've also merged one commit from LadybirdBrowser/ladybird#1944 - the other appeared to be related to a headless test runner I don't think exists here. That commit contains the machinery for imported WPT tests to actually run under our test runner. Running the tests locally, it seems some of the tests actually run better under Serenity than they did under Ladybird when the changes were made Will go though and fix those up too lol. |
By moving `Certificate` to `LibCrypto` it is possible to reuse a bunch of code from in `LibCrypto` itself. It also moves some constants and pieces of code to a more appropriate place than `LibTLS`. This also makes future work on WebCryptoAPI easier. (cherry picked from commit 49c388b)
Now that `Certificate` has been moved, the OID constants are easily reachable in `LibCrypto`. (cherry picked from commit 506e490)
f86be6b to
9f74211
Compare
10 participants
I am working towards being able to support ECDSA-521 in Serenity, so .pages.dev sites work again :^)
This is my initial tranche of pull requests merges:
generate_aes_key()LadybirdBrowser/ladybird#2143extractablekey property inSubtleCrypto.deriveKey()LadybirdBrowser/ladybird#2354Certificateto LibCrypto LadybirdBrowser/ladybird#2563There is also a commit adapted from LadybirdBrowser/ladybird#1258 (to omit padding for base64). Possibly more of that PR could be merged, and adapted - but I considered it out of scope for this round.