feat: harden remote serve and reuse connections
Testing / remote-protocol-compat (0.9.5) (push) Successful in 56s
Testing / remote-protocol-compat (0.9.3) (push) Successful in 59s
Testing / test (push) Successful in 1m1s
Build & Publish Package / publish (push) Successful in 33s
Package Extension / package-extension (push) Successful in 36s
Testing / remote-protocol-compat (0.9.5) (push) Successful in 56s
Testing / remote-protocol-compat (0.9.3) (push) Successful in 59s
Testing / test (push) Successful in 1m1s
Build & Publish Package / publish (push) Successful in 33s
Package Extension / package-extension (push) Successful in 36s
- Gate TCP serve commands with safe-by-default policies, per-key allow tokens, per-key rate limiting, and audit labels. - Reuse authenticated encrypted remote sessions and parallelize/caches multi-browser fanout to reduce repeated handshake roundtrips. - Increase paged native-host batch size with extension-side byte budgeting to speed large tab listings safely. - Point install output at public Chrome Web Store / Firefox AMO listings by default, with --dev preserving unpacked workflows. - Share search-engine metadata between CLI and SDK and bump the package/extension version to 0.16.0. - Cover the new security, pooling, paging, install, and fanout behavior with expanded Python and extension tests.
This commit is contained in:
@@ -10,6 +10,7 @@ class ServeControlMixin:
|
||||
addr: tuple
|
||||
command: str
|
||||
auth_keys_path: Path | None
|
||||
auth_label: str | None
|
||||
|
||||
async def send_error(self, msg: str, msg_id=None) -> None: ...
|
||||
async def send_ok(self, payload, command: str | None = None) -> None: ...
|
||||
@@ -23,25 +24,32 @@ class ServeControlMixin:
|
||||
try:
|
||||
clients = send_command("clients.list", profile=target.profile, suppress_pq_warning=True)
|
||||
if clients:
|
||||
browser_name = clients[0].get("name")
|
||||
if browser_name:
|
||||
item["browserName"] = browser_name
|
||||
# Carry the full client info so a remote `clients` command can render
|
||||
# from this single roundtrip instead of issuing another clients.list.
|
||||
info = clients[0]
|
||||
for src, dst in (("name", "browserName"), ("version", "version"), ("extensionVersion", "extensionVersion")):
|
||||
value = info.get(src)
|
||||
if value:
|
||||
item[dst] = value
|
||||
except Exception:
|
||||
pass
|
||||
targets.append(item)
|
||||
await self.send_ok(targets, self.command)
|
||||
log_request(self.addr, self.command, None, "OK")
|
||||
log_request(self.addr, self.command, None, "OK", identity=self.auth_label)
|
||||
return True
|
||||
|
||||
if self.command == "browser-cli.auth.keys":
|
||||
if self.auth_keys_path is None:
|
||||
await self.send_error("no authorized keys file configured on this server")
|
||||
log_request(self.addr, self.command, None, "ERROR", "no authorized keys file")
|
||||
log_request(self.addr, self.command, None, "ERROR", "no authorized keys file", identity=self.auth_label)
|
||||
return True
|
||||
from browser_cli.auth import load_authorized_keys_with_names
|
||||
entries = [{"pubkey": pk, "name": name} for pk, name in load_authorized_keys_with_names(self.auth_keys_path)]
|
||||
from browser_cli.auth import load_authorized_keys_with_policies
|
||||
entries = [
|
||||
{"pubkey": pk, "name": name, "allow": cats}
|
||||
for pk, name, cats in load_authorized_keys_with_policies(self.auth_keys_path)
|
||||
]
|
||||
await self.send_ok(entries, self.command)
|
||||
log_request(self.addr, self.command, None, "OK")
|
||||
log_request(self.addr, self.command, None, "OK", identity=self.auth_label)
|
||||
return True
|
||||
|
||||
if self.command == "browser-cli.auth.trust":
|
||||
@@ -54,14 +62,27 @@ class ServeControlMixin:
|
||||
log_request(self.addr, self.command, None, "ERROR", "no authorized keys file")
|
||||
return True
|
||||
from browser_cli.auth import add_authorized_key
|
||||
from browser_cli.serve.security import policy_from_categories
|
||||
args = msg.get("args") or {}
|
||||
pubkey = str(args.get("pubkey") or "")
|
||||
name = str(args.get("name") or "")
|
||||
categories = args.get("allow")
|
||||
if not re.fullmatch(r"[0-9a-f]{64}", pubkey):
|
||||
await self.send_error("invalid pubkey: expected 64 lowercase hex characters")
|
||||
log_request(self.addr, self.command, None, "ERROR", "invalid pubkey")
|
||||
log_request(self.addr, self.command, None, "ERROR", "invalid pubkey", identity=self.auth_label)
|
||||
return True
|
||||
added = add_authorized_key(self.auth_keys_path, pubkey, name)
|
||||
if categories is not None:
|
||||
if not isinstance(categories, list):
|
||||
await self.send_error("invalid allow: expected a list of category strings")
|
||||
log_request(self.addr, self.command, None, "ERROR", "invalid allow", identity=self.auth_label)
|
||||
return True
|
||||
try:
|
||||
policy_from_categories(categories) # validate before persisting
|
||||
except ValueError as exc:
|
||||
await self.send_error(str(exc))
|
||||
log_request(self.addr, self.command, None, "ERROR", "invalid allow category", identity=self.auth_label)
|
||||
return True
|
||||
added = add_authorized_key(self.auth_keys_path, pubkey, name, categories)
|
||||
await self.send_ok({"added": added}, self.command)
|
||||
log_request(self.addr, self.command, None, "OK" if added else "ALREADY_TRUSTED")
|
||||
log_request(self.addr, self.command, None, "OK" if added else "ALREADY_TRUSTED", identity=self.auth_label)
|
||||
return True
|
||||
|
||||
@@ -6,11 +6,19 @@ from rich.console import Console
|
||||
|
||||
console = Console()
|
||||
|
||||
def log_request(addr: tuple, command: str, profile: str | None, status: str, error: str | None = None) -> None:
|
||||
def log_request(
|
||||
addr: tuple,
|
||||
command: str,
|
||||
profile: str | None,
|
||||
status: str,
|
||||
error: str | None = None,
|
||||
identity: str | None = None,
|
||||
) -> None:
|
||||
ts = datetime.now().strftime("%H:%M:%S")
|
||||
addr_str = f"{addr[0]}:{addr[1]}"
|
||||
identity_str = f"[magenta]{identity}[/magenta] " if identity else ""
|
||||
profile_str = f"[dim]{profile}[/dim] " if profile else ""
|
||||
if error:
|
||||
console.print(f"[dim]{ts}[/dim] {addr_str} {profile_str}[cyan]{command}[/cyan] [red]{status}[/red] {error}")
|
||||
console.print(f"[dim]{ts}[/dim] {addr_str} {identity_str}{profile_str}[cyan]{command}[/cyan] [red]{status}[/red] {error}")
|
||||
else:
|
||||
console.print(f"[dim]{ts}[/dim] {addr_str} {profile_str}[cyan]{command}[/cyan] [green]{status}[/green]")
|
||||
console.print(f"[dim]{ts}[/dim] {addr_str} {identity_str}{profile_str}[cyan]{command}[/cyan] [green]{status}[/green]")
|
||||
|
||||
@@ -18,6 +18,7 @@ class ServeProxyMixin:
|
||||
command: str
|
||||
compress: bool
|
||||
accept_encoding: dict | None
|
||||
auth_label: str | None
|
||||
|
||||
async def send_error(self, msg: str, msg_id=None) -> None: ...
|
||||
async def send_payload(self, data: bytes) -> None: ...
|
||||
@@ -35,7 +36,7 @@ class ServeProxyMixin:
|
||||
sock_path = resolve_socket(resolved_profile)
|
||||
except BrowserNotConnected as e:
|
||||
await self.send_error(str(e))
|
||||
log_request(self.addr, self.command, resolved_profile, "ERROR", "browser not connected")
|
||||
log_request(self.addr, self.command, resolved_profile, "ERROR", "browser not connected", identity=self.auth_label)
|
||||
return
|
||||
|
||||
try:
|
||||
@@ -46,7 +47,7 @@ class ServeProxyMixin:
|
||||
await self.send_browser_response(adapt_response(resp_payload, self.command, self.client_ver), resolved_profile)
|
||||
except (OSError, json.JSONDecodeError, ConnectionError) as e:
|
||||
await self.send_error(str(e))
|
||||
log_request(self.addr, self.command, resolved_profile, "ERROR", str(e))
|
||||
log_request(self.addr, self.command, resolved_profile, "ERROR", str(e), identity=self.auth_label)
|
||||
|
||||
async def _windows_roundtrip(self, sock_path: str, payload: bytes) -> bytes:
|
||||
from multiprocessing.connection import Client as PipeClient
|
||||
@@ -74,6 +75,6 @@ class ServeProxyMixin:
|
||||
else:
|
||||
await self.send_payload(resp_payload)
|
||||
if resp_data.get("success", True):
|
||||
log_request(self.addr, self.command, resolved_profile, "OK")
|
||||
log_request(self.addr, self.command, resolved_profile, "OK", identity=self.auth_label)
|
||||
else:
|
||||
log_request(self.addr, self.command, resolved_profile, "ERROR", resp_data.get("error", ""))
|
||||
log_request(self.addr, self.command, resolved_profile, "ERROR", resp_data.get("error", ""), identity=self.auth_label)
|
||||
|
||||
@@ -9,17 +9,20 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import json
|
||||
import socket
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
|
||||
from browser_cli import transport
|
||||
from browser_cli.command_security import assert_command_allowed
|
||||
from browser_cli.compat import adapt_auth
|
||||
from browser_cli.constants import REMOTE_SESSION_IDLE_TIMEOUT
|
||||
from browser_cli.framing import async_recv_frame, async_send_frame
|
||||
from browser_cli.serve.auth import ServeAuthMixin
|
||||
from browser_cli.serve.challenge import build_challenge as _build_challenge, load_auth_keys as _load_auth_keys
|
||||
from browser_cli.serve.control import ServeControlMixin
|
||||
from browser_cli.serve.logging import console, log_request
|
||||
from browser_cli.serve.proxy import ServeProxyMixin
|
||||
from browser_cli.serve.security import ServeSecurity
|
||||
|
||||
async def _async_framed_send(writer: asyncio.StreamWriter, data: bytes) -> None:
|
||||
await async_send_frame(writer, data)
|
||||
@@ -38,12 +41,15 @@ class ServeRequest(ServeAuthMixin, ServeControlMixin, ServeProxyMixin):
|
||||
nonce: str
|
||||
pq_private_key: object | None = None
|
||||
compress: bool = True
|
||||
security: ServeSecurity = field(default_factory=ServeSecurity)
|
||||
|
||||
response_secret: bytes | None = None
|
||||
accept_encoding: dict | None = None
|
||||
client_ver: str = "0"
|
||||
msg_id: object = None
|
||||
command: str = "?"
|
||||
auth_pubkey: str | None = None
|
||||
auth_label: str | None = None
|
||||
|
||||
async def send_payload(self, data: bytes) -> None:
|
||||
if self.response_secret is not None:
|
||||
@@ -89,11 +95,73 @@ class ServeRequest(ServeAuthMixin, ServeControlMixin, ServeProxyMixin):
|
||||
msg = await self.authenticate(msg)
|
||||
if msg is None:
|
||||
return
|
||||
self._apply_identity(msg)
|
||||
await self._dispatch(msg)
|
||||
# Once an encrypted session is established, keep serving further commands on
|
||||
# the same connection — the client may reuse it without re-authenticating.
|
||||
# Safe because every frame carries a fresh AEAD nonce (see pq_encrypt).
|
||||
while self.response_secret is not None:
|
||||
nxt = await self._read_session_message()
|
||||
if nxt is None:
|
||||
return
|
||||
await self._dispatch(nxt)
|
||||
|
||||
def _apply_identity(self, msg: dict) -> None:
|
||||
"""Record the authenticated pubkey (if any) for per-key policy and audit logs."""
|
||||
pub = (msg.get("pubkey") or "").strip().lower()
|
||||
self.auth_pubkey = pub or None
|
||||
self.auth_label = self.security.label_for(self.auth_pubkey)
|
||||
|
||||
async def _enforce_rate_limit(self) -> bool:
|
||||
limiter = self.security.rate_limiter
|
||||
if limiter is None or limiter.allow(self.auth_pubkey or str(self.addr[0])):
|
||||
return True
|
||||
await self.send_error("rate limit exceeded; slow down and retry")
|
||||
log_request(self.addr, self.command, None, "DENIED", "rate limit exceeded", identity=self.auth_label)
|
||||
return False
|
||||
|
||||
async def _dispatch(self, msg: dict) -> None:
|
||||
self.accept_encoding = msg.get("accept_encoding")
|
||||
if not await self._enforce_rate_limit():
|
||||
return
|
||||
# Gate every command — including server control commands like the key-management
|
||||
# ones — so the policy is enforced before handle_control_command acts on it.
|
||||
try:
|
||||
assert_command_allowed(self.command, self.security.effective_policy(self.auth_pubkey))
|
||||
except PermissionError as exc:
|
||||
await self.send_error(str(exc))
|
||||
log_request(self.addr, self.command, None, "DENIED", "blocked by command policy", identity=self.auth_label)
|
||||
return
|
||||
if await self.handle_control_command(msg):
|
||||
return
|
||||
await self.forward_to_browser(msg)
|
||||
|
||||
async def _read_session_message(self) -> dict | None:
|
||||
"""Read the next command on an established encrypted session, or None to close."""
|
||||
try:
|
||||
payload = await asyncio.wait_for(_async_recv_all(self.reader), timeout=REMOTE_SESSION_IDLE_TIMEOUT)
|
||||
except (asyncio.TimeoutError, ConnectionError, OSError):
|
||||
return None
|
||||
if not payload:
|
||||
return None
|
||||
try:
|
||||
outer = json.loads(payload)
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
return None
|
||||
if not isinstance(outer, dict) or "encrypted" not in outer:
|
||||
return None # an authenticated session only accepts encrypted frames
|
||||
from browser_cli.auth import pq_decrypt
|
||||
try:
|
||||
inner = json.loads(pq_decrypt(self.response_secret, "request", outer["encrypted"]))
|
||||
except Exception:
|
||||
return None
|
||||
if not isinstance(inner, dict):
|
||||
return None
|
||||
inner = adapt_auth(inner, self.client_ver)
|
||||
self.msg_id = inner.get("id")
|
||||
self.command = inner.get("command", "?")
|
||||
return inner
|
||||
|
||||
async def _async_proxy_request(
|
||||
reader: asyncio.StreamReader,
|
||||
writer: asyncio.StreamWriter,
|
||||
@@ -104,8 +172,12 @@ async def _async_proxy_request(
|
||||
nonce: str,
|
||||
pq_private_key=None,
|
||||
compress: bool = True,
|
||||
security: ServeSecurity | None = None,
|
||||
) -> None:
|
||||
await ServeRequest(reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress).run()
|
||||
await ServeRequest(
|
||||
reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress,
|
||||
security if security is not None else ServeSecurity(),
|
||||
).run()
|
||||
|
||||
async def _async_handle_client(
|
||||
reader: asyncio.StreamReader,
|
||||
@@ -115,6 +187,7 @@ async def _async_handle_client(
|
||||
auth_keys_path: Path | None,
|
||||
compress: bool = True,
|
||||
conn_limit: asyncio.Semaphore | None = None,
|
||||
security: ServeSecurity | None = None,
|
||||
) -> None:
|
||||
if conn_limit is None:
|
||||
conn_limit = asyncio.Semaphore(64)
|
||||
@@ -130,7 +203,7 @@ async def _async_handle_client(
|
||||
await _async_framed_send(writer, json.dumps(challenge_msg).encode())
|
||||
except OSError:
|
||||
return
|
||||
await _async_proxy_request(reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress)
|
||||
await _async_proxy_request(reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress, security)
|
||||
finally:
|
||||
conn_limit.release()
|
||||
writer.close()
|
||||
@@ -145,12 +218,13 @@ def _handle_client(
|
||||
profile: str | None,
|
||||
auth_keys_path: Path | None,
|
||||
compress: bool = True,
|
||||
security: ServeSecurity | None = None,
|
||||
) -> None:
|
||||
"""Run one accepted socket through the async serve pipeline."""
|
||||
|
||||
async def _run() -> None:
|
||||
reader, writer = await asyncio.open_connection(sock=client_sock)
|
||||
await _async_handle_client(reader, writer, addr, profile, auth_keys_path, compress)
|
||||
await _async_handle_client(reader, writer, addr, profile, auth_keys_path, compress, None, security)
|
||||
|
||||
try:
|
||||
asyncio.run(_run())
|
||||
@@ -160,12 +234,19 @@ def _handle_client(
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
async def _serve_async(host: str, port: int, profile: str | None, auth_keys_path: Path | None, compress: bool) -> None:
|
||||
async def _serve_async(
|
||||
host: str,
|
||||
port: int,
|
||||
profile: str | None,
|
||||
auth_keys_path: Path | None,
|
||||
compress: bool,
|
||||
security: ServeSecurity | None = None,
|
||||
) -> None:
|
||||
conn_limit = asyncio.Semaphore(64)
|
||||
|
||||
async def _client_connected(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None:
|
||||
peer = writer.get_extra_info("peername") or ("?", 0)
|
||||
await _async_handle_client(reader, writer, peer, profile, auth_keys_path, compress, conn_limit)
|
||||
await _async_handle_client(reader, writer, peer, profile, auth_keys_path, compress, conn_limit, security)
|
||||
|
||||
server = await asyncio.start_server(_client_connected, host, port, backlog=16)
|
||||
async with server:
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
"""Server-side authorization, per-key policy and rate limiting for ``browser-cli serve``.
|
||||
|
||||
This bundles the three serve-time security concerns that travel together through
|
||||
the connection-handling chain:
|
||||
|
||||
- ``policy`` the server-wide default ``CommandPolicy`` (from ``--allow-*``)
|
||||
- ``key_policies`` optional per-pubkey overrides parsed from the ``allow:`` token
|
||||
in the ``authorized_keys`` file
|
||||
- ``key_names`` pubkey -> friendly name (from authorized_keys), for audit logs
|
||||
- ``rate_limiter`` optional per-identity token-bucket throttle
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
import time
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
|
||||
from browser_cli.command_security import CommandPolicy
|
||||
|
||||
# ── per-key authorization ───────────────────────────────────────────────────────
|
||||
|
||||
_CATEGORY_FLAGS = {
|
||||
"read-page": "allow_read_page",
|
||||
"control": "allow_control",
|
||||
"dangerous": "allow_dangerous",
|
||||
"keys": "allow_keys",
|
||||
}
|
||||
|
||||
def policy_from_categories(categories) -> CommandPolicy:
|
||||
"""Build a CommandPolicy from category strings (``all``/``safe``/``read-page``/``control``/``dangerous``)."""
|
||||
cats = [str(c).strip().lower() for c in categories]
|
||||
if "all" in cats:
|
||||
return CommandPolicy.unrestricted()
|
||||
kwargs: dict[str, bool] = {}
|
||||
for cat in cats:
|
||||
if cat in ("", "safe"):
|
||||
continue
|
||||
flag = _CATEGORY_FLAGS.get(cat)
|
||||
if flag is None:
|
||||
raise ValueError(
|
||||
f"unknown command category {cat!r}; expected one of: all, safe, read-page, control, dangerous"
|
||||
)
|
||||
kwargs[flag] = True
|
||||
return CommandPolicy(**kwargs)
|
||||
|
||||
def key_policies_from_authorized_keys(path: Path | str | None) -> dict[str, CommandPolicy]:
|
||||
"""Build ``{pubkey: CommandPolicy}`` from the ``allow:`` tokens in authorized_keys.
|
||||
|
||||
Only keys that carry an explicit ``allow:`` token get an entry; keys without
|
||||
one fall back to the server-wide default policy. Pubkeys are normalised to
|
||||
lowercase hex. Raises ``ValueError`` on an unknown category so the server fails
|
||||
loudly at startup rather than silently mis-gating.
|
||||
"""
|
||||
if path is None:
|
||||
return {}
|
||||
from browser_cli.auth import load_authorized_keys_with_policies
|
||||
|
||||
out: dict[str, CommandPolicy] = {}
|
||||
for pubkey, _name, categories in load_authorized_keys_with_policies(Path(path)):
|
||||
if categories is not None:
|
||||
out[pubkey.strip().lower()] = policy_from_categories(categories)
|
||||
return out
|
||||
|
||||
# ── per-identity rate limiting ───────────────────────────────────────────────────
|
||||
|
||||
class RateLimiter:
|
||||
"""Token bucket keyed by identity (pubkey, or client address when unauthenticated).
|
||||
|
||||
``rate`` is the sustained refill in tokens/second; ``burst`` is the bucket
|
||||
capacity (defaults to ``rate``). ``rate <= 0`` disables limiting entirely.
|
||||
Thread-safe so it can be shared across all connections of one serve process.
|
||||
"""
|
||||
|
||||
def __init__(self, rate: float, burst: float | None = None) -> None:
|
||||
self.rate = float(rate)
|
||||
self.capacity = float(burst) if burst is not None else max(float(rate), 1.0)
|
||||
self._buckets: dict[str, tuple[float, float]] = {}
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def allow(self, key: str) -> bool:
|
||||
if self.rate <= 0:
|
||||
return True
|
||||
now = time.monotonic()
|
||||
with self._lock:
|
||||
tokens, last = self._buckets.get(key, (self.capacity, now))
|
||||
tokens = min(self.capacity, tokens + (now - last) * self.rate)
|
||||
if tokens < 1.0:
|
||||
self._buckets[key] = (tokens, now)
|
||||
return False
|
||||
self._buckets[key] = (tokens - 1.0, now)
|
||||
return True
|
||||
|
||||
# ── bundled server security context ──────────────────────────────────────────────
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ServeSecurity:
|
||||
policy: CommandPolicy = field(default_factory=CommandPolicy.unrestricted)
|
||||
key_policies: dict[str, CommandPolicy] = field(default_factory=dict)
|
||||
key_names: dict[str, str] = field(default_factory=dict)
|
||||
rate_limiter: RateLimiter | None = None
|
||||
|
||||
def effective_policy(self, pubkey: str | None) -> CommandPolicy:
|
||||
"""Per-key override if one exists for this pubkey, else the server default."""
|
||||
if pubkey and pubkey in self.key_policies:
|
||||
return self.key_policies[pubkey]
|
||||
return self.policy
|
||||
|
||||
def label_for(self, pubkey: str | None) -> str | None:
|
||||
"""Audit label for log lines: ``<name> <short-pubkey>…`` or just the short pubkey."""
|
||||
if not pubkey:
|
||||
return None
|
||||
short = f"{pubkey[:8]}…"
|
||||
name = self.key_names.get(pubkey, "")
|
||||
return f"{name} {short}".strip() if name else short
|
||||
Reference in New Issue
Block a user