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/3] 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 e98274bb35f964cd3a9debe87e5194bdde3f56a0 Mon Sep 17 00:00:00 2001
From: felipe barreto <fbarreto@localhost.localdomain>
Date: Wed, 3 May 2017 16:29:45 -0300
Subject: [PATCH 2/3] Checking the emails in SAN extension

---
 ipaserver/plugins/cert.py                | 33 +++++++++++++------
 ipatests/test_xmlrpc/test_cert_plugin.py | 54 +++++++++++++++++++-------------
 2 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index a0b2b83..7c2f2ed 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -705,7 +705,12 @@ 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 +866,31 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw):
         )
 
 
-def _emails_are_valid(cert_emails, principal_emails):
+def _emails_are_valid(subject_emails, san_emails, principal_emails):
     """
-    Checks if any email addr from DN does not appear in ldap entry,
-    comparing the domain part case-insensitively.
+    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
+
     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..8039b9b 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -254,29 +254,41 @@ 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
-        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
-
+        NameAttr = namedtuple('NameAttr', 'value')
+
+        subject_addrs = [NameAttr(u'a...@email.com')]
+        result = _emails_are_valid(subject_addrs, [], [u'a...@email.com'])
+        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 is result, result
+
+        result = _emails_are_valid([], [], [u'a...@email.com'])
+        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 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 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 is result, result
 
 
 @pytest.mark.tier1

From 4cab064908281aeb5bfda7f163df6b0728a5a513 Mon Sep 17 00:00:00 2001
From: felipe barreto <fbarreto@localhost.localdomain>
Date: Wed, 3 May 2017 17:07:59 -0300
Subject: [PATCH 3/3] Fixing code style

---
 ipaserver/plugins/cert.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 7c2f2ed..1bf88ff 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -882,12 +882,12 @@ def lower_domain(email):
         email_splited = email.split('@')
         return '{}@{}'.format(email_splited[0], email_splited[1].lower())
 
-    principal_emails_lower = map(lower_domain, principal_emails)
+    principal_emails_lower = list(map(lower_domain, principal_emails))
     subject_emails = [attr.value for attr in subject_emails]
     san_emails = [attr.value for attr in san_emails]
 
-    subject_emails_lower = map(lower_domain, subject_emails)
-    san_emails_lower = map(lower_domain, san_emails)
+    subject_emails_lower = list(map(lower_domain, subject_emails))
+    san_emails_lower = list(map(lower_domain, san_emails))
 
     return (set(principal_emails_lower).issubset(subject_emails_lower) or
             set(principal_emails_lower).issubset(san_emails_lower))
-- 
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

Reply via email to