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

Rewrite the WebCrypto ECDH using wasm-bindgen #980

Merged
merged 3 commits into from
Apr 10, 2019
Merged

Conversation

tomaka
Copy link
Member

@tomaka tomaka commented Feb 27, 2019

It was untested and not working anyway. We don't lose anything by rewriting it.
This PR is tested and works against the Rust secio implementation.

cc #23

@ghost ghost assigned tomaka Feb 27, 2019
@ghost ghost added the in progress label Feb 27, 2019
@tomaka
Copy link
Member Author

tomaka commented Feb 27, 2019

Note that while it is not great to assume that we are in a browser environment when compiling for WASM, it is intended that we eventually merge impl_ring.rs and impl_webcrypto.rs into one code that uses a library that successfully compiles for WASM.
(EDIT: once such a library exists)


/// We use a `SendWrapper` from the `send_wrapper` crate around our JS data type. JavaScript data
/// types are not `Send`/`Sync`, but since WASM is single-threaded we know that we're only ever
/// going to access them from the same thread.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless the Threads feature gets implemented eventually. I think Chrome versions >= 70 already support it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I don't know how to solve that and suggest opening an issue instead of blocking this PR. It is not a safety problem, only a panicking problem.

let _ = tx.take()
.expect("JavaScript promise has been resolved twice") // TODO: prove
.send(out);
// Note: contrary to what one might think, we shouldn't add the "deriveBits" usage.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain that bit?

Copy link
Member Author

Choose a reason for hiding this comment

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

The importKey function takes a list of "usages": https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/importKey
For me it was obvious that one should pass the "deriveBits" usage, but if I remember correctly in practice both Firefox and Chrome produce an error if I do that.

@tomaka tomaka merged commit 6e0a38b into libp2p:master Apr 10, 2019
@tomaka tomaka deleted the webcrypto branch April 10, 2019 21:52
@ghost ghost removed the in progress label Apr 10, 2019
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.

2 participants