From 9b3b161d82fdacbbebecac37da9c01793689e61d Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Tue, 1 Sep 2020 13:43:09 +1000 Subject: [PATCH 1/4] snap: Fix "stack smashing" error in wrapper certbot.wrapper had implicit dependencies on sed, awk and coreutils, which were being accidentally provided through the host system. Because certbot.wrapper modifies LD_LIBRARY_PATH, this was causing some systems to load an incompatible combination of shared libraries, resulting sed crashing. This commit reduces the dependencies of this script to just gawk, and explicitly stages it as part of the Certbot snap. It additionally moves invocations of all host system programs to a moment prior to the modification of LD_LIBRARY_PATH, and the invocation of snapped programs to after the modification. Fixes #8245 --- certbot.wrapper | 6 +++++- certbot/CHANGELOG.md | 1 + snap/snapcraft.yaml | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/certbot.wrapper b/certbot.wrapper index 909f2e369..63be01675 100755 --- a/certbot.wrapper +++ b/certbot.wrapper @@ -27,10 +27,14 @@ case "${SNAP_ARCH}" in exit 1 esac +snap_connections=$(snap connections certbot) + PARTIAL_LIBRARY_PATH="${SNAP}/usr/lib/${ARCH_TRIPLET}/" export LD_LIBRARY_PATH="${PARTIAL_LIBRARY_PATH}:${LD_LIBRARY_PATH}" export CERTBOT_AUGEAS_PATH="${PARTIAL_LIBRARY_PATH}libaugeas.so.0" +# Below this line, all invoked programs must be provided by the snap rather than the host (#8245) + join() { sep=$1 first=$2 @@ -42,6 +46,6 @@ join() { fi } -paths=$(for plugin_snap in $(snap connections certbot|sed -n '2,$p'|awk '$1=="content[certbot-1]"{print $3}'|cut -d: -f1); do echo /snap/$plugin_snap/current/lib/python3.8/site-packages; done) +paths=$(for plugin_snap in $(gawk 'NR>1 { if ($1 == "content[certbot-1]") { split($3,a,":"); print a[1]; } }'<<<"$snap_connections"); do echo /snap/$plugin_snap/current/lib/python3.8/site-packages; done) export CERTBOT_PLUGIN_PATH=$(join : $paths) exec certbot "$@" diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 6f39a27b9..78495ceda 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -19,6 +19,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). fail to load the Augeas library it depends on has been fixed. * The `acme` library can now tell the ACME server to clear contact information by passing an empty `tuple` to the `contact` field of a `Registration` message. +* Fixed the `*** stack smashing detected ***` error in the Certbot snap on some systems. More details about these changes can be found on our GitHub repo. diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index b29899c31..342f32a08 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -71,6 +71,8 @@ parts: - python3-distutils - python3-pkg-resources - python3.8-minimal + # added for certbot.wrapper script: + - gawk # To build cryptography and cffi if needed build-packages: [gcc, libffi-dev, libssl-dev, git, libaugeas-dev, python3-dev] build-environment: From f146c997d7c9f0313a779148b533e0de5a52d326 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Wed, 2 Sep 2020 17:27:52 +1000 Subject: [PATCH 2/4] snap: Don't modify LD_LIBRARY_PATH --- certbot.wrapper | 24 ++++-------------------- snap/hooks/configure | 2 +- snap/hooks/prepare-plug-plugin | 2 +- snap/snapcraft.yaml | 3 ++- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/certbot.wrapper b/certbot.wrapper index 63be01675..973f8c6ac 100755 --- a/certbot.wrapper +++ b/certbot.wrapper @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh -x set -e # This code is based on snapcraft's own patch to work around this problem at @@ -27,25 +27,9 @@ case "${SNAP_ARCH}" in exit 1 esac -snap_connections=$(snap connections certbot) +export CERTBOT_AUGEAS_PATH="${SNAP}/usr/lib/${ARCH_TRIPLET}/libaugeas.so.0" -PARTIAL_LIBRARY_PATH="${SNAP}/usr/lib/${ARCH_TRIPLET}/" -export LD_LIBRARY_PATH="${PARTIAL_LIBRARY_PATH}:${LD_LIBRARY_PATH}" -export CERTBOT_AUGEAS_PATH="${PARTIAL_LIBRARY_PATH}libaugeas.so.0" +CERTBOT_PLUGIN_PATH="$(curl -s --unix /run/snapd.socket "http://localhost/v2/connections?snap=certbot&interface=content" | jq -r '.result.established | map(select(.plug.plug == "plugin" and ."plug-attrs".content == "certbot-1") | "/snap/"+.slot.snap+"/current/lib/python3.8/site-packages" ) | join(":")')" +export CERTBOT_PLUGIN_PATH -# Below this line, all invoked programs must be provided by the snap rather than the host (#8245) - -join() { - sep=$1 - first=$2 - if [ "$first" != "" ]; then - shift 2 - echo -n "${first}" - for item in "$@"; do echo -n "${sep}${item}"; done - echo - fi -} - -paths=$(for plugin_snap in $(gawk 'NR>1 { if ($1 == "content[certbot-1]") { split($3,a,":"); print a[1]; } }'<<<"$snap_connections"); do echo /snap/$plugin_snap/current/lib/python3.8/site-packages; done) -export CERTBOT_PLUGIN_PATH=$(join : $paths) exec certbot "$@" diff --git a/snap/hooks/configure b/snap/hooks/configure index 2678c47b2..9545f8b8a 100644 --- a/snap/hooks/configure +++ b/snap/hooks/configure @@ -1,3 +1,3 @@ -#!/bin/bash -e +#!/bin/sh -e exit 0 diff --git a/snap/hooks/prepare-plug-plugin b/snap/hooks/prepare-plug-plugin index ee309addf..f2f7ff86b 100644 --- a/snap/hooks/prepare-plug-plugin +++ b/snap/hooks/prepare-plug-plugin @@ -1,4 +1,4 @@ -#!/bin/bash -e +#!/bin/sh -e if [ "$(snapctl get trust-plugin-with-root)" = "ok" ]; then # allow the connection, but reset config to allow for other slots to go through this auth flow diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 342f32a08..c32e8c6b3 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -72,7 +72,8 @@ parts: - python3-pkg-resources - python3.8-minimal # added for certbot.wrapper script: - - gawk + - curl + - jq # To build cryptography and cffi if needed build-packages: [gcc, libffi-dev, libssl-dev, git, libaugeas-dev, python3-dev] build-environment: From 6fc2516a13aa6651505d1e1bcd71c650806e0cbf Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Wed, 2 Sep 2020 17:31:40 +1000 Subject: [PATCH 3/4] leftover tracing --- certbot.wrapper | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot.wrapper b/certbot.wrapper index 973f8c6ac..cbf0ddf9f 100755 --- a/certbot.wrapper +++ b/certbot.wrapper @@ -1,4 +1,4 @@ -#!/bin/sh -x +#!/bin/sh set -e # This code is based on snapcraft's own patch to work around this problem at From 7f22561237be8569dac79a49c84a6f69f03b58d2 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Thu, 3 Sep 2020 10:14:15 +1000 Subject: [PATCH 4/4] snap: revert curl/jq in wrapper, use gawk for now --- certbot.wrapper | 2 +- snap/snapcraft.yaml | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/certbot.wrapper b/certbot.wrapper index cbf0ddf9f..b7ce120bd 100755 --- a/certbot.wrapper +++ b/certbot.wrapper @@ -29,7 +29,7 @@ esac export CERTBOT_AUGEAS_PATH="${SNAP}/usr/lib/${ARCH_TRIPLET}/libaugeas.so.0" -CERTBOT_PLUGIN_PATH="$(curl -s --unix /run/snapd.socket "http://localhost/v2/connections?snap=certbot&interface=content" | jq -r '.result.established | map(select(.plug.plug == "plugin" and ."plug-attrs".content == "certbot-1") | "/snap/"+.slot.snap+"/current/lib/python3.8/site-packages" ) | join(":")')" +CERTBOT_PLUGIN_PATH="$(snap connections certbot | gawk 'BEGIN {ORS=""} NR>1 { if ($1 == "content[certbot-1]") { split($3,a,":"); PLUGINS=PLUGINS":/snap/"a[1]"/current/lib/python3.8/site-packages/"; next; } } END { print substr(PLUGINS, 2) }')" export CERTBOT_PLUGIN_PATH exec certbot "$@" diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index c32e8c6b3..342f32a08 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -72,8 +72,7 @@ parts: - python3-pkg-resources - python3.8-minimal # added for certbot.wrapper script: - - curl - - jq + - gawk # To build cryptography and cffi if needed build-packages: [gcc, libffi-dev, libssl-dev, git, libaugeas-dev, python3-dev] build-environment: