diff --git a/implementations/main.cpp b/implementations/main.cpp index 7ad1923..3ea2fcf 100644 --- a/implementations/main.cpp +++ b/implementations/main.cpp @@ -1021,6 +1021,8 @@ struct CommandResult { std::uint32_t gid = 0; std::uint32_t fid = 0; std::int32_t samplesRemaining = -1; + // Only for ENUMERATE: the fids of the templates the group holds. + std::vector fids; bool Ok() const { return invoked && result == 0 && rc == 0; } }; @@ -1110,6 +1112,8 @@ CommandResult SendCommand(qcomtee_object* app, fingerprintd::ta::Cmd cmd, out.fid = ta::MatchedFid(reqOut); out.samplesRemaining = ta::SamplesRemaining(reqOut); } + if (cmd == ta::Cmd::Enumerate) + out.fids = ta::EnumeratedFids(reqOut, out.rc); if (g_taLog) DumpTaLog(rspOut); return out; } @@ -1294,8 +1298,10 @@ public: // store: an enrolment, or a template removed underneath us. // Remove templates from the TRUSTLET, which is what makes a delete a // delete. Dropping the name from the map only stops the finger being - // offered; the template keeps its slot, and with - // enable_duplicated_finger_checking on, that slot is what refuses the + // offered; the template keeps its slot until the group's next enrolment + // or delete-all removes it as unnamed -- and in group 0, or under a map + // the daemon does not trust, for good, where with + // enable_duplicated_finger_checking on that slot refuses the // re-enrolment of the same finger. // // The group must be active first: the trustlet compares the gid against @@ -1348,14 +1354,46 @@ public: std::println("SET_ACTIVE_GROUP gid={}", gid); auto g = SendCommand(app_, ta::Cmd::SetActiveGroup, sag); Report(ta::Cmd::SetActiveGroup, g); - auto e = SendCommand(app_, ta::Cmd::Enumerate, {}); + // With room for every slot the group can hold, so the reply names the + // templates and not only how many there are. + std::vector en(ta::EnumeratePayloadSize); + ta::BuildEnumeratePayload(en, fingerprintd::store::MaxFingersPerGroup); + auto e = SendCommand(app_, ta::Cmd::Enumerate, en); Report(ta::Cmd::Enumerate, e); - std::println(" templates loaded: {}", e.rc); + std::println(" templates loaded: {}{}", e.rc, FidList(e.fids)); gid_ = gid; templatesLoaded_ = e.invoked ? e.rc : -1; + loadedFids_ = e.invoked ? e.fids : std::vector{}; return templatesLoaded_; } + // The loaded templates that none of `named` accounts for. A template + // without a name is offered for nothing -- a verify filters on the map's + // fids, standby arms from them -- but it still holds one of the group's + // slots and, with duplicate-finger checking on, refuses the re-enrolment + // of the finger it was taken from. How they arise: the map lives on the + // rootfs and the templates on the persist partition, so a reflash keeps + // the templates and loses every name (fennek182, 2026-10-13: five of + // them, ENROLL answering -207); and a finger re-enrolled under its old + // name left its old template behind until this release. + std::vector Unnamed(const std::vector& named) const { + std::vector out; + for (std::uint32_t fid : loadedFids_) + if (fid != 0 && !std::ranges::contains(named, fid) && !std::ranges::contains(out, fid)) + out.push_back(fid); + return out; + } + + const std::vector& LoadedFids() const { return loadedFids_; } + + static std::string FidList(const std::vector& fids) { + if (fids.empty()) return {}; + std::string s = " (fid"; + for (std::size_t i = 0; i < fids.size(); i++) + s += std::format("{}{}", i ? ", " : " ", fids[i]); + return s + ")"; + } + // ---- Enrolment // // One sample per PRESS: touch on the rising edge, release on the falling @@ -1369,7 +1407,8 @@ public: std::string why; }; - EnrolOutcome Enrol(std::uint32_t gid, std::atomic& cancel, + EnrolOutcome Enrol(std::uint32_t gid, const std::vector& named, bool tidy, + std::atomic& cancel, const std::function& onStage, const std::function& onRetry, int maxFrames) { @@ -1383,6 +1422,23 @@ public: } // An enrolment adds a template, so this one reloads for real. SetActiveGroup(gid, /*force*/ true); + // Templates no name accounts for go before the new one comes: they + // can match nothing, and each one is a slot this enrolment may need + // or the duplicate the trustlet would refuse it for. Not in group 0 + // (store::GroupIsManaged), and not on the word of a map the daemon + // could not read in full (`tidy`): a removed template is gone for + // good. + auto stray = Unnamed(named); + if (!stray.empty() && fingerprintd::store::GroupIsManaged(gid)) { + if (tidy) { + std::println("enrol: {} template(s) loaded that no finger name accounts for; " + "removing them first", stray.size()); + RemoveTemplates(gid, stray); + } else { + std::println("enrol: {} template(s) loaded that no finger name accounts for, " + "left in place: the map could not be read in full", stray.size()); + } + } // Stock's opening sequence. AUTHENTICATE is what arms the capture // session; CANCEL and RESET_LOCKOUT bracket it. @@ -1523,12 +1579,11 @@ public: }; // `accept` is the set of fids that count as a match for THIS request. The - // trustlet identifies against every template loaded in the group, and a - // group accumulates them: re-enrolling a finger does NOT replace its - // template, it adds one, because FF_CMD_TA_REMOVE is not implemented. So + // trustlet identifies against every template loaded in the group, so // without this filter a verify answers for fingers the caller did not ask - // about, and for stale templates no longer named by anything -- which is - // both wrong by fprintd's contract and quietly ruins any measurement. + // about, and for templates no name accounts for (one an enrolment has not + // yet removed, or one in group 0) -- which is both wrong by fprintd's + // contract and quietly ruins any measurement. VerifyOutcome Verify(std::uint32_t gid, std::atomic& cancel, int maxFrames, const std::vector& accept) { namespace ta = fingerprintd::ta; @@ -1928,6 +1983,7 @@ private: // What the last real SET_ACTIVE_GROUP reported, so a repeat claim can be // answered without making the trustlet re-read the store. -1 = unknown. int templatesLoaded_ = -1; + std::vector loadedFids_; // Set when a fold succeeded; cleared by the save. bool templateDirty_ = false; // Frames folded during the verify loop itself, carried into the harvest so @@ -1948,7 +2004,12 @@ struct Job { std::string finger; std::vector acceptFids; // Verify: which fids count // Remove: which fids to drop + // Claim/Enroll: the fids the map names GDBusMethodInvocation* invocation = nullptr; // Claim/Remove reply asynchronously + bool everything = false; // Remove: the unnamed templates too + bool tidy = false; // Enroll: the map is trusted (read in + // full), so it may say what is unnamed + std::uint64_t seq = 0; // Enroll/Verify: the operation this job runs }; // Everything the worker sends back to the main thread. Delivered by g_idle_add @@ -1961,6 +2022,8 @@ struct Event { std::string status; std::uint32_t fid = 0; int templates = 0; + int unnamed = 0; // ClaimDone: of those, how many the map has no name for + std::uint64_t seq = 0; // EnrollStatus/VerifyStatus: the operation it reports on GDBusMethodInvocation* invocation = nullptr; }; @@ -1974,6 +2037,25 @@ void PostEvent(std::unique_ptr ev); // defined with the D-Bus code std::atomic g_standbyWanted{false}; std::atomic g_standbyRunning{false}; +// Which client operation the worker's jobs and events belong to. EnrollStart +// and VerifyStart give the claim a fresh number, the job and every event it +// posts carry it, and a Stop or the claim's end clears it. A job still +// queued under a cleared number exits at dequeue instead of running an +// operation nobody wants (the cancel flag is reset at dequeue, so it alone +// would not stop it), and a result that arrives under one is not the +// current operation's: it is neither named nor reported. Without this, a +// stop followed by a new start could name a late result under the new +// finger and remove the just-saved template as unnamed. The main thread +// owns the counter; the worker reads the live number from the atomic. +// +// The same handshake as standby's `wanted`/`running`: a stop clears the +// number BEFORE it cancels, and the worker resets its cancel flag BEFORE it +// reads the number, so whichever side goes second sees the other -- a job +// dequeued around a stop either reads the cleared number and exits, or +// keeps the cancel that comes after its reset. +std::uint64_t g_opSeqCounter = 0; +std::atomic g_liveOpSeq{0}; + class Worker { public: void Start() { @@ -2037,14 +2119,36 @@ private: auto ev = std::make_unique(Event{ .kind = Event::Kind::ClaimDone }); ev->ok = n >= 0; ev->templates = n; + ev->unnamed = static_cast(session_.Unnamed(j.acceptFids).size()); ev->invocation = j.invocation; PostEvent(std::move(ev)); break; } case Job::Kind::Remove: { - int n = session_.RemoveTemplates(j.uid, j.acceptFids); - std::println("remove: {} of {} template(s) removed for uid {}", - n < 0 ? 0 : n, j.acceptFids.size(), j.uid); + std::vector fids = j.acceptFids; + // "Every finger" is every template in the group, the ones no + // name accounts for included: after a reflash the map is empty + // and they are all this user has to delete. On the user's + // word, not the map's trust -- nothing is being kept on the + // map's account here. Group 0 is left alone, as in Enrol. + // Reloaded, not cached: the cached list can lack a template + // enrolled since, and "every finger" means the group as it is. + // A group that would not reload is a failure, not a delete + // of nothing: the client must not hear "done" while the + // templates stay. + bool loaded = true; + if (j.everything && fingerprintd::store::GroupIsManaged(j.uid)) { + if (session_.SetActiveGroup(j.uid, /*force*/ true) >= 0) + for (std::uint32_t fid : session_.Unnamed(fids)) fids.push_back(fid); + else + loaded = false; + } + int n = loaded ? session_.RemoveTemplates(j.uid, fids) : -1; + if (loaded) + std::println("remove: {} of {} template(s) removed for uid {}", + n < 0 ? 0 : n, fids.size(), j.uid); + else + std::println(std::cerr, "remove: group {} would not load, nothing removed", j.uid); auto ev = std::make_unique(Event{ .kind = Event::Kind::RemoveDone }); ev->ok = n >= 0; ev->templates = n; @@ -2053,8 +2157,16 @@ private: break; } case Job::Kind::Enroll: { + if (j.seq != g_liveOpSeq.load()) { + // Stopped while it was queued. + std::println("enrol: operation {} was stopped before it started", j.seq); + auto ev = std::make_unique(Event{ .kind = Event::Kind::EnrollStatus }); + ev->seq = j.seq; ev->status = ""; ev->done = false; + PostEvent(std::move(ev)); + break; + } auto o = session_.Enrol( - j.uid, cancel_, + j.uid, j.acceptFids, j.tidy, cancel_, [&](int accepted, int total) { // Logged as well as signalled. A run whose refusals // exist only as D-Bus traffic cannot be counted from @@ -2063,17 +2175,20 @@ private: // refused nothing", which was wrong. std::println(" enrol: sample ACCEPTED ({}/{})", accepted, total); auto ev = std::make_unique(Event{ .kind = Event::Kind::EnrollStatus }); + ev->seq = j.seq; ev->status = "enroll-stage-passed"; PostEvent(std::move(ev)); }, [&] { std::println(" enrol: sample REFUSED (retry-scan)"); auto ev = std::make_unique(Event{ .kind = Event::Kind::EnrollStatus }); + ev->seq = j.seq; ev->status = "enroll-retry-scan"; PostEvent(std::move(ev)); }, /*maxFrames*/ 600); auto ev = std::make_unique(Event{ .kind = Event::Kind::EnrollStatus }); + ev->seq = j.seq; ev->done = true; ev->ok = o.saved; ev->fid = o.fid; @@ -2136,8 +2251,17 @@ private: // timeout, and a claimant that vanishes from the bus has its // claim dropped -- and by VerifyStop, which is what sets // cancel_. See the idle-wait note in Verify(). + if (j.seq != g_liveOpSeq.load()) { + // Stopped while it was queued; without this it would poll + // until someone pressed. + std::println("verify: operation {} was stopped before it started", j.seq); + auto ev = std::make_unique(Event{ .kind = Event::Kind::VerifyStatus }); + ev->seq = j.seq; ev->status = ""; ev->done = false; + PostEvent(std::move(ev)); + break; + } auto o = session_.Verify(j.uid, cancel_, /*maxFrames*/ 600, j.acceptFids); - while (!o.decided && !o.cancelled && o.presses == 0) { + while (!o.decided && !o.cancelled && o.presses == 0 && j.seq == g_liveOpSeq.load()) { std::println("verify: {} frame(s), sensor never touched -- still waiting", o.frames); o = session_.Verify(j.uid, cancel_, /*maxFrames*/ 600, j.acceptFids); @@ -2146,6 +2270,7 @@ private: o.cancelled ? "cancelled" : !o.decided ? "undecided" : o.matched ? "MATCH" : "NO MATCH", o.presses, o.frames); auto ev = std::make_unique(Event{ .kind = Event::Kind::VerifyStatus }); + ev->seq = j.seq; ev->done = true; ev->fid = o.fid; if (o.cancelled) { ev->status = ""; ev->done = false; } @@ -2321,6 +2446,7 @@ struct Claim { std::uint32_t uid = 0; Op op = Op::None; std::string finger; // for the op in progress + std::uint64_t opSeq = 0; // the op in progress, 0 = none (see g_liveOpSeq) fingerprintd::store::Map fingers; }; Claim g_claim; @@ -2335,6 +2461,7 @@ void DropClaim(const char* why) { g_dbus_connection_signal_unsubscribe(g_conn, g_claimWatch); g_claimWatch = 0; } + g_liveOpSeq.store(0); // before the cancel: see g_liveOpSeq if (g_claim.op != Op::None && g_worker) g_worker->CancelOp(); if (g_claim.held) std::println("claim by {} for {} dropped: {}", g_claim.sender, g_claim.user, why); @@ -2368,28 +2495,62 @@ std::string MapPath(std::uint32_t uid) { fingerprintd::store::Map LoadMap(std::uint32_t uid, bool quiet = false) { std::string path = MapPath(uid); - std::ifstream f(path); - if (!f) { - if (!quiet) std::println("map {}: {}", path, ::strerror(errno)); - return {}; + // Absent is "no fingers" and the map that says so is trusted. Any other + // failure, at the open or during the read, is a map the daemon cannot + // see: it must not decide which templates are unnamed on that account, + // and it must never be written over. GLib's read reports a failed read + // as an error, where a stream would hand back what it got so far and + // make an EIO look like a short, clean map; and it tells an absent file + // from every other failure, which is the one distinction this needs. + gchar* contents = nullptr; + gsize len = 0; + GError* err = nullptr; + if (!g_file_get_contents(path.c_str(), &contents, &len, &err)) { + bool absent = g_error_matches(err, G_FILE_ERROR, G_FILE_ERROR_NOENT); + if (!quiet) std::println("map {}: {}", path, err ? err->message : "unknown error"); + g_clear_error(&err); + return absent ? fingerprintd::store::Map{} : fingerprintd::store::Map::Unreadable(); } - std::string text((std::istreambuf_iterator(f)), std::istreambuf_iterator()); + std::string text(contents, len); + g_free(contents); auto m = fingerprintd::store::Map::Decode(text); - if (!quiet) std::println("map {}: {} bytes, {} finger(s)", path, text.size(), m.Size()); + if (!quiet) + std::println("map {}: {} bytes, {} finger(s){}", path, text.size(), m.Size(), + m.Skipped() ? std::format(", {} line(s) not understood, kept as they are", + m.Skipped()) + : std::string()); return m; } -void SaveMap(std::uint32_t uid, const fingerprintd::store::Map& m) { +// Writes the map and says whether it landed. GLib writes a temp file in the +// same directory, fsyncs it, renames it over the old file and fsyncs the +// directory, so a crash leaves either the old map or the new one, never an +// empty file that would read as a trusted map naming nothing. The map +// decides which templates the next enrolment keeps, so a save that did not +// reach the disk leaves the old map in place rather than replace it with a +// shorter one, and the caller hears about it. +bool SaveMap(std::uint32_t uid, const fingerprintd::store::Map& m) { std::error_code ec; std::filesystem::create_directories(g_stateDir, ec); std::string path = MapPath(uid); - std::string tmp = path + ".tmp"; - { - std::ofstream f(tmp, std::ios::trunc); - f << m.Encode(); + // A map the daemon could not read is not one it may replace: the file + // may name fingers this object does not. + if (!m.Readable()) { + std::println(std::cerr, "map {}: not written: it could not be read, and the file stays as it is", path); + return false; } - ::chmod(tmp.c_str(), 0600); - std::filesystem::rename(tmp, path, ec); + std::string enc = m.Encode(); + GError* err = nullptr; + if (!g_file_set_contents_full(path.c_str(), enc.data(), static_cast(enc.size()), + static_cast(G_FILE_SET_CONTENTS_CONSISTENT + | G_FILE_SET_CONTENTS_DURABLE), + 0600, &err)) { + std::println(std::cerr, "map {}: not written ({}); the old map stays", path, + err ? err->message : "unknown error"); + g_clear_error(&err); + return false; + } + return true; } void EmitDevice(const char* signal, GVariant* params) { @@ -2753,8 +2914,10 @@ void PostEvent(std::unique_ptr ev) { case Event::Kind::ClaimDone: if (ev->invocation) { if (ev->ok) { - std::println("claimed by {} for {} (uid {}, {} template(s) loaded)", - g_claim.sender, g_claim.user, g_claim.uid, ev->templates); + std::println("claimed by {} for {} (uid {}, {} template(s) loaded{})", + g_claim.sender, g_claim.user, g_claim.uid, ev->templates, + ev->unnamed ? std::format(", {} of them unnamed", ev->unnamed) + : std::string()); g_dbus_method_invocation_return_value(ev->invocation, nullptr); } else { DropClaim("group selection failed"); @@ -2773,32 +2936,59 @@ void PostEvent(std::unique_ptr ev) { } else { // The names are already gone from the map by this point. // Saying so is better than a bare failure: the fingers - // will not be offered again, but their templates still - // hold slots, which is what will refuse a re-enrolment. + // will not be offered again, and their templates, now + // unnamed, go with the group's next enrolment or + // delete-all. ReturnError(ev->invocation, "Internal", "names removed, but the trustlet templates were not"); } } break; - case Event::Kind::EnrollStatus: - if (!ev->status.empty()) - EmitDevice("EnrollStatus", g_variant_new("(sb)", ev->status.c_str(), ev->done ? TRUE : FALSE)); + case Event::Kind::EnrollStatus: { + if (ev->seq != g_claim.opSeq) { + // From an enrolment the client has since stopped, or a claim + // since dropped: not this operation's result. A template it + // saved is unnamed and goes with the group's next enrolment. + if (ev->done && ev->ok && ev->fid != 0) + std::println("enrol: a result of a stopped enrolment arrived (fid {}); it stays unnamed", + ev->fid); + break; + } + std::string status = ev->status; if (ev->done && ev->ok) { auto f = fingerprintd::store::FingerFromName(g_claim.finger); if (f && ev->fid != 0) { + // The name is saved BEFORE the client hears "completed". A + // save that did not land makes this an enrolment that + // failed, with the claim's map put back -- not a success + // whose finger is missing from the next list and never + // offered. + auto kept = g_claim.fingers; g_claim.fingers.Add(*f, ev->fid); - SaveMap(g_claim.uid, g_claim.fingers); - std::println("enrolled {} for uid {} as fid {}", g_claim.finger, g_claim.uid, ev->fid); + if (SaveMap(g_claim.uid, g_claim.fingers)) { + std::println("enrolled {} for uid {} as fid {}", g_claim.finger, g_claim.uid, ev->fid); + } else { + g_claim.fingers = kept; + status = "enroll-failed"; + std::println(std::cerr, + "enrolled {} for uid {} as fid {}, but its name could not be saved: " + "reported as failed; the template is unnamed and goes with the group's " + "next enrolment", g_claim.finger, g_claim.uid, ev->fid); + } } else { std::println(std::cerr, "enrolled, but the trustlet reported no fid -- the finger cannot be " "named until it is seen in a verification"); } } + if (!status.empty()) + EmitDevice("EnrollStatus", g_variant_new("(sb)", status.c_str(), ev->done ? TRUE : FALSE)); break; + } case Event::Kind::VerifyStatus: { namespace ac = fingerprintd::actions; namespace store = fingerprintd::store; + if (ev->seq != g_claim.opSeq) break; // a verify the client has since stopped std::string status = ev->status; const ac::Rule* rule = nullptr; std::optional matched; @@ -2894,7 +3084,8 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* g_pendingClaim = inv; WatchClaimant(g_claim.sender); StandbyCancel(); // the client's op comes first; standby returns with Release - g_worker->Post(Job{ .kind = Job::Kind::Claim, .uid = g_claim.uid, .invocation = inv }); + g_worker->Post(Job{ .kind = Job::Kind::Claim, .uid = g_claim.uid, + .acceptFids = g_claim.fingers.AllFids(), .invocation = inv }); return; } if (method == "Release") { @@ -2947,13 +3138,31 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* } } auto m = LoadMap(uid); + if (!m.Readable()) { + ReturnError(inv, "Internal", "the finger map could not be read; nothing is removed until it can be"); + return; + } + // An operation in progress for this user has the claim's own copy of + // the map and a job on the worker whose keep-list came from it; a + // delete underneath would remove what that job is about to name. + if (g_claim.held && g_claim.uid == uid && g_claim.op != Op::None) { + ReturnError(inv, "AlreadyInUse", "an operation is in progress for this user"); + return; + } // The fids have to be read BEFORE the names go: the map is the only // place that remembers which template belongs to which finger. - std::vector fids; - for (const auto& e : m.Entries()) - if (!one || e.finger == *one) fids.push_back(e.fid); + std::vector fids = one ? m.FidsNamed(*one) : m.AllFids(); + // Deleting every finger also deletes the templates the map has no + // name for, which is why the job is posted even with no fids: after + // a reflash the map is empty and the trustlet is full. + bool everything = !one; if (one) m.Remove(*one); else m.Clear(); - SaveMap(uid, m); + if (!SaveMap(uid, m)) { + // The disk still names them; removing their templates now would + // leave names that can never match. + ReturnError(inv, "Internal", "the finger map could not be written; nothing is removed"); + return; + } if (g_claim.held && g_claim.uid == uid) g_claim.fingers = m; // A standby verify for this user holds the fids it was armed with and // keeps the worker busy; stop it so the removal below runs, and so it @@ -2966,16 +3175,16 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* // purpose: a finger whose template survives a failed removal is a slot // leak, while a name that survives a successful one would keep // offering a finger that can no longer match. - std::println("deleting finger name(s) for uid {}; removing {} trustlet template(s)", - uid, fids.size()); - if (fids.empty()) { + std::println("deleting finger name(s) for uid {}; removing {} named trustlet template(s){}", + uid, fids.size(), everything ? " and every unnamed one" : ""); + if (fids.empty() && !everything) { g_dbus_method_invocation_return_value(inv, nullptr); return; } // Only the worker thread ever invokes the trustlet, so the reply waits // for it -- fprintd's Delete methods are synchronous to the client. g_worker->Post(Job{ .kind = Job::Kind::Remove, .uid = uid, - .acceptFids = fids, .invocation = inv }); + .acceptFids = fids, .invocation = inv, .everything = everything }); return; } if (method == "EnrollStart" || method == "VerifyStart") { @@ -2996,11 +3205,51 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* ReturnError(inv, "InvalidFingername", "unknown finger"); return; } + // Nothing changes for an enrolment the store cannot take: the + // worker would refuse it after the name below was already gone. + if (g_sfsReadOnly || !g_rpmbWrite) { + ReturnError(inv, "Internal", "the template store is read-only; nothing is enrolled"); + return; + } + // The claim's copy is re-read from disk: the file may have been + // rebuilt by hand since the claim, and what it says now is what + // decides this enrolment. + g_claim.fingers = LoadMap(g_claim.uid); + // An enrolment ends in a save, and a map the daemon could not + // read is one it must not write over. + if (!g_claim.fingers.Readable()) { + ReturnError(inv, "Internal", "the finger map could not be read; nothing is enrolled until it can be"); + return; + } if (g_claim.fingers.Full() && !g_claim.fingers.Has(*store::FingerFromName(finger))) { EmitDevice("EnrollStatus", g_variant_new("(sb)", "enroll-data-full", TRUE)); g_dbus_method_invocation_return_value(inv, nullptr); return; } + // Re-enrolling a finger replaces it, and the trustlet makes that + // an order: with duplicate-finger checking on, a re-enrolment of a + // finger it still holds does not complete (2026-09-04: one attempt + // 0 accepted of 7 presses, another stalled at sample 8 of 20). The + // name goes now and the template follows on the + // worker, as one no name accounts for -- a name kept through a + // cancelled enrolment would offer a finger that can no longer + // match. Both halves under the one rule: a managed group, a map + // the daemon trusts. Otherwise the name stays and the trustlet + // refuses the duplicate, as it did before this release. + auto f = *store::FingerFromName(finger); + bool tidy = g_claim.fingers.Trusted(); + if (store::GroupIsManaged(g_claim.uid) && tidy && g_claim.fingers.Has(f)) { + auto kept = g_claim.fingers; + g_claim.fingers.Remove(f); + if (!SaveMap(g_claim.uid, g_claim.fingers)) { + // The drop did not land: the disk still names the template + // the worker would remove. Keep both, and say so. + g_claim.fingers = kept; + ReturnError(inv, "Internal", "the finger map could not be written; nothing is enrolled or removed"); + return; + } + std::println("enrol: {} is being re-enrolled; its old template goes first", finger); + } } else { if (finger != store::AnyFinger && !store::FingerFromName(finger)) { ReturnError(inv, "InvalidFingername", "unknown finger"); @@ -3013,6 +3262,8 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* } g_claim.op = enroll ? Op::Enroll : Op::Verify; g_claim.finger = finger; + g_claim.opSeq = ++g_opSeqCounter; + g_liveOpSeq.store(g_claim.opSeq); g_dbus_method_invocation_return_value(inv, nullptr); if (!enroll) { // The trustlet identifies against every template in the group, so @@ -3025,7 +3276,11 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* // Which templates may answer this request: the named finger's fid, or // every named finger's for "any". A fid that no name maps to -- a // template left behind by an earlier enrolment -- answers for nothing. + // For an enrolment the same list says which templates to KEEP: every + // fid the map carries, the shadowed lines' included -- a template the + // map names is never "unnamed", whatever the trust gate then decides. std::vector accept; + if (enroll) accept = g_claim.fingers.AllFids(); if (!enroll) { for (const auto& e : g_claim.fingers.Entries()) if (finger == store::AnyFinger || store::NameOf(e.finger) == finger) @@ -3037,7 +3292,8 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* } g_worker->Post(Job{ .kind = enroll ? Job::Kind::Enroll : Job::Kind::Verify, .uid = g_claim.uid, .finger = finger, - .acceptFids = std::move(accept) }); + .acceptFids = std::move(accept), + .tidy = g_claim.fingers.Trusted(), .seq = g_claim.opSeq }); return; } if (method == "EnrollStop" || method == "VerifyStop") { @@ -3055,9 +3311,12 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant* // it, and clearing the op ourselves on `done` made every one of them // fail with NoActionInProgress. Cancelling a loop that already ended // is harmless. - g_worker->CancelOp(); + // The number is cleared before the cancel: see g_liveOpSeq. g_claim.op = Op::None; g_claim.finger.clear(); + g_claim.opSeq = 0; + g_liveOpSeq.store(0); + g_worker->CancelOp(); g_dbus_method_invocation_return_value(inv, nullptr); return; } @@ -3207,7 +3466,9 @@ int RunProbe(bool doAuth, bool doEnrol, bool doCalSave, bool doLearnProbe, std::atomic cancel{false}; if (doEnrol) { for (int c = 3; c > 0; c--) { std::println("*** press and LIFT, repeatedly, in {}... ***", c); std::this_thread::sleep_for(std::chrono::seconds(1)); } - auto o = s.Enrol(gid, cancel, + // The probe has no name map, so every loaded template counts as + // named: a diagnostic does not tidy the group it is probing. + auto o = s.Enrol(gid, s.LoadedFids(), /*tidy*/ false, cancel, [](int a, int t) { std::println(" [{}/{}] accepted -- LIFT, then press again", a, t); }, [] { std::println(" press rejected -- LIFT, shift the finger, press again"); }, frames); diff --git a/interfaces/Fingerprintd-Store.cppm b/interfaces/Fingerprintd-Store.cppm index 4eefcc4..97bfdb7 100644 --- a/interfaces/Fingerprintd-Store.cppm +++ b/interfaces/Fingerprintd-Store.cppm @@ -79,6 +79,13 @@ export namespace fingerprintd::store { using Gid = std::uint32_t; inline Gid GidForUid(std::uint32_t uid) { return uid; } + // Whether the daemon may remove a template of this group that the map + // does not name. Group 0 is left alone: it is root's, which no session + // user is, and the group a stock Android install's primary user also + // writes. Every path that removes an unnamed template, and the name drop + // that precedes one, asks this and nothing else. + inline bool GroupIsManaged(Gid gid) { return gid != 0; } + // common.max_enrolling_fingers, from the trustlet config we ship. inline constexpr std::size_t MaxFingersPerGroup = 5; @@ -104,11 +111,36 @@ export namespace fingerprintd::store { return true; } + // Removing a finger takes its entry and every shadowed line for it: + // a line the decoder understood but could not make the finger's + // entry (a repeated name, a sixth finger) must not come back as the + // finger's name once the entry is gone. The fids all go to the + // caller, whose delete removes their templates. bool Remove(Finger f) { auto n = std::erase_if(entries_, [&](const Entry& e) { return e.finger == f; }); + n += std::erase_if(shadowed_, [&](const Entry& e) { return e.finger == f; }); return n > 0; } - void Clear() { entries_.clear(); } + // "Every finger" includes the ones in lines the daemon could not + // read, so this is the one call that drops those lines too. The + // templates go with the same delete, named or not: that removal is + // on the user's word, not on this map's. + void Clear() { entries_.clear(); shadowed_.clear(); unparsed_.clear(); } + + // Every fid a finger's lines carry: its entry's and its shadowed + // ones'. What a delete of that finger removes from the trustlet. + std::vector FidsNamed(Finger f) const { + std::vector out; + for (const Entry& e : entries_) if (e.finger == f) out.push_back(e.fid); + for (const Entry& e : shadowed_) if (e.finger == f) out.push_back(e.fid); + return out; + } + std::vector AllFids() const { + std::vector out; + for (const Entry& e : entries_) out.push_back(e.fid); + for (const Entry& e : shadowed_) out.push_back(e.fid); + return out; + } // A match reports an id; this turns it back into a name. std::optional Lookup(std::uint32_t fid) const { @@ -127,19 +159,45 @@ export namespace fingerprintd::store { // ---- Serialisation // - // One "name fid" per line. Deliberately boring and greppable: this - // file is recoverable by hand if it is ever lost, because losing it - // costs names, not templates. + // One "name fid" per line. Deliberately boring and greppable, because + // this file can be rebuilt by hand: the claim log prints the fids the + // trustlet holds ("templates loaded: N (fid ...)"). It is worth + // rebuilding promptly, and a rebuild counts from the next enrolment + // on, a held claim's included (the map is re-read at every + // EnrollStart). The map decides which of a group's templates an + // enrolment or a delete-all keeps -- a template no line names is + // removed by the next one, in a managed group -- so losing the map + // costs the names at once and the unnamed templates at that point. + // + // The lines Decode could not use are written back unchanged, after + // the entries: a save must not turn a map the daemon did not + // understand into one it does, because the next load would trust + // it and the next enrolment remove what those lines named. std::string Encode() const { std::string out; for (const Entry& e : entries_) out += std::format("{} {}\n", NameOf(e.finger), e.fid); + for (const Entry& e : shadowed_) + out += std::format("{} {}\n", NameOf(e.finger), e.fid); + for (const std::string& l : unparsed_) + out += l + '\n'; return out; } - // Unknown finger names and malformed lines are skipped, not fatal: a - // corrupt map must degrade to "fewer names known", never to a daemon - // that will not start and therefore a phone that cannot be unlocked. + // A non-empty line that does not become its own entry is kept and + // counted, not dropped and not fatal: a damaged map must degrade to + // "fewer names known", never to a daemon that will not start and + // therefore a phone that cannot be unlocked. A line the decoder + // cannot read -- no space, an unknown finger name, a bad number, + // fid 0 -- is kept as text; one it can read but cannot seat -- a + // finger named twice, a sixth finger -- is kept as the finger and + // fid it names, so a delete of that finger finds it. The kept lines + // are what keep "fewer names" from becoming "fewer templates": a map + // with a line it could not use is not trusted to say which + // templates are unnamed (Trusted()), and Encode carries the lines + // along so the next load is not either. A file that could not be + // read at all is Unreadable(); only an absent file means "no + // fingers". static Map Decode(std::string_view text) { Map m; std::size_t pos = 0; @@ -149,21 +207,52 @@ export namespace fingerprintd::store { ? std::string_view::npos : nl - pos); pos = (nl == std::string_view::npos) ? text.size() + 1 : nl + 1; if (line.empty()) continue; - std::size_t sp = line.find(' '); - if (sp == std::string_view::npos) continue; - auto f = FingerFromName(line.substr(0, sp)); - if (!f) continue; - std::uint32_t fid = 0; - auto num = line.substr(sp + 1); - auto [ptr, ec] = std::from_chars(num.data(), num.data() + num.size(), fid); - if (ec != std::errc{} || ptr != num.data() + num.size()) continue; - m.Add(*f, fid); + if (!m.Parse(line)) m.unparsed_.emplace_back(line); } return m; } + // The lines Decode could not use: the unreadable ones verbatim, the + // unseatable ones as entries. Either non-empty means the map on disk + // says more than this object's entries do. + std::size_t Skipped() const { return unparsed_.size() + shadowed_.size(); } + const std::vector& Unparsed() const { return unparsed_; } + const std::vector& Shadowed() const { return shadowed_; } + + // The empty map that stands for a file the daemon could not open or + // read for a reason other than its absence. It is never saved: a + // file the daemon could not see is not a file it may replace. + static Map Unreadable() { Map m; m.readable_ = false; return m; } + bool Readable() const { return readable_; } + + // Whether this object says everything the map on disk says: read in + // full, every line its own entry. Only then may it decide which of + // the trustlet's templates are unnamed; nothing is removed on the + // word of a map that is not. + bool Trusted() const { return readable_ && unparsed_.empty() && shadowed_.empty(); } + private: + // One line into one entry, or false for a line the decoder cannot + // read. A line it can read but cannot seat -- the finger is already + // named, or the map is full -- is shadowed: the earlier line's fid + // would otherwise be lost in memory while it still stands on disk. + bool Parse(std::string_view line) { + std::size_t sp = line.find(' '); + if (sp == std::string_view::npos) return false; + auto f = FingerFromName(line.substr(0, sp)); + if (!f) return false; + std::uint32_t fid = 0; + auto num = line.substr(sp + 1); + auto [ptr, ec] = std::from_chars(num.data(), num.data() + num.size(), fid); + if (ec != std::errc{} || ptr != num.data() + num.size() || fid == 0) return false; + if (Has(*f) || Full()) { shadowed_.push_back({ *f, fid }); return true; } + return Add(*f, fid); + } + std::vector entries_; + std::vector shadowed_; + std::vector unparsed_; + bool readable_ = true; }; // Per-user file under the daemon's state directory. Root-only: it is not diff --git a/interfaces/Fingerprintd-Ta.cppm b/interfaces/Fingerprintd-Ta.cppm index efc011f..ff5ee55 100644 --- a/interfaces/Fingerprintd-Ta.cppm +++ b/interfaces/Fingerprintd-Ta.cppm @@ -370,6 +370,25 @@ export namespace fingerprintd::ta { detail::StoreU32(out, RemoveFidOff, fid); } + // ENUMERATE (0x2005). One u32 at payload+0: how many entries the caller + // has room for, 0 to ask for the count alone -- which is what an empty + // payload asked since 2026-09-02, so until now the daemon knew how many + // templates a group held and never which. The reply's rc is the count, + // and with room given the payload from +4 carries one 8-byte entry per + // loaded template whose second word is the fid: asked with room for 5 + // on a group of 4, the three fids the map named came back in it + // (2026-10-13). The first word is not read. Decoded by EnumeratedFids + // below. + inline constexpr std::size_t EnumeratePayloadSize = 0x04; + inline constexpr std::size_t EnumerateEntriesOff = 0x04; + inline constexpr std::size_t EnumerateEntrySize = 0x08; + inline constexpr std::size_t EnumerateEntryFidOff = 0x04; + + inline void BuildEnumeratePayload(std::span out, std::uint32_t capacity) { + std::ranges::fill(out.first(EnumeratePayloadSize), std::byte{0}); + detail::StoreU32(out, 0, capacity); + } + // SET_ACTIVE_GROUP writes its gid to device+0x30 and AUTHENTICATE compares // its own against the same field (0xeb08), logging // "templates with gid(%u != %u) hasn't been loaded." and returning -200 on @@ -471,6 +490,21 @@ export namespace fingerprintd::ta { return detail::LoadU32(response, ResponsePayloadOff + RespFidOff); } + // The fids an ENUMERATE reply lists, given the count it reported. Bounded + // by the buffer, not by trust in the count; an entry the trustlet did not + // write reads as fid 0, which nothing downstream acts on. + inline std::vector EnumeratedFids(std::span response, + std::int32_t count) { + std::vector fids; + if (count <= 0) return fids; + for (std::size_t i = 0; i < static_cast(count); i++) { + std::size_t off = ResponsePayloadOff + EnumerateEntriesOff + i * EnumerateEntrySize; + if (off + EnumerateEntrySize > response.size()) break; + fids.push_back(detail::LoadU32(response, off + EnumerateEntryFidOff)); + } + return fids; + } + // ---- The verdict ------------------------------------------------------ // // A frame is one of three things and only the third is a verdict. This was diff --git a/packaging/fplearn.sh b/packaging/fplearn.sh index 5bbf6c0..b8fbb87 100755 --- a/packaging/fplearn.sh +++ b/packaging/fplearn.sh @@ -27,9 +27,11 @@ # WIPE FIRST when re-enrolling a finger that is already in the group. Measured # 2026-09-04: the duplicated-finger check refuses a finger the group already # holds -- 0 accepted of 7 presses, rc=0 on every one, while a never-enrolled -# finger progressed normally. Re-enrolment ADDS a template and there is no -# trustlet-side remove, so the old one has to go first. `enrol` refuses a -# finger name that is already in the map and points here. +# finger progressed normally. Since 0.2.6 the daemon removes a re-enrolled +# finger's old template itself before ENROLL (in a managed group, on a map it +# read in full). This protocol still starts from a wiped group, so that the +# template it measures has no history: `enrol` refuses a finger name that is +# already in the map and points here. # # WHY IT IS SHAPED LIKE THIS. Learning is cumulative: every matched press folds # frames into the stored template, so a second run is not a repeat of the first @@ -205,9 +207,9 @@ enrol) F=${2:-right-middle-finger} if sudo grep -q "^$F " "$MAP" 2>/dev/null; then echo "$F is already enrolled (in $MAP)." - echo "The trustlet REFUSES to re-enrol a finger it already holds; run" + echo "This protocol measures a template with no history; run" echo " fplearn.sh wipe" - echo "first. (Measured 2026-09-04: 0 accepted of 7 presses, rc=0 each.)" + echo "first. (The daemon replaces a re-enrolled finger's template by itself since 0.2.6.)" exit 1 fi echo "=== enrol $F ===" diff --git a/tests/Store/main.cpp b/tests/Store/main.cpp index b7523fb..b9a310b 100644 --- a/tests/Store/main.cpp +++ b/tests/Store/main.cpp @@ -5,12 +5,14 @@ /* Fingerprintd:Store unit tests. -The map is small, but two of its properties are load-bearing. A corrupt or +The map is small, but three of its properties are load-bearing. A corrupt or partly-unreadable map must degrade to "fewer names known" rather than to a -daemon that refuses to start, because that daemon is what unlocks the phone. -And fid 0 must never be storable or matchable: 0 is what the trustlet writes -into the fid field when authentication FAILS, so a stored 0 would turn every -rejection into a match. +daemon that refuses to start, because that daemon is what unlocks the phone -- +and it must say so, because the map now decides which templates an enrolment +removes, and a map with lines it could not read must not decide that. Fid 0 +must never be storable or matchable: 0 is what the trustlet writes into the +fid field when authentication FAILS, so a stored 0 would turn every rejection +into a match. And group 0 is never managed. */ import std; import Fingerprintd; @@ -48,7 +50,7 @@ int main() { for (const Entry& e : decoded.Entries()) if (e.fid == 0) anyZero = true; Check(!anyZero, "no entry ever holds fid 0, however it was created"); - Check(decoded.Size() == 1, "the zero-fid lines were dropped by the decoder"); + Check(decoded.Size() == 1, "the zero-fid lines make no entry"); } // ---- Names round-trip through fprintd's vocabulary @@ -128,12 +130,12 @@ int main() { "left-ring-finger 0\n" // the failure sentinel "right-thumb 8\n"; // good Map m = Map::Decode(damaged); - Check(m.Size() == 2, "only the two good lines survive"); + Check(m.Size() == 2, "only the two good lines become entries"); auto a = m.Lookup(1296911490); auto b = m.Lookup(8); Check(a && *a == Finger::RightIndex, "first good entry kept"); Check(b && *b == Finger::RightThumb, "last good entry kept"); - Check(!m.Has(Finger::LeftRing), "the zero-fid line was dropped"); + Check(!m.Has(Finger::LeftRing), "the zero-fid line made no entry"); Check(!m.Has(Finger::LeftMiddle), "trailing junk rejected, not truncated to 12"); // Total garbage yields an empty map, not a throw. @@ -146,8 +148,67 @@ int main() { Check(noNl.Size() == 1, "a missing trailing newline is not a lost entry"); } + // ---- A damaged map is not trusted, and stays that way through a save + { + const char* damaged = "right-thumb 1819675103\nleft-index-finger notanumber\nnonsense\n" + "right-index-finger 0\nright-index 123\n"; + Map m = Map::Decode(damaged); + Check(m.Size() == 1, "the one good line is kept"); + Check(m.Skipped() == 4 && m.Unparsed().size() == 4, + "the bad number, the line without a space, the fid 0 and the unknown name are kept as text"); + Check(Map::Decode("right-thumb 7\n\n").Skipped() == 0, "empty lines are not damage"); + Check(Map::Decode("").Skipped() == 0, "an empty map is a clean map"); + Check(!m.Trusted(), "a damaged map is not trusted"); + Check(Map::Decode("").Trusted(), "an absent or empty map is: no fingers is a true answer"); + + // The round trip: a save after an enrolment must not launder the damage. + m.Add(Finger::LeftThumb, 42); + Map again = Map::Decode(m.Encode()); + Check(!again.Trusted(), "still not trusted after Add + Encode + Decode"); + Check(again.Size() == 2 && again.Has(Finger::LeftThumb), "and the new finger was saved"); + Check(again.Unparsed().size() == 4 && again.Unparsed()[3] == "right-index 123", + "the lines it could not use come back verbatim"); + Check(Map::Decode(Map::Decode(damaged).Encode()).Encode() == Map::Decode(damaged).Encode(), + "a second round trip changes nothing"); + + // A finger named twice: the second line parses, but the first line's + // fid would be lost in memory while it still stands on disk. It is + // kept as data, so a delete of that finger finds it. + Map twice = Map::Decode("right-thumb 111\nright-thumb 222\n"); + Check(twice.Size() == 1 && twice.Lookup(111).has_value(), "the first line wins"); + Check(!twice.Trusted() && twice.Shadowed().size() == 1 && twice.Shadowed()[0].fid == 222, + "the second is kept as the finger and fid it names"); + Check(twice.Skipped() == 1 && twice.Unparsed().empty(), "and counted, but not as text"); + Check(Map::Decode(twice.Encode()).Encode() == "right-thumb 111\nright-thumb 222\n", + "it survives a round trip as the same line"); + auto named = twice.FidsNamed(Finger::RightThumb); + Check(named.size() == 2 && named[0] == 111 && named[1] == 222, "a delete of the finger gets both fids"); + Check(twice.Remove(Finger::RightThumb), "removing the finger"); + Map after = Map::Decode(twice.Encode()); + Check(!after.Has(Finger::RightThumb) && after.Trusted() && after.Size() == 0, + "takes the shadowed line with it: nothing comes back under that name"); + + // A sixth finger shadows the same way, and goes with its finger's delete. + Map six = Map::Decode("left-thumb 1\nleft-index-finger 2\nleft-middle-finger 3\n" + "left-ring-finger 4\nleft-little-finger 5\nright-thumb 6\n"); + Check(six.Size() == 5 && six.Full() && !six.Trusted() && six.Shadowed().size() == 1, "the sixth is shadowed"); + Check(six.FidsNamed(Finger::RightThumb).size() == 1 && six.FidsNamed(Finger::RightThumb)[0] == 6, + "and its finger's delete gets its fid"); + Check(six.AllFids().size() == 6, "every fid the file carries"); + + // Deleting every finger drops the kept lines too: "every" means every. + m.Clear(); + Check(m.Size() == 0 && m.Trusted(), "cleared, and trusted again"); + + Check(!Map::Unreadable().Trusted(), "a map that could not be read is not trusted"); + Check(!Map::Unreadable().Readable() && Map::Decode(damaged).Readable(), "and is the only kind that is not readable"); + Check(Map::Unreadable().Size() == 0, "and it names nothing"); + } + // ---- Groups { + Check(!GroupIsManaged(0), "group 0 is never tidied"); + Check(GroupIsManaged(10000), "a session user's group is"); Check(GidForUid(1000) == 1000, "gid is the uid"); Check(GidForUid(0) == 0, "root maps too"); Check(GidForUid(4294967295u) == 4294967295u, "the whole uid range maps"); diff --git a/tests/Ta/main.cpp b/tests/Ta/main.cpp index fdd84a9..95707fa 100644 --- a/tests/Ta/main.cpp +++ b/tests/Ta/main.cpp @@ -306,6 +306,33 @@ int main() { "reading at the payload offset directly gives the wrong word"); } + // ---- ENUMERATE: room for N entries in, {gid, fid} per template out + { + std::vector en(EnumeratePayloadSize); + BuildEnumeratePayload(en, 5); + Check(en.size() == 4 && Get32(en, 0) == 5, "the capacity is the one word"); + + std::vector resp(256); + auto put32 = [&](std::size_t off, std::uint32_t v) { + for (std::size_t i = 0; i < 4; i++) + resp[off + i] = static_cast((v >> (8 * i)) & 0xFF); + }; + std::size_t e0 = ResponsePayloadOff + EnumerateEntriesOff; + put32(e0 + 0, 10000); put32(e0 + 4, 1376187901); + put32(e0 + 8, 10000); put32(e0 + 12, 1103829000); + put32(e0 + 16, 10000); put32(e0 + 20, 7); + auto fids = EnumeratedFids(resp, 2); + Check(fids.size() == 2, "as many entries as the count says"); + Check(fids.size() == 2 && fids[0] == 1376187901 && fids[1] == 1103829000, + "the fid is the second word of each 8-byte entry"); + Check(EnumeratedFids(resp, 0).empty(), "a count of 0 lists nothing"); + Check(EnumeratedFids(resp, -2).empty(), "an error code lists nothing"); + // The count is the trustlet's word, the buffer is ours: a count past + // the buffer stops at the buffer. + Check(EnumeratedFids(resp, 1000).size() == (resp.size() - e0) / EnumerateEntrySize, + "bounded by the reply buffer, never by the count"); + } + // ---- The request/response envelope { std::vector req(256);