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

[Process] Be able to not inherit ENV var #24397

Closed
lyrixx opened this Issue Oct 2, 2017 · 7 comments

Comments

Projects
None yet
5 participants
@lyrixx
Member

lyrixx commented Oct 2, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version 4.0

Nowadays, symfony recommend using env var to configure the application.
So in the env we can find some API credentials.

If an application is using the Process composant some information can leak via the env variables.

So I would like to be able to NOT inherit from the parent process to be as safe as possible.

For now the solution to disable the propagation is to use this code:


$process = new Symfony\Component\Process\Process('env');

$env = array_combine(array_keys($_SERVER), array_fill(0, count($_SERVER), false));

$process->run(null, $env);

echo $process->getOutput();

It's not really easy :/


And if you need a use case: https://twitter.com/o_cee/status/892306836199800836 (yeah NPM drama inside, but anyway it could happens with composer too !)


Finally here I'm just asking for a way to not inherit env var. But Ideally Symfony, by default, should not inherit env var (but it's another story)

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 2, 2017

Member

The issue with not inheriting env vars by default is that some of them are required to perform correctly.
The most obvious is PATH, but there is also http_proxy, and some others I don't know about, and even others on Windows.
That's why I made this move in 3.3: you should inherit the env vars you don't know about, and deal only with the ones you know about (setting or unsetting them).
Usetting all env vars looks like a bad practice to me.
But I may be wrong, so I'm happy you triggered the discussion :)

Member

nicolas-grekas commented Oct 2, 2017

The issue with not inheriting env vars by default is that some of them are required to perform correctly.
The most obvious is PATH, but there is also http_proxy, and some others I don't know about, and even others on Windows.
That's why I made this move in 3.3: you should inherit the env vars you don't know about, and deal only with the ones you know about (setting or unsetting them).
Usetting all env vars looks like a bad practice to me.
But I may be wrong, so I'm happy you triggered the discussion :)

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 2, 2017

Member

OH, and BTW, crendentials should be stored in files, not in env var ;)

Member

nicolas-grekas commented Oct 2, 2017

OH, and BTW, crendentials should be stored in files, not in env var ;)

@lyrixx

This comment has been minimized.

Show comment
Hide comment
@lyrixx

lyrixx Oct 2, 2017

Member

About HTTP_PROXY, PATH you are totally right.

About the CWD, I disagree because a script should not rely on the cwd. That's why in Symfony we are using direname(__FILE__) (yeah SF 1) to be able to locate a file.
In bash I keep using direname $0 every time to no depends on the cwd.

But again, here I just would like a way to disable this propagation.
So if someone want to disable the propagation (so opt-in) he has to forward at least HTTP_PROXY.

And if we disable all propagation by default, Symfony could have a white list of legit env var (PATH, HTTP_PROXY ...)


FYI:

docker run -it --rm ubuntu env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=43ff4d444380
TERM=xterm
HOME=/root

so

1/ Here is the list of default env var on ubuntu (in docker)
2/ Docker by default does not inherit env var


OH, and BTW, crendentials should be stored in files, not in env var ;)

Do you have any pointer for that?

Because according to https://12factor.net/config =>

An app’s config is everything that is likely to vary between deploys (staging, production, developer environments, etc). This includes:

Resource handles to the database, Memcached, and other backing services
Credentials to external services such as Amazon S3 or Twitter
Per-deploy values such as the canonical hostname for the deploy
Apps sometimes store config as constants in the code. This is a violation of twelve-factor, which requires strict separation of config from code. Config varies substantially across deploys, code does not.
[...]
The twelve-factor app stores config in environment variables (often shortened to env vars or env). Env vars are easy to change between deploys without changing any code; unlike config files, there is little chance of them being checked into the code repo accidentally; and unlike custom config files, or other config mechanisms such as Java System Properties, they are a language- and OS-agnostic standard.


So, IMHO, password and co. should go to env var.

Symfony reads the conf (param.yml) file only once per deploy (except if you clear cache). So if you want to change the twitter secret key (for exemple) you have to rewrite the file, clear the cache and so slow down the application.

With an env var, you can do that very smoothly without slow request. And clearing the cache when there are lot of traffic is not really possible. So basically you need a new deploy :/

Member

lyrixx commented Oct 2, 2017

About HTTP_PROXY, PATH you are totally right.

About the CWD, I disagree because a script should not rely on the cwd. That's why in Symfony we are using direname(__FILE__) (yeah SF 1) to be able to locate a file.
In bash I keep using direname $0 every time to no depends on the cwd.

But again, here I just would like a way to disable this propagation.
So if someone want to disable the propagation (so opt-in) he has to forward at least HTTP_PROXY.

And if we disable all propagation by default, Symfony could have a white list of legit env var (PATH, HTTP_PROXY ...)


FYI:

docker run -it --rm ubuntu env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=43ff4d444380
TERM=xterm
HOME=/root

so

1/ Here is the list of default env var on ubuntu (in docker)
2/ Docker by default does not inherit env var


OH, and BTW, crendentials should be stored in files, not in env var ;)

Do you have any pointer for that?

Because according to https://12factor.net/config =>

An app’s config is everything that is likely to vary between deploys (staging, production, developer environments, etc). This includes:

Resource handles to the database, Memcached, and other backing services
Credentials to external services such as Amazon S3 or Twitter
Per-deploy values such as the canonical hostname for the deploy
Apps sometimes store config as constants in the code. This is a violation of twelve-factor, which requires strict separation of config from code. Config varies substantially across deploys, code does not.
[...]
The twelve-factor app stores config in environment variables (often shortened to env vars or env). Env vars are easy to change between deploys without changing any code; unlike config files, there is little chance of them being checked into the code repo accidentally; and unlike custom config files, or other config mechanisms such as Java System Properties, they are a language- and OS-agnostic standard.


So, IMHO, password and co. should go to env var.

Symfony reads the conf (param.yml) file only once per deploy (except if you clear cache). So if you want to change the twitter secret key (for exemple) you have to rewrite the file, clear the cache and so slow down the application.

With an env var, you can do that very smoothly without slow request. And clearing the cache when there are lot of traffic is not really possible. So basically you need a new deploy :/

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Oct 2, 2017

Member

@lyrixx docker does not recommend putting secrets in env anymore, but in special files (that are managed by Docker itself in a special place as it manages the secrets for you).
And Symfony 3.4 supports reading a file content dynamically at runtime, precisely to support this pattern.

Member

stof commented Oct 2, 2017

@lyrixx docker does not recommend putting secrets in env anymore, but in special files (that are managed by Docker itself in a special place as it manages the secrets for you).
And Symfony 3.4 supports reading a file content dynamically at runtime, precisely to support this pattern.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@zdenekdrahos

This comment has been minimized.

Show comment
Hide comment
@zdenekdrahos

zdenekdrahos Oct 8, 2017

Contributor

Offtopic about Symfony + env
So it seems, that a configuration best practice for sensitive information is a little bit outdated.
The 12-factor app is even mentioned in Symfony v3.2 article about env. I guess that it will be the first article that a user finds if he searches something like symfony env, symfony twelve factors, ...

Contributor

zdenekdrahos commented Oct 8, 2017

Offtopic about Symfony + env
So it seems, that a configuration best practice for sensitive information is a little bit outdated.
The 12-factor app is even mentioned in Symfony v3.2 article about env. I guess that it will be the first article that a user finds if he searches something like symfony env, symfony twelve factors, ...

@lyrixx

This comment has been minimized.

Show comment
Hide comment
@lyrixx

lyrixx Oct 13, 2017

Member

Actually, I don't really need this feature. I'm closing it.

Member

lyrixx commented Oct 13, 2017

Actually, I don't really need this feature. I'm closing it.

@lyrixx lyrixx closed this Oct 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment