Support of RSA #85
Merged
Support of RSA #85
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
olegbespalov
force-pushed
the
feat/rsa
branch
4 times, most recently
from
2e90a4a to
af8f773
Compare
olegbespalov
requested review from
mstoykov and
oleiade
and removed request for
a team
Draft
|
I think this closes #84 too. Thanks for the quick implementation |
oleiade
reviewed
Oct 25, 2024
oleiade
approved these changes
Oct 25, 2024
olegbespalov
force-pushed
the
feat/rsa
branch
from
af8f773 to
76cdae6
Compare
mstoykov
reviewed
Oct 25, 2024
| - setup({explicit_done: true}); | ||
| + // setup({explicit_done: true}); | ||
|
|
||
| - var subtle = self.crypto.subtle; // Change to test prefixed implementations |
There was a problem hiding this comment.
I wonder if it won't be easier to actually set self to globalThis which seems to be what it meant to do https://developer.mozilla.org/en-US/docs/Web/API/Window/self
There was a problem hiding this comment.
Sure, let me address this after merging this and #87 as part as clean up
mstoykov
reviewed
Oct 25, 2024
Comment on lines
+192
to
+196
| gotErr := ts.EventLoop.Start(func() error { | ||
| err := executeTestScripts(ts.VU.Runtime(), webPlatformTestSuite+"encrypt_decrypt", "rsa_vectors.js", "rsa.js") | ||
| require.NoError(t, err) | ||
|
|
||
| _, err = ts.VU.Runtime().RunString(`run_test()`) |
There was a problem hiding this comment.
nit: a lot of this test semm ... the same apart from the name and the some strings here.
It might be .. .better to make them into a table test?
There was a problem hiding this comment.
Sure, let me address this after merging this and #87 as part as clean up
olegbespalov
force-pushed
the
feat/rsa
branch
from
07e911a to
3732583
Compare
mstoykov
approved these changes
Oct 25, 2024
olegbespalov
force-pushed
the
feat/rsa
branch
from
3732583 to
20c8ea5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR brings support for the RSA algorithms across available APIs.
Sorry for the huge PR, but the predominant issue is that to make RSA useful (sign, verify, encrypt, decrypt), we need to have a lot of functionality already (generation, export, import), so it's kind of all related. However, I did break pieces in commits, so you could try checking them.
The vital highlight could be that for the RSA-PSS algorithm, we don't support saltLength 0 (since golang SDK doesn't support it). So, I'm thinking of also writing a log message whenever a user uses this to bring attention. But this will be in a separate PR.
Why?
Closes: #47
Closes: #84