Skip to content

API-key rotation leaves old key valid forever; WASM upload endpoint hardcodes tenant 0 with no auth (2 sub-items) #62

@hollanf

Description

@hollanf

(1) API-key rotation writes an overlap_ends_at and replaces_key_id marker but verify() never consults them, so the old key is never revoked — a leaked key remains valid indefinitely after the operator believes it has been rotated out. (2) The WASM upload HTTP handler performs no authentication and pins tenant_id = 0 in a comment that claims middleware will provide auth — middleware that is not present in the reviewed router.


1. HIGH — rotate() never invalidates the old key; verify() doesn't check rotation metadata

File: nodedb/src/control/security/auth_apikey.rs:114-138, 162-199

pub fn verify(&self, token: &str) -> Option<AuthApiKey> {
    ...
    if key.is_revoked { return None; }
    if key.expires_at > 0 && now_secs() > key.expires_at { return None; }
    Some(key.clone())                        // ← overlap_ends_at / replaces_key_id ignored
}

pub fn rotate(&self, old_key_id: &str, overlap_hours: u64) -> Option<String> {
    let new_token = self.create_key(..., 0 /* no expiry */);
    ...
    if let Some(new_key) = keys.get_mut(&new_key_id) {
        new_key.replaces_key_id = Some(old_key_id.to_string());
        new_key.overlap_ends_at = overlap_ends;
    }
    // ← old key is NOT revoked, NOT given an expires_at
    Some(new_token)
}

verify() checks only is_revoked and expires_at. rotate() creates a new key with expires_days = 0 (no expiry), writes metadata onto the new key (replaces_key_id, overlap_ends_at), and does nothing to the old key. The old key was originally created with expires_days = 0 too, so after rotation it stays valid forever.

Operators who rotate believe the old key becomes invalid once the overlap window elapses. It does not. A leaked key that triggered the rotation is still accepted months later.

Repro:

let old = store.create_key("u1", 1, vec![], 0, 0, 0);
let old_id = store.verify(&old).unwrap().key_id;
let _new = store.rotate(&old_id, 1 /* 1-hour overlap */);
std::thread::sleep(Duration::from_secs(2));
assert!(store.verify(&old).is_some()); // still valid; rotation is a no-op from a security standpoint

2. HIGH — wasm_upload HTTP handler has no authentication and hardcodes tenant_id = 0

File: nodedb/src/control/server/http/routes/wasm_upload.rs:20-79

pub async fn upload_wasm(
    State(state): State<Arc<SharedState>>,
    Path(name): Path<String>,
    body: Bytes,
) -> impl IntoResponse {
    ...
    // Use tenant 0 for now — multi-tenant HTTP auth is handled by middleware.
    let tenant_id = 0u32;
    let mut func = match catalog.get_function(tenant_id, &name) { ... };
    ...
    let hash = wasm::store::store_wasm_binary(catalog, &body, config.max_binary_size)?;
    func.wasm_hash = Some(hash.clone());
    catalog.put_function(&func)?;

The handler takes Bytes and Path<String> — no HeaderMap, no resolve_auth(), no identity extraction. The layer above (startup_gate_middleware in http/server.rs:108) checks startup phase, not identity. The comment claims "multi-tenant HTTP auth is handled by middleware" but no such middleware exists in build_router in the reviewed tree.

If this route is mounted into a router, any network-reachable client can replace any tenant-0 function's bytecode:

curl -X PUT --data-binary @evil.wasm http://nodedb:port/v1/functions/some_fn/wasm

Additionally, store_wasm_binary feeds raw body bytes to wasmtime with only config.max_binary_size as a guard — no per-tenant resource accounting, no AoT compile limits, no import allow-list verification at upload time.

Even in deployments where this route is not currently mounted, the hardcoded tenant_id = 0 is a latent footgun: the moment anyone adds the route to the router or exposes a dev endpoint, every function in tenant 0 is replaceable by any unauthenticated caller.


Checklist

  • 1. verify() must also check is_superseded() / overlap_ends_at and return None past the overlap window. rotate() must set old_key.revoked_at (or expires_at = overlap_ends) on the old key, not just decorate the new one.
  • 2. upload_wasm must require an authenticated caller and use that identity's tenant_id. Remove the tenant_id = 0u32 hardcode. Add route-level auth middleware in build_router before mounting this handler.

Both items are independently verifiable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions