From 6c4faf2f768dbc01de67db8b806cc74b48d999be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20R=C3=B6nnblom?= Date: Fri, 11 Nov 2022 17:18:37 +0100 Subject: [PATCH] Avoid clients erroneously keeping orphans from being removed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In case a client connects over TLS, the client's X.509 subject key identifier (SKI) will be used as a server-side user id. In case the client certificate changes (and the SKI with it) and the Pathfinder protocol connection is torn down and reestablished (e.g., due to a network outage), the client will typically connect under a new user id, but the same client id. The server currently accepts such clients, but does not allow them to republish services owned by the previous user id. The services published under the old user id would be marked as orphans, and will eventually be removed, allowing the new client to republish the services. This patch fixes a bug where such a reconnecting client would keep orphaned services alive upon disconnecting. This in turn, with current libpaf behavior, result in a situation where services published under the original user id would never be removed, if the service TTL was larger than the retry period. For a better solution than using SKI for access control, see issue #18. Signed-off-by: Mattias Rönnblom --- paf/sd.py | 8 ++++++-- test/test_paf.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/paf/sd.py b/paf/sd.py index b2c7fca..c544736 100644 --- a/paf/sd.py +++ b/paf/sd.py @@ -369,8 +369,12 @@ def client_disconnect(self, client_id): self.resource_manager.deallocate(user_id, ResourceType.CLIENT) now = time.time() for service in self.get_services_with_client_id(client_id): - with service.modify(): - service.make_orphan(now) + try: + service.check_access(user_id) + with service.modify(): + service.make_orphan(now) + except PermissionError: + pass client_subscriptions = \ list(self.get_subscriptions_with_client_id(client_id)) for subscription in client_subscriptions: diff --git a/test/test_paf.py b/test/test_paf.py index 3c58dc5..5585a5d 100644 --- a/test/test_paf.py +++ b/test/test_paf.py @@ -645,6 +645,43 @@ def test_republish_same_generation_orphan_from_different_client(server): run_republish_orphan(domain_addr, new_generation=False) +@pytest.mark.fast +def test_orphan_unaffected_by_same_client_wrong_user(tls_server): + domain_addr = tls_server.default_domain().default_addr() + client_id = client.allocate_client_id() + + os.environ['XCM_TLS_CERT'] = CLIENT_CERTS[0] + conn_owner = client.connect(domain_addr, client_id=client_id) + + service_id = conn_owner.service_id() + generation = 1 + service_props = { + "name": {"service-x"}, + } + service_ttl = 2 + + conn_owner.publish(service_id, generation, service_props, service_ttl) + + conn_owner.close() + + time.sleep(service_ttl / 2) + + os.environ['XCM_TLS_CERT'] = CLIENT_CERTS[1] + conn_non_owner = client.connect(domain_addr, client_id=client_id) + conn_non_owner.ping() + conn_non_owner.close() + + time.sleep(service_ttl / 2 + 0.25) + + conn = client.connect(domain_addr) + + # the non-owning client id should not affect the orphan status, and + # thus the service should be removed by now + assert len(conn.services()) == 0 + + conn.close() + + @pytest.mark.fast def test_reconnect_immediate_disconnect(server): domain_addr = server.random_domain().random_addr()