diff options
| author | ohemorange <ebportnoy@gmail.com> | 2020-06-26 01:36:29 +0300 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-06-26 01:36:29 +0300 |
| commit | 713b91495b4d9ff062afdede19add92a4aa8933d (patch) | |
| tree | 004f421f275d744b429b28dc389da6439b7a8adb /snap | |
| parent | 0f4c31c9c74cc390912700c6bcd2099b5e62d1b7 (diff) | |
Fix paths when calling out to programs outside of snap (#8108)
Fixes #8093.
This PR modifies and audits all uses of `subprocess` and `Popen` outside of tests, `certbot-ci/`, `certbot-compatibility-test/`, `letsencrypt-auto-source/`, `tools/`, and `windows-installer/`. Calls to outside programs have their `env` modified to remove the `SNAP` components of paths, if they exist. This includes any calls made from hooks, calls to `apachectl` and `nginx`, and to `openssl` from `ocsp.py`.
For testing manually, rsync flags will look something like:
```
rsync -avzhe ssh root@focal.domain:/home/certbot/certbot/certbot_*_amd64.snap .
rsync -avzhe ssh certbot_*_amd64.snap root@centos7.domain:/root/certbot/
```
With these modifications, `certbot plugins --prepare` now passes on Centos 7.
If I'm wrong and we package the `openssl` binary, the modifications should be removed from `ocsp.py`, and `env` should be passed into `run_script` rather than set internally in its calls from nginx and apache.
One caveat with this approach is the disconnect between why it's a problem (packaging) and where it's solved (internal to Certbot). I considered a wrapping approach, but we'd still have to audit specific calls. I think the best way to address this is robust testing; specifically, running the snap on other systems.
For hooks, all calls will remove the snap paths if they exist. This is probably fine, because even if the hook intends to call back into certbot, it can do that, it'll just create a new snap.
I'm not sure if we need these modifications for the Mac OS X/ Darwin calls, but they can't hurt.
* Add method to plugins util to get env without snap paths
* Use modified environment in Nginx plugin
* Pass through env to certbot.util.run_script
* Use modified environment in Apache plugin
* move env_no_snap_for_external_calls to certbot.util
* Set env internally to run_script, since we use that only to call out
* Add env to mac subprocess calls in certbot.util
* Add env to openssl call in ocsp.py
* Add env for hooks calls in certbot.compat.misc.
* Pass env into execute_command to avoid circular dependency
* Update hook test to assert called with env
* Fix mypy type hint to account for new param
* Change signature to include Optional
* go back to using CERTBOT_PLUGIN_PATH
* no need to modify PYTHONPATH in env
* robustly detect when we're in a snap
* Improve env util fxn docstring
* Update changelog
* Add unit tests for env_no_snap_for_external_calls
* Import compat.os
Diffstat (limited to 'snap')
| -rw-r--r-- | snap/snapcraft.yaml | 2 |
1 files changed, 2 insertions, 0 deletions
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index e958939b3..14c1fc9e7 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -25,6 +25,7 @@ apps: PATH: "$SNAP/bin:$SNAP/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games" AUGEAS_LENS_LIB: "$SNAP/usr/share/augeas/lenses/dist" LD_LIBRARY_PATH: "$SNAP/usr/lib/x86_64-linux-gnu/:$LD_LIBRARY_PATH" + CERTBOT_SNAPPED: "True" renew: command: certbot.wrapper -q renew daemon: oneshot @@ -32,6 +33,7 @@ apps: PATH: "$SNAP/bin:$SNAP/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games" AUGEAS_LENS_LIB: $SNAP/usr/share/augeas/lenses/dist LD_LIBRARY_PATH: "$SNAP/usr/lib/x86_64-linux-gnu/:$LD_LIBRARY_PATH" + CERTBOT_SNAPPED: "True" # Run approximately twice a day with randomization timer: 00:00~24:00/2 |