From 62ce979c71f8e397eb19e23beab46f438958eab3 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Tue, 9 Nov 2021 17:19:20 +0100 Subject: [PATCH 01/11] Ref #575 - Add new configuration settings for set validation --- docs/releases/4.3.0.rst | 5 ++ irrd/conf/__init__.py | 118 ++++++--------------------- irrd/conf/known_keys.py | 91 +++++++++++++++++++++ irrd/conf/test_conf.py | 29 ++++++- irrd/integration_tests/run.py | 15 ++++ irrd/rpsl/rpsl_objects.py | 40 +++++---- irrd/rpsl/tests/test_rpsl_objects.py | 8 +- irrd/updates/tests/test_parser.py | 2 +- 8 files changed, 193 insertions(+), 115 deletions(-) create mode 100644 docs/releases/4.3.0.rst create mode 100644 irrd/conf/known_keys.py diff --git a/docs/releases/4.3.0.rst b/docs/releases/4.3.0.rst new file mode 100644 index 000000000..b3dad79d5 --- /dev/null +++ b/docs/releases/4.3.0.rst @@ -0,0 +1,5 @@ +============================ +Release notes for IRRd 4.3.0 +============================ + +* removal of compatibility.permit_non_hierarchical_as_set_name diff --git a/irrd/conf/__init__.py b/irrd/conf/__init__.py index 32071e288..32f51b103 100644 --- a/irrd/conf/__init__.py +++ b/irrd/conf/__init__.py @@ -19,88 +19,7 @@ PASSWORD_HASH_DUMMY_VALUE = 'DummyValue' SOURCE_NAME_RE = re.compile('^[A-Z][A-Z0-9-]*[A-Z0-9]$') RPKI_IRR_PSEUDO_SOURCE = 'RPKI' - -# Note that sources are checked separately, -# and 'access_lists' is always permitted -KNOWN_CONFIG_KEYS = DottedDict({ - 'database_url': {}, - 'database_readonly': {}, - 'redis_url': {}, - 'piddir': {}, - 'user': {}, - 'group': {}, - 'server': { - 'http': { - 'interface': {}, - 'port': {}, - 'status_access_list': {}, - 'workers': {}, - 'forwarded_allowed_ips': {}, - }, - 'whois': { - 'interface': {}, - 'port': {}, - 'access_list': {}, - 'max_connections': {}, - }, - }, - 'email': { - 'from': {}, - 'footer': {}, - 'smtp': {}, - 'recipient_override': {}, - 'notification_header': {}, - }, - 'auth': { - 'override_password': {}, - 'authenticate_related_mntners': {}, - 'gnupg_keyring': {}, - }, - 'rpki': { - 'roa_source': {}, - 'roa_import_timer': {}, - 'slurm_source': {}, - 'pseudo_irr_remarks': {}, - 'notify_invalid_enabled': {}, - 'notify_invalid_subject': {}, - 'notify_invalid_header': {}, - }, - 'scopefilter': { - 'prefixes': {}, - 'asns': {}, - }, - 'log': { - 'logfile_path': {}, - 'level': {}, - 'logging_config_path': {}, - }, - 'sources_default': {}, - 'compatibility': { - 'inetnum_search_disabled': {}, - 'irrd42_migration_in_progress': {}, - 'permit_non_hierarchical_as_set_name': {}, - 'ipv4_only_route_set_members': {}, - } -}) - -KNOWN_SOURCES_KEYS = { - 'authoritative', - 'keep_journal', - 'nrtm_host', - 'nrtm_port', - 'import_source', - 'import_serial_source', - 'import_timer', - 'object_class_filter', - 'export_destination', - 'export_destination_unfiltered', - 'export_timer', - 'nrtm_access_list', - 'nrtm_access_list_unfiltered', - 'strict_import_keycert_objects', - 'rpki_excluded', - 'scopefilter_excluded', -} +VALID_SET_AUTNUM_AUTHENTICATION = ['disabled', 'opportunistic', 'required'] LOGGING = { 'version': 1, @@ -162,6 +81,9 @@ def __init__(self, user_config_path: Optional[str]=None, commit=True): Load the default config and load and check the user provided config. If a logfile was specified, direct logs there. """ + from .known_keys import KNOWN_CONFIG_KEYS, KNOWN_SOURCES_KEYS + self.known_config_keys = KNOWN_CONFIG_KEYS + self.known_sources_keys = KNOWN_SOURCES_KEYS self.user_config_path = user_config_path if user_config_path else CONFIG_PATH_DEFAULT default_config_path = str(Path(__file__).resolve().parents[0] / 'default_config.yaml') default_config_yaml = yaml.safe_load(open(default_config_path)) @@ -211,10 +133,10 @@ def get_setting_live(self, setting_name: str, default: Any=None) -> Any: """ if setting_name.startswith('sources'): components = setting_name.split('.') - if len(components) == 3 and components[2] not in KNOWN_SOURCES_KEYS: + if len(components) == 3 and components[2] not in self.known_sources_keys: raise ValueError(f'Unknown setting {setting_name}') elif not setting_name.startswith('access_lists'): - if KNOWN_CONFIG_KEYS.get(setting_name) is None: + if self.known_config_keys.get(setting_name) is None: raise ValueError(f'Unknown setting {setting_name}') env_key = 'IRRD_' + setting_name.upper().replace('.', '_') @@ -289,18 +211,24 @@ def _check_staging_config(self) -> List[str]: errors = [] config = self.user_config_staging - for key, value in config.items(): - if key in ['sources', 'access_lists']: - continue - known = KNOWN_CONFIG_KEYS.get(key) - if known is None: - errors.append(f'Unknown setting key: {key}') + def _validate_subconfig(key, value): if hasattr(value, 'items'): for key2, value2 in value.items(): subkey = key + '.' + key2 - known_sub = KNOWN_CONFIG_KEYS.get(subkey) + known_sub = self.known_config_keys.get(subkey) + if known_sub is None: errors.append(f'Unknown setting key: {subkey}') + _validate_subconfig(subkey, value2) + + for key, value in config.items(): + if key in ['sources', 'access_lists']: + continue + known = self.known_config_keys.get(key) + + if known is None: + errors.append(f'Unknown setting key: {key}') + _validate_subconfig(key, value) if not self._check_is_str(config, 'database_url'): errors.append('Setting database_url is required.') @@ -337,6 +265,12 @@ def _check_staging_config(self) -> List[str]: if not self._check_is_str(config, 'auth.gnupg_keyring'): errors.append('Setting auth.gnupg_keyring is required.') + for set_name, params in config.get('auth.set_creation', {}).items(): + if not isinstance(params.get('prefix_required', False), bool): + errors.append(f'Setting auth.set_creation.{set_name}.prefix_required must be a bool') + if params.get('autnum_authentication') and params['autnum_authentication'].lower() not in VALID_SET_AUTNUM_AUTHENTICATION: + errors.append(f'Setting auth.set_creation.{set_name}.autnum_authentication must be one of {VALID_SET_AUTNUM_AUTHENTICATION} if set') + for name, access_list in config.get('access_lists', {}).items(): for item in access_list: try: @@ -365,7 +299,7 @@ def _check_staging_config(self) -> List[str]: has_authoritative_sources = False for name, details in config.get('sources', {}).items(): - unknown_keys = set(details.keys()) - KNOWN_SOURCES_KEYS + unknown_keys = set(details.keys()) - self.known_sources_keys if unknown_keys: errors.append(f'Unknown key(s) under source {name}: {", ".join(unknown_keys)}') if details.get('authoritative'): diff --git a/irrd/conf/known_keys.py b/irrd/conf/known_keys.py new file mode 100644 index 000000000..e3610758e --- /dev/null +++ b/irrd/conf/known_keys.py @@ -0,0 +1,91 @@ +from irrd.vendor.dotted.collection import DottedDict +from irrd.rpsl.rpsl_objects import OBJECT_CLASS_MAPPING, RPSLSet + +# Note that sources are checked separately, +# and 'access_lists' is always permitted +KNOWN_CONFIG_KEYS = DottedDict({ + 'database_url': {}, + 'database_readonly': {}, + 'redis_url': {}, + 'piddir': {}, + 'user': {}, + 'group': {}, + 'server': { + 'http': { + 'interface': {}, + 'port': {}, + 'status_access_list': {}, + 'workers': {}, + 'forwarded_allowed_ips': {}, + }, + 'whois': { + 'interface': {}, + 'port': {}, + 'access_list': {}, + 'max_connections': {}, + }, + }, + 'email': { + 'from': {}, + 'footer': {}, + 'smtp': {}, + 'recipient_override': {}, + 'notification_header': {}, + }, + 'auth': { + 'override_password': {}, + 'authenticate_related_mntners': {}, + 'gnupg_keyring': {}, + 'set_creation': { + rpsl_object_class: {'prefix_required': {}, 'autnum_authentication': {}} + for rpsl_object_class in [ + set_object.rpsl_object_class + for set_object in OBJECT_CLASS_MAPPING.values() + if issubclass(set_object, RPSLSet) + ] + ['DEFAULT'] + }, + }, + 'rpki': { + 'roa_source': {}, + 'roa_import_timer': {}, + 'slurm_source': {}, + 'pseudo_irr_remarks': {}, + 'notify_invalid_enabled': {}, + 'notify_invalid_subject': {}, + 'notify_invalid_header': {}, + }, + 'scopefilter': { + 'prefixes': {}, + 'asns': {}, + }, + 'log': { + 'logfile_path': {}, + 'level': {}, + 'logging_config_path': {}, + }, + 'sources_default': {}, + 'compatibility': { + 'inetnum_search_disabled': {}, + 'irrd42_migration_in_progress': {}, + 'ipv4_only_route_set_members': {}, + } +}) + +KNOWN_SOURCES_KEYS = { + 'authoritative', + 'keep_journal', + 'nrtm_host', + 'nrtm_port', + 'import_source', + 'import_serial_source', + 'import_timer', + 'object_class_filter', + 'export_destination', + 'export_destination_unfiltered', + 'export_timer', + 'nrtm_access_list', + 'nrtm_access_list_unfiltered', + 'strict_import_keycert_objects', + 'rpki_excluded', + 'scopefilter_excluded', +} diff --git a/irrd/conf/test_conf.py b/irrd/conf/test_conf.py index 7a98c9032..79339fdd0 100644 --- a/irrd/conf/test_conf.py +++ b/irrd/conf/test_conf.py @@ -73,7 +73,18 @@ def test_load_valid_reload_valid_config(self, monkeypatch, save_yaml_config, tmp } }, 'auth': { - 'gnupg_keyring': str(tmpdir) + 'gnupg_keyring': str(tmpdir), + 'authenticate_related_mntners': True, + 'set_creation': { + 'as-set': { + 'prefix_required': True, + 'autnum_authentication': 'opportunistic', + }, + 'DEFAULT': { + 'prefix_required': True, + 'autnum_authentication': 'required', + }, + }, }, 'sources_default': ['TESTDB2', 'TESTDB'], 'sources': { @@ -188,7 +199,7 @@ def test_load_valid_reload_invalid_config(self, save_yaml_config, tmpdir, caplog } }, 'auth': { - 'gnupg_keyring': str(tmpdir) + 'gnupg_keyring': str(tmpdir), }, 'rpki': { 'roa_source': 'https://example.com/roa.json', @@ -241,6 +252,17 @@ def test_load_invalid_config(self, save_yaml_config, tmpdir): '192.0.2.2.1' }, }, + 'auth': { + 'set_creation': { + 'as-set': { + 'prefix_required': 'not-a-bool', + 'autnum_authentication': 'unknown-value', + }, + 'not-a-real-set': { + 'prefix_required': True, + }, + }, + }, 'rpki': { 'roa_source': 'https://example.com/roa.json', 'roa_import_timer': 'foo', @@ -297,6 +319,9 @@ def test_load_invalid_config(self, save_yaml_config, tmpdir): assert 'Setting email.recipient_override must be an email address if set.' in str(ce.value) assert 'Settings user and group must both be defined, or neither.' in str(ce.value) assert 'Setting auth.gnupg_keyring is required.' in str(ce.value) + assert 'Unknown setting key: auth.set_creation.not-a-real-set.prefix_required' in str(ce.value) + assert 'Setting auth.set_creation.as-set.prefix_required must be a bool' in str(ce.value) + assert 'Setting auth.set_creation.as-set.autnum_authentication must be one of' in str(ce.value) assert 'Access lists doesnotexist, invalid-list referenced in settings, but not defined.' in str(ce.value) assert 'Setting server.http.status_access_list must be a string, if defined.' in str(ce.value) assert 'Invalid item in access list bad-list: IPv4 Address with more than 4 bytes.' in str(ce.value) diff --git a/irrd/integration_tests/run.py b/irrd/integration_tests/run.py index 9a1513a42..c5025fbac 100644 --- a/irrd/integration_tests/run.py +++ b/irrd/integration_tests/run.py @@ -765,6 +765,21 @@ def _start_irrds(self): 'auth': { 'gnupg_keyring': None, 'override_password': '$1$J6KycItM$MbPaBU6iFSGFV299Rk7Di0', + 'set_creation': { + 'filter-set': { + 'prefix_required': False, + }, + 'peering-set': { + 'prefix_required': False, + }, + 'route-set': { + 'prefix_required': False, + }, + 'rtr-set': { + 'prefix_required': False, + }, + }, + }, 'email': { diff --git a/irrd/rpsl/rpsl_objects.py b/irrd/rpsl/rpsl_objects.py index e97651803..3b58c5439 100644 --- a/irrd/rpsl/rpsl_objects.py +++ b/irrd/rpsl/rpsl_objects.py @@ -31,6 +31,23 @@ def rpsl_object_from_text(text, strict_validation=True, default_source: Optional return klass(from_text=text, strict_validation=strict_validation, default_source=default_source) +class RPSLSet(RPSLObject): + def clean_for_create(self) -> bool: + if get_setting(f'auth.set_creation.{self.rpsl_object_class}.prefix_required') is False: + return True + if get_setting('auth.set_creation.DEFAULT.prefix_required') is False: + return True + + first_segment = self.pk().split(':')[0] + try: + parse_as_number(first_segment) + return True + except ValidationError as ve: + self.messages.error(f'{self.rpsl_object_class} names must be hierarchical and the first ' + f'component must be an AS number, e.g. "AS65537:{first_segment}": {str(ve)}') + return False + + class RPSLAsBlock(RPSLObject): fields = OrderedDict([ ('as-block', RPSLASBlockField(primary_key=True, lookup_key=True)), @@ -45,7 +62,7 @@ class RPSLAsBlock(RPSLObject): ]) -class RPSLAsSet(RPSLObject): +class RPSLAsSet(RPSLSet): fields = OrderedDict([ ('as-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='AS')), ('descr', RPSLTextField(multiple=True, optional=True)), @@ -60,19 +77,6 @@ class RPSLAsSet(RPSLObject): ('source', RPSLGenericNameField()), ]) - def clean_for_create(self) -> bool: - if get_setting('compatibility.permit_non_hierarchical_as_set_name'): - return True - - first_segment = self.pk().split(':')[0] - try: - parse_as_number(first_segment) - return True - except ValidationError as ve: - self.messages.error('AS set names must be hierarchical and the first component must ' - f'be an AS number, e.g. "AS65537:AS-EXAMPLE": {str(ve)}') - return False - class RPSLAutNum(RPSLObject): fields = OrderedDict([ @@ -117,7 +121,7 @@ class RPSLDomain(RPSLObject): ]) -class RPSLFilterSet(RPSLObject): +class RPSLFilterSet(RPSLSet): fields = OrderedDict([ ('filter-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='FLTR')), ('descr', RPSLTextField(multiple=True, optional=True)), @@ -355,7 +359,7 @@ def _auth_lines(self, password_hashes=True) -> List[Union[str, List[str]]]: return [auth for auth in lines if ' ' not in auth] -class RPSLPeeringSet(RPSLObject): +class RPSLPeeringSet(RPSLSet): fields = OrderedDict([ ('peering-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='PRNG')), ('descr', RPSLTextField(multiple=True, optional=True)), @@ -432,7 +436,7 @@ class RPSLRoute(RPSLObject): ]) -class RPSLRouteSet(RPSLObject): +class RPSLRouteSet(RPSLSet): fields = OrderedDict([ ('route-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='RS')), ('members', RPSLRouteSetMembersField(ip_version=4, lookup_key=True, optional=True, multiple=True)), @@ -475,7 +479,7 @@ class RPSLRoute6(RPSLObject): ]) -class RPSLRtrSet(RPSLObject): +class RPSLRtrSet(RPSLSet): fields = OrderedDict([ ('rtr-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='RTRS')), ('descr', RPSLTextField(multiple=True, optional=True)), diff --git a/irrd/rpsl/tests/test_rpsl_objects.py b/irrd/rpsl/tests/test_rpsl_objects.py index e81f02e65..7c4ffdc27 100644 --- a/irrd/rpsl/tests/test_rpsl_objects.py +++ b/irrd/rpsl/tests/test_rpsl_objects.py @@ -154,9 +154,13 @@ def test_clean_for_create(self, config_override): assert obj.__class__ == RPSLAsSet assert not obj.messages.errors() assert not obj.clean_for_create() - assert 'AS set names must be hierarchical and the first ' in obj.messages.errors()[0] + assert 'as-set names must be hierarchical and the first ' in obj.messages.errors()[0] - config_override({'compatibility': {'permit_non_hierarchical_as_set_name': True}}) + config_override({'auth': {'set_creation': {'as-set': {'prefix_required': False}}}}) + obj = rpsl_object_from_text(rpsl_text) + assert obj.clean_for_create() + + config_override({'auth': {'set_creation': {'DEFAULT': {'prefix_required': False}}}}) obj = rpsl_object_from_text(rpsl_text) assert obj.clean_for_create() diff --git a/irrd/updates/tests/test_parser.py b/irrd/updates/tests/test_parser.py index 430a07859..c9e4cb5e0 100644 --- a/irrd/updates/tests/test_parser.py +++ b/irrd/updates/tests/test_parser.py @@ -133,7 +133,7 @@ def test_validates_for_create(self, prepare_mocks): assert not result.validate() assert result.status == UpdateRequestStatus.ERROR_PARSING assert len(result.error_messages) == 1 - assert 'AS set names must be hierarchical and the first' in result.error_messages[0] + assert 'as-set names must be hierarchical and the first' in result.error_messages[0] # Test again with an UPDATE (which then fails on auth to stop) mock_dh.execute_query = lambda query: [{'object_text': SAMPLE_AS_SET}] From 5df272af42753bd4a1b6958de6b5629a95fae3b4 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 10 Nov 2021 14:56:25 +0100 Subject: [PATCH 02/11] Add rough implementation of autnum_authentication --- irrd/conf/__init__.py | 8 +- irrd/rpsl/parser.py | 1 + irrd/rpsl/rpsl_objects.py | 20 ++-- irrd/updates/parser_state.py | 18 ++++ irrd/updates/tests/test_validators.py | 146 +++++++++++++++++++++++++- irrd/updates/validators.py | 44 +++++++- 6 files changed, 217 insertions(+), 20 deletions(-) diff --git a/irrd/conf/__init__.py b/irrd/conf/__init__.py index 32f51b103..3117e7b03 100644 --- a/irrd/conf/__init__.py +++ b/irrd/conf/__init__.py @@ -19,7 +19,7 @@ PASSWORD_HASH_DUMMY_VALUE = 'DummyValue' SOURCE_NAME_RE = re.compile('^[A-Z][A-Z0-9-]*[A-Z0-9]$') RPKI_IRR_PSEUDO_SOURCE = 'RPKI' -VALID_SET_AUTNUM_AUTHENTICATION = ['disabled', 'opportunistic', 'required'] + LOGGING = { 'version': 1, @@ -265,11 +265,13 @@ def _validate_subconfig(key, value): if not self._check_is_str(config, 'auth.gnupg_keyring'): errors.append('Setting auth.gnupg_keyring is required.') + from irrd.updates.parser_state import RPSLSetAutnumAuthenticationMode for set_name, params in config.get('auth.set_creation', {}).items(): if not isinstance(params.get('prefix_required', False), bool): errors.append(f'Setting auth.set_creation.{set_name}.prefix_required must be a bool') - if params.get('autnum_authentication') and params['autnum_authentication'].lower() not in VALID_SET_AUTNUM_AUTHENTICATION: - errors.append(f'Setting auth.set_creation.{set_name}.autnum_authentication must be one of {VALID_SET_AUTNUM_AUTHENTICATION} if set') + valid_auth = [mode.value for mode in RPSLSetAutnumAuthenticationMode] + if params.get('autnum_authentication') and params['autnum_authentication'].lower() not in valid_auth: + errors.append(f'Setting auth.set_creation.{set_name}.autnum_authentication must be one of {valid_auth} if set') for name, access_list in config.get('access_lists', {}).items(): for item in access_list: diff --git a/irrd/rpsl/parser.py b/irrd/rpsl/parser.py index 02c7b4588..95ebdd24e 100644 --- a/irrd/rpsl/parser.py +++ b/irrd/rpsl/parser.py @@ -68,6 +68,7 @@ class RPSLObject(metaclass=RPSLObjectMeta): prefix_length: Optional[int] = None rpki_status: RPKIStatus = RPKIStatus.not_found scopefilter_status: ScopeFilterStatus = ScopeFilterStatus.in_scope + pk_first_segment: Optional[str] = None default_source: Optional[str] = None # noqa: E704 (flake8 bug) # Whether this object has a relation to RPKI ROA data, and therefore RPKI # checks should be performed in certain scenarios. Enabled for route/route6. diff --git a/irrd/rpsl/rpsl_objects.py b/irrd/rpsl/rpsl_objects.py index 3b58c5439..e712b523e 100644 --- a/irrd/rpsl/rpsl_objects.py +++ b/irrd/rpsl/rpsl_objects.py @@ -33,19 +33,21 @@ def rpsl_object_from_text(text, strict_validation=True, default_source: Optional class RPSLSet(RPSLObject): def clean_for_create(self) -> bool: - if get_setting(f'auth.set_creation.{self.rpsl_object_class}.prefix_required') is False: - return True - if get_setting('auth.set_creation.DEFAULT.prefix_required') is False: - return True - - first_segment = self.pk().split(':')[0] + # TODO: validate this attribute in the tests + self.pk_first_segment = self.pk().split(':')[0] try: - parse_as_number(first_segment) + parse_as_number(self.pk_first_segment) return True except ValidationError as ve: + self.pk_first_segment = None + if get_setting(f'auth.set_creation.{self.rpsl_object_class}.prefix_required') is False: + return True + if get_setting('auth.set_creation.DEFAULT.prefix_required') is False: + return True self.messages.error(f'{self.rpsl_object_class} names must be hierarchical and the first ' - f'component must be an AS number, e.g. "AS65537:{first_segment}": {str(ve)}') - return False + f'component must be an AS number, e.g. "AS65537:{self.pk_first_segment}": {str(ve)}') + + return False class RPSLAsBlock(RPSLObject): diff --git a/irrd/updates/parser_state.py b/irrd/updates/parser_state.py index 4ecac299e..ea0546e14 100644 --- a/irrd/updates/parser_state.py +++ b/irrd/updates/parser_state.py @@ -1,5 +1,7 @@ from enum import unique, Enum +from irrd.conf import get_setting + @unique class UpdateRequestType(Enum): @@ -19,3 +21,19 @@ class UpdateRequestStatus(Enum): ERROR_ROA = 'error: conflict with existing ROA' ERROR_SCOPEFILTER = 'error: not in scope' ERROR_NON_AUTHORITIVE = 'error: attempt to update object in non-authoritive database' + + +@unique +class RPSLSetAutnumAuthenticationMode(Enum): + DISABLED = 'disabled' + OPPORTUNISTIC = 'opportunistic' + REQUIRED = 'required' + + @staticmethod + def for_set_name(set_name: str): + setting = get_setting(f'auth.set_creation.{set_name}.autnum_authentication') + if not setting: + setting = get_setting('auth.set_creation.DEFAULT.autnum_authentication') + if not setting: + return RPSLSetAutnumAuthenticationMode.DISABLED + return getattr(RPSLSetAutnumAuthenticationMode, setting.upper()) diff --git a/irrd/updates/tests/test_validators.py b/irrd/updates/tests/test_validators.py index c47bbb84d..d2120b748 100644 --- a/irrd/updates/tests/test_validators.py +++ b/irrd/updates/tests/test_validators.py @@ -6,7 +6,7 @@ from pytest import raises from irrd.rpsl.rpsl_objects import rpsl_object_from_text -from irrd.utils.rpsl_samples import (SAMPLE_MNTNER, SAMPLE_MNTNER_CRYPT, +from irrd.utils.rpsl_samples import (SAMPLE_AS_SET, SAMPLE_FILTER_SET, SAMPLE_MNTNER, SAMPLE_MNTNER_CRYPT, SAMPLE_MNTNER_MD5, SAMPLE_PERSON, SAMPLE_ROUTE, SAMPLE_ROUTE6) from irrd.utils.test_utils import flatten_mock_calls @@ -146,7 +146,6 @@ def test_existing_person_mntner_change(self, prepare_mocks): validator.passwords = [SAMPLE_MNTNER_MD5] result = validator.process_auth(person_new, person_old) assert not result.is_valid() - print(result.error_messages) assert result.error_messages == {'Authorisation for person PERSON-TEST failed: ' 'must be authenticated by one of: TEST-MNT, TEST-NEW-MNT'} @@ -244,7 +243,7 @@ def test_modify_mntner(self, prepare_mocks, config_override): } -class TestAuthValidatorRelatedObjects: +class TestAuthValidatorRelatedRouteObjects: def test_related_route_exact_inetnum(self, prepare_mocks, config_override): validator, mock_dq, mock_dh = prepare_mocks route = rpsl_object_from_text(SAMPLE_ROUTE) @@ -413,6 +412,7 @@ def test_related_route_no_match_v6(self, prepare_mocks, config_override): result = validator.process_auth(route, None) assert result.is_valid() + # TODO: this grouping is confusing re sources assert flatten_mock_calls(mock_dq, flatten_objects=True) == [ ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], @@ -433,3 +433,143 @@ def test_related_route_no_match_v6(self, prepare_mocks, config_override): ['first_only', (), {}], ['ip_less_specific_one_level', ('2001:db8::/48',), {}], ] + + +class TestAuthValidatorRelatedAutNumObjects: + def test_as_set_autnum_disabled(self, prepare_mocks, config_override): + config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'disabled'}}}}) + validator, mock_dq, mock_dh = prepare_mocks + as_set = rpsl_object_from_text(SAMPLE_AS_SET) + assert as_set.clean_for_create() # fill pk_first_segment + mock_dh.execute_query = lambda q: [ + {'object_text': SAMPLE_MNTNER.replace('MD5', '')}, # mntner for object + ] + + validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT] + result = validator.process_auth(as_set, None) + assert result.is_valid() + assert flatten_mock_calls(mock_dq, flatten_objects=True) == [ + ['sources', (['TEST'],), {}], + ['object_classes', (['mntner'],), {}], + ['rpsl_pks', ({'TEST-MNT'},), {}], + ] + + def test_as_set_autnum_opportunistic_exists(self, prepare_mocks, config_override): + config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'opportunistic'}}}}) + validator, mock_dq, mock_dh = prepare_mocks + as_set = rpsl_object_from_text(SAMPLE_AS_SET) + assert as_set.clean_for_create() # fill pk_first_segment + query_results = itertools.cycle([ + [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{ + # attempt to look for matching aut-num + 'object_class': 'aut-num', + 'rpsl_pk': 'AS655375', + 'parsed_data': {'mnt-by': ['RELATED-MNT']} + }], + [{'object_text': SAMPLE_MNTNER.replace('CRYPT', '')}], # related mntner retrieval + ]) + mock_dh.execute_query = lambda q: next(query_results) + + validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT] + result = validator.process_auth(as_set, None) + assert result.is_valid() + assert flatten_mock_calls(mock_dq, flatten_objects=True) == [ + ['sources', (['TEST'],), {}], + ['object_classes', (['mntner'],), {}], + ['rpsl_pks', ({'TEST-MNT'},), {}], + + ['sources', (['TEST'],), {}], + ['object_classes', (['aut-num'],), {}], + ['first_only', (), {}], + ['rpsl_pk', ('AS65537',), {}], + + ['sources', (['TEST'],), {}], + ['object_classes', (['mntner'],), {}], + ['rpsl_pks', ({'RELATED-MNT'},), {}] + ] + + validator = AuthValidator(mock_dh, None) + validator.passwords = [SAMPLE_MNTNER_CRYPT] # related only has MD5, so this is invalid + result = validator.process_auth(as_set, None) + assert not result.is_valid() + assert result.error_messages == { + 'Authorisation for as-set AS65537:AS-SETTEST failed: must be authenticated by one of: ' + 'RELATED-MNT - from parent aut-num AS655375' + } + + result = validator.process_auth(as_set, as_set) + assert result.is_valid() + + config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'disabled'}}}}) + result = validator.process_auth(as_set, None) + assert result.is_valid() + + # Default is disabled + config_override({}) + result = validator.process_auth(as_set, None) + assert result.is_valid() + + def test_as_set_autnum_opportunistic_does_not_exist(self, prepare_mocks, config_override): + config_override({'auth': {'set_creation': {'DEFAULT': {'autnum_authentication': 'opportunistic'}}}}) + validator, mock_dq, mock_dh = prepare_mocks + as_set = rpsl_object_from_text(SAMPLE_AS_SET) + assert as_set.clean_for_create() # fill pk_first_segment + query_results = itertools.cycle([ + [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [], # attempt to look for matching aut-num + ]) + mock_dh.execute_query = lambda q: next(query_results) + + validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT] + result = validator.process_auth(as_set, None) + assert result.is_valid() + assert flatten_mock_calls(mock_dq, flatten_objects=True) == [ + ['sources', (['TEST'],), {}], + ['object_classes', (['mntner'],), {}], + ['rpsl_pks', ({'TEST-MNT'},), {}], + + ['sources', (['TEST'],), {}], + ['object_classes', (['aut-num'],), {}], + ['first_only', (), {}], + ['rpsl_pk', ('AS65537',), {}], + ] + + def test_as_set_autnum_required_does_not_exist(self, prepare_mocks, config_override): + config_override({'auth': {'set_creation': {'DEFAULT': {'autnum_authentication': 'required'}}}}) + validator, mock_dq, mock_dh = prepare_mocks + as_set = rpsl_object_from_text(SAMPLE_AS_SET) + assert as_set.clean_for_create() # fill pk_first_segment + query_results = itertools.cycle([ + [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [], # attempt to look for matching aut-num + ]) + mock_dh.execute_query = lambda q: next(query_results) + + validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT] + result = validator.process_auth(as_set, None) + assert not result.is_valid() + assert result.error_messages == { + 'Creating this object requires an aut-num for AS65537 to exist.', + } + + def test_filter_set_autnum_required_no_prefix(self, prepare_mocks, config_override): + config_override({'auth': {'set_creation': {'DEFAULT': { + 'autnum_authentication': 'required', + 'prefix_required': False, + }}}}) + validator, mock_dq, mock_dh = prepare_mocks + filter_set = rpsl_object_from_text(SAMPLE_FILTER_SET) + assert filter_set.clean_for_create() + mock_dh.execute_query = lambda q: [ + {'object_text': SAMPLE_MNTNER.replace('MD5', '')}, # mntner for object + ] + + validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT] + result = validator.process_auth(filter_set, None) + assert result.is_valid() + assert flatten_mock_calls(mock_dq, flatten_objects=True) == [ + ['sources', (['TEST'],), {}], + ['object_classes', (['mntner'],), {}], + ['rpsl_pks', ({'TEST-MNT'},), {}], + ] diff --git a/irrd/updates/validators.py b/irrd/updates/validators.py index c159f739e..ca38167a5 100644 --- a/irrd/updates/validators.py +++ b/irrd/updates/validators.py @@ -8,10 +8,10 @@ from irrd.conf import get_setting from irrd.rpsl.parser import RPSLObject -from irrd.rpsl.rpsl_objects import RPSLMntner, rpsl_object_from_text +from irrd.rpsl.rpsl_objects import RPSLMntner, rpsl_object_from_text, RPSLSet from irrd.storage.database_handler import DatabaseHandler from irrd.storage.queries import RPSLDatabaseQuery -from .parser_state import UpdateRequestType +from .parser_state import RPSLSetAutnumAuthenticationMode, UpdateRequestType if TYPE_CHECKING: # pragma: no cover # http://mypy.readthedocs.io/en/latest/common_issues.html#import-cycles @@ -43,6 +43,7 @@ class ReferenceValidator: in the same update message. To handle this, the validator can be preloaded with objects that should be considered valid. """ + def __init__(self, database_handler: DatabaseHandler) -> None: self.database_handler = database_handler self._cache: Set[Tuple[str, str, str]] = set() @@ -212,7 +213,7 @@ def process_auth(self, rpsl_obj_new: RPSLObject, rpsl_obj_current: Optional[RPSL else: result.mntners_notify = mntner_objs_new if get_setting('auth.authenticate_related_mntners'): - mntners_related = self._find_related_mntners(rpsl_obj_new) + mntners_related = self._find_related_mntners(rpsl_obj_new, result) if mntners_related: related_object_class, related_pk, related_mntner_list = mntners_related logger.debug(f'Checking auth for related object {related_object_class} / ' @@ -289,8 +290,7 @@ def _generate_failure_message(self, result: ValidatorResult, failed_mntner_list: msg += f' - from parent {related_object_class} {related_pk}' result.error_messages.add(msg) - @functools.lru_cache(maxsize=50) - def _find_related_mntners(self, rpsl_obj_new: RPSLObject) -> Optional[Tuple[str, str, List[str]]]: + def _find_related_mntners(self, rpsl_obj_new: RPSLObject, result: ValidatorResult) -> Optional[Tuple[str, str, List[str]]]: """ Find the maintainers of the related object to rpsl_obj_new, if any. This is used to authorise creating objects - authentication may be @@ -301,10 +301,14 @@ def _find_related_mntners(self, rpsl_obj_new: RPSLObject) -> Optional[Tuple[str, - PK of the related object - List of maintainers for the related object (at least one must pass) Returns None of no related objects were found that should be authenticated. + + Custom error messages may be added directly to the passed ValidatorResult. """ related_object = None if rpsl_obj_new.rpsl_object_class in ['route', 'route6']: related_object = self._find_related_object_route(rpsl_obj_new) + if issubclass(rpsl_obj_new.__class__, RPSLSet): + related_object = self._find_related_object_set(rpsl_obj_new, result) if related_object: mntners = related_object.get('parsed_data', {}).get('mnt-by', []) @@ -312,6 +316,7 @@ def _find_related_mntners(self, rpsl_obj_new: RPSLObject) -> Optional[Tuple[str, return None + @functools.lru_cache(maxsize=50) def _find_related_object_route(self, rpsl_obj_new: RPSLObject): """ Find the related inetnum/route object to rpsl_obj_new, which must be a route(6). @@ -341,6 +346,35 @@ def _find_related_object_route(self, rpsl_obj_new: RPSLObject): return None + def _find_related_object_set(self, rpsl_obj_new: RPSLObject, result: ValidatorResult): + """ + Find the related aut-num object to rpsl_obj_new, which must be a set object, + depending on settings. + Returns a dict as returned by the database handler. + """ + @functools.lru_cache(maxsize=50) + def _find_in_db(): + query = _init_related_object_query('aut-num', rpsl_obj_new).rpsl_pk(rpsl_obj_new.pk_first_segment) + aut_nums = list(self.database_handler.execute_query(query)) + if aut_nums: + return aut_nums[0] + + if not rpsl_obj_new.pk_first_segment: + return None + + mode = RPSLSetAutnumAuthenticationMode.for_set_name(rpsl_obj_new.rpsl_object_class) + if mode == RPSLSetAutnumAuthenticationMode.DISABLED: + return None + + aut_num = _find_in_db() + if aut_num: + return aut_num + elif mode == RPSLSetAutnumAuthenticationMode.REQUIRED: + result.error_messages.add( + f'Creating this object requires an aut-num for {rpsl_obj_new.pk_first_segment} to exist.' + ) + return None + def _init_related_object_query(rpsl_object_class: str, rpsl_obj_new: RPSLObject) -> RPSLDatabaseQuery: query = RPSLDatabaseQuery().sources([rpsl_obj_new.source()]) From c8da78728286c41f65f116dd350958f1eaa1d740 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 10 Nov 2021 16:36:15 +0100 Subject: [PATCH 03/11] cleanup --- irrd/rpsl/parser.py | 2 +- irrd/rpsl/rpsl_objects.py | 9 ++++----- irrd/rpsl/tests/test_rpsl_objects.py | 4 ++++ irrd/updates/tests/test_validators.py | 12 ++++++------ irrd/updates/validators.py | 6 +++--- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/irrd/rpsl/parser.py b/irrd/rpsl/parser.py index 95ebdd24e..70f54dddf 100644 --- a/irrd/rpsl/parser.py +++ b/irrd/rpsl/parser.py @@ -68,7 +68,7 @@ class RPSLObject(metaclass=RPSLObjectMeta): prefix_length: Optional[int] = None rpki_status: RPKIStatus = RPKIStatus.not_found scopefilter_status: ScopeFilterStatus = ScopeFilterStatus.in_scope - pk_first_segment: Optional[str] = None + pk_asn_segment: Optional[str] = None default_source: Optional[str] = None # noqa: E704 (flake8 bug) # Whether this object has a relation to RPKI ROA data, and therefore RPKI # checks should be performed in certain scenarios. Enabled for route/route6. diff --git a/irrd/rpsl/rpsl_objects.py b/irrd/rpsl/rpsl_objects.py index e712b523e..9b07dd8ea 100644 --- a/irrd/rpsl/rpsl_objects.py +++ b/irrd/rpsl/rpsl_objects.py @@ -33,19 +33,18 @@ def rpsl_object_from_text(text, strict_validation=True, default_source: Optional class RPSLSet(RPSLObject): def clean_for_create(self) -> bool: - # TODO: validate this attribute in the tests - self.pk_first_segment = self.pk().split(':')[0] + self.pk_asn_segment = self.pk().split(':')[0] try: - parse_as_number(self.pk_first_segment) + parse_as_number(self.pk_asn_segment) return True except ValidationError as ve: - self.pk_first_segment = None + self.pk_asn_segment = None if get_setting(f'auth.set_creation.{self.rpsl_object_class}.prefix_required') is False: return True if get_setting('auth.set_creation.DEFAULT.prefix_required') is False: return True self.messages.error(f'{self.rpsl_object_class} names must be hierarchical and the first ' - f'component must be an AS number, e.g. "AS65537:{self.pk_first_segment}": {str(ve)}') + f'component must be an AS number, e.g. "AS65537:{self.pk_asn_segment}": {str(ve)}') return False diff --git a/irrd/rpsl/tests/test_rpsl_objects.py b/irrd/rpsl/tests/test_rpsl_objects.py index 7c4ffdc27..a5a48db38 100644 --- a/irrd/rpsl/tests/test_rpsl_objects.py +++ b/irrd/rpsl/tests/test_rpsl_objects.py @@ -142,6 +142,7 @@ def test_parse(self): ] assert obj.references_strong_inbound() == set() assert obj.source() == 'TEST' + assert obj.pk_asn_segment == 'AS65537' assert obj.parsed_data['members'] == ['AS65538', 'AS65539', 'AS65537', 'AS-OTHERSET'] # Field parsing will cause our object to look slightly different than the original, hence the replace() @@ -154,15 +155,18 @@ def test_clean_for_create(self, config_override): assert obj.__class__ == RPSLAsSet assert not obj.messages.errors() assert not obj.clean_for_create() + assert not obj.pk_asn_segment assert 'as-set names must be hierarchical and the first ' in obj.messages.errors()[0] config_override({'auth': {'set_creation': {'as-set': {'prefix_required': False}}}}) obj = rpsl_object_from_text(rpsl_text) assert obj.clean_for_create() + assert not obj.pk_asn_segment config_override({'auth': {'set_creation': {'DEFAULT': {'prefix_required': False}}}}) obj = rpsl_object_from_text(rpsl_text) assert obj.clean_for_create() + assert not obj.pk_asn_segment class TestRPSLAutNum: diff --git a/irrd/updates/tests/test_validators.py b/irrd/updates/tests/test_validators.py index d2120b748..b32fbde8a 100644 --- a/irrd/updates/tests/test_validators.py +++ b/irrd/updates/tests/test_validators.py @@ -138,6 +138,7 @@ def test_existing_person_mntner_change(self, prepare_mocks): ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT', 'TEST-NEW-MNT'},), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-OLD-MNT'},), {}], # TEST-MNT is cached @@ -266,8 +267,8 @@ def test_related_route_exact_inetnum(self, prepare_mocks, config_override): ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['inetnum'],), {}], ['first_only', (), {}], ['ip_exact', ('192.0.2.0/24',), {}], @@ -367,8 +368,8 @@ def test_related_route_less_specific_route(self, prepare_mocks, config_override) ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['inetnum'],), {}], ['first_only', (), {}], ['ip_exact', ('192.0.2.0/24',), {}], @@ -412,13 +413,12 @@ def test_related_route_no_match_v6(self, prepare_mocks, config_override): result = validator.process_auth(route, None) assert result.is_valid() - # TODO: this grouping is confusing re sources assert flatten_mock_calls(mock_dq, flatten_objects=True) == [ ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['inet6num'],), {}], ['first_only', (), {}], ['ip_exact', ('2001:db8::/48',), {}], @@ -440,7 +440,7 @@ def test_as_set_autnum_disabled(self, prepare_mocks, config_override): config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'disabled'}}}}) validator, mock_dq, mock_dh = prepare_mocks as_set = rpsl_object_from_text(SAMPLE_AS_SET) - assert as_set.clean_for_create() # fill pk_first_segment + assert as_set.clean_for_create() # fill pk_asn_segment mock_dh.execute_query = lambda q: [ {'object_text': SAMPLE_MNTNER.replace('MD5', '')}, # mntner for object ] @@ -458,7 +458,7 @@ def test_as_set_autnum_opportunistic_exists(self, prepare_mocks, config_override config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'opportunistic'}}}}) validator, mock_dq, mock_dh = prepare_mocks as_set = rpsl_object_from_text(SAMPLE_AS_SET) - assert as_set.clean_for_create() # fill pk_first_segment + assert as_set.clean_for_create() # fill pk_asn_segment query_results = itertools.cycle([ [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object [{ diff --git a/irrd/updates/validators.py b/irrd/updates/validators.py index ca38167a5..e494dd5b8 100644 --- a/irrd/updates/validators.py +++ b/irrd/updates/validators.py @@ -354,12 +354,12 @@ def _find_related_object_set(self, rpsl_obj_new: RPSLObject, result: ValidatorRe """ @functools.lru_cache(maxsize=50) def _find_in_db(): - query = _init_related_object_query('aut-num', rpsl_obj_new).rpsl_pk(rpsl_obj_new.pk_first_segment) + query = _init_related_object_query('aut-num', rpsl_obj_new).rpsl_pk(rpsl_obj_new.pk_asn_segment) aut_nums = list(self.database_handler.execute_query(query)) if aut_nums: return aut_nums[0] - if not rpsl_obj_new.pk_first_segment: + if not rpsl_obj_new.pk_asn_segment: return None mode = RPSLSetAutnumAuthenticationMode.for_set_name(rpsl_obj_new.rpsl_object_class) @@ -371,7 +371,7 @@ def _find_in_db(): return aut_num elif mode == RPSLSetAutnumAuthenticationMode.REQUIRED: result.error_messages.add( - f'Creating this object requires an aut-num for {rpsl_obj_new.pk_first_segment} to exist.' + f'Creating this object requires an aut-num for {rpsl_obj_new.pk_asn_segment} to exist.' ) return None From 50be8c44f8240b55919502114118ce2ecc352797 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 10 Nov 2021 16:42:59 +0100 Subject: [PATCH 04/11] authenticate_related_mntners -> authenticate_parents_route_creation --- irrd/conf/default_config.yaml | 2 +- irrd/conf/known_keys.py | 2 +- irrd/conf/test_conf.py | 2 +- irrd/updates/tests/test_parser.py | 2 +- irrd/updates/tests/test_validators.py | 2 +- irrd/updates/validators.py | 24 +++++++++++++----------- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/irrd/conf/default_config.yaml b/irrd/conf/default_config.yaml index 9d095bbee..a99d7d22e 100644 --- a/irrd/conf/default_config.yaml +++ b/irrd/conf/default_config.yaml @@ -44,7 +44,7 @@ irrd: max_connections: 10 auth: gnupg_keyring: null - authenticate_related_mntners: true + authenticate_parents_route_creation: true email: footer: '' notification_header: | diff --git a/irrd/conf/known_keys.py b/irrd/conf/known_keys.py index e3610758e..6b326832e 100644 --- a/irrd/conf/known_keys.py +++ b/irrd/conf/known_keys.py @@ -34,7 +34,7 @@ }, 'auth': { 'override_password': {}, - 'authenticate_related_mntners': {}, + 'authenticate_parents_route_creation': {}, 'gnupg_keyring': {}, 'set_creation': { rpsl_object_class: {'prefix_required': {}, 'autnum_authentication': {}} diff --git a/irrd/conf/test_conf.py b/irrd/conf/test_conf.py index 79339fdd0..f661e8618 100644 --- a/irrd/conf/test_conf.py +++ b/irrd/conf/test_conf.py @@ -74,7 +74,7 @@ def test_load_valid_reload_valid_config(self, monkeypatch, save_yaml_config, tmp }, 'auth': { 'gnupg_keyring': str(tmpdir), - 'authenticate_related_mntners': True, + 'authenticate_parents_route_creation': True, 'set_creation': { 'as-set': { 'prefix_required': True, diff --git a/irrd/updates/tests/test_parser.py b/irrd/updates/tests/test_parser.py index c9e4cb5e0..70b100b58 100644 --- a/irrd/updates/tests/test_parser.py +++ b/irrd/updates/tests/test_parser.py @@ -700,7 +700,7 @@ def test_check_auth_invalid_update_mntner_override_hash_empty(self, prepare_mock assert 'Ignoring override password, auth.override_password not set.' in caplog.text def test_check_valid_related_mntners_disabled(self, prepare_mocks, config_override): - config_override({'auth': {'authenticate_related_mntners': False}}) + config_override({'auth': {'authenticate_parents_route_creation': False}}) mock_dq, mock_dh = prepare_mocks query_answers = [ diff --git a/irrd/updates/tests/test_validators.py b/irrd/updates/tests/test_validators.py index b32fbde8a..b0446898a 100644 --- a/irrd/updates/tests/test_validators.py +++ b/irrd/updates/tests/test_validators.py @@ -287,7 +287,7 @@ def test_related_route_exact_inetnum(self, prepare_mocks, config_override): 'RELATED-MNT - from parent inetnum 192.0.2.0-192.0.2.255' } - config_override({'auth': {'authenticate_related_mntners': False}}) + config_override({'auth': {'authenticate_parents_route_creation': False}}) result = validator.process_auth(route, None) assert result.is_valid() config_override({}) diff --git a/irrd/updates/validators.py b/irrd/updates/validators.py index e494dd5b8..b0101f25a 100644 --- a/irrd/updates/validators.py +++ b/irrd/updates/validators.py @@ -212,17 +212,16 @@ def process_auth(self, rpsl_obj_new: RPSLObject, rpsl_obj_current: Optional[RPSL result.mntners_notify = mntner_objs_current else: result.mntners_notify = mntner_objs_new - if get_setting('auth.authenticate_related_mntners'): - mntners_related = self._find_related_mntners(rpsl_obj_new, result) - if mntners_related: - related_object_class, related_pk, related_mntner_list = mntners_related - logger.debug(f'Checking auth for related object {related_object_class} / ' - f'{related_pk} with mntners {related_mntner_list}') - valid, mntner_objs_related = self._check_mntners(related_mntner_list, source) - if not valid: - self._generate_failure_message(result, related_mntner_list, rpsl_obj_new, - related_object_class, related_pk) - result.mntners_notify = mntner_objs_related + mntners_related = self._find_related_mntners(rpsl_obj_new, result) + if mntners_related: + related_object_class, related_pk, related_mntner_list = mntners_related + logger.debug(f'Checking auth for related object {related_object_class} / ' + f'{related_pk} with mntners {related_mntner_list}') + valid, mntner_objs_related = self._check_mntners(related_mntner_list, source) + if not valid: + self._generate_failure_message(result, related_mntner_list, rpsl_obj_new, + related_object_class, related_pk) + result.mntners_notify = mntner_objs_related if isinstance(rpsl_obj_new, RPSLMntner): if not rpsl_obj_current: @@ -322,6 +321,9 @@ def _find_related_object_route(self, rpsl_obj_new: RPSLObject): Find the related inetnum/route object to rpsl_obj_new, which must be a route(6). Returns a dict as returned by the database handler. """ + if not get_setting('auth.authenticate_parents_route_creation'): + return None + inetnum_class = { 'route': 'inetnum', 'route6': 'inet6num', From e1cc657468810fc963ebc6602ae4973ae1a70aa6 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Thu, 11 Nov 2021 15:40:59 +0100 Subject: [PATCH 05/11] Add docs for #575 --- docs/admins/configuration.rst | 91 ++++++++++++++++++++++++++++----- docs/releases/4.1.0.rst | 2 +- docs/spelling_wordlist.txt | 3 ++ docs/users/database-changes.rst | 37 ++++++++++++-- 4 files changed, 113 insertions(+), 20 deletions(-) diff --git a/docs/admins/configuration.rst b/docs/admins/configuration.rst index 12317da2c..9db4e5b55 100644 --- a/docs/admins/configuration.rst +++ b/docs/admins/configuration.rst @@ -17,6 +17,7 @@ a key ``roa_source`` under a key ``rpki``. .. contents:: :backlinks: none :local: + :depth: 2 Example configuration file -------------------------- @@ -294,21 +295,21 @@ Email |br| `parent of newly created object(s).` -Authentication -~~~~~~~~~~~~~~ +Authentication and validation +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * ``auth.override_password``: a salted MD5 hash of the override password, which can be used to override any authorisation requirements for authoritative databases. |br| **Default**: not defined, no override password will be accepted. |br| **Change takes effect**: upon the next update attempt. -* ``auth.authenticate_related_mntners``: whether to check for - :ref:`related object maintainers ` when processing - updates. - |br| **Default**: true, check enabled - |br| **Change takes effect**: upon the next update attempt. * ``auth.gnupg_keyring``: the full path to the gnupg keyring. |br| **Default**: not defined, but required. |br| **Change takes effect**: after full IRRd restart. +* ``auth.authenticate_parents_route_creation``: whether to check for + :ref:`related object maintainers ` when users create + new `route(6)` objects. + |br| **Default**: true, check enabled + |br| **Change takes effect**: upon the next update attempt. .. danger:: @@ -317,6 +318,75 @@ Authentication changes. Therefore, the keyring referred to by ``auth.gnupg_keyring`` can not be simply reset, or PGP authentications may fail. +.. _conf-auth-set-creation: + +auth.set_creation +""""""""""""""""" +The ``auth.set_creation`` setting configures the requirements when creating new +RPSL set objects. These are `as-set`, `filter-set`, `peering-set`, `route-set` +and `rtr-set`. It is highly customisable, but therefore also more complex +than most other settings. + +There are two underlying settings: + +* ``prefix_required`` configures whether an ASN prefix is required in the name + of a set object. When enabled ``AS-EXAMPLE`` is invalid, while + ``AS65537:AS-EXAMPLE`` or ``AS65537:AS-EXAMPLE:AS-CUSTOMERS`` + are valid. +* ``autnum_authentication`` controls whether the user also needs to pass + authentication for the `aut-num` corresponding to the AS number used as the set + name prefix. For example, if the set name is ``AS65537:AS-EXAMPLE:AS-CUSTOMERS``, + this setting may require the creation to also pass authentication for the + `aut-num` AS65537. + The options are ``disabled``, ``opportunistic`` or ``required``. + When disabled, this check is skipped. For opportunistic, the check is used, but + passes if the aut-num does not exist. For required, the check is used and fails + if the aut-num does not exist. + +Note that even when ``autnum_authentication`` is set to ``required``, +if at the same time ``prefix_required`` is set to false, a set can be created +without a prefix or with one, per ``prefix_required``. +But if it has a prefix, there *must* be a corresponding +aut-num object for which authentication *must* pass, per ``autnum_authentication``. + +You can configure one default for all set classes under the key ``DEFAULT``, +and/or specific settings for specific classes using the class name as key. +An example:: + + irrd: + auth: + set_creation: + DEFAULT: + prefix_required: true + autnum_authentication: opportunistic + as-set: + prefix_required: true + autnum_authentication: required + rtr-set: + prefix_required: false + autnum_authentication: disabled + +This example means: + +* New ``as-set`` objects must include an ASN prefix in their name, an `aut-num` + corresponding that AS number must exist, and the user must pass authentication + for that `aut-num` object. +* New ``rtr-set`` objects are not required to include an ASN prefix in their + name, but this is permitted. The user never has to pass authentication for + the corresponding `aut-num` object, regardless of whether it exists. +* All other new set objects must include an ASN prefix in their name + and the user must pass authentication for the corresponding `aut-num` object, + if it exists. If the `aut-num` does not exist, the check passes. + +All checks are only applied when users create new set objects in authoritative +databases. Authoritative updates to existing objects, deletions, or objects from +mirrors are never affected. When looking for corresponding `aut-num` objects, +IRRd only looks in the same IRR source. + +|br| **Default**: TODO +|br| **Change takes effect**: upon the next update attempt. + + Access lists ~~~~~~~~~~~~ @@ -752,13 +822,6 @@ Compatibility See the :doc:`4.2.0 release notes ` for details. |br| **Default**: ``false``, operating normally. |br| **Change takes effect**: after SIGHUP, for all subsequent queries. -* ``compatibility.permit_non_hierarchical_as_set_name``: by default, - `as-set` objects created in authoritative databases are required to have a - hierarchical name, like ``AS65540:AS-CUSTOMERS``. For example, - ``AS-CUSTOMERS`` would not be allowed. If this setting is set to ``true``, - this name requirement does not apply, and ``AS-CUSTOMERS`` is permitted. - |br| **Default**: ``false``, hierarchical name required. - |br| **Change takes effect**: after SIGHUP, for all subsequent updates. * ``compatibility.ipv4_only_route_set_members``: if set to ``true``, ``!i`` queries will not return IPv6 prefixes. This option can be used for limited compatibility with IRRd version 2. Enabling this setting may have a diff --git a/docs/releases/4.1.0.rst b/docs/releases/4.1.0.rst index e981fa62c..502a06c04 100644 --- a/docs/releases/4.1.0.rst +++ b/docs/releases/4.1.0.rst @@ -96,7 +96,7 @@ Other changes for further details. * When users create `route(6)` objects in authoritative databases, IRRd also verifies authorisation from - :ref:`related object maintainers `. This behaviour + :ref:`related object maintainers `. This behaviour is enabled by default, but can be disabled with the ``auth.authenticate_related_mntners`` setting. * The ``!j`` command has changed, and now is exclusively used to check diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 47eb200a1..9a9bcadcb 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -1,5 +1,7 @@ ASes +aut CPython +customisable Escribano IPtables IPv @@ -46,6 +48,7 @@ nginx noreply nrtm ntt +num ov pidfile postgres diff --git a/docs/users/database-changes.rst b/docs/users/database-changes.rst index ba5705c2e..a7e683682 100644 --- a/docs/users/database-changes.rst +++ b/docs/users/database-changes.rst @@ -269,11 +269,13 @@ When you create a new `mntner`, a submission must pass authorisation for one of the auth methods of the new mntner. You can submit other objects that depend on the new `mntner` in the same submission. -.. _auth-related-mntners: +.. _auth-related-mntners-route: +Related maintainers in route objects +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ When you create new `route(6)` objects, authentication also needs to pass for the parent object. IRRd searches for the parent object in the following -order, only considering the first match: +order, only considering the first match, only looking in the same IRR source: * An `inet(6)num` that is an exact match to the new `route(6)`. * The smallest `inet(6)num` that is a less specific of the new `route(6)`. @@ -282,7 +284,32 @@ order, only considering the first match: If no objects match, there is no parent object, and there are no extra authentication requirements. This behaviour can be disabled by setting -``auth.authenticate_related_mntners`` to false. +``auth.authenticate_parents_route_creation`` to false. +These requirements do not apply when you change or delete existing objects. + +.. _auth-related-mntners-set: + +Related maintainers in set objects +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +When you create new set objects, you may need to pass authentication for the +parent `aut-num` object. +RPSL set objects are `as-set`, `filter-set`, `peering-set`, `route-set` and +`rtr-set`. + +The details of this behaviour and the strictness of the checks are +:ref:`configured by the IRR operator `. This may +include a requirement to: + +* Include an ASN prefix in the name of your set, e.g. ``AS65537:AS-EXAMPLE`` + being valid, but ``AS-EXAMPLE`` being invalid. +* Pass authentication for the corresponding `aut-num`, e.g. AS65537 in the + last example, skipping this check if the `aut-num` does not exist. +* Pass authentication for the corresponding `aut-num`, e.g. AS65537 in the + last example, failing this check if the `aut-num` does not exist. + +These requirements do not apply when you change or delete existing objects. +When looking for corresponding `aut-num` objects, +IRRd only looks in the same IRR source. Object templates ---------------- @@ -323,12 +350,12 @@ This template shows: * The `member-of` attribute is a look-up key, meaning it can be used with ``-i`` queries. * The `member-of` attribute references to the `route-set` class. It is a - weak references, meaning the referred `route-set` does not have to exist, + weak reference, meaning the referred `route-set` does not have to exist, but is required to meet the syntax of a `route-set` name. The attribute is also optional, so it can be left out entirely. * The `admin-c` and `tech-c` attributes reference a `role` or `person`. This means they may refer to either object class, but must be a - reference to a valid, existing `person` or `role. This `person` or + reference to a valid, existing `person` or `role`. This `person` or `role` can be created as part of the same submission. From 38b8f3742be179680dd5ad27e3806f8ff94aa2d2 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 24 Nov 2021 12:07:30 +0100 Subject: [PATCH 06/11] Document defaults --- docs/admins/configuration.rst | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/admins/configuration.rst b/docs/admins/configuration.rst index 9db4e5b55..eb7becbc3 100644 --- a/docs/admins/configuration.rst +++ b/docs/admins/configuration.rst @@ -349,41 +349,44 @@ without a prefix or with one, per ``prefix_required``. But if it has a prefix, there *must* be a corresponding aut-num object for which authentication *must* pass, per ``autnum_authentication``. -You can configure one default for all set classes under the key ``DEFAULT``, +You can configure one common for all set classes under the key ``COMMON``, and/or specific settings for specific classes using the class name as key. An example:: irrd: auth: set_creation: - DEFAULT: + COMMON: prefix_required: true - autnum_authentication: opportunistic + autnum_authentication: required as-set: prefix_required: true - autnum_authentication: required + autnum_authentication: opportunistic rtr-set: prefix_required: false autnum_authentication: disabled This example means: -* New ``as-set`` objects must include an ASN prefix in their name, an `aut-num` - corresponding that AS number must exist, and the user must pass authentication - for that `aut-num` object. +* New `as-set` objects must include an ASN prefix in their name + and the user must pass authentication for the corresponding `aut-num` object, + if it exists. If the `aut-num` does not exist, the check passes. * New ``rtr-set`` objects are not required to include an ASN prefix in their name, but this is permitted. The user never has to pass authentication for the corresponding `aut-num` object, regardless of whether it exists. -* All other new set objects must include an ASN prefix in their name - and the user must pass authentication for the corresponding `aut-num` object, - if it exists. If the `aut-num` does not exist, the check passes. +* All other new set objects must include an ASN prefix in their name, an `aut-num` + corresponding that AS number must exist, and the user must pass authentication + for that `aut-num` object. All checks are only applied when users create new set objects in authoritative databases. Authoritative updates to existing objects, deletions, or objects from mirrors are never affected. When looking for corresponding `aut-num` objects, IRRd only looks in the same IRR source. -|br| **Default**: TODO +**Default**: ``prefix_required`` is enabled, ``autnum_authentication`` +set to ``opportunistic`` for all sets. Note that settings under the +``COMMON`` key override these IRRd defaults, and settings under set-specific +keys in turn override settings under the ``COMMON`` key. |br| **Change takes effect**: upon the next update attempt. From 9341c07fe899e6d0d43f89e80cd6d64fa80a08aa Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 24 Nov 2021 12:14:24 +0100 Subject: [PATCH 07/11] DEFAULT -> COMMON and extract magic strings --- irrd/conf/__init__.py | 1 + irrd/conf/known_keys.py | 3 ++- irrd/conf/test_conf.py | 2 +- irrd/rpsl/rpsl_objects.py | 4 ++-- irrd/rpsl/tests/test_rpsl_objects.py | 6 ++++-- irrd/updates/parser_state.py | 4 ++-- irrd/updates/tests/test_validators.py | 19 +++++++++++++------ 7 files changed, 25 insertions(+), 14 deletions(-) diff --git a/irrd/conf/__init__.py b/irrd/conf/__init__.py index 3117e7b03..d98564134 100644 --- a/irrd/conf/__init__.py +++ b/irrd/conf/__init__.py @@ -19,6 +19,7 @@ PASSWORD_HASH_DUMMY_VALUE = 'DummyValue' SOURCE_NAME_RE = re.compile('^[A-Z][A-Z0-9-]*[A-Z0-9]$') RPKI_IRR_PSEUDO_SOURCE = 'RPKI' +AUTH_SET_CREATION_COMMON_KEY = 'COMMON' LOGGING = { diff --git a/irrd/conf/known_keys.py b/irrd/conf/known_keys.py index 6b326832e..20b58ef31 100644 --- a/irrd/conf/known_keys.py +++ b/irrd/conf/known_keys.py @@ -1,3 +1,4 @@ +from irrd.conf import AUTH_SET_CREATION_COMMON_KEY from irrd.vendor.dotted.collection import DottedDict from irrd.rpsl.rpsl_objects import OBJECT_CLASS_MAPPING, RPSLSet @@ -42,7 +43,7 @@ set_object.rpsl_object_class for set_object in OBJECT_CLASS_MAPPING.values() if issubclass(set_object, RPSLSet) - ] + ['DEFAULT'] + ] + [AUTH_SET_CREATION_COMMON_KEY] }, }, 'rpki': { diff --git a/irrd/conf/test_conf.py b/irrd/conf/test_conf.py index f661e8618..430c975d4 100644 --- a/irrd/conf/test_conf.py +++ b/irrd/conf/test_conf.py @@ -80,7 +80,7 @@ def test_load_valid_reload_valid_config(self, monkeypatch, save_yaml_config, tmp 'prefix_required': True, 'autnum_authentication': 'opportunistic', }, - 'DEFAULT': { + 'COMMON': { 'prefix_required': True, 'autnum_authentication': 'required', }, diff --git a/irrd/rpsl/rpsl_objects.py b/irrd/rpsl/rpsl_objects.py index 9b07dd8ea..f6b2ebd84 100644 --- a/irrd/rpsl/rpsl_objects.py +++ b/irrd/rpsl/rpsl_objects.py @@ -2,7 +2,7 @@ from typing import Set, List, Optional, Union -from irrd.conf import PASSWORD_HASH_DUMMY_VALUE, get_setting +from irrd.conf import AUTH_SET_CREATION_COMMON_KEY, PASSWORD_HASH_DUMMY_VALUE, get_setting from irrd.utils.pgp import get_gpg_instance from .config import PASSWORD_HASHERS from .fields import (RPSLTextField, RPSLIPv4PrefixField, RPSLIPv4PrefixesField, RPSLIPv6PrefixField, @@ -41,7 +41,7 @@ def clean_for_create(self) -> bool: self.pk_asn_segment = None if get_setting(f'auth.set_creation.{self.rpsl_object_class}.prefix_required') is False: return True - if get_setting('auth.set_creation.DEFAULT.prefix_required') is False: + if get_setting(f'auth.set_creation.{AUTH_SET_CREATION_COMMON_KEY}.prefix_required') is False: return True self.messages.error(f'{self.rpsl_object_class} names must be hierarchical and the first ' f'component must be an AS number, e.g. "AS65537:{self.pk_asn_segment}": {str(ve)}') diff --git a/irrd/rpsl/tests/test_rpsl_objects.py b/irrd/rpsl/tests/test_rpsl_objects.py index a5a48db38..1b545bbf0 100644 --- a/irrd/rpsl/tests/test_rpsl_objects.py +++ b/irrd/rpsl/tests/test_rpsl_objects.py @@ -5,7 +5,7 @@ from pytest import raises from pytz import timezone -from irrd.conf import PASSWORD_HASH_DUMMY_VALUE +from irrd.conf import PASSWORD_HASH_DUMMY_VALUE, AUTH_SET_CREATION_COMMON_KEY from irrd.utils.rpsl_samples import (object_sample_mapping, SAMPLE_MALFORMED_EMPTY_LINE, SAMPLE_MALFORMED_ATTRIBUTE_NAME, SAMPLE_UNKNOWN_CLASS, SAMPLE_MISSING_MANDATORY_ATTRIBUTE, @@ -163,7 +163,9 @@ def test_clean_for_create(self, config_override): assert obj.clean_for_create() assert not obj.pk_asn_segment - config_override({'auth': {'set_creation': {'DEFAULT': {'prefix_required': False}}}}) + config_override({'auth': {'set_creation': { + AUTH_SET_CREATION_COMMON_KEY: {'prefix_required': False} + }}}) obj = rpsl_object_from_text(rpsl_text) assert obj.clean_for_create() assert not obj.pk_asn_segment diff --git a/irrd/updates/parser_state.py b/irrd/updates/parser_state.py index ea0546e14..7566080b6 100644 --- a/irrd/updates/parser_state.py +++ b/irrd/updates/parser_state.py @@ -1,6 +1,6 @@ from enum import unique, Enum -from irrd.conf import get_setting +from irrd.conf import get_setting, AUTH_SET_CREATION_COMMON_KEY @unique @@ -33,7 +33,7 @@ class RPSLSetAutnumAuthenticationMode(Enum): def for_set_name(set_name: str): setting = get_setting(f'auth.set_creation.{set_name}.autnum_authentication') if not setting: - setting = get_setting('auth.set_creation.DEFAULT.autnum_authentication') + setting = get_setting(f'auth.set_creation.{AUTH_SET_CREATION_COMMON_KEY}.autnum_authentication') if not setting: return RPSLSetAutnumAuthenticationMode.DISABLED return getattr(RPSLSetAutnumAuthenticationMode, setting.upper()) diff --git a/irrd/updates/tests/test_validators.py b/irrd/updates/tests/test_validators.py index b0446898a..d203f8ce9 100644 --- a/irrd/updates/tests/test_validators.py +++ b/irrd/updates/tests/test_validators.py @@ -5,6 +5,7 @@ import pytest from pytest import raises +from irrd.conf import AUTH_SET_CREATION_COMMON_KEY from irrd.rpsl.rpsl_objects import rpsl_object_from_text from irrd.utils.rpsl_samples import (SAMPLE_AS_SET, SAMPLE_FILTER_SET, SAMPLE_MNTNER, SAMPLE_MNTNER_CRYPT, SAMPLE_MNTNER_MD5, SAMPLE_PERSON, @@ -511,7 +512,9 @@ def test_as_set_autnum_opportunistic_exists(self, prepare_mocks, config_override assert result.is_valid() def test_as_set_autnum_opportunistic_does_not_exist(self, prepare_mocks, config_override): - config_override({'auth': {'set_creation': {'DEFAULT': {'autnum_authentication': 'opportunistic'}}}}) + config_override({'auth': {'set_creation': { + AUTH_SET_CREATION_COMMON_KEY: {'autnum_authentication': 'opportunistic'} + }}}) validator, mock_dq, mock_dh = prepare_mocks as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_first_segment @@ -536,7 +539,9 @@ def test_as_set_autnum_opportunistic_does_not_exist(self, prepare_mocks, config_ ] def test_as_set_autnum_required_does_not_exist(self, prepare_mocks, config_override): - config_override({'auth': {'set_creation': {'DEFAULT': {'autnum_authentication': 'required'}}}}) + config_override({'auth': {'set_creation': { + AUTH_SET_CREATION_COMMON_KEY: {'autnum_authentication': 'required'} + }}}) validator, mock_dq, mock_dh = prepare_mocks as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_first_segment @@ -554,10 +559,12 @@ def test_as_set_autnum_required_does_not_exist(self, prepare_mocks, config_overr } def test_filter_set_autnum_required_no_prefix(self, prepare_mocks, config_override): - config_override({'auth': {'set_creation': {'DEFAULT': { - 'autnum_authentication': 'required', - 'prefix_required': False, - }}}}) + config_override({'auth': {'set_creation': { + AUTH_SET_CREATION_COMMON_KEY: { + 'autnum_authentication': 'required', + 'prefix_required': False, + } + }}}) validator, mock_dq, mock_dh = prepare_mocks filter_set = rpsl_object_from_text(SAMPLE_FILTER_SET) assert filter_set.clean_for_create() From 1b17f47de423809198f9caf4c6e22b3b6375898f Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 24 Nov 2021 13:42:42 +0100 Subject: [PATCH 08/11] Update default setting --- irrd/conf/default_config.yaml | 5 +++++ irrd/updates/parser_state.py | 2 -- irrd/updates/tests/test_validators.py | 8 +------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/irrd/conf/default_config.yaml b/irrd/conf/default_config.yaml index a99d7d22e..748c615c3 100644 --- a/irrd/conf/default_config.yaml +++ b/irrd/conf/default_config.yaml @@ -45,6 +45,11 @@ irrd: auth: gnupg_keyring: null authenticate_parents_route_creation: true + set_creation: + COMMON: + prefix_required: true + autnum_authentication: opportunistic + email: footer: '' notification_header: | diff --git a/irrd/updates/parser_state.py b/irrd/updates/parser_state.py index 7566080b6..acaf918cf 100644 --- a/irrd/updates/parser_state.py +++ b/irrd/updates/parser_state.py @@ -34,6 +34,4 @@ def for_set_name(set_name: str): setting = get_setting(f'auth.set_creation.{set_name}.autnum_authentication') if not setting: setting = get_setting(f'auth.set_creation.{AUTH_SET_CREATION_COMMON_KEY}.autnum_authentication') - if not setting: - return RPSLSetAutnumAuthenticationMode.DISABLED return getattr(RPSLSetAutnumAuthenticationMode, setting.upper()) diff --git a/irrd/updates/tests/test_validators.py b/irrd/updates/tests/test_validators.py index d203f8ce9..c27880fa1 100644 --- a/irrd/updates/tests/test_validators.py +++ b/irrd/updates/tests/test_validators.py @@ -455,8 +455,7 @@ def test_as_set_autnum_disabled(self, prepare_mocks, config_override): ['rpsl_pks', ({'TEST-MNT'},), {}], ] - def test_as_set_autnum_opportunistic_exists(self, prepare_mocks, config_override): - config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'opportunistic'}}}}) + def test_as_set_autnum_opportunistic_exists_default(self, prepare_mocks, config_override): validator, mock_dq, mock_dh = prepare_mocks as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_asn_segment @@ -506,11 +505,6 @@ def test_as_set_autnum_opportunistic_exists(self, prepare_mocks, config_override result = validator.process_auth(as_set, None) assert result.is_valid() - # Default is disabled - config_override({}) - result = validator.process_auth(as_set, None) - assert result.is_valid() - def test_as_set_autnum_opportunistic_does_not_exist(self, prepare_mocks, config_override): config_override({'auth': {'set_creation': { AUTH_SET_CREATION_COMMON_KEY: {'autnum_authentication': 'opportunistic'} From 2fd1fec32f3dbdb357f7ff1ed325e7e438d23c8f Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Wed, 24 Nov 2021 14:41:15 +0100 Subject: [PATCH 09/11] Add release notes --- docs/admins/configuration.rst | 1 - docs/releases/4.3.0.rst | 37 ++++++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/docs/admins/configuration.rst b/docs/admins/configuration.rst index eb7becbc3..a5f547ee7 100644 --- a/docs/admins/configuration.rst +++ b/docs/admins/configuration.rst @@ -390,7 +390,6 @@ keys in turn override settings under the ``COMMON`` key. |br| **Change takes effect**: upon the next update attempt. - Access lists ~~~~~~~~~~~~ * ``access_lists.{list_name}``: a list of permitted IPv4 and/or IPv6 addresses diff --git a/docs/releases/4.3.0.rst b/docs/releases/4.3.0.rst index b3dad79d5..e37683c14 100644 --- a/docs/releases/4.3.0.rst +++ b/docs/releases/4.3.0.rst @@ -2,4 +2,39 @@ Release notes for IRRd 4.3.0 ============================ -* removal of compatibility.permit_non_hierarchical_as_set_name +Changes to related object authentication and settings +----------------------------------------------------- +In version 4.2, IRRd required newly created authoritative `as-set` objects +to have a hierarchical name where the first element is an AS number. +In 4.3, this feature has been significantly expanded. + +For all RPSL set objects, IRRd can now be configured to require: + +* Including an ASN prefix in the name of the set, e.g. ``AS65537:AS-EXAMPLE`` + being valid, but ``AS-EXAMPLE`` being invalid. +* Passing authentication for the corresponding `aut-num`, e.g. AS65537 in the + last example, skipping this check if the `aut-num` does not exist. +* Passing authentication for the corresponding `aut-num`, e.g. AS65537 in the + last example, failing this check if the `aut-num` does not exist. + +The first two options, requiring a prefix with opportunistic `aut-num` authentication, +is now the default for all set objects. +You can :ref:`change the configuration ` for specific +RPSL set objects, or set your own common configuration that applies to all sets. + +The ``compatibility.permit_non_hierarchical_as_set_name`` setting has been +removed, as it is now covered by the ``prefix_required`` setting. + +The ``auth.authenticate_related_mntners`` setting has been renamed to +``auth.authenticate_parents_route_creation``, as this setting exclusively +relates to :ref:`authentication for route(6) objects ` +and needs to be distinct from the configuration for RPSL set objects. + +If you were using ``auth.authenticate_related_mntners`` or +``compatibility.permit_non_hierarchical_as_set_name``, you need to update +your configuration. + +All checks are only applied when users create new set objects in authoritative +databases. Authoritative updates to existing objects, deletions, or objects from +mirrors are never affected. When looking for related objects, +IRRd only looks in the same IRR source. From 90423195bc62aa83c38c731313c12bb05782bc61 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Thu, 25 Nov 2021 14:26:13 +0100 Subject: [PATCH 10/11] Apply suggestions from code review --- docs/releases/4.3.0.rst | 4 ++-- docs/users/database-changes.rst | 4 ++-- irrd/updates/validators.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/releases/4.3.0.rst b/docs/releases/4.3.0.rst index e37683c14..8583d772c 100644 --- a/docs/releases/4.3.0.rst +++ b/docs/releases/4.3.0.rst @@ -13,9 +13,9 @@ For all RPSL set objects, IRRd can now be configured to require: * Including an ASN prefix in the name of the set, e.g. ``AS65537:AS-EXAMPLE`` being valid, but ``AS-EXAMPLE`` being invalid. * Passing authentication for the corresponding `aut-num`, e.g. AS65537 in the - last example, skipping this check if the `aut-num` does not exist. + example, skipping this check if the `aut-num` does not exist. * Passing authentication for the corresponding `aut-num`, e.g. AS65537 in the - last example, failing this check if the `aut-num` does not exist. + example, failing this check if the `aut-num` does not exist. The first two options, requiring a prefix with opportunistic `aut-num` authentication, is now the default for all set objects. diff --git a/docs/users/database-changes.rst b/docs/users/database-changes.rst index a7e683682..7928c8ccb 100644 --- a/docs/users/database-changes.rst +++ b/docs/users/database-changes.rst @@ -303,9 +303,9 @@ include a requirement to: * Include an ASN prefix in the name of your set, e.g. ``AS65537:AS-EXAMPLE`` being valid, but ``AS-EXAMPLE`` being invalid. * Pass authentication for the corresponding `aut-num`, e.g. AS65537 in the - last example, skipping this check if the `aut-num` does not exist. + example, skipping this check if the `aut-num` does not exist. * Pass authentication for the corresponding `aut-num`, e.g. AS65537 in the - last example, failing this check if the `aut-num` does not exist. + example, failing this check if the `aut-num` does not exist. These requirements do not apply when you change or delete existing objects. When looking for corresponding `aut-num` objects, diff --git a/irrd/updates/validators.py b/irrd/updates/validators.py index b0101f25a..b43a93f98 100644 --- a/irrd/updates/validators.py +++ b/irrd/updates/validators.py @@ -299,7 +299,7 @@ def _find_related_mntners(self, rpsl_obj_new: RPSLObject, result: ValidatorResul - object class of the related object - PK of the related object - List of maintainers for the related object (at least one must pass) - Returns None of no related objects were found that should be authenticated. + Returns None if no related objects were found that should be authenticated. Custom error messages may be added directly to the passed ValidatorResult. """ From 6943fcfdff3b4d26198207d108e540c39a234ce7 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Thu, 25 Nov 2021 15:47:21 +0100 Subject: [PATCH 11/11] Cleanup --- irrd/conf/__init__.py | 6 ++-- irrd/updates/tests/test_validators.py | 44 ++++++++++++++------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/irrd/conf/__init__.py b/irrd/conf/__init__.py index d98564134..9e6890e14 100644 --- a/irrd/conf/__init__.py +++ b/irrd/conf/__init__.py @@ -213,7 +213,7 @@ def _check_staging_config(self) -> List[str]: config = self.user_config_staging def _validate_subconfig(key, value): - if hasattr(value, 'items'): + if isinstance(value, (DottedDict, dict)): for key2, value2 in value.items(): subkey = key + '.' + key2 known_sub = self.known_config_keys.get(subkey) @@ -225,9 +225,7 @@ def _validate_subconfig(key, value): for key, value in config.items(): if key in ['sources', 'access_lists']: continue - known = self.known_config_keys.get(key) - - if known is None: + if self.known_config_keys.get(key) is None: errors.append(f'Unknown setting key: {key}') _validate_subconfig(key, value) diff --git a/irrd/updates/tests/test_validators.py b/irrd/updates/tests/test_validators.py index c27880fa1..463c01211 100644 --- a/irrd/updates/tests/test_validators.py +++ b/irrd/updates/tests/test_validators.py @@ -18,6 +18,8 @@ VALID_PW = 'override-password' INVALID_PW = 'not-override-password' VALID_PW_HASH = '$1$J6KycItM$MbPaBU6iFSGFV299Rk7Di0' +MNTNER_OBJ_CRYPT_PW = SAMPLE_MNTNER.replace('MD5', '') +MNTNER_OBJ_MD5_PW = SAMPLE_MNTNER.replace('CRYPT', '') @pytest.fixture() @@ -45,7 +47,7 @@ def test_valid_override(self, prepare_mocks, config_override): assert result.used_override person = rpsl_object_from_text(SAMPLE_PERSON) - result = validator.process_auth(person, person) + result = validator.process_auth(person, rpsl_obj_current=person) assert result.is_valid(), result.error_messages assert result.used_override @@ -113,23 +115,23 @@ def test_existing_person_mntner_change(self, prepare_mocks): [ { 'object_class': 'mntner', - 'object_text': SAMPLE_MNTNER.replace('TEST-MNT', 'TEST-NEW-MNT').replace('MD5', 'nomd5') + 'object_text': MNTNER_OBJ_CRYPT_PW.replace('TEST-MNT', 'TEST-NEW-MNT') }, { 'object_class': 'mntner', - 'object_text': SAMPLE_MNTNER.replace('MD5', 'nomd5').replace('CRYPT', 'nocrypt') + 'object_text': MNTNER_OBJ_MD5_PW.replace('MD5', 'nomd5') }, ], [ { 'object_class': 'mntner', - 'object_text': SAMPLE_MNTNER.replace('TEST-MNT', 'TEST-OLD-MNT').replace('CRYPT', 'nocrypt') + 'object_text': MNTNER_OBJ_MD5_PW.replace('TEST-MNT', 'TEST-OLD-MNT') }, ], ]) mock_dh.execute_query = lambda q: next(query_results) validator.passwords = [SAMPLE_MNTNER_CRYPT, SAMPLE_MNTNER_MD5] - result = validator.process_auth(person_new, person_old) + result = validator.process_auth(person_new, rpsl_obj_current=person_old) assert result.is_valid(), result.error_messages assert not result.used_override @@ -146,13 +148,13 @@ def test_existing_person_mntner_change(self, prepare_mocks): ] validator.passwords = [SAMPLE_MNTNER_MD5] - result = validator.process_auth(person_new, person_old) + result = validator.process_auth(person_new, rpsl_obj_current=person_old) assert not result.is_valid() assert result.error_messages == {'Authorisation for person PERSON-TEST failed: ' 'must be authenticated by one of: TEST-MNT, TEST-NEW-MNT'} validator.passwords = [SAMPLE_MNTNER_CRYPT] - result = validator.process_auth(person_new, person_old) + result = validator.process_auth(person_new, rpsl_obj_current=person_old) assert not result.is_valid() assert result.error_messages == {'Authorisation for person PERSON-TEST failed: ' 'must be authenticated by one of: TEST-MNT, TEST-OLD-MNT'} @@ -213,7 +215,7 @@ def test_modify_mntner(self, prepare_mocks, config_override): assert not result.info_messages # This counts as submitting all new hashes, but not matching any password - new_mntner = rpsl_object_from_text(SAMPLE_MNTNER.replace('CRYPT', '').replace('MD5', '')) + new_mntner = rpsl_object_from_text(MNTNER_OBJ_CRYPT_PW.replace('CRYPT', '')) validator.passwords = [SAMPLE_MNTNER_MD5] result = validator.process_auth(new_mntner, mntner) assert not result.is_valid() @@ -250,14 +252,14 @@ def test_related_route_exact_inetnum(self, prepare_mocks, config_override): validator, mock_dq, mock_dh = prepare_mocks route = rpsl_object_from_text(SAMPLE_ROUTE) query_results = itertools.cycle([ - [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{'object_text': MNTNER_OBJ_CRYPT_PW}], # mntner for object [{ # attempt to look for exact inetnum 'object_class': 'inetnum', 'rpsl_pk': '192.0.2.0-192.0.2.255', 'parsed_data': {'mnt-by': ['RELATED-MNT']} }], - [{'object_text': SAMPLE_MNTNER.replace('CRYPT', '')}], # related mntner retrieval + [{'object_text': MNTNER_OBJ_MD5_PW}], # related mntner retrieval ]) mock_dh.execute_query = lambda q: next(query_results) @@ -300,7 +302,7 @@ def test_related_route_less_specific_inetnum(self, prepare_mocks, config_overrid validator, mock_dq, mock_dh = prepare_mocks route = rpsl_object_from_text(SAMPLE_ROUTE) query_results = itertools.cycle([ - [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{'object_text': MNTNER_OBJ_CRYPT_PW}], # mntner for object [], # attempt to look for exact inetnum [{ # attempt to look for one level less specific inetnum @@ -308,7 +310,7 @@ def test_related_route_less_specific_inetnum(self, prepare_mocks, config_overrid 'rpsl_pk': '192.0.2.0-192.0.2.255', 'parsed_data': {'mnt-by': ['RELATED-MNT']} }], - [{'object_text': SAMPLE_MNTNER.replace('CRYPT', '')}], # related mntner retrieval + [{'object_text': MNTNER_OBJ_MD5_PW}], # related mntner retrieval ]) mock_dh.execute_query = lambda q: next(query_results) @@ -348,7 +350,7 @@ def test_related_route_less_specific_route(self, prepare_mocks, config_override) validator, mock_dq, mock_dh = prepare_mocks route = rpsl_object_from_text(SAMPLE_ROUTE) query_results = itertools.cycle([ - [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{'object_text': MNTNER_OBJ_CRYPT_PW}], # mntner for object [], # attempt to look for exact inetnum [], # attempt to look for one level less specific inetnum [{ @@ -357,7 +359,7 @@ def test_related_route_less_specific_route(self, prepare_mocks, config_override) 'rpsl_pk': '192.0.2.0/24AS65537', 'parsed_data': {'mnt-by': ['RELATED-MNT']} }], - [{'object_text': SAMPLE_MNTNER.replace('CRYPT', '')}], # related mntner retrieval + [{'object_text': MNTNER_OBJ_MD5_PW}], # related mntner retrieval ]) mock_dh.execute_query = lambda q: next(query_results) @@ -443,7 +445,7 @@ def test_as_set_autnum_disabled(self, prepare_mocks, config_override): as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_asn_segment mock_dh.execute_query = lambda q: [ - {'object_text': SAMPLE_MNTNER.replace('MD5', '')}, # mntner for object + {'object_text': MNTNER_OBJ_CRYPT_PW}, # mntner for object ] validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT] @@ -460,14 +462,14 @@ def test_as_set_autnum_opportunistic_exists_default(self, prepare_mocks, config_ as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_asn_segment query_results = itertools.cycle([ - [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{'object_text': MNTNER_OBJ_CRYPT_PW}], # mntner for object [{ # attempt to look for matching aut-num 'object_class': 'aut-num', 'rpsl_pk': 'AS655375', 'parsed_data': {'mnt-by': ['RELATED-MNT']} }], - [{'object_text': SAMPLE_MNTNER.replace('CRYPT', '')}], # related mntner retrieval + [{'object_text': MNTNER_OBJ_MD5_PW}], # related mntner retrieval ]) mock_dh.execute_query = lambda q: next(query_results) @@ -498,7 +500,7 @@ def test_as_set_autnum_opportunistic_exists_default(self, prepare_mocks, config_ 'RELATED-MNT - from parent aut-num AS655375' } - result = validator.process_auth(as_set, as_set) + result = validator.process_auth(as_set, rpsl_obj_current=as_set) assert result.is_valid() config_override({'auth': {'set_creation': {'as-set': {'autnum_authentication': 'disabled'}}}}) @@ -513,7 +515,7 @@ def test_as_set_autnum_opportunistic_does_not_exist(self, prepare_mocks, config_ as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_first_segment query_results = itertools.cycle([ - [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{'object_text': MNTNER_OBJ_CRYPT_PW}], # mntner for object [], # attempt to look for matching aut-num ]) mock_dh.execute_query = lambda q: next(query_results) @@ -540,7 +542,7 @@ def test_as_set_autnum_required_does_not_exist(self, prepare_mocks, config_overr as_set = rpsl_object_from_text(SAMPLE_AS_SET) assert as_set.clean_for_create() # fill pk_first_segment query_results = itertools.cycle([ - [{'object_text': SAMPLE_MNTNER.replace('MD5', '')}], # mntner for object + [{'object_text': MNTNER_OBJ_CRYPT_PW}], # mntner for object [], # attempt to look for matching aut-num ]) mock_dh.execute_query = lambda q: next(query_results) @@ -563,7 +565,7 @@ def test_filter_set_autnum_required_no_prefix(self, prepare_mocks, config_overri filter_set = rpsl_object_from_text(SAMPLE_FILTER_SET) assert filter_set.clean_for_create() mock_dh.execute_query = lambda q: [ - {'object_text': SAMPLE_MNTNER.replace('MD5', '')}, # mntner for object + {'object_text': MNTNER_OBJ_CRYPT_PW}, # mntner for object ] validator.passwords = [SAMPLE_MNTNER_MD5, SAMPLE_MNTNER_CRYPT]