Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add checking of NRTMv4 server history rewriting in our client #970

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions irrd/mirroring/nrtm4/nrtm4_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def _run_client(self) -> bool:
version=unf.version,
current_key=used_key,
next_key=unf.next_signing_key,
previous_file_hashes=self._validate_aggregate_previous_file_hashes_from_unf(unf),
)
if self.last_status != new_status:
self.database_handler.record_nrtm4_client_status(
Expand Down Expand Up @@ -194,7 +195,10 @@ def _deserialize_unf(self, unf_content: str) -> Tuple[bytes, str]:

def _current_db_status(self) -> Tuple[bool, NRTM4ClientDatabaseStatus]:
"""Look up the current status of self.source in the database."""
query = DatabaseStatusQuery().source(self.source)
query = DatabaseStatusQuery(
DatabaseStatusQuery.get_default_columns()
+ [DatabaseStatusQuery.columns.nrtm4_client_previous_file_hashes]
).source(self.source)
result = self.database_handler.execute_query(query)
try:
status = next(result)
Expand All @@ -203,9 +207,10 @@ def _current_db_status(self) -> Tuple[bool, NRTM4ClientDatabaseStatus]:
version=status["nrtm4_client_version"],
current_key=status["nrtm4_client_current_key"],
next_key=status["nrtm4_client_next_key"],
previous_file_hashes=status["nrtm4_client_previous_file_hashes"],
)
except StopIteration:
return False, NRTM4ClientDatabaseStatus(None, None, None, None)
return False, NRTM4ClientDatabaseStatus(None, None, None, None, None)

def _find_next_version(self, unf: NRTM4UpdateNotificationFile, last_version: Optional[int] = None):
"""
Expand Down Expand Up @@ -249,6 +254,32 @@ def _find_next_version(self, unf: NRTM4UpdateNotificationFile, last_version: Opt

return next_version

def _validate_aggregate_previous_file_hashes_from_unf(
self, unf: NRTM4UpdateNotificationFile
) -> Dict[str, List[str]]:
"""
Check if the server hasn't been rewriting history, which is obviously not allowed.
Also produces the new value for "previous_file_hashes"
"""
current_files = {f"snapshot-{unf.snapshot.version}": [str(unf.snapshot.url), unf.snapshot.hash]}
for delta in unf.deltas:
current_files[f"delta-{delta.version}"] = [str(delta.url), delta.hash]
if not self.last_status.previous_file_hashes:
return current_files

for current_file_reference, current_file_details in current_files.items():
if current_file_reference in self.last_status.previous_file_hashes:
previous_file_details = self.last_status.previous_file_hashes[current_file_reference]
if current_file_details != previous_file_details:
raise NRTM4ClientError(
f"{self.source}: Reference {current_file_reference} has filename"
f" '{current_file_details[0]}' with hash '{current_file_details[1]}' in current"
f" Update Notification File, but had filename '{previous_file_details[0]}' with hash"
f" '{previous_file_details[1]}' in a previous Update Notification File. Server is"
" rewriting history."
)
return current_files

def _load_snapshot(self, unf: NRTM4UpdateNotificationFile):
"""
Load a snapshot into the database.
Expand Down
80 changes: 76 additions & 4 deletions irrd/mirroring/nrtm4/tests/test_nrtm4_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,20 @@
MOCK_SESSION_ID = "ca128382-78d9-41d1-8927-1ecef15275be"

MOCK_SNAPSHOT_URL = "https://example.com/snapshot.2.json"
MOCK_SNAPSHOT_FILENAME = MOCK_SNAPSHOT_URL.split("/")[-1]
MOCK_DELTA3_URL = "https://example.com/delta.3.json"
MOCK_DELTA3_FILENAME = MOCK_DELTA3_URL.split("/")[-1]
MOCK_DELTA4_URL = "https://example.com/delta.4.json"
MOCK_DELTA4_FILENAME = MOCK_DELTA4_URL.split("/")[-1]
MOCK_UNF_URL = "https://example.com/" + UPDATE_NOTIFICATION_FILENAME

VALID_PREVIOUS_FILE_HASHES = {
# URL is actually reused as the hash in our test data
"snapshot-3": [MOCK_SNAPSHOT_FILENAME, MOCK_SNAPSHOT_URL],
"delta-3": [MOCK_DELTA3_FILENAME, MOCK_DELTA3_URL],
"delta-4": [MOCK_DELTA4_FILENAME, MOCK_DELTA4_URL],
}

