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

Tweaks to the std library #699

Merged
merged 3 commits into from
Feb 19, 2023
Merged

Conversation

fare
Copy link
Collaborator

@fare fare commented Feb 16, 2023

  • Use sqlite3_prepare_v3 instead of _v2
  • Use libcrypto instead of urandom for random-bytes, which is cheaper and more portable

fare added 2 commits February 16, 2023 15:38
Don't directly use the more expensive /dev/urandom that openssl already uses
but for initialization only.
Copy link
Collaborator

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, but see rand comments.

I dont feel particularly strongly about it nonetheless, so up to you.

(let (count (read-subu8vector bytes start end *urandom*))
(unless (eq? count (##fx- end start))
(error "Could not read enough random bytes" count start end))))
(let (result (RAND_bytes bytes start end))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need initialization?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From < https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html >:

On all major platforms supported by OpenSSL (including the Unix-like platforms and Windows), OpenSSL is configured to automatically seed the CSPRNG on first use using the operating systems's random generator.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it reasonable to replace urandom with this?
It sounds generous and might take users by surprise.

@@ -126,14 +126,12 @@
buf)
#f))))

(def *urandom* (open-input-file "/dev/urandom"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looses functionality i think; it was possible previously to seed the rng with something useful for reproducible runs (say a file read from /dev/urandom).
We no longer have this capability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproducibility is important for testing, and non-reproducibility for security. I think there should be two separate primitives for each, and code that needs to be tested with one yet run with the other might have a vector to choose between the two.

The crypto library looks like the right place for the actual non-reproducible bit. Akshully, we'd ideally add the additional functions for entropy control, too.

@vyzo
Copy link
Collaborator

vyzo commented Feb 19, 2023

I would still like to keep the option of using urandom; no matter how you sugar it, if you want strong randomness it's the only game in town.

@fare
Copy link
Collaborator Author

fare commented Feb 19, 2023

RAND_bytes already implicitly uses urandom at lazy initialization time (and whatever else works on Windows). I suppose we could add additional functions to query urandom. But then again, there's also /dev/random in case you want the kernel to guarantee some entropy (which might matter at boot time on some embedded devices).

@vyzo
Copy link
Collaborator

vyzo commented Feb 19, 2023

Sounds like a good opportunity to generally improve the situation api wise.

@fare
Copy link
Collaborator Author

fare commented Feb 19, 2023

Agreed that a better API is needed at some point. Actually, I am thinking about porting some property-based testing library (as made popular by Haskell's QuickCheck) to Gerbil Scheme at some point, and the deterministic PRNG aspect will then become salient.

Make it easier to copy-paste the name of a command by not displaying colon
(:) after the command. This also matches how git and other software display
their sub-commands in their help.

See brief discussion in PR mighty-gerbils#648.
@fare fare merged commit 71cee54 into mighty-gerbils:master Feb 19, 2023
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.

2 participants