From 2cf0ad5cfd2d558c844bc9640c121fa35ebb1c30 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 25 Aug 2021 17:13:55 +0200 Subject: [PATCH] Add URI system records for KDC MIT KRB5 1.15 introduced KDC service discovery with URI records. _kerberos and _kpasswd URI records can provide TCP, UDP, and Kerberos KDC-Proxy references. URI lookups take precedence over SRV lookups, falling back to SRV lookups if no URI records are found. Also reduce TTL for system records from one day to one hour. It allows users to remove or update discovery entries in a timely fashion. See: https://web.mit.edu/kerberos/krb5-latest/doc/admin/realm_config.html#kdc-discovery Fixes: https://pagure.io/freeipa/issue/8968 Signed-off-by: Christian Heimes Reviewed-By: Rob Crittenden --- ipaserver/dns_data_management.py | 65 ++++++++++- .../test_integration/test_dns_locations.py | 101 ++++++++++++++++-- 2 files changed, 152 insertions(+), 14 deletions(-) diff --git a/ipaserver/dns_data_management.py b/ipaserver/dns_data_management.py index aad00062a..c4516f703 100644 --- a/ipaserver/dns_data_management.py +++ b/ipaserver/dns_data_management.py @@ -32,6 +32,7 @@ logger = logging.getLogger(__name__) IPA_DEFAULT_MASTER_SRV_REC = ( # srv record name, port (DNSName('_ldap._tcp'), 389), + # Kerberos records are provided for MIT KRB5 < 1.15 and AD (DNSName('_kerberos._tcp'), 88), (DNSName('_kerberos._udp'), 88), (DNSName('_kerberos-master._tcp'), 88), @@ -40,6 +41,20 @@ IPA_DEFAULT_MASTER_SRV_REC = ( (DNSName('_kpasswd._udp'), 464), ) +IPA_DEFAULT_MASTER_URI_REC = ( + # URI record name, URI template + + # MIT KRB5 1.15+ prefers URI records for service discovery + # scheme: always krb5srv + # flags: empty or 'm' for primary server + # transport: 'tcp', 'udp', or 'kkdcp') + # residual: 'hostname', 'hostname:port', or 'https://' URL + (DNSName('_kerberos'), "krb5srv:m:tcp:{hostname}"), + (DNSName('_kerberos'), "krb5srv:m:udp:{hostname}"), + (DNSName('_kpasswd'), "krb5srv:m:tcp:{hostname}"), + (DNSName('_kpasswd'), "krb5srv:m:udp:{hostname}"), +) + IPA_DEFAULT_ADTRUST_SRV_REC = ( # srv record name, port (DNSName('_ldap._tcp.Default-First-Site-Name._sites.dc._msdcs'), 389), @@ -67,6 +82,8 @@ class IPASystemRecords: # fixme do it configurable PRIORITY_HIGH = 0 PRIORITY_LOW = 50 + # FIXME: use TTL from config + TTL = 3600 def __init__(self, api_instance, all_servers=False): self.api_instance = api_instance @@ -134,7 +151,35 @@ class IPASystemRecords: rdataset = zone_obj.get_rdataset( r_name, rdatatype.SRV, create=True) - rdataset.add(rd, ttl=86400) # FIXME: use TTL from config + rdataset.add(rd, ttl=self.TTL) + + def __add_uri_records( + self, zone_obj, hostname, rname_uri_map, + weight=100, priority=0, location=None + ): + assert isinstance(hostname, DNSName) + assert isinstance(priority, int) + assert isinstance(weight, int) + + if location: + suffix = self.__get_location_suffix(location) + else: + suffix = self.domain_abs + + for name, uri_template in rname_uri_map: + uri = uri_template.format(hostname=hostname.make_absolute()) + rd = rdata.from_text( + rdataclass.IN, rdatatype.URI, + '{0} {1} {2}'.format( + priority, weight, uri + ) + ) + + r_name = name.derelativize(suffix) + + rdataset = zone_obj.get_rdataset( + r_name, rdatatype.URI, create=True) + rdataset.add(rd, ttl=self.TTL) def __add_ca_records_from_hostname(self, zone_obj, hostname): assert isinstance(hostname, DNSName) and hostname.is_absolute() @@ -163,7 +208,7 @@ class IPASystemRecords: for rd in rrset: rdataset = zone_obj.get_rdataset( r_name, rd.rdtype, create=True) - rdataset.add(rd, ttl=86400) # FIXME: use TTL from config + rdataset.add(rd, ttl=self.TTL) def __add_kerberos_txt_rec(self, zone_obj): # FIXME: with external DNS, this should generate records for all @@ -174,7 +219,7 @@ class IPASystemRecords: rdataset = zone_obj.get_rdataset( r_name, rdatatype.TXT, create=True ) - rdataset.add(rd, ttl=86400) # FIXME: use TTL from config + rdataset.add(rd, ttl=self.TTL) def _add_base_dns_records_for_server( self, zone_obj, hostname, roles=None, include_master_role=True, @@ -198,6 +243,12 @@ class IPASystemRecords: IPA_DEFAULT_MASTER_SRV_REC, weight=server['weight'] ) + self.__add_uri_records( + zone_obj, + hostname_abs, + IPA_DEFAULT_MASTER_URI_REC, + weight=server['weight'] + ) if 'CA server' in eff_roles: self.__add_ca_records_from_hostname(zone_obj, hostname_abs) @@ -244,6 +295,14 @@ class IPASystemRecords: priority=priority, location=location ) + self.__add_uri_records( + zone_obj, + hostname_abs, + IPA_DEFAULT_MASTER_URI_REC, + weight=server['weight'], + priority=priority, + location=location + ) if 'AD trust controller' in eff_roles: self.__add_srv_records( diff --git a/ipatests/test_integration/test_dns_locations.py b/ipatests/test_integration/test_dns_locations.py index 6eac0d011..44900af80 100644 --- a/ipatests/test_integration/test_dns_locations.py +++ b/ipatests/test_integration/test_dns_locations.py @@ -28,6 +28,17 @@ IPA_DEFAULT_MASTER_SRV_REC = ( (DNSName(u'_kpasswd._udp'), 464), ) +IPA_DEFAULT_MASTER_URI_REC = ( + ( + DNSName('_kerberos'), + ("krb5srv:m:tcp:{hostname}", "krb5srv:m:udp:{hostname}") + ), + ( + DNSName('_kpasswd'), + ("krb5srv:m:tcp:{hostname}", "krb5srv:m:udp:{hostname}") + ), +) + IPA_DEFAULT_ADTRUST_SRV_REC = ( # srv record name, port (DNSName(u'_ldap._tcp.Default-First-Site-Name._sites.dc._msdcs'), 389), @@ -79,6 +90,21 @@ def _gen_expected_srv_rrset(rname, port, servers, ttl=86400): ) +def _gen_expected_uri_rrset(rname, uri_templates, servers, ttl=86400): + rdata_list = [ + "{prio} {weight} {uri}".format( + prio=prio, + weight=weight, + uri=uri_template.format(hostname=servername.make_absolute()), + ) + for uri_template in uri_templates + for prio, weight, servername in servers + ] + return dns.rrset.from_text_list( + rname, ttl, dns.rdataclass.IN, dns.rdatatype.URI, rdata_list + ) + + def _gen_expected_a_rrset(rname, servers, ttl=86400): return dns.rrset.from_text_list(rname, ttl, dns.rdataclass.IN, dns.rdatatype.A, servers) @@ -174,6 +200,20 @@ class TestDNSLocations(IntegrationTest): "with IP: '{}' for name '{}' (expected:\n{}\ngot:\n{})". format(server_ip, name_abs, expected, query)) + def _test_URI_rec_against_server(self, server_ip, domain, expected_servers, + rec_list=IPA_DEFAULT_MASTER_URI_REC): + for rname, uri_templates in rec_list: + name_abs = rname.derelativize(domain) + expected = _gen_expected_uri_rrset( + name_abs, uri_templates, expected_servers) + query = resolve_records_from_server( + name_abs, 'URI', server_ip) + + assert expected == query, ( + "Expected and received DNS data do not match on server " + "with IP: '{}' for name '{}' (expected:\n{}\ngot:\n{})". + format(server_ip, name_abs, expected, query)) + def test_without_locations(self): """Servers are not in locations, this tests if basic system records are generated properly""" @@ -185,6 +225,9 @@ class TestDNSLocations(IntegrationTest): for ip in (self.master.ip, self.replicas[0].ip, self.replicas[1].ip): self._test_SRV_rec_against_server(ip, self.domain, expected_servers) + self._test_URI_rec_against_server( + ip, self.domain, expected_servers + ) def test_nsupdate_without_locations(self): """Test nsupdate file generated by dns-update-system-records @@ -227,11 +270,19 @@ class TestDNSLocations(IntegrationTest): ) self._test_SRV_rec_against_server( - self.replicas[0].ip, domain_prague_loc, servers_prague_loc) + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) + self._test_URI_rec_against_server( + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) for ip in (self.master.ip, self.replicas[1].ip): self._test_SRV_rec_against_server( - ip, domain_without_loc, servers_without_loc) + ip, domain_without_loc, servers_without_loc + ) + self._test_URI_rec_against_server( + ip, domain_without_loc, servers_without_loc + ) def test_two_replicas_in_location(self): """Put second replica to location and test if records changed properly @@ -270,13 +321,25 @@ class TestDNSLocations(IntegrationTest): self.master.domain.name).make_absolute()) self._test_SRV_rec_against_server( - self.replicas[0].ip, domain_prague_loc, servers_prague_loc) + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) + self._test_URI_rec_against_server( + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) self._test_SRV_rec_against_server( - self.replicas[1].ip, domain_paris_loc, servers_paris_loc) + self.replicas[1].ip, domain_paris_loc, servers_paris_loc + ) + self._test_URI_rec_against_server( + self.replicas[1].ip, domain_paris_loc, servers_paris_loc + ) self._test_SRV_rec_against_server( - self.master.ip, domain_without_loc, servers_without_loc) + self.master.ip, domain_without_loc, servers_without_loc + ) + self._test_URI_rec_against_server( + self.master.ip, domain_without_loc, servers_without_loc + ) def test_all_servers_in_location(self): """Put master (as second server) to location and test if records @@ -308,11 +371,19 @@ class TestDNSLocations(IntegrationTest): self.master.domain.name).make_absolute()) self._test_SRV_rec_against_server( - self.replicas[0].ip, domain_prague_loc, servers_prague_loc) + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) + self._test_URI_rec_against_server( + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) for ip in (self.replicas[1].ip, self.master.ip): - self._test_SRV_rec_against_server(ip, domain_paris_loc, - servers_paris_loc) + self._test_SRV_rec_against_server( + ip, domain_paris_loc, servers_paris_loc + ) + self._test_URI_rec_against_server( + ip, domain_paris_loc, servers_paris_loc + ) def test_change_weight(self): """Change weight of master and test if records changed properly @@ -347,11 +418,19 @@ class TestDNSLocations(IntegrationTest): self.master.domain.name).make_absolute()) self._test_SRV_rec_against_server( - self.replicas[0].ip, domain_prague_loc, servers_prague_loc) + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) + self._test_URI_rec_against_server( + self.replicas[0].ip, domain_prague_loc, servers_prague_loc + ) for ip in (self.replicas[1].ip, self.master.ip): - self._test_SRV_rec_against_server(ip, domain_paris_loc, - servers_paris_loc) + self._test_SRV_rec_against_server( + ip, domain_paris_loc, servers_paris_loc + ) + self._test_URI_rec_against_server( + ip, domain_paris_loc, servers_paris_loc + ) def test_change_weight_relative_zero_0(self): """Change weight of one master and check on relative weight %