From db7a4f7b4e0837fb3c1d690e9c18d04ba19e9c63 Mon Sep 17 00:00:00 2001 From: Karn Date: Tue, 18 Aug 2026 04:49:50 +0530 Subject: [PATCH] feat(devices): tell same-machine rows apart by origin and fingerprint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A browser's device key lives in IndexedDB, which the browser scopes to the origin, so the same browser reaching the same daemon from 127.0.0.1, localhost and the vite dev server enrols as three devices — and their rows all read "Browser on " with nothing but dates to tell them apart. Record the Origin header of the enrolment POST on the local registry row and show it on the Devices row, beside a short fingerprint of the device id. First seen wins; older rows are backfilled on their next load (the browser enrols on every load); the origin stays out of the certificate, because it is a fact about how this machine was reached rather than about the device; and a row with no origin — paired by QR, admitted on a fleet cert — renders as ordinary. Closes #71 Co-Authored-By: Claude Fable 5 --- internal/crypto/devices.go | 50 ++++++++++++++++++++++++ internal/crypto/devices_test.go | 58 ++++++++++++++++++++++++++++ internal/daemon/fleet.go | 42 +++++++++++++++++++++ internal/daemon/fleet_test.go | 67 ++++++++++++++++++++++++++++++++- internal/daemon/server.go | 1 + internal/wire/control.go | 9 +++++ web/src/client/protocol.ts | 13 +++++++ web/src/routes/devices.test.tsx | 42 +++++++++++++++++++++ web/src/routes/devices.tsx | 58 +++++++++++++++++++++++++--- 9 files changed, 333 insertions(+), 7 deletions(-) diff --git a/internal/crypto/devices.go b/internal/crypto/devices.go index b02f8e7..425a8a8 100644 --- a/internal/crypto/devices.go +++ b/internal/crypto/devices.go @@ -29,6 +29,17 @@ type Device struct { // fleet directory will publish it — and empty for devices paired // before the fleet key existed. Cert []byte `json:"cert,omitempty"` + + // Origin is where this device first reached this machine from — the + // Origin header on its enrolment POST, e.g. "http://localhost:7719". + // A hint for telling otherwise-identical rows apart, never an identity: + // it is whatever the browser sent, it is only known for devices that + // enrolled over loopback, and empty is the ordinary state of every + // other row — one paired by ceremony, one admitted on a fleet cert, or + // one from before origins were recorded. First seen wins (SetOrigin), + // and it stays out of the certificate: it is a fact about how this + // machine was reached, not about the device. + Origin string `json:"origin,omitempty"` } // DeviceID derives the identity from the key itself, so an entry cannot @@ -265,6 +276,45 @@ func (s *DeviceStore) SetCert(publicKey, cert []byte) (bool, error) { return false, nil } +// SetOrigin records where a device enrolled from, once, reporting whether it +// wrote. A row that already has an origin keeps it — first seen wins, so a +// key that reaches this daemon from more than one place is labelled by where +// it turned up rather than by wherever its tab last loaded — and an empty +// origin, an unregistered key or a repeat are quiet no-ops, because the +// enrolment endpoint calls this on every page load. +// +// No revocation check, unlike SetCert and Relabel, and the asymmetry is the +// point: those write a credential, this writes a display hint. Revoking +// removes the row itself, so a revoked key has nothing here to stamp. +func (s *DeviceStore) SetOrigin(publicKey []byte, origin string) (bool, error) { + if len(publicKey) != 32 { + return false, fmt.Errorf("crypto: device key must be 32 bytes, got %d", len(publicKey)) + } + if origin == "" { + return false, nil + } + s.mu.Lock() + defer s.mu.Unlock() + devices, err := s.load() + if err != nil { + return false, err + } + for i := range devices { + if !bytes.Equal(devices[i].PublicKey, publicKey) { + continue + } + if devices[i].Origin != "" { + return false, nil + } + devices[i].Origin = origin + if err := s.save(devices); err != nil { + return false, err + } + return true, nil + } + return false, nil +} + // Relabel renames a device and replaces its certificate together, reporting // whether anything changed. Missing key, empty label or empty cert: no change, // no error. diff --git a/internal/crypto/devices_test.go b/internal/crypto/devices_test.go index 8d7a704..e519e4c 100644 --- a/internal/crypto/devices_test.go +++ b/internal/crypto/devices_test.go @@ -483,3 +483,61 @@ func TestRemoveByKeyComparesTheWholeKey(t *testing.T) { t.Fatalf("RemoveByKey of an absent key = %v, %v; want false and no error", ok, err) } } + +// TestSetOriginKeepsTheFirstOrigin: the origin is a first-seen hint, written +// once and never replaced. One device key can legitimately reach this daemon +// from more than one place — the vite dev server today, loopback tomorrow — +// and a field that tracked the latest would make every row read "wherever the +// tab last loaded from", which answers nothing at the moment of revoking. +func TestSetOriginKeepsTheFirstOrigin(t *testing.T) { + dir := t.TempDir() + s := NewDeviceStore(dir) + + key := testKey(t) + if _, err := s.Add("phone", key, nil); err != nil { + t.Fatalf("Add: %v", err) + } + + wrote, err := s.SetOrigin(key, "http://127.0.0.1:7719") + if err != nil || !wrote { + t.Fatalf("SetOrigin on a device with none = %v, %v; want it written", wrote, err) + } + dev, ok, err := s.FindByKey(key) + if err != nil || !ok { + t.Fatalf("FindByKey = %v, %v", ok, err) + } + if dev.Origin != "http://127.0.0.1:7719" { + t.Fatalf("stored origin = %q, want the one just written", dev.Origin) + } + + // A different origin later changes nothing: first seen wins. + wrote, err = s.SetOrigin(key, "http://localhost:7719") + if err != nil || wrote { + t.Fatalf("SetOrigin over an existing origin = %v, %v; want it refused", wrote, err) + } + dev, _, _ = s.FindByKey(key) + if dev.Origin != "http://127.0.0.1:7719" { + t.Fatalf("stored origin = %q; an existing origin was replaced", dev.Origin) + } + + // An empty origin records nothing — it is the ordinary state of a device + // that never enrolled over loopback, not a value. + fresh := testKey(t) + if _, err := s.Add("laptop", fresh, nil); err != nil { + t.Fatalf("Add: %v", err) + } + if wrote, err := s.SetOrigin(fresh, ""); err != nil || wrote { + t.Fatalf("SetOrigin with an empty origin = %v, %v; want a no-op", wrote, err) + } + + // A key nobody paired gets nothing, and no error: the caller's question is + // "did this stamp a row", and for a row that is not there the answer is no. + if wrote, err := s.SetOrigin(testKey(t), "http://127.0.0.1:7719"); err != nil || wrote { + t.Fatalf("SetOrigin for an unregistered key = %v, %v; want a no-op", wrote, err) + } + + // A malformed key is an error, as it is on every other mutation here. + if _, err := s.SetOrigin(key[:16], "http://127.0.0.1:7719"); err == nil { + t.Fatal("SetOrigin with a short key = nil, want an error") + } +} diff --git a/internal/daemon/fleet.go b/internal/daemon/fleet.go index 5c212ce..d465761 100644 --- a/internal/daemon/fleet.go +++ b/internal/daemon/fleet.go @@ -203,6 +203,29 @@ const EnrolPath = "/api/fleet/enrol" // allows for a strictly larger body. const maxEnrolBytes = 4 << 10 +// maxOriginBytes bounds what enrolOrigin will record. A real Origin is a +// scheme, a host and a port; 256 covers any of this daemon's own with room +// to spare. +const maxOriginBytes = 256 + +// enrolOrigin is what enrolment records about where the browser is: the +// request's Origin header, or nothing worth keeping. +// +// It is trusted exactly as far as a hint on a Devices row needs to be, and no +// further — recorded on the local registry row, shown beside the label, never +// minted into the certificate. By the time this runs, provenance +// (checkProvenance) has already refused any Origin that is not one of this +// daemon's own, so the value names which of the daemon's spellings the tab is +// on: 127.0.0.1 or localhost. "null" is the opaque origin — a browser +// declining to say — which is the same fact as no header at all. +func enrolOrigin(r *http.Request) string { + o := r.Header.Get("Origin") + if o == "null" || len(o) > maxOriginBytes { + return "" + } + return o +} + // enrolRequest is what the loopback UI posts: its own device public key, and // nothing else. // @@ -464,6 +487,19 @@ func (s *Server) handleEnrol(w http.ResponseWriter, r *http.Request) { if fresh, ok := s.relabelEnrolled(dev, key, fi); ok { cert = fresh } + // And the row's origin, back-filled when it never had one. This is + // the path every pre-origin row takes on its next page load — the + // browser calls this endpoint on every load — so older rows come + // right on their own rather than being singled out, and a row that + // has an origin keeps it (SetOrigin, first seen wins). Best effort + // like the relabel above; the broadcast is because nothing else + // pushes a list for this, and the screens already looking should not + // hold a row the registry has just outgrown. + if wrote, err := s.identity.Devices.SetOrigin(key, enrolOrigin(r)); err != nil { + s.logger().Warn("could not record this browser's origin", "device", dev.ID, "err", err) + } else if wrote { + s.broadcastDeviceList() + } } else { label := enrolLabel(s.hostname) cert, err = fi.Key.Sign(fleet.DeviceCert{ @@ -485,6 +521,12 @@ func (s *Server) handleEnrol(w http.ResponseWriter, r *http.Request) { } s.logger().Info("enrolled this machine's own browser as a fleet device", "device", dev.ID, "label", dev.Label, "pairedOn", fi.MachineID) + // Where the browser came from, stamped before the broadcast below + // announces the row. Best effort: an enrolment holding a usable + // certificate must not fail over a display hint. + if _, err := s.identity.Devices.SetOrigin(key, enrolOrigin(r)); err != nil { + s.logger().Warn("could not record this browser's origin", "device", dev.ID, "err", err) + } // Nothing is published to the fleet directory, exactly as the pairing // ceremony publishes nothing: device certificates go to the device they // are about and nowhere else (spec/fleet-trust.md, "Device certificates diff --git a/internal/daemon/fleet_test.go b/internal/daemon/fleet_test.go index a3a52e6..f42ab0f 100644 --- a/internal/daemon/fleet_test.go +++ b/internal/daemon/fleet_test.go @@ -224,6 +224,14 @@ func newEnrolServer(t *testing.T, machineID string) (*httptest.Server, *Server, // postEnrol is the request the loopback UI makes: a POST from this daemon's // own origin, authenticated by the session cookie, carrying one key. func postEnrol(t *testing.T, ts *httptest.Server, publicKey []byte) *http.Response { + t.Helper() + return postEnrolFrom(t, ts, publicKey, ts.URL) +} + +// postEnrolFrom is postEnrol from a chosen origin. The daemon owns two +// (127.0.0.1 and localhost, same port), and which of them the tab is on is +// exactly the fact enrolment records. +func postEnrolFrom(t *testing.T, ts *httptest.Server, publicKey []byte, origin string) *http.Response { t.Helper() body, err := json.Marshal(map[string]string{ "publicKey": base64.StdEncoding.EncodeToString(publicKey), @@ -237,7 +245,7 @@ func postEnrol(t *testing.T, ts *httptest.Server, publicKey []byte) *http.Respon } req.Header.Set("Content-Type", "application/json") req.AddCookie(&http.Cookie{Name: tsCookie(ts), Value: tok}) - req.Header.Set("Origin", ts.URL) + req.Header.Set("Origin", origin) req.Header.Set("Sec-Fetch-Site", "same-origin") resp, err := http.DefaultClient.Do(req) if err != nil { @@ -635,6 +643,63 @@ func TestEnrolBackfillsACertificateADeviceHasNone(t *testing.T) { } } +// TestEnrolRecordsWhereTheBrowserCameFrom. +// +// Several loopback tabs enrol as several devices — IndexedDB is scoped to the +// origin, so http://127.0.0.1:7719 and http://localhost:7719 hold different +// keys — and their rows used to be identical: same label, same machine, +// nothing but dates. The Origin header on the enrolment POST is the one fact +// that differs, the daemon already has it, and nothing new is asked for or +// trusted: it is recorded as a hint on the local row, never minted into the +// certificate. +func TestEnrolRecordsWhereTheBrowserCameFrom(t *testing.T) { + ts, srv, _ := newEnrolServer(t, "karns-mbp-a1b2-0f9a12cd") + key := deviceKey(0x5c) + + enrolOK(t, ts, key) + list := devices(t, srv) + if len(list) != 1 || list[0].Origin != ts.URL { + t.Fatalf("registry = %+v, want one row with origin %q", list, ts.URL) + } + + // The wire carries it, so a Devices screen can put it on the row. + wl, err := srv.deviceList() + if err != nil { + t.Fatalf("deviceList: %v", err) + } + if len(wl.Devices) != 1 || wl.Devices[0].Origin != ts.URL { + t.Fatalf("deviceList = %+v, want the enrolment origin on the row", wl.Devices) + } + + // A later load does not rewrite history: the row keeps the origin it + // enrolled from, whichever of the daemon's origins the tab is on today. + other := strings.Replace(ts.URL, "127.0.0.1", "localhost", 1) + if resp := postEnrolFrom(t, ts, key, other); resp.StatusCode != http.StatusOK { + t.Fatalf("re-enrolment from %q = %d, want 200", other, resp.StatusCode) + } + if list := devices(t, srv); list[0].Origin != ts.URL { + t.Errorf("origin after a re-enrolment = %q, want the first one %q", list[0].Origin, ts.URL) + } + + // A row from before origins were recorded picks one up on its next load — + // the browser calls this endpoint on every load, so the backfill is free + // and older rows are never singled out. + older := deviceKey(0x5d) + if _, err := srv.identity.Devices.Add("an older enrolment", older, nil); err != nil { + t.Fatalf("Add: %v", err) + } + if resp := postEnrolFrom(t, ts, older, other); resp.StatusCode != http.StatusOK { + t.Fatalf("enrolling the older row = %d, want 200", resp.StatusCode) + } + dev, ok, err := srv.identity.Devices.FindByKey(older) + if err != nil || !ok { + t.Fatalf("FindByKey = %v, %v", ok, err) + } + if dev.Origin != other { + t.Errorf("older row's origin = %q, want it back-filled to %q", dev.Origin, other) + } +} + // TestDeviceListSaysWhichMachinePairedEachRow. // // A machine on a fleet admits devices two ways — the ones it paired itself, and diff --git a/internal/daemon/server.go b/internal/daemon/server.go index 20c106d..90e7e90 100644 --- a/internal/daemon/server.go +++ b/internal/daemon/server.go @@ -1487,6 +1487,7 @@ func (s *Server) deviceList() (wire.DeviceList, error) { PairedAt: d.PairedAt.Unix(), LastSeen: d.LastSeen.Unix(), PairedOn: pairedOn(fleetPub, d.Cert), + Origin: d.Origin, }) } return wire.DeviceList{Devices: infos}, nil diff --git a/internal/wire/control.go b/internal/wire/control.go index dc64cb0..80ec44e 100644 --- a/internal/wire/control.go +++ b/internal/wire/control.go @@ -524,6 +524,15 @@ type DeviceInfo struct { // own — the conservative direction, since it is the one that does not offer // a fleet-wide revoke on the strength of a blob that proved nothing. PairedOn string `json:"pairedOn,omitempty"` + // Origin is where the device first reached this machine from — the Origin + // header on its enrolment POST, recorded on the local row and never minted + // into the certificate (crypto.Device). It exists because several loopback + // tabs enrol as several devices, one per browser origin, and their rows + // are otherwise identical. A hint, not an identity: absent is the ordinary + // state of a device paired by ceremony, admitted on a fleet cert, or + // enrolled before origins were recorded, and a screen must render absent + // as unremarkable rather than suspicious. + Origin string `json:"origin,omitempty"` } // DeviceList answers devices, and follows a revoke that succeeded. diff --git a/web/src/client/protocol.ts b/web/src/client/protocol.ts index 7cac2c7..dd492ba 100644 --- a/web/src/client/protocol.ts +++ b/web/src/client/protocol.ts @@ -138,6 +138,19 @@ export interface DeviceInfo { * revoke on the strength of a blob that proved nothing. */ pairedOn?: string + /** + * Where the device first reached the daemon from — the Origin header on its + * enrolment POST, e.g. "http://localhost:7719" — recorded on the daemon's + * local row and never minted into the certificate. + * + * It exists because several loopback tabs enrol as several devices, one per + * browser origin (IndexedDB scopes the device key to the origin), and their + * rows are otherwise identical. A hint, not an identity: absent is the + * ordinary state of a device paired by ceremony, admitted on a fleet cert, + * or enrolled before origins were recorded, and a screen must render absent + * as unremarkable rather than suspicious. + */ + origin?: string } // Client -> server. diff --git a/web/src/routes/devices.test.tsx b/web/src/routes/devices.test.tsx index 8e490a6..ec1e03c 100644 --- a/web/src/routes/devices.test.tsx +++ b/web/src/routes/devices.test.tsx @@ -187,6 +187,48 @@ describe('DevicesRoute', () => { expect(screen.getByText('2h ago')).toBeTruthy() }) + /** + * The indistinguishable-rows fix (issue #71): several loopback tabs enrol as + * several devices — one per browser origin — and their rows read identically: + * same label, same machine, nothing but dates. The daemon now records the + * origin the enrolment came from and every row carries a short fingerprint of + * its key, so four rows saying "Browser on mb.local" can finally be told + * apart. + */ + it('tells identically-labelled rows apart by origin and fingerprint', async () => { + const { sock } = await mountDevices() + + listed(sock, [ + device({ id: 'aa11bb22cc33', label: 'Browser on mb.local', origin: 'http://127.0.0.1:7719' }), + device({ id: 'dd44ee55ff66', label: 'Browser on mb.local', origin: 'http://localhost:7719' }), + ]) + + // The origin reads as a place, not a URL: the scheme says nothing a reader + // of this list acts on, and the host and port are the whole distinction. + expect(screen.getByText('127.0.0.1:7719')).toBeTruthy() + expect(screen.getByText('localhost:7719')).toBeTruthy() + expect(screen.queryByText('http://127.0.0.1:7719')).toBeNull() + + // The fingerprint is the id's first six characters — the id is already a + // digest of the device key, so nothing new is derived here. + expect(screen.getByText('aa11bb')).toBeTruthy() + expect(screen.getByText('dd44ee')).toBeTruthy() + }) + + it('renders a row with no origin as ordinary, not as suspect', async () => { + const { sock } = await mountDevices() + + // A phone paired by QR has no meaningful origin, and every row from + // before origins were recorded has none either. Both are the ordinary + // state: the row shows its dates and its fingerprint and says nothing + // about what it does not know. + listed(sock, [device({ id: 'aa11bb22cc33', label: 'iPhone' })]) + + expect(screen.getByText('iPhone')).toBeTruthy() + expect(screen.getByText('aa11bb')).toBeTruthy() + expect(screen.queryByText(/unknown/i)).toBeNull() + }) + it('asks before it revokes', async () => { const { sock } = await mountDevices() listed(sock, [device({ id: 'aa11bb22cc33', label: 'iPhone' })]) diff --git a/web/src/routes/devices.tsx b/web/src/routes/devices.tsx index 8a70d03..24b7901 100644 --- a/web/src/routes/devices.tsx +++ b/web/src/routes/devices.tsx @@ -165,6 +165,35 @@ const META_TEXT = 'text-xs/6 whitespace-nowrap tabular-nums text-zinc-500 dark:t /** The quiet per-row control, at rest and while it is armed. */ const ROW_BUTTON = 'text-zinc-500 dark:text-zinc-400' +/** + * How much of a device id a row shows as its fingerprint. + * + * The id is already a digest of the device key, so the row derives nothing + * new — it prints the first six characters, which is enough to tell apart the + * handful of rows one machine holds while staying narrow enough to sit beside + * a label on a phone. It exists because rows can otherwise be identical to + * the word (see the origin note below), and it is on every row, because a row + * on a sibling machine has no origin to be told apart by. + */ +const FINGERPRINT_CHARS = 6 + +/** + * The origin a device enrolled from, said as a place rather than a URL. + * + * The daemon records the Origin header of the enrolment POST, so several + * loopback tabs — each a device of its own, because the device key lives in + * storage the browser scopes per origin — stop reading identically. The + * scheme is dropped because it distinguishes nothing a reader of this list + * acts on; the host and port are the whole difference. Null for a row with no + * origin, which is ordinary rather than suspect — a phone paired by QR, a + * device admitted on the fleet's word, or a row from before origins were + * recorded — and the row simply says nothing about what it does not know. + */ +function enrolledFrom(origin?: string): string | null { + if (origin === undefined || origin === '') return null + return origin.replace(/^[a-z][a-z0-9+.-]*:\/\//i, '') +} + /** * The paired devices, one row each, with the revoke each row carries. * @@ -382,8 +411,20 @@ function DeviceRow({ }) { return (
  • - - {d.label} + {/* + The label and its fingerprint share the flexible slot, so the + fingerprint sits beside the words it disambiguates rather than drifting + into the dates on the right. The label is what truncates; six characters + of digest are the one part of the row that must never be the part that + goes. + */} + + + {d.label} + + + {d.id.slice(0, FINGERPRINT_CHARS)} + {/* Said in the row rather than left to the label, because the label is @@ -397,11 +438,16 @@ function DeviceRow({ )} {/* - Paired first, last seen second, reading right — and the first of - the two is what goes at narrow widths. Which was seen when is the - live fact, the one somebody checks before revoking; when it was - paired is history, and history is what a phone can do without. + Where it enrolled from, then paired, then last seen, reading right — + and the first two are what go at narrow widths. Which was seen when is + the live fact, the one somebody checks before revoking; where a tab + enrolled and when it paired are history, and history is what a phone + can do without — the rows a phone most needs to tell apart are its + siblings with no origin at all, which the fingerprint covers. */} + {enrolledFrom(d.origin) !== null && ( + {enrolledFrom(d.origin)} + )} paired {ago(d.pairedAt)} {ago(d.lastSeen)} {!revocable ? null : armed ? (