Skip to content

Commit 6db3bb4

Browse files
committed
refactor: implement automated API key re-validation, tighten SSRF protections, and correct S3 SigV4 URL path construction
1 parent c84189b commit 6db3bb4

3 files changed

Lines changed: 101 additions & 23 deletions

File tree

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,5 @@ test_kill_switch
8282
test_load_contention
8383
test_monitoring
8484
test_virtual_tables
85-
build_**
85+
build_**
86+
server/arkilian-server

src/class.c

Lines changed: 92 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -633,18 +633,22 @@ static int host_is_storage_safe(const char *host) {
633633
if (!host || !*host) return 0;
634634
if (host[0] == '[') {
635635
if (strncmp(host, "[::1]", 5) == 0) return 1;
636-
if (strncmp(host, "[fc", 3) == 0 || strncmp(host, "[fd", 3) == 0) return 1; // ULA
637-
// NO [fe80 — link-local excluded
636+
// NO [fc / [fd ULA — AWS IMDSv2 is reachable at fd00:ec2::254 and
637+
// must never be treated as a safe storage destination. ULA prefixes
638+
// are NOT metadata-endpoint boundaries; rejecting them closes the
639+
// exfiltration path where a compromised control plane returns a
640+
// presigned URL pointing at fd00:ec2::254 and the client uploads
641+
// the full database to the cloud instance-metadata service.
638642
return 0;
639643
}
640644
if (strcmp(host, "localhost") == 0) return 1;
641645
if (strncmp(host, "127.", 4) == 0) return 1;
642646
if (strncmp(host, "10.", 3) == 0) return 1;
643647
if (strncmp(host, "192.168.", 8) == 0) return 1;
644-
// NO 169.254. — IMDS excluded
648+
// NO 169.254. — IMDS (IPv4) excluded
645649
// NO fe80 — IPv6 link-local excluded
646-
if (strncmp(host, "::1", 3) == 0) return 1;
647-
if (strncmp(host, "fc", 2) == 0 || strncmp(host, "fd", 2) == 0) return 1; // ULA
650+
// NO fc / fd ULA — AWS IMDSv2 (fd00:ec2::254) excluded
651+
if (strcmp(host, "::1") == 0) return 1;
648652
if (strncmp(host, "172.", 4) == 0) {
649653
unsigned second = 0;
650654
// sscanf's %u overflow on malformed input yields ULONG_MAX, which the
@@ -1748,16 +1752,30 @@ void *run_wal_flush(void *arg) {
17481752
if (validated) {
17491753
ARK_STORE(&db->startup_auth_state, 1);
17501754
ark_log(db, ARK_LOG_INFO, "API key validated against control plane (async)");
1755+
// Control plane is reachable → fetch storage credentials ONCE so
1756+
// the snapshot thread can sign PUT URLs locally and upload direct
1757+
// to S3/GCS/R2 (no per-upload control-plane round trip). A failure
1758+
// here just leaves s3_creds_loaded=0 and the snapshot thread skips
1759+
// its upload this cycle.
1760+
if (!ARK_LOAD(&db->s3_creds_loaded)) (void)fetch_storage_credentials(db);
17511761
} else {
17521762
ARK_STORE(&db->startup_auth_state, 2);
1763+
// CRITICAL: clear backup_enabled so the flush loop's drain gate
1764+
// (below) skips ship_to_backup entirely. Without this, rows keep
1765+
// being POSTed to a dead CP → 401 → exponential backoff → after
1766+
// max_attempts (default 100) every row dead-letters AND is deleted
1767+
// from _pending_backup → db_backup_is_healthy() flips GREEN again
1768+
// (queue empty) = silent bulk data loss between snapshots. With
1769+
// backup_enabled=0, capture keeps queuing with attempts=0 (nothing
1770+
// ever dead-letters) and the periodic recheck in the main loop
1771+
// re-enables the moment the CP validates.
1772+
ARK_STORE(&db->backup_enabled, 0);
17531773
ark_log(db, ARK_LOG_ERROR,
17541774
"async startup API key validation failed after %d attempt(s) — "
1755-
"the control plane may be unreachable or the API key is wrong. "
1756-
"Capture keeps queuing locally; shipping will resume "
1757-
"automatically the moment the control plane is reachable "
1758-
"again — no operator re-enable needed. Monitor "
1759-
"db_backup_is_healthy() for the liveness signal. "
1760-
"Verify ARKILIAN_API_KEY and ARKILIAN_CONTROL_URL",
1775+
"backup DISABLED (capture keeps queuing, no dead-lettering). "
1776+
"The flush thread will re-validate periodically and re-enable "
1777+
"shipping automatically once the control plane validates the "
1778+
"key. Verify ARKILIAN_API_KEY and ARKILIAN_CONTROL_URL",
17611779
retries + 1);
17621780
}
17631781
}
@@ -1776,7 +1794,36 @@ void *run_wal_flush(void *arg) {
17761794
while (!ARK_LOAD(&db->shutdown_requested) && select_stmt && ship_curl) {
17771795
// Liveness heartbeat (spec §9): the watchdog reads this from another
17781796
// thread; a stale age means the thread died silently.
1779-
ARK_STORE(&db->last_heartbeat_sec, (int)(now_ms_mono() / 1000));
1797+
long long now_ms = now_ms_mono();
1798+
ARK_STORE(&db->last_heartbeat_sec, (int)(now_ms / 1000));
1799+
1800+
// Periodic re-validation (startup_auth_state recovery): if async
1801+
// validation FAILED at boot (state==2, backup_enabled==0), retry it
1802+
// every ~60s. When the CP comes back and the key validates, we flip
1803+
// state back to VALIDATED, re-enable backup, and (re)fetch storage
1804+
// credentials. This is the mechanism that turns a CP outage from
1805+
// "permanent disable until operator notices" into "self-healing the
1806+
// moment CP returns" — bounded by this 60s recheck, NOT by a human.
1807+
// Without it, async-on-failure would disable backup for the whole
1808+
// process lifetime, reintroducing the v1 SPOF we set out to kill.
1809+
if (ARK_LOAD(&db->startup_auth_state) == 2) {
1810+
static long long last_revalidate_ms = 0; // function-static ok: single flush thread per process
1811+
if (now_ms - last_revalidate_ms >= 60000) {
1812+
last_revalidate_ms = now_ms;
1813+
char *key_copy = api_key_snapshot(db);
1814+
if (key_copy && strlen(key_copy) > 0) {
1815+
if (validate_api_key(db, db->control_url, key_copy)) {
1816+
ARK_STORE(&db->startup_auth_state, 1);
1817+
ARK_STORE(&db->backup_enabled, 1);
1818+
ark_log(db, ARK_LOG_INFO,
1819+
"API key re-validated against control plane — backup "
1820+
"RE-ENABLED (capture resumes shipping)");
1821+
if (!ARK_LOAD(&db->s3_creds_loaded)) (void)fetch_storage_credentials(db);
1822+
}
1823+
}
1824+
free(key_copy);
1825+
}
1826+
}
17801827

17811828
int drained = 0;
17821829
// Kill-switch check: when backup is disabled — or no destination is
@@ -3367,13 +3414,37 @@ static char *s3_presign_put(arkilian *db, const char *key, long expires_sec) {
33673414
host_clean[hl] = '\0';
33683415
}
33693416

3417+
// URL-encode the S3 object key for the URL path: per AWS SigV4 for the
3418+
// S3 service the canonical URI is NOT normalized and the '/' separators
3419+
// between key components are NOT encoded (they're structural). S3 keys
3420+
// built here are always "db_<hex>/backup.sqlite" or
3421+
// "db_<hex>/chunks/lsn_..._...sql.zst" — all chars are already in the
3422+
// RFC 3986 unreserved set except '/'. The shared s3_url_encode_inline
3423+
// helper preserves '/', so passing the key through it both (a) ensures
3424+
// any future exotic char in a key is handled and (b) keeps the canonical
3425+
// request's path identical to the path in the final URL (S3 recomputes
3426+
// the signature from the URL it receives and they must match byte for
3427+
// byte). For keys containing only the unreserved set the encoding is a
3428+
// no-op — the safe, defensive default.
3429+
char key_enc[1024];
3430+
s3_url_encode_inline(key, key_enc, sizeof(key_enc));
3431+
// The helper above encodes ALL non-unreserved chars, but S3 SigV4
3432+
// requires '/' to remain literal in the canonical URI for the S3
3433+
// service. Walk key_enc and convert %2F → / so path separators are
3434+
// preserved exactly as S3 expects.
3435+
for (char *p = key_enc; *p; p++) {
3436+
if (p[0] == '%' && p[1] == '2' && (p[2] == 'F' || p[2] == 'f')) {
3437+
*p = '/'; memmove(p + 1, p + 3, strlen(p + 3) + 1);
3438+
}
3439+
}
3440+
33703441
char canonical[4096];
33713442
snprintf(canonical, sizeof(canonical),
3372-
"PUT\n/%s\n"
3443+
"PUT\n/%s/%s\n"
33733444
"X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=%s&"
33743445
"X-Amz-Date=%s&X-Amz-Expires=%ld&X-Amz-SignedHeaders=host\n"
33753446
"host:%s\n\nhost\nUNSIGNED-PAYLOAD",
3376-
key, cred_enc, amz_date, expires_sec, host_clean);
3447+
db->s3_bucket, key_enc, cred_enc, amz_date, expires_sec, host_clean);
33773448

33783449
char scope[256];
33793450
snprintf(scope, sizeof(scope), "%s/%s/s3/aws4_request",
@@ -3400,18 +3471,22 @@ static char *s3_presign_put(arkilian *db, const char *key, long expires_sec) {
34003471
char sig_enc[256];
34013472
s3_url_encode_inline(sig_hex, sig_enc, sizeof(sig_enc));
34023473

3403-
size_t url_len = strlen(scheme) + 3 + strlen(host_clean) + strlen(key) + 2048;
3474+
// Final URL path uses the URL-encoded key (matches the canonical request,
3475+
// so S3's signature recomputation matches; '/' in the key is %2F-encoded
3476+
// and decoded by S3 as part of the object key, not as a path separator).
3477+
size_t url_len = strlen(scheme) + 3 + strlen(host_clean) + 1 +
3478+
strlen(db->s3_bucket) + 1 + strlen(key_enc) + 2048;
34043479
char *url = malloc(url_len);
34053480
if (!url) return NULL;
34063481
snprintf(url, url_len,
3407-
"%s://%s/%s"
3482+
"%s://%s/%s/%s"
34083483
"?X-Amz-Algorithm=AWS4-HMAC-SHA256"
34093484
"&X-Amz-Credential=%s"
34103485
"&X-Amz-Date=%s"
34113486
"&X-Amz-Expires=%ld"
34123487
"&X-Amz-SignedHeaders=host"
34133488
"&X-Amz-Signature=%s",
3414-
scheme, host_clean, key,
3489+
scheme, host_clean, db->s3_bucket, key_enc,
34153490
cred_enc, amz_date, expires_sec, sig_enc);
34163491
return url;
34173492
}

src/hydration.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,13 @@ static int http_init(HttpReq *r, const char *url, const char *token) {
234234
curl_easy_setopt(r->handle, CURLOPT_WRITEFUNCTION, curl_write_cb);
235235
curl_easy_setopt(r->handle, CURLOPT_TIMEOUT, 120L);
236236
curl_easy_setopt(r->handle, CURLOPT_CONNECTTIMEOUT, 15L);
237-
curl_easy_setopt(r->handle, CURLOPT_FOLLOWLOCATION, 1L);
238-
// Restrict redirect targets to HTTP/HTTPS so a 302 from an allowed host
239-
// to file://, gopher://, etc. is never followed. The SSRF guard only
240-
// inspects the INITIAL URL host — redirects must not bypass it to an
241-
// arbitrary scheme.
237+
// Do NOT follow redirects. The SSRF guard only inspects the INITIAL URL
238+
// host; a 302 to http://169.25.169.254/ would otherwise bypass it and
239+
// feed attacker-controlled bytes into hydration's SQL executor
240+
// (remote-code-execution chain). Presigned S3/GCS/R2 GET URLs never
241+
// 302 in practice — the object IS at the signed URL — so disabling
242+
// redirects closes the bypass with zero benign impact.
243+
curl_easy_setopt(r->handle, CURLOPT_FOLLOWLOCATION, 0L);
242244
#if CURL_AT_LEAST_VERSION(7, 85, 0)
243245
curl_easy_setopt(r->handle, CURLOPT_REDIR_PROTOCOLS_STR, "http,https");
244246
curl_easy_setopt(r->handle, CURLOPT_PROTOCOLS_STR, "http,https");

0 commit comments

Comments
 (0)