Vault SSH dynamic mode may damage authorized_keys file in case of concurrent connections

[samm-git]

Environment:
Vault v0.9.6 ('7e1fbde40afee241f81ef08700e7987d86fc7242')

• Vault Version: Linux
• Operating System/Architecture: amd64

Expected Behavior:

vault ssh in dynamic mode is expected to correctly update authorized_keys file.

Actual Behavior:

File is updated correctly if there are no concurrent connections, but became damaged if there is > 1 concurrent ssh connection at time.

Steps to Reproduce:

1. Configure vault ssh backend in dynamic mode using default script
2. Try to establish many session concurrently. This may end with damaged authorized_keys file

Important Factoids:

I been able to identify root cause for this. Script used in the https://github.com/hashicorp/vault/blob/master/builtin/logical/ssh/linux_install_script.go does not provide any kind of locking and will not work correctly in case of concurrent update. To proof that i extracted the script and emulated it in concurrent and non-concurrent environment. Result provided below:

Running in non-concurrent mode

root@tst:/tmp/test# rm -f authorized_file; echo "key 001" > 001 ; echo "key 000" > 000 ; (bash ./script.sh install 000 /tmp/test/authorized_file > /dev/null ; bash ./script.sh install 001 /tmp/test/authorized_file > /dev/null); cat authorized_file
key 000
key 001

Running in concurrent mode

This would provide different results on different runs. I been getting empty file, file with > 1 same line, etc.

# rm -f authorized_file; echo "key 001" > 001 ; echo "key 000" > 000 ; (bash ./script.sh install 000 /tmp/test/authorized_file > /dev/null & bash ./script.sh install 001 /tmp/test/authorized_file > /dev/null); cat authorized_file
key 000

To fix this issue file locking (e.g. using flock) needs to be implemented to avoid concurrent access to the authorized_file.

[jefferai]

You can modify that script however you like. I know in the past locking mechanisms were generally not consistent across distros. If you find something that you think works across all distros, feel free to open a PR with the suggested changes.

Additionally note that the dynamic keys mechanism is not recommended.

[samm-git]

Thank you for reply and sorry for re-opening. I tested flock based locking on my distros and it seems to work fine. Below is a script. Let me know if you think that it does make a sense to make a PR with it

#!/bin/bash
#
# This is a default script which installs or uninstalls an RSA public key to/from
# authorized_keys file in a typical linux machine.
#
# If the platform differs or if the binaries used in this script are not available
# in target machine, use the 'install_script' parameter with 'roles/' endpoint to
# register a custom script (applicable for Dynamic type only).
#
# Vault server runs this script on the target machine with the following params:
#
# $1:INSTALL_OPTION: "install" or "uninstall"
#
# $2:PUBLIC_KEY_FILE: File name containing public key to be installed. Vault server
# uses UUID as name to avoid collisions with public keys generated for other requests.
#
# $3:AUTH_KEYS_FILE: Absolute path of the authorized_keys file.
# Currently, vault uses /home/<username>/.ssh/authorized_keys as the path.
#
# [Note: This script will be run by Vault using the registered admin username.
# Notice that some commands below are run as 'sudo'. For graceful execution of
# this script there should not be any password prompts. So, disable password
# prompt for the admin username registered with Vault.
set -e
# Storing arguments into variables, to increase readability of the script.
INSTALL_OPTION=$1
PUBLIC_KEY_FILE=$2
AUTH_KEYS_FILE=$3
# Delete the public key file and the temporary file
function cleanup
{
	rm -f "$PUBLIC_KEY_FILE" temp_$PUBLIC_KEY_FILE
}
# 'cleanup' will be called if the script ends or if any command fails.
trap cleanup EXIT
# Return if the option is anything other than 'install' or 'uninstall'.
if [ "$INSTALL_OPTION" != "install" ] && [ "$INSTALL_OPTION" != "uninstall" ]; then
	exit 1
fi

(
	flock --timeout 10 200
	# Create the .ssh directory and authorized_keys file if it does not exist
	SSH_DIR=$(dirname $AUTH_KEYS_FILE)
	sudo mkdir -p "$SSH_DIR"
	sudo touch "$AUTH_KEYS_FILE"
	# Remove the key from authorized_keys file if it is already present.
	# This step is common for both install and uninstall.  Note that grep's
	# return code is ignored, thus if grep fails all keys will be removed
	# rather than none and it fails secure
	sudo grep -vFf "$PUBLIC_KEY_FILE" "$AUTH_KEYS_FILE" > temp_$PUBLIC_KEY_FILE || true
	cat temp_$PUBLIC_KEY_FILE | sudo tee "$AUTH_KEYS_FILE"
	# Append the new public key to authorized_keys file
	if [ "$INSTALL_OPTION" == "install" ]; then
		cat "$PUBLIC_KEY_FILE" | sudo tee --append "$AUTH_KEYS_FILE"
	fi
) 200> ${AUTH_KEYS_FILE}.lock

[samm-git]

@jefferai