URL: https://github.com/freeipa/freeipa/pull/736 Author: felipevolpone Title: #736: Fixing the cert-request command comparing whole email address case-sensitively. Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/736/head:pr736 git checkout pr736
From 97725d8185e1d9431395ffa1aa39ceaf38090c8e Mon Sep 17 00:00:00 2001 From: Felipe Volpone <felipevolp...@gmail.com> Date: Fri, 5 May 2017 11:25:08 -0300 Subject: [PATCH 1/4] Fixing the cert-request comparing whole email address case-sensitively. Now, the cert-request command compares the domain part of the email case-insensitively. https://pagure.io/freeipa/issue/5919 --- ipaserver/plugins/cert.py | 29 ++++++++++++++++++++++++++--- ipatests/test_xmlrpc/test_cert_plugin.py | 23 +++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 1a425de..f43f1f0 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -798,7 +798,9 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): # fail if any email addr from DN does not appear in ldap entry email_addrs = csr_obj.subject.get_attributes_for_oid( cryptography.x509.oid.NameOID.EMAIL_ADDRESS) - if len(set(email_addrs) - set(principal_obj.get('mail', []))) > 0: + csr_emails = [attr.value for attr in email_addrs] + if not _emails_are_valid(csr_emails, + principal_obj.get('mail', [])): raise errors.ValidationError( name='csr', error=_( @@ -884,8 +886,8 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): "match requested principal") % gn.name) elif isinstance(gn, cryptography.x509.general_name.RFC822Name): if principal_type == USER: - if principal_obj and gn.value not in principal_obj.get( - 'mail', []): + if not _emails_are_valid([gn.value], + principal_obj.get('mail', [])): raise errors.ValidationError( name='csr', error=_( @@ -953,6 +955,27 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): ) +def _emails_are_valid(csr_emails, principal_emails): + """ + Checks if any email address from certificate does not + appear in ldap entry, comparing the domain part case-insensitively. + """ + + if not any(principal_emails): + return False + + def lower_domain(email): + email_splited = email.split('@', 1) + email_splited[1] = email_splited[1].lower() + + return '@'.join(email_splited) + + principal_emails_lower = set(map(lower_domain, principal_emails)) + csr_emails_lower = set(map(lower_domain, csr_emails)) + + return csr_emails_lower.issubset(principal_emails_lower) + + def principal_to_principal_type(principal): if principal.is_user: return USER diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index 51c20b6..3bdb60e 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -251,6 +251,29 @@ def test_00010_cleanup(self): res = api.Command['service_find'](self.service_princ) assert res['count'] == 0 + def test_00011_email_are_valid(self): + """ + Verify the different scenarios when checking if any email addr + from DN or SAN extension does not appear in ldap entry. + """ + + from ipaserver.plugins.cert import _emails_are_valid + email_addrs = [u'a...@email.com'] + result = _emails_are_valid(email_addrs, [u'a...@email.com']) + assert True == result, result + + email_addrs = [u'a...@email.com'] + result = _emails_are_valid(email_addrs, [u'a...@email.com', + u'anot...@email.com']) + assert True == result, result + + result = _emails_are_valid([], [u'a...@email.com']) + assert False == result, result + + email_addrs = [u'invalidEmailAddress'] + result = _emails_are_valid(email_addrs, []) + assert False == result, result + @pytest.mark.tier1 class test_cert_find(XMLRPC_test): From ecc44fa5c4e317da96abf48fc440e1a9ad0c482d Mon Sep 17 00:00:00 2001 From: Felipe Volpone <felipevolp...@gmail.com> Date: Fri, 5 May 2017 12:17:15 -0300 Subject: [PATCH 2/4] Fixing tests --- ipaserver/plugins/cert.py | 2 +- ipatests/test_xmlrpc/test_cert_plugin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index f43f1f0..9d1fe49 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -961,7 +961,7 @@ def _emails_are_valid(csr_emails, principal_emails): appear in ldap entry, comparing the domain part case-insensitively. """ - if not any(principal_emails): + if not any(principal_emails) or not any(csr_emails): return False def lower_domain(email): diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index 3bdb60e..7c8a4d2 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -251,7 +251,7 @@ def test_00010_cleanup(self): res = api.Command['service_find'](self.service_princ) assert res['count'] == 0 - def test_00011_email_are_valid(self): + def test_00011_emails_are_valid(self): """ Verify the different scenarios when checking if any email addr from DN or SAN extension does not appear in ldap entry. From ec839515efe6d3489b79252de8cc0b96798edd04 Mon Sep 17 00:00:00 2001 From: Felipe Volpone <felipevolp...@gmail.com> Date: Wed, 10 May 2017 10:14:15 -0300 Subject: [PATCH 3/4] Fixing validations if csr_emails or principals_emails are not provided. --- ipaserver/plugins/cert.py | 8 +++----- ipatests/test_xmlrpc/test_cert_plugin.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 9d1fe49..8251701 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -961,14 +961,12 @@ def _emails_are_valid(csr_emails, principal_emails): appear in ldap entry, comparing the domain part case-insensitively. """ - if not any(principal_emails) or not any(csr_emails): - return False - def lower_domain(email): email_splited = email.split('@', 1) - email_splited[1] = email_splited[1].lower() + if len(email_splited) > 1: + email_splited[1] = email_splited[1].lower() - return '@'.join(email_splited) + return ''.join(email_splited) principal_emails_lower = set(map(lower_domain, principal_emails)) csr_emails_lower = set(map(lower_domain, csr_emails)) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index 7c8a4d2..0de5b75 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -268,7 +268,7 @@ def test_00011_emails_are_valid(self): assert True == result, result result = _emails_are_valid([], [u'a...@email.com']) - assert False == result, result + assert True == result, result email_addrs = [u'invalidEmailAddress'] result = _emails_are_valid(email_addrs, []) From b45cab8de1c61b5fc89487c44b76897c35adcd93 Mon Sep 17 00:00:00 2001 From: Felipe Volpone <felipevolp...@gmail.com> Date: Wed, 10 May 2017 10:24:47 -0300 Subject: [PATCH 4/4] Fixing typos --- ipaserver/plugins/cert.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 8251701..e6b53ff 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -962,11 +962,11 @@ def _emails_are_valid(csr_emails, principal_emails): """ def lower_domain(email): - email_splited = email.split('@', 1) - if len(email_splited) > 1: - email_splited[1] = email_splited[1].lower() + email_splitted = email.split('@', 1) + if len(email_splitted) > 1: + email_splitted[1] = email_splitted[1].lower() - return ''.join(email_splited) + return ''.join(email_splitted) principal_emails_lower = set(map(lower_domain, principal_emails)) csr_emails_lower = set(map(lower_domain, csr_emails))
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