MOCK_UNF = {
"nrtm_version": 4,
"timestamp": "2022-01-01T15:00:00Z",
Expand All @@ -34,18 +44,18 @@
"version": 4,
"snapshot": {
"version": 3,
"url": MOCK_SNAPSHOT_URL.split("/")[-1],
"url": MOCK_SNAPSHOT_FILENAME,
"hash": MOCK_SNAPSHOT_URL,
},
"deltas": [
{
"version": 3,
"url": MOCK_DELTA3_URL.split("/")[-1],
"url": MOCK_DELTA3_FILENAME,
"hash": MOCK_DELTA3_URL,
},
{
"version": 4,
"url": MOCK_DELTA4_URL.split("/")[-1],
"url": MOCK_DELTA4_FILENAME,
"hash": MOCK_DELTA4_URL,
},
],
Expand Down Expand Up @@ -136,6 +146,7 @@ def test_valid_from_snapshot(self, prepare_nrtm4_test, caplog):
"nrtm4_client_version": None,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -156,6 +167,8 @@ def test_valid_from_delta(self, prepare_nrtm4_test, caplog):
"nrtm4_client_version": 2,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
# Also tests the scenario for previous hashes, which do match
"nrtm4_client_previous_file_hashes": {"delta-3": [MOCK_DELTA3_FILENAME, MOCK_DELTA3_URL]},
}
]
)
Expand Down Expand Up @@ -202,6 +215,7 @@ def test_invalid_empty_delta(self, prepare_nrtm4_test, tmp_path, monkeypatch):
"nrtm4_client_version": 2,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand Down Expand Up @@ -231,6 +245,7 @@ def test_invalid_delta_key_error(self, prepare_nrtm4_test, tmp_path, monkeypatch
"nrtm4_client_version": 2,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -250,6 +265,7 @@ def test_invalid_unf_version_too_low(self, prepare_nrtm4_test):
"nrtm4_client_version": 6,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -269,6 +285,7 @@ def test_session_id_mismatch(self, prepare_nrtm4_test, caplog):
"nrtm4_client_version": 2,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -289,6 +306,7 @@ def test_delta_gap(self, prepare_nrtm4_test, caplog):
"nrtm4_client_version": 1,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -309,6 +327,7 @@ def test_force_reload(self, prepare_nrtm4_test, caplog):
"nrtm4_client_version": 2,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand Down Expand Up @@ -339,6 +358,7 @@ def test_valid_up_to_date(self, prepare_nrtm4_test, caplog):
"nrtm4_client_version": 4,
"nrtm4_client_current_key": None,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -353,6 +373,7 @@ def test_valid_up_to_date(self, prepare_nrtm4_test, caplog):
version=4,
current_key=MOCK_UNF_PUBLIC_KEY,
next_key=MOCK_UNF_PUBLIC_KEY_OTHER,
previous_file_hashes=VALID_PREVIOUS_FILE_HASHES,
),
},
),
Expand Down Expand Up @@ -392,6 +413,47 @@ def test_invalid_signature_from_config(self, prepare_nrtm4_test, config_override
NRTM4Client("TEST", mock_dh).run_client()
assert "any known keys" in str(exc)

def test_invalid_hash_change_history_rewrite(self, prepare_nrtm4_test, config_override):
mock_dh = MockDatabaseHandler()
mock_dh.reset_mock()
mock_dh.query_responses[DatabaseStatusQuery] = iter(
[
{
"force_reload": False,
"nrtm4_client_session_id": UUID(MOCK_SESSION_ID),
"nrtm4_client_version": 4,
"nrtm4_client_current_key": MOCK_UNF_PUBLIC_KEY,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": {
"delta-3": [MOCK_DELTA3_FILENAME, "incorrect-hash"]
},
}
]
)
with pytest.raises(NRTM4ClientError) as exc:
NRTM4Client("TEST", mock_dh).run_client()
assert "rewriting history" in str(exc)

def test_invalid_filename_change_history_rewrite(self, prepare_nrtm4_test, config_override):
mock_dh = MockDatabaseHandler()
mock_dh.reset_mock()
mock_dh.query_responses[DatabaseStatusQuery] = iter(
[
{
"force_reload": False,
"nrtm4_client_session_id": UUID(MOCK_SESSION_ID),
"nrtm4_client_version": 4,
"nrtm4_client_current_key": MOCK_UNF_PUBLIC_KEY,
"nrtm4_client_next_key": None,
# URL is used as hash in the mock data
"nrtm4_client_previous_file_hashes": {"delta-3": ["changed filename", MOCK_DELTA3_URL]},
}
]
)
with pytest.raises(NRTM4ClientError) as exc:
NRTM4Client("TEST", mock_dh).run_client()
assert "rewriting history" in str(exc)

def test_invalid_current_db_key_with_valid_config_key(self, prepare_nrtm4_test, config_override):
config_override(
{
Expand All @@ -416,6 +478,7 @@ def test_invalid_current_db_key_with_valid_config_key(self, prepare_nrtm4_test,
# Does not match, but must be used
"nrtm4_client_current_key": MOCK_UNF_PUBLIC_KEY_OTHER,
"nrtm4_client_next_key": MOCK_UNF_PUBLIC_KEY_OTHER,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand Down Expand Up @@ -446,6 +509,7 @@ def test_uses_current_db_key(self, prepare_nrtm4_test, config_override):
"nrtm4_client_version": 4,
"nrtm4_client_current_key": MOCK_UNF_PUBLIC_KEY,
"nrtm4_client_next_key": None,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand Down Expand Up @@ -475,6 +539,7 @@ def test_key_rotation(self, prepare_nrtm4_test, config_override, caplog):
# Does not match, but must be used
"nrtm4_client_current_key": MOCK_UNF_PUBLIC_KEY_OTHER,
"nrtm4_client_next_key": MOCK_UNF_PUBLIC_KEY,
"nrtm4_client_previous_file_hashes": None,
}
]
)
Expand All @@ -489,14 +554,20 @@ def test_key_rotation(self, prepare_nrtm4_test, config_override, caplog):
version=4,
current_key=MOCK_UNF_PUBLIC_KEY,
next_key=MOCK_UNF_PUBLIC_KEY_OTHER,
previous_file_hashes=VALID_PREVIOUS_FILE_HASHES,
),
},
),
]
assert "key rotated" in caplog.text

def _assert_import_queries(self, mock_dh, expect_reload=True):
assert mock_dh.queries == [DatabaseStatusQuery().source("TEST")]
assert mock_dh.queries == [
DatabaseStatusQuery(
DatabaseStatusQuery.get_default_columns()
+ [DatabaseStatusQuery.columns.nrtm4_client_previous_file_hashes]
).source("TEST")
]
expected = (
(
[
Expand Down Expand Up @@ -550,6 +621,7 @@ def _assert_import_queries(self, mock_dh, expect_reload=True):
version=4,
current_key=MOCK_UNF_PUBLIC_KEY,
next_key=MOCK_UNF_PUBLIC_KEY_OTHER,
previous_file_hashes=VALID_PREVIOUS_FILE_HASHES,
),
},
),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"""Add nrtm_client_previous_file_hashes field to database_status

Revision ID: e1e649b5f8bb
Revises: a635d2217a48
Create Date: 2024-11-08 17:39:40.872329

"""
import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = "e1e649b5f8bb"
down_revision = "a635d2217a48"
branch_labels = None
depends_on = None


def upgrade():
op.add_column(
"database_status",
sa.Column(
"nrtm4_client_previous_file_hashes", postgresql.JSONB(astext_type=sa.Text()), nullable=True
),
)


def downgrade():
op.drop_column("database_status", "nrtm4_client_previous_file_hashes")
1 change: 1 addition & 0 deletions irrd/storage/database_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,7 @@ def finalise_transaction(self):
nrtm4_client_version=status.version,
nrtm4_client_current_key=status.current_key,
nrtm4_client_next_key=status.next_key,
nrtm4_client_previous_file_hashes=status.previous_file_hashes,
rpsl_data_updated=datetime.now(timezone.utc),
)
)
Expand Down
2 changes: 2 additions & 0 deletions irrd/storage/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class NRTM4ClientDatabaseStatus:
version: Optional[int]
current_key: Optional[str]
next_key: Optional[str]
previous_file_hashes: Optional[Dict[str, List[str]]]


@dataclasses.dataclass
Expand Down Expand Up @@ -285,6 +286,7 @@ class RPSLDatabaseStatus(Base): # type: ignore
nrtm4_client_version = sa.Column(sa.Integer)
nrtm4_client_current_key = sa.Column(sa.Text)
nrtm4_client_next_key = sa.Column(sa.Text)
nrtm4_client_previous_file_hashes = sa.Column(pg.JSONB, nullable=True)

nrtm4_server_session_id = sa.Column(pg.UUID(as_uuid=True))
nrtm4_server_version = sa.Column(sa.Integer)
Expand Down
Loading