From 9ba9dceb1a709f47c059d2e21d1e4e5b1b81d00f Mon Sep 17 00:00:00 2001 From: Evilham Date: Sun, 7 Feb 2021 02:29:52 +0100 Subject: [PATCH 01/12] [__letsencrypt_cert] Revamp explorers, add locking. This would fix #839 Certbot uses locking [1] even for read-only operations and does not properly use exit codes, which means that sometimes it would print: "Another instance of Certbot is already running" and exit with success. However, the previous explorers would take that as the certificate being absent and would trigger code generation. The issue was made worse by having many explorers running certbot, so for N certificates, we'd run certbot N*4 times, potentially "in parallel". [1]: https://certbot.eff.org/docs/using.html#id5 This patch joins all explorers in one to avoid starting multiple remote python processes and uses a cdist-specific lock in /tmp/certbot.cdist.lock with a 60 seconds timeout. It has been tested with certbot 0.31.0 and 0.17 that the: from certbot.main import main trick works. It is somewhat well documented so it can be somewhat relied upon. --- .../__letsencrypt_cert/explorer/certbot-path | 3 - .../explorer/certificate-data | 78 +++++++++++++++++++ .../explorer/certificate-domains | 8 -- .../explorer/certificate-exists | 13 ---- .../explorer/certificate-is-test | 14 ---- .../type/__letsencrypt_cert/gencode-remote | 11 ++- cdist/conf/type/__letsencrypt_cert/manifest | 2 +- 7 files changed, 87 insertions(+), 42 deletions(-) delete mode 100755 cdist/conf/type/__letsencrypt_cert/explorer/certbot-path create mode 100755 cdist/conf/type/__letsencrypt_cert/explorer/certificate-data delete mode 100755 cdist/conf/type/__letsencrypt_cert/explorer/certificate-domains delete mode 100755 cdist/conf/type/__letsencrypt_cert/explorer/certificate-exists delete mode 100755 cdist/conf/type/__letsencrypt_cert/explorer/certificate-is-test diff --git a/cdist/conf/type/__letsencrypt_cert/explorer/certbot-path b/cdist/conf/type/__letsencrypt_cert/explorer/certbot-path deleted file mode 100755 index 3c6076df..00000000 --- a/cdist/conf/type/__letsencrypt_cert/explorer/certbot-path +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/sh -e - -command -v certbot 2>/dev/null || true diff --git a/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data b/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data new file mode 100755 index 00000000..13f0d4d1 --- /dev/null +++ b/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data @@ -0,0 +1,78 @@ +#!/bin/sh -e +certbot_path="$(command -v certbot 2>/dev/null || true)" +# Defaults +certificate_exists="no" +certificate_is_test="no" + +if [ -n "${certbot_path}" ]; then + # Find python executable that has access to certbot's module + python_path="$(head -n 1 "${certbot_path}" | cut -d '!' -f 2)" + + # Use a lock for cdist due to certbot not exiting with failure + # or having any flags for concurrent use. + _certbot() { + ${python_path} - 2>/dev/null < "${existing_domains}" + certificate_is_test="$(_explorer_var certificate_is_test)" sort -uo "${requested_domains}" "${requested_domains}" sort -uo "${existing_domains}" "${existing_domains}" diff --git a/cdist/conf/type/__letsencrypt_cert/manifest b/cdist/conf/type/__letsencrypt_cert/manifest index b4464366..d28e78df 100755 --- a/cdist/conf/type/__letsencrypt_cert/manifest +++ b/cdist/conf/type/__letsencrypt_cert/manifest @@ -1,6 +1,6 @@ #!/bin/sh -certbot_fullpath="$(cat "${__object:?}/explorer/certbot-path")" +certbot_fullpath="$(grep "^certbot_path:" "${__object:?}/explorer/certificate-data" | cut -d ':' -f 2-)" if [ -z "${certbot_fullpath}" ]; then os="$(cat "${__global:?}/explorer/os")" From 01c9f793576811ef1b167a58f79e82aeabb45b78 Mon Sep 17 00:00:00 2001 From: Evilham Date: Sun, 7 Feb 2021 20:19:50 +0100 Subject: [PATCH 02/12] [__letsencrypt_cert] Simplify derivation of python path. This also supports odd setups with '!' in the path. --- cdist/conf/type/__letsencrypt_cert/explorer/certificate-data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data b/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data index 13f0d4d1..ff62e742 100755 --- a/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data +++ b/cdist/conf/type/__letsencrypt_cert/explorer/certificate-data @@ -6,7 +6,7 @@ certificate_is_test="no" if [ -n "${certbot_path}" ]; then # Find python executable that has access to certbot's module - python_path="$(head -n 1 "${certbot_path}" | cut -d '!' -f 2)" + python_path=$(sed -n '1s/^#! *//p' "${certbot_path}") # Use a lock for cdist due to certbot not exiting with failure # or having any flags for concurrent use. From 60ed6f35fdf5111461c62e0df59e638ff4c5cf68 Mon Sep 17 00:00:00 2001 From: Evilham Date: Tue, 23 Feb 2021 13:41:00 +0100 Subject: [PATCH 03/12] [__letsencrypt_cert] Be more consistent with temp files Noticed this while doing previous work on the type. --- cdist/conf/type/__letsencrypt_cert/gencode-remote | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cdist/conf/type/__letsencrypt_cert/gencode-remote b/cdist/conf/type/__letsencrypt_cert/gencode-remote index 801ac033..60a77ec3 100755 --- a/cdist/conf/type/__letsencrypt_cert/gencode-remote +++ b/cdist/conf/type/__letsencrypt_cert/gencode-remote @@ -33,7 +33,7 @@ case "${state}" in fi if [ "${certificate_exists}" = "yes" ]; then - existing_domains="${__object}/existing_domains" + existing_domains=$(mktemp "${TMPDIR:-/tmp}/existing_domains.cdist.XXXXXXXXXX") tail -n +4 "${__object:?}/explorer/certificate-data" | grep -v '^$' > "${existing_domains}" certificate_is_test="$(_explorer_var certificate_is_test)" From 60fd7ba1f38382761c366cbc0425ddaec62f1164 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Sun, 28 Feb 2021 13:37:23 +0100 Subject: [PATCH 04/12] Release 6.9.5 --- docs/changelog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog b/docs/changelog index 88dda0aa..95e36b88 100644 --- a/docs/changelog +++ b/docs/changelog @@ -1,7 +1,7 @@ Changelog --------- -next: +6.9.5: 2021-02-28 * Core: preos: Fix passing cdist debug parameter (Darko Poljak) * Type __sshd_config: Produce error if invalid config is generated, fix processing of AuthenticationMethods and AuthorizedKeysFile, document explorer bug (Dennis Camera) * Explorer memory: Fix result units; support Solaris (Dennis Camera) From 8ef19d47f6b5fb6cdcba7a9a3b8890388dbd7023 Mon Sep 17 00:00:00 2001 From: Dennis Camera Date: Mon, 1 Mar 2021 17:59:25 +0100 Subject: [PATCH 05/12] [type/__pyvenv] Fix example (--user -> --owner) --- cdist/conf/type/__pyvenv/man.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cdist/conf/type/__pyvenv/man.rst b/cdist/conf/type/__pyvenv/man.rst index 8085ff12..e2e4a1e6 100644 --- a/cdist/conf/type/__pyvenv/man.rst +++ b/cdist/conf/type/__pyvenv/man.rst @@ -61,7 +61,7 @@ EXAMPLES __pyvenv /home/foo/fooenv --pyvenv /usr/local/bin/pyvenv-3.4 # Create python virtualenv for user foo. - __pyvenv /home/foo/fooenv --group foo --user foo + __pyvenv /home/foo/fooenv --group foo --owner foo # Create python virtualenv with specific parameters. __pyvenv /home/services/djangoenv --venvparams "--copies --system-site-packages" From e7d33891df945831d54d164eb13f2a9ae4f9dd8e Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 2 Mar 2021 09:29:33 +0100 Subject: [PATCH 06/12] ++changelog --- docs/changelog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog b/docs/changelog index 95e36b88..072fcb6b 100644 --- a/docs/changelog +++ b/docs/changelog @@ -1,6 +1,9 @@ Changelog --------- +next: + * Type __pyvenv: Fix user example in man page (Dennis Camera) + 6.9.5: 2021-02-28 * Core: preos: Fix passing cdist debug parameter (Darko Poljak) * Type __sshd_config: Produce error if invalid config is generated, fix processing of AuthenticationMethods and AuthorizedKeysFile, document explorer bug (Dennis Camera) From ea0126dd8117c051612e6b6abfe895f5f722c584 Mon Sep 17 00:00:00 2001 From: Steven Armstrong Date: Fri, 5 Mar 2021 16:11:49 +0100 Subject: [PATCH 07/12] Make local state dir available to custom remote scripts Signed-off-by: Steven Armstrong --- cdist/config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cdist/config.py b/cdist/config.py index e84f6f84..19d5bd70 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -420,6 +420,9 @@ class Config: exec_path=sys.argv[0], save_output_streams=args.save_output_streams) + # Make __global state dir available to custom remote scripts. + os.environ['__global'] = local.base_path + remote = cdist.exec.remote.Remote( target_host=target_host, remote_exec=remote_exec, From ecba284fc8ce68486c5f1f87a863409fcad0f106 Mon Sep 17 00:00:00 2001 From: Steven Armstrong Date: Fri, 5 Mar 2021 16:13:02 +0100 Subject: [PATCH 08/12] changelog++ Signed-off-by: Steven Armstrong --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index 072fcb6b..ad93a595 100644 --- a/docs/changelog +++ b/docs/changelog @@ -3,6 +3,7 @@ Changelog next: * Type __pyvenv: Fix user example in man page (Dennis Camera) + * Core: config: Make local state directory available to custom remotes (Steven Armstrong 6.9.5: 2021-02-28 * Core: preos: Fix passing cdist debug parameter (Darko Poljak) From fb19f342669d18e6decb21ee4cfb2b22e46748d9 Mon Sep 17 00:00:00 2001 From: Dennis Camera Date: Tue, 9 Mar 2021 21:15:26 +0100 Subject: [PATCH 09/12] [type/__ssh_authorized_key] Only grep if file exists --- cdist/conf/type/__ssh_authorized_key/explorer/entry | 1 + cdist/conf/type/__ssh_authorized_key/gencode-remote | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cdist/conf/type/__ssh_authorized_key/explorer/entry b/cdist/conf/type/__ssh_authorized_key/explorer/entry index ccab0afc..aca0f2b9 100755 --- a/cdist/conf/type/__ssh_authorized_key/explorer/entry +++ b/cdist/conf/type/__ssh_authorized_key/explorer/entry @@ -25,6 +25,7 @@ type_and_key="$(tr ' ' '\n' < "$__object/parameter/key"| awk '/^(ssh|ecdsa)-[^ ] if [ -n "${type_and_key}" ] then file="$(cat "$__object/parameter/file")" + test -e "$file" || exit 0 # get any entries that match the type and key diff --git a/cdist/conf/type/__ssh_authorized_key/gencode-remote b/cdist/conf/type/__ssh_authorized_key/gencode-remote index f37aa565..61c77fb9 100755 --- a/cdist/conf/type/__ssh_authorized_key/gencode-remote +++ b/cdist/conf/type/__ssh_authorized_key/gencode-remote @@ -37,9 +37,9 @@ tmpfile=\$(mktemp ${file}.cdist.XXXXXXXXXX) # preserve ownership and permissions of existing file if [ -f "$file" ]; then cp -p "$file" "\$tmpfile" + grep -v -F -x '$line' '$file' >\$tmpfile fi -grep -v -F -x '$line' '$file' > \$tmpfile || true -mv -f "\$tmpfile" "$file" +cat "\$tmpfile" >"$file" DONE } From 31cc592aa10902c3e83357ee4a0b9300b5e34efa Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Wed, 10 Mar 2021 19:25:04 +0100 Subject: [PATCH 10/12] ++changelog --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index ad93a595..c63b901a 100644 --- a/docs/changelog +++ b/docs/changelog @@ -4,6 +4,7 @@ Changelog next: * Type __pyvenv: Fix user example in man page (Dennis Camera) * Core: config: Make local state directory available to custom remotes (Steven Armstrong + * Type __ssh_authorized_key: grep only if file exists (Dennis Camera) 6.9.5: 2021-02-28 * Core: preos: Fix passing cdist debug parameter (Darko Poljak) From e47c4dd8a4cc441b94fe73d362d79525d01a4bb1 Mon Sep 17 00:00:00 2001 From: Dennis Camera Date: Tue, 9 Mar 2021 19:59:05 +0100 Subject: [PATCH 11/12] [type/__sshd_config] Whitelist OpenBMC in manifest --- cdist/conf/type/__sshd_config/manifest | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cdist/conf/type/__sshd_config/manifest b/cdist/conf/type/__sshd_config/manifest index 566bde90..e37afebb 100755 --- a/cdist/conf/type/__sshd_config/manifest +++ b/cdist/conf/type/__sshd_config/manifest @@ -39,7 +39,14 @@ in (freebsd|netbsd|openbsd) # whitelist ;; + (openbmc-phosphor) + # whitelist + # OpenBMC can be configured with dropbear and OpenSSH. + # If dropbear is used, the state explorer will already fail because it + # cannot find the sshd binary. + ;; (*) + : "${__type:?}" # make shellcheck happy printf 'Your operating system (%s) is currently not supported by this type (%s)\n' \ "${os}" "${__type##*/}" >&2 printf 'Please contribute an implementation for it if you can.\n' >&2 From 10ca1c12fd25a9f2b92cebbc5d375a1723ed5e2f Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 12 Mar 2021 08:21:03 +0100 Subject: [PATCH 12/12] ++changelog --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index c63b901a..42a74d04 100644 --- a/docs/changelog +++ b/docs/changelog @@ -5,6 +5,7 @@ next: * Type __pyvenv: Fix user example in man page (Dennis Camera) * Core: config: Make local state directory available to custom remotes (Steven Armstrong * Type __ssh_authorized_key: grep only if file exists (Dennis Camera) + * Type __sshd_config: Whitelist OpenBMC (Dennis Camera) 6.9.5: 2021-02-28 * Core: preos: Fix passing cdist debug parameter (Darko Poljak)