Blob Blame History Raw
From 128500198d3782a76616cf1d971d5aeb17e8c1da 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           | 62 +++++++++++++++++--
 2 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index d353bc3ea..fe7ea34f5 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -827,13 +827,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
@@ -866,7 +866,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 bf4de05bf..c0475d30d 100644
--- a/ipatests/test_xmlrpc/test_cert_request_ip_address.py
+++ b/ipatests/test_xmlrpc/test_cert_request_ip_address.py
@@ -28,10 +28,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 = f'iptest.{api.env.domain}'
+host_shortname = 'iptest'
+host_fqdn = f'{host_shortname}.{api.env.domain}'
 host_princ = f'host/{host_fqdn}'
 host_ptr = f'{host_fqdn}.'
 
+host2_shortname = 'iptest2'
+host2_fqdn = f'{host2_shortname}.{api.env.domain}'
+host2_princ = f'host/{host2_fqdn}'
+host2_ptr = f'{host2_fqdn}.'
+
 other_fqdn = f'other.{api.env.domain}'
 other_ptr = f'{other_fqdn}.'
 
@@ -39,6 +45,10 @@ ipv4_address = '169.254.0.42'
 ipv4_revzone_s = '0.254.169.in-addr.arpa.'
 ipv4_revrec_s = '42'
 
+host2_ipv4_address = '169.254.0.43'
+host2_ipv4_revzone_s = '0.254.169.in-addr.arpa.'
+host2_ipv4_revrec_s = '43'
+
 ipv6_address = 'fe80::8f18:bdab:4299:95fa'
 ipv6_revzone_s = '0.0.0.0.0.0.0.0.0.0.0.0.0.8.e.f.ip6.arpa.'
 ipv6_revrec_s = 'a.f.5.9.9.9.2.4.b.a.d.b.8.1.f.8'
@@ -46,7 +56,13 @@ ipv6_revrec_s = 'a.f.5.9.9.9.2.4.b.a.d.b.8.1.f.8'
 
 @pytest.fixture(scope='class')
 def host(request, xmlrpc_setup):
-    tr = HostTracker('iptest')
+    tr = HostTracker(host_shortname)
+    return tr.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def host2(request, xmlrpc_setup):
+    tr = HostTracker(host2_shortname)
     return tr.make_fixture(request)
 
 
@@ -88,6 +104,12 @@ def ipv6_revzone(host):
     yield from _zone_setup(host, ipv6_revzone_s)
 
 
+@pytest.fixture(scope='class')
+def host2_ipv4_ptr(host2, ipv4_revzone):
+    yield from _record_setup(
+        host2, ipv4_revzone, host2_ipv4_revrec_s, ptrrecord=host2_ptr)
+
+
 @pytest.fixture(scope='class')
 def ipv4_ptr(host, ipv4_revzone):
     yield from _record_setup(
@@ -100,16 +122,22 @@ def ipv6_ptr(host, ipv6_revzone):
         host, ipv6_revzone, ipv6_revrec_s, ptrrecord=host_ptr)
 
 
+@pytest.fixture(scope='class')
+def host2_ipv4_a(host2):
+    yield from _record_setup(
+        host2, api.env.domain, host2_shortname, arecord=host2_ipv4_address)
+
+
 @pytest.fixture(scope='class')
 def ipv4_a(host):
     yield from _record_setup(
-        host, api.env.domain, 'iptest', arecord=ipv4_address)
+        host, api.env.domain, host_shortname, arecord=ipv4_address)
 
 
 @pytest.fixture(scope='class')
 def ipv6_aaaa(host):
     yield from _record_setup(
-        host, api.env.domain, 'iptest', aaaarecord=ipv6_address)
+        host, api.env.domain, host_shortname, aaaarecord=ipv6_address)
 
 
 @pytest.fixture(scope='class')
@@ -210,6 +238,12 @@ csr_cname2 = csr([
     x509.DNSName(f'cname2.{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
@@ -449,3 +483,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