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

More subtle implementations #211

Merged
merged 1 commit into from
Jan 12, 2024
Merged

Conversation

boorad
Copy link
Collaborator

@boorad boorad commented Dec 12, 2023

Adds more crypto.subtle method implementations by activating more of Node's C++ code and converting it to work with JSI.

Plans:

This builds on #199 and #200 branches/commits.

@boorad
Copy link
Collaborator Author

boorad commented Dec 12, 2023

@ospfranco halp! Can you help me understand the two return values of KeyObjectHandle in prepareSecretKey? Am I too strict on types, and should mirror your use of any in the functions above?

@ospfranco
Copy link
Collaborator

I don't have that much time to look into the entire thing, but KeyObjectHandle will be a host object, at least for the import/export I didn't need to type it, all the operations that run on it are from the native side, so I just left it as any, if you need any method to be called on it from JS you should type it.

@ali-fs
Copy link

ali-fs commented Dec 15, 2023

hey
thank you for the implementation
any plan to implement deriveKey as well?

@boorad
Copy link
Collaborator Author

boorad commented Dec 15, 2023

any plan to implement deriveKey as well?

It's possible. I need to get one working and fully-tested first. A lot of copied/commented code gets activated for each exported function, so I'll take an inventory when done and see how hard it would be. 🤞

cpp/Cipher/MGLRsa.cpp Outdated Show resolved Hide resolved
@boorad boorad marked this pull request as draft December 28, 2023 03:58
@boorad
Copy link
Collaborator Author

boorad commented Dec 28, 2023

@Szymon20000 @ospfranco I'm done with two tasks (see description), but this feels like an already-too-big PR. Maybe it will look better after #200 is merged, but I'd like some guidance. I can break up the other tasks into discrete PRs if you would like. I may also overhaul the test rig to be a bit closer to Node's fixture-based one.

🙏 let me know, thx!

@boorad boorad marked this pull request as ready for review January 3, 2024 15:11
@Szymon20000
Copy link
Member

@boorad Looks good to me.
Just one more question before wee merge it.
Did you check if all tests are passing that were passing before this pr?

@boorad
Copy link
Collaborator Author

boorad commented Jan 12, 2024

@Szymon20000 I did check, and kept track in a Discord thread. It's the same before and after, save for my new test on webcrypto.

But it got me thinking, and I have been poking around on the test app to see if I can get some more stats/results to be visible and possibly published somewhere.

@boorad
Copy link
Collaborator Author

boorad commented Jan 12, 2024

Also, are you cool with the aliases used for std::variant and std::option? I'm not sure if that's a good C++ practice or not, but it sure makes it easy to look and see what's going on as we convert Node to JSI.

@Szymon20000 Szymon20000 merged commit 2cd2f96 into margelo:main Jan 12, 2024
boorad added a commit to boorad/react-native-quick-crypto that referenced this pull request Jan 18, 2024
@boorad boorad deleted the more-subtle-impls branch January 25, 2024 19:18
Szymon20000 added a commit that referenced this pull request Feb 7, 2024
* begin reworking test suites

* dedicated hooks dir, eslint gha fix attempt

* gha updates, fix lint issues from #211

* useRunTests hook working, still fighting mocha suites/tests

* oops

* consistent naming

* fix android

* update podfile.lock

* fixed random,hash tests and upgraded chai

* bump cocoapods version

* bump GHA ruby version

* install gems

* more GHA/ios build fun

* fix basic sign/verify test

* comment out failing public cipher & sign/verify, fix generateKeyPair tests

* cleanup

* cpp lint

* more github actions updates

* finally the big re-work of tests

* js lint

* Test Results screen changes

* docs and cleanup

* better emojis for github

* add/use test fixtures from Node.js

* fix pbkdf2 deriveBits tests

* quote github actions output correctly, duh

* always more stragglers during self-review

* ugh, more coffee

---------

Co-authored-by: Szymon Kapała <[email protected]>
@boorad boorad mentioned this pull request Apr 18, 2024
@boorad boorad self-assigned this Apr 27, 2024
@boorad
Copy link
Collaborator Author

boorad commented Apr 27, 2024

hey thank you for the implementation any plan to implement deriveKey as well?

@ali-fs I'm accepting sponsorships through Github now. Not sure if you can sponsor or not, but this deriveKey method impl will be handled in a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants