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

Take advantage of AT_RANDOM for seeding the PRNG #8173

Closed
wants to merge 1 commit into from

Conversation

daurnimator
Copy link
Contributor

Restores some code from 53987c9/#7482

  • Avoid calling the Gimli permute function so we don't pull Gimli into every executable.
  • Add an opt-out in the form of root.use_AT_RANDOM_auxval

Restores some code from 53987c9.

  - Avoid calling the Gimli permute function so we don't pull Gimli into every executable.
  - Add an opt-out in the form of root.use_AT_RANDOM_auxval
@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Mar 7, 2021
@jedisct1
Copy link
Contributor

jedisct1 commented Mar 7, 2021

I still feel like this is not a good idea. Or, maybe the switch should be set to false by default.

If dlopen()ed libraries do the same thing, they will end up with a static, all zero seed.

If our Zig code is used as a library, an application may also have cleared it before.

This also constrains us to a 128 bit seed. Which is fine, but 256 bit doesn't hurt, especially with a new and non-ideal permutation.

@daurnimator
Copy link
Contributor Author

If dlopen()ed libraries do the same thing, they will end up with a static, all zero seed.

If our Zig code is used as a library, an application may also have cleared it before.

The code is only run when zig is the entrypoint: the call site is inside of posixCallMainAndExit.
For other contexts (e.g. a library) then it won't get run and nothing changes from the status quo.

an application may also have cleared it before.

See:

    // don't use AT_RANDOM if it has been zeroed out
    if (mem.allEqual(u8, ptr, 0)) return;

Though due to the aforementioned reason, this should never be hit by "normal" code.

@jedisct1
Copy link
Contributor

jedisct1 commented Mar 7, 2021

Also, clearing it is nice, but with SSP turned on, a copy is still present.

@jedisct1
Copy link
Contributor

jedisct1 commented Mar 7, 2021

The code is only run when zig is the entrypoint: the call site is inside of posixCallMainAndExit.

That doesn't address the issue of libraries possibly using AT_RANDOM being dlopen()d by a Zig application.

@daurnimator
Copy link
Contributor Author

with SSP turned on, a copy is still present.

oh? howso/where?

That doesn't address the issue of libraries possibly using AT_RANDOM being dlopen()d by a Zig application.

  1. isn't that prevented by us overwriting AT_RANDOM?
  2. dlopen in particular is (currently) only available when linking libc; which usually means that our _start code won't be used (and hence won't use AT_RANDOM).

@LemonBoy
Copy link
Contributor

LemonBoy commented Mar 7, 2021

What's the problem you're trying to solve here? The default state for the TLSPRNG is set to .unitinialized and it will be properly seeded the first time it is used.

@jedisct1
Copy link
Contributor

jedisct1 commented Mar 7, 2021

oh? howso/where?

Isn't this in fs:0x28?

The stack canary has to persist somewhere till the end of the process. And if what we clear is not a copy, this kinda defeats the purpose of SSP.

But I may be completely wrong here; this is in a glibc context, maybe we don't even support SSP otherwise.

Still, that change doesn't make me comfortable at all and I'm not convinced that it solves any actual problem.

UPDATE: Quick note on canaries.

On x86_64 anything compiled with stack protection expects the canary (a copy of the AT_RANDOM value) at fs:0x28. This includes C files compiled by Zig, that have stack protection on by default.

But when we don't link with libc, we currently don't initialize this, ending up with an all zero canary, making stack protection useless.

@daurnimator
Copy link
Contributor Author

UPDATE: Quick note on canaries.

On x86_64 anything compiled with stack protection expects the canary (a copy of the AT_RANDOM value) at fs:0x28. This includes C files compiled by Zig, that have stack protection on by default.

But when we don't link with libc, we currently don't initialize this, ending up with an all zero canary, making stack protection useless.

hmmm. I didn't know this. Where is this documented?

@andrewrk andrewrk closed this May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants