sharplispers / ironclad

A cryptographic toolkit written in Common Lisp
BSD 3-Clause "New" or "Revised" License
166 stars 28 forks source link

Shouldn't prng-random-data for os-prng use a shared stream (guarded with a lock)? #30

Closed Yehouda closed 2 years ago

Yehouda commented 4 years ago

Currently, there is a stream open for an os-prng that has been used. That may mean many streams. Maybe it can use a shared stream, and just guard it with a a lock. Something like:

[ setup *os-prng-global-stream* and *os-prng-stream-lock* ]

(let ((seq (make-array num-bytes :element-type '(unsigned-byte 8))))
   (unless  (bt:with-lock-held (*os-prng-stream-lock*)
                      (>= (read-sequence seq *os-prng-global-stream*) num-bytes))
      (error 'ironclad-error :format-control "Failed to get random data."))
    seq)
glv2 commented 4 years ago

I did a few simple tests on the impact of a lock on the run time of the prng-random-data function (with SBCL 2.0.4):

I'll have to check if there are functions in Ironclad that use a lot of short reads of random data to see if the slow-down because of the lock would be acceptable.

Yehouda commented 4 years ago

If short reads are common, you can optimize them by buffering longer reads. If the buffering is done per process, the buffer can be accessed withot a lock and without kernel calls.

Yehouda commented 4 years ago

There is also getrandom now, which is more civilized interface. Maybe you should check if it is available and use iit when it is. http://man7.org/linux/man-pages/man2/getrandom.2.html

glv2 commented 4 years ago

I pushed a commit (91d124d592809d3ba2aaf545deceec605decb426) implementing the stream-and-lock version. Tell me if you get some issues with.

I also made a few tests with an alien function for getrandom, but I'm not very satisfied by it, as it's almost twice slower than the version with the lock. I'll see if I can improve it...