We currently allow joiners to use a snapshot from a predecessor service, without checking the signature on that snapshot, and then relying on later checks (and potentially waiting until consensus, for the merkle checks in signature verification) to recognise it was a legitimate place to start.
Sketching a timeline:
- Service A
- Node A.p
- Writes transactions 1 through 500
- Creates and writes `snapshot_495_500.committed`
- Dies
- Service B
- Node B.r
- Trying to recover from service A
- Finds `snapshot_495_500.committed` locally
- Confirms `snapshot_495_500.committed` was signed by A
- Recovers from `snapshot_495_500.committed`
- Writes `service_identity = B` in transaction 501
- Completes recovery and opens
- Writes transactions 501 through 600
- Node B.j
- Tries to join Service B (with no knowledge of A)
- Finds `snapshot_495_500.committed`
- Doesn't verify the signature on `snapshot_495_500.committed`
- Starts from `snapshot_495_500.committed`
- Submits a join request, is accepted
- Receives 496 through 600 via consensus
There's a risk that if Node B.j for some reason found a snapshot from a different service X (in practice, this is most likely to be some failed recovery B', but for these purposes its equivalent to a totally unrelated service), it doesn't recognise that until the last step here fails.
Specifically, in the code:
node_state.h : find_local_startup_snapshot()
for (const auto& [snapshot_seqno, snapshot_path] : committed_snapshots)
{
...
try
{
verify_snapshot(segments, config.recover.previous_service_identity);
}
NB: for a joiner, config.recover.previous_service_identity is std::nullopt.
This is specifically when looking for a local/already-held snapshot. The fetch path is different and always calls verify_snapshot(segments, join_config.service_cert); (ie - "the snapshot you[service] served me better be signed by you[service]").
We think we can improve this, by raising the lower-bound that the join-target uses to decide whether a joiner's startup seqno is "recent enough". Specifically, where we currently have a bound on "how many snapshots in the past" they may be, we should also require that this is a snapshot from the current service. This should be cheap to apply, falling back to the existing retry logic for fetches and joins if it fails. This may introduce a slight delay after recovery, where joiners must wait for a snapshot to be created before they can fetch it and join, but ensures they can locally verify that snapshot, and we never proceed with unverified contents.
We currently allow joiners to use a snapshot from a predecessor service, without checking the signature on that snapshot, and then relying on later checks (and potentially waiting until consensus, for the merkle checks in signature verification) to recognise it was a legitimate place to start.
Sketching a timeline:
There's a risk that if Node B.j for some reason found a snapshot from a different service X (in practice, this is most likely to be some failed recovery B', but for these purposes its equivalent to a totally unrelated service), it doesn't recognise that until the last step here fails.
Specifically, in the code:
node_state.h : find_local_startup_snapshot()
NB: for a joiner,
config.recover.previous_service_identityisstd::nullopt.This is specifically when looking for a local/already-held snapshot. The fetch path is different and always calls
verify_snapshot(segments, join_config.service_cert);(ie - "the snapshot you[service] served me better be signed by you[service]").We think we can improve this, by raising the lower-bound that the join-target uses to decide whether a joiner's startup seqno is "recent enough". Specifically, where we currently have a bound on "how many snapshots in the past" they may be, we should also require that this is a snapshot from the current service. This should be cheap to apply, falling back to the existing retry logic for fetches and joins if it fails. This may introduce a slight delay after recovery, where joiners must wait for a snapshot to be created before they can fetch it and join, but ensures they can locally verify that snapshot, and we never proceed with unverified contents.