From 7b7c2388510c68277fc0069685fa2cf48f1d5d45 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 24 2018 19:03:15 +0000 Subject: [PATCH 1/3] Add a small ACLChecker script This script can be used as ssh command if gitolite is not used to make sure that the user is able to perform any actions on the repo, including cloning. Signed-off-by: Patrick Uiterwijk --- diff --git a/files/aclchecker.py b/files/aclchecker.py new file mode 100644 index 0000000..db3b0b0 --- /dev/null +++ b/files/aclchecker.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +""" + (c) 2014-2018 - Copyright Red Hat Inc + + Authors: + Patrick Uiterwijk + +""" + +from __future__ import unicode_literals, print_function + +import subprocess +import sys +import os + +# Since this is run by sshd, we don't have a way to set environment +# variables ahead of time +if "PAGURE_CONFIG" not in os.environ and os.path.exists( + "/etc/pagure/pagure.cfg" +): + os.environ["PAGURE_CONFIG"] = "/etc/pagure/pagure.cfg" + +# Here starts the code +import pagure +import pagure.lib +from pagure.utils import is_repo_user +from pagure.config import config as pagure_config + + +# Get the arguments +if len(sys.argv) != 2: + print("Invalid call, too few arguments", file=sys.stderr) + sys.exit(1) +remoteuser = sys.argv[1] + +args = os.environ["SSH_ORIGINAL_COMMAND"].split(" ") +# Expects: +if len(args) != 2: + print("Invalid call, too few inner arguments", file=sys.stderr) + sys.exit(1) + + +cmd = args[0] +path = args[1] +if cmd not in ("git-receive-pack", "git-upload-pack"): + print("Invalid call, invalid operation", file=sys.stderr) + sys.exit(1) + +# Git will encode the file path argument within single quotes +if path[0] != "'" or path[-1] != "'": + print("Invalid call: invalid path", file=sys.stderr) + sys.exit(1) +path = path[1:-1] + +if os.path.isabs(path): + print("Non-full path expected, not %s" % path, file=sys.stderr) + sys.exit(1) + +if not path.endswith(".git"): + path = path + ".git" + +session = pagure.lib.create_session(pagure_config["DB_URL"]) +if not session: + raise Exception("Unable to initialize db session") + +gitdir = os.path.join(pagure_config["GIT_FOLDER"], path) +(repotype, username, namespace, repo) = pagure.lib.git.get_repo_info_from_path( + gitdir, hide_notfound=True +) + +if repo is None: + print("Repo not found", file=sys.stderr) + sys.exit(1) + +project = pagure.lib.get_authorized_project( + session, repo, user=username, namespace=namespace, asuser=remoteuser +) + +if not project: + print("Repo not found", file=sys.stderr) + sys.exit(1) + +if repotype != "main" and not is_repo_user(project, remoteuser): + print("Repo not found", file=sys.stderr) + sys.exit(1) + +# Now go run git +# We verified that cmd is either "git-receive-pack" or "git-send-pack" +# and "gitdir" is a full, absolute, path within GIT_FOLDER that points to +# the canonical location for this git repo. +os.execvp(cmd, [cmd, gitdir]) diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 89a27f4..2a9bd61 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -5435,7 +5435,7 @@ def issues_history_stats(session, project): return output -def get_authorized_project(session, project_name, user=None, namespace=None): +def get_authorized_project(session, project_name, user=None, namespace=None, asuser=None): """ Retrieving the project with user permission constraint :arg session: The SQLAlchemy session to use @@ -5446,6 +5446,8 @@ def get_authorized_project(session, project_name, user=None, namespace=None): :type user: String :arg namespace: Pagure namespace :type namespace: String + :arg asuser: Username to check for access + :type asuser: String :return: The project object if project is public or user has permissions for the project else it returns None :rtype: Project @@ -5453,7 +5455,7 @@ def get_authorized_project(session, project_name, user=None, namespace=None): """ repo = pagure.lib._get_project(session, project_name, user, namespace) - if repo and repo.private and not pagure.utils.is_repo_user(repo): + if repo and repo.private and not pagure.utils.is_repo_user(repo, asuser): return None return repo diff --git a/pagure/lib/git.py b/pagure/lib/git.py index e9ac914..08b3f80 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1261,7 +1261,7 @@ def get_commit_subject(commit, abspath): return subject -def get_repo_info_from_path(gitdir): +def get_repo_info_from_path(gitdir, hide_notfound=False): """ Returns the name, username, namespace and type of a git directory This gets computed based on the *_FOLDER's in the config file, @@ -1269,6 +1269,10 @@ def get_repo_info_from_path(gitdir): Args: gitdir (string): Path of the canonical git repository + hide_notfound (bool): Whether to return a tuple with None's instead of + raising an error if the regenerated repo didn't exist. + Can be used to hide the difference between no project access vs not + existing when looking up private repos. Return: (tuple): Tuple with (repotype, username, namespace, repo) Some of these elements may be None if not applicable. """ @@ -1356,7 +1360,10 @@ def get_repo_info_from_path(gitdir): % (rebuiltpath, gitdir) ) if not os.path.exists(rebuiltpath): - raise ValueError("Splitting gitdir %s failed" % gitdir) + if hide_notfound: + return (None, None, None, None) + else: + raise ValueError("Splitting gitdir %s failed" % gitdir) return (repotype, username, namespace, repo) diff --git a/pagure/utils.py b/pagure/utils.py index 25165ca..0ab261d 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -161,12 +161,11 @@ def is_repo_committer(repo_obj, username=None): def is_repo_user(repo_obj, username=None): """ Return whether the user has some access in the provided repo. """ - if not authenticated(): - return False - if username: user = username else: + if not authenticated(): + return False user = flask.g.fas_user.username if is_admin(): From 6e47ded35c905821667c38460b693650a3f4d87d Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 24 2018 19:08:01 +0000 Subject: [PATCH 2/3] Implement initial keyhelper This can be configured in sshd as the AuthorizedKeysCommand to dynamically get the keys from the Pagure database on the fly. The current version is very naive and just loops over all the users if user lookup isn't used. This should be improved in the future by making the user SSH keys be stored in the schema like DeployKey's, where each key has its own entry in a table and we can look them up by key ID. Signed-off-by: Patrick Uiterwijk --- diff --git a/doc/configuration.rst b/doc/configuration.rst index 6a4c885..b274f1e 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -1536,6 +1536,30 @@ not set to be integrated with repoSpanner. Defaults to: ``{}`` +SSH_KEYS_USERNAME_LOOKUP +~~~~~~~~~~~~~~~~~~~~~~~~ + +This configuration key is used by the keyhelper script to indicate that the +git username should be used and looked up. Use this if the username that is sent +to ssh is specific for a unique Pagure user (i.e. not using a single "git@" user +for all git operations). + + +SSH_KEYS_USERNAME_EXPECT +~~~~~~~~~~~~~~~~~~~~~~~~ + +This configuration key should contain the username that is used for git if a single +SSH user is used for all git ssh traffic (i.e. "git"). + + +SSH_KEYS_OPTIONS +~~~~~~~~~~~~~~~~ + +This configuration key provides the options added to keys as they are returned +to sshd, in the same format as AuthorizedKeysFile +(see "AUTHORIZED_KEYS FILE FORMAT" in sshd(8)). + + Deprecated configuration keys ----------------------------- diff --git a/files/keyhelper.py b/files/keyhelper.py new file mode 100644 index 0000000..c248a61 --- /dev/null +++ b/files/keyhelper.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +""" + (c) 2014-2018 - Copyright Red Hat Inc + + Authors: + Patrick Uiterwijk + +""" + +from __future__ import unicode_literals, print_function + +import sys +import os + +# Since this is run by sshd, we don't have a way to set environment +# variables ahead of time +if "PAGURE_CONFIG" not in os.environ and os.path.exists( + "/etc/pagure/pagure.cfg" +): + os.environ["PAGURE_CONFIG"] = "/etc/pagure/pagure.cfg" + +# Here starts the code +import pagure +import pagure.lib +from pagure.config import config as pagure_config +from pagure.lib.model import User, DeployKey + + +# Get the arguments +# Expect sshd config: +# AuthorizedKeysCommand: "%u" "%h" "%t" "%f" +# +# At this moment, we ignore the homedir and fingerprint, since looking +# up a key by fingerprint would require some model changes (ssh keys would +# need to be stored in a fashion like DeployKeys). +# But to not break installations in the future, we should ask installations +# to set up sshd in a way that it will work if we use them in the future. +if len(sys.argv) < 5: + print("Invalid call, too few arguments", file=sys.stderr) + sys.exit(1) + + +username, userhome, keytype, fingerprint = sys.argv[1:5] +username_lookup = pagure_config["SSH_KEYS_USERNAME_LOOKUP"] +expect_username = pagure_config["SSH_KEYS_USERNAME_EXPECT"] + + +if not username_lookup: + if not expect_username: + print("Pagure keyhelper configured incorrectly", file=sys.stderr) + sys.exit(1) + + if username != expect_username: + # Nothing to look up, this user is not git-related + sys.exit(0) + + +session = pagure.lib.create_session(pagure_config["DB_URL"]) +if not session: + print("Unable to get database access") + sys.exit(1) + + +# First try to figure out if this is a deploykey. +# We can look those up very quickly, since those are already +# indexed by key fingerprint. +query = session.query(DeployKey).filter( + DeployKey.ssh_search_key == fingerprint +) +for dkey in query.all(): + keyenv = { + "username": "deploykey_%s_%s" + % (werkzeug.secure_filename(dkey.project.fullname), dkey.id) + } + print( + "%s %s" + % (pagure_config["SSH_KEYS_OPTIONS"] % keyenv, dkey.public_ssh_key) + ) + sys.exit(0) + + +# Now look if it's a normal user +query = session.query(User) +if username_lookup: + query = query.filter(User.user == username) + +for user in query.all(): + for key in user.public_ssh_key.split("\n"): + # Make slightly more sane + key = key.strip() + # Check if this could even be a valid key + key = key.split(" ") + # Should be at the very least ["", " Date: Sep 24 2018 19:11:43 +0000 Subject: [PATCH 3/3] Add keyhelper and aclchecker to spec Signed-off-by: Patrick Uiterwijk --- diff --git a/files/pagure.spec b/files/pagure.spec index 425f1c4..3b57cb3 100644 --- a/files/pagure.spec +++ b/files/pagure.spec @@ -236,6 +236,10 @@ install -p -m 644 createdb.py $RPM_BUILD_ROOT/%{_datadir}/pagure/pagure_createdb # Install the api_key_expire_mail.py script install -p -m 644 files/api_key_expire_mail.py $RPM_BUILD_ROOT/%{_datadir}/pagure/api_key_expire_mail.py +# Install the keyhelper and aclcheck scripts +install -p -m 644 files/aclchecker.py $RPM_BUILD_ROOT/%{_datadir}/pagure/aclchecker.py +install -p -m 644 files/keyhelper.py $RPM_BUILD_ROOT/%{_datadir}/pagure/keyhelper.py + # Install the alembic configuration file install -p -m 644 files/alembic.ini $RPM_BUILD_ROOT/%{_sysconfdir}/pagure/alembic.ini @@ -301,6 +305,8 @@ sed -e "s|#!/usr/bin/env python|#!%{__python}|" -i \ $RPM_BUILD_ROOT/%{_datadir}/pagure/comment_email_milter.py \ $RPM_BUILD_ROOT/%{_datadir}/pagure/pagure_createdb.py \ $RPM_BUILD_ROOT/%{_datadir}/pagure/api_key_expire_mail.py \ + $RPM_BUILD_ROOT/%{_datadir}/pagure/aclchecker.py \ + $RPM_BUILD_ROOT/%{_datadir}/pagure/keyhelper.py \ $RPM_BUILD_ROOT/%{python_sitelib}/pagure/hooks/files/*.py \ $RPM_BUILD_ROOT/%{python_sitelib}/pagure/hooks/files/post-receive \ $RPM_BUILD_ROOT/%{python_sitelib}/pagure/hooks/files/pre-receive \