From 8e3bb541744b1f7724df39a8332cfa59a5bff99f Mon Sep 17 00:00:00 2001 From: Elia Palme Date: Wed, 28 Feb 2024 19:59:41 +0100 Subject: [PATCH 1/6] provide command and arguments in a list --- src/certificator/certificator.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/certificator/certificator.py b/src/certificator/certificator.py index 970f99a8..72022fc3 100644 --- a/src/certificator/certificator.py +++ b/src/certificator/certificator.py @@ -19,6 +19,7 @@ import re import threading import sys +from shlex import quote # Checks if an environment variable injected to F7T is a valid True value # var <- object @@ -447,20 +448,27 @@ def receive(): return jsonify(description='Invalid command'), 400 force_command = f"-O force-command=\"{force_command} {force_opt}\"" - force_command = force_command.replace('$', '\$') + #force_command = force_command.replace('$', '\$') # create temp dir to store certificate for this request td = tempfile.mkdtemp(prefix = "cert") os.symlink(PUB_USER_KEY_PATH, f"{td}/user-key.pub") # link on temp dir - command = f"ssh-keygen -s {CA_KEY_PATH} -n {username} -V {ssh_expire} -I {CA_KEY_PATH} {force_command} {td}/user-key.pub " + commands = ["ssh-keygen", + "-s {CA_KEY_PATH}", + "-n {username}".format(username=username), + "-V {ssh_expire}".format(ssh_expire=ssh_expire), + "-I {CA_KEY_PATH}".format(CA_KEY_PATH=CA_KEY_PATH), + "{force_command}".format(force_command=quote(force_command)), + "{td}/user-key.pub".format(td=td) + ] except Exception as e: logging.error(e) return jsonify(description=f"Error creating certificate: {e}", error=-1), 400 try: - result = subprocess.check_output([command], shell=True) + result = subprocess.run(commands, shell=False) with open(td + '/user-key-cert.pub', 'r') as cert_file: cert = cert_file.read() From fc9a3f5d30ffc2fbd903ce4ffdec7bdcd9f584e8 Mon Sep 17 00:00:00 2001 From: Elia Palme Date: Wed, 28 Feb 2024 20:00:23 +0100 Subject: [PATCH 2/6] clean up --- src/certificator/certificator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/certificator/certificator.py b/src/certificator/certificator.py index 72022fc3..9f8e6c3e 100644 --- a/src/certificator/certificator.py +++ b/src/certificator/certificator.py @@ -19,7 +19,6 @@ import re import threading import sys -from shlex import quote # Checks if an environment variable injected to F7T is a valid True value # var <- object From a882859d0438dd8c12c87eec1581af006dc2a2d0 Mon Sep 17 00:00:00 2001 From: Elia Palme Date: Thu, 29 Feb 2024 08:55:39 +0100 Subject: [PATCH 3/6] Clean up and adding comments --- src/certificator/certificator.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/certificator/certificator.py b/src/certificator/certificator.py index 9f8e6c3e..4b921abc 100644 --- a/src/certificator/certificator.py +++ b/src/certificator/certificator.py @@ -446,28 +446,33 @@ def receive(): app.logger.error(f"Forbidden char on command or option: {force_command} {force_opt}") return jsonify(description='Invalid command'), 400 - force_command = f"-O force-command=\"{force_command} {force_opt}\"" - #force_command = force_command.replace('$', '\$') # create temp dir to store certificate for this request td = tempfile.mkdtemp(prefix = "cert") os.symlink(PUB_USER_KEY_PATH, f"{td}/user-key.pub") # link on temp dir - commands = ["ssh-keygen", - "-s {CA_KEY_PATH}", - "-n {username}".format(username=username), - "-V {ssh_expire}".format(ssh_expire=ssh_expire), - "-I {CA_KEY_PATH}".format(CA_KEY_PATH=CA_KEY_PATH), - "{force_command}".format(force_command=quote(force_command)), + command = ["ssh-keygen", + "-s", + "{CA_KEY_PATH}".format(CA_KEY_PATH=CA_KEY_PATH), + "-n", + "{username}".format(username=username), + "-V", + "{ssh_expire}".format(ssh_expire=ssh_expire), + "-I", + "{CA_KEY_PATH}".format(CA_KEY_PATH=CA_KEY_PATH), + "-O", + "force-command={force_command} {force_opt}".format(force_command=force_command,force_opt=force_opt), "{td}/user-key.pub".format(td=td) ] - + except Exception as e: logging.error(e) return jsonify(description=f"Error creating certificate: {e}", error=-1), 400 + try: - result = subprocess.run(commands, shell=False) + #To prvent shell hijacking don't run the with shell=True + result = subprocess.run(command, shell=False, check=True) with open(td + '/user-key-cert.pub', 'r') as cert_file: cert = cert_file.read() From 77ab779f2eeb26835ec1cb60cb13a1bc11508514 Mon Sep 17 00:00:00 2001 From: Elia Palme Date: Thu, 29 Feb 2024 09:10:14 +0100 Subject: [PATCH 4/6] typo fix --- src/certificator/certificator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/certificator/certificator.py b/src/certificator/certificator.py index 4b921abc..8d9b01d0 100644 --- a/src/certificator/certificator.py +++ b/src/certificator/certificator.py @@ -471,7 +471,7 @@ def receive(): try: - #To prvent shell hijacking don't run the with shell=True + #To prvent shell hijacking don't run commands with shell=True result = subprocess.run(command, shell=False, check=True) with open(td + '/user-key-cert.pub', 'r') as cert_file: cert = cert_file.read() From decec7def0dbbc13808d408beb35ebde192c5cbd Mon Sep 17 00:00:00 2001 From: Elia Palme Date: Thu, 29 Feb 2024 11:47:31 +0100 Subject: [PATCH 5/6] aesthetic string formatting --- src/certificator/certificator.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/certificator/certificator.py b/src/certificator/certificator.py index 8d9b01d0..9e9a88c4 100644 --- a/src/certificator/certificator.py +++ b/src/certificator/certificator.py @@ -453,16 +453,16 @@ def receive(): command = ["ssh-keygen", "-s", - "{CA_KEY_PATH}".format(CA_KEY_PATH=CA_KEY_PATH), + f"{CA_KEY_PATH}", "-n", - "{username}".format(username=username), + f"{username}", "-V", - "{ssh_expire}".format(ssh_expire=ssh_expire), + f"{ssh_expire}", "-I", - "{CA_KEY_PATH}".format(CA_KEY_PATH=CA_KEY_PATH), + f"{CA_KEY_PATH}", "-O", - "force-command={force_command} {force_opt}".format(force_command=force_command,force_opt=force_opt), - "{td}/user-key.pub".format(td=td) + f"force-command={force_command} {force_opt}", + f"{td}/user-key.pub" ] except Exception as e: From 7834c227486708990f7e2189b96a792104cd404a Mon Sep 17 00:00:00 2001 From: Elia Palme Date: Thu, 29 Feb 2024 13:15:51 +0100 Subject: [PATCH 6/6] updated change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7db3800c..8e755e68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed check when submitted an empty batch file on `POST /compute/jobs/upload` - Fixed error message when `GET /status/systems` encounters error in one filesystem - Fixed SSH connection error catching +- Fixed secured "ssh-keygen" command execution ### Changed