threefoldtech / cloud-container

A builder for a simple initramfs image to run container over virtiofs inside cloud-hypervisor
Apache License 2.0
0 stars 1 forks source link

How to escape environment variables #5

Closed OmarElawady closed 2 years ago

OmarElawady commented 2 years ago

Currently, the environment variables is appended in /etc/environment and then parsed in the cloud-container init script (using sh). This can work with extra escaping from zos side or from cloud-container side, but will break the /etc/environment syntax expected by other processes that parses it (e.g. systemd). The compatibility is important in case someone deployed an image with systemd in it, it will parse /etc/environment in a different way than ours and override what it parsed leading to malformed environment variables.

/etc/environment however appears to be limited in the values it can represent. I tried in a virtualbox VM to include a hash in a variable value by escaping and quoting without success (and it looks like I'm not the only one)

Another semi-irrelevant problem is that /etc/environment is readable by all processes. Which can be thought of as a security issue if the user expects that the environment variables are passed to the init process and it manages them the way it wants.

A way to go is to create a file /.zosenv with a sh syntax in zos and handle it in init and then delete it. This will probably make ssh sessions for example without these variables, but imho this should be an expected behavior. If someone wants it to be seen in all bash sessions, they should write it in ~/.bashrc at initialization.

muhamadazmy commented 2 years ago

Well, you are raising very good points here. The reason I choose /etc/environment is that I wanted to be as close to the standard way of setting global environment variables as possible. But indeed, there is no point of doing this if we only need to pass the environment variables to PID 1 (init)

I agree with you that having a custom files .zosenv is a good name or even a .zosrc in case we decided to include more custom initialization code in the future since it's gonna be an sh script anyway.

OmarElawady commented 2 years ago

done in https://github.com/threefoldtech/cloud-container/pull/6