Commit 3ba2ca38 authored by Jérome Perrin's avatar Jérome Perrin

Fix collective.recipe.shelloutput running "too early"

Our software using sshd were sometimes failing in tests, because the way they publish key fingerprint was racy.

It is based on `collective.recipe.shelloutput`, which as we can see in the [recipe code](https://github.com/collective/collective.recipe.shelloutput/blob/78e15c19/collective/recipe/shelloutput/__init__.py) operates on `__init__`.

We are using `collective.recipe.shelloutput` to capture the output of `ssh-keygen -lf $KEY` and this must run after the file `$KEY` is generated ( it is generated by another `plone.recipe.command` version). We were trying to run the `collective.recipe.shelloutput` after the `plone.recipe.command`, but that was incorrect anyway, because `collective.recipe.shelloutput` reads the file at `__init__` step, where `plone.recipe.command` creates the file at `install` step.
As we could see in test suite, it was sometimes working, when `slapos node instance` ran only once, but it sometimes  working, when `slapos node instance` ran more than once, for example because a promise failed and `slapos node instance` was retried.

Since `collective.recipe.shelloutput` does not take into account the exit code of the command but simply capture with `"Error ..."` whatever the command might output on stderr, we add another step checking that the captured output  is not `"Error ..."` and if it is cause a buildout error so that `slapos node instance` is retried and then succeed.

What should happen now is:
 1. `collective.recipe.shelloutput` reads the key fingerprint, the file is not present so it captures `"Error ..."``
 2. a `plone.recipe.command` creates the key
 3. another `plone.recipe.command` checks that the captured fingerprint is not `"Error ..."` it fails
 4. buildout restarts
 5. `collective.recipe.shelloutput` reads key fingerprint correctly.

Slaprunner has been heavily modified, because it was using a `sshkeys_authority` which was incompatible with this as it uses symlinks for keys. Since we don't know what is the purpose of `sshkeys_authority`, we rewrote that software to use simple commands instead of that "ssh keys authority".

/reviewed-on nexedi/slapos!681
parents 2331b9d7 4ba5d113
...@@ -19,4 +19,4 @@ md5sum = c4ac5de141ae6a64848309af03e51d88 ...@@ -19,4 +19,4 @@ md5sum = c4ac5de141ae6a64848309af03e51d88
[template-selenium] [template-selenium]
filename = instance-selenium.cfg.in filename = instance-selenium.cfg.in
md5sum = 4f557a7b3aa9b4df1ca1fa6a754ca657 md5sum = 1f0b67d2a542e94380c35afc9cd1946b
...@@ -184,18 +184,23 @@ extra-args=-t dsa ...@@ -184,18 +184,23 @@ extra-args=-t dsa
<=ssh-keygen-base <=ssh-keygen-base
extra-args=-t ecdsa -b 521 extra-args=-t ecdsa -b 521
[ssh-key-fingerprint-command] [ssh-key-fingerprint-shelloutput]
recipe = plone.recipe.command
# recent openssh client display ECDSA key's fingerprint as SHA256
command = ${openssh-output:keygen} -lf $${ssh-host-ecdsa-key:output}
[ssh-key-fingerprint]
recipe = collective.recipe.shelloutput recipe = collective.recipe.shelloutput
# XXX because collective.recipe.shelloutput ignore errors, we run the same # recent openssh client display ECDSA key's fingerprint as SHA256
# command in a plone.recipe.command so that if fails if something goes wrong.
commands = commands =
fingerprint = $${ssh-key-fingerprint-command:command} fingerprint = ${openssh-output:keygen} -lf $${ssh-host-ecdsa-key:output}
[ssh-key-fingerprint]
recipe = plone.recipe.command
stop-on-error = true
# XXX because collective.recipe.shelloutput ignore errors and capture output
# "Error ...", we use a plone.recipe.command to check that this command did
# not fail.
# This command will always fail on first buildout run, because
# collective.recipe.shelloutput is evaluated at buildout recipes __init__ step,
# but the key file is created later at install step.
command = echo "$${:fingerprint}" | ( grep ^Error || exit 0 && exit 1 )
fingerprint = $${ssh-key-fingerprint-shelloutput:fingerprint}
[sshd-config] [sshd-config]
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
......
...@@ -18,7 +18,7 @@ md5sum = 8b78e32b877d591400746ec7fd68ed4c ...@@ -18,7 +18,7 @@ md5sum = 8b78e32b877d591400746ec7fd68ed4c
[template-runner] [template-runner]
filename = instance-runner.cfg filename = instance-runner.cfg
md5sum = 87545b1f9f3865c8cb1347edeb340678 md5sum = 1216494c03752f0a3c1755e190eed3dc
[template-runner-import-script] [template-runner-import-script]
filename = template/runner-import.sh.jinja2 filename = template/runner-import.sh.jinja2
...@@ -26,7 +26,7 @@ md5sum = fc22e2d2f03ce58631f157a5b4943e15 ...@@ -26,7 +26,7 @@ md5sum = fc22e2d2f03ce58631f157a5b4943e15
[instance-runner-import] [instance-runner-import]
filename = instance-runner-import.cfg.in filename = instance-runner-import.cfg.in
md5sum = b450c474464a326f3d0b98728460ac97 md5sum = 918fb2984cb2ed7afba9200167f98a0f
[instance-runner-export] [instance-runner-export]
filename = instance-runner-export.cfg.in filename = instance-runner-export.cfg.in
...@@ -50,7 +50,7 @@ md5sum = 525e37ea8b2acf6209869999b15071a6 ...@@ -50,7 +50,7 @@ md5sum = 525e37ea8b2acf6209869999b15071a6
[template-slapos-cfg] [template-slapos-cfg]
filename = template/slapos.cfg.in filename = template/slapos.cfg.in
md5sum = da113b3e3e7bac9cc215fede7c4911a5 md5sum = e6a3ca1604ae5458248135cd6de0f3e6
[template-parameters] [template-parameters]
filename = parameters.xml.in filename = parameters.xml.in
...@@ -82,4 +82,4 @@ md5sum = 75aab99c995ca841f93fc77fc9116c37 ...@@ -82,4 +82,4 @@ md5sum = 75aab99c995ca841f93fc77fc9116c37
[template-buildout-shared-part-list] [template-buildout-shared-part-list]
filename = template/buildout-shared-part-list.in filename = template/buildout-shared-part-list.in
md5sum = 3203c9ad0b30d3ee39a809a067efff8d md5sum = 3203c9ad0b30d3ee39a809a067efff8d
\ No newline at end of file
...@@ -14,12 +14,8 @@ parts += ...@@ -14,12 +14,8 @@ parts +=
slaprunner-promise slaprunner-promise
slaprunner-supervisord-wrapper slaprunner-supervisord-wrapper
runner-sshd-add-authorized-key runner-sshd-add-authorized-key
runner-sshd-graceful
runner-sshd-promise runner-sshd-promise
runner-sshkeys-authority runner-sshd-service
runner-sshkeys-authority-service
runner-sshkeys-sshd
runner-sshkeys-sshd-service
runtestsuite runtestsuite
shellinabox shellinabox
shellinabox-service shellinabox-service
......
...@@ -15,12 +15,8 @@ common-runner-parts = ...@@ -15,12 +15,8 @@ common-runner-parts =
apache-httpd-promise apache-httpd-promise
slaprunner-supervisord-wrapper slaprunner-supervisord-wrapper
runner-sshd-add-authorized-key runner-sshd-add-authorized-key
runner-sshd-graceful
runner-sshd-promise runner-sshd-promise
runner-sshkeys-authority runner-sshd-service
runner-sshkeys-authority-service
runner-sshkeys-sshd
runner-sshkeys-sshd-service
runtestsuite runtestsuite
symlinks symlinks
shellinabox shellinabox
...@@ -177,9 +173,7 @@ shared_root = $${runnerdirectory:shared-root} ...@@ -177,9 +173,7 @@ shared_root = $${runnerdirectory:shared-root}
buildout-shared-part-list-dump = ${template-buildout-shared-part-list:output} buildout-shared-part-list-dump = ${template-buildout-shared-part-list:output}
pidfile-software = $${directory:run}/slapgrid-cp.pid pidfile-software = $${directory:run}/slapgrid-cp.pid
pidfile-instance = $${directory:run}/slapgrid-sr.pid pidfile-instance = $${directory:run}/slapgrid-sr.pid
ssh_client = ${openssh:location}/bin/ssh public_key = $${runner-sshd-ssh-host-rsa-key:output}
public_key = $${runner-sshd-raw-server:rsa-keyfile}.pub
private_key = $${runner-sshd-raw-server:rsa-keyfile}
instance-monitor-url = https://[$${:ipv6}]:$${slap-parameter:monitor-httpd-port} instance-monitor-url = https://[$${:ipv6}]:$${slap-parameter:monitor-httpd-port}
etc_dir = $${directory:etc} etc_dir = $${directory:etc}
log_dir = $${directory:log} log_dir = $${directory:log}
...@@ -256,106 +250,72 @@ ip = $${slap-network-information:global-ipv6} ...@@ -256,106 +250,72 @@ ip = $${slap-network-information:global-ipv6}
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
rendered = $${directory:etc}/runner-sshd.conf rendered = $${directory:etc}/runner-sshd.conf
path_pid = $${directory:run}/runner-sshd.pid path_pid = $${directory:run}/runner-sshd.pid
host_key = $${directory:ssh}/runner_server_key.rsa
template = inline: template = inline:
PidFile $${:path_pid} PidFile $${:path_pid}
Port $${runner-sshd-port:port} Port $${runner-sshd-port:port}
ListenAddress $${slap-network-information:global-ipv6} ListenAddress $${slap-network-information:global-ipv6}
Protocol 2 Protocol 2
UsePrivilegeSeparation no UsePrivilegeSeparation no
HostKey $${:host_key} HostKey $${runner-sshd-ssh-host-rsa-key:output}
HostKey $${runner-sshd-ssh-host-ecdsa-key:output}
PasswordAuthentication no PasswordAuthentication no
PubkeyAuthentication yes PubkeyAuthentication yes
AuthorizedKeysFile $${buildout:directory}/.ssh/authorized_keys AuthorizedKeysFile $${buildout:directory}/.ssh/authorized_keys
ForceCommand cd $${directory:home}; if [ -z "$SSH_ORIGINAL_COMMAND" ]; then HOME=$${directory:home} $${shell-environment:shell} -l; else HOME=$${directory:home} SHELL=$${shell-environment:shell} PATH=$${shell-environment:path} eval "$SSH_ORIGINAL_COMMAND"; fi ForceCommand cd $${directory:home}; if [ -z "$SSH_ORIGINAL_COMMAND" ]; then HOME=$${directory:home} $${shell-environment:shell} -l; else HOME=$${directory:home} SHELL=$${shell-environment:shell} PATH=$${shell-environment:path} eval "$SSH_ORIGINAL_COMMAND"; fi
Subsystem sftp ${openssh:location}/libexec/sftp-server Subsystem sftp ${openssh:location}/libexec/sftp-server
[runner-sshd-raw-server] [runner-sshd-service]
recipe = slapos.cookbook:wrapper recipe = slapos.cookbook:wrapper
host = $${slap-network-information:global-ipv6}
rsa-keyfile = $${runner-sshd-config:host_key}
home = $${directory:ssh}
command-line = ${openssh:location}/sbin/sshd -D -e -f $${runner-sshd-config:rendered} command-line = ${openssh:location}/sbin/sshd -D -e -f $${runner-sshd-config:rendered}
wrapper-path = $${directory:bin}/runner_raw_sshd
[runner-sshd-authorized-key]
<= runner-sshd-raw-server
recipe = slapos.cookbook:dropbear.add_authorized_key
key = $${slap-parameter:user-authorized-key}
[runner-sshd-server]
recipe = collective.recipe.template
log = $${directory:log}/runner-sshd.log
input = inline:#!/bin/sh
exec $${runner-sshd-raw-server:wrapper-path} >> $${:log} 2>&1
output = $${directory:bin}/runner_raw_sshd_log
mode = 700
[runner-sshd-graceful]
recipe = slapos.cookbook:wrapper
command-line = $${directory:bin}/killpidfromfile $${runner-sshd-config:path_pid} SIGHUP
wrapper-path = $${directory:scripts}/runner-sshd-graceful
[runner-sshkeys-directory]
recipe = slapos.cookbook:mkdirectory
requests = $${directory:sshkeys}/runner-requests/
keys = $${directory:sshkeys}/runner-keys/
[runner-sshkeys-authority]
recipe = slapos.cookbook:sshkeys_authority
request-directory = $${runner-sshkeys-directory:requests}
keys-directory = $${runner-sshkeys-directory:keys}
wrapper = $${directory:bin}/runner_sshkeys_authority
keygen-binary = ${openssh:location}/bin/ssh-keygen
[runner-sshkeys-authority-service]
recipe = slapos.cookbook:wrapper
command-line = $${runner-sshkeys-authority:wrapper}
wrapper-path = $${directory:services}/runner-sshkeys-authority
hash-existing-files = $${buildout:directory}/software_release/buildout.cfg hash-existing-files = $${buildout:directory}/software_release/buildout.cfg
[runner-sshkeys-sshd]
<= runner-sshkeys-authority
recipe = slapos.cookbook:sshkeys_authority.request
name = sshd
type = rsa
executable = $${runner-sshd-server:output}
public-key = $${runner-sshd-raw-server:rsa-keyfile}.pub
private-key = $${runner-sshd-raw-server:rsa-keyfile}
wrapper = $${directory:bin}/runner-sshd
[runner-sshkeys-sshd-service]
recipe = slapos.cookbook:wrapper
command-line = $${runner-sshkeys-sshd:wrapper}
wrapper-path = $${directory:services}/runner-sshd wrapper-path = $${directory:services}/runner-sshd
hash-existing-files = $${buildout:directory}/software_release/buildout.cfg
[runner-sshd-add-authorized-key] [runner-sshd-add-authorized-key]
recipe = slapos.cookbook:dropbear.add_authorized_key recipe = slapos.cookbook:dropbear.add_authorized_key
home = $${buildout:directory} home = $${buildout:directory}
key = $${slap-parameter:user-authorized-key} key = $${slap-parameter:user-authorized-key}
[runner-sshkeys-publickey-fingerprint-cmd] [runner-sshd-ssh-keygen-base]
recipe = plone.recipe.command recipe = plone.recipe.command
command = bash -o pipefail -c "$${runner-sshkeys-authority:keygen-binary} -lf $${runner-sshkeys-sshd:public-key} | cut -f 2 -d\ | sed 's/+/%2B/g' | sed 's/\//%2F/g' | sed 's/SHA256://'" output = $${directory:etc}/$${:_buildout_section_name_}
command = ${openssh-output:keygen} -f $${:output} -N '' $${:extra-args}
[runner-sshkeys-publickey-fingerprint-shelloutput] [runner-sshd-ssh-host-rsa-key]
<=runner-sshd-ssh-keygen-base
extra-args=-t rsa
[runner-sshd-ssh-host-ecdsa-key]
<=runner-sshd-ssh-keygen-base
extra-args=-t ecdsa -b 521
[runner-sshd-publickey-fingerprint-shelloutput]
recipe = collective.recipe.shelloutput recipe = collective.recipe.shelloutput
# XXX because collective.recipe.shelloutput ignore errors, we run the same # XXX because collective.recipe.shelloutput ignore errors, we run the same
# command in a plone.recipe.command so that if fails if something goes wrong. # command in a plone.recipe.command so that if fails if something goes wrong.
commands = commands =
fingerprint = $${runner-sshkeys-publickey-fingerprint-cmd:command} fingerprint = bash -o pipefail -c "${openssh-output:keygen} -lf $${runner-sshd-ssh-host-ecdsa-key:output} | cut -f 2 -d\ | sed 's/+/%2B/g' | sed 's/\//%2F/g' | sed 's/SHA256://'"
[runner-sshkeys-publickey-fingerprint] [runner-sshd-publickey-fingerprint]
# fingerprint for ssh url, see # fingerprint for ssh url, see
# https://tools.ietf.org/id/draft-salowey-secsh-uri-00.html#connparam # https://tools.ietf.org/id/draft-salowey-secsh-uri-00.html#connparam
# https://winscp.net/eng/docs/session_url#hostkey # https://winscp.net/eng/docs/session_url#hostkey
_fingerprint = $${runner-sshd-publickey-fingerprint-shelloutput:fingerprint}
# format is host-key-alg-fingerprint, but we know that # format is host-key-alg-fingerprint, but we know that
# $${runner-sshkeys-sshd:public-key} is rsa so for host-key-alg # $${runner-sshkeys-sshd:public-key} is rsa so for host-key-alg
# we just use use rsa. # we just use use rsa.
fingerprint = ssh-rsa-$${runner-sshkeys-publickey-fingerprint-shelloutput:fingerprint} fingerprint = ssh-rsa-$${:_fingerprint}
# XXX because collective.recipe.shelloutput ignore errors and capture output
# "Error ...", we use a plone.recipe.command to check that this command did
# not fail.
# This command will always fail on first buildout run, because
# collective.recipe.shelloutput is evaluated at buildout recipes __init__ step,
# but the key file is created later at install step.
recipe = plone.recipe.command
stop-on-error = true
command = echo "$${:_fingerprint}" | ( grep ^Error || exit 0 && exit 1 )
#--------------------------- #---------------------------
#-- #--
...@@ -640,7 +600,7 @@ backend-url = $${slaprunner:access-url} ...@@ -640,7 +600,7 @@ backend-url = $${slaprunner:access-url}
init-user = $${runner-htpasswd:user} init-user = $${runner-htpasswd:user}
init-password = $${runner-htpasswd:password} init-password = $${runner-htpasswd:password}
ssh-command = ssh $${user-info:pw-name}@$${slap-network-information:global-ipv6} -p $${runner-sshd-port:port} ssh-command = ssh $${user-info:pw-name}@$${slap-network-information:global-ipv6} -p $${runner-sshd-port:port}
ssh-url = ssh://$${user-info:pw-name};fingerprint=$${runner-sshkeys-publickey-fingerprint:fingerprint}@[$${slap-network-information:global-ipv6}]:$${runner-sshd-port:port} ssh-url = ssh://$${user-info:pw-name};fingerprint=$${runner-sshd-publickey-fingerprint:fingerprint}@[$${slap-network-information:global-ipv6}]:$${runner-sshd-port:port}
git-public-url = https://[$${httpd-parameters:global_ip}]:$${httpd-parameters:global_port}/git-public/ git-public-url = https://[$${httpd-parameters:global_ip}]:$${httpd-parameters:global_port}/git-public/
git-private-url = https://[$${httpd-parameters:global_ip}]:$${httpd-parameters:global_port}/git/ git-private-url = https://[$${httpd-parameters:global_ip}]:$${httpd-parameters:global_port}/git/
monitor-base-url = $${monitor-publish-parameters:monitor-base-url} monitor-base-url = $${monitor-publish-parameters:monitor-base-url}
......
...@@ -58,10 +58,8 @@ host = {{ slaprunner['ipv4'] }} ...@@ -58,10 +58,8 @@ host = {{ slaprunner['ipv4'] }}
port = {{ slaprunner['proxy_port'] }} port = {{ slaprunner['proxy_port'] }}
database_uri = {{ slaprunner['proxy_database'] }} database_uri = {{ slaprunner['proxy_database'] }}
[sshkeys_authority]
ssh_client = {{ slaprunner['ssh_client'] }}
public_key = {{ slaprunner['public_key'] }}
private_key = {{ slaprunner['private_key'] }}
[gitclient] [gitclient]
git = {{ slaprunner['git-binary'] }} git = {{ slaprunner['git-binary'] }}
[sshkeys_authority]
public_key = {{ slaprunner['public_key'] }}
...@@ -246,7 +246,6 @@ class ServicesTestCase(SlaprunnerTestCase): ...@@ -246,7 +246,6 @@ class ServicesTestCase(SlaprunnerTestCase):
] ]
expected_process_names = [ expected_process_names = [
'slaprunner-supervisord-{hash}-on-watch', 'slaprunner-supervisord-{hash}-on-watch',
'runner-sshkeys-authority-{hash}-on-watch',
'runner-sshd-{hash}-on-watch', 'runner-sshd-{hash}-on-watch',
'slaprunner-httpd-{hash}-on-watch', 'slaprunner-httpd-{hash}-on-watch',
'gunicorn-{hash}-on-watch', 'gunicorn-{hash}-on-watch',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment