From f02f94e4ad8cee7dee0f96580cb5c0cd15f26fa4 Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Thu, 30 May 2024 16:12:39 +0200 Subject: [PATCH 1/8] Add support for SVCB and HTTPS records These records are defined in [RFC 9460](https://datatracker.ietf.org/doc/html/rfc9460) and are used for "Service Binding" and provide clients with all connection information for network services. Modern browsers already query for these records and several larger websites have [adopted usage of HTTPS records](https://blog.apnic.net/2023/12/18/use-of-https-resource-records/) already. --- octodns/record/svcb.py | 251 +++++++++++++ tests/test_octodns_record_svcb.py | 587 ++++++++++++++++++++++++++++++ tests/zones/unit.tests.tst | 11 + 3 files changed, 849 insertions(+) create mode 100644 octodns/record/svcb.py create mode 100644 tests/test_octodns_record_svcb.py diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py new file mode 100644 index 0000000..393e5b4 --- /dev/null +++ b/octodns/record/svcb.py @@ -0,0 +1,251 @@ +# +# This file describes the SVCB and HTTPS records as defined in RFC 9460 +# It also supports the 'ech' SvcParam as defined in draft-ietf-tls-svcb-ech-02 +# + +from base64 import b64decode +from binascii import Error as binascii_error +from ipaddress import AddressValueError, IPv4Address, IPv6Address + +from fqdn import FQDN + +from ..equality import EqualityTupleMixin +from ..idna import idna_encode +from .base import Record, ValuesMixin, unquote +from .chunked import _ChunkedValue +from .rr import RrParseError + +SUPPORTED_PARAMS = {} + + +def validate_svcparam_port(svcparamvalue): + reasons = [] + try: + port = int(svcparamvalue) + if 0 < port > 65535: + reasons.append(f'port {port} is not a valid number') + except ValueError: + reasons.append('port is not a number') + return reasons + + +def validate_svcparam_alpn(svcparamvalue): + reasons = [] + alpns = svcparamvalue.split(',') + for alpn in alpns: + reasons += _ChunkedValue.validate(alpn, 'SVCB') + return reasons + + +def validate_svcparam_iphint(ip_version, svcparamvalue): + reasons = [] + addresses = svcparamvalue.split(',') + for address in addresses: + try: + if ip_version == 4: + IPv4Address(address) + if ip_version == 6: + IPv6Address(address) + except AddressValueError: + reasons.append( + f'ip{ip_version}hint "{address}" is not a valid IPv{ip_version} address' + ) + return reasons + + +def validate_svcparam_ip4hint(svcparamvalue): + return validate_svcparam_iphint(4, svcparamvalue) + + +def validate_svcparam_ip6hint(svcparamvalue): + return validate_svcparam_iphint(6, svcparamvalue) + + +def validate_svcparam_mandatory(svcparamvalue): + reasons = [] + mandatories = svcparamvalue.split(',') + for mandatory in mandatories: + if ( + mandatory not in SUPPORTED_PARAMS.keys() + and not mandatory.startswith('key') + ): + reasons.append(f'unsupported SvcParam "{mandatory}" in mandatory') + if mandatory.startswith('key'): + reasons += validate_svckey_number(mandatory) + return reasons + + +def validate_svcparam_ech(svcparamvalue): + try: + b64decode(svcparamvalue, validate=True) + except binascii_error: + return ['ech SvcParam is invalid Base64'] + + +def validate_svckey_number(paramkey): + try: + paramkeynum = int(paramkey[3:]) + if 7 < paramkeynum > 65535: + return [f'SvcParam key "{paramkey}" has wrong key number'] + except ValueError: + return [f'SvcParam key "{paramkey}" has wrong format'] + return [] + + +SUPPORTED_PARAMS = { + 'no-default-alpn': {'has_arg': False}, + 'alpn': {'validate': validate_svcparam_alpn}, + 'port': {'validate': validate_svcparam_port}, + 'ipv4hint': {'validate': validate_svcparam_ip4hint}, + 'ipv6hint': {'validate': validate_svcparam_ip6hint}, + 'mandatory': {'validate': validate_svcparam_mandatory}, + 'ech': {'validate': validate_svcparam_ech}, +} + + +class SvcbValue(EqualityTupleMixin, dict): + + @classmethod + def parse_rdata_text(cls, value): + try: + # XXX: these are called SvcPriority, TargetName, and SvcParams in RFC 9460 section 2. + # Should we mirror these names, or are priority, target and params good enough? + # XXX: Should we split params into the specific ParamKeys and ParamValues? + (priority, target, *params) = value.split(' ') + except ValueError: + raise RrParseError() + try: + priority = int(priority) + except ValueError: + pass + target = unquote(target) + return {'priority': priority, 'target': target, 'params': params} + + @classmethod + def validate(cls, data, _): + reasons = [] + for value in data: + priority = -1 + if 'priority' not in value: + reasons.append('missing priority') + else: + try: + priority = int(value.get('priority', 0)) + if priority < 0 or priority > 65535: + reasons.append(f'invalid priority ' f'"{priority}"') + except ValueError: + reasons.append( + f'invalid priority ' f'"{value["priority"]}"' + ) + + if 'target' not in value or value['target'] == '': + reasons.append('missing target') + else: + target = str(value.get('target', '')) + target = idna_encode(target) + if not target.endswith('.'): + reasons.append(f'SVCB value "{target}" missing trailing .') + if target != '.' and not FQDN(target).is_valid: + reasons.append( + f'Invalid SVCB target "{target}" is not a valid FQDN.' + ) + + if 'params' in value: + params = value.get('params', list()) + if priority == 0 and len(params) != 0: + reasons.append('params set on AliasMode SVCB record') + for param in params: + # XXX: Should we test for keys existing when set in 'mandatory'? + paramkey, *paramvalue = param.split('=') + if paramkey.startswith('key'): + reasons += validate_svckey_number(paramkey) + continue + if ( + paramkey not in SUPPORTED_PARAMS.keys() + and not paramkey.startswith('key') + ): + reasons.append(f'Unknown SvcParam {paramkey}') + continue + if SUPPORTED_PARAMS[paramkey].get('has_arg', True): + reasons += SUPPORTED_PARAMS[paramkey]['validate']( + paramvalue[0] + ) + if ( + not SUPPORTED_PARAMS[paramkey].get('has_arg', True) + and len(paramvalue) != 0 + ): + reasons.append( + f'SvcParam {paramkey} has value when it should not' + ) + + return reasons + + @classmethod + def process(cls, values): + return [cls(v) for v in values] + + def __init__(self, value): + super().__init__( + { + 'priority': int(value['priority']), + 'target': idna_encode(value['target']), + 'params': value.get('params', list()), + } + ) + + @property + def priority(self): + return self['priority'] + + @priority.setter + def priority(self, value): + self['priority'] = value + + @property + def target(self): + return self['target'] + + @target.setter + def target(self, value): + self['target'] = value + + @property + def params(self): + return self['params'] + + @params.setter + def params(self, value): + self['params'] = value + + @property + def rdata_text(self): + params = '' + if len(self.params) != 0: + params = f' {" ".join(self.params)}' + return f'{self.priority} {self.target}{params}' + + def __hash__(self): + return hash(self.__repr__()) + + def _equality_tuple(self): + return (self.priority, self.target, self.params) + + def __repr__(self): + params = '' + if len(self.params) != 0: + params = f' {" ".join(self.params)}' + return f"'{self.priority} {self.target}{params}'" + + +class SvcbRecord(ValuesMixin, Record): + _type = 'SVCB' + _value_type = SvcbValue + + +class HttpsRecord(ValuesMixin, Record): + _type = 'HTTPS' + _value_type = SvcbValue + + +Record.register_type(SvcbRecord) +Record.register_type(HttpsRecord) diff --git a/tests/test_octodns_record_svcb.py b/tests/test_octodns_record_svcb.py new file mode 100644 index 0000000..940fa96 --- /dev/null +++ b/tests/test_octodns_record_svcb.py @@ -0,0 +1,587 @@ +# +# +# + +from unittest import TestCase + +from helpers import SimpleProvider + +from octodns.record import Record +from octodns.record.exception import ValidationError +from octodns.record.rr import RrParseError +from octodns.record.svcb import SvcbRecord, SvcbValue +from octodns.zone import Zone + + +class TestRecordSvcb(TestCase): + zone = Zone('unit.tests.', []) + + def test_svcb(self): + aliasmode_value = SvcbValue( + {'priority': 0, 'target': 'foo.example.com.'} + ) + aliasmode_data = {'ttl': 300, 'value': aliasmode_value} + a = SvcbRecord(self.zone, 'alias', aliasmode_data) + self.assertEqual('alias', a.name) + self.assertEqual('alias.unit.tests.', a.fqdn) + self.assertEqual(300, a.ttl) + self.assertEqual(aliasmode_value['priority'], a.values[0].priority) + self.assertEqual(aliasmode_value['target'], a.values[0].target) + self.assertEqual(aliasmode_value['params'], a.values[0].params) + self.assertEqual(aliasmode_data, a.data) + + servicemode_values = [ + SvcbValue( + { + 'priority': 1, + 'target': 'foo.example.com.', + 'params': 'port=8002', + } + ), + SvcbValue( + { + 'priority': 2, + 'target': 'foo.example.net.', + 'params': 'port=8080', + } + ), + ] + servicemode_data = {'ttl': 300, 'values': servicemode_values} + b = SvcbRecord(self.zone, 'service', servicemode_data) + self.assertEqual('service', b.name) + self.assertEqual('service.unit.tests.', b.fqdn) + self.assertEqual(300, b.ttl) + self.assertEqual( + servicemode_values[0]['priority'], b.values[0].priority + ) + self.assertEqual(servicemode_values[0]['target'], b.values[0].target) + self.assertEqual(servicemode_values[0]['params'], b.values[0].params) + self.assertEqual( + servicemode_values[1]['priority'], b.values[1].priority + ) + self.assertEqual(servicemode_values[1]['target'], b.values[1].target) + self.assertEqual(servicemode_values[1]['params'], b.values[1].params) + self.assertEqual(servicemode_data, b.data) + + target = SimpleProvider() + # No changes with self + self.assertFalse(b.changes(b, target)) + # Diff in priority causes change + other = SvcbRecord( + self.zone, 'service2', {'ttl': 30, 'values': servicemode_values} + ) + other.values[0].priority = 22 + change = b.changes(other, target) + self.assertEqual(change.existing, b) + self.assertEqual(change.new, other) + # Diff in target causes change + other.values[0].priority = b.values[0].priority + other.values[0].target = 'blabla.example.com' + change = b.changes(other, target) + self.assertEqual(change.existing, b) + self.assertEqual(change.new, other) + # Diff in params causes change + other.values[0].target = b.values[0].target + other.values[0].params = 'port=8888' + change = b.changes(other, target) + self.assertEqual(change.existing, b) + self.assertEqual(change.new, other) + + # __repr__ doesn't blow up + a.__repr__() + b.__repr__() + + def test_svcb_value_rdata_text(self): + # empty string won't parse + with self.assertRaises(RrParseError): + SvcbValue.parse_rdata_text('') + + # single word won't parse + with self.assertRaises(RrParseError): + SvcbValue.parse_rdata_text('nope') + + # priority not int + self.assertEqual( + {'priority': 'one', 'target': 'foo.example.com', 'params': list()}, + SvcbValue.parse_rdata_text('one foo.example.com'), + ) + + # valid with params + self.assertEqual( + { + 'priority': 1, + 'target': 'svcb.unit.tests.', + 'params': ['port=8080'], + }, + SvcbValue.parse_rdata_text('1 svcb.unit.tests. port=8080'), + ) + + # quoted target + self.assertEqual( + {'priority': 1, 'target': 'svcb.unit.tests.', 'params': list()}, + SvcbValue.parse_rdata_text('1 "svcb.unit.tests."'), + ) + + zone = Zone('unit.tests.', []) + a = SvcbRecord( + zone, + 'svc', + { + 'ttl': 32, + 'value': { + 'priority': 1, + 'target': 'svcb.unit.tests.', + 'params': ['port=8080'], + }, + }, + ) + self.assertEqual(1, a.values[0].priority) + self.assertEqual('svcb.unit.tests.', a.values[0].target) + self.assertEqual(['port=8080'], a.values[0].params) + + # both directions should match + rdata = '1 svcb.unit.tests. port=8080' + record = SvcbRecord( + zone, 'svc', {'ttl': 32, 'value': SvcbValue.parse_rdata_text(rdata)} + ) + self.assertEqual(rdata, record.values[0].rdata_text) + + # both directions should match + rdata = '0 svcb.unit.tests.' + record = SvcbRecord( + zone, 'svc', {'ttl': 32, 'value': SvcbValue.parse_rdata_text(rdata)} + ) + self.assertEqual(rdata, record.values[0].rdata_text) + + def test_svcb_value(self): + a = SvcbValue({'priority': 0, 'target': 'foo.', 'params': list()}) + b = SvcbValue({'priority': 1, 'target': 'foo.', 'params': list()}) + c = SvcbValue( + {'priority': 0, 'target': 'foo.', 'params': ['port=8080']} + ) + d = SvcbValue( + { + 'priority': 0, + 'target': 'foo.', + 'params': ['alpn=h2,h3', 'port=8080'], + } + ) + e = SvcbValue( + {'priority': 0, 'target': 'mmm.', 'params': ['ipv4hint=192.0.2.1']} + ) + + self.assertEqual(a, a) + self.assertEqual(b, b) + self.assertEqual(c, c) + self.assertEqual(d, d) + self.assertEqual(e, e) + + self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(a, d) + self.assertNotEqual(a, e) + self.assertNotEqual(b, a) + self.assertNotEqual(b, c) + self.assertNotEqual(b, d) + self.assertNotEqual(b, e) + self.assertNotEqual(c, a) + self.assertNotEqual(c, b) + self.assertNotEqual(c, d) + self.assertNotEqual(c, e) + self.assertNotEqual(d, a) + self.assertNotEqual(d, b) + self.assertNotEqual(d, c) + self.assertNotEqual(d, e) + self.assertNotEqual(e, a) + self.assertNotEqual(e, b) + self.assertNotEqual(e, c) + self.assertNotEqual(e, d) + + self.assertTrue(a < b) + self.assertTrue(a < c) + + self.assertTrue(b > a) + self.assertTrue(b > c) + + self.assertTrue(c > a) + self.assertTrue(c < b) + + self.assertTrue(a <= b) + self.assertTrue(a <= c) + self.assertTrue(a <= a) + self.assertTrue(a >= a) + + self.assertTrue(b >= a) + self.assertTrue(b >= c) + self.assertTrue(b >= b) + self.assertTrue(b <= b) + + self.assertTrue(c >= a) + self.assertTrue(c <= b) + self.assertTrue(c >= c) + self.assertTrue(c <= c) + + # Hash + values = set() + values.add(a) + self.assertTrue(a in values) + self.assertFalse(b in values) + values.add(b) + self.assertTrue(b in values) + + def test_validation(self): + # doesn't blow up + Record.new( + self.zone, + 'svcb', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 1, 'target': 'foo.bar.baz.'}, + }, + ) + + # Wildcards are fine + Record.new( + self.zone, + '*', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 1, 'target': 'foo.bar.baz.'}, + }, + ) + + # missing priority + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'target': 'foo.bar.baz.'}, + }, + ) + self.assertEqual(['missing priority'], ctx.exception.reasons) + + # invalid priority + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 'foo', 'target': 'foo.bar.baz.'}, + }, + ) + self.assertEqual(['invalid priority "foo"'], ctx.exception.reasons) + + # invalid priority (out of range) + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 100000, 'target': 'foo.bar.baz.'}, + }, + ) + self.assertEqual(['invalid priority "100000"'], ctx.exception.reasons) + + # missing target + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + {'type': 'SVCB', 'ttl': 600, 'value': {'priority': 1}}, + ) + self.assertEqual(['missing target'], ctx.exception.reasons) + + # invalid target + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 1, 'target': 'foo.bar.baz'}, + }, + ) + self.assertEqual( + ['SVCB value "foo.bar.baz" missing trailing .'], + ctx.exception.reasons, + ) + + # falsey target + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 1, 'target': ''}, + }, + ) + self.assertEqual(['missing target'], ctx.exception.reasons) + + # target must be a valid FQDN + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 1, 'target': 'bla foo.bar.com.'}, + }, + ) + self.assertEqual( + ['Invalid SVCB target "bla foo.bar.com." is not a valid FQDN.'], + ctx.exception.reasons, + ) + + # target can be root label + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': {'priority': 1, 'target': '.'}, + }, + ) + + # Params can't be set for AliasMode + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 0, + 'target': 'foo.bar.com.', + 'params': ['port=8000'], + }, + }, + ) + self.assertEqual( + ['params set on AliasMode SVCB record'], ctx.exception.reasons + ) + + # Unknown param + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['blablabla=222'], + }, + }, + ) + self.assertEqual(['Unknown SvcParam blablabla'], ctx.exception.reasons) + + # Port number invalid + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['port=100000'], + }, + }, + ) + self.assertEqual( + ['port 100000 is not a valid number'], ctx.exception.reasons + ) + + # Port number not an int + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['port=foo'], + }, + }, + ) + self.assertEqual(['port is not a number'], ctx.exception.reasons) + + # no-default-alpn set + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['no-default-alpn'], + }, + }, + ) + + # no-default-alpn has value + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['no-default-alpn=foobar'], + }, + }, + ) + self.assertEqual( + ['SvcParam no-default-alpn has value when it should not'], + ctx.exception.reasons, + ) + + # alpn is broken + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['alpn=h2,😅'], + }, + }, + ) + self.assertEqual(['non ASCII character in "😅"'], ctx.exception.reasons) + + # ipv4hint + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['ipv4hint=192.0.2.0,500.500.30.30'], + }, + }, + ) + self.assertEqual( + ['ip4hint "500.500.30.30" is not a valid IPv4 address'], + ctx.exception.reasons, + ) + + # ipv6hint + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['ipv6hint=2001:db8:43::1,notanip'], + }, + }, + ) + self.assertEqual( + ['ip6hint "notanip" is not a valid IPv6 address'], + ctx.exception.reasons, + ) + + # mandatory + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['mandatory=ipv4hint,unknown,key4444'], + }, + }, + ) + self.assertEqual( + ['unsupported SvcParam "unknown" in mandatory'], + ctx.exception.reasons, + ) + + # ech + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': ['ech=dG90YWxseUZha2VFQ0hPcHRpb24'], + }, + }, + ) + self.assertEqual( + ['ech SvcParam is invalid Base64'], ctx.exception.reasons + ) + + # broken keyNNNN format + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'priority': 1, + 'target': 'foo.bar.com.', + 'params': [ + 'key100000=foo', + 'key3333=bar', + 'keyXXX=foo', + ], + }, + }, + ) + self.assertEqual( + [ + 'SvcParam key "key100000" has wrong key number', + 'SvcParam key "keyXXX" has wrong format', + ], + ctx.exception.reasons, + ) diff --git a/tests/zones/unit.tests.tst b/tests/zones/unit.tests.tst index 82549ea..5c08baf 100644 --- a/tests/zones/unit.tests.tst +++ b/tests/zones/unit.tests.tst @@ -55,3 +55,14 @@ aaaa 600 IN AAAA 2601:644:500:e210:62f8:1dff:feb8:947a ; CNAME Records cname 300 IN CNAME unit.tests. included 300 IN CNAME unit.tests. + +; SVCB and HTTPS records +svcb-alias 300 IN SVCB 0 alias-target.unit.test. + +svcb-service 300 IN SVCB 1 . ipv4hint=192.0.2.4 port=9000 +svcb-service 300 IN SVCB 2 svcb-target.unit.test. port=9001 + +https-alias 300 IN HTTPS 0 alias-target.unit.test. + +https-service 300 IN HTTPS 1 . ipv4hint=192.0.2.4 port=9000 +https-service 300 IN HTTPS 2 svcb-target.unit.test. port=9001 From 23cc45dad94c8877ed8805010294db70209569bd Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Mon, 3 Jun 2024 09:36:47 +0200 Subject: [PATCH 2/8] SVCB: rename variables to RFC names --- octodns/record/svcb.py | 108 +++++++-------- tests/test_octodns_record_svcb.py | 213 +++++++++++++++++------------- 2 files changed, 178 insertions(+), 143 deletions(-) diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py index 393e5b4..8ff40af 100644 --- a/octodns/record/svcb.py +++ b/octodns/record/svcb.py @@ -92,6 +92,7 @@ def validate_svckey_number(paramkey): return [] +# cc https://datatracker.ietf.org/doc/html/rfc9460#keys SUPPORTED_PARAMS = { 'no-default-alpn': {'has_arg': False}, 'alpn': {'validate': validate_svcparam_alpn}, @@ -108,53 +109,55 @@ class SvcbValue(EqualityTupleMixin, dict): @classmethod def parse_rdata_text(cls, value): try: - # XXX: these are called SvcPriority, TargetName, and SvcParams in RFC 9460 section 2. - # Should we mirror these names, or are priority, target and params good enough? # XXX: Should we split params into the specific ParamKeys and ParamValues? - (priority, target, *params) = value.split(' ') + (svcpriority, targetname, *svcparams) = value.split(' ') except ValueError: raise RrParseError() try: - priority = int(priority) + svcpriority = int(svcpriority) except ValueError: pass - target = unquote(target) - return {'priority': priority, 'target': target, 'params': params} + targetname = unquote(targetname) + return { + 'svcpriority': svcpriority, + 'targetname': targetname, + 'svcparams': svcparams, + } @classmethod def validate(cls, data, _): reasons = [] for value in data: - priority = -1 - if 'priority' not in value: - reasons.append('missing priority') + svcpriority = -1 + if 'svcpriority' not in value: + reasons.append('missing svcpriority') else: try: - priority = int(value.get('priority', 0)) - if priority < 0 or priority > 65535: - reasons.append(f'invalid priority ' f'"{priority}"') + svcpriority = int(value.get('svcpriority', 0)) + if svcpriority < 0 or svcpriority > 65535: + reasons.append(f'invalid priority ' f'"{svcpriority}"') except ValueError: - reasons.append( - f'invalid priority ' f'"{value["priority"]}"' - ) + reasons.append(f'invalid priority "{value["svcpriority"]}"') - if 'target' not in value or value['target'] == '': - reasons.append('missing target') + if 'targetname' not in value or value['targetname'] == '': + reasons.append('missing targetname') else: - target = str(value.get('target', '')) - target = idna_encode(target) - if not target.endswith('.'): - reasons.append(f'SVCB value "{target}" missing trailing .') - if target != '.' and not FQDN(target).is_valid: + targetname = str(value.get('targetname', '')) + targetname = idna_encode(targetname) + if not targetname.endswith('.'): reasons.append( - f'Invalid SVCB target "{target}" is not a valid FQDN.' + f'SVCB value "{targetname}" missing trailing .' + ) + if targetname != '.' and not FQDN(targetname).is_valid: + reasons.append( + f'Invalid SVCB target "{targetname}" is not a valid FQDN.' ) - if 'params' in value: - params = value.get('params', list()) - if priority == 0 and len(params) != 0: - reasons.append('params set on AliasMode SVCB record') - for param in params: + if 'svcparams' in value: + svcparams = value.get('svcparams', list()) + if svcpriority == 0 and len(svcparams) != 0: + reasons.append('svcparams set on AliasMode SVCB record') + for param in svcparams: # XXX: Should we test for keys existing when set in 'mandatory'? paramkey, *paramvalue = param.split('=') if paramkey.startswith('key'): @@ -187,54 +190,51 @@ class SvcbValue(EqualityTupleMixin, dict): def __init__(self, value): super().__init__( { - 'priority': int(value['priority']), - 'target': idna_encode(value['target']), - 'params': value.get('params', list()), + 'svcpriority': int(value['svcpriority']), + 'targetname': idna_encode(value['targetname']), + 'svcparams': value.get('svcparams', list()), } ) @property - def priority(self): - return self['priority'] + def svcpriority(self): + return self['svcpriority'] - @priority.setter - def priority(self, value): - self['priority'] = value + @svcpriority.setter + def svcpriority(self, value): + self['svcpriority'] = value @property - def target(self): - return self['target'] + def targetname(self): + return self['targetname'] - @target.setter - def target(self, value): - self['target'] = value + @targetname.setter + def targetname(self, value): + self['targetname'] = value @property - def params(self): - return self['params'] + def svcparams(self): + return self['svcparams'] - @params.setter - def params(self, value): - self['params'] = value + @svcparams.setter + def svcparams(self, value): + self['svcparams'] = value @property def rdata_text(self): params = '' - if len(self.params) != 0: - params = f' {" ".join(self.params)}' - return f'{self.priority} {self.target}{params}' + if len(self.svcparams) != 0: + params = f' {" ".join(self.svcparams)}' + return f'{self.svcpriority} {self.targetname}{params}' def __hash__(self): return hash(self.__repr__()) def _equality_tuple(self): - return (self.priority, self.target, self.params) + return (self.svcpriority, self.targetname, self.svcparams) def __repr__(self): - params = '' - if len(self.params) != 0: - params = f' {" ".join(self.params)}' - return f"'{self.priority} {self.target}{params}'" + return f"'{self.rdata_text}'" class SvcbRecord(ValuesMixin, Record): diff --git a/tests/test_octodns_record_svcb.py b/tests/test_octodns_record_svcb.py index 940fa96..729b977 100644 --- a/tests/test_octodns_record_svcb.py +++ b/tests/test_octodns_record_svcb.py @@ -18,31 +18,33 @@ class TestRecordSvcb(TestCase): def test_svcb(self): aliasmode_value = SvcbValue( - {'priority': 0, 'target': 'foo.example.com.'} + {'svcpriority': 0, 'targetname': 'foo.example.com.'} ) aliasmode_data = {'ttl': 300, 'value': aliasmode_value} a = SvcbRecord(self.zone, 'alias', aliasmode_data) self.assertEqual('alias', a.name) self.assertEqual('alias.unit.tests.', a.fqdn) self.assertEqual(300, a.ttl) - self.assertEqual(aliasmode_value['priority'], a.values[0].priority) - self.assertEqual(aliasmode_value['target'], a.values[0].target) - self.assertEqual(aliasmode_value['params'], a.values[0].params) + self.assertEqual( + aliasmode_value['svcpriority'], a.values[0].svcpriority + ) + self.assertEqual(aliasmode_value['targetname'], a.values[0].targetname) + self.assertEqual(aliasmode_value['svcparams'], a.values[0].svcparams) self.assertEqual(aliasmode_data, a.data) servicemode_values = [ SvcbValue( { - 'priority': 1, - 'target': 'foo.example.com.', - 'params': 'port=8002', + 'svcpriority': 1, + 'targetname': 'foo.example.com.', + 'svcparams': 'port=8002', } ), SvcbValue( { - 'priority': 2, - 'target': 'foo.example.net.', - 'params': 'port=8080', + 'svcpriority': 2, + 'targetname': 'foo.example.net.', + 'svcparams': 'port=8080', } ), ] @@ -52,15 +54,23 @@ class TestRecordSvcb(TestCase): self.assertEqual('service.unit.tests.', b.fqdn) self.assertEqual(300, b.ttl) self.assertEqual( - servicemode_values[0]['priority'], b.values[0].priority + servicemode_values[0]['svcpriority'], b.values[0].svcpriority + ) + self.assertEqual( + servicemode_values[0]['targetname'], b.values[0].targetname + ) + self.assertEqual( + servicemode_values[0]['svcparams'], b.values[0].svcparams ) - self.assertEqual(servicemode_values[0]['target'], b.values[0].target) - self.assertEqual(servicemode_values[0]['params'], b.values[0].params) self.assertEqual( - servicemode_values[1]['priority'], b.values[1].priority + servicemode_values[1]['svcpriority'], b.values[1].svcpriority + ) + self.assertEqual( + servicemode_values[1]['targetname'], b.values[1].targetname + ) + self.assertEqual( + servicemode_values[1]['svcparams'], b.values[1].svcparams ) - self.assertEqual(servicemode_values[1]['target'], b.values[1].target) - self.assertEqual(servicemode_values[1]['params'], b.values[1].params) self.assertEqual(servicemode_data, b.data) target = SimpleProvider() @@ -70,19 +80,19 @@ class TestRecordSvcb(TestCase): other = SvcbRecord( self.zone, 'service2', {'ttl': 30, 'values': servicemode_values} ) - other.values[0].priority = 22 + other.values[0].svcpriority = 22 change = b.changes(other, target) self.assertEqual(change.existing, b) self.assertEqual(change.new, other) # Diff in target causes change - other.values[0].priority = b.values[0].priority - other.values[0].target = 'blabla.example.com' + other.values[0].svcpriority = b.values[0].svcpriority + other.values[0].targetname = 'blabla.example.com' change = b.changes(other, target) self.assertEqual(change.existing, b) self.assertEqual(change.new, other) # Diff in params causes change - other.values[0].target = b.values[0].target - other.values[0].params = 'port=8888' + other.values[0].targetname = b.values[0].targetname + other.values[0].svcparams = 'port=8888' change = b.changes(other, target) self.assertEqual(change.existing, b) self.assertEqual(change.new, other) @@ -102,23 +112,31 @@ class TestRecordSvcb(TestCase): # priority not int self.assertEqual( - {'priority': 'one', 'target': 'foo.example.com', 'params': list()}, + { + 'svcpriority': 'one', + 'targetname': 'foo.example.com', + 'svcparams': list(), + }, SvcbValue.parse_rdata_text('one foo.example.com'), ) # valid with params self.assertEqual( { - 'priority': 1, - 'target': 'svcb.unit.tests.', - 'params': ['port=8080'], + 'svcpriority': 1, + 'targetname': 'svcb.unit.tests.', + 'svcparams': ['port=8080'], }, SvcbValue.parse_rdata_text('1 svcb.unit.tests. port=8080'), ) # quoted target self.assertEqual( - {'priority': 1, 'target': 'svcb.unit.tests.', 'params': list()}, + { + 'svcpriority': 1, + 'targetname': 'svcb.unit.tests.', + 'svcparams': list(), + }, SvcbValue.parse_rdata_text('1 "svcb.unit.tests."'), ) @@ -129,15 +147,15 @@ class TestRecordSvcb(TestCase): { 'ttl': 32, 'value': { - 'priority': 1, - 'target': 'svcb.unit.tests.', - 'params': ['port=8080'], + 'svcpriority': 1, + 'targetname': 'svcb.unit.tests.', + 'svcparams': ['port=8080'], }, }, ) - self.assertEqual(1, a.values[0].priority) - self.assertEqual('svcb.unit.tests.', a.values[0].target) - self.assertEqual(['port=8080'], a.values[0].params) + self.assertEqual(1, a.values[0].svcpriority) + self.assertEqual('svcb.unit.tests.', a.values[0].targetname) + self.assertEqual(['port=8080'], a.values[0].svcparams) # both directions should match rdata = '1 svcb.unit.tests. port=8080' @@ -154,20 +172,28 @@ class TestRecordSvcb(TestCase): self.assertEqual(rdata, record.values[0].rdata_text) def test_svcb_value(self): - a = SvcbValue({'priority': 0, 'target': 'foo.', 'params': list()}) - b = SvcbValue({'priority': 1, 'target': 'foo.', 'params': list()}) + a = SvcbValue( + {'svcpriority': 0, 'targetname': 'foo.', 'svcparams': list()} + ) + b = SvcbValue( + {'svcpriority': 1, 'targetname': 'foo.', 'svcparams': list()} + ) c = SvcbValue( - {'priority': 0, 'target': 'foo.', 'params': ['port=8080']} + {'svcpriority': 0, 'targetname': 'foo.', 'svcparams': ['port=8080']} ) d = SvcbValue( { - 'priority': 0, - 'target': 'foo.', - 'params': ['alpn=h2,h3', 'port=8080'], + 'svcpriority': 0, + 'targetname': 'foo.', + 'svcparams': ['alpn=h2,h3', 'port=8080'], } ) e = SvcbValue( - {'priority': 0, 'target': 'mmm.', 'params': ['ipv4hint=192.0.2.1']} + { + 'svcpriority': 0, + 'targetname': 'mmm.', + 'svcparams': ['ipv4hint=192.0.2.1'], + } ) self.assertEqual(a, a) @@ -237,7 +263,7 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 1, 'target': 'foo.bar.baz.'}, + 'value': {'svcpriority': 1, 'targetname': 'foo.bar.baz.'}, }, ) @@ -248,7 +274,7 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 1, 'target': 'foo.bar.baz.'}, + 'value': {'svcpriority': 1, 'targetname': 'foo.bar.baz.'}, }, ) @@ -260,10 +286,10 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'target': 'foo.bar.baz.'}, + 'value': {'targetname': 'foo.bar.baz.'}, }, ) - self.assertEqual(['missing priority'], ctx.exception.reasons) + self.assertEqual(['missing svcpriority'], ctx.exception.reasons) # invalid priority with self.assertRaises(ValidationError) as ctx: @@ -273,7 +299,10 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 'foo', 'target': 'foo.bar.baz.'}, + 'value': { + 'svcpriority': 'foo', + 'targetname': 'foo.bar.baz.', + }, }, ) self.assertEqual(['invalid priority "foo"'], ctx.exception.reasons) @@ -286,7 +315,10 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 100000, 'target': 'foo.bar.baz.'}, + 'value': { + 'svcpriority': 100000, + 'targetname': 'foo.bar.baz.', + }, }, ) self.assertEqual(['invalid priority "100000"'], ctx.exception.reasons) @@ -296,9 +328,9 @@ class TestRecordSvcb(TestCase): Record.new( self.zone, 'foo', - {'type': 'SVCB', 'ttl': 600, 'value': {'priority': 1}}, + {'type': 'SVCB', 'ttl': 600, 'value': {'svcpriority': 1}}, ) - self.assertEqual(['missing target'], ctx.exception.reasons) + self.assertEqual(['missing targetname'], ctx.exception.reasons) # invalid target with self.assertRaises(ValidationError) as ctx: @@ -308,7 +340,7 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 1, 'target': 'foo.bar.baz'}, + 'value': {'svcpriority': 1, 'targetname': 'foo.bar.baz'}, }, ) self.assertEqual( @@ -324,10 +356,10 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 1, 'target': ''}, + 'value': {'svcpriority': 1, 'targetname': ''}, }, ) - self.assertEqual(['missing target'], ctx.exception.reasons) + self.assertEqual(['missing targetname'], ctx.exception.reasons) # target must be a valid FQDN with self.assertRaises(ValidationError) as ctx: @@ -337,7 +369,10 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 1, 'target': 'bla foo.bar.com.'}, + 'value': { + 'svcpriority': 1, + 'targetname': 'bla foo.bar.com.', + }, }, ) self.assertEqual( @@ -352,7 +387,7 @@ class TestRecordSvcb(TestCase): { 'type': 'SVCB', 'ttl': 600, - 'value': {'priority': 1, 'target': '.'}, + 'value': {'svcpriority': 1, 'targetname': '.'}, }, ) @@ -365,14 +400,14 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 0, - 'target': 'foo.bar.com.', - 'params': ['port=8000'], + 'svcpriority': 0, + 'targetname': 'foo.bar.com.', + 'svcparams': ['port=8000'], }, }, ) self.assertEqual( - ['params set on AliasMode SVCB record'], ctx.exception.reasons + ['svcparams set on AliasMode SVCB record'], ctx.exception.reasons ) # Unknown param @@ -384,9 +419,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['blablabla=222'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['blablabla=222'], }, }, ) @@ -401,9 +436,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['port=100000'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['port=100000'], }, }, ) @@ -420,9 +455,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['port=foo'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['port=foo'], }, }, ) @@ -436,9 +471,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['no-default-alpn'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['no-default-alpn'], }, }, ) @@ -452,9 +487,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['no-default-alpn=foobar'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['no-default-alpn=foobar'], }, }, ) @@ -472,9 +507,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['alpn=h2,😅'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['alpn=h2,😅'], }, }, ) @@ -489,9 +524,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['ipv4hint=192.0.2.0,500.500.30.30'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['ipv4hint=192.0.2.0,500.500.30.30'], }, }, ) @@ -509,9 +544,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['ipv6hint=2001:db8:43::1,notanip'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['ipv6hint=2001:db8:43::1,notanip'], }, }, ) @@ -529,9 +564,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['mandatory=ipv4hint,unknown,key4444'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['mandatory=ipv4hint,unknown,key4444'], }, }, ) @@ -549,9 +584,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': ['ech=dG90YWxseUZha2VFQ0hPcHRpb24'], + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': ['ech=dG90YWxseUZha2VFQ0hPcHRpb24'], }, }, ) @@ -568,9 +603,9 @@ class TestRecordSvcb(TestCase): 'type': 'SVCB', 'ttl': 600, 'value': { - 'priority': 1, - 'target': 'foo.bar.com.', - 'params': [ + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': [ 'key100000=foo', 'key3333=bar', 'keyXXX=foo', From 88464687487673247a163885b3379e67c2cd88f8 Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Mon, 3 Jun 2024 10:54:38 +0200 Subject: [PATCH 3/8] SVCB: Make SvcParams a dict --- octodns/record/svcb.py | 54 ++++++++++++++-------- tests/test_octodns_record_svcb.py | 74 ++++++++++++++++++------------- 2 files changed, 77 insertions(+), 51 deletions(-) diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py index 8ff40af..3ec3a39 100644 --- a/octodns/record/svcb.py +++ b/octodns/record/svcb.py @@ -118,10 +118,19 @@ class SvcbValue(EqualityTupleMixin, dict): except ValueError: pass targetname = unquote(targetname) + params = dict() + for svcparam in svcparams: + paramkey, *paramvalue = svcparam.split('=') + if paramkey in params.keys(): + raise RrParseError(f'{paramkey} is specified twice') + if len(paramvalue) != 0: + params[paramkey] = paramvalue[0] + continue + params[paramkey] = None return { 'svcpriority': svcpriority, 'targetname': targetname, - 'svcparams': svcparams, + 'svcparams': params, } @classmethod @@ -154,31 +163,30 @@ class SvcbValue(EqualityTupleMixin, dict): ) if 'svcparams' in value: - svcparams = value.get('svcparams', list()) + svcparams = value.get('svcparams', dict()) if svcpriority == 0 and len(svcparams) != 0: reasons.append('svcparams set on AliasMode SVCB record') - for param in svcparams: + for svcparamkey, svcparamvalue in svcparams.items(): # XXX: Should we test for keys existing when set in 'mandatory'? - paramkey, *paramvalue = param.split('=') - if paramkey.startswith('key'): - reasons += validate_svckey_number(paramkey) + if svcparamkey.startswith('key'): + reasons += validate_svckey_number(svcparamkey) continue if ( - paramkey not in SUPPORTED_PARAMS.keys() - and not paramkey.startswith('key') + svcparamkey not in SUPPORTED_PARAMS.keys() + and not svcparamkey.startswith('key') ): - reasons.append(f'Unknown SvcParam {paramkey}') + reasons.append(f'Unknown SvcParam {svcparamkey}') continue - if SUPPORTED_PARAMS[paramkey].get('has_arg', True): - reasons += SUPPORTED_PARAMS[paramkey]['validate']( - paramvalue[0] + if SUPPORTED_PARAMS[svcparamkey].get('has_arg', True): + reasons += SUPPORTED_PARAMS[svcparamkey]['validate']( + svcparamvalue ) if ( - not SUPPORTED_PARAMS[paramkey].get('has_arg', True) - and len(paramvalue) != 0 + not SUPPORTED_PARAMS[svcparamkey].get('has_arg', True) + and svcparamvalue is not None ): reasons.append( - f'SvcParam {paramkey} has value when it should not' + f'SvcParam {svcparamkey} has value when it should not' ) return reasons @@ -192,7 +200,7 @@ class SvcbValue(EqualityTupleMixin, dict): { 'svcpriority': int(value['svcpriority']), 'targetname': idna_encode(value['targetname']), - 'svcparams': value.get('svcparams', list()), + 'svcparams': value.get('svcparams', dict()), } ) @@ -223,15 +231,23 @@ class SvcbValue(EqualityTupleMixin, dict): @property def rdata_text(self): params = '' - if len(self.svcparams) != 0: - params = f' {" ".join(self.svcparams)}' + for svcparamkey, svcparamvalue in self.svcparams.items(): + params += f' {svcparamkey}' + if svcparamvalue is not None: + params += f'={svcparamvalue}' return f'{self.svcpriority} {self.targetname}{params}' def __hash__(self): return hash(self.__repr__()) def _equality_tuple(self): - return (self.svcpriority, self.targetname, self.svcparams) + params = set() + for svcparamkey, svcparamvalue in self.svcparams.items(): + if svcparamvalue is not None: + params.add(f'{svcparamkey}={svcparamvalue}') + else: + params.add(f'{svcparamkey}') + return (self.svcpriority, self.targetname, params) def __repr__(self): return f"'{self.rdata_text}'" diff --git a/tests/test_octodns_record_svcb.py b/tests/test_octodns_record_svcb.py index 729b977..767d3f0 100644 --- a/tests/test_octodns_record_svcb.py +++ b/tests/test_octodns_record_svcb.py @@ -37,14 +37,14 @@ class TestRecordSvcb(TestCase): { 'svcpriority': 1, 'targetname': 'foo.example.com.', - 'svcparams': 'port=8002', + 'svcparams': {'port': 8002}, } ), SvcbValue( { 'svcpriority': 2, 'targetname': 'foo.example.net.', - 'svcparams': 'port=8080', + 'svcparams': {'port': 8080}, } ), ] @@ -86,13 +86,13 @@ class TestRecordSvcb(TestCase): self.assertEqual(change.new, other) # Diff in target causes change other.values[0].svcpriority = b.values[0].svcpriority - other.values[0].targetname = 'blabla.example.com' + other.values[0].targetname = 'blabla.example.com.' change = b.changes(other, target) self.assertEqual(change.existing, b) self.assertEqual(change.new, other) # Diff in params causes change other.values[0].targetname = b.values[0].targetname - other.values[0].svcparams = 'port=8888' + other.values[0].svcparams = {'port': '8888'} change = b.changes(other, target) self.assertEqual(change.existing, b) self.assertEqual(change.new, other) @@ -110,12 +110,16 @@ class TestRecordSvcb(TestCase): with self.assertRaises(RrParseError): SvcbValue.parse_rdata_text('nope') + # Double keys are not allowed + with self.assertRaises(RrParseError): + SvcbValue.parse_rdata_text('1 foo.example.com port=8080 port=8084') + # priority not int self.assertEqual( { 'svcpriority': 'one', 'targetname': 'foo.example.com', - 'svcparams': list(), + 'svcparams': dict(), }, SvcbValue.parse_rdata_text('one foo.example.com'), ) @@ -125,9 +129,11 @@ class TestRecordSvcb(TestCase): { 'svcpriority': 1, 'targetname': 'svcb.unit.tests.', - 'svcparams': ['port=8080'], + 'svcparams': {'port': '8080', 'no-default-alpn': None}, }, - SvcbValue.parse_rdata_text('1 svcb.unit.tests. port=8080'), + SvcbValue.parse_rdata_text( + '1 svcb.unit.tests. port=8080 no-default-alpn' + ), ) # quoted target @@ -135,7 +141,7 @@ class TestRecordSvcb(TestCase): { 'svcpriority': 1, 'targetname': 'svcb.unit.tests.', - 'svcparams': list(), + 'svcparams': dict(), }, SvcbValue.parse_rdata_text('1 "svcb.unit.tests."'), ) @@ -149,16 +155,16 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'svcb.unit.tests.', - 'svcparams': ['port=8080'], + 'svcparams': {'port': '8080'}, }, }, ) self.assertEqual(1, a.values[0].svcpriority) self.assertEqual('svcb.unit.tests.', a.values[0].targetname) - self.assertEqual(['port=8080'], a.values[0].svcparams) + self.assertEqual({'port': '8080'}, a.values[0].svcparams) # both directions should match - rdata = '1 svcb.unit.tests. port=8080' + rdata = '1 svcb.unit.tests. port=8080 no-default-alpn' record = SvcbRecord( zone, 'svc', {'ttl': 32, 'value': SvcbValue.parse_rdata_text(rdata)} ) @@ -173,26 +179,30 @@ class TestRecordSvcb(TestCase): def test_svcb_value(self): a = SvcbValue( - {'svcpriority': 0, 'targetname': 'foo.', 'svcparams': list()} + {'svcpriority': 0, 'targetname': 'foo.', 'svcparams': dict()} ) b = SvcbValue( - {'svcpriority': 1, 'targetname': 'foo.', 'svcparams': list()} + {'svcpriority': 1, 'targetname': 'foo.', 'svcparams': dict()} ) c = SvcbValue( - {'svcpriority': 0, 'targetname': 'foo.', 'svcparams': ['port=8080']} + { + 'svcpriority': 0, + 'targetname': 'foo.', + 'svcparams': {'port': 8080, 'no-default-alpn': None}, + } ) d = SvcbValue( { 'svcpriority': 0, 'targetname': 'foo.', - 'svcparams': ['alpn=h2,h3', 'port=8080'], + 'svcparams': {'alpn': 'h2,h3', 'port': 8080}, } ) e = SvcbValue( { 'svcpriority': 0, 'targetname': 'mmm.', - 'svcparams': ['ipv4hint=192.0.2.1'], + 'svcparams': {'ipv4hint': '192.0.2.1'}, } ) @@ -402,7 +412,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 0, 'targetname': 'foo.bar.com.', - 'svcparams': ['port=8000'], + 'svcparams': {'port': '8000'}, }, }, ) @@ -421,7 +431,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['blablabla=222'], + 'svcparams': {'blablabla': '222'}, }, }, ) @@ -438,7 +448,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['port=100000'], + 'svcparams': {'port': 100000}, }, }, ) @@ -457,7 +467,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['port=foo'], + 'svcparams': {'port': 'foo'}, }, }, ) @@ -473,7 +483,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['no-default-alpn'], + 'svcparams': {'no-default-alpn': None}, }, }, ) @@ -489,7 +499,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['no-default-alpn=foobar'], + 'svcparams': {'no-default-alpn': 'foobar'}, }, }, ) @@ -509,7 +519,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['alpn=h2,😅'], + 'svcparams': {'alpn': 'h2,😅'}, }, }, ) @@ -526,7 +536,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['ipv4hint=192.0.2.0,500.500.30.30'], + 'svcparams': {'ipv4hint': '192.0.2.0,500.500.30.30'}, }, }, ) @@ -546,7 +556,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['ipv6hint=2001:db8:43::1,notanip'], + 'svcparams': {'ipv6hint': '2001:db8:43::1,notanip'}, }, }, ) @@ -566,7 +576,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['mandatory=ipv4hint,unknown,key4444'], + 'svcparams': {'mandatory': 'ipv4hint,unknown,key4444'}, }, }, ) @@ -586,7 +596,7 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': ['ech=dG90YWxseUZha2VFQ0hPcHRpb24'], + 'svcparams': {'ech': ' dG90YWxseUZha2VFQ0hPcHRpb24'}, }, }, ) @@ -605,11 +615,11 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': [ - 'key100000=foo', - 'key3333=bar', - 'keyXXX=foo', - ], + 'svcparams': { + 'key100000': 'foo', + 'key3333': 'bar', + 'keyXXX': 'foo', + }, }, }, ) From b53993e9885456a02ddd539509f0be341aed9da3 Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Tue, 4 Jun 2024 22:31:32 +0200 Subject: [PATCH 4/8] SVCB: force scvparamvalue lists to be lists --- octodns/record/svcb.py | 78 +++++++++++++++++++++++-------- tests/test_octodns_record_svcb.py | 54 +++++++++++++++++---- 2 files changed, 104 insertions(+), 28 deletions(-) diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py index 3ec3a39..c7bcfb0 100644 --- a/octodns/record/svcb.py +++ b/octodns/record/svcb.py @@ -29,18 +29,26 @@ def validate_svcparam_port(svcparamvalue): return reasons +def validate_list(svcparamkey, svcparamvalue): + if not isinstance(svcparamvalue, list): + return [f'{svcparamkey} is not a list'] + return list() + + def validate_svcparam_alpn(svcparamvalue): - reasons = [] - alpns = svcparamvalue.split(',') - for alpn in alpns: + reasons = validate_list('alpn', svcparamvalue) + if len(reasons) != 0: + return reasons + for alpn in svcparamvalue: reasons += _ChunkedValue.validate(alpn, 'SVCB') return reasons def validate_svcparam_iphint(ip_version, svcparamvalue): - reasons = [] - addresses = svcparamvalue.split(',') - for address in addresses: + reasons = validate_list(f'ipv{ip_version}hint', svcparamvalue) + if len(reasons) != 0: + return reasons + for address in svcparamvalue: try: if ip_version == 4: IPv4Address(address) @@ -48,23 +56,24 @@ def validate_svcparam_iphint(ip_version, svcparamvalue): IPv6Address(address) except AddressValueError: reasons.append( - f'ip{ip_version}hint "{address}" is not a valid IPv{ip_version} address' + f'ipv{ip_version}hint "{address}" is not a valid IPv{ip_version} address' ) return reasons -def validate_svcparam_ip4hint(svcparamvalue): +def validate_svcparam_ipv4hint(svcparamvalue): return validate_svcparam_iphint(4, svcparamvalue) -def validate_svcparam_ip6hint(svcparamvalue): +def validate_svcparam_ipv6hint(svcparamvalue): return validate_svcparam_iphint(6, svcparamvalue) def validate_svcparam_mandatory(svcparamvalue): - reasons = [] - mandatories = svcparamvalue.split(',') - for mandatory in mandatories: + reasons = validate_list('mandatory', svcparamvalue) + if len(reasons) != 0: + return reasons + for mandatory in svcparamvalue: if ( mandatory not in SUPPORTED_PARAMS.keys() and not mandatory.startswith('key') @@ -92,14 +101,30 @@ def validate_svckey_number(paramkey): return [] +def parse_rdata_text_svcparamvalue_list(svcparamvalue): + return svcparamvalue.split(',') + + # cc https://datatracker.ietf.org/doc/html/rfc9460#keys SUPPORTED_PARAMS = { 'no-default-alpn': {'has_arg': False}, - 'alpn': {'validate': validate_svcparam_alpn}, + 'alpn': { + 'validate': validate_svcparam_alpn, + 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, + }, 'port': {'validate': validate_svcparam_port}, - 'ipv4hint': {'validate': validate_svcparam_ip4hint}, - 'ipv6hint': {'validate': validate_svcparam_ip6hint}, - 'mandatory': {'validate': validate_svcparam_mandatory}, + 'ipv4hint': { + 'validate': validate_svcparam_ipv4hint, + 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, + }, + 'ipv6hint': { + 'validate': validate_svcparam_ipv6hint, + 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, + }, + 'mandatory': { + 'validate': validate_svcparam_mandatory, + 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, + }, 'ech': {'validate': validate_svcparam_ech}, } @@ -109,7 +134,6 @@ class SvcbValue(EqualityTupleMixin, dict): @classmethod def parse_rdata_text(cls, value): try: - # XXX: Should we split params into the specific ParamKeys and ParamValues? (svcpriority, targetname, *svcparams) = value.split(' ') except ValueError: raise RrParseError() @@ -125,6 +149,15 @@ class SvcbValue(EqualityTupleMixin, dict): raise RrParseError(f'{paramkey} is specified twice') if len(paramvalue) != 0: params[paramkey] = paramvalue[0] + if ( + SUPPORTED_PARAMS.get(paramkey, {}).get( + 'parse_rdata_text', None + ) + is not None + ): + params[paramkey] = SUPPORTED_PARAMS[paramkey][ + 'parse_rdata_text' + ](paramvalue[0]) continue params[paramkey] = None return { @@ -231,10 +264,14 @@ class SvcbValue(EqualityTupleMixin, dict): @property def rdata_text(self): params = '' + # XXX: sort params by key number for svcparamkey, svcparamvalue in self.svcparams.items(): params += f' {svcparamkey}' if svcparamvalue is not None: - params += f'={svcparamvalue}' + if isinstance(svcparamvalue, list): + params += f'={",".join(svcparamvalue)}' + else: + params += f'={svcparamvalue}' return f'{self.svcpriority} {self.targetname}{params}' def __hash__(self): @@ -244,7 +281,10 @@ class SvcbValue(EqualityTupleMixin, dict): params = set() for svcparamkey, svcparamvalue in self.svcparams.items(): if svcparamvalue is not None: - params.add(f'{svcparamkey}={svcparamvalue}') + if isinstance(svcparamvalue, list): + params.add(f'{svcparamkey}={",".join(svcparamvalue)}') + else: + params.add(f'{svcparamkey}={svcparamvalue}') else: params.add(f'{svcparamkey}') return (self.svcpriority, self.targetname, params) diff --git a/tests/test_octodns_record_svcb.py b/tests/test_octodns_record_svcb.py index 767d3f0..51c25ec 100644 --- a/tests/test_octodns_record_svcb.py +++ b/tests/test_octodns_record_svcb.py @@ -164,7 +164,7 @@ class TestRecordSvcb(TestCase): self.assertEqual({'port': '8080'}, a.values[0].svcparams) # both directions should match - rdata = '1 svcb.unit.tests. port=8080 no-default-alpn' + rdata = '1 svcb.unit.tests. port=8080 no-default-alpn ipv4hint=192.0.2.2,192.0.2.53' record = SvcbRecord( zone, 'svc', {'ttl': 32, 'value': SvcbValue.parse_rdata_text(rdata)} ) @@ -195,14 +195,14 @@ class TestRecordSvcb(TestCase): { 'svcpriority': 0, 'targetname': 'foo.', - 'svcparams': {'alpn': 'h2,h3', 'port': 8080}, + 'svcparams': {'alpn': ['h2', 'h3'], 'port': 8080}, } ) e = SvcbValue( { 'svcpriority': 0, 'targetname': 'mmm.', - 'svcparams': {'ipv4hint': '192.0.2.1'}, + 'svcparams': {'ipv4hint': ['192.0.2.1']}, } ) @@ -519,12 +519,42 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': {'alpn': 'h2,😅'}, + 'svcparams': {'alpn': ['h2', '😅']}, }, }, ) self.assertEqual(['non ASCII character in "😅"'], ctx.exception.reasons) + # svcbvaluelist that is not a list + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + 'foo', + { + 'type': 'SVCB', + 'ttl': 600, + 'value': { + 'svcpriority': 1, + 'targetname': 'foo.bar.com.', + 'svcparams': { + 'ipv4hint': '192.0.2.1,192.0.2.2', + 'ipv6hint': '2001:db8::1', + 'mandatory': 'ipv6hint', + 'alpn': 'h2,h3', + }, + }, + }, + ) + self.assertEqual( + [ + 'ipv4hint is not a list', + 'ipv6hint is not a list', + 'mandatory is not a list', + 'alpn is not a list', + ], + ctx.exception.reasons, + ) + # ipv4hint with self.assertRaises(ValidationError) as ctx: Record.new( @@ -536,12 +566,14 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': {'ipv4hint': '192.0.2.0,500.500.30.30'}, + 'svcparams': { + 'ipv4hint': ['192.0.2.0', '500.500.30.30'] + }, }, }, ) self.assertEqual( - ['ip4hint "500.500.30.30" is not a valid IPv4 address'], + ['ipv4hint "500.500.30.30" is not a valid IPv4 address'], ctx.exception.reasons, ) @@ -556,12 +588,14 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': {'ipv6hint': '2001:db8:43::1,notanip'}, + 'svcparams': { + 'ipv6hint': ['2001:db8:43::1', 'notanip'] + }, }, }, ) self.assertEqual( - ['ip6hint "notanip" is not a valid IPv6 address'], + ['ipv6hint "notanip" is not a valid IPv6 address'], ctx.exception.reasons, ) @@ -576,7 +610,9 @@ class TestRecordSvcb(TestCase): 'value': { 'svcpriority': 1, 'targetname': 'foo.bar.com.', - 'svcparams': {'mandatory': 'ipv4hint,unknown,key4444'}, + 'svcparams': { + 'mandatory': ['ipv4hint', 'unknown', 'key4444'] + }, }, }, ) From b19cf82d0552d6934fa520f174697f572a462354 Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Wed, 5 Jun 2024 10:30:20 +0200 Subject: [PATCH 5/8] SVCB: sort rdata svcparams --- octodns/record/svcb.py | 21 ++++++++++++++++----- tests/test_octodns_record_svcb.py | 2 +- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py index c7bcfb0..348601f 100644 --- a/octodns/record/svcb.py +++ b/octodns/record/svcb.py @@ -105,27 +105,37 @@ def parse_rdata_text_svcparamvalue_list(svcparamvalue): return svcparamvalue.split(',') +def svcparamkeysort(svcparamkey): + if svcparamkey.startswith('key'): + return int(svcparamkey[3:]) + return SUPPORTED_PARAMS[svcparamkey]['key_num'] + + # cc https://datatracker.ietf.org/doc/html/rfc9460#keys SUPPORTED_PARAMS = { - 'no-default-alpn': {'has_arg': False}, + 'no-default-alpn': {'key_num': 2, 'has_arg': False}, 'alpn': { + 'key_num': 1, 'validate': validate_svcparam_alpn, 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, }, - 'port': {'validate': validate_svcparam_port}, + 'port': {'key_num': 3, 'validate': validate_svcparam_port}, 'ipv4hint': { + 'key_num': 4, 'validate': validate_svcparam_ipv4hint, 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, }, 'ipv6hint': { + 'key_num': 6, 'validate': validate_svcparam_ipv6hint, 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, }, 'mandatory': { + 'key_num': 0, 'validate': validate_svcparam_mandatory, 'parse_rdata_text': parse_rdata_text_svcparamvalue_list, }, - 'ech': {'validate': validate_svcparam_ech}, + 'ech': {'key_num': 5, 'validate': validate_svcparam_ech}, } @@ -264,9 +274,10 @@ class SvcbValue(EqualityTupleMixin, dict): @property def rdata_text(self): params = '' - # XXX: sort params by key number - for svcparamkey, svcparamvalue in self.svcparams.items(): + sorted_svcparamkeys = sorted(self.svcparams, key=svcparamkeysort) + for svcparamkey in sorted_svcparamkeys: params += f' {svcparamkey}' + svcparamvalue = self.svcparams.get(svcparamkey, None) if svcparamvalue is not None: if isinstance(svcparamvalue, list): params += f'={",".join(svcparamvalue)}' diff --git a/tests/test_octodns_record_svcb.py b/tests/test_octodns_record_svcb.py index 51c25ec..6566f16 100644 --- a/tests/test_octodns_record_svcb.py +++ b/tests/test_octodns_record_svcb.py @@ -164,7 +164,7 @@ class TestRecordSvcb(TestCase): self.assertEqual({'port': '8080'}, a.values[0].svcparams) # both directions should match - rdata = '1 svcb.unit.tests. port=8080 no-default-alpn ipv4hint=192.0.2.2,192.0.2.53' + rdata = '1 svcb.unit.tests. no-default-alpn port=8080 ipv4hint=192.0.2.2,192.0.2.53 key3333=foobar' record = SvcbRecord( zone, 'svc', {'ttl': 32, 'value': SvcbValue.parse_rdata_text(rdata)} ) From 83a7e057538f36d4b359ecd681624d59b574c0f5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 5 Jun 2024 14:26:29 -0700 Subject: [PATCH 6/8] changelog entry for SVCB & HTTP records --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f727f41..c883a1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## v1.?.? - 2024-??-?? - ??? +* Add support for SVCB and HTTPS records * Allow DS records to be specified for managed sub-zones, same as NS * Fix CAA rdata parsing to allow values with tags From 214c859a1e6425ba4bac7fb446d52b2eba1132cd Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Thu, 6 Jun 2024 09:09:05 +0200 Subject: [PATCH 7/8] SCVB: Fix issue with coverage and older python versions --- octodns/record/svcb.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py index 348601f..4562786 100644 --- a/octodns/record/svcb.py +++ b/octodns/record/svcb.py @@ -159,15 +159,11 @@ class SvcbValue(EqualityTupleMixin, dict): raise RrParseError(f'{paramkey} is specified twice') if len(paramvalue) != 0: params[paramkey] = paramvalue[0] - if ( - SUPPORTED_PARAMS.get(paramkey, {}).get( - 'parse_rdata_text', None - ) - is not None - ): - params[paramkey] = SUPPORTED_PARAMS[paramkey][ - 'parse_rdata_text' - ](paramvalue[0]) + parse_rdata_text = SUPPORTED_PARAMS.get(paramkey, {}).get( + 'parse_rdata_text', None + ) + if parse_rdata_text is not None: + params[paramkey] = parse_rdata_text(paramvalue[0]) continue params[paramkey] = None return { From df85637d300262f53625396b56889b2cda45edfb Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 6 Jun 2024 15:47:40 -0700 Subject: [PATCH 8/8] Replace contine with else, to fix coverage on 3.8/9 --- octodns/record/svcb.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/record/svcb.py b/octodns/record/svcb.py index 4562786..904c912 100644 --- a/octodns/record/svcb.py +++ b/octodns/record/svcb.py @@ -164,8 +164,8 @@ class SvcbValue(EqualityTupleMixin, dict): ) if parse_rdata_text is not None: params[paramkey] = parse_rdata_text(paramvalue[0]) - continue - params[paramkey] = None + else: + params[paramkey] = None return { 'svcpriority': svcpriority, 'targetname': targetname,