URL: https://github.com/freeipa/freeipa/pull/1355 Author: rcritten Title: #1355: [Backport][ipa-4-6] Use the user-provided CA chain file in connections & check for file existence Action: opened
PR body: """ This PR was opened automatically because PR #1047 was pushed to master and backport to ipa-4-6 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1355/head:pr1355 git checkout pr1355
From 9523a698a2f2c3a3a16ac2c7bf93e26d109a3f31 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Wed, 6 Sep 2017 16:23:03 -0400 Subject: [PATCH 1/3] Use the CA chain file from the RPC context The value can be passed in the create_connection() call but wasn't used outside that call. It already defaults to api.env.tls_ca_cert so the context.ca_certfile should be used instead so the caller can override the cert chain on a per-connection basis. This may be handy in the future when there is IPA-to-IPA trust, or for IPA-to-IPA migration. https://pagure.io/freeipa/issue/7145 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipalib/rpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipalib/rpc.py b/ipalib/rpc.py index c9d47d95ff..8689aadff2 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -561,7 +561,7 @@ def make_connection(self, host): conn = create_https_connection( host, 443, - api.env.tls_ca_cert, + getattr(context, 'ca_certfile', None), tls_version_min=api.env.tls_version_min, tls_version_max=api.env.tls_version_max) From b7ebe45f4cd9bd2ac5a95917d69b6bfe165fc004 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Wed, 6 Sep 2017 21:32:49 -0400 Subject: [PATCH 2/3] Add test to ensure that properties are being set in rpcclient Upon a connection several values should be available within the connextion context. Test that they are being set properly. --- ipatests/test_ipalib/test_rpc.py | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/ipatests/test_ipalib/test_rpc.py b/ipatests/test_ipalib/test_rpc.py index 7fe058092d..d743a1b7c5 100644 --- a/ipatests/test_ipalib/test_rpc.py +++ b/ipatests/test_ipalib/test_rpc.py @@ -28,8 +28,10 @@ # pylint: disable=import-error from six.moves.xmlrpc_client import Binary, Fault, dumps, loads # pylint: enable=import-error +from six.moves import urllib from ipatests.util import raises, assert_equal, PluginTester, DummyClass +from ipatests.util import Fuzzy from ipatests.data import binary_bytes, utf8_bytes, unicode_str from ipalib.frontend import Command from ipalib.request import context, Connection @@ -341,3 +343,52 @@ def test_help_many_params(self): "command 'system.methodHelp' takes at most 1 argument") else: raise AssertionError('did not raise') + + +class test_rpcclient_context(PluginTester): + """ + Test the context in `ipalib.rpc.rpcclient` plugin. + """ + def setup(self): + try: + api.Backend.rpcclient.connect(ca_certfile='foo') + except (errors.NetworkError, IOError): + raise nose.SkipTest('%r: Server not available: %r' % + (__name__, api.env.xmlrpc_uri)) + + def teardown(self): + if api.Backend.rpcclient.isconnected(): + api.Backend.rpcclient.disconnect() + + def test_context_cafile(self): + """ + Test that ca_certfile is set in `ipalib.rpc.rpcclient.connect` + """ + ca_certfile = getattr(context, 'ca_certfile', None) + assert_equal(ca_certfile, 'foo') + + def test_context_principal(self): + """ + Test that principal is set in `ipalib.rpc.rpcclient.connect` + """ + principal = getattr(context, 'principal', None) + assert_equal(principal, 'admin@%s' % api.env.realm) + + def test_context_request_url(self): + """ + Test that request_url is set in `ipalib.rpc.rpcclient.connect` + """ + request_url = getattr(context, 'request_url', None) + assert_equal(request_url, 'https://%s/ipa/session/json' % api.env.host) + + def test_context_session_cookie(self): + """ + Test that session_cookie is set in `ipalib.rpc.rpcclient.connect` + """ + fuzzy_cookie = Fuzzy('^ipa_session=MagBearerToken=[A-Za-z0-9+\/]+=*;$') + + session_cookie = getattr(context, 'session_cookie', None) + # pylint-2 is incorrectly spewing Too many positional arguments + # pylint: disable=E1121 + unquoted = urllib.parse.unquote(session_cookie) + assert(unquoted == fuzzy_cookie) From 9e7f022bd6ed32f3dba03b5c7c17b6314390fc0d Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Wed, 6 Sep 2017 16:24:39 -0400 Subject: [PATCH 3/3] If the cafile is not present or readable then raise an exception This can happen on the API level if a user passes in None as cafile or if the value passed in does not exist or is not readable by the IPA framework user. This will also catch situations where /etc/ipa/ca.crt has incorrect permissions and will provide more useful information than just [Errno 13] Permission denied. https://pagure.io/freeipa/issue/7145 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipalib/util.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ipalib/util.py b/ipalib/util.py index 2aa484efb7..7a5dc19e77 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -313,6 +313,10 @@ def create_https_connection( raise RuntimeError("cafile argument is required to perform server " "certificate verification") + if not os.path.isfile(cafile) or not os.access(cafile, os.R_OK): + raise RuntimeError("cafile \'{file}\' doesn't exist or is unreadable". + format(file=cafile)) + # remove the slice of negating protocol options according to options tls_span = get_proper_tls_version_span(tls_version_min, tls_version_max)
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
