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

rand: use arc4random as fallback when available #10672

Closed
wants to merge 1 commit into from

Conversation

piru
Copy link

@piru piru commented Mar 4, 2023

Normally curl uses cryptographically strong random provided by the selected SSL backend. If compiled without SSL support, a naive built-in function was used instead.

Generally this was okay, but it will result in some downsides for non- SSL builds, such as predictable temporary file names.

This change ensures that arc4random will be used instead, if available.

@piru piru force-pushed the use-arc4random-when-available branch from da8bd84 to bdb741e Compare March 4, 2023 07:19
Normally curl uses cryptographically strong random provided by the
selected SSL backend. If compiled without SSL support, a naive built-in
function was used instead.

Generally this was okay, but it will result in some downsides for non-
SSL builds, such as predictable temporary file names.

This change ensures that arc4random will be used instead, if available.
@piru piru force-pushed the use-arc4random-when-available branch from bdb741e to a9c481d Compare March 4, 2023 08:34
@bagder bagder closed this in 755ddbe Mar 6, 2023
@bagder
Copy link
Member

bagder commented Mar 6, 2023

Thanks!

@piru piru deleted the use-arc4random-when-available branch March 6, 2023 11:57
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
Normally curl uses cryptographically strong random provided by the
selected SSL backend. If compiled without SSL support, a naive built-in
function was used instead.

Generally this was okay, but it will result in some downsides for non-
SSL builds, such as predictable temporary file names.

This change ensures that arc4random will be used instead, if available.

Closes curl#10672
vszakats added a commit to vszakats/curl that referenced this pull request Nov 5, 2023
autotools unexpectedly detects `arc4random` because it is also looking
into dependency libs. One dependency, LibreSSL, happens to publish an
`arc4random` function (via its shared lib before v3.7, also via static
lib as of v3.8.2). When trying to use this function in `lib/rand.c`,
its protoype is missing. To fix that, curl included a prototype, but
that used a C99 type without including `stdint.h`, causing:

```
../../lib/rand.c:37:1: error: unknown type name 'uint32_t'
   37 | uint32_t arc4random(void);
      | ^
1 error generated.
```

This patch improves this by dropping the local prototype and instead
limiting `arc4random` use for non-OpenSSL builds. OpenSSL builds provide
their own random source anyway.

The better fix would be to teach autotools to not link dependency libs
while detecting `arc4random`.

LibreSSL publishing a non-namespaced `arc4random` tracked here:
libressl/portable#928

Regression from 755ddbe curl#10672

Fixes curl#12257
Closes #xxxxx
vszakats added a commit that referenced this pull request Nov 6, 2023
autotools unexpectedly detects `arc4random` because it is also looking
into dependency libs. One dependency, LibreSSL, happens to publish an
`arc4random` function (via its shared lib before v3.7, also via static
lib as of v3.8.2). When trying to use this function in `lib/rand.c`,
its protoype is missing. To fix that, curl included a prototype, but
that used a C99 type without including `stdint.h`, causing:

```
../../lib/rand.c:37:1: error: unknown type name 'uint32_t'
   37 | uint32_t arc4random(void);
      | ^
1 error generated.
```

This patch improves this by dropping the local prototype and instead
limiting `arc4random` use for non-OpenSSL builds. OpenSSL builds provide
their own random source anyway.

The better fix would be to teach autotools to not link dependency libs
while detecting `arc4random`.

LibreSSL publishing a non-namespaced `arc4random` tracked here:
libressl/portable#928

Regression from 755ddbe #10672

Reviewed-by: Daniel Stenberg
Fixes #12257
Closes #12274
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants