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 6210297824b61c20e3ca70dff3c48ffd47aee29e Mon Sep 17 00:00:00 2001 From: felipe barreto <fbarreto@localhost.localdomain> Date: Wed, 26 Apr 2017 11:08:35 -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 | 20 +++++++++++++++++++- ipatests/test_xmlrpc/test_cert_plugin.py | 25 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 9f90107..a0b2b83 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -705,7 +705,8 @@ 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: + if not _emails_are_valid(email_addrs, + principal_obj.get('mail', [])): raise errors.ValidationError( name='csr', error=_( @@ -860,6 +861,23 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): ) +def _emails_are_valid(cert_emails, principal_emails): + """ + Checks if any email addr from DN does not appear in ldap entry, + comparing the domain part case-insensitively. + """ + + def lower_domain(email): + return email.split('@')[0] + '@' + email.split('@')[1].lower() + + principal_emails_lower = [lower_domain(email) for email in principal_emails] + + email_addrs = [attr.value for attr in cert_emails] + cert_emails_lower = [lower_domain(email) for email in email_addrs] + + return not any(set(cert_emails_lower) - set(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 0b8277b..cd8ee7b 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -253,6 +253,31 @@ def test_00010_cleanup(self): res = api.Command['service_find'](self.service_princ) assert res['count'] == 0 + def test_00011_email_are_valid(self): + from ipaserver.plugins.cert import _emails_are_valid + from collections import namedtuple + NameAttribute = namedtuple('NameAttribute', 'value') + + cert = [NameAttribute(u'a...@email.com')] + result = _emails_are_valid(cert, [u'a...@email.com']) + assert True == result, result + + cert = [NameAttribute(u'a...@email.com')] + result = _emails_are_valid(cert, [u'a...@email.com', u'anot...@email.com']) + assert True == result, result + + cert = [NameAttribute(u'a...@email.com'), NameAttribute('anot...@email.com')] + result = _emails_are_valid(cert, [u'a...@email.com']) + assert False == result, result + + result = _emails_are_valid([], [u'a...@email.com']) + assert True == result, result + + cert = [NameAttribute(u'a...@email.com')] + result = _emails_are_valid(cert, []) + assert False == result, result + + @pytest.mark.tier1 class test_cert_find(XMLRPC_test): From 943a287a71384e10d2f13e8402907f0d3d10a085 Mon Sep 17 00:00:00 2001 From: felipe barreto <fbarreto@localhost.localdomain> Date: Tue, 2 May 2017 12:29:46 -0300 Subject: [PATCH 2/4] Checking the emails in SAN extension --- ipaserver/plugins/cert.py | 31 ++++++++++++++++++++----------- ipatests/test_xmlrpc/test_cert_plugin.py | 28 +++++++++++++++++----------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index a0b2b83..88cf6d4 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -705,7 +705,11 @@ 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 not _emails_are_valid(email_addrs, + + san_email_addrs = csr_obj.extensions.get_extension_for_oid( + cryptography.x509.oid.ExtensionOID.SUBJECT_ALTERNATIVE_NAME) + + if not _emails_are_valid(email_addrs, san_email_addrs, principal_obj.get('mail', [])): raise errors.ValidationError( name='csr', @@ -861,21 +865,26 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): ) -def _emails_are_valid(cert_emails, principal_emails): - """ - Checks if any email addr from DN does not appear in ldap entry, - comparing the domain part case-insensitively. - """ +def _emails_are_valid(subject_emails, san_emails, principal_emails): + if not any(principal_emails): + return False def lower_domain(email): - return email.split('@')[0] + '@' + email.split('@')[1].lower() + if '@' not in email: + return '' + + email_splited = email.split('@') + return '{}@{}'.format(email_splited[0], email_splited[1].lower()) - principal_emails_lower = [lower_domain(email) for email in principal_emails] + principal_emails_lower = map(lower_domain, principal_emails) + subject_emails = [attr.value for attr in subject_emails] + san_emails = [attr.value for attr in san_emails] - email_addrs = [attr.value for attr in cert_emails] - cert_emails_lower = [lower_domain(email) for email in email_addrs] + subject_emails_lower = map(lower_domain, subject_emails) + san_emails_lower = map(lower_domain, san_emails) - return not any(set(cert_emails_lower) - set(principal_emails_lower)) + return (set(principal_emails_lower).issubset(subject_emails_lower) or + set(principal_emails_lower).issubset(san_emails_lower)) def principal_to_principal_type(principal): diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index cd8ee7b..b76d3f1 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -256,27 +256,33 @@ def test_00010_cleanup(self): def test_00011_email_are_valid(self): from ipaserver.plugins.cert import _emails_are_valid from collections import namedtuple - NameAttribute = namedtuple('NameAttribute', 'value') + NameAttr = namedtuple('NameAttr', 'value') - cert = [NameAttribute(u'a...@email.com')] - result = _emails_are_valid(cert, [u'a...@email.com']) + subject_addrs = [NameAttr(u'a...@email.com')] + result = _emails_are_valid(subject_addrs, [], [u'a...@email.com']) assert True == result, result - cert = [NameAttribute(u'a...@email.com')] - result = _emails_are_valid(cert, [u'a...@email.com', u'anot...@email.com']) + san_addrs = [NameAttr(u'a...@email.com'), NameAttr(u'anot...@email.com')] + result = _emails_are_valid([], san_addrs, [u'a...@email.com']) assert True == result, result - cert = [NameAttribute(u'a...@email.com'), NameAttribute('anot...@email.com')] - result = _emails_are_valid(cert, [u'a...@email.com']) + result = _emails_are_valid([], [], [u'a...@email.com']) assert False == result, result - result = _emails_are_valid([], [u'a...@email.com']) - assert True == result, result + subject_addrs = [NameAttr(u'a...@email.com')] + san_addrs = [NameAttr(u'a...@email.com')] + result = _emails_are_valid(subject_addrs, san_addrs, []) + assert False == result, result - cert = [NameAttribute(u'a...@email.com')] - result = _emails_are_valid(cert, []) + subject_addrs = [NameAttr(u'invalidEmailAddress')] + san_addrs = [NameAttr(u'va...@email.com')] + result = _emails_are_valid(subject_addrs, san_addrs, []) assert False == result, result + subject_addrs = [NameAttr(u'va...@email.com')] + san_addrs = [NameAttr(u'invalidEmailAddress')] + result = _emails_are_valid(subject_addrs, san_addrs, []) + assert False == result, result @pytest.mark.tier1 From 47edf4b1f51fcc9d7b432b8d59ee9af1063f97ee Mon Sep 17 00:00:00 2001 From: felipe barreto <fbarreto@localhost.localdomain> Date: Tue, 2 May 2017 12:44:02 -0300 Subject: [PATCH 3/4] Adding code comments and docstrings --- ipaserver/plugins/cert.py | 5 +++++ ipatests/test_xmlrpc/test_cert_plugin.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 88cf6d4..2b812fb 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -866,6 +866,11 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): def _emails_are_valid(subject_emails, san_emails, principal_emails): + """ + Checks if any email addr from DN or SAN extension does not + appear in ldap entry, comparing the domain part case-insensitively. + """ + if not any(principal_emails): return False diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index b76d3f1..bc3be91 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -254,6 +254,11 @@ def test_00010_cleanup(self): 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 from collections import namedtuple NameAttr = namedtuple('NameAttr', 'value') From e8d567c46ea494d7dc9eeafe5c0ed1f95cb9ffa6 Mon Sep 17 00:00:00 2001 From: felipe barreto <fbarreto@localhost.localdomain> Date: Wed, 3 May 2017 14:18:45 -0300 Subject: [PATCH 4/4] Fixing unit tests --- ipatests/test_xmlrpc/test_cert_plugin.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index bc3be91..734b211 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -265,29 +265,29 @@ def test_00011_email_are_valid(self): subject_addrs = [NameAttr(u'a...@email.com')] result = _emails_are_valid(subject_addrs, [], [u'a...@email.com']) - assert True == result, result + assert True is result, result san_addrs = [NameAttr(u'a...@email.com'), NameAttr(u'anot...@email.com')] result = _emails_are_valid([], san_addrs, [u'a...@email.com']) - assert True == result, result + assert True is result, result result = _emails_are_valid([], [], [u'a...@email.com']) - assert False == result, result + assert False is result, result subject_addrs = [NameAttr(u'a...@email.com')] san_addrs = [NameAttr(u'a...@email.com')] result = _emails_are_valid(subject_addrs, san_addrs, []) - assert False == result, result + assert False is result, result subject_addrs = [NameAttr(u'invalidEmailAddress')] san_addrs = [NameAttr(u'va...@email.com')] result = _emails_are_valid(subject_addrs, san_addrs, []) - assert False == result, result + assert False is result, result subject_addrs = [NameAttr(u'va...@email.com')] san_addrs = [NameAttr(u'invalidEmailAddress')] result = _emails_are_valid(subject_addrs, san_addrs, []) - assert False == result, result + assert False is result, result @pytest.mark.tier1
-- 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
