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

Add wrappers for crypto_aead_chacha20poly1305_* #141

Closed
wants to merge 14 commits into from

Conversation

aadavids
Copy link
Contributor

Addresses Issue #107.

Includes both the original implementation and the IETF implementation. I've tried to follow the style of the other wrappers, but if you have any suggestions, let me know.

@aadavids
Copy link
Contributor Author

Looks like the build is failing because my code uses some macros in the IETF version not present until libsodium version 1.0.9.

What do you think would be the best way of handling this? I can remove the IETF wrappers from this pull request for now, and add them in later once you're ready to update the supported libsodium version.

@tarcieri
Copy link
Contributor

@aadavids generally we have gated features on the supported libsodium versions, although I think we've largely eliminated that from the current codebase.

I would suggest checking the libsodium version and for nonsupported versions, stubbing methods with NotImplementedError if the version they're using is too old.

@aadavids
Copy link
Contributor Author

aadavids commented Oct 4, 2016

I looked at the previous versions of the code and added in a version guard similar to what was present before. The build is now failing on a rubocop issue, after adding the AEAD test data, the test vectors file now has too many lines (107 vs 100 haha).
Should we just move test_vectors.rb to the spec directory? Seems to make more sense having it there anyways. Or try to split it up?

@tarcieri
Copy link
Contributor

tarcieri commented Oct 4, 2016

@aadavids the test vectors are used when the library boots, so they should stay where they are.

You can probably #rubocop:disable ClassLength on that file though

@aadavids
Copy link
Contributor Author

aadavids commented Oct 4, 2016

@tarcieri Ah, that makes sense now. Updated with the rubocop change.

@aadavids
Copy link
Contributor Author

aadavids commented Oct 4, 2016

Any idea why the jruby checks are failing?

@tarcieri
Copy link
Contributor

tarcieri commented Oct 4, 2016

It looks like one of the dependencies requires Ruby 2.0+ now. We can probably drop support for Ruby 1.9.

@aadavids
Copy link
Contributor Author

aadavids commented Oct 6, 2016

I removed the jruby (ruby 1.9) job from travis. It also builds against libsodium 1.0.10 now, which is the latest supported by rbnacl-libsodium.

@tarcieri
Copy link
Contributor

@aadavids can you rebase on master and use master's .travis.yml?

@tarcieri
Copy link
Contributor

tarcieri commented Dec 4, 2016

Rebased and merged as 72841d0

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.

None yet

2 participants