Update curve25519.rb #153
Update curve25519.rb #153
Conversation
|
That's not quite valid Ruby syntax, although the build appears broken on 2.3+ for other reasons |
|
Hah, my Ruby kung-fu is weak. I think that'll do the trick? |
| self.class.scalarmult_curve25519(result, integer, @point) | ||
| ret = self.class.scalarmult_curve25519(result, integer, @point) | ||
| if ret != 0 | ||
| raise "Invalid curve25519 result" |
There was a problem hiding this comment.
This should probably raise CryptoError
|
I think the logic is inverted. This is causing several tests to fail? |
|
I thought it returned |
| @@ -61,7 +61,9 @@ def mult(integer) | |||
| Util.check_length(integer, SCALARBYTES, "integer") | |||
|
|
|||
| result = Util.zeros(SCALARBYTES) | |||
| self.class.scalarmult_curve25519(result, integer, @point) | |||
| if self.class.scalarmult_curve25519(result, integer, @point) != 0 | |||
| raise CryptoError "Invalid curve25519 result" | |||
There was a problem hiding this comment.
Missing a comma here (should be raise CryptoError, "Invalid... go go wacky Ruby syntax), but the test failures seem to indicate this path is being taken in tests that are expected to pass.
This could probably use a test that triggers the offending behavior to ensure the error is raised as expected.
There was a problem hiding this comment.
This wants to be written as unless self.class.scalarmult_curve25519(result, integer, @point)
The sodium_function wrapper that exposes the C API to ruby converts 0 returns from libsodium into true and all others into false.
Agreed on the need for a test to test this condition fails as expected though.
There was a problem hiding this comment.
Interesting. Thanks for the feedback! I'll get another commit pushed. :)
|
Well, the tests are passing again but it's failing on a style check (RuboCop, which you can run locally with Other than that, I think this just needs a test for the degenerate key case and it should be good to go. |
This fixes the RuboCop errors from @paragonie-scott's #153 PR, adds a test, and adopts a belt-and-suspenders approach by trying to detect degenerate keys eagerly (still also handling errors surfaced from libsodium itself)
This fixes the RuboCop errors from @paragonie-scott's #153 PR, adds a test, and adopts a belt-and-suspenders approach by trying to detect degenerate keys eagerly (still also handling errors surfaced from libsodium itself)
|
Merged as 8b3acc1 |
See #152