From 87afc31838166409e29d2de750f10622cf1ddc46 Mon Sep 17 00:00:00 2001

From: Fraser Tweedale <ftweedal@redhat.com>

Date: Thu, 11 Jun 2020 22:42:38 +1000

Subject: [PATCH] fix iPAddress cert issuance for >1 host/service

The 'cert_request' command accumulates DNS names from the CSR,

before checking that all IP addresses in the CSR are reachable from

those DNS names.  Before adding a DNS name to the set, we check that

that it corresponds to the FQDN of a known host/service principal

(including principal aliases).  When a DNS name maps to a

"alternative" principal (i.e.  not the one given via the 'principal'

argument), this check was not being performed correctly.

Specifically, we were looking for the 'krbprincipalname' field on

the RPC response object directly, instead of its 'result' field.

To resolve the issue, dereference the RPC response to its 'result'

field before invoking the '_dns_name_matches_principal' subroutine.

Fixes: https://pagure.io/freeipa/issue/8368

Reviewed-By: Rob Crittenden <rcritten@redhat.com>

---

 ipaserver/plugins/cert.py                     |  6 +-

 .../test_cert_request_ip_address.py           | 64 +++++++++++++++++--

 2 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py

index 57ad1327feb62d5f45266bc9d5c6b8fba75a81aa..4af5c97f5722a7799509764df93c2433661dba20 100644

--- a/ipaserver/plugins/cert.py

+++ b/ipaserver/plugins/cert.py

@@ -814,13 +814,13 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):

                 try:

                     if principal_type == HOST:

                         alt_principal_obj = api.Command['host_show'](

-                            name, all=True)

+                            name, all=True)['result']

                     elif principal_type == KRBTGT:

                         alt_principal = kerberos.Principal(

                             (u'host', name), principal.realm)

                     elif principal_type == SERVICE:

                         alt_principal_obj = api.Command['service_show'](

-                            alt_principal, all=True)

+                            alt_principal, all=True)['result']

                 except errors.NotFound:

                     # We don't want to issue any certificates referencing

                     # machines we don't know about. Nothing is stored in this

@@ -853,7 +853,7 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):

                         pass

                     # Now check write access and caacl

-                    altdn = alt_principal_obj['result']['dn']

+                    altdn = alt_principal_obj['dn']

                     if not ldap.can_write(altdn, "usercertificate"):

                         raise errors.ACIError(info=_(

                             "Insufficient privilege to create a certificate "

diff --git a/ipatests/test_xmlrpc/test_cert_request_ip_address.py b/ipatests/test_xmlrpc/test_cert_request_ip_address.py

index 560a82da318933a9f2b8bf2a498f4eb6659ac2b3..e657df03fd110a39be24e156bf7da69769d4e79a 100644

--- a/ipatests/test_xmlrpc/test_cert_request_ip_address.py

+++ b/ipatests/test_xmlrpc/test_cert_request_ip_address.py

@@ -29,10 +29,16 @@ from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker

 from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker

 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test

-host_fqdn = u'iptest.{}'.format(api.env.domain)

+host_shortname = u'iptest'

+host_fqdn = u'{}.{}'.format(host_shortname, api.env.domain)

 host_princ = u'host/{}'.format(host_fqdn)

 host_ptr = u'{}.'.format(host_fqdn)

+host2_shortname = u'iptest2'

+host2_fqdn = u'{}.{}'.format(host2_shortname, api.env.domain)

+host2_princ = u'host/{}'.format(host2_fqdn)

+host2_ptr = u'{}.'.format(host2_fqdn)

+

 other_fqdn = u'other.{}'.format(api.env.domain)

 other_ptr = u'{}.'.format(other_fqdn)

@@ -40,6 +46,10 @@ ipv4_address = u'169.254.0.42'

 ipv4_revzone_s = u'0.254.169.in-addr.arpa.'

 ipv4_revrec_s = u'42'

+host2_ipv4_address = u'169.254.0.43'

+host2_ipv4_revzone_s = u'0.254.169.in-addr.arpa.'

+host2_ipv4_revrec_s = u'43'

+

 ipv6_address = u'fe80::8f18:bdab:4299:95fa'

 ipv6_revzone_s = u'0.0.0.0.0.0.0.0.0.0.0.0.0.8.e.f.ip6.arpa.'

 ipv6_revrec_s = u'a.f.5.9.9.9.2.4.b.a.d.b.8.1.f.8'

@@ -47,7 +57,13 @@ ipv6_revrec_s = u'a.f.5.9.9.9.2.4.b.a.d.b.8.1.f.8'

 @pytest.fixture(scope='class')

 def host(request):

-    tr = HostTracker('iptest')

+    tr = HostTracker(host_shortname)

+    return tr.make_fixture(request)

+

+

+@pytest.fixture(scope='class')

+def host2(request):

+    tr = HostTracker(host2_shortname)

     return tr.make_fixture(request)

@@ -91,6 +107,13 @@ def ipv6_revzone(host):

         yield x

+@yield_fixture(scope='class')

+def host2_ipv4_ptr(host2, ipv4_revzone):

+    for x in _record_setup(

+            host2, ipv4_revzone, host2_ipv4_revrec_s, ptrrecord=host2_ptr):

+        yield x

+

+

 @yield_fixture(scope='class')

 def ipv4_ptr(host, ipv4_revzone):

     for x in _record_setup(

@@ -105,17 +128,24 @@ def ipv6_ptr(host, ipv6_revzone):

         yield x

+@yield_fixture(scope='class')

+def host2_ipv4_a(host2):

+    for x in _record_setup(

+            host2, api.env.domain, host2_shortname, arecord=host2_ipv4_address):

+        yield x

+

+

 @yield_fixture(scope='class')

 def ipv4_a(host):

     for x in _record_setup(

-            host, api.env.domain, u'iptest', arecord=ipv4_address):

+            host, api.env.domain, host_shortname, arecord=ipv4_address):

         yield x

 @yield_fixture(scope='class')

 def ipv6_aaaa(host):

     for x in _record_setup(

-            host, api.env.domain, u'iptest', aaaarecord=ipv6_address):

+            host, api.env.domain, host_shortname, aaaarecord=ipv6_address):

         yield x

@@ -221,6 +251,12 @@ csr_cname2 = csr([

     x509.DNSName(u'cname2.{}'.format(api.env.domain)),

     x509.IPAddress(ipaddress.ip_address(ipv4_address)),

 ])

+csr_two_dnsname_two_ip = csr([

+    x509.DNSName(host_fqdn),

+    x509.IPAddress(ipaddress.ip_address(ipv4_address)),

+    x509.DNSName(host2_fqdn),

+    x509.IPAddress(ipaddress.ip_address(host2_ipv4_address)),

+])

 @pytest.fixture

@@ -463,3 +499,23 @@ class TestIPAddressCNAME(XMLRPC_test):

     def test_two_levels(self, host, csr_cname2):

         with pytest.raises(errors.ValidationError, match=PAT_FWD):

             host.run_command('cert_request', csr_cname2, principal=host_princ)

+

+

+@pytest.mark.tier1

+class TestTwoHostsTwoIPAddresses(XMLRPC_test):

+    """

+    Test certificate issuance with CSR containing two hosts

+    and two IP addresses (one for each host).

+

+    """

+    def test_host_exists(

+        self, host, host2, ipv4_a, ipv4_ptr, host2_ipv4_a, host2_ipv4_ptr,

+    ):

+        # for convenience, this test also establishes the DNS

+        # record fixtures, which have class scope

+        host.ensure_exists()

+        host2.ensure_exists()

+

+    def test_issuance(self, host, csr_two_dnsname_two_ip):

+        host.run_command(

+            'cert_request', csr_two_dnsname_two_ip, principal=host_princ)

-- 

2.26.2