diff --git a/docs/users/database-changes.rst b/docs/users/database-changes.rst index 5b1d5665d..8ed7cc8b7 100644 --- a/docs/users/database-changes.rst +++ b/docs/users/database-changes.rst @@ -79,7 +79,11 @@ Notifications to maintainers or the address in the ``notify`` attribute are If an invalid override password is used, or if no override password was configured, the invalid use is logged, and authentication and notification -proceeds as usual, as if no override password was provided. +proceeds as usual, **as if no override password was provided.** + +.. note:: + New `mntner` objects can only be created using the override password. + Working with auth hash masking ------------------------------ @@ -107,6 +111,7 @@ Any other scenario, like submitting a mix of dummy and real hashes, or submitting dummy hashes along with multiple ``password`` attributes in the message, is considered an error. + Referential integrity --------------------- IRRd enforces referential integrity between objects. This means it is not @@ -123,6 +128,7 @@ fail, as A still exists.) In the same way, it's possible to create multiple objects that depend on each other in the same submission to IRRd. + Authentication checks --------------------- When changing an object, authentication must pass for one of the @@ -138,6 +144,7 @@ When creating a new `mntner`, a submission must pass authorisation for one of the auth methods of the new mntner. Other objects can be submitted that depend on the new `mntner` in the same submission. + Object templates ---------------- @@ -183,6 +190,7 @@ This template shows: * The `admin-c` and `tech-c` attributes reference a `role` or `person`. This means they may refer to either object class. + Notifications ------------- IRRd will always reply to a submission with a report on the requested @@ -190,6 +198,7 @@ changes. Depending on the request and its result, additional notifications may be sent. The overview below details all notifications that may be sent. + Authentication and notification overview ---------------------------------------- diff --git a/irrd/integration_tests/run.py b/irrd/integration_tests/run.py index a9a19e317..96b54fd5e 100644 --- a/irrd/integration_tests/run.py +++ b/irrd/integration_tests/run.py @@ -52,9 +52,7 @@ SAMPLE_INET6NUM, SAMPLE_INETNUM, SAMPLE_KEY_CERT, - SAMPLE_MNTNER_CLEAN, SAMPLE_PEERING_SET, - SAMPLE_PERSON, SAMPLE_PERSON.replace('PERSON-TEST', 'DUMY2-TEST'), SAMPLE_ROLE, SAMPLE_ROUTE, @@ -93,7 +91,7 @@ def test_irrd_integration(self, tmpdir): self._start_irrds() # Attempt to load a mntner with valid auth, but broken references. - self._submit_update(self.config_path1, SAMPLE_MNTNER + '\n\npassword: md5-password') + self._submit_update(self.config_path1, SAMPLE_MNTNER + '\n\noverride: override-password') messages = self._retrieve_mails() assert len(messages) == 1 mail_text = self._extract_message_body(messages[0]) @@ -110,7 +108,7 @@ def test_irrd_integration(self, tmpdir): # Load a regular valid mntner and person into the DB, and verify # the contents of the result. self._submit_update(self.config_path1, - SAMPLE_MNTNER_CLEAN + '\n\n' + SAMPLE_PERSON + '\n\npassword: md5-password') + SAMPLE_MNTNER_CLEAN + '\n\n' + SAMPLE_PERSON + '\n\noverride: override-password') messages = self._retrieve_mails() assert len(messages) == 1 mail_text = self._extract_message_body(messages[0]) @@ -299,8 +297,9 @@ def test_irrd_integration(self, tmpdir): assert 'No entries found for the selected source(s)' in person_text assert 'PERSON-TEST' not in person_text - # Load samples of all known objects. - self._submit_update(self.config_path1, LARGE_UPDATE + '\n\npassword: md5-password') + # Load the mntner and person again, using the override password + self._submit_update(self.config_path1, + SAMPLE_MNTNER_CLEAN + '\n\n' + SAMPLE_PERSON + '\n\noverride: override-password') messages = self._retrieve_mails() assert len(messages) == 1 mail_text = self._extract_message_body(messages[0]) @@ -309,6 +308,17 @@ def test_irrd_integration(self, tmpdir): assert messages[0]['To'] == 'Sasha ' assert '\nCreate succeeded: [mntner] TEST-MNT\n' in mail_text assert '\nCreate succeeded: [person] PERSON-TEST\n' in mail_text + assert 'email footer' in mail_text + assert 'Generated by IRRd version ' in mail_text + + # Load samples of all known objects, using the mntner password + self._submit_update(self.config_path1, LARGE_UPDATE + '\n\npassword: md5-password') + messages = self._retrieve_mails() + assert len(messages) == 3 + mail_text = self._extract_message_body(messages[0]) + assert messages[0]['Subject'] == 'SUCCESS: my subject' + assert messages[0]['From'] == 'from@example.com' + assert messages[0]['To'] == 'Sasha ' assert '\nINFO: AS number as065537 was reformatted as AS65537\n' in mail_text assert '\nCreate succeeded: [filter-set] FLTR-SETTEST\n' in mail_text assert '\nINFO: Address range 192.0.2.0 - 192.0.02.255 was reformatted as 192.0.2.0 - 192.0.2.255\n' in mail_text @@ -466,7 +476,7 @@ def _start_irrds(self): 'auth': { 'gnupg_keyring': None, - 'override_password': None, + 'override_password': '$1$J6KycItM$MbPaBU6iFSGFV299Rk7Di0', }, 'email': { diff --git a/irrd/rpsl/tests/test_rpsl_objects.py b/irrd/rpsl/tests/test_rpsl_objects.py index b4fb1eee9..4969c9962 100644 --- a/irrd/rpsl/tests/test_rpsl_objects.py +++ b/irrd/rpsl/tests/test_rpsl_objects.py @@ -321,8 +321,8 @@ def test_parse_invalid_partial_dummy_hash(self): @pytest.mark.usefixtures('tmp_gpg_dir') # noqa: F811 def test_verify(self, tmp_gpg_dir): rpsl_text = object_sample_mapping[RPSLMntner().rpsl_object_class] - # Unknown hashes should simply be ignored. - obj = rpsl_object_from_text(rpsl_text + 'auth: UNKNOWN_HASH foo') + # Unknown hashes and invalid hashes should simply be ignored. + obj = rpsl_object_from_text(rpsl_text + 'auth: UNKNOWN_HASH foo\nauth: MD5-PW 💩') assert obj.verify_auth(['crypt-password']) assert obj.verify_auth(['md5-password']) diff --git a/irrd/updates/tests/test_handler.py b/irrd/updates/tests/test_handler.py index e24d066bb..ba770b861 100644 --- a/irrd/updates/tests/test_handler.py +++ b/irrd/updates/tests/test_handler.py @@ -11,7 +11,7 @@ @pytest.fixture() -def prepare_mocks(monkeypatch): +def prepare_mocks(monkeypatch, config_override): monkeypatch.setenv('IRRD_SOURCES_TEST_AUTHORITATIVE', '1') mock_dh = Mock() monkeypatch.setattr('irrd.updates.handler.DatabaseHandler', lambda enable_preload_update=True: mock_dh) @@ -21,6 +21,7 @@ def prepare_mocks(monkeypatch): monkeypatch.setattr('irrd.updates.validators.RPSLDatabaseQuery', lambda: mock_dq) mock_email = Mock() monkeypatch.setattr('irrd.utils.email.send_email', mock_email) + config_override({'auth': {'override_password': '$1$J6KycItM$MbPaBU6iFSGFV299Rk7Di0'}}) yield mock_dq, mock_dh, mock_email @@ -28,7 +29,7 @@ class TestChangeSubmissionHandler: # NOTE: the scope of this test also includes ChangeRequest, ReferenceValidator and AuthValidator - # this is more of an update handler integration test. - def test_parse_valid_new_objects(self, prepare_mocks): + def test_parse_valid_new_objects_with_override(self, prepare_mocks): mock_dq, mock_dh, mock_email = prepare_mocks mock_dh.execute_query = lambda query: [] @@ -51,7 +52,7 @@ def test_parse_valid_new_objects(self, prepare_mocks): changed: 2016-10-05T10:41:15Z source: TEST - password: md5-password + override: override-password inetnum: 80.16.151.184 - 80.016.151.191 netname: NETECONOMY-MG41731 @@ -75,9 +76,6 @@ def test_parse_valid_new_objects(self, prepare_mocks): ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pk', ('TEST-MNT',), {}], ['sources', (['TEST'],), {}], ['object_classes', (['inetnum'],), {}], ['rpsl_pk', ('80.16.151.184 - 80.16.151.191',), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}] ] assert mock_dh.mock_calls[0][0] == 'upsert_rpsl_object' @@ -132,7 +130,7 @@ def test_parse_valid_new_objects(self, prepare_mocks): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ """) - def test_parse_valid_new_objects_pgp_key(self, prepare_mocks): + def test_parse_valid_new_person_existing_mntner_pgp_key(self, prepare_mocks): mock_dq, mock_dh, mock_email = prepare_mocks person_text = textwrap.dedent(""" @@ -433,7 +431,7 @@ def test_parse_invalid_cascading_failure(self, prepare_mocks): changed: 2016-10-05T10:41:15Z source: TEST - password: md5-password + override: override-password inetnum: 80.16.151.184 - 80.016.151.191 netname: NETECONOMY-MG41731 @@ -466,15 +464,9 @@ def test_parse_invalid_cascading_failure(self, prepare_mocks): ['sources', (['TEST'],), {}], ['object_classes', (['inetnum'],), {}], ['rpsl_pk', ('80.16.151.184 - 80.16.151.191',), {}], ['sources', (['TEST'],), {}], ['object_classes', (['person'],), {}], ['rpsl_pk', ('PERSON-TEST',), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'OTHER-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['role', 'person'],), {}], - ['rpsl_pk', ('PERSON-TEST',), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['role', 'person'],), {}], - ['rpsl_pk', ('PERSON-TEST',), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pk', ('OTHER-MNT',), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['role', 'person'],), {}], ['rpsl_pk', ('PERSON-TEST',), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['role', 'person'],), {}], ['rpsl_pk', ('PERSON-TEST',), {}], ['sources', (['TEST'],), {}], ['object_classes', (['role', 'person'],), {}], ['rpsl_pk', ('PERSON-TEST',), {}], ] assert flatten_mock_calls(mock_dh) == [ @@ -544,7 +536,7 @@ def test_parse_invalid_cascading_failure(self, prepare_mocks): changed: 2009-07-24T17:00:00Z source: TEST - ERROR: Authorisation for person PERSON-TEST failed: must by authenticated by one of: OTHER-MNT + ERROR: Object OTHER-MNT referenced in field mnt-by not found in database TEST - must reference mntner. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ """) @@ -652,7 +644,17 @@ def test_parse_invalid_single_failure_invalid_password(self, prepare_mocks): def test_parse_invalid_cascading_failure_invalid_password(self, prepare_mocks): mock_dq, mock_dh, mock_email = prepare_mocks - mock_dh.execute_query = lambda query: [] + + query_results = iter([ + [], + [{'object_text': SAMPLE_MNTNER}], + [], + [{'object_text': SAMPLE_MNTNER}], + [{'object_text': SAMPLE_MNTNER}], + [{'object_text': SAMPLE_MNTNER}], + [{'object_text': SAMPLE_MNTNER}], + ]) + mock_dh.execute_query = lambda query: next(query_results) rpsl_text = textwrap.dedent(""" person: Placeholder Person Object @@ -698,10 +700,8 @@ def test_parse_invalid_cascading_failure_invalid_password(self, prepare_mocks): ['sources', (['TEST'],), {}], ['object_classes', (['inetnum'],), {}], ['rpsl_pk', ('80.16.151.184 - 80.16.151.191',), {}], ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], - ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pks', ({'TEST-MNT'},), {}], + ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], + ['rpsl_pks', ({'OTHER1-MNT', 'OTHER2-MNT'},), {}], ] assert flatten_mock_calls(mock_dh) == [ ['commit', (), {}], @@ -717,8 +717,8 @@ def test_parse_invalid_cascading_failure_invalid_password(self, prepare_mocks): Modify: 0 Delete: 0 Number of objects processed with errors: 3 - Create: 3 - Modify: 0 + Create: 2 + Modify: 1 Delete: 0 DETAILED EXPLANATION: @@ -739,7 +739,7 @@ def test_parse_invalid_cascading_failure_invalid_password(self, prepare_mocks): ERROR: Authorisation for person PERSON-TEST failed: must by authenticated by one of: TEST-MNT --- - Create FAILED: [mntner] TEST-MNT + Modify FAILED: [mntner] TEST-MNT mntner: TEST-MNT admin-c: PERSON-TEST @@ -750,6 +750,8 @@ def test_parse_invalid_cascading_failure_invalid_password(self, prepare_mocks): changed: 2016-10-05T10:41:15Z source: TEST + ERROR: Authorisation for mntner TEST-MNT failed: must by authenticated by one of: TEST-MNT + ERROR: Authorisation for mntner TEST-MNT failed: must by authenticated by one of: TEST-MNT, OTHER1-MNT, OTHER2-MNT ERROR: Authorisation failed for the auth methods on this mntner object. --- diff --git a/irrd/updates/tests/test_parser.py b/irrd/updates/tests/test_parser.py index 3fad6e2b0..296a28d5f 100644 --- a/irrd/updates/tests/test_parser.py +++ b/irrd/updates/tests/test_parser.py @@ -345,7 +345,6 @@ def test_check_auth_valid_update_mntner(self, prepare_mocks): assert result_inetnum._check_auth() assert not result_inetnum.error_messages - def test_check_auth_valid_create_mntner_referencing_self(self, prepare_mocks): mock_dq, mock_dh = prepare_mocks @@ -354,7 +353,7 @@ def test_check_auth_valid_create_mntner_referencing_self(self, prepare_mocks): reference_validator = ReferenceValidator(mock_dh) auth_validator = AuthValidator(mock_dh) - result_mntner = parse_change_requests(SAMPLE_MNTNER + 'password: md5-password', + result_mntner = parse_change_requests(SAMPLE_MNTNER + 'override: override-password', mock_dh, auth_validator, reference_validator)[0] auth_validator.pre_approve([result_mntner]) @@ -365,9 +364,6 @@ def test_check_auth_valid_create_mntner_referencing_self(self, prepare_mocks): ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], ['rpsl_pk', ('TEST-MNT',), {}], - ['sources', (['TEST'],), {}], - ['object_classes', (['mntner'],), {}], - ['rpsl_pks', ({'OTHER1-MNT', 'OTHER2-MNT', 'TEST-MNT'},), {}], ] def test_check_auth_invalid_create_mntner_referencing_self_wrong_password(self, prepare_mocks): @@ -378,41 +374,15 @@ def test_check_auth_invalid_create_mntner_referencing_self_wrong_password(self, reference_validator = ReferenceValidator(mock_dh) auth_validator = AuthValidator(mock_dh) - result_mntner = parse_change_requests(SAMPLE_MNTNER + 'password: invalid-password', + result_mntner = parse_change_requests(SAMPLE_MNTNER + 'override: invalid-password', mock_dh, auth_validator, reference_validator)[0] auth_validator.pre_approve([result_mntner]) assert not result_mntner._check_auth() - assert result_mntner.error_messages == ['Authorisation failed for the auth methods on this mntner object.'] - - assert flatten_mock_calls(mock_dq) == [ - ['sources', (['TEST'],), {}], - ['object_classes', (['mntner'],), {}], - ['rpsl_pk', ('TEST-MNT',), {}], - ['sources', (['TEST'],), {}], - ['object_classes', (['mntner'],), {}], - ['rpsl_pks', ({'OTHER1-MNT', 'OTHER2-MNT', 'TEST-MNT'},), {}], + assert result_mntner.error_messages == [ + 'New mntner objects must be added by an administrator.', ] - def test_check_auth_invalid_create_mntner_referencing_self_with_dummy_passwords(self, prepare_mocks): - mock_dq, mock_dh = prepare_mocks - - mock_dh.execute_query = lambda query: [] - - reference_validator = ReferenceValidator(mock_dh) - auth_validator = AuthValidator(mock_dh) - - # Submit the mntner with dummy password values as would be returned by queries. - # This should not be allowed in new objects. - data = SAMPLE_MNTNER.replace('LEuuhsBJNFV0Q', PASSWORD_HASH_DUMMY_VALUE) - data = data.replace('$1$fgW84Y9r$kKEn9MUq8PChNKpQhO6BM.', PASSWORD_HASH_DUMMY_VALUE) - result_mntner = parse_change_requests(data + 'password: crypt-password', - mock_dh, auth_validator, reference_validator)[0] - auth_validator.pre_approve([result_mntner]) - - assert not result_mntner._check_auth() - assert result_mntner.error_messages == ['Authorisation failed for the auth methods on this mntner object.'] - assert flatten_mock_calls(mock_dq) == [ ['sources', (['TEST'],), {}], ['object_classes', (['mntner'],), {}], @@ -474,7 +444,6 @@ def test_check_auth_invalid_update_mntner_submits_new_object_with_mixed_dummy_ha result_mntner = parse_change_requests(data + 'password: md5-password', mock_dh, auth_validator, reference_validator)[0] auth_validator.pre_approve([result_mntner]) - result_mntner._check_auth() assert not result_mntner.is_valid() assert result_mntner.error_messages == [ 'Either all password auth hashes in a submitted mntner must be dummy objects, or none.', diff --git a/irrd/updates/validators.py b/irrd/updates/validators.py index ceb24c697..2c47f2ea8 100644 --- a/irrd/updates/validators.py +++ b/irrd/updates/validators.py @@ -209,8 +209,11 @@ def process_auth(self, rpsl_obj_new: RPSLObject, rpsl_obj_current: Optional[RPSL result.mntners_notify = mntner_objs_new if isinstance(rpsl_obj_new, RPSLMntner): - # Dummy auth values are only permitted in existing objects, which are never pre-approved. - if rpsl_obj_new.has_dummy_auth_value() and rpsl_obj_new.pk() not in self._pre_approved: + if not rpsl_obj_current: + result.error_messages.add('New mntner objects must be added by an administrator.') + return result + # Dummy auth values are only permitted in existing objects + if rpsl_obj_new.has_dummy_auth_value(): if len(self.passwords) == 1: logger.debug(f'Object {rpsl_obj_new} submitted with dummy hash values and single password, ' f'replacing all hashes with currently supplied password.')