RbNaCl 2.0

[Tony Arcieri]

Hey there RbNaClers,

I think it's time to start discussing RbNaCl 2.0. There were obviously a few mistakes made in the original version of the library which require some backwards compatible fixes to correct. In my mind they are as follows: (I have explicitly labeled breaking changes)

• Everything is namespaced as "Crypto" in RbNaCl 1.0 to match NaCl itself. I think the "Crypto" constant is fine and should be preserved, but the canonical name for the module should be "RbNaCl". The "Crypto" alias can be implemented as "Crypto = RbNaCl"
• [BREAKING] The encoding system should be removed in favor of handling this in higher level libraries implemented on top of RbNaCl (e.g. Krypt) or a separate gem entirely that deals specifically with encodings
• [BREAKING] VerifyKey#verify should take the signature as the first argument, instead of the message. This matches the other APIs, which generally take a key or a nonce as the first argument
• [BREAKING(?)] Separate classes should be provided for each primitive. The "default" primitives for e.g. Crypto::Box or Crypto::SecretBox should be implemented as aliases (e.g. Crypto::Box = Crypto::Boxes::Curve25519XSalsa20Poly1305) or via a wrapper class for Crypto::*Box (as namelessjon proposed)

Thoughts?

--
Tony Arcieri

[Stephen Touset]

On Jul 22, 2013, at 1:05 PM, Tony Arcieri <tony.a...@gmail.com> wrote:

> • Everything is namespaced as "Crypto" in RbNaCl 1.0 to match NaCl itself. I think the "Crypto" constant is fine and should be preserved, but the canonical name for the module should be "RbNaCl". The "Crypto" alias can be implemented as "Crypto = RbNaCl"

Since you brought it up, I just wanted to toss my perspective out there about this again. It's a small thing, but I don' think it's a good idea to be hijacking constants outside of your own gem's "namespace". While the `crypto` gem has been dead for awhile (I actually emailed the author awhile ago about him "donating" it to the cause, but he declined), that doesn't prevent some *other* gem from coming along and doing the same.

Having been a repeat victim of drive-by namespacing conflicts before (I authored the `version` gem, and have had several other popular gems implement a class named `::Version` or have a file named `lib/version.rb`), it's a massive pain in the ass to figure out what gem is actually causing the conflict. And with a common word like `Crypto`, I expect you guys will run into similar situations.

If you do want to continue using the `Crypto` namespace (which I honestly can't really fault you for) at least have the library structured like:

rb_na_cl/
lib/
rb_na_cl.rb
crypto.rb

And have crypto.rb pretty much contain nothing but

require 'rb_na_cl'

Crypto = RbNaCl

This way if there ever *is* a conflict, it's relatively easy to fix.

> • [BREAKING] The encoding system should be removed in favor of handling this in higher level libraries implemented on top of RbNaCl (e.g. Krypt) or a separate gem entirely that deals specifically with encodings
> • [BREAKING(?)] Separate classes should be provided for each primitive. The "default" primitives for e.g. Crypto::Box or Crypto::SecretBox should be implemented as aliases (e.g. Crypto::Box = Crypto::Boxes::Curve25519XSalsa20Poly1305) or via a wrapper class for Crypto::*Box (as namelessjon proposed)

These sounds oddly familiar. :) Glad to hear you guys are considering this approach.

--
Stephen Touset
ste...@touset.org

[Jonathan Stott]

On 22 July 2013 23:51, Stephen Touset <ste...@touset.org> wrote:
>
> On Jul 22, 2013, at 1:05 PM, Tony Arcieri <tony.a...@gmail.com> wrote:
>
>> • Everything is namespaced as "Crypto" in RbNaCl 1.0 to match NaCl itself. I think the "Crypto" constant is fine and should be preserved, but the canonical name for the module should be "RbNaCl". The "Crypto" alias can be implemented as "Crypto = RbNaCl"
>

> And have crypto.rb pretty much contain nothing but
>
> require 'rb_na_cl'
>
> Crypto = RbNaCl
>
> This way if there ever *is* a conflict, it's relatively easy to fix.

I like the idea of doing it this way. Though I don't think we need to
go for the fully technically correct 'rb_na_cl'. Is someone really
going to make a gem with a Rbnacl constant that would be conflicted by
lib/rbnacl.

>> • [BREAKING] The encoding system should be removed in favor of handling this in higher level libraries implemented on top of RbNaCl (e.g. Krypt) or a separate gem entirely that deals specifically with encodings

I agree on this one. But then I've said this since you added the
encodings. I assume this would also hold for other higher level
constructs such as an encrypted key file?

> [BREAKING] VerifyKey#verify should take the signature as the first argument, instead of the message. This matches the other APIs, which generally take a key or a nonce as the first argument

I agree with this one too. Trying hard not say "I told you so" ;)

>> • [BREAKING(?)] Separate classes should be provided for each primitive. The "default" primitives for e.g. Crypto::Box or Crypto::SecretBox should be implemented as aliases (e.g. Crypto::Box = Crypto::Boxes::Curve25519XSalsa20Poly1305) or via a wrapper class for Crypto::*Box (as namelessjon proposed)

> These sounds oddly familiar. :) Glad to hear you guys are considering this approach.

They existed in the mainline for a short time. It was a good point you raised.

Yeah, it makes a lot of sense. Less tied to the specific
implementation, as long as they duck type closely. Unless you're
specifically checking for what class you're getting (and we should
encourage the use of the symbol names instead) it shouldn't break
things.

I also think we should return specialized objects which can e.g. tell
you the primitive used, which also respond to #to_str, #to_s etc and
are generally pretty stringlike. This information could then be used
by a higher level construct. Maybe it isn't needed though.

Should we support calling sodium_init()?

Regards
Jonathan

[Tony Arcieri]

On Tue, Jul 23, 2013 at 12:59 PM, Jonathan Stott <jonatha...@gmail.com> wrote:

I like the idea of doing it this way.  Though I don't think we need to

go for the fully technically correct 'rb_na_cl'.  Is someone really
going to make a gem with a Rbnacl constant that would be conflicted by
lib/rbnacl.

I don't think there's really a need to use 'rb_na_cl'. The present require directive is for 'rbnacl' which seems fine to me.

A crypto.rb file containing the Crypto constant also seems fine.

I agree on this one.  But then I've said this since you added the
encodings.  I assume this would also hold for other higher level
constructs such as an encrypted key file?

Yes, although something like Stephen's Sodium::Buffer might make sense.

 

I agree with this one too.  Trying hard not say "I told you so" ;)

You can "told you so" away if you so please! :P

 

Yeah, it makes a lot of sense.  Less tied to the specific

implementation, as long as they duck type closely.  Unless you're
specifically checking for what class you're getting (and we should
encourage the use of the symbol names instead) it shouldn't break
things.

I think we can keep RbNaCl::Box and RbNaCl::SecretBox and make the other implementations in pluralized modules, e.g.: RbNaCl::Boxes::Curve25519XSalsa20Poly1305 and RbNaCl::SecretBoxes::XSalsa20Poly1305

 

I also think we should return specialized objects which can e.g. tell
you the primitive used, which also respond to #to_str, #to_s etc and
are generally pretty stringlike.  This information could then be used
by a higher level construct.  Maybe it isn't needed though.

It seems good to me.

 

Should we support calling sodium_init()?

It would be cool, although I have been tracking what alternative implementations are actually present in Sodium now.

--
Tony Arcieri

[Stephen Touset]

On Jul 23, 2013, at 1:06 PM, Tony Arcieri <tony.a...@gmail.com> wrote:

> On Tue, Jul 23, 2013 at 12:59 PM, Jonathan Stott <jonatha...@gmail.com> wrote:
>> I like the idea of doing it this way. Though I don't think we need to
>> go for the fully technically correct 'rb_na_cl'. Is someone really
>> going to make a gem with a Rbnacl constant that would be conflicted by
>> lib/rbnacl.
>
> I don't think there's really a need to use 'rb_na_cl'. The present require directive is for 'rbnacl' which seems fine to me.
>
> A crypto.rb file containing the Crypto constant also seems fine.

Just trying to be technically correct (the best kind of correct). :)

> Yes, although something like Stephen's Sodium::Buffer might make sense.

Here be dragons.

I've finally gotten Sodium::Buffer to a reasonable state, but it's taken a lot of effort and many tweaks from the original approach.

Sodium::Buffer now internally allocates its own memory for the buffer, and stores its location in an FFI::Pointer (to avoid the garbage collector copying its bytes around, as you warned me about). And Sodium::Buffer#to_s now returns a String copied from that pointer, but wrapped inside of a proxy that handles the memory management. Without doing it this way, I don't believe there's a way to wipe the String's memory on garbage collection without introducing a circular dependency on something in the finalizer itself, preventing the String from ever being collected.

It's not perfect (lots of Ruby functions implemented in C expect honest-to-god Strings, not proxy-object-that-forwards-all-methods-to-a-string Strings), but it's as good as I think is possible in Ruby. You're welcome to the code, or if you have an idea for a better approach, I encourage you to try.

> Yeah, it makes a lot of sense. Less tied to the specific
> implementation, as long as they duck type closely. Unless you're
> specifically checking for what class you're getting (and we should
> encourage the use of the symbol names instead) it shouldn't break
> things.

They can all duck-type identically, from my experience.

The trick, in my experience, was to make RbNaCl::Box (for example) respond exactly as if it is an RbNaCl::Box::WhateverTheDefaultImplementationIs. And you can call RbNaCl::Box.implementation[:curve25519xsalsa20poly1305] to explicitly ask for a particular implementation.

> I think we can keep RbNaCl::Box and RbNaCl::SecretBox and make the other implementations in pluralized modules, e.g.: RbNaCl::Boxes::Curve25519XSalsa20Poly1305 and RbNaCl::SecretBoxes::XSalsa20Poly1305

Not a fan. But that might just be me.

> I also think we should return specialized objects which can e.g. tell
> you the primitive used, which also respond to #to_str, #to_s etc and
> are generally pretty stringlike. This information could then be used
> by a higher level construct. Maybe it isn't needed though.

That might be a bit much. As long as RbNaCl::SecretBox (and friends) respond to #primitive, you should be good. And that way is a lot simpler.

> Should we support calling sodium_init()?

In Sodium, I just call sodium_init() when the library is first loaded.

https://github.com/stouset/sodium/blob/master/lib/sodium/ffi/crypto.rb#L99

Technically, it's not threadsafe, but just document that you shouldn't require the library in independent threads. Since 99% of Ruby code doesn't dynamically load libraries, I consider it not too big a deal.

--
Stephen Touset
ste...@touset.org

[Tony Arcieri]

On Wed, Jul 24, 2013 at 11:05 AM, Stephen Touset <ste...@touset.org> wrote:

Technically, it's not threadsafe, but just document that you shouldn't require the library in independent threads. Since 99% of Ruby code doesn't dynamically load libraries, I consider it not too big a deal.

If you really want to ensure thread safety, wrap it in Thread.exclusive { ... } 

--
Tony Arcieri

[Tony Arcieri]

First up: remove the encoding system!

https://github.com/cryptosphere/rbnacl/pull/65

--
Tony Arcieri