From 8db5de5a810fa780fce739ba402a8eb95b8a070e Mon Sep 17 00:00:00 2001 From: mprahl Date: Tue, 12 Sep 2017 10:10:37 -0400 Subject: [PATCH 1/3] Add .vscode to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c6cd4e2b..d60b5391 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,4 @@ server.crt server.key test_module_build_service.db tests/vcr-request-data +.vscode From 79a79fdcf2b75a295916517851925f78b638cb42 Mon Sep 17 00:00:00 2001 From: mprahl Date: Wed, 13 Sep 2017 10:04:03 -0400 Subject: [PATCH 2/3] Support calling the tests by using pytest instead of just py.test --- module_build_service/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module_build_service/config.py b/module_build_service/config.py index 38c2b6cd..24d10e58 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -72,7 +72,7 @@ def init_config(app): if 'MBS_CONFIG_SECTION' in app.request.environ: config_section = app.request.environ['MBS_CONFIG_SECTION'] # TestConfiguration shall only be used for running tests, otherwise... - if any(['nosetests' in arg or 'noserunner.py' in arg or 'py.test' in arg or 'pytest.py' in arg for arg in sys.argv]): + if any(['nosetests' in arg or 'noserunner.py' in arg or 'py.test' in arg or 'pytest' in arg for arg in sys.argv]): config_section = 'TestConfiguration' from conf import config config_module = config From 0f6d7a55c5dc79398b7c3ccb74cb34e73739ef04 Mon Sep 17 00:00:00 2001 From: mprahl Date: Wed, 13 Sep 2017 13:34:25 -0400 Subject: [PATCH 3/3] Add Kerberos + LDAP authentication support --- README.rst | 24 ++++ conf/config.py | 1 + module_build_service/auth.py | 202 +++++++++++++++++++++++++++++-- module_build_service/config.py | 47 ++++++++ requirements.txt | 2 + tests/test_auth.py | 211 +++++++++++++++++++++++++++++++-- 6 files changed, 466 insertions(+), 21 deletions(-) diff --git a/README.rst b/README.rst index 16acbf62..f7cf55be 100644 --- a/README.rst +++ b/README.rst @@ -641,6 +641,30 @@ the following rules (all of them are evaluated from top to bottom): value, DevConfiguration is forced and ``config.py`` is used directly from the MBS's develop instance. For more information see ``docs/CONTRIBUTING.rst``. + +Setting Up Kerberos + LDAP Authentication +========================================= + +MBS defaults to using OIDC as its authentication mechanism. It additionally +supports Kerberos + LDAP, where Kerberos proves the user's identity and LDAP +is used to determine the user's group membership. To configure this, the following +must be set in ``/etc/module-build-service/config.py``: + +- ``AUTH_METHOD`` must be set to ``'kerberos'``. +- ``KERBEROS_HTTP_HOST`` can override the hostname MBS will present itself as when + performing Kerberos authentication. If this is not set, Python will try to guess the + hostname of the server. +- ``KERBEROS_KEYTAB`` is the path to the keytab used by MBS. If this is not set, + the environment variable ``KRB5_KTNAME`` will be used. +- ``LDAP_URI`` is the URI to connect to LDAP (e.g. ``'ldaps://ldap.domain.local:636'`` + or ``'ldap://ldap.domain.local'``). +- ``LDAP_GROUPS_DN`` is the distinguished name of the container or organizational unit where groups + are located (e.g. ``'ou=groups,dc=domain,dc=local'``). MBS does not search the tree below the + distinguished name specified here for security reasons because it ensures common names are + unique. +- ``ALLOWED_GROUPS`` and ``ADMIN_GROUPS`` both need to declare the common name of the LDAP groups, + not the distinguished name. + Development =========== diff --git a/conf/config.py b/conf/config.py index fbb97078..7274db30 100644 --- a/conf/config.py +++ b/conf/config.py @@ -164,6 +164,7 @@ class TestConfiguration(BaseConfiguration): KOJI_REPOSITORY_URL = 'https://kojipkgs.stg.fedoraproject.org/repos' SCMURLS = ["git://pkgs.stg.fedoraproject.org/modules/"] + AUTH_METHOD = 'oidc' class ProdConfiguration(BaseConfiguration): diff --git a/module_build_service/auth.py b/module_build_service/auth.py index 58b87f20..1b7c0244 100644 --- a/module_build_service/auth.py +++ b/module_build_service/auth.py @@ -20,14 +20,33 @@ # SOFTWARE. # # Written by Jan Kaluza +# Written by Matt Prahl """Auth system based on the client certificate and FAS account""" - -from module_build_service.errors import Unauthorized, Forbidden -from module_build_service import app, log +import json +import os +from socket import gethostname +import ssl import requests -import json +import kerberos +from flask import Response +# Starting with Flask 0.9, the _app_ctx_stack is the correct one, +# before that we need to use the _request_ctx_stack. +try: + from flask import _app_ctx_stack as stack +except ImportError: + from flask import _request_ctx_stack as stack +from werkzeug.exceptions import Unauthorized as FlaskUnauthorized +import ldap3 +from dogpile.cache import make_region + +from module_build_service.errors import Unauthorized, Forbidden +from module_build_service import app, log, conf + + +client_secrets = None +region = make_region().configure('dogpile.cache.memory') def _json_loads(content): @@ -35,8 +54,6 @@ def _json_loads(content): content = content.decode('utf-8') return json.loads(content) -client_secrets = None - def _load_secrets(): global client_secrets @@ -80,15 +97,10 @@ def _get_user_info(token): return resp.json() -def get_user(request): +def get_user_oidc(request): """ Returns the client's username and groups based on the OIDC token provided. """ - - if app.config['NO_AUTH'] is True: - log.debug("Authorization is disabled.") - return "anonymous", {"packager"} - _load_secrets() if "authorization" not in request.headers: @@ -139,3 +151,169 @@ def get_user(request): raise Exception(error) return data["username"], groups + + +# Insired by https://pagure.io/waiverdb/blob/master/f/waiverdb/auth.py which was +# inspired by https://github.com/mkomitee/flask-kerberos/blob/master/flask_kerberos.py +class KerberosAuthenticate(object): + def __init__(self): + if conf.kerberos_http_host: + hostname = conf.kerberos_http_host + else: + hostname = gethostname() + self.service_name = "HTTP@{0}".format(hostname) + + # If the config specifies a keytab to use, then override the KRB5_KTNAME + # environment variable + if conf.kerberos_keytab: + os.environ['KRB5_KTNAME'] = conf.kerberos_keytab + + if 'KRB5_KTNAME' in os.environ: + try: + principal = kerberos.getServerPrincipalDetails('HTTP', hostname) + except kerberos.KrbError as error: + raise Unauthorized( + 'Kerberos: authentication failed with "{0}"'.format(str(error))) + + log.debug('Kerberos: server is identifying as "{0}"'.format(principal)) + else: + raise Unauthorized('Kerberos: set the config value of "KERBEROS_KEYTAB" or the ' + 'environment variable "KRB5_KTNAME" to your keytab file') + + def _gssapi_authenticate(self, token): + """ + Performs GSSAPI Negotiate Authentication + On success also stashes the server response token for mutual authentication + at the top of request context with the name kerberos_token, along with the + authenticated user principal with the name kerberos_user. + """ + state = None + ctx = stack.top + try: + rc, state = kerberos.authGSSServerInit(self.service_name) + if rc != kerberos.AUTH_GSS_COMPLETE: + log.error('Kerberos: unable to initialize server context') + return None + + rc = kerberos.authGSSServerStep(state, token) + if rc == kerberos.AUTH_GSS_COMPLETE: + log.debug('Kerberos: completed GSSAPI negotiation') + ctx.kerberos_token = kerberos.authGSSServerResponse(state) + ctx.kerberos_user = kerberos.authGSSServerUserName(state) + return rc + elif rc == kerberos.AUTH_GSS_CONTINUE: + log.debug('Kerberos: continuing GSSAPI negotiation') + return kerberos.AUTH_GSS_CONTINUE + else: + log.debug('Kerberos: unable to step server context') + return None + except kerberos.GSSError as error: + log.error('Kerberos: unable to authenticate: {0}'.format(str(error))) + return None + finally: + if state: + kerberos.authGSSServerClean(state) + + def process_request(self, token): + """ + Authenticates the current request using Kerberos. + """ + kerberos_user = None + kerberos_token = None + ctx = stack.top + rc = self._gssapi_authenticate(token) + if rc == kerberos.AUTH_GSS_COMPLETE: + kerberos_user = ctx.kerberos_user + kerberos_token = ctx.kerberos_token + elif rc != kerberos.AUTH_GSS_CONTINUE: + raise Forbidden('Invalid Kerberos ticket') + + return kerberos_user, kerberos_token + + +def get_user_kerberos(request): + user = None + if 'Authorization' not in request.headers: + response = Response('Unauthorized', 401, {'WWW-Authenticate': 'Negotiate'}) + raise FlaskUnauthorized(response=response) + header = request.headers.get('Authorization') + token = ''.join(header.strip().split()[1:]) + user, kerberos_token = KerberosAuthenticate().process_request(token) + # Remove the realm + user = user.split('@')[0] + groups = get_ldap_group_membership(user) + return user, set(groups) + + +@region.cache_on_arguments() +def get_ldap_group_membership(uid): + """ Small wrapper on getting the group membership so that we can use caching + :param uid: a string of the uid of the user + :return: a list of groups the user is a member of + """ + ldap_con = Ldap() + return ldap_con.get_user_membership(uid) + + +class Ldap(object): + """ A class that handles LDAP connections and queries + """ + connection = None + base_dn = None + + def __init__(self): + if not conf.ldap_uri: + raise Forbidden('LDAP_URI must be set in server config.') + if conf.ldap_groups_dn: + self.base_dn = conf.ldap_groups_dn + else: + raise Forbidden('LDAP_GROUPS_DN must be set in server config.') + + if conf.ldap_uri.startswith('ldaps://'): + tls = ldap3.Tls(ca_certs_file='/etc/pki/tls/certs/ca-bundle.crt', + validate=ssl.CERT_REQUIRED) + server = ldap3.Server(conf.ldap_uri, use_ssl=True, tls=tls) + else: + server = ldap3.Server(conf.ldap_uri) + self.connection = ldap3.Connection(server) + try: + self.connection.open() + except ldap3.core.exceptions.LDAPSocketOpenError as error: + log.error('The connection to "{0}" failed. The following error was raised: {1}' + .format(conf.ldap_uri, str(error))) + raise Forbidden('The connection to the LDAP server failed. Group membership ' + 'couldn\'t be obtained.') + + def get_user_membership(self, uid): + """ Gets the group membership of a user + :param uid: a string of the uid of the user + :return: a list of common names of the posixGroups the user is a member of + """ + ldap_filter = '(memberUid={0})'.format(uid) + # Only get the groups in the base container/OU + self.connection.search(self.base_dn, ldap_filter, search_scope=ldap3.LEVEL, + attributes=['cn']) + groups = self.connection.response + try: + return [group['attributes']['cn'][0] for group in groups] + except KeyError: + log.exception('The LDAP groups could not be determined based on the search results ' + 'of "{0}"'.format(str(groups))) + return [] + + +def get_user(request): + """ Authenticates the user and returns the username and group name + :param request: a Flask request + :return: a tuple with a string representing the user name and a set with the user's group + membership such as ('mprahl', {'factory2', 'devel'}) + """ + if conf.no_auth is True: + log.debug('Authorization is disabled.') + return 'anonymous', {'packager'} + + get_user_func_name = 'get_user_{0}'.format(conf.auth_method) + get_user_func = globals().get(get_user_func_name) + if not get_user_func: + raise RuntimeError('The function "{0}" is not implemented'.format(get_user_func_name)) + return get_user_func(request) diff --git a/module_build_service/config.py b/module_build_service/config.py index 24d10e58..ef833f97 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -26,6 +26,7 @@ import imp import os +import re from os import sys @@ -358,6 +359,29 @@ class Config(object): 'type': list, 'default': ['/etc/module-build-service/yum.conf', 'conf/yum.conf'], 'desc': 'List of yum config file paths in order of preference.'}, + 'auth_method': { + 'type': str, + 'default': 'oidc', + 'desc': 'Authentiation method to MBS. Options are oidc or kerberos'}, + 'kerberos_http_host': { + 'type': str, + 'default': '', + 'desc': ('Hardcodes the HTTP host MBS identifies as in Kerberos. If this isn\'t set, ' + 'it will be derived dynamically.')}, + 'kerberos_keytab': { + 'type': str, + 'default': '', + 'desc': ('Overrides the use of the environment variable KRB5_KTNAME, which specifies ' + 'the location to the Kerberos keytab for authentication.')}, + 'ldap_uri': { + 'type': str, + 'default': '', + 'desc': 'LDAP URI to query for group information when using Kerberos authentication'}, + 'ldap_groups_dn': { + 'type': str, + 'default': '', + 'desc': ('The distinguished name of the container or organizational unit containing ' + 'the groups in LDAP')} } def __init__(self, conf_section_obj): @@ -488,3 +512,26 @@ class Config(object): if i < 0: raise ValueError('NUM_CONCURRENT_BUILDS must be >= 0') self._num_concurrent_builds = i + + def _setifok_auth_method(self, s): + s = str(s) + if s.lower() not in ('oidc', 'kerberos'): + raise ValueError('Unsupported authentication method') + self._auth_method = s.lower() + + def _setifok_kerberos_keytab(self, s): + keytab = str(s) + if keytab: + keytab = os.path.expanduser(keytab) + if not os.path.exists(keytab): + raise ValueError('The path set for KERBEROS_KEYTAB does not exist') + + self._kerberos_keytab = keytab + + def _setifok_ldap_uri(self, s): + ldap_uri = str(s) + + if ldap_uri and not re.match(r'^(?:ldap(?:s)?:\/\/.+)$', ldap_uri): + raise ValueError('LDAP_URI is invalid. It must start with "ldap://" or "ldaps://"') + + self._ldap_uri = ldap_uri diff --git a/requirements.txt b/requirements.txt index 85f698f1..1d8a34d9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,9 @@ fedmsg funcsigs # Python2 only futures # Python 2 only httplib2 +kerberos kobo>=0.5.0 +ldap3 m2crypto m2ext mock diff --git a/tests/test_auth.py b/tests/test_auth.py index 6eb38be0..d34ec3d2 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -19,16 +19,21 @@ # SOFTWARE. # # Written by Ralph Bean +# Written by Matt Prahl +from os import path, environ from nose.tools import eq_ - import unittest import mock -from mock import patch +from mock import patch, PropertyMock, Mock +import kerberos +import ldap3 +from flask import Response +from werkzeug.exceptions import Unauthorized as FlaskUnauthorized import module_build_service.auth import module_build_service.errors -from os import path +import module_build_service.config as mbs_config class TestAuthModule(unittest.TestCase): @@ -100,12 +105,12 @@ class TestAuthModule(unittest.TestCase): eq_(username, name) eq_(groups, set(get_user_info.return_value["groups"])) - def test_disable_authentication(self): - with patch.dict('module_build_service.app.config', {'NO_AUTH': True}, clear=True): - request = mock.MagicMock() - username, groups = module_build_service.auth.get_user(request) - eq_(username, "anonymous") - eq_(groups, {"packager"}) + @patch.object(mbs_config.Config, 'no_auth', new_callable=PropertyMock, return_value=True) + def test_disable_authentication(self, conf_no_auth): + request = mock.MagicMock() + username, groups = module_build_service.auth.get_user(request) + eq_(username, "anonymous") + eq_(groups, {"packager"}) @patch('module_build_service.auth.client_secrets', None) def test_misconfiguring_oidc_client_secrets_should_be_failed(self): @@ -173,3 +178,191 @@ class TestAuthModule(unittest.TestCase): self.assertEquals(str(cm.exception), "OIDC_REQUIRED_SCOPE must be set in server config.") + + +class KerberosMockConfig(object): + def __init__(self, uri='ldaps://test.example.local:636', dn='ou=groups,dc=domain,dc=local', + kt='/path/to/keytab', host='mbs.domain.local'): + """ + :param uri: a string overriding config.ldap_uri + :param dn: a string overriding config.ldap_groups_dn + :param kt: a string overriding config.kerberos_keytab + :param host: a string overriding config.kerberos_http_host + """ + self.uri = uri + self.dn = dn + self.kt = kt + self.host = host + + def __enter__(self): + self.auth_method_p = patch.object( + mbs_config.Config, 'auth_method', new_callable=PropertyMock) + mocked_auth_method = self.auth_method_p.start() + mocked_auth_method.return_value = 'kerberos' + + self.ldap_uri_p = patch.object( + mbs_config.Config, 'ldap_uri', new_callable=PropertyMock) + mocked_ldap_uri = self.ldap_uri_p.start() + mocked_ldap_uri.return_value = self.uri + + self.ldap_dn_p = patch.object( + mbs_config.Config, 'ldap_groups_dn', new_callable=PropertyMock) + mocked_ldap_dn = self.ldap_dn_p.start() + mocked_ldap_dn.return_value = self.dn + + self.kerberos_keytab_p = patch.object( + mbs_config.Config, 'kerberos_keytab', new_callable=PropertyMock) + mocked_kerberos_keytab = self.kerberos_keytab_p.start() + mocked_kerberos_keytab.return_value = self.kt + + self.kerberos_http_host_p = patch.object( + mbs_config.Config, 'kerberos_http_host', new_callable=PropertyMock) + mocked_kerberos_http_host = self.kerberos_http_host_p.start() + mocked_kerberos_http_host.return_value = self.host + + def __exit__(self, *args): + self.auth_method_p.stop() + self.ldap_uri_p.stop() + self.ldap_dn_p.stop() + self.kerberos_keytab_p.stop() + self.kerberos_http_host_p.stop() + + +class TestAuthModuleKerberos(unittest.TestCase): + @patch('kerberos.authGSSServerInit', return_value=(kerberos.AUTH_GSS_COMPLETE, object())) + @patch('kerberos.authGSSServerStep', return_value=kerberos.AUTH_GSS_COMPLETE) + @patch('kerberos.authGSSServerResponse', return_value='STOKEN') + @patch('kerberos.authGSSServerUserName', return_value='mprahl@EXAMPLE.ORG') + @patch('kerberos.authGSSServerClean') + @patch('kerberos.getServerPrincipalDetails') + @patch.dict('os.environ') + @patch('module_build_service.auth.stack') + def test_get_user_kerberos(self, stack, principal, clean, name, response, + step, init): + """ + Test that authentication works with Kerberos and LDAP + """ + mock_top = Mock() + stack.return_value = mock_top + + headers = {'Authorization': 'foobar'} + request = mock.MagicMock() + request.headers.return_value = mock.MagicMock(spec_set=dict) + request.headers.__getitem__.side_effect = headers.__getitem__ + request.headers.__setitem__.side_effect = headers.__setitem__ + request.headers.__contains__.side_effect = headers.__contains__ + + # Create the mock LDAP instance + server = ldap3.Server('ldaps://test.domain.local') + connection = ldap3.Connection(server, client_strategy=ldap3.MOCK_SYNC) + base_dn = 'dc=domain,dc=local' + factory_group_attrs = { + 'objectClass': ['top', 'posixGroup'], + 'memberUid': ['mprahl', 'tbrady'], + 'gidNumber': 1234, + 'cn': ['factory2-devs'] + } + devs_group_attrs = { + 'objectClass': ['top', 'posixGroup'], + 'memberUid': ['mprahl', 'mikeb'], + 'gidNumber': 1235, + 'cn': ['devs'] + } + athletes_group_attrs = { + 'objectClass': ['top', 'posixGroup'], + 'memberUid': ['tbrady', 'rgronkowski'], + 'gidNumber': 1236, + 'cn': ['athletes'] + } + mprahl_attrs = { + 'memberOf': ['cn=Employee,ou=groups,{0}'.format(base_dn)], + 'uid': ['mprahl'], + 'cn': ['mprahl'], + 'objectClass': ['top', 'person'] + } + connection.strategy.add_entry('cn=factory2-devs,ou=groups,{0}'.format(base_dn), + factory_group_attrs) + connection.strategy.add_entry('cn=athletes,ou=groups,{0}'.format(base_dn), + athletes_group_attrs) + connection.strategy.add_entry('cn=devs,ou=groups,{0}'.format(base_dn), devs_group_attrs) + connection.strategy.add_entry('cn=mprahl,ou=users,{0}'.format(base_dn), mprahl_attrs) + + groups = {'devs', 'factory2-devs'} + with patch('ldap3.Connection') as mock_ldap_con, KerberosMockConfig(): + mock_ldap_con.return_value = connection + assert module_build_service.auth.get_user_kerberos(request) == ('mprahl', groups) + + def test_auth_header_not_set(self): + """ + Test that an Unauthorized exception is returned when there is no authorization header + set. + """ + headers = {} + request = mock.MagicMock() + request.headers.return_value = mock.MagicMock(spec_set=dict) + request.headers.__getitem__.side_effect = headers.__getitem__ + request.headers.__setitem__.side_effect = headers.__setitem__ + request.headers.__contains__.side_effect = headers.__contains__ + + with KerberosMockConfig(): + try: + module_build_service.auth.get_user_kerberos(request) + assert False, 'Unauthorized error not raised' + except FlaskUnauthorized as error: + assert error.response.www_authenticate.to_header().strip() == 'Negotiate' + assert error.response.status == '401 UNAUTHORIZED' + + @patch.dict(environ) + def test_keytab_not_set(self): + """ + Test that authentication fails when the keytab is not set + """ + if 'KRB5_KTNAME' in environ: + del environ['KRB5_KTNAME'] + + headers = {'Authorization': 'foobar'} + request = mock.MagicMock() + request.headers.return_value = mock.MagicMock(spec_set=dict) + request.headers.__getitem__.side_effect = headers.__getitem__ + request.headers.__setitem__.side_effect = headers.__setitem__ + request.headers.__contains__.side_effect = headers.__contains__ + + with KerberosMockConfig(kt=''): + try: + module_build_service.auth.get_user_kerberos(request) + assert False, 'Unauthorized error not raised' + except module_build_service.errors.Unauthorized as error: + assert str(error) == ('Kerberos: set the config value of "KERBEROS_KEYTAB" ' + 'or the environment variable "KRB5_KTNAME" to your ' + 'keytab file') + + # Set the return value to something not 0 (continue) or 1 (complete) + @patch('kerberos.authGSSServerInit', return_value=(100, object())) + @patch('kerberos.authGSSServerStep', return_value=kerberos.AUTH_GSS_COMPLETE) + @patch('kerberos.authGSSServerResponse', return_value='STOKEN') + @patch('kerberos.authGSSServerUserName', return_value='mprahl@EXAMPLE.ORG') + @patch('kerberos.authGSSServerClean') + @patch('kerberos.getServerPrincipalDetails') + @patch.dict('os.environ') + @patch('module_build_service.auth.stack') + def test_get_user_kerberos_invalid_ticket(self, stack, principal, clean, name, response, + step, init): + """ + Test that authentication fails with an invalid Kerberos ticket + """ + mock_top = Mock() + stack.return_value = mock_top + + headers = {'Authorization': 'foobar'} + request = mock.MagicMock() + request.headers.return_value = mock.MagicMock(spec_set=dict) + request.headers.__getitem__.side_effect = headers.__getitem__ + request.headers.__setitem__.side_effect = headers.__setitem__ + request.headers.__contains__.side_effect = headers.__contains__ + + with KerberosMockConfig(): + try: + module_build_service.auth.get_user_kerberos(request) + assert False, 'Forbidden error not raised' + except module_build_service.errors.Forbidden as error: + assert str(error) == ('Invalid Kerberos ticket')