Harden: concurrency safety (db lock timeout, cache lock, retry, portal)
- get_connection sets PRAGMA busy_timeout so concurrent writers under a threaded server wait instead of hitting "database is locked". - Provision retries credential generation on IMSI/ICCID collision (up to 5x) instead of surfacing a duplicate as a 500. - Guard the module-level profile cache with a threading.Lock. - Captive portal disables the activate button while a request is in flight to avoid double provisioning. Tests: +2 (collision retry + give-up); 84 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
0be02bd1f4
commit
c6423b1bad
11
CHANGELOG.md
11
CHANGELOG.md
|
|
@ -26,9 +26,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
`ensure_parent_dir` helper that no-ops on an empty parent.
|
||||
- Open5GS/free5gc adapters now use the configured `profile.apn` for the core
|
||||
session/DNN name instead of a hardcoded `"internet"`.
|
||||
- Connections set `PRAGMA busy_timeout` (5s) so concurrent writers under a
|
||||
threaded server wait for the lock instead of raising "database is locked".
|
||||
- Provisioning retries on the (astronomically rare) IMSI/ICCID collision
|
||||
instead of returning a 500.
|
||||
- The in-memory profile cache is guarded by a lock (thread-safe under a
|
||||
threaded server), and the captive portal disables the activate button while
|
||||
a request is in flight to prevent double provisioning.
|
||||
- Added `test_app.py` (API-level: provision, rate-limit, token download,
|
||||
404 on unknown/enumerated ids), `test_utils.py` (MAC/IMSI/op_key/config/
|
||||
ensure_parent_dir), and adapter APN coverage.
|
||||
404 on unknown/enumerated ids, collision retry/give-up), `test_utils.py`
|
||||
(MAC/IMSI/op_key/config/ensure_parent_dir), and adapter APN coverage.
|
||||
|
||||
### Added
|
||||
- **Phase-2 profile generation.** `create_esim_profile` now emits a structured
|
||||
|
|
|
|||
|
|
@ -15,7 +15,13 @@ function showError(msg) {
|
|||
showState('error');
|
||||
}
|
||||
|
||||
var inFlight = false;
|
||||
|
||||
async function activate() {
|
||||
if (inFlight) return; // ignore double-clicks while a request is pending
|
||||
inFlight = true;
|
||||
var btn = document.getElementById('btn-activate');
|
||||
if (btn) btn.disabled = true;
|
||||
showState('loading');
|
||||
|
||||
try {
|
||||
|
|
@ -49,6 +55,9 @@ async function activate() {
|
|||
|
||||
} catch (err) {
|
||||
showError('Could not reach the server. Make sure you are connected to the WiFi network.');
|
||||
} finally {
|
||||
inFlight = false;
|
||||
if (btn) btn.disabled = false;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import io
|
|||
import logging
|
||||
import os
|
||||
import secrets
|
||||
import threading
|
||||
import time
|
||||
from collections import OrderedDict
|
||||
from datetime import datetime, timezone
|
||||
|
|
@ -43,16 +44,23 @@ CONFIG_PATH = os.environ.get("CONFIG_PATH", "config.yaml")
|
|||
# The cache is size-capped so it cannot grow without bound.
|
||||
_MAX_CACHED_PROFILES = 256
|
||||
_profile_cache: "OrderedDict[str, bytes]" = OrderedDict()
|
||||
_cache_lock = threading.Lock()
|
||||
_start_time = time.time()
|
||||
|
||||
|
||||
def _cache_profile(token: str, data: bytes) -> None:
|
||||
with _cache_lock:
|
||||
_profile_cache[token] = data
|
||||
_profile_cache.move_to_end(token)
|
||||
while len(_profile_cache) > _MAX_CACHED_PROFILES:
|
||||
_profile_cache.popitem(last=False)
|
||||
|
||||
|
||||
def _get_cached_profile(token: str):
|
||||
with _cache_lock:
|
||||
return _profile_cache.get(token)
|
||||
|
||||
|
||||
def create_app(config_path: str = CONFIG_PATH) -> Flask:
|
||||
cfg = load_config(config_path)
|
||||
|
||||
|
|
@ -120,20 +128,29 @@ def create_app(config_path: str = CONFIG_PATH) -> Flask:
|
|||
{"error": "Rate limit exceeded. One eSIM profile per device per hour."}
|
||||
), 429
|
||||
|
||||
# Retry on the astronomically-unlikely IMSI/ICCID collision so a
|
||||
# duplicate does not surface to the user as a 500.
|
||||
sub_id = None
|
||||
for attempt in range(5):
|
||||
imsi = generate_imsi(mcc, mnc)
|
||||
ki = generate_ki()
|
||||
iccid = generate_iccid()
|
||||
opc = derive_opc(ki, op_bytes)
|
||||
|
||||
ki_hex = bytes_to_hex(ki)
|
||||
opc_hex = bytes_to_hex(opc)
|
||||
try:
|
||||
sub_id = add_subscriber(conn, imsi, iccid, ki_hex, opc_hex, mac)
|
||||
break
|
||||
except ValueError as exc:
|
||||
logger.warning(f"Credential collision (attempt {attempt + 1}), retrying: {exc}")
|
||||
if sub_id is None:
|
||||
logger.error("Provision failed: could not allocate a unique IMSI/ICCID")
|
||||
return jsonify({"error": "Profile generation failed"}), 500
|
||||
|
||||
profile_bytes = create_esim_profile(
|
||||
imsi, ki, opc, iccid, profile_name, carrier_name,
|
||||
smdp_address=smdp_address, apn=apn,
|
||||
)
|
||||
|
||||
sub_id = add_subscriber(conn, imsi, iccid, ki_hex, opc_hex, mac)
|
||||
token = secrets.token_urlsafe(24)
|
||||
_cache_profile(token, profile_bytes)
|
||||
|
||||
|
|
@ -164,7 +181,7 @@ def create_app(config_path: str = CONFIG_PATH) -> Flask:
|
|||
# Capability URL: only the holder of the random token can download the
|
||||
# profile. Never distinguish "wrong token" from "expired/evicted" —
|
||||
# both are a plain 404 so tokens can't be probed.
|
||||
profile_bytes = _profile_cache.get(token)
|
||||
profile_bytes = _get_cached_profile(token)
|
||||
if profile_bytes is None:
|
||||
return jsonify({"error": "Profile not found"}), 404
|
||||
|
||||
|
|
|
|||
|
|
@ -35,9 +35,14 @@ def init_db(db_path: str) -> None:
|
|||
|
||||
|
||||
def get_connection(db_path: str) -> sqlite3.Connection:
|
||||
"""Return an open connection with Row factory enabled."""
|
||||
conn = sqlite3.connect(db_path, detect_types=sqlite3.PARSE_DECLTYPES)
|
||||
"""Return an open connection with Row factory enabled.
|
||||
|
||||
Sets a busy timeout so that under a threaded server concurrent writers
|
||||
wait for the lock instead of immediately raising "database is locked".
|
||||
"""
|
||||
conn = sqlite3.connect(db_path, detect_types=sqlite3.PARSE_DECLTYPES, timeout=5.0)
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA busy_timeout = 5000")
|
||||
return conn
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -71,6 +71,32 @@ class TestProvision:
|
|||
r = client.post("/api/provision", json={"mac_address": "de:ad:be:ef:00:01"})
|
||||
assert r.status_code == 429
|
||||
|
||||
def test_provision_retries_on_collision(self, client, monkeypatch):
|
||||
import server.app as appmod
|
||||
real_add = appmod.add_subscriber
|
||||
calls = {"n": 0}
|
||||
|
||||
def flaky(conn, imsi, iccid, ki, opc, mac):
|
||||
calls["n"] += 1
|
||||
if calls["n"] == 1:
|
||||
raise ValueError("Duplicate subscriber")
|
||||
return real_add(conn, imsi, iccid, ki, opc, mac)
|
||||
|
||||
monkeypatch.setattr(appmod, "add_subscriber", flaky)
|
||||
r = client.post("/api/provision", json={"mac_address": "aa:bb:cc:ab:cd:ef"})
|
||||
assert r.status_code == 200
|
||||
assert calls["n"] == 2 # failed once, succeeded on retry
|
||||
|
||||
def test_provision_gives_up_after_persistent_collision(self, client, monkeypatch):
|
||||
import server.app as appmod
|
||||
|
||||
def always_dup(*a, **k):
|
||||
raise ValueError("Duplicate subscriber")
|
||||
|
||||
monkeypatch.setattr(appmod, "add_subscriber", always_dup)
|
||||
r = client.post("/api/provision", json={"mac_address": "aa:bb:cc:ab:cd:11"})
|
||||
assert r.status_code == 500
|
||||
|
||||
|
||||
class TestProfileDownload:
|
||||
def test_download_with_token(self, client):
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user