Skip to content

nip55: bound blocking encrypted I/O on auto-sign path (runWithTimeout can't interrupt blocking FFI; per-request commit()) #314

@kwsantiago

Description

@kwsantiago

Problem

In Nip55ContentProvider.evaluateAutoSignPolicy (app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt:242), the opted-in rate check now runs:

val recorded = runWithTimeout {
    try { limiter.checkAndRecord(callerPackage, elapsedRealtime, currentTimeMillis) }
    catch (e: Exception) { ...; null }
}
recorded ?: return PolicyResult.FallToUi(...)

This correctly fails closed on exception/timeout (good, that was the security fix in e40c899). But two robustness costs remain on the auto-sign hot path:

  1. The runWithTimeout wrapper provides no real timeout. SigningRateLimiter.checkAndRecord is a non-suspend, blocking FFI call (keep_mobile.kt:9686), so withTimeoutOrNull has no suspension point to cancel at. A slow or stuck encrypted-prefs write blocks the binder thread for the full duration with no enforced bound, while holding a concurrentRequestSemaphore(4) permit. The wrapper only guards against semaphore exhaustion, not hangs.

  2. commit() is synchronous encrypted disk I/O per request. AndroidSigningRateLimiterStorage.save/remove use commit() (deliberate, for durability) and the Rust limiter persists on every call, so each auto-sign request now does a blocking AES-GCM keystore write inline. It runs on Dispatchers.IO/binder threads (no main-thread ANR), but adds latency to the signing path and can pile up under concurrency.

Neither is a security regression (the path fails closed), but together they make the auto-sign path vulnerable to latency/throughput degradation if encrypted I/O stalls.

Options to consider

  • Move the blocking FFI call onto a bounded-timeout executor that can actually interrupt it, or impose a hard wall-clock budget independent of cooperative cancellation.
  • Evaluate whether per-request commit() is required, or whether batching / apply() with a periodic forced flush preserves the durability guarantee at lower hot-path cost.
  • Gate checkAndRecord with the concurrentRequestSemaphore so Kotlin-side serialization bounds concurrent blocking I/O (pending confirmation the Rust side already holds a mutex across the storage round-trip).

Context

Surfaced during PR review of the persistent-limiter refactor (branch nip55-persistent-limiter-android, commit e40c899).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestmediumnip55NIP-55 signer protocolp3Lowest Priority

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions