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

Move to cryptonite #32

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

Move to cryptonite #32

wants to merge 1 commit into from

Conversation

nwf
Copy link

@nwf nwf commented Jun 19, 2015

This at least compiles and passes the self-tests, but that's about all I'm willing to say about it with any certainty. Review would be much appreciated!

@meteficha
Copy link
Member

Thanks for looking into this, @nwf!

  • @vincenthz, @snoyberg, is there anything I should know about migrating packages to use cryptonite? Are there any drawbacks?
  • @vincenthz, where does cprng-aes fit on the new cryptonite world? Besides an AES-based CPRNG not being available on cryptonite, it seems that the ChaCha DRG doesn't support automatic reseeding.

@vincenthz
Copy link

Thanks @nwf !

There shouldn't be any drawbacks moving to cryptonite, the only thing I can think of, is that I haven't committed to provide a stable API yet. Although I don't expect changes to happens all too often from now.

cprng-aes hasn't been moved into cryptonite yet, although it wouldn't be hard to provide a AES-CTR DRG. For automatic reseeding, I'm not sure what to do right now; The more I think about it, it's easier to just throw away a DRG at random interval and create a brand new one, or just using the system entropy. It might be nice to have a discussion about this.

A bit out of topic, but clientsession could profit to move to cryptonite, to move to modern and fast cipher, like chacha for ciphering and Poly1305 for auth, which should be faster than AES-CTR + Skein without any hw support (and about the same speed with). It's easy to get rid of the DRG completely too there:

encrypt :: Keys -> Input -> IO Output
encrypt (chachaKey, polyKey) plaintext = do
    nonce <- getEntropy 12
    let encryptedData     = fst $ ChaCha.combine (ChaCha.initialize 12 chachaKey nonce) plaintext
        encryptedDataIV   = B.concat [nonce, encryptedData]
        mac               = Poly1305.auth polyKey encryptedDataIV
     in B.concat [encryptedDataIV, mac]

@meteficha
Copy link
Member

clientsession is from a time when there wasn't automatic reseeding, so there is a lot of complicated code dealing with this. However, take a look at the nonce package, made with recent cprng-aes. It is my opinion that auto-reseeding should be implemented in one place once and for all. Perhaps as a different module available on cryptonite instead of being part of the DRG API.

Regarding ChaCha + Poly1305, I'm not opposed to moving to different crypto should it prove to be beneficial. This reminds me of #23. BTW, though, what do you mean by getting rid of the DRG? You still have a getEntropy call. Is getEntropy internally using a DRG on cryptonite?

@vincenthz
Copy link

The DRG represent a pure generator that give you re-playability (useful for testing) and/or filtering the system randomness (for example removing potential weakness). It does this at the expense of having something truly random, and it increases the window of "bad-randomness" if your state is cloned (e.g. VM clone) or guessed.

In many cases where you need something actually random (entropy), it's probably much more efficient to ask the system to gives entropy. In the case of client session, it's very likely that you are in this situation. This does "solve" the reseeding problem, as the system entropy do not have to be reseeded.

For the ChaCha + Poly1305, it's very related to #23 indeed. It's more up-to-date with what modern crypto{graphers/users} like to uses at this specific point of time (ChaCha is almost trivial to implement compared to AES, and got excellent properties, likewise for Poly1305); It's also has the benefit of being faster than AES_CTR+Skein (not verified by benchmarks, but interpolated from multiples sources), specially on hardware that do not support AESNI.

@meteficha
Copy link
Member

I agree it's useful to have something that does not reseed itself. So what I would love is a wrapper around any DRG/CPRNG adding automatic reseeding.

Regarding entropy, using the system entropy is usually orders of magnitude more expensive than using a reseeding CPRNG. We can't afford to read from /dev/urandom on every HTTP request. Unless I'm wrong and reading /dev/urandom isn't so expensive.

@vincenthz
Copy link

I've made a quick benchmarks with ChaChaDRG, SystemDRG (both from cryptonite) and cprng-aes

(7.8.4, osx, rdrand, no aesni), generating 4096 bytes in chunks of 64 bytes:

DRG Time
SystemDRG 9.560 μs (9.320 μs .. 9.832 μs)
ChaChaDRG 55.69 μs (54.26 μs .. 57.53 μs)
AESCTR 150.4 μs (147.1 μs .. 153.9 μs)

(7.10, linux, rdrand + aesni)

DRG Time
SystemDRG 3.597 μs (3.596 μs .. 3.597 μs)
ChaChaDRG 43.00 μs (42.83 μs .. 43.19 μs)
AESCTR 15.67 μs (15.66 μs .. 15.70 μs)

(7.10, windows, no rdrand, no aesni)

DRG Time
SystemDRG 9.56 us
ChaChaDRG 110 us
AESCTR 560 us

@nwf
Copy link
Author

nwf commented Jun 22, 2015

Especially in light of those performance numbers, I'd welcome moving as much as possible (i.e. everything but possibly some parts of test harnesses?) to SystemDRG. Letting the kernel provide entropy and manage reseeds and all that seems much better than re-implementing all of that again; more eyes on that code, more users of that code, more likely to be kept up to date, less effort from the Haskell community, etc. :)

@vincenthz
Copy link

Would be nice to revive this, to move away from old packages. @snoyberg are you happy for me to do a PR to update the crypto parts (when I get to it) ?

@snoyberg
Copy link
Member

snoyberg commented Apr 8, 2017 via email

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

4 participants