Pentest fixes: pin the runtime source host, evict rather than refuse; say on the page that llama.cpp is fetched
Browse files- distinct_agent/runtime.py +41 -13
- distinct_server/control_plane.py +31 -5
- distinct_server/models.py +1 -1
- distinct_server/render.py +32 -24
- tests/test_claim_flow.py +38 -5
distinct_agent/runtime.py
CHANGED
|
@@ -89,21 +89,42 @@ class RuntimeVerificationError(RuntimeUnavailable):
|
|
| 89 |
"""The bytes arrived but did not match the recorded digest."""
|
| 90 |
|
| 91 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 92 |
class _RuntimeRedirects(urllib.request.HTTPRedirectHandler):
|
| 93 |
max_redirections = 10
|
| 94 |
|
| 95 |
-
|
| 96 |
-
def _permitted(url: str) -> bool:
|
| 97 |
-
parsed = urllib.parse.urlsplit(url)
|
| 98 |
-
if parsed.scheme != "https":
|
| 99 |
-
return False
|
| 100 |
-
host = (parsed.hostname or "").casefold().rstrip(".")
|
| 101 |
-
if not host:
|
| 102 |
-
return False
|
| 103 |
-
return any(
|
| 104 |
-
host == allowed or host.endswith("." + allowed)
|
| 105 |
-
for allowed in RUNTIME_REDIRECT_HOSTS
|
| 106 |
-
)
|
| 107 |
|
| 108 |
def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: D102
|
| 109 |
if not self._permitted(newurl):
|
|
@@ -134,7 +155,7 @@ class RuntimePin:
|
|
| 134 |
|
| 135 |
@property
|
| 136 |
def recorded(self) -> bool:
|
| 137 |
-
return bool(self.url and len(self.sha256) == 64)
|
| 138 |
|
| 139 |
|
| 140 |
def platform_key(system: str = "", machine: str = "") -> str:
|
|
@@ -267,6 +288,13 @@ def ensure(
|
|
| 267 |
return existing
|
| 268 |
|
| 269 |
resolved = key or platform_key()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 270 |
pin = pin_for(resolved, pins_path)
|
| 271 |
if pin is None:
|
| 272 |
raise RuntimeUnavailable(
|
|
|
|
| 89 |
"""The bytes arrived but did not match the recorded digest."""
|
| 90 |
|
| 91 |
|
| 92 |
+
def permitted_source(url: str) -> bool:
|
| 93 |
+
"""Whether this worker is willing to fetch a runtime from ``url`` at all.
|
| 94 |
+
|
| 95 |
+
THE FIRST URL NEEDS THE SAME CHECK AS EVERY REDIRECT.
|
| 96 |
+
|
| 97 |
+
The redirect handler below refuses to follow a hop off GitHub, which is the
|
| 98 |
+
obvious half of the rule; the half that a penetration test found missing was
|
| 99 |
+
the URL the download *starts* at. That one comes out of a JSON file, so
|
| 100 |
+
anybody who could alter ``llama_runtime.json`` -- a bad commit, a merged
|
| 101 |
+
pull request nobody read closely, a tampered checkout -- could point every
|
| 102 |
+
volunteer's worker at a host of their choosing and have it make the request.
|
| 103 |
+
|
| 104 |
+
The recorded digest still means no substituted *file* is ever installed, so
|
| 105 |
+
this was not a path to running someone else's code. It was still an
|
| 106 |
+
arbitrary outbound request made by every worker that updated, which is a
|
| 107 |
+
thing worth having and not a thing worth leaving lying around. Now the
|
| 108 |
+
allowlist is applied where the URL is read, so a pin pointing anywhere else
|
| 109 |
+
is not a download that fails: it is a pin that does not count as recorded.
|
| 110 |
+
"""
|
| 111 |
+
|
| 112 |
+
parsed = urllib.parse.urlsplit(url)
|
| 113 |
+
if parsed.scheme != "https":
|
| 114 |
+
return False
|
| 115 |
+
host = (parsed.hostname or "").casefold().rstrip(".")
|
| 116 |
+
if not host:
|
| 117 |
+
return False
|
| 118 |
+
return any(
|
| 119 |
+
host == allowed or host.endswith("." + allowed)
|
| 120 |
+
for allowed in RUNTIME_REDIRECT_HOSTS
|
| 121 |
+
)
|
| 122 |
+
|
| 123 |
+
|
| 124 |
class _RuntimeRedirects(urllib.request.HTTPRedirectHandler):
|
| 125 |
max_redirections = 10
|
| 126 |
|
| 127 |
+
_permitted = staticmethod(permitted_source)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 128 |
|
| 129 |
def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: D102
|
| 130 |
if not self._permitted(newurl):
|
|
|
|
| 155 |
|
| 156 |
@property
|
| 157 |
def recorded(self) -> bool:
|
| 158 |
+
return bool(self.url and len(self.sha256) == 64 and permitted_source(self.url))
|
| 159 |
|
| 160 |
|
| 161 |
def platform_key(system: str = "", machine: str = "") -> str:
|
|
|
|
| 288 |
return existing
|
| 289 |
|
| 290 |
resolved = key or platform_key()
|
| 291 |
+
raw = load_pins(pins_path).get(resolved)
|
| 292 |
+
if raw is not None and raw.url and not permitted_source(raw.url):
|
| 293 |
+
raise RuntimeUnavailable(
|
| 294 |
+
f"the recorded llama.cpp source for {resolved} is {raw.url!r}, which is "
|
| 295 |
+
"not GitHub. Nothing was fetched. A pin that points anywhere else has "
|
| 296 |
+
"been tampered with; report it rather than working around it."
|
| 297 |
+
)
|
| 298 |
pin = pin_for(resolved, pins_path)
|
| 299 |
if pin is None:
|
| 300 |
raise RuntimeUnavailable(
|
distinct_server/control_plane.py
CHANGED
|
@@ -367,11 +367,37 @@ class ControlPlane:
|
|
| 367 |
except ValueError as exc:
|
| 368 |
raise ValidationError("claim code is not a usable code") from exc
|
| 369 |
|
| 370 |
-
|
| 371 |
-
|
| 372 |
-
|
| 373 |
-
|
| 374 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 375 |
if len(self._agents) >= self.limits.max_agents:
|
| 376 |
raise CapacityError("agent registry is full")
|
| 377 |
if any(
|
|
|
|
| 367 |
except ValueError as exc:
|
| 368 |
raise ValidationError("claim code is not a usable code") from exc
|
| 369 |
|
| 370 |
+
# A FULL POOL MUST NOT BE A CLOSED DOOR.
|
| 371 |
+
#
|
| 372 |
+
# A penetration test filled this pool from one unauthenticated
|
| 373 |
+
# client at about six registrations a second, and the ceiling then
|
| 374 |
+
# did exactly what a ceiling does: it refused everybody, including
|
| 375 |
+
# the volunteer who had just started a worker for the first time.
|
| 376 |
+
# Protecting the registry by turning the feature off for real users
|
| 377 |
+
# is the wrong trade, and it is the trade an attacker was choosing.
|
| 378 |
+
#
|
| 379 |
+
# So a full pool evicts its oldest waiter instead. A legitimate
|
| 380 |
+
# registration now always succeeds; what a flood costs is the slot
|
| 381 |
+
# of whoever registered longest ago, and a flood mostly evicts its
|
| 382 |
+
# own earlier entries because it owns nearly all of them.
|
| 383 |
+
#
|
| 384 |
+
# RESIDUAL RISK, STATED RATHER THAN PAPERED OVER. Sustained flooding
|
| 385 |
+
# can still evict a real worker in the window between it printing a
|
| 386 |
+
# code and its owner typing that code in. The worker says so, in
|
| 387 |
+
# those words, and starting it again gives a fresh code. Closing
|
| 388 |
+
# that gap properly needs per-source limiting, which this layer
|
| 389 |
+
# cannot do: it has no address, by design. The place for it is the
|
| 390 |
+
# route in front, and it is not pretended to be here.
|
| 391 |
+
waiting = sorted(
|
| 392 |
+
(
|
| 393 |
+
(record.claim_expires_at, agent_id)
|
| 394 |
+
for agent_id, record in self._agents.items()
|
| 395 |
+
if record.claim_digest
|
| 396 |
+
),
|
| 397 |
+
)
|
| 398 |
+
while len(waiting) >= self.limits.max_unclaimed_agents:
|
| 399 |
+
_, oldest = waiting.pop(0)
|
| 400 |
+
del self._agents[oldest]
|
| 401 |
if len(self._agents) >= self.limits.max_agents:
|
| 402 |
raise CapacityError("agent registry is full")
|
| 403 |
if any(
|
distinct_server/models.py
CHANGED
|
@@ -81,7 +81,7 @@ class ControlPlaneLimits:
|
|
| 81 |
#: How long an unclaimed worker waits to be claimed before the server
|
| 82 |
#: forgets it. Long enough to walk to the browser, short enough that an
|
| 83 |
#: abandoned worker does not hold a slot.
|
| 84 |
-
claim_ttl_seconds: float =
|
| 85 |
poll_interval_seconds: float = 5.0
|
| 86 |
missed_polls_before_offline: int = 3
|
| 87 |
# A floor on the offline threshold so that lowering the poll interval
|
|
|
|
| 81 |
#: How long an unclaimed worker waits to be claimed before the server
|
| 82 |
#: forgets it. Long enough to walk to the browser, short enough that an
|
| 83 |
#: abandoned worker does not hold a slot.
|
| 84 |
+
claim_ttl_seconds: float = 600.0
|
| 85 |
poll_interval_seconds: float = 5.0
|
| 86 |
missed_polls_before_offline: int = 3
|
| 87 |
# A floor on the offline threshold so that lowering the poll interval
|
distinct_server/render.py
CHANGED
|
@@ -336,32 +336,40 @@ def join_this_server(
|
|
| 336 |
"a <code>setup.py</code> this project does not have.</p>"
|
| 337 |
)
|
| 338 |
|
| 339 |
-
# STEP THREE
|
| 340 |
-
# MENTIONED.
|
| 341 |
#
|
| 342 |
-
#
|
| 343 |
-
#
|
| 344 |
-
#
|
| 345 |
-
#
|
| 346 |
-
#
|
|
|
|
|
|
|
|
|
|
| 347 |
#
|
| 348 |
-
#
|
| 349 |
-
#
|
| 350 |
-
#
|
| 351 |
-
#
|
| 352 |
-
#
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 353 |
runner = (
|
| 354 |
-
'<p class="c-join__note">
|
| 355 |
-
"
|
| 356 |
-
|
| 357 |
-
|
| 358 |
-
"
|
| 359 |
-
|
| 360 |
-
"
|
| 361 |
-
|
| 362 |
-
"<code>--
|
| 363 |
-
"
|
| 364 |
-
"
|
|
|
|
| 365 |
)
|
| 366 |
|
| 367 |
# STEP FOUR: the run, with this server's address and this browser's own
|
|
@@ -408,7 +416,7 @@ def join_this_server(
|
|
| 408 |
f"{install}</div></li>"
|
| 409 |
'<li class="c-step-num"><span class="c-step-num__n">3</span>'
|
| 410 |
'<div class="c-step-num__body">'
|
| 411 |
-
'<p class="c-step-num__head">
|
| 412 |
f"{runner}</div></li>"
|
| 413 |
'<li class="c-step-num"><span class="c-step-num__n">4</span>'
|
| 414 |
'<div class="c-step-num__body"><p class="c-step-num__head">Run it, and claim it</p>'
|
|
|
|
| 336 |
"a <code>setup.py</code> this project does not have.</p>"
|
| 337 |
)
|
| 338 |
|
| 339 |
+
# STEP THREE IS NOT "GO AND FETCH A DEPENDENCY".
|
|
|
|
| 340 |
#
|
| 341 |
+
# It was, briefly, and that was a bad answer to a fair complaint. The panel
|
| 342 |
+
# first said nothing about llama.cpp at all, so a volunteer got through
|
| 343 |
+
# every step and then met "this worker has no llama.cpp to run a model
|
| 344 |
+
# with" -- a requirement the page had never named. The fix for that was a
|
| 345 |
+
# step telling them to go to another project's releases page, pick the
|
| 346 |
+
# right build out of a dozen, unzip it and edit their PATH, and the
|
| 347 |
+
# reaction to that was the right one: *why do I have to provide this, it
|
| 348 |
+
# should be baked into the repo*.
|
| 349 |
#
|
| 350 |
+
# It cannot be in the repository -- a llama.cpp release is around a hundred
|
| 351 |
+
# megabytes per platform per accelerator, and a Space clone would carry all
|
| 352 |
+
# of them -- but "not in the repository" was never the same thing as "your
|
| 353 |
+
# problem". The worker now fetches the pinned build itself and verifies it
|
| 354 |
+
# against a recorded SHA-256 before installing it, exactly as it already
|
| 355 |
+
# did for model weights. See distinct_agent/runtime.py.
|
| 356 |
+
#
|
| 357 |
+
# So this step is no longer an instruction. It is a description of what the
|
| 358 |
+
# command in the next step will do, which is here because a worker that
|
| 359 |
+
# quietly downloads twenty megabytes should say so first.
|
| 360 |
runner = (
|
| 361 |
+
'<p class="c-join__note">Nothing to install by hand. A model needs '
|
| 362 |
+
"<strong>llama.cpp</strong> to run it, and the worker fetches the pinned "
|
| 363 |
+
"build on its first start — about 20 MB — then checks it against a "
|
| 364 |
+
"recorded SHA-256 before installing it. If the bytes do not match, "
|
| 365 |
+
"nothing is installed and it says so.</p>"
|
| 366 |
+
'<p class="c-join__note">Already have a build you would rather use, or '
|
| 367 |
+
"want a CUDA or Metal one? Pass "
|
| 368 |
+
"<code>--llama-server /path/to/llama-server</code> and nothing is "
|
| 369 |
+
"downloaded. On a metered connection, <code>--no-fetch-runtime</code> "
|
| 370 |
+
"stops it fetching anything. And <code>--demo-runner</code> starts the "
|
| 371 |
+
"worker with no model at all — it still joins, and still runs every tool "
|
| 372 |
+
"and skill in the library.</p>"
|
| 373 |
)
|
| 374 |
|
| 375 |
# STEP FOUR: the run, with this server's address and this browser's own
|
|
|
|
| 416 |
f"{install}</div></li>"
|
| 417 |
'<li class="c-step-num"><span class="c-step-num__n">3</span>'
|
| 418 |
'<div class="c-step-num__body">'
|
| 419 |
+
'<p class="c-step-num__head">What it needs to run a model</p>'
|
| 420 |
f"{runner}</div></li>"
|
| 421 |
'<li class="c-step-num"><span class="c-step-num__n">4</span>'
|
| 422 |
'<div class="c-step-num__body"><p class="c-step-num__head">Run it, and claim it</p>'
|
tests/test_claim_flow.py
CHANGED
|
@@ -111,15 +111,48 @@ def test_an_unclaimed_worker_expires(plane: ControlPlane) -> None:
|
|
| 111 |
plane.claim_agent(code, "user:alice", now=later)
|
| 112 |
|
| 113 |
|
| 114 |
-
def
|
| 115 |
"""The one unauthenticated write on this server cannot be accumulated."""
|
| 116 |
|
| 117 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 118 |
with pytest.raises(CapacityError):
|
| 119 |
-
|
| 120 |
-
|
|
|
|
|
|
|
| 121 |
made += 1
|
| 122 |
-
assert made == plane.limits.
|
| 123 |
|
| 124 |
|
| 125 |
def test_expiry_frees_the_ceiling(plane: ControlPlane) -> None:
|
|
|
|
| 111 |
plane.claim_agent(code, "user:alice", now=later)
|
| 112 |
|
| 113 |
|
| 114 |
+
def test_the_unclaimed_pool_is_bounded(plane: ControlPlane) -> None:
|
| 115 |
"""The one unauthenticated write on this server cannot be accumulated."""
|
| 116 |
|
| 117 |
+
for _ in range(plane.limits.max_unclaimed_agents * 3):
|
| 118 |
+
plane.register_unclaimed(_caps(), new_pairing_code())
|
| 119 |
+
waiting = sum(1 for r in plane._agents.values() if r.claim_digest)
|
| 120 |
+
assert waiting <= plane.limits.max_unclaimed_agents
|
| 121 |
+
|
| 122 |
+
|
| 123 |
+
def test_a_flood_never_locks_a_real_volunteer_out(plane: ControlPlane) -> None:
|
| 124 |
+
"""A full pool evicts its oldest waiter rather than refusing everybody.
|
| 125 |
+
|
| 126 |
+
Refusing is what an attacker filling the pool is trying to buy: the feature
|
| 127 |
+
turned off for real users. The oldest waiter loses its slot instead, and a
|
| 128 |
+
flood mostly evicts its own earlier entries.
|
| 129 |
+
"""
|
| 130 |
+
|
| 131 |
+
for index in range(plane.limits.max_unclaimed_agents):
|
| 132 |
+
plane.register_unclaimed(_caps(f"flood{index}"), new_pairing_code(), now=1000.0 + index)
|
| 133 |
+
|
| 134 |
+
mine = new_pairing_code()
|
| 135 |
+
credential = plane.register_unclaimed(_caps("volunteer"), mine, now=2000.0)
|
| 136 |
+
assert plane.claim_state(credential.agent_id, now=2000.0) == "waiting"
|
| 137 |
+
assert plane.claim_agent(mine, "user:alice", now=2000.0) == credential.agent_id
|
| 138 |
+
|
| 139 |
+
|
| 140 |
+
def test_the_registry_itself_still_has_a_hard_ceiling(plane: ControlPlane) -> None:
|
| 141 |
+
"""Eviction bounds the unclaimed pool; claimed agents are still capped."""
|
| 142 |
+
|
| 143 |
+
codes = [new_pairing_code() for _ in range(plane.limits.max_unclaimed_agents)]
|
| 144 |
+
for index, code in enumerate(codes):
|
| 145 |
+
plane.register_unclaimed(_caps(f"w{index}"), code)
|
| 146 |
+
for index, code in enumerate(codes):
|
| 147 |
+
plane.claim_agent(code, f"user:{index}")
|
| 148 |
+
made = len(codes)
|
| 149 |
with pytest.raises(CapacityError):
|
| 150 |
+
while made < plane.limits.max_agents + 10:
|
| 151 |
+
code = new_pairing_code()
|
| 152 |
+
plane.register_unclaimed(_caps(), code)
|
| 153 |
+
plane.claim_agent(code, "user:alice")
|
| 154 |
made += 1
|
| 155 |
+
assert made == plane.limits.max_agents
|
| 156 |
|
| 157 |
|
| 158 |
def test_expiry_frees_the_ceiling(plane: ControlPlane) -> None:
|