Skip to content

Commit

Permalink
[4.3.x] Fix #891 - Fix legacy password hash support
Browse files Browse the repository at this point in the history
The auth line checker did correctly include legacy validators,
but this only happened after AuthValidator initialised the RPSLMntner
object with strict validation, discarding invalid auth lines.

(cherry picked from commit 2cec2ce)
  • Loading branch information
mxsasha committed Jan 16, 2024
1 parent 817caa5 commit 0fda596
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 79 deletions.
5 changes: 0 additions & 5 deletions irrd/rpsl/rpsl_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ def clean(self):
"Either all password auth hashes in a submitted mntner must be dummy objects, or none."
)

<<<<<<< HEAD
def verify_auth(self, passwords: List[str], keycert_obj_pk: Optional[str] = None) -> bool:
"""
Verify whether one of a given list of passwords matches
Expand All @@ -457,10 +456,6 @@ def verify_auth(self, passwords: List[str], keycert_obj_pk: Optional[str] = None
except ValueError:
pass
return False
=======
def verify_auth(self, passwords: List[str], keycert_obj_pk: Optional[str] = None) -> Optional[str]:
return verify_auth_lines(self.parsed_data.get("auth", []), passwords, keycert_obj_pk)
>>>>>>> eb51512 (Fix #891 - Gracefully handle auth for a mntner with no enabled methods)

def has_dummy_auth_value(self) -> bool:
"""
Expand Down
91 changes: 18 additions & 73 deletions irrd/updates/tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,99 +124,44 @@ def test_valid_new_person(self, prepare_mocks):
["rpsl_pks", ({"TEST-MNT"},), {}],
]

<<<<<<< HEAD
=======
def test_new_person_with_authless_mntner(self, prepare_mocks, config_override):
# "authless" meaning: no auth lines that are currently enabled - #891
def test_new_person_with_legacy_hash(self, prepare_mocks, config_override):
config_override(
{
"auth": {"password_hashers": {"crypt-pw": "disabled"}},
"auth": {"password_hashers": {"crypt-pw": "legacy"}},
}
)

validator, mock_dq, mock_dh = prepare_mocks
person = rpsl_object_from_text(SAMPLE_PERSON)
cryptonly_maintainer = "\n".join(
line for line in SAMPLE_MNTNER.splitlines() if not line.startswith("auth:") or "CRYPT" in line
)
print(cryptonly_maintainer)
mock_dh.execute_query = lambda q: [
{"object_class": "mntner", "object_text": cryptonly_maintainer},
{"object_class": "mntner", "object_text": SAMPLE_MNTNER},
]

validator.passwords = [SAMPLE_MNTNER_MD5]
validator.passwords = [SAMPLE_MNTNER_CRYPT]
result = validator.process_auth(person, None)
assert not result.is_valid()
assert result.is_valid()

def test_new_person_with_authless_mntner(self, prepare_mocks, config_override):
# "authless" meaning: no auth lines that are currently enabled - #891
config_override(
{
"auth": {"password_hashers": {"crypt-pw": "disabled"}},
}
)

def test_valid_new_person_api_key(self, prepare_mocks, monkeypatch):
validator, mock_dq, mock_dh = prepare_mocks
person = rpsl_object_from_text(SAMPLE_PERSON)
mock_mntner = AuthMntner(rpsl_mntner_pk="TEST-MNT")
mock_sa_session = UnifiedAlchemyMagicMock(
data=[
(
[
mock.call.query(AuthMntner),
mock.call.filter(
AuthMntner.rpsl_mntner_pk == "TEST-MNT", AuthMntner.rpsl_mntner_source == "TEST"
),
],
[mock_mntner],
)
]
cryptonly_maintainer = "\n".join(
line for line in SAMPLE_MNTNER.splitlines() if not line.startswith("auth:") or "CRYPT" in line
)
mock_api_key = AuthApiTokenFactory.build(mntner=mock_mntner)
monkeypatch.setattr("irrd.updates.validators.saorm.Session", lambda bind: mock_sa_session)

mock_dh._connection = None
mock_dh.execute_query = lambda q: [
{"object_class": "mntner", "object_text": SAMPLE_MNTNER},
{"object_class": "mntner", "object_text": cryptonly_maintainer},
]
mock_sa_session.all = lambda: [mock_api_key]

validator.api_keys = ["key"]
result = validator.process_auth(person, None)
assert result.is_valid(), result.error_messages
assert result.mntners_notify[0].pk() == "TEST-MNT"
assert result.auth_method == AuthMethod.MNTNER_API_KEY
assert result.auth_through_mntner == "TEST-MNT"
assert result.auth_through_internal_mntner.rpsl_mntner_pk == "TEST-MNT"

mock_sa_session.filter.assert_has_calls(
[
mock.call(
AuthMntner.rpsl_mntner_pk == "TEST-MNT",
AuthMntner.rpsl_mntner_source == "TEST",
AuthApiToken.token.in_(["key"]),
),
]
)

validator.origin = AuthoritativeChangeOrigin.webui
result = validator.process_auth(person, None)
assert not result.is_valid(), result.error_messages

validator.origin = AuthoritativeChangeOrigin.email
result = validator.process_auth(person, None)
assert result.is_valid(), result.error_messages

mock_api_key.enabled_email = False
validator.origin = AuthoritativeChangeOrigin.email
result = validator.process_auth(person, None)
assert not result.is_valid(), result.error_messages

mock_api_key.ip_restriction = "192.0.2.0/26,192.0.2.64/26"
validator.remote_ip = IP("192.0.2.1")
validator.origin = AuthoritativeChangeOrigin.webapi
result = validator.process_auth(person, None)
assert result.is_valid(), result.error_messages

validator.remote_ip = IP("192.0.2.200")
validator.origin = AuthoritativeChangeOrigin.webapi
validator.passwords = [SAMPLE_MNTNER_CRYPT]
result = validator.process_auth(person, None)
assert not result.is_valid(), result.error_messages
assert not result.is_valid()

>>>>>>> eb51512 (Fix #891 - Gracefully handle auth for a mntner with no enabled methods)
def test_existing_person_mntner_change(self, prepare_mocks):
validator, mock_dq, mock_dh = prepare_mocks
# TEST-MNT is in both maintainers
Expand Down
2 changes: 1 addition & 1 deletion irrd/updates/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def _check_mntners(self, mntner_pk_list: List[str], source: str) -> Tuple[bool,
query = query.object_classes(["mntner"]).rpsl_pks(mntner_pks_to_resolve)
results = self.database_handler.execute_query(query)

retrieved_mntner_objs: List[RPSLMntner] = [rpsl_object_from_text(r["object_text"]) for r in results] # type: ignore
retrieved_mntner_objs: List[RPSLMntner] = [rpsl_object_from_text(r["object_text"], strict_validation=False) for r in results] # type: ignore
self._mntner_db_cache.update(retrieved_mntner_objs)
mntner_objs += retrieved_mntner_objs

Expand Down

0 comments on commit 0fda596

Please sign in to comment.