Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OpenSSL] Treat types as opaque #1023

Conversation

donny-dont
Copy link
Contributor

@donny-dont donny-dont commented May 25, 2022

309956d

[OpenSSL] Treat types as opaque
https://bugs.webkit.org/show_bug.cgi?id=239858

Reviewed by Basuke Suzuki.

LibreSSL 3.5.x made a number of types opaque for compatibility with OpenSSL. Unfortunately
a number of places in the WebCrypto implementation were accessing these structs directly.
Modify the code to use the provided functions to access and modify the types in the same
manner they were being used.

Modified WebCore::convertToBigNumber to return a BIGNUMPtr with a newly allocated BIGNUM.
This change was done because all calls to the function were returning a new BIGNUM rather
than reusing one after transitioning to opaque types. By returning a BIGNUMPtr leaks are
prevented. In the case where OpenSSL expects ownership to be transferred release() is
used.

* Source/WebCore/crypto/openssl/CryptoAlgorithmECDSAOpenSSL.cpp:
* Source/WebCore/crypto/openssl/CryptoKeyECOpenSSL.cpp:
* Source/WebCore/crypto/openssl/CryptoKeyRSAOpenSSL.cpp:
* Source/WebCore/crypto/openssl/OpenSSLUtilities.cpp:
* Source/WebCore/crypto/openssl/OpenSSLUtilities.h:

Canonical link: https://commits.webkit.org/251252@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295168 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@donny-dont donny-dont requested a review from basuke May 25, 2022 20:33
@basuke
Copy link
Contributor

basuke commented May 26, 2022

Looks good to me, but how are the tests covered? Jitsu mentioned about the regression on bugzilla.

@donny-dont
Copy link
Contributor Author

@basuke yea we fixed that. There was a wrong call to one of the getters.

Copy link
Contributor

@basuke basuke left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@donny-dont donny-dont added the merge-queue Applied to send a pull request to merge-queue label Jun 3, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit 309956d into WebKit:main Jun 3, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r295168 (251252@main): https://commits.webkit.org/251252@main

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

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label Jun 3, 2022
eocanha pushed a commit to eocanha/WebKit that referenced this pull request Mar 16, 2023
…ki/wpe-2.38/restore_corrupted_sqlite_file_of_localstorage

Restore/recreate corrupted SQLite file of local storage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants