Avoid clients erroneously keeping orphans from being removed

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 <[email protected]>

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:

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()