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.

Sign up for GitHub

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

Jump to bottom

macOS use SecRandomCopyBytes instead of getentropy #20466

Merged
merged 2 commits into from
Oct 5, 2022
Merged

macOS use SecRandomCopyBytes instead of getentropy #20466

merged 2 commits into from
Oct 5, 2022

Conversation

iffy
Copy link
Contributor

@iffy iffy commented Sep 30, 2022

This changes the urandom proc to use SecRandomCopyBytes on macOS instead of getentropy because:

This fixes my application that I currently build for a macOS 10.10 machine after upgrading to Nim 1.6 from Nim 1.4. This supersedes and closes #20460

@iffy
Copy link
Contributor Author

iffy commented Sep 30, 2022

Ooo, nice side benefit: I can now compile nim on macOS 10.10 again!

@iffy
Copy link
Contributor Author

iffy commented Sep 30, 2022

@ringabout I just realized that this change I'm proposing is pretty much the same as what you did in ringabout@c467017 Any reason why you didn't end up merging that change in?

@iffy
Copy link
Contributor Author

iffy commented Oct 4, 2022

CI is only failing for the unrelated problem fixed by #20495. Can this be merged?

@iffy iffy mentioned this pull request Oct 4, 2022
Closed
@Araq
Copy link
Member

Araq commented Oct 5, 2022

Ping @ringabout

@ringabout
Copy link
Member

I just realized that this change I'm proposing is pretty much the same as what you did in ringabout@c467017 Any reason why you didn't end up merging that change in?

Because I don't want to introduce dependency -framework Security when having a better solution for newer macOS. Depends on how old OS we are willing to support, it is fine to merge this if we stick to backwards compatibility.

@iffy
Copy link
Contributor Author

iffy commented Oct 5, 2022

FWIW, some people have experienced Apple not allowing apps that use getentropy: https://dev.gnupg.org/T5375

What's the reason for not wanting to pull in the Security framework? I think it's available on all macOS machines.

If it's really undesirable to pull in the Security framework, this post makes me think it might be possible to use CCRandomGenerateBytes which might be in libSystem. I haven't had time to check yet.

@ringabout
Copy link
Member

See also rust-random/getrandom#38 (comment)

@Varriount Varriount merged commit 594e93a into nim-lang:devel Oct 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 594e93a

Hint: mm: orc; opt: speed; options: -d:release
162672 lines; 8.026s; 665.031MiB peakmem

@iffy iffy mentioned this pull request Feb 17, 2023
Merged
Varriount added a commit that referenced this pull request Mar 2, 2023
@iffy @Varriount @ringabout
Backport #20466 - macOS use SecRandomCopyBytes instead of getentropy (#…
5c36f24 
…21389)

* On macOS use SecRandomCopyBytes instead of getentropy (which is only available on macOS 10.12+)

* Change passL to passl

---------

Co-authored-by: Clay Sweetser <[email protected]>
Co-authored-by: ringabout <[email protected]>
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
@iffy @capocasa
macOS use SecRandomCopyBytes instead of getentropy (nim-lang#20466)
9dcb910 
* On macOS use SecRandomCopyBytes instead of getentropy (which is only available on macOS 10.12+)

* Change passL to passl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iffy @Araq @ringabout @Varriount