Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

[wip] Add libsodium #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[wip] Add libsodium #83

wants to merge 1 commit into from

Conversation

iefserge
Copy link
Member

@iefserge iefserge commented Aug 21, 2015

This PR adds statically linked libsodium. This is not very nice compared to using a js version, but this should load and compute faster. JS version also slows down browserify builds. Eventually this could be replaced by the WASM compiled module and loader that doesn't require using browserify.

  • add function bindings
  • fix randombytes (call to runtime.random.getRandomValues())

@facekapow
Copy link
Contributor

I'm working (based on this PR) on creating JS bindings for libsodium. Should I create methods that provide ease-of-use interfaces to the functions, or just functions that are raw interfaces? Like hash(stringParameter) vs rawHash(stringParameter, stringLength).

@iefserge
Copy link
Member Author

iefserge commented Oct 9, 2015

I think easy-you-use functions, to prevent possible incorrect usage.
Like hash(string | uint8array)

@piranna
Copy link

piranna commented Oct 9, 2015

I would do the easy-to-use functions in Javascript, that internaly use low-level C++ bindings. I think is the most clean and flexible scheme.

@iefserge
Copy link
Member Author

@facekapow @piranna what do you guys think about the idea to implement window.crypto (would be visible as global.crypto) spec that matches browsers? https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto
Spec uses promises in API (should be fine to use with async/await), it'll be possible to do computations async on another CPU core etc.

@piranna
Copy link

piranna commented Jan 16, 2016

Between a w3c spec or the Node.js API I will alwahs advocate for the first
one. Don't knlw how we could develop the Node.js compatibilith layer on top
of it, though...
El 16/1/2016 21:49, "Serge" notifications@github.com escribió:

@facekapow https://github.com/facekapow @piranna
https://github.com/piranna what do you guys think about the idea to
implement window.crypto (would be visible as global.crypto) spec that
matches browsers?
https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto
Spec uses promises in API (should be fine to use with async/await), it'll
be possible to do computations async on another CPU core etc.


Reply to this email directly or view it on GitHub
#83 (comment).

@iefserge
Copy link
Member Author

@piranna yeah, I think generally it would be pretty nice to be compatible with the browser/web platform. Also things like w3c streams spec https://streams.spec.whatwg.org/, web workers spec, fetch api etc.

@facekapow
Copy link
Contributor

I think that maintaining compatability with the Broswer should always be more important. But, to work with both APIs, we could have one internal API that covers both and 2 external APIs, one for Node, and one for the browser, which would use the internal API.

@piranna
Copy link

piranna commented Jan 18, 2016

Not a bad idea... a low level, thin wrapper layer on top of OpenSSL, and
later compatibility libraries for both w3c and Node.js libraries, I like it
:-D The same would be for other APIs like FileSystem vs fs module or
XmlHttpRequest vs http module :-)
El 18/1/2016 3:04, "facekapow" notifications@github.com escribió:

I think that maintaining compatability with the Broswer should always be
more important. But, to work with both APIs, we could have one internal API
that covers both and 2 external APIs, one for Node, and one for the
browser, which would use the internal API.


Reply to this email directly or view it on GitHub
#83 (comment).

@facekapow
Copy link
Contributor

I need help. I can't figure out what I did to screw up crypto_secretbox. When running my tests on this branch, crypto_secretbox_open_easy errored. The only thing I changed is that I made key into a Uint8Array (on both JS and C++). And I've checked and rechecked the code. The key is the same, and I'm passing in the ciphertext and the nonce I get from crypto_secretbox_easy, but still nothing.

@iefserge
Copy link
Member Author

@facekapow ok, I'll look into it

@facekapow
Copy link
Contributor

Same thing with crypto_aead_chacha20poly1305.

@facekapow
Copy link
Contributor

Well, I forgot to squash the commits 😦

@facekapow
Copy link
Contributor

@iefserge Could you enable squash merging on the repo, for when this is merged? (I'm pretty sure having all those commits as part of the git history isn't such a good idea, sorry)

@iefserge
Copy link
Member Author

iefserge commented Jul 2, 2016

@facekapow yeah, let's squash them. Diff is pretty crazy here, merge libsodium files separately first? I guess it should make it easier to work with the PR. Not sure how hard it would be to split it though, I can look into it if you'd like.

@facekapow
Copy link
Contributor

Sure, right now I'm trying to figure out what's the problem with having keys as Uint8Arrays, because none of the decryption functions work. I'm going to revert back to keys as strings locally and try that out.

@iefserge
Copy link
Member Author

iefserge commented Jul 2, 2016

@facekapow ok, I'm going to merge this commit 25d6bcb (I guess I should've done that earlier), it will create a conflict with this branch, after that you should be able to do something like

git checkout add-libsodium
git fetch
git reset --hard origin/master
git cherry-pick COMMIT1    // cherry-pick all your commits
git cherry-pick COMMIT2
...
git push -f

sounds good or maybe there is an easier way?:)

@facekapow
Copy link
Contributor

Sounds great, I certainly don't know a better way.

@facekapow
Copy link
Contributor

facekapow commented Jul 3, 2016

@iefserge There, history has been fixed, and the conflicts are resolved. 😃

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

Successfully merging this pull request may close these issues.

3 participants