diff --git a/.agents/sow/SOW-status.md b/.agents/sow/SOW-status.md index 92099810..f62c9314 100644 --- a/.agents/sow/SOW-status.md +++ b/.agents/sow/SOW-status.md @@ -43,13 +43,11 @@ order; the user may reorder it. differs (completed 2026-11-06, options A and 2B). 22. SOW-0175 - Split the journalctl rewrites by responsibility (tracked by user decision 2026-11-07; after SOW-0097). -23. SOW-0176 - Single ownership follow-ups: open decisions 8 and 9, I7 and - divergences found in SOW-0097 (needs user decisions). +23. SOW-0177 - Report the validated sealed range from verification (tracked + 2026-11-08 from SOW-0176; needs a public API decision). ## Current -None. - ## Pending - SOW-0066 - V1 Release: v1.0.0 API freeze and release; not scheduled (user @@ -77,14 +75,15 @@ None. by user decision. - SOW-0171 - Continue Sealing When Reopening A Sealed File: tracked 2026-11-03 (SOW-0156 decision N1); not scheduled. +- SOW-0177 - Report The Validated Sealed Range From Verification: tracked from + SOW-0176 item I7; needs a public API decision. - SOW-0175 - Split The journalctl Rewrites By Responsibility: tracked 2026-11-07 (SOW-0097 decision 7); after SOW-0097. -- SOW-0176 - Single Ownership Follow-ups: tracked 2026-11-08 from SOW-0097; - needs user decisions (legacy export contract, Rust list_boots Result) plus - I7 and six Go/Rust or SDK/systemd divergences. ## Recently Closed Or Completed +- SOW-0176 - Single Ownership Follow-ups: completed 2026-11-08; follow-up + SOW-0177. - SOW-0174 - Rust Registry Unwatch Leaves Files When The Directory Key Differs: completed 2026-11-06. Unwatch forgets exactly the files no remaining watch covers, overlapping watches work in any order, failures diff --git a/.agents/sow/done/SOW-0176-20261108-single-ownership-followups.md b/.agents/sow/done/SOW-0176-20261108-single-ownership-followups.md new file mode 100644 index 00000000..c47577aa --- /dev/null +++ b/.agents/sow/done/SOW-0176-20261108-single-ownership-followups.md @@ -0,0 +1,418 @@ +# SOW-0176 - Single Ownership Follow-ups: Open Decisions And Divergences + +## Status + +Status: completed + +`completed` is the successful terminal status. `done` is a directory name, not a status value. Do not use `Status: done` or `Status: complete`. + +Sub-state: activated 2026-11-08 on branch `sow-0176/single-ownership-followups` +(stacked on SOW-0097); decisions 8 and 9 made; paused 2026-11-08 by user +decision while the stacked PRs' review findings are fixed (decision 1A); +resumed 2026-11-08 with every decision made (see `## Implications And +Decisions`). + +## Requirements + +### Purpose + +Finish what SOW-0097 (single ownership of journal behavior) left for the +user: two public-contract decisions, the verify-key item that needs an FSS +oracle, and Go/Rust or SDK/systemd divergences found while consolidating. + +### User Request + +SOW-0097 user decisions: one owner per journal behavior, public SDK function +in both languages, tests first, Go/Rust parity. Public API and user-visible +changes are the user's to decide. + +### Assistant Understanding + +Facts (each found during SOW-0097, behavior unchanged by it): + +1. Decision 8: legacy `ExportEntry` (Go) / `export_entry_bytes` (Rust) keep + the SOW-0043 lossless contract (invalid and non-UTF-8 field names stay in + the output), while `EntryEncoder` stops at them as systemd v262 does. +2. Decision 9: Rust `DirectoryReader::list_boots` and `SdJournal::list_boots` + return `Vec` and fall back to one boot per file header on read + errors; Go `DirectoryReader.ListBoots` returns an error. +3. I7: both CLIs follow systemd 255 verify-key handling; v262 parses per file + with different messages; the pinned v262 oracle is built without FSS, so + parity cannot be certified; the SDK key parser matches neither. +4. For a file without `_BOOT_ID` DATA, v262 `--list-boots` prints "No boot + found." while both rewrites list ENTRY header boot IDs. +5. On `main_entry_array_missing.journal` (verify negative corpus) v262 fails + to open the file, Go `--list-boots` prints "No boot found." and Rust lists + the boot. +6. Go directory retention never deletes `*.journal~` files; check against the + Rust writer and the systemd vacuum rule. +7. Rust `DirectoryReader::open_files` does not deduplicate repeated paths; + Go does (both CLIs now call `UniqueFiles` / `unique_files`). +8. Subdirectory read errors during directory discovery are handled + differently by Go and Rust. +9. The Go reader fails a whole entry on a payload without `=`; the Rust + reader keeps the entry. + +Unknowns: + +- Which side is right for items 4-9, against systemd v260.1/v262 sources. + +### Acceptance Criteria + +- Decisions 8 and 9 made by the user and applied in both languages, or + recorded as rejected. +- I7 done with an FSS-enabled v262 oracle, or rejected with evidence. +- Items 4-9: each checked against the owning oracle and fixed in both + languages with tests, or rejected with evidence. + +## Analysis + +Evidence is in SOW-0097 (Execution Log, F2 sweep, Implications And +Decisions), plus the 2026-11-08 investigations: + +- Item 4: v262 `discover_next_id` (systemd/systemd @ v262 + src/shared/logs-show.c:2021-2159) needs `_BOOT_ID` DATA; with none, + `--list-boots` prints "No boot found." and `-b 0` fails. The SDK header + fallback (`go/journal/boots.go`, `rust/src/journal/src/boots.rs`) lists the + header boot, after which `-b 0` prints "-- No entries --". No oracle + fixture lacks `_BOOT_ID`. Related review threads: PR#26 T88/T90/T91/T92/T95. +- Item 5: v262 refuses `entry_array_offset == 0 && tail_entry_array_offset != + 0` at open (systemd/systemd @ v260.1 src/libsystemd/sd-journal/ + journal-file.c:646). Go recovers an empty prefix (0 entries everywhere); + Rust activates recovery lazily, so filtered reads and `boot_ranges` see 5 + entries while unfiltered reads see 0. +- Item 6 confirmed: Go retention parses only `.journal` names + (`go/journal/log.go:1297-1322`); Rust counts and deletes `.journal~` first + (`journal-log-writer/src/log/chain.rs:142-162`); journald vacuums them + (journal-vacuum.c:217-248). +- Item 7 confirmed: Rust `DirectoryReader::open_files_with_options` opens + duplicates (same file twice: Rust 10 entries, Go 5); the facade already + dedupes; systemd dedupes (sd-journal.c:1611-1643). +- Item 8 mostly refuted: both skip unreadable subdirectories and fail on an + unreadable root; narrow differences: mid-listing read errors and the error + class for a missing root. +- Item 9 confirmed: Go `readEntryAt` fails the whole entry on a payload + without `=` or an unreadable DATA object (`go/journal/reader_entry.go: + 146-155`); Rust keeps the raw payload (`reader_helpers.rs:61-79`); systemd + returns payloads as-is (sd-journal.c:2889-2898). Affects the Go facade + seqnum/monotonic getters and Netdata row expansion. +- New: single-value field maps keep the first repeated value in Go and the + last in Rust (`reader_helpers.rs:75`); systemd returns the first. +- I7: an FSS-enabled v262 oracle needs OpenSSL (corrected 2026-11-08; the first + analysis said libgcrypt). + +## Pre-Implementation Gate + +Status: ready + +Problem / root-cause model: parallel Go/Rust and CLI/SDK implementations +drifted before single ownership was a project rule. + +Evidence reviewed: SOW-0097. + +Affected contracts and surfaces: Go and Rust reader, directory and export +APIs; both journalctl rewrites; Go directory retention. + +Existing patterns to reuse: SOW-0097 goldens (`tests/output/`), query and +verify matrices. + +Risk and blast radius: decisions 8 and 9 change public contracts; item 6 +changes retention (data deletion); item 9 changes what readers return. + +Sensitive data handling plan: + +- Synthetic fixtures and public systemd test data only. + +Implementation plan: per item, test first, both languages. + +Validation plan: unit tests in both languages plus the query, directory and +verify matrices with the pinned oracles. + +Artifact impact plan: specs and API docs for any contract change. + +Open decisions: none; all recorded under `## Implications And Decisions`. + +## Implications And Decisions + +User decisions on 2026-11-08 (user agreed with both recommendations): + +- Decision 8 = A: legacy `ExportEntry` / `export_entry_bytes` keep the + SOW-0043 lossless contract. No code change; rejected as a change. +- Decision 9 = B: Rust `DirectoryReader::list_boots` and + `SdJournal::list_boots` return `Result`, as Go does; the header-summary + fallback is removed. + +User decisions on 2026-11-08 about the open stacked PRs' bot reviews: + +- 1 = A: fix every valid finding on the branch it belongs to, bottom-up, + merge up the stack, rerun validation, reply in and resolve every thread. + This SOW pauses until that is done. +- 2 = agree: not fixed in the PRs, replied with reasons: test-helper + duplication (T33/T44), npm lockfile (T86), `runFollow` complexity + (T54, SOW-0175); the reader-level live posting bound (T67) and Netdata + non-UTF-8 namespace-name normalization (T69/T84) are added to this SOW. + +Investigation results for items 3-9 (2026-11-08, read-only helpers, verified +where noted): see `## Analysis`. + +User decisions on 2026-11-08 on the options below: item 4 = B, 5 = B, 6 = A, +7 = A, 8 = A, 9 = A, new = yes, I7 = A. I7 = A is the user's approval to install +`libgcrypt20-dev` system-wide, the one exception to the repository boundary. + +Options as presented (evidence in `## Analysis`): + +- Item 4, boot listing without `_BOOT_ID` DATA. A: drop the header fallback + everywhere (matches v262, but `ListBoots` returns nothing for files the core + writer makes without that field). B: CLIs match v262 and SDK callers keep the + fallback through a call option (CLI parity and useful SDK results, at the cost + of two documented behaviors). C: keep as is (the CLI contradiction stays). + Recommendation: B, long-term-best. +- Item 5, main entry array missing. A: reject at open as systemd does (changes + the SOW-0159 recovery contract). B: Rust detects it at open as Go does (parity; + still differs from systemd for this corruption). C: both. Recommendation: B, + surgical. +- Item 6, Go retention ignores `.journal~`. A: Go adopts Rust's counting and + deletion (bounded disk use; Go starts deleting disposed files, oldest first). + B: Rust stops counting them (unbounded growth in both). Recommendation: A, + long-term-best. +- Item 7, Rust `open_files` with repeated paths. A: call the existing + `unique_files` helper (parity, no known cost). Recommendation: A, surgical. +- Item 8, unreadable subdirectories. A: record the two narrow differences and + close. B: align both (low value). Recommendation: A, surgical. +- Item 9, Go fails a whole entry on a damaged field. A: Go behaves like Rust and + systemd (no lost rows; callers no longer get an error for that corruption). B: + Rust errors like Go (diverges from systemd, loses rows). Recommendation: A, + long-term-best. +- New, repeated field in the single-value map. Rust keeps the first value, as Go + and `sd_journal_get_data` do; callers relying on the last value would see a + change. Recommendation: yes, surgical. +- I7, verify-key parity with v262. A: install `libgcrypt20-dev` and rebuild the + oracle (system package, needs approval). B: build libgcrypt inside `.local/` + (no system change, more effort). C: reject for now; the CLIs keep the 255 + behavior until a release needs it. Recommendation: C, surgical. + +## Plan + +One chunk per item, test first, both languages, committed when verified: + +1. Item 7, Rust `open_files` deduplicates. +2. New, Rust single-value maps keep the first repeated value. +3. Item 9, Go keeps entries with damaged fields. +4. Item 5, Rust detects a missing main entry array at open. +5. Item 6, Go retention counts and deletes `*.journal~`. +6. Item 4, CLIs match v262; SDK callers opt into the header fallback. +7. T67 reader-level live posting bound; T69/T84 Netdata non-UTF-8 namespace + names. +8. I7, FSS-enabled v262 oracle and verify-key parity. +9. Item 8, record the two differences and close. + +## Implementation And Review Plan + +Implementation: the project manager implements test-first, one item at a time, +committing verified chunks, in both languages unless a decision says otherwise. + +Reviewers: whole-SOW self-review after local validation; tests are the gate. +External reviewers are not used. + +Repository boundary block for every helper-agent prompt: + +```text +CRITICAL REPOSITORY BOUNDARY: +- Do not make changes outside this repository for any reason. +- Repository path: current repository root. +- You may inspect external references read-only when the task requires it. +- Write, edit, delete, move, reset, checkout, install, generate, cache, or format nothing outside this repository. +- The only write exception outside the repository is /tmp. +- Prefer .local/ inside this repository for scratch work, generated temporary files, cloned references, logs, and working notes. +``` + +Failure handling: implementation, validation, reviewer and audit failures stop +advancement until fixed or decided by the user, and are recorded in the +Execution Log. + +## Execution Log + +### 2026-11-08 + +- I7 prerequisite, correction: `libgcrypt20-dev` 1.10.3-2ubuntu0.2 and its + dependency `libgpg-error-dev` were installed with the user's approval, then + found unnecessary: v262 sealing uses OpenSSL (`src/shared/journal-authenticate.c:21`, + `fsprg-openssl.c`), and the earlier analysis naming libgcrypt was wrong. + `libssl-dev` was already installed, so the oracle is rebuilt with + `-Dopenssl=enabled` (`+OPENSSL`). + +- Item 4: the default boot list has no header fallback, as v262; a + `BootListOptions` flag opts in. Noncanonical `_BOOT_ID` DATA counts as present + (T90/T91), and read errors are returned. +- Item 5: Rust activates recovery at open when the main entry array is missing. +- Item 6: Go retention counts and deletes `*.journal~`, first and oldest first. + Same-path fix: Rust did not count disposed chain-named files + (`@...@-.journal~`), which parse as an unknown source. Go + max-age stops depending on the delete order. +- T67: Rust `file_boot_ranges` stops at `reader_last_visible_entry`. +- T69/T84: non-UTF-8 namespace and remote source names become U+FFFD in both. + +- Item 7: Rust `DirectoryReader::open_files_with_options` calls `unique_files`. +- New: Rust single-value maps keep the first repeated value. +- Item 9: Go keeps an entry with a damaged field and returns a payload without + `=` in `Payloads` only. Where Rust and systemd differed, systemd decided: + a DATA object with the wrong type or a bad size is skipped in both languages + (`sd-journal.c` skips `-EADDRNOTAVAIL` and `-EBADMSG`). Decompression failures + still fail the entry in both, as before. + Tests: `tests/entry_damage.rs`, `reader_entry_damage_test.go`. + +- Bot-review decision 1A delivered on the stack below this branch and merged + up; PR #26 T60 and T72 are answered by this file's decision list and + Implementation And Review Plan. + +- Decision 9 implemented: `DirectoryReader::list_boots` returns + `Result>` (the internal `try_list_boots` and the header + fallback are gone); `SdJournal::list_boots` returns + `Result, Error>`. Callers were the facade, tests and the + conformance adapter (already on `SdJournalListBoots`); Netdata does not + call it. Docs (`rust/README.md`, `docs/Rust-API.md`) and + `product-scope.md` updated. `cargo test --workspace` PASS; wiki check and + the `rust-list-boots` example PASS. + +- I7 implemented with the OpenSSL-enabled v262 oracle. `ParseVerificationKey` / + `parse_verification_key` are now the single owner of the key grammar (v262 + `journal_auth_load_key`: dashes between seed bytes, `sscanf %lx` whitespace, + sign, `0x`, saturation, content after the interval ignored, zero values and + overflowing start rejected). The duplicate CLI validators are gone. Both CLIs + judge the key per file and stop on the first rejection, with v262's message. + Differential of 29 keys on sealed and plain files against stock v262: 0 + exit-code differences (`.local/pr-review/keydiff.py`; includes a truly wrong + seed, which the earlier all-zero case could not test). +- I7 remaining stderr differences: v262 prints `=> Validated from ...` after + PASS on sealed files and `FAIL: %s: %m` with an errno string. The first needs + a new public SDK report of the sealed range, tracked by pending SOW-0177. The + second is kept: our text names the failing check and offset, and exit codes + match. +- Item 8 closed by decision A. Two narrow differences stay and are accepted: (1) + a read error in the middle of listing a directory; (2) the error class for a + missing root directory. Both skip unreadable subdirectories and fail on an + unreadable root, as the analysis found. +- Self-review found `lossyUTF8` replaced every invalid byte while Rust's + `to_string_lossy` replaces a maximal truncated sequence once, so a truncated + multi-byte name differed between languages. Fixed in Go with cases matched + against Rust's output. + +## Validation + +Acceptance criteria evidence: + +- Decisions 8 and 9: applied in both languages (Execution Log). +- I7: done against the OpenSSL-enabled v262 oracle; 29 keys on sealed and plain + files, 0 exit-code differences. +- Items 4-7, 9, New, T67, T69/T84: fixed in both languages with paired tests. + Item 8: closed by decision A with the two differences recorded. + +Tests or equivalent validation (2026-11-08, stock = systemd 262 oracle): + +- `go test ./journal ./cmd/...` PASS; `CGO_ENABLED=0 go build ./journal` PASS; + `cargo test --workspace` PASS. +- `run_byte_identity.py --final-state all`: PASS, every `all_equal` true. +- `run_verify_matrix.py`, `run_directory_matrix.py`: PASS. +- `run_mixed_directory_matrix.py`: 42/42. `run_live_matrix.py`: 18/18 (2 + writers, 30 entries each, stock, libsystemd and SDK readers, final verify). +- `run_journalctl_query_matrix.py` with follow: PASS, including the new + no-`_BOOT_ID` exit and stdout cases. +- `run_reader_hardening.py --go-adapter --rust-adapter + --journalctl + .local/sow0045-oracle/systemd-v262-build/journalctl --out `, + all three groups (chains, streams, recovery): PASS, 552 observations. +- `check_v260_manifest.py`, `run_parser_parity.py`, `check_wiki_docs.py`, + `verify_examples.py`, `check_duplication.py`: PASS. + +Real-use evidence: + +- Both built CLIs and stock v262 run against the same sealed and plain files in + the key differential, and against the matrix fixtures above. + +Reviewer findings: + +- Project-manager review of the whole changed surface; no external reviewers, + per `AGENTS.md`. One finding: Go `lossyUTF8` differed from Rust on truncated + multi-byte sequences. Fixed and tested. Core-purity search of the diff found + no process launches or host-path access. Compressed-DATA failures still fail + the entry in both languages by design. + +- PR review, same scope. Fixed: Go payload visitors and both languages' entry + enumeration now skip a damaged DATA object as `GetEntry` does (systemd + sd-journal.c:2896); Rust stops trusting the tail hint alone while a writer + holds the file (the Go writer publishes it before `n_entries`); doc wording + for retention order, field-level damage and `list_boots` errors. Rejected: + file-identity check before retention deletes, because journald also unlinks + by name (journal-vacuum.c:263, 307) and one writer owns the directory. + Undecompressible DATA: the user decided on 2026-11-09 that it costs one field + (decision 1B), delivered by SOW-0178; this SOW keeps failing the entry. + +Same-failure scan: + +- Key parsing: no other parser remains (`grep` for `valid_verification_key`, + `validVerificationKey`). Lossy conversion: `lossyUTF8` is the only Go + conversion of source names. Retention: the root-retention path has its own + contract and was not changed. + +Sensitive data gate: + +- Durable artifacts contain synthetic fixtures and public systemd references + only. + +Artifact maintenance gate: + +- AGENTS.md: no change; no workflow or project rule changed. +- Runtime project skills: no change; the matrix guidance still holds. +- Specs: `journalctl-v260-parity-matrix.md` (verify-key) and `product-scope.md` + (boot listing, retention). +- End-user/operator docs: `docs/Go-API.md`, `docs/Rust-API.md`, `go/API.md`, + `rust/README.md`, `docs/Reader-APIs.md`, `docs/Writer-APIs.md`. +- End-user/operator skills: none exist. +- SOW lifecycle: `Status: completed`, moved to `done/`. +- SOW-status.md: updated. + +Specs update: + +- As listed in the artifact maintenance gate. + +Project skills update: + +- None needed; no new way of working. + +End-user/operator docs update: + +- As listed in the artifact maintenance gate. + +End-user/operator skills update: + +- None exist. + +Lessons: + +- See `## Lessons Extracted`. + +Follow-up mapping: + +- `=> Validated` summary line: tracked by SOW-0177. +- Undecompressible DATA costs one field: delivered by SOW-0178. +- `FAIL` wording and item 8's two differences: accepted, recorded above. + +## Outcome + +All decisions delivered in Go and Rust. Verify-key handling now matches v262 on +exit status through one SDK parser per language, against an FSS-capable oracle. + +## Lessons Extracted + +- Check a feature's real dependency in the oracle's source before asking for a + system package: v262 sealing needs OpenSSL, not libgcrypt. +- A differential case is only evidence if its inputs differ: an all-zero + "wrong" seed equalled the good one. + +## Followup + +SOW-0177 tracks the validated-range summary. + +## Regression Log + +None. diff --git a/.agents/sow/pending/SOW-0176-20261108-single-ownership-followups.md b/.agents/sow/pending/SOW-0176-20261108-single-ownership-followups.md deleted file mode 100644 index 671c65ec..00000000 --- a/.agents/sow/pending/SOW-0176-20261108-single-ownership-followups.md +++ /dev/null @@ -1,129 +0,0 @@ -# SOW-0176 - Single Ownership Follow-ups: Open Decisions And Divergences - -## Status - -Status: open - -`completed` is the successful terminal status. `done` is a directory name, not a status value. Do not use `Status: done` or `Status: complete`. - -Sub-state: needs user decisions; tracked from SOW-0097 on 2026-11-08. - -## Requirements - -### Purpose - -Finish what SOW-0097 (single ownership of journal behavior) left for the -user: two public-contract decisions, the verify-key item that needs an FSS -oracle, and Go/Rust or SDK/systemd divergences found while consolidating. - -### User Request - -SOW-0097 user decisions: one owner per journal behavior, public SDK function -in both languages, tests first, Go/Rust parity. Public API and user-visible -changes are the user's to decide. - -### Assistant Understanding - -Facts (each found during SOW-0097, behavior unchanged by it): - -1. Decision 8: legacy `ExportEntry` (Go) / `export_entry_bytes` (Rust) keep - the SOW-0043 lossless contract (invalid and non-UTF-8 field names stay in - the output), while `EntryEncoder` stops at them as systemd v262 does. -2. Decision 9: Rust `DirectoryReader::list_boots` and `SdJournal::list_boots` - return `Vec` and fall back to one boot per file header on read - errors; Go `DirectoryReader.ListBoots` returns an error. -3. I7: both CLIs follow systemd 255 verify-key handling; v262 parses per file - with different messages; the pinned v262 oracle is built without FSS, so - parity cannot be certified; the SDK key parser matches neither. -4. For a file without `_BOOT_ID` DATA, v262 `--list-boots` prints "No boot - found." while both rewrites list ENTRY header boot IDs. -5. On `main_entry_array_missing.journal` (verify negative corpus) v262 fails - to open the file, Go `--list-boots` prints "No boot found." and Rust lists - the boot. -6. Go directory retention never deletes `*.journal~` files; check against the - Rust writer and the systemd vacuum rule. -7. Rust `DirectoryReader::open_files` does not deduplicate repeated paths; - Go does (both CLIs now call `UniqueFiles` / `unique_files`). -8. Subdirectory read errors during directory discovery are handled - differently by Go and Rust. -9. The Go reader fails a whole entry on a payload without `=`; the Rust - reader keeps the entry. - -Unknowns: - -- Which side is right for items 4-9, against systemd v260.1/v262 sources. - -### Acceptance Criteria - -- Decisions 8 and 9 made by the user and applied in both languages, or - recorded as rejected. -- I7 done with an FSS-enabled v262 oracle, or rejected with evidence. -- Items 4-9: each checked against the owning oracle and fixed in both - languages with tests, or rejected with evidence. - -## Analysis - -Evidence is in SOW-0097 (Execution Log, F2 sweep, Implications And -Decisions). Refresh at activation. - -## Pre-Implementation Gate - -Status: needs-user-decision - -Problem / root-cause model: parallel Go/Rust and CLI/SDK implementations -drifted before single ownership was a project rule. - -Evidence reviewed: SOW-0097. - -Affected contracts and surfaces: Go and Rust reader, directory and export -APIs; both journalctl rewrites; Go directory retention. - -Existing patterns to reuse: SOW-0097 goldens (`tests/output/`), query and -verify matrices. - -Risk and blast radius: decisions 8 and 9 change public contracts; item 6 -changes retention (data deletion); item 9 changes what readers return. - -Sensitive data handling plan: synthetic fixtures only. - -Implementation plan: per item, test first, both languages. - -Validation plan: unit tests in both languages plus the query, directory and -verify matrices with the pinned oracles. - -Artifact impact plan: specs and API docs for any contract change. - -Open decisions: 8 and 9 above; whether to build an FSS-enabled v262 oracle -for I7. - -## Implications And Decisions - -Pending. - -## Plan - -Pending activation. - -## Execution Log - -None. - -## Validation - -Pending. - -## Outcome - -Pending. - -## Lessons Extracted - -None yet. - -## Followup - -None yet. - -## Regression Log - -None. diff --git a/.agents/sow/pending/SOW-0177-20261108-verify-sealed-range-report.md b/.agents/sow/pending/SOW-0177-20261108-verify-sealed-range-report.md new file mode 100644 index 00000000..4c4c1e3c --- /dev/null +++ b/.agents/sow/pending/SOW-0177-20261108-verify-sealed-range-report.md @@ -0,0 +1,159 @@ +# SOW-0177 - Report The Validated Sealed Range From Verification + +## Status + +Status: open + +`completed` is the successful terminal status. `done` is a directory name, not a status value. Do not use `Status: done` or `Status: complete`. + +Sub-state: tracked from SOW-0176 (item I7) on 2026-11-08; needs a public API decision. + +## Requirements + +### Purpose + +Make `journalctl --verify --verify-key` print the same validated-range summary +as systemd v262 for sealed files. + +### User Request + +Follow-up of the SOW-0176 verify-key parity decision (I7 = A). + +### Assistant Understanding + +Facts: + +- After `PASS`, v262 prints `=> Validated from %s to %s, final %s entries not + sealed.`, `=> No sealing yet, %s of entries not sealed.` or `=> No sealing + yet, no entries in file.` (systemd/systemd @ 8cc40e0c5e9234bf45084751ac53b1fbfe70b492 + `src/journal/journalctl-misc.c:70-85`). +- `VerifyFileWithKey` and `verify_file_with_key` return only success or failure. + Both already track the realtime of the last verified TAG. + +Inferences: + +- Exposing the range needs a new public result type in both SDKs, plus + `format_timespan` output in the CLIs. + +Unknowns: + +- The public API shape, decided at activation. + +### Acceptance Criteria + +- Both CLIs print the three v262 summary forms, compared against the pinned + OpenSSL-enabled v262 oracle. +- The range comes from one public SDK function per language. + +## Analysis + +Sources checked: SOW-0176 keydiff results and the files cited above. + +## Pre-Implementation Gate + +Status: blocked + +Problem / root-cause model: + +- The verifier does not return the range v262 reports. + +Evidence reviewed: + +- See `## Requirements`. + +Affected contracts and surfaces: + +- Go and Rust verification APIs, both journalctl rewrites. + +Existing patterns to reuse: + +- `verifySealed` / `verify_sealed` tag tracking. + +Risk and blast radius: + +- Additive API; stderr output only. + +Sensitive data handling plan: + +- Synthetic fixtures only. + +Implementation plan: + +1. Decide the API, add it in both languages, print the summary, compare with the oracle. + +Validation plan: + +- `tests/interoperability/run_verify_matrix.py` and a stderr comparison. + +Artifact impact plan: + +- AGENTS.md: none expected. +- Runtime project skills: none expected. +- Specs: journalctl parity matrix. +- End-user/operator docs: API pages. +- End-user/operator skills: none. +- SOW lifecycle: pending until activated. + +Open-source reference evidence: + +- Cited above. + +Open decisions: + +- Public API shape. + +## Implications And Decisions + +None yet. + +## Plan + +See the implementation plan above. + +## Implementation And Review Plan + +Implementation: + +- The project manager implements the approved API in both languages, with one + chunk for the SDK result and one for the CLI output. + +Reviewers: + +- Whole-SOW self-review after local validation; tests are the gate. External + reviewers are not used. + +Repository boundary block for every external-agent prompt: + +```text +CRITICAL REPOSITORY BOUNDARY: +- Do not make changes outside this repository for any reason. +- Repository path: current repository root. +- You may inspect external references read-only when the task requires it. +- Write, edit, delete, move, reset, checkout, install, generate, cache, or format nothing outside this repository. +- The only write exception outside the repository is /tmp. +- Prefer .local/ inside this repository for scratch work, generated temporary files, cloned references, logs, and working notes. +``` + +## Execution Log + +None yet. + +## Validation + +Pending. + +## Outcome + +Pending. + +## Lessons Extracted + +Pending. + +## Followup + +None yet. + +## Regression Log + +None. diff --git a/.agents/sow/specs/journalctl-v260-parity-matrix.md b/.agents/sow/specs/journalctl-v260-parity-matrix.md index 03409b38..c4df5be1 100644 --- a/.agents/sow/specs/journalctl-v260-parity-matrix.md +++ b/.agents/sow/specs/journalctl-v260-parity-matrix.md @@ -154,7 +154,7 @@ Every official v262 output mode is `file-backed-required`: | Option | Args | Classification | Required portable behavior | | --- | --- | --- | --- | -| `--verify-key=` | key | file-backed-required | Verify sealed journal files when combined with `--verify` or implied verification. Invalid seed/key text must fail with official-style parse errors. | +| `--verify-key=` | key | file-backed-required | Verify sealed journal files when combined with `--verify` or implied verification. Key text follows v262 `journal_auth_load_key` through the SDK parser, is judged per file, and a rejected key stops the run with v262's message. | | `--interval=` | duration | recognized-no-op | Parse for `--setup-keys`; no portable behavior exists without `--setup-keys`, so it is accepted as a no-op elsewhere. | | `--force` | none | recognized-no-op | Parse for `--setup-keys`; no portable behavior exists without `--setup-keys`, so it is accepted as a no-op elsewhere. | | `--setup-keys` | none | recognized-unsupported | Official v262 checks `/var/log/journal`, reads host machine and boot IDs, and writes `/var/log/journal//fss`; portable file-backed commands must reject it instead of mutating host journal state. Evidence: `systemd/systemd @ 8cc40e0c5e9234bf45084751ac53b1fbfe70b492`, `src/journal/journalctl-authenticate.c:80`, `src/journal/journalctl-authenticate.c:89`, `src/journal/journalctl-authenticate.c:93`, `src/journal/journalctl-authenticate.c:97`, `src/journal/journalctl-authenticate.c:139`. | diff --git a/.agents/sow/specs/product-scope.md b/.agents/sow/specs/product-scope.md index 9c18160e..0b21a34b 100644 --- a/.agents/sow/specs/product-scope.md +++ b/.agents/sow/specs/product-scope.md @@ -480,6 +480,9 @@ Root retention counts file lengths including preallocation, expires by tail saved time, and evicts whole files in tail/path order. Default retention policies and file format are unchanged; Go now applies cleanup after a successful successor-creation retry following a safe creation failure. +Ordinary `Log` retention in both languages counts disposed `*.journal~` files and +deletes them before archived history under file-count and byte limits, as +journald does; an age limit deletes any file older than the cutoff. `MaintainRootRetention` finalizes verified retired active files and only seals live actives for idle expiry or required size-policy allocation changes, leaving successors lazy even after eager startup. Startup, @@ -641,6 +644,10 @@ Current shared high-level directory writer API slice: leaves the file in place. Default chain naming treats only online files as active, so an unknown-state chain file stays behind as history while the new file still continues its sequence. +- Rust and Go non-root directory retention counts disposed `*.journal~` files + of the same source, both `@...` forms, and deletes them oldest first + and before archives, as journald's vacuum does. Max-age retention deletes any + expired file whatever its place in that order. - Rust and Go writers refuse entry timestamps with realtime outside `1..2^55-1` or monotonic at or above 2^55 before mutation (Go `ErrTimestampOutOfRange`, Rust `JournalError::TimestampOutOfRange`); the @@ -1079,11 +1086,12 @@ Current Go reader feature slice: - boot listing (`Reader.ListBoots`, `DirectoryReader.ListBoots`, `SdJournalListBoots`) reports every boot from the `_BOOT_ID` DATA entry arrays, merged across files in directory order and ordered by last entry, - first entry and ID. When no usable canonical index exists it scans entry - headers instead, so header IDs can name boots then; a directory scans only - when no file has such an index or one cannot be read. Only canonical - lowercase `_BOOT_ID` values name indexed boots. It does not change matches - or position. Both journalctl rewrites use it; + first entry and ID. A journal with no `_BOOT_ID` DATA lists no boots, as + v262 journalctl does; `HeaderFallback` (Rust `header_fallback`) scans entry + headers then, and for a directory only when no file has `_BOOT_ID` DATA. + Noncanonical `_BOOT_ID` DATA counts as present but names no boot. Read + failures return an error. It does not change matches or position. Both + journalctl rewrites use the default; - file-backed journalctl output modes cover the stock v262 family: `short`, `short-full`, `short-iso`, `short-iso-precise`, `short-precise`, `short-monotonic`, `short-delta`, `short-unix`, @@ -1229,9 +1237,8 @@ Current Rust reader feature slice: contract as the Go `EntryEncoder`; both journalctl rewrites render through it. `export_entry_bytes` and `json_entry` keep the older lossless contract; - boot listing (`FileReader::list_boots`, `DirectoryReader::list_boots`, - `SdJournalListBoots`) follows the Go boot-listing contract; - `DirectoryReader::list_boots` and `SdJournal::list_boots` cannot return an - error and fall back to one boot per file header when files cannot be read; + `SdJournalListBoots`) follows the Go boot-listing contract, including + returning an error when files cannot be read; - file-backed journalctl output modes cover the stock v262 family: `short`, `short-full`, `short-iso`, `short-iso-precise`, `short-precise`, `short-monotonic`, `short-delta`, `short-unix`, diff --git a/docs/Go-API.md b/docs/Go-API.md index 07fe2284..ecb99017 100644 --- a/docs/Go-API.md +++ b/docs/Go-API.md @@ -232,9 +232,11 @@ formats. `ExportEntry` and `JSONEntry` keep their older, lossless contract. ## List Boots `ListBoots` lists every boot as `journalctl --list-boots` does, from the -`_BOOT_ID` indexes when a usable canonical index exists, otherwise from entry -headers (a directory scans headers only when no file has such an index or one -cannot be read), without changing matches or the read position. +`_BOOT_ID` indexes, without changing matches or the read position. A journal +with no `_BOOT_ID` DATA lists no boots. Pass +`BootListOptions{HeaderFallback: true}` to `ListBootsWithOptions` to list the +boot IDs from entry headers instead; a directory does so only when no file has +`_BOOT_ID` DATA. ```go @@ -759,6 +761,8 @@ if err := journal.VerifyFile("/var/log/journal/example/system.journal"); err != } ``` +`ParseVerificationKey` validates a key with the same grammar as systemd v262. + Use `VerifyFileWithKey` for sealed files when a verification key is available. Verification is for integrity checks, not normal query serving. File-path verification uses the same bounded reader access architecture as diff --git a/docs/Reader-APIs.md b/docs/Reader-APIs.md index a42bf631..f687041d 100644 --- a/docs/Reader-APIs.md +++ b/docs/Reader-APIs.md @@ -108,6 +108,12 @@ Detection is lazy where the native access path permits it. A zero/`None` status means no loss has been observed; it does not certify integrity. A catalog query does not validate unrelated entry arrays. +A single unreadable DATA object, or one without a field separator, costs that +field only when reading the current entry: `GetEntry`/`get_entry` still return +the entry and the payload visitor and enumeration calls continue, and a payload +without `=` stays in the raw payload list. A DATA object that cannot be decompressed still fails the +entry. + Recovery is read-only: the SDK does not reconstruct or overwrite damaged source journals. Journald abandons a corrupt file and writes to a new one. Normal live publication can make temporarily unavailable data readable; this is distinct diff --git a/docs/Rust-API.md b/docs/Rust-API.md index a29acd9d..d7cb601a 100644 --- a/docs/Rust-API.md +++ b/docs/Rust-API.md @@ -217,24 +217,26 @@ contract. ## List Boots `list_boots` lists every boot as `journalctl --list-boots` does, from the -`_BOOT_ID` indexes when a usable canonical index exists, otherwise from entry -headers (a directory scans headers only when no file has such an index or one -cannot be read), without changing matches or the read position. +`_BOOT_ID` indexes, without changing matches or the read position. A journal +with no `_BOOT_ID` DATA lists no boots. Pass +`BootListOptions { header_fallback: true }` to `list_boots_with_options` to +list the boot IDs from entry headers instead; a directory does so only when no +file has `_BOOT_ID` DATA. ```rust use journal::DirectoryReader; let reader = DirectoryReader::open("/var/log/journal")?; -for boot in reader.list_boots() { +for boot in reader.list_boots()? { println!("{} {} {} {}", boot.index, boot.boot_id, boot.first_entry, boot.last_entry); } # Ok::<(), Box>(()) ``` -`FileReader::list_boots` and `SdJournalListBoots` return an error when the -file cannot be read; `DirectoryReader::list_boots` falls back to one boot per -file header instead. +`FileReader::list_boots`, `DirectoryReader::list_boots` and +`SdJournalListBoots` return an error when an open file cannot be read. Opening +a directory skips files that cannot be opened, so those are not reported here. ## Use Snapshot Bounds For Query Workloads @@ -659,6 +661,9 @@ verify_file("/var/log/journal/example/system.journal")?; # Ok::<(), Box>(()) ``` +`parse_verification_key()` validates a key with the same grammar as systemd +v262. + Use `verify_file_with_key()` for sealed files when a verification key is available. Verification is for integrity checks, not normal query serving. File-path verification uses the same bounded reader access architecture as diff --git a/docs/Writer-APIs.md b/docs/Writer-APIs.md index 33df13c5..f430bd70 100644 --- a/docs/Writer-APIs.md +++ b/docs/Writer-APIs.md @@ -215,7 +215,10 @@ Default behavior: - the active/current file counts toward file and byte envelopes; - the active/current file is never selected for deletion to satisfy retention; - retention is enforced when an active file is opened or created, and can also - be enforced explicitly. + be enforced explicitly; +- disposed `.journal~` files count toward the limits; file-count and byte + limits delete them before archived history, as journald does, while an age + limit deletes any file older than the cutoff. Use explicit size and duration limits for production backends. The SDK derives systemd-like file-size defaults from retention envelopes when supported by the diff --git a/go/API.md b/go/API.md index 6c4c352a..62ce0826 100644 --- a/go/API.md +++ b/go/API.md @@ -83,10 +83,13 @@ libsystemd/Netdata `jf` needed by Netdata readers: `Reader.ListBoots`, `DirectoryReader.ListBoots` and `SdJournalListBoots` list every boot, oldest first, as `journalctl --list-boots` does: each boot's first and last entries come from its `_BOOT_ID` DATA entry array (merged in directory -order across files). When no usable canonical index exists they scan entry -headers instead, and a directory does so only when no file has such an index -or one cannot be read. Only the canonical lowercase `_BOOT_ID` spelling names -an indexed boot. Listing boots does not change matches or the read position. +order across files). A journal with no `_BOOT_ID` DATA lists no boots, as v262 +`journalctl` does. `ListBootsWithOptions` and `SdJournalListBootsWithOptions` +with `BootListOptions{HeaderFallback: true}` list the boot IDs from entry +headers in that case; a directory does so only when no file has `_BOOT_ID` +DATA. Only the canonical lowercase `_BOOT_ID` spelling names a boot. Listing +boots returns an error when a file cannot be read, and does not change matches +or the read position. `SdJournalSeekCursor` accepts a syntactically valid cursor as a seek location even when no exact entry exists. Use `SdJournalTestCursor` to check whether the diff --git a/go/cmd/journalctl/main.go b/go/cmd/journalctl/main.go index dec3f5cd..097a8d38 100644 --- a/go/cmd/journalctl/main.go +++ b/go/cmd/journalctl/main.go @@ -24,6 +24,9 @@ import ( var errSilentNoData = errors.New("no matching entries") +// errVerificationKeyRejected is already reported on stderr in systemd's wording. +var errVerificationKeyRejected = errors.New("verification key rejected") + func noDataError(quiet bool, message string) error { if quiet { return errSilentNoData @@ -287,7 +290,7 @@ func (f *optionalStringFlag) IsBoolFlag() bool { func main() { if err := run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr); err != nil { - if !errors.Is(err, errSilentNoData) { + if !errors.Is(err, errSilentNoData) && !errors.Is(err, errVerificationKeyRejected) { fmt.Fprintf(os.Stderr, "Error: %v\n", err) } os.Exit(1) @@ -2947,10 +2950,6 @@ func (f *followOutput) syncVisitedCursor() error { } func runVerify(input cliInput, verifyKey string, hasVerifyKey, quiet bool, stdout, stderr io.Writer) error { - if err := validateVerificationKeyOption(verifyKey, hasVerifyKey, stderr); err != nil { - return err - } - files, directoryInput, err := input.journalFiles("verify") if err != nil { return err @@ -2965,6 +2964,9 @@ func runVerify(input cliInput, verifyKey string, hasVerifyKey, quiet bool, stdou var firstErr error for _, path := range files { if err := verifyOneFile(path, verifyKey, hasVerifyKey, directoryInput, quiet, stderr); err != nil { + if errors.Is(err, errVerificationKeyRejected) { + return err + } if firstErr == nil { firstErr = err } @@ -3734,12 +3736,14 @@ func formatJournalBytes(bytes uint64) string { return fmt.Sprintf("%dB", bytes) } -func validateVerificationKeyOption(verifyKey string, hasVerifyKey bool, stderr io.Writer) error { - if !hasVerifyKey || validVerificationKey(verifyKey) { - return nil +// loadVerificationKey runs per file, as systemd does, so a key is judged only +// once a journal file is reached. +func loadVerificationKey(key string, stderr io.Writer) error { + if _, err := journal.ParseVerificationKey(key); err != nil { + fmt.Fprintln(stderr, "Failed to load verification key: Key was rejected by service") + return errVerificationKeyRejected } - fmt.Fprintln(stderr, "Failed to parse seed.") - return errors.New("failed to parse seed") + return nil } func verifyOneFile(path, verifyKey string, hasVerifyKey, directoryInput, quiet bool, stderr io.Writer) error { @@ -3747,6 +3751,11 @@ func verifyOneFile(path, verifyKey string, hasVerifyKey, directoryInput, quiet b if err != nil { return reportVerifyOpenError(path, err, directoryInput, stderr) } + if hasVerifyKey { + if err := loadVerificationKey(verifyKey, stderr); err != nil { + return err + } + } if sealed && !hasVerifyKey { return reportVerifyMissingKey(path, stderr) } @@ -3820,63 +3829,6 @@ func flagWasSet(fs *flag.FlagSet, name string) bool { return wasSet } -func validVerificationKey(key string) bool { - seedEnd, ok := consumeVerificationSeed(key) - if !ok || seedEnd >= len(key) || key[seedEnd] != '/' { - return false - } - startEnd, ok := consumeHex(key, seedEnd+1) - if !ok || startEnd >= len(key) || key[startEnd] != '-' { - return false - } - end, ok := consumeHex(key, startEnd+1) - return ok && end == len(key) && hexRangeHasNonZero(key[startEnd+1:end]) -} - -func consumeVerificationSeed(key string) (int, bool) { - i := 0 - for c := 0; c < 12; c++ { - next, ok := consumeVerificationSeedByte(key, i) - if !ok { - return 0, false - } - i = next - } - return i, true -} - -func consumeVerificationSeedByte(key string, start int) (int, bool) { - i := start - for i < len(key) && key[i] == '-' { - i++ - } - if i+2 > len(key) || !isHex(key[i]) || !isHex(key[i+1]) { - return 0, false - } - return i + 2, true -} - -func hexRangeHasNonZero(s string) bool { - for _, b := range s { - if b != '0' { - return true - } - } - return false -} - -func consumeHex(s string, start int) (int, bool) { - i := start - for i < len(s) && isHex(s[i]) { - i++ - } - return i, i > start -} - -func isHex(b byte) bool { - return ('0' <= b && b <= '9') || ('a' <= b && b <= 'f') || ('A' <= b && b <= 'F') -} - // tableIndexWidth follows stock table formatting: IDX is as wide as the widest // index, and at least as wide as its header when the header is shown. func tableIndexWidth(indexes []int, quiet bool) int { diff --git a/go/cmd/journalctl/main_test.go b/go/cmd/journalctl/main_test.go index 343d7194..91ed1e8e 100644 --- a/go/cmd/journalctl/main_test.go +++ b/go/cmd/journalctl/main_test.go @@ -418,11 +418,18 @@ func TestRunVerifyKeyInvalidSeed(t *testing.T) { if stdout.Len() != 0 { t.Fatalf("expected no stdout, got: %q", stdout.String()) } - if !strings.Contains(stderr.String(), "Failed to parse seed.") { + if !strings.Contains(stderr.String(), "Failed to load verification key: Key was rejected by service") { t.Fatalf("expected parse seed error in stderr, got: %q", stderr.String()) } } +func TestRunVerifyKeyJudgedOnlyWhenAFileIsReached(t *testing.T) { + var stdout, stderr bytes.Buffer + if err := run([]string{"--verify-key=not-a-key", "--directory", t.TempDir()}, strings.NewReader(""), &stdout, &stderr); err != nil { + t.Fatalf("no file reached, so the key is not judged: %v (%s)", err, stderr.String()) + } +} + func TestRunVerifyKeyEmptySeed(t *testing.T) { path := writeCLIJournal(t, []cliEntry{{message: "verify-key-empty", priority: "6"}}) @@ -433,7 +440,7 @@ func TestRunVerifyKeyEmptySeed(t *testing.T) { if stdout.Len() != 0 { t.Fatalf("expected no stdout, got: %q", stdout.String()) } - if !strings.Contains(stderr.String(), "Failed to parse seed.") { + if !strings.Contains(stderr.String(), "Failed to load verification key: Key was rejected by service") { t.Fatalf("expected parse seed error in stderr, got: %q", stderr.String()) } } diff --git a/go/journal/boots.go b/go/journal/boots.go index 8ba6bd0a..1150a89f 100644 --- a/go/journal/boots.go +++ b/go/journal/boots.go @@ -11,25 +11,36 @@ type bootRange struct { first, last directoryEntryKey } +// BootListOptions selects how boots are listed. +type BootListOptions struct { + // HeaderFallback lists the boot IDs in ENTRY headers when the journal has + // no _BOOT_ID DATA at all, which the core writer can produce. journalctl + // --list-boots reports no boots for such a journal. + HeaderFallback bool +} + // bootRanges returns the first and last visible entries of every boot that -// has _BOOT_ID DATA in this file, from the DATA objects' entry postings. This -// is what a _BOOT_ID match with SeekHead/SeekTail finds, without decoding -// entries or touching the caller's matches and position. -func (r *Reader) bootRanges() ([]bootRange, error) { +// has _BOOT_ID DATA in this file, from the DATA objects' entry postings, and +// whether the file has any _BOOT_ID DATA. This is what a _BOOT_ID match with +// SeekHead/SeekTail finds, without decoding entries or touching the caller's +// matches and position. +func (r *Reader) bootRanges() ([]bootRange, bool, error) { // A long-lived reader must describe the file as it is now, like Next does. if _, err := r.refreshEntryOffsets(); err != nil { - return nil, err + return nil, false, err } if len(r.entryOffsets) == 0 { - return nil, nil + return nil, false, nil } lastVisible := r.entryOffsets[len(r.entryOffsets)-1] var ranges []bootRange + hasData := false err := r.visitFieldDataObjects([]byte("_BOOT_ID"), func(offset uint64, _ dataHeader, payload []byte) error { value, ok := bytes.CutPrefix(payload, []byte("_BOOT_ID=")) if !ok { return nil } + hasData = true bootID, ok := parseCanonicalBootID(value) if !ok { return nil @@ -63,7 +74,7 @@ func (r *Reader) bootRanges() ([]bootRange, error) { ranges = append(ranges, bootRange{bootID: bootID, first: first, last: last}) return nil }) - return ranges, err + return ranges, hasData, err } // parseCanonicalBootID accepts only the 32 lowercase hex digits journald @@ -126,8 +137,16 @@ func (r *Reader) scanBootTimes(boots map[UUID]*bootTimes) error { // ListBoots lists the boots in this file, oldest first, as journalctl // --list-boots does. It does not change matches or the read position. func (r *Reader) ListBoots() ([]BootInfo, error) { - ranges, err := r.bootRanges() - if err == nil && len(ranges) > 0 { + return r.ListBootsWithOptions(BootListOptions{}) +} + +// ListBootsWithOptions is ListBoots with the given options. +func (r *Reader) ListBootsWithOptions(options BootListOptions) ([]BootInfo, error) { + ranges, hasData, err := r.bootRanges() + if err != nil { + return nil, err + } + if hasData || !options.HeaderFallback { times := make([]bootTimes, 0, len(ranges)) for _, boot := range ranges { times = append(times, bootTimes{bootID: boot.bootID, first: boot.first.realtime, last: boot.last.realtime}) @@ -146,15 +165,21 @@ func (r *Reader) ListBoots() ([]BootInfo, error) { // a _BOOT_ID match reaches from the head and from the tail of the merged // directory order. It does not change matches or the read position. func (dr *DirectoryReader) ListBoots() ([]BootInfo, error) { + return dr.ListBootsWithOptions(BootListOptions{}) +} + +// ListBootsWithOptions is ListBoots with the given options. HeaderFallback +// applies only when no file in the directory has _BOOT_ID DATA. +func (dr *DirectoryReader) ListBootsWithOptions(options BootListOptions) ([]BootInfo, error) { merged := make(map[UUID]*bootRange) var order []UUID - indexed := true + anyData := false for _, r := range dr.files { - ranges, err := r.bootRanges() + ranges, hasData, err := r.bootRanges() if err != nil { - indexed = false - break + return nil, err } + anyData = anyData || hasData for _, boot := range ranges { current := merged[boot.bootID] if current == nil { @@ -171,7 +196,7 @@ func (dr *DirectoryReader) ListBoots() ([]BootInfo, error) { } } } - if indexed && len(merged) > 0 { + if anyData || !options.HeaderFallback { times := make([]bootTimes, 0, len(order)) for _, id := range order { boot := merged[id] diff --git a/go/journal/boots_test.go b/go/journal/boots_test.go index 5d578680..3d6d8ec7 100644 --- a/go/journal/boots_test.go +++ b/go/journal/boots_test.go @@ -106,13 +106,17 @@ func TestListBootsMultiBoot(t *testing.T) { } } -// Journals without _BOOT_ID DATA fall back to entry header boot IDs. +// Journals without _BOOT_ID DATA list no boots, as v262 journalctl does, +// unless the caller opts into the entry header boot IDs. func TestListBootsFallsBackToEntryHeaders(t *testing.T) { path := filepath.Join(t.TempDir(), "system.journal") writeBootTestJournal(t, path, 0x30, false, []bootTestEntry{{0xd0, 500, 1}, {0xd0, 510, 2}, {0xe0, 600, 1}}) r := mustOpenReaderFile(t, path) defer r.Close() - got, err := r.ListBoots() + if got, err := r.ListBoots(); err != nil || len(got) != 0 { + t.Fatalf("default ListBoots = %+v, %v; want none", got, err) + } + got, err := r.ListBootsWithOptions(BootListOptions{HeaderFallback: true}) if err != nil { t.Fatal(err) } @@ -143,6 +147,21 @@ func TestListBootsIgnoresNonCanonicalBootIDData(t *testing.T) { } } +// Noncanonical _BOOT_ID DATA still counts as present, so the fallback must not +// report boots a _BOOT_ID match would not find. +func TestListBootsFallbackIgnoresAllNonCanonicalData(t *testing.T) { + path := filepath.Join(t.TempDir(), "system.journal") + writeBootTestJournalData(t, path, 0x51, func(b byte) string { + return strings.ToUpper(bootTestID(b).String()) + }, []bootTestEntry{{0xbb, 200, 1}}) + r := mustOpenReaderFile(t, path) + defer r.Close() + got, err := r.ListBootsWithOptions(BootListOptions{HeaderFallback: true}) + if err != nil || len(got) != 0 { + t.Fatalf("ListBootsWithOptions = %+v, %v; want none", got, err) + } +} + // ListBoots leaves the caller's matches and read position alone. func TestListBootsKeepsMatchesAndPosition(t *testing.T) { path := filepath.Join(t.TempDir(), "system.journal") diff --git a/go/journal/facade.go b/go/journal/facade.go index 513290d0..4945c77b 100644 --- a/go/journal/facade.go +++ b/go/journal/facade.go @@ -617,10 +617,18 @@ func formatEntryText(entry *Entry) string { } func SdJournalListBoots(j *sdJournal) ([]BootInfo, error) { - if lister, ok := j.reader.(interface{ ListBoots() ([]BootInfo, error) }); ok { - return lister.ListBoots() + return SdJournalListBootsWithOptions(j, BootListOptions{}) +} + +// SdJournalListBootsWithOptions is SdJournalListBoots with the given options. +func SdJournalListBootsWithOptions(j *sdJournal, options BootListOptions) ([]BootInfo, error) { + lister, ok := j.reader.(interface { + ListBootsWithOptions(BootListOptions) ([]BootInfo, error) + }) + if !ok { + return nil, ErrUnsupported } - return nil, ErrUnsupported + return lister.ListBootsWithOptions(options) } func SdJournalQueryUniqueState(j *sdJournal, field string) error { diff --git a/go/journal/live_reader_test.go b/go/journal/live_reader_test.go index 11e0b277..38077504 100644 --- a/go/journal/live_reader_test.go +++ b/go/journal/live_reader_test.go @@ -220,7 +220,7 @@ func TestJournalctlListBoots(t *testing.T) { } defer dr.Close() - boots, err := dr.ListBoots() + boots, err := dr.ListBootsWithOptions(BootListOptions{HeaderFallback: true}) if err != nil { t.Fatalf("ListBoots error: %v", err) } diff --git a/go/journal/log.go b/go/journal/log.go index a5b18881..737d7c94 100644 --- a/go/journal/log.go +++ b/go/journal/log.go @@ -272,6 +272,7 @@ type archivedJournalFile struct { headSeqnum uint64 headRealtime uint64 size uint64 + disposed bool } type chainState struct { @@ -1323,6 +1324,40 @@ func parseArchivedJournalName(name, source string) (archivedJournalFile, bool) { return archivedJournalFile{headSeqnum: headSeqnum, headRealtime: headRealtime}, true } +// disposedMicrosLimit is systemd's realtime limit; earlier SDK releases wrote +// disposed-name timestamps in nanoseconds, which are all above it. +const disposedMicrosLimit = 1 << 55 + +// parseDisposedJournalName reads @-.journal~. For +// retention the timestamp stands in for head realtime and the number for head +// seqnum, as the Rust repository orders these files. +func parseDisposedJournalName(name, source string) (archivedJournalFile, bool) { + stem, ok := strings.CutSuffix(name, ".journal~") + if !ok { + return archivedJournalFile{}, false + } + at := strings.LastIndexByte(stem, '@') + if at < 0 || (stem[:at] != source && !strings.HasPrefix(stem[:at], source+"@")) { + return archivedJournalFile{}, false + } + stamp, number, ok := strings.Cut(stem[at+1:], "-") + if !ok { + return archivedJournalFile{}, false + } + realtime, err := strconv.ParseUint(stamp, 16, 64) + if err != nil { + return archivedJournalFile{}, false + } + seqnum, err := strconv.ParseUint(number, 16, 64) + if err != nil { + return archivedJournalFile{}, false + } + if realtime >= disposedMicrosLimit { + realtime /= 1000 + } + return archivedJournalFile{headSeqnum: seqnum, headRealtime: realtime, disposed: true}, true +} + func validateJournalSource(source string) error { if source == "" || source == "." || source == ".." { return errInvalidJournal diff --git a/go/journal/log_retention.go b/go/journal/log_retention.go index a841efe1..788d3619 100644 --- a/go/journal/log_retention.go +++ b/go/journal/log_retention.go @@ -72,7 +72,7 @@ func (l *Log) enforceRetention(protectedPath string) error { } func (l *Log) newRetentionRun(protectedPath string) (retentionRun, error) { - files, _, err := l.archivedFiles() + files, _, err := l.scanJournalFiles(true) if err != nil { return retentionRun{}, err } @@ -102,7 +102,11 @@ func (l *Log) newRetentionRun(protectedPath string) (retentionRun, error) { } } + // Disposed files go first: journald vacuums them before history. sort.Slice(files, func(i, j int) bool { + if files[i].disposed != files[j].disposed { + return files[i].disposed + } if files[i].headRealtime != files[j].headRealtime { return files[i].headRealtime < files[j].headRealtime } @@ -138,16 +142,12 @@ func (l *Log) enforceRetentionMaxAge(run *retentionRun) error { return nil } cutoff := retentionAgeCutoff(*l.retention.MaxAge) - for len(run.files) > 0 { - if run.files[0].headRealtime > cutoff { - break - } + for { ok, err := l.deleteOldestRetainedFile(run, &cutoff) if err != nil || !ok { return err } } - return nil } func retentionAgeCutoff(maxAge time.Duration) uint64 { @@ -162,8 +162,9 @@ func retentionAgeCutoff(maxAge time.Duration) uint64 { func (l *Log) deleteOldestRetainedFile(run *retentionRun, cutoff *uint64) (bool, error) { deleteIndex := -1 for i, file := range run.files { + // The order is not by age across disposed and archived files. if cutoff != nil && file.headRealtime > *cutoff { - break + continue } if run.activePath == "" || file.path != run.activePath { deleteIndex = i @@ -185,6 +186,10 @@ func (l *Log) deleteOldestRetainedFile(run *retentionRun, cutoff *uint64) (bool, } func (l *Log) archivedFiles() ([]archivedJournalFile, uint64, error) { + return l.scanJournalFiles(false) +} + +func (l *Log) scanJournalFiles(includeDisposed bool) ([]archivedJournalFile, uint64, error) { entries, err := os.ReadDir(l.machineDir) if err != nil { return nil, 0, err @@ -196,6 +201,9 @@ func (l *Log) archivedFiles() ([]archivedJournalFile, uint64, error) { continue } archived, ok := parseArchivedJournalName(entry.Name(), l.source) + if !ok && includeDisposed { + archived, ok = parseDisposedJournalName(entry.Name(), l.source) + } if !ok { continue } diff --git a/go/journal/log_retention_disposed_test.go b/go/journal/log_retention_disposed_test.go new file mode 100644 index 00000000..79a2d315 --- /dev/null +++ b/go/journal/log_retention_disposed_test.go @@ -0,0 +1,72 @@ +package journal + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" +) + +// Paired with Rust retention_counts_and_deletes_disposed_files_first. +func TestRetentionCountsAndDeletesDisposedFilesFirst(t *testing.T) { + root := t.TempDir() + log := mustNewLogForTest(t, root, LogConfig{ + Options: testOptions(), + Source: "system", + RotationPolicy: RotationPolicy{}.WithMaxEntries(1), + RetentionPolicy: RetentionPolicy{}.WithMaxFiles(2), + }, "retention") + defer closeLogForTest(t, log, "retention") + appendLogRange(t, log, "keep", "", 0, 2, 1_700_002_900_000_000) + dir := log.JournalDirectory() + + // Newer than every archive, to prove disposed files still go first. + stamp := uint64(time.Now().UnixMicro()) + chainName := fmt.Sprintf("system@%032x-%016x-%016x@%016x-%016x.journal~", 1, 1, 1, stamp, 2) + strictName := fmt.Sprintf("system@%016x-%016x.journal~", stamp, 1) + foreign := fmt.Sprintf("other@%016x-%016x.journal~", stamp, 1) + for _, name := range []string{chainName, strictName, foreign} { + if err := os.WriteFile(filepath.Join(dir, name), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + if err := log.EnforceRetention(); err != nil { + t.Fatal(err) + } + assertPathDoesNotExist(t, filepath.Join(dir, chainName), "chain disposed") + assertPathDoesNotExist(t, filepath.Join(dir, strictName), "strict disposed") + if _, err := os.Stat(filepath.Join(dir, foreign)); err != nil { + t.Fatalf("another source's file was touched: %v", err) + } + if _, err := os.Stat(log.ActivePath()); err != nil { + t.Fatalf("active file: %v", err) + } +} + +// A recent disposed file sorts first, and must not stop older archives aging out. +func TestRetentionMaxAgeSkipsRecentDisposedFile(t *testing.T) { + now := uint64(time.Now().UnixMicro()) + log := &Log{} + maxAge := time.Hour + log.retention.MaxAge = &maxAge + dir := t.TempDir() + recent := filepath.Join(dir, "recent") + old := filepath.Join(dir, "old") + for _, path := range []string{recent, old} { + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + run := retentionRun{files: []archivedJournalFile{ + {path: recent, headRealtime: now, disposed: true}, + {path: old, headRealtime: now - uint64(2*time.Hour/time.Microsecond)}, + }, fileCount: 2} + if err := log.enforceRetentionMaxAge(&run); err != nil { + t.Fatal(err) + } + assertPathDoesNotExist(t, old, "expired archive") + if _, err := os.Stat(recent); err != nil { + t.Fatalf("recent disposed file: %v", err) + } +} diff --git a/go/journal/netdata.go b/go/journal/netdata.go index 4f8ef1f4..277f8947 100644 --- a/go/journal/netdata.go +++ b/go/journal/netdata.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" "time" + "unicode/utf8" ) const ( @@ -3023,11 +3024,61 @@ func journalFileExactSourceName(path string) string { name = trimJournalSuffix(name) } if strings.HasPrefix(name, "remote-") { - return name + return lossyUTF8(name) } return "" } - return localNamespaceSourceName(path) + return lossyUTF8(localNamespaceSourceName(path)) +} + +// lossyUTF8 replaces each maximal invalid subsequence with U+FFFD, exactly as +// Rust's to_string_lossy does, so both languages expose the same source name. +func lossyUTF8(s string) string { + if utf8.ValidString(s) { + return s + } + var out strings.Builder + for len(s) > 0 { + r, size := utf8.DecodeRuneInString(s) + if r == utf8.RuneError && size == 1 { + out.WriteRune(utf8.RuneError) + s = s[invalidUTF8PrefixLen(s):] + continue + } + out.WriteString(s[:size]) + s = s[size:] + } + return out.String() +} + +// invalidUTF8PrefixLen returns how many bytes of an invalid sequence form a +// truncated but so far well-formed character. +func invalidUTF8PrefixLen(s string) int { + want, low, high := 0, byte(0x80), byte(0xBF) + switch lead := s[0]; { + case lead >= 0xC2 && lead <= 0xDF: + want = 2 + case lead >= 0xE0 && lead <= 0xEF: + want = 3 + if lead == 0xE0 { + low = 0xA0 + } else if lead == 0xED { + high = 0x9F + } + case lead >= 0xF0 && lead <= 0xF4: + want = 4 + if lead == 0xF0 { + low = 0x90 + } else if lead == 0xF4 { + high = 0x8F + } + } + n := 1 + for n < want && n < len(s) && s[n] >= low && s[n] <= high { + n++ + low, high = 0x80, 0xBF + } + return n } func trimJournalSuffix(name string) string { diff --git a/go/journal/netdata_names_test.go b/go/journal/netdata_names_test.go index 1aa13e15..3de107b5 100644 --- a/go/journal/netdata_names_test.go +++ b/go/journal/netdata_names_test.go @@ -33,7 +33,22 @@ func TestNetdataNonUTF8JournalNamesAreListedAndClassified(t *testing.T) { t.Fatalf("source type = %d, want %d", got, want) } remote := "/var/log/journal/remote/remote-h\xffst@x.journal" - if got := journalFileExactSourceName(remote); got != "remote-h\xffst" { + if got := journalFileExactSourceName(remote); got != "remote-h\uFFFDst" { t.Fatalf("exact source name = %q", got) } + for raw, want := range map[string]string{ + "a\xe2\x82b": "a\uFFFDb", // one truncated character + "a\xff\xfeb": "a\uFFFD\uFFFDb", // two invalid bytes + "a\xed\xa0b": "a\uFFFD\uFFFDb", // surrogate lead is not a prefix + "a\xf0\x9f\x98": "a\uFFFD", // truncated four-byte character + "a\xe2\x82\xacb": "a\u20ACb", // valid text is unchanged + } { + if got := lossyUTF8(raw); got != want { + t.Fatalf("lossyUTF8(%q) = %q, want %q", raw, got, want) + } + } + namespaced := filepath.Join("/var/log/journal", "machine.n\xffs", "system.journal") + if got := journalFileExactSourceName(namespaced); got != "namespace-n\uFFFDs" { + t.Fatalf("namespace source name = %q", got) + } } diff --git a/go/journal/reader_entry.go b/go/journal/reader_entry.go index 56de3425..e8c6fe2e 100644 --- a/go/journal/reader_entry.go +++ b/go/journal/reader_entry.go @@ -2,6 +2,7 @@ package journal import ( "bytes" + "errors" "fmt" "unicode/utf8" ) @@ -32,6 +33,9 @@ func (r *Reader) VisitEntryPayloads(visitor func([]byte) error) error { for _, dataOff := range offsets { payload, err := r.readDataPayloadTemp(dataOff) if err != nil { + if recoverableEntryDataError(err) { + continue + } return err } if err := visitor(payload); err != nil { @@ -120,17 +124,20 @@ func (r *Reader) EnumerateEntryPayload() ([]byte, bool, error) { return nil, false, err } } - if r.entryDataIndex >= len(r.entryDataOffsets) { - r.clearEntryDataState() - return nil, false, nil - } - dataOff := r.entryDataOffsets[r.entryDataIndex] - r.entryDataIndex++ - payload, err := r.readDataPayloadRow(dataOff) - if err != nil { - return nil, false, err + for r.entryDataIndex < len(r.entryDataOffsets) { + dataOff := r.entryDataOffsets[r.entryDataIndex] + r.entryDataIndex++ + payload, err := r.readDataPayloadRow(dataOff) + if err != nil { + if recoverableEntryDataError(err) { + continue + } + return nil, false, err + } + return payload, true, nil } - return payload, true, nil + r.clearEntryDataState() + return nil, false, nil } func (r *Reader) readEntryAt(offset uint64) (*Entry, error) { @@ -147,15 +154,18 @@ func (r *Reader) readEntryAt(offset uint64) (*Entry, error) { for _, dataOff := range entries { payload, err := r.readDataPayloadTemp(dataOff) if err != nil { + if recoverableEntryDataError(err) { + continue + } return nil, fmt.Errorf("read data object at offset %d for entry at offset %d: %w", dataOff, offset, err) } - nameBytes, value, ok := splitRawPayload(payload) - if !ok { - return nil, fmt.Errorf("%w: data object at offset %d has no field separator", errCorruptObject, dataOff) - } payloadCopy := cloneBytes(payload) payloads = append(payloads, payloadCopy) + nameBytes, value, ok := splitRawPayload(payload) + if !ok { + continue + } nameCopy := cloneBytes(nameBytes) valueCopy := cloneBytes(value) rawFields = append(rawFields, RawField{Name: nameCopy, Value: valueCopy}) @@ -207,6 +217,13 @@ func formatCursorFromDirectoryKey(key directoryEntryKey) string { key.xorHash) } +// recoverableEntryDataError reports a bad DATA object that costs one field, not +// the entry; sd_journal_enumerate_data skips those too (sd-journal.c). +func recoverableEntryDataError(err error) bool { + var bounds readerBoundsError + return errors.Is(err, errCorruptObject) || errors.As(err, &bounds) +} + func (r *Reader) readDataPayloadTemp(offset uint64) ([]byte, error) { headerBuf, err := r.readSlice(offset, objectHeaderSize) if err != nil { diff --git a/go/journal/reader_entry_damage_test.go b/go/journal/reader_entry_damage_test.go new file mode 100644 index 00000000..7ba93fa9 --- /dev/null +++ b/go/journal/reader_entry_damage_test.go @@ -0,0 +1,137 @@ +package journal + +import ( + "bytes" + "encoding/binary" + "os" + "path/filepath" + "testing" +) + +// Paired with Rust tests/entry_damage.rs. +const objectHeaderToPayload = 64 + +func TestDamagedDataObjectCostsTheFieldNotTheEntry(t *testing.T) { + cases := []struct { + name string + patch func([]byte, int) + payloadKept bool + }{ + {"no-separator", func(b []byte, at int) { b[at+6] = '_' }, true}, + {"wrong-type", func(b []byte, at int) { b[at-objectHeaderToPayload] = 3 }, false}, + {"oversized", func(b []byte, at int) { + binary.LittleEndian.PutUint64(b[at-objectHeaderToPayload+8:], 1<<40) + }, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "damage.journal") + w, err := Create(path, testOptions()) + if err != nil { + t.Fatal(err) + } + if err := w.AppendMapWithOptions(map[string]string{"MESSAGE": "kept", "BROKEN": "1"}, testEntryOptions(1)); err != nil { + t.Fatal(err) + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + data := readFileForTest(t, path) + at := bytes.Index(data, []byte("BROKEN=1")) + if at < 0 { + t.Fatal("payload not in file") + } + c.patch(data, at) + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatal(err) + } + + j, err := SdJournalOpenFiles([]string{path}, 0) + if err != nil { + t.Fatal(err) + } + defer j.Close() + if n, err := SdJournalNext(j); err != nil || n != 1 { + t.Fatalf("next = %d, %v", n, err) + } + entry, err := SdJournalGetEntry(j) + if err != nil { + t.Fatal(err) + } + if got := string(entry.Fields["MESSAGE"]); got != "kept" { + t.Fatalf("MESSAGE = %q", got) + } + if _, ok := entry.Fields["BROKEN"]; ok { + t.Fatal("damaged field was returned") + } + want := 1 + if c.payloadKept { + want = 2 + } + if len(entry.Payloads) != want { + t.Fatalf("payloads = %d, want %d", len(entry.Payloads), want) + } + assertDamagedPayloads := func(api string, got [][]byte) { + t.Helper() + kept := false + for _, payload := range got { + kept = kept || bytes.Equal(payload, []byte("MESSAGE=kept")) + if bytes.HasPrefix(payload, []byte("BROKEN=")) { + t.Fatalf("%s returned the damaged field: %q", api, got) + } + } + if len(got) != want || !kept { + t.Fatalf("%s payloads = %q", api, got) + } + } + assertDamagedPayloads("GetEntry", entry.Payloads) + + r, err := OpenFile(path) + if err != nil { + t.Fatal(err) + } + defer r.Close() + if ok, err := r.Step(); err != nil || !ok { + t.Fatalf("step = %v, %v", ok, err) + } + collected, err := r.CollectEntryPayloads() + if err != nil { + t.Fatal(err) + } + assertDamagedPayloads("CollectEntryPayloads", collected) + var enumerated [][]byte + for { + payload, ok, err := r.EnumerateEntryPayload() + if err != nil { + t.Fatal(err) + } + if !ok { + break + } + enumerated = append(enumerated, bytes.Clone(payload)) + } + assertDamagedPayloads("EnumerateEntryPayload", enumerated) + }) + } +} + +// Paired with Rust missing_main_entry_array_hides_every_entry. +func TestMissingMainEntryArrayHidesEveryEntry(t *testing.T) { + for _, filtered := range []bool{false, true} { + path := headerGateJournal(t, false) + patchHeader(t, path, 176, u64Bytes(0)) + j, err := SdJournalOpenFiles([]string{path}, 0) + if err != nil { + t.Fatal(err) + } + defer j.Close() + if filtered { + if err := SdJournalAddMatch(j, []byte("MESSAGE=gate")); err != nil { + t.Fatal(err) + } + } + if n, err := SdJournalNext(j); err != nil || n != 0 { + t.Fatalf("filtered=%v next = %d, %v", filtered, n, err) + } + } +} diff --git a/go/journal/reader_recovery_test.go b/go/journal/reader_recovery_test.go index 76ef41d7..0569ed2b 100644 --- a/go/journal/reader_recovery_test.go +++ b/go/journal/reader_recovery_test.go @@ -700,7 +700,7 @@ func TestRecoveryDirectoryRepairPreservesPendingMergeRows(t *testing.T) { if len(dr.RecoveryStatuses()) != 0 { t.Fatal("directory status stale") } - boots, err := dr.ListBoots() + boots, err := dr.ListBootsWithOptions(BootListOptions{HeaderFallback: true}) if err != nil || len(boots) != 1 || boots[0].LastEntry != 3000 { t.Fatalf("boots=%v %v", boots, err) } diff --git a/go/journal/verify.go b/go/journal/verify.go index 9019b8e3..e14a3ec6 100644 --- a/go/journal/verify.go +++ b/go/journal/verify.go @@ -6,7 +6,7 @@ import ( "encoding/binary" "fmt" "hash" - "strconv" + "math/bits" ) // VerificationError indicates a journal file failed structural integrity verification. @@ -123,104 +123,118 @@ func verifyFileWithKeyOptions(path string, verificationKey string, opts ReaderOp return verifyReaderStrict(r) } - seed, startEpoch, intervalUsec, err := parseVerificationKey(verificationKey) + key, err := ParseVerificationKey(verificationKey) if err != nil { return &VerificationError{Reason: fmt.Sprintf("journal verification failed: %v", err)} } - if err := verifySealed(source, header, seed, startEpoch, intervalUsec); err != nil { + if err := verifySealed(source, header, key.Seed, key.StartEpoch, key.IntervalUsec); err != nil { return err } return verifyReaderStrict(r) } -// parseVerificationKey parses a systemd-style verification key. -// Format: 12 seed bytes as hex pairs (optional dash separators), then /START_HEX-INTERVAL_HEX. -func parseVerificationKey(key string) (seed [12]byte, start, interval uint64, err error) { - seed, i, err := parseVerificationSeed(key) +// VerificationKey is a parsed Forward Secure Sealing verification key. +type VerificationKey struct { + Seed [12]byte + StartEpoch uint64 + IntervalUsec uint64 +} + +// ParseVerificationKey parses "/-" with the grammar of +// systemd v262 journal_auth_load_key, including its tolerance for content +// after the interval. +func ParseVerificationKey(key string) (VerificationKey, error) { + var parsed VerificationKey + i, err := parseVerificationSeed(key, &parsed.Seed) if err != nil { - return seed, 0, 0, err + return VerificationKey{}, err } if i >= len(key) || key[i] != '/' { - return seed, 0, 0, fmt.Errorf("invalid verification key: missing / separator") + return VerificationKey{}, fmt.Errorf("invalid verification key: missing / separator") + } + start, next, ok := parseVerificationNumber(key, i+1) + if !ok || next >= len(key) || key[next] != '-' { + return VerificationKey{}, fmt.Errorf("invalid verification key: bad start hex") + } + interval, _, ok := parseVerificationNumber(key, next+1) + if !ok { + return VerificationKey{}, fmt.Errorf("invalid verification key: bad interval hex") + } + if start == 0 || interval == 0 { + return VerificationKey{}, fmt.Errorf("invalid verification key: zero start or interval") } - start, interval, err = parseVerificationRange(key, i+1) - return seed, start, interval, err + if high, _ := bits.Mul64(start, interval); high != 0 { + return VerificationKey{}, fmt.Errorf("invalid verification key: start time overflows") + } + parsed.StartEpoch = start + parsed.IntervalUsec = interval + return parsed, nil } -func parseVerificationSeed(key string) ([12]byte, int, error) { - var seed [12]byte +func parseVerificationSeed(key string, seed *[12]byte) (int, error) { i := 0 for c := range seed { - next, b, err := parseVerificationSeedByte(key, i) - if err != nil { - return seed, 0, err + for i < len(key) && key[i] == '-' { + i++ } - seed[c] = b - i = next + if i+2 > len(key) || !isHexDigit(key[i]) || !isHexDigit(key[i+1]) { + return 0, fmt.Errorf("invalid verification key: bad seed hex") + } + seed[c] = hexDigitValue(key[i])<<4 | hexDigitValue(key[i+1]) + i += 2 } - return seed, i, nil + return i, nil } -func parseVerificationSeedByte(key string, start int) (int, byte, error) { +// parseVerificationNumber follows sscanf's %lx, which systemd uses here: +// leading whitespace, a sign, a 0x prefix, and saturation on overflow. +func parseVerificationNumber(key string, start int) (uint64, int, bool) { i := start - for i < len(key) && key[i] == '-' { + for i < len(key) && (key[i] == ' ' || ('\t' <= key[i] && key[i] <= '\r')) { i++ } - if i+2 > len(key) { - return 0, 0, fmt.Errorf("invalid verification key: seed too short") + negative := false + if i < len(key) && (key[i] == '+' || key[i] == '-') { + negative = key[i] == '-' + i++ } - b, err := strconv.ParseUint(key[i:i+2], 16, 8) - if err != nil { - return 0, 0, fmt.Errorf("invalid verification key: bad seed hex") + if i+1 < len(key) && key[i] == '0' && (key[i+1] == 'x' || key[i+1] == 'X') { + i += 2 } - return i + 2, byte(b), nil -} - -func parseVerificationRange(key string, startOffset int) (uint64, uint64, error) { - start, next, err := parseVerificationHexValue(key, startOffset, "start") - if err != nil { - return 0, 0, err + digits := i + var value uint64 + overflow := false + for i < len(key) && isHexDigit(key[i]) { + overflow = overflow || value>>60 != 0 + value = value<<4 | uint64(hexDigitValue(key[i])) + i++ } - if next >= len(key) || key[next] != '-' { - return 0, 0, fmt.Errorf("invalid verification key: bad start hex") + if i == digits { + return 0, 0, false } - - interval, next, err := parseVerificationHexValue(key, next+1, "interval") - if err != nil { - return 0, 0, err - } - if next != len(key) { - return 0, 0, fmt.Errorf("invalid verification key: trailing data") + if overflow { + return ^uint64(0), i, true } - if interval == 0 { - return 0, 0, fmt.Errorf("invalid verification key: zero interval") + if negative { + value = -value } - return start, interval, nil + return value, i, true } -func parseVerificationHexValue(key string, offset int, name string) (uint64, int, error) { - next, ok := consumeVerificationHex(key, offset) - if !ok { - return 0, 0, fmt.Errorf("invalid verification key: bad %s hex", name) - } - value, err := strconv.ParseUint(key[offset:next], 16, 64) - if err != nil { - return 0, 0, fmt.Errorf("invalid verification key: bad %s hex", name) - } - return value, next, nil +func isHexDigit(b byte) bool { + return ('0' <= b && b <= '9') || ('a' <= b && b <= 'f') || ('A' <= b && b <= 'F') } -func consumeVerificationHex(s string, start int) (int, bool) { - i := start - for i < len(s) && isHexDigit(s[i]) { - i++ +func hexDigitValue(b byte) byte { + switch { + case b <= '9': + return b - '0' + case b <= 'F': + return b - 'A' + 10 + default: + return b - 'a' + 10 } - return i, i > start -} - -func isHexDigit(b byte) bool { - return ('0' <= b && b <= '9') || ('a' <= b && b <= 'f') || ('A' <= b && b <= 'F') } type sealedVerifyObject struct { diff --git a/go/journal/verify_key_test.go b/go/journal/verify_key_test.go new file mode 100644 index 00000000..acb9d649 --- /dev/null +++ b/go/journal/verify_key_test.go @@ -0,0 +1,56 @@ +package journal + +import "testing" + +const verifyKeySeed = "0123456789abcdef01234567" + +// Accepted and rejected shapes were checked against the systemd v262 CLI. +func TestParseVerificationKeyFollowsSystemdGrammar(t *testing.T) { + const maxUint64 = ^uint64(0) + for _, tc := range []struct { + name string + key string + wantStart uint64 + wantInterval uint64 + wantErr bool + }{ + {name: "plain", key: verifyKeySeed + "/1-f4240", wantStart: 1, wantInterval: 0xf4240}, + {name: "dashed seed", key: "-012345-6789ab--cdef01234567/1-2", wantStart: 1, wantInterval: 2}, + {name: "trailing content", key: verifyKeySeed + "/1-2x", wantStart: 1, wantInterval: 2}, + {name: "spaces", key: verifyKeySeed + "/ 1- 2", wantStart: 1, wantInterval: 2}, + {name: "signs", key: verifyKeySeed + "/+1-+2", wantStart: 1, wantInterval: 2}, + {name: "prefix", key: verifyKeySeed + "/0x1-0X2", wantStart: 1, wantInterval: 2}, + {name: "negative wraps", key: verifyKeySeed + "/1--1", wantStart: 1, wantInterval: maxUint64}, + {name: "saturates", key: verifyKeySeed + "/1-1ffffffffffffffff", wantStart: 1, wantInterval: maxUint64}, + {name: "empty", key: "", wantErr: true}, + {name: "signed seed", key: "+a" + verifyKeySeed[2:] + "/1-2", wantErr: true}, + {name: "long seed", key: verifyKeySeed + "ab/1-2", wantErr: true}, + {name: "no slash", key: verifyKeySeed, wantErr: true}, + {name: "no interval", key: verifyKeySeed + "/1", wantErr: true}, + {name: "space before dash", key: verifyKeySeed + "/1 -2", wantErr: true}, + {name: "zero start", key: verifyKeySeed + "/0-2", wantErr: true}, + {name: "zero interval", key: verifyKeySeed + "/1-0", wantErr: true}, + {name: "prefix only", key: verifyKeySeed + "/0x-2", wantErr: true}, + {name: "start time overflow", key: verifyKeySeed + "/ffffffffffffffff-2", wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := ParseVerificationKey(tc.key) + if tc.wantErr { + if err == nil { + t.Fatalf("expected rejection, got %+v", got) + } + return + } + if err != nil { + t.Fatal(err) + } + if got.StartEpoch != tc.wantStart || got.IntervalUsec != tc.wantInterval { + t.Fatalf("got start %#x interval %#x", got.StartEpoch, got.IntervalUsec) + } + want := [12]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67} + if got.Seed != want { + t.Fatalf("got seed %x", got.Seed) + } + }) + } +} diff --git a/rust/README.md b/rust/README.md index ed82e507..c3ca08f7 100644 --- a/rust/README.md +++ b/rust/README.md @@ -261,12 +261,11 @@ Reader limitations: - `FileReader::list_boots`, `DirectoryReader::list_boots` and `SdJournalListBoots` list every boot from the `_BOOT_ID` DATA entry arrays, - as `journalctl --list-boots` does, falling back to an entry-header scan when - no usable canonical index exists (for a directory, only when no file has one - or one cannot be read). `DirectoryReader::list_boots` and - `SdJournal::list_boots` cannot return an error, so when the files cannot be - read they report one boot per file header; `SdJournalListBoots` and - `FileReader::list_boots` return the error; + as `journalctl --list-boots` does. A journal with no `_BOOT_ID` DATA lists no + boots unless the caller passes `BootListOptions { header_fallback: true }` to + the `*_with_options` forms (for a directory, it applies only when no file has + `_BOOT_ID` DATA). All of them return an error when the files cannot be read, + as Go does; - full systemd object-graph verification parity is tracked separately; - daemon-only journalctl operations are not implemented. diff --git a/rust/src/cmd/journalctl/main.rs b/rust/src/cmd/journalctl/main.rs index 0e097eb0..f8d1dabf 100644 --- a/rust/src/cmd/journalctl/main.rs +++ b/rust/src/cmd/journalctl/main.rs @@ -24,7 +24,7 @@ use journal::{ SdJournalListBoots, SdJournalNext, SdJournalOpenDirectoryPath, SdJournalOpenFilePaths, SdJournalPrevious, SdJournalSeekCursorDirection, SdJournalSeekHead, SdJournalSeekRealtimeUsec, SdJournalSeekTail, SdJournalTestCursor, SdJournalVisitUniqueValues, parse_match_bytes, - parse_match_string, verify_file, verify_file_with_key, + parse_match_string, parse_verification_key, verify_file, verify_file_with_key, }; use output::{OutputOptions, OutputRenderer}; use regex::Regex; @@ -440,7 +440,7 @@ enum OutputModeArg { fn main() { if let Err(err) = run() { - if !err.is::() { + if !err.is::() && !err.is::() { eprintln!("Error: {err}"); } exit(1); @@ -1242,11 +1242,6 @@ fn run_verify(path: &Path, verify_key: Option<&str>) -> Result<()> { } fn run_verify_input(input: &CliInput, verify_key: Option<&str>, quiet: bool) -> Result<()> { - if verify_key.is_some_and(|key| !valid_verification_key(key)) { - eprintln!("Failed to parse seed."); - return Err(anyhow!("failed to parse seed")); - } - let (files, directory_input) = input.journal_files("verify")?; if files.is_empty() { @@ -1272,6 +1267,14 @@ fn run_verify_input(input: &CliInput, verify_key: Option<&str>, quiet: bool) -> } }; + // Judged per file, as systemd does, and fatal for the whole run. + if let Some(key) = verify_key { + if parse_verification_key(key).is_err() { + eprintln!("Failed to load verification key: Key was rejected by service"); + return Err(VerificationKeyRejected.into()); + } + } + if sealed && verify_key.is_some() { match verify_file_with_key(file, verify_key.unwrap()) { Ok(()) => { @@ -2133,43 +2136,6 @@ fn format_journal_bytes(bytes: u64) -> String { format!("{bytes}B") } -fn valid_verification_key(key: &str) -> bool { - let bytes = key.as_bytes(); - let mut i = 0usize; - for _ in 0..12 { - while i < bytes.len() && bytes[i] == b'-' { - i += 1; - } - if i + 2 > bytes.len() || !is_hex(bytes[i]) || !is_hex(bytes[i + 1]) { - return false; - } - i += 2; - } - if i >= bytes.len() || bytes[i] != b'/' { - return false; - } - i += 1; - - let (next, ok) = consume_hex(bytes, i); - if !ok || next >= bytes.len() || bytes[next] != b'-' { - return false; - } - let (end, ok) = consume_hex(bytes, next + 1); - ok && end == bytes.len() && bytes[next + 1..end].iter().any(|b| *b != b'0') -} - -fn consume_hex(bytes: &[u8], start: usize) -> (usize, bool) { - let mut i = start; - while i < bytes.len() && is_hex(bytes[i]) { - i += 1; - } - (i, i > start) -} - -fn is_hex(b: u8) -> bool { - b.is_ascii_hexdigit() -} - fn apply_matches(journal: &mut SdJournal, matches: &[OsString]) -> Result<()> { for item in matches { if item.as_encoded_bytes() == b"+" { @@ -4144,7 +4110,7 @@ mod tests { let path = repo_root().join("fixtures/systemd/test-data/no-rtc/system.journal.zst"); let err = run_verify(&path, Some("synthetic-test-key")).expect_err("verify should fail"); assert!( - err.to_string().contains("failed to parse seed"), + err.to_string().contains("verification key rejected"), "expected parse seed error, got: {err}" ); } @@ -4154,7 +4120,7 @@ mod tests { let path = repo_root().join("fixtures/systemd/test-data/no-rtc/system.journal.zst"); let err = run_verify(&path, Some("")).expect_err("verify should fail"); assert!( - err.to_string().contains("failed to parse seed"), + err.to_string().contains("verification key rejected"), "expected parse seed error, got: {err}" ); } @@ -4177,6 +4143,13 @@ mod tests { run_verify(dir.path(), None).expect("empty directory verification should pass"); } + #[test] + fn verify_key_is_judged_only_when_a_file_is_reached() { + let dir = tempfile::tempdir().expect("create temp dir"); + run_verify(dir.path(), Some("not-a-key")) + .expect("no file reached, so the key is not judged"); + } + fn test_uuid(n: u8) -> uuid::Uuid { let mut bytes = [0u8; 16]; bytes[15] = n; @@ -4660,6 +4633,15 @@ mod tests { } } +#[derive(Debug)] +struct VerificationKeyRejected; +impl std::fmt::Display for VerificationKeyRejected { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "verification key rejected") + } +} +impl std::error::Error for VerificationKeyRejected {} + #[derive(Debug)] struct SilentNoData; impl std::fmt::Display for SilentNoData { diff --git a/rust/src/crates/journal-core/src/file/reader_recovery.rs b/rust/src/crates/journal-core/src/file/reader_recovery.rs index 1e607bb1..5ff830e1 100644 --- a/rust/src/crates/journal-core/src/file/reader_recovery.rs +++ b/rust/src/crates/journal-core/src/file/reader_recovery.rs @@ -388,6 +388,33 @@ impl JournalFile { .transpose() } + /// The last entry a reader may see: the recovered prefix's end, else the + /// header's tail entry, else the end of the global entry array. + #[doc(hidden)] + pub fn reader_last_visible_entry(&self) -> Result> { + if let Some(prefix) = self.recovery.get() { + return Ok(prefix.last_entry); + } + let header = self.journal_header_ref(); + if header.n_entries == 0 { + return Ok(None); + } + let tail = (header.header_size >= 272) + .then(|| NonZeroU64::new(header.tail_entry_offset)) + .flatten(); + // A writer may publish the tail hint before n_entries, so only a file + // no writer holds can use the hint without walking the array. + if header.state != JournalState::Online as u8 && tail.is_some() { + return Ok(tail); + } + let mut offsets = Vec::new(); + self.entry_offsets(&mut offsets)?; + Ok(match (tail, offsets.last().copied()) { + (Some(tail), Some(last)) => Some(tail.min(last)), + (_, last) => last, + }) + } + #[doc(hidden)] pub fn recover_reader_tail(&self, err: &JournalError) -> Result { if !self.recovery_enabled diff --git a/rust/src/crates/journal-log-writer/src/log/chain.rs b/rust/src/crates/journal-log-writer/src/log/chain.rs index d21bf826..f8a5e83b 100644 --- a/rust/src/crates/journal-log-writer/src/log/chain.rs +++ b/rust/src/crates/journal-log-writer/src/log/chain.rs @@ -115,6 +115,16 @@ fn tail_state_from_raw_header(file: &repository::File) -> Option { }) } +/// A disposed chain-named file keeps `@...` in its stem, which the +/// repository parses as an unknown source. Retention must still bound it. +fn is_disposed_chain_file(file: &repository::File, source_name: &str) -> bool { + file.is_disposed() + && Path::new(file.path()) + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.starts_with(&format!("{source_name}@"))) +} + impl OwnedChain { pub(super) fn new(path: PathBuf, machine_id: Uuid, source: Source) -> Result { #[cfg(debug_assertions)] @@ -147,7 +157,9 @@ impl OwnedChain { let Some(file) = repository::File::from_path(&file_path) else { continue; }; - if file.origin().source != chain.source { + if file.origin().source != chain.source + && !is_disposed_chain_file(&file, &chain.source_name) + { continue; } @@ -797,6 +809,51 @@ fn sync_directory(_path: &Path) -> std::io::Result<()> { mod tests { use super::*; + // Paired with Go TestRetentionCountsAndDeletesDisposedFilesFirst. + #[test] + fn retention_counts_and_deletes_disposed_files_first() { + use std::fs; + + let tmp = tempfile::tempdir().expect("create temp dir"); + let machine_id = Uuid::new_v4(); + let path = tmp.path().join(machine_id.to_string()); + fs::create_dir(&path).expect("create machine-id dir"); + let archived = create_chain_file(&path, "system", machine_id, 1, 1).expect("chain file"); + fs::write(archived.path(), b"journal").expect("write archive"); + let stamp = 1_700_000_000_000_000_u64; + let chain_name = format!( + "system@{:032x}-{:016x}-{:016x}@{stamp:016x}-{:016x}.journal~", + 1, 1, 1, 2 + ); + let strict_name = format!("system@{stamp:016x}-{:016x}.journal~", 1); + let foreign = format!("other@{stamp:016x}-{:016x}.journal~", 1); + for name in [&chain_name, &strict_name, &foreign] { + fs::write(path.join(name), b"x").expect("write disposed"); + } + + let mut chain = + OwnedChain::new(path.clone(), machine_id, Source::System).expect("create chain"); + assert_eq!(chain.inner.len(), 3, "foreign source is not counted"); + assert!( + chain + .delete_oldest_file(None) + .expect("delete") + .unwrap() + .is_disposed() + ); + assert!( + chain + .delete_oldest_file(None) + .expect("delete") + .unwrap() + .is_disposed() + ); + assert!(!path.join(&chain_name).exists()); + assert!(!path.join(&strict_name).exists()); + assert!(path.join(&foreign).exists()); + assert!(Path::new(archived.path()).exists()); + } + #[cfg(unix)] #[test] fn delete_oldest_file_preserves_accounting_on_remove_error() { diff --git a/rust/src/journal/src/boots.rs b/rust/src/journal/src/boots.rs index afd9bfc0..fb78e1cd 100644 --- a/rust/src/journal/src/boots.rs +++ b/rust/src/journal/src/boots.rs @@ -35,9 +35,13 @@ impl FileReader { /// Lists the boots in this file, oldest first, as `journalctl /// --list-boots` does. It does not change matches or the read position. pub fn list_boots(&self) -> Result> { - if let Ok(ranges) = self.boot_ranges() - && !ranges.is_empty() - { + self.list_boots_with_options(BootListOptions::default()) + } + + /// [`Self::list_boots`] with the given options. + pub fn list_boots_with_options(&self, options: BootListOptions) -> Result> { + let (ranges, has_data) = self.boot_ranges()?; + if has_data || !options.header_fallback { return Ok(order_boots(ranges.iter().map(BootRange::times).collect())); } let mut boots = HashMap::new(); @@ -46,10 +50,11 @@ impl FileReader { } /// First and last entries of every boot that has `_BOOT_ID` DATA in this - /// file, from the DATA objects' entry arrays. This is what a `_BOOT_ID` - /// match with seek_head/seek_tail finds, without decoding entries or - /// touching the caller's matches and position. - pub(crate) fn boot_ranges(&self) -> Result> { + /// file, from the DATA objects' entry arrays, and whether the file has any + /// `_BOOT_ID` DATA. This is what a `_BOOT_ID` match with + /// seek_head/seek_tail finds, without decoding entries or touching the + /// caller's matches and position. + pub(crate) fn boot_ranges(&self) -> Result<(Vec, bool)> { let seqnum_id = self.header_snapshot.header.seqnum_id; self.inner .with_file(|file| with_reader_recovery(file, || file_boot_ranges(file, seqnum_id))) @@ -82,9 +87,13 @@ fn with_reader_recovery( } } -fn file_boot_ranges(file: &JournalFile, seqnum_id: [u8; 16]) -> CoreResult> { +fn file_boot_ranges( + file: &JournalFile, + seqnum_id: [u8; 16], +) -> CoreResult<(Vec, bool)> { let mut data_offsets = Vec::new(); let mut decompressed = Vec::new(); + let mut has_data = false; for item in file.field_data_objects_with_offsets(b"_BOOT_ID")? { let (offset, data) = item?; let payload = if data.is_compressed() { @@ -94,9 +103,9 @@ fn file_boot_ranges(file: &JournalFile, seqnum_id: [u8; 16]) -> CoreResult } else { data.raw_payload() }; - let boot_id = payload - .strip_prefix(b"_BOOT_ID=") - .and_then(parse_canonical_boot_id); + let value = payload.strip_prefix(b"_BOOT_ID="); + has_data |= value.is_some(); + let boot_id = value.and_then(parse_canonical_boot_id); drop(data); if let Some(boot_id) = boot_id { data_offsets.push((offset, boot_id)); @@ -114,6 +123,10 @@ fn file_boot_ranges(file: &JournalFile, seqnum_id: [u8; 16]) -> CoreResult xor_hash: entry.header.xor_hash, }) }; + // Postings may reach past the visible tail of a dirty or growing journal. + let Some(last_visible) = file.reader_last_visible_entry()? else { + return Ok((Vec::new(), has_data)); + }; let mut ranges = Vec::with_capacity(data_offsets.len()); for (offset, boot_id) in data_offsets { let first = file.data_object_directed_partition_point( @@ -123,10 +136,12 @@ fn file_boot_ranges(file: &JournalFile, seqnum_id: [u8; 16]) -> CoreResult )?; let last = file.data_object_directed_partition_point( offset, - |_| Ok(true), + |candidate| Ok(candidate <= last_visible), ArrayDirection::Backward, )?; - if let (Some(first), Some(last)) = (first, last) { + if let (Some(first), Some(last)) = (first, last) + && first <= last_visible + { ranges.push(BootRange { boot_id, first: key_at(first)?, @@ -134,7 +149,7 @@ fn file_boot_ranges(file: &JournalFile, seqnum_id: [u8; 16]) -> CoreResult }); } } - Ok(ranges) + Ok((ranges, has_data)) } fn file_boot_times(file: &JournalFile) -> CoreResult> { @@ -158,7 +173,7 @@ fn file_boot_times(file: &JournalFile) -> CoreResult, times: BootTimes) { +fn merge_boot_times(boots: &mut HashMap<[u8; 16], BootTimes>, times: BootTimes) { boots .entry(times.boot_id) .and_modify(|boot| { diff --git a/rust/src/journal/src/directory.rs b/rust/src/journal/src/directory.rs index 40291cda..635d8770 100644 --- a/rust/src/journal/src/directory.rs +++ b/rust/src/journal/src/directory.rs @@ -1,5 +1,5 @@ use super::*; -use crate::boots::{BootRange, BootTimes, merge_boot_times, order_boots}; +use crate::boots::{BootRange, order_boots}; use std::collections::{HashMap, HashSet, VecDeque}; const DIRECTORY_UNIQUE_CACHE_CAPACITY: usize = 8; @@ -142,16 +142,19 @@ impl DirectoryReader { I: IntoIterator, P: AsRef, { + let paths: Vec = paths + .into_iter() + .map(|p| p.as_ref().to_path_buf()) + .collect(); let mut files = Vec::new(); - for path in paths { - let path = path.as_ref(); - if !path.is_file() || !is_journal_file_name(path) { + for path in unique_files(&paths) { + if !path.is_file() || !is_journal_file_name(&path) { return Err(SdkError::InvalidPath(format!( "not a journal file: {}", path.display() ))); } - files.push(FileReader::open_with_options(path, options)?); + files.push(FileReader::open_with_options(&path, options)?); } Self::from_readers(files, false) @@ -882,21 +885,19 @@ impl DirectoryReader { /// `journalctl --list-boots` does. A boot's first and last entries are the /// ones a `_BOOT_ID` match reaches from the head and from the tail of the /// merged directory order. It does not change matches or the read - /// position. If the files cannot be read it falls back to one boot per - /// file header. - pub fn list_boots(&self) -> Vec { - self.try_list_boots() - .unwrap_or_else(|_| self.list_header_boots()) + /// position. + pub fn list_boots(&self) -> Result> { + self.list_boots_with_options(BootListOptions::default()) } - pub(crate) fn try_list_boots(&self) -> Result> { + /// [`Self::list_boots`] with the given options. `header_fallback` applies + /// only when no file in the directory has `_BOOT_ID` DATA. + pub fn list_boots_with_options(&self, options: BootListOptions) -> Result> { let mut merged: HashMap<[u8; 16], BootRange> = HashMap::new(); - let mut indexed = true; + let mut any_data = false; for reader in &self.files { - let Ok(ranges) = reader.boot_ranges() else { - indexed = false; - break; - }; + let (ranges, has_data) = reader.boot_ranges()?; + any_data |= has_data; for boot in ranges { merged .entry(boot.boot_id) @@ -911,7 +912,7 @@ impl DirectoryReader { .or_insert(boot); } } - if indexed && !merged.is_empty() { + if any_data || !options.header_fallback { return Ok(order_boots(merged.values().map(BootRange::times).collect())); } let mut boots = HashMap::new(); @@ -921,27 +922,6 @@ impl DirectoryReader { Ok(order_boots(boots.into_values().collect())) } - /// One boot per file header: the pre-index summary, kept only as the - /// fallback of the infallible list_boots. - fn list_header_boots(&self) -> Vec { - let mut boots: HashMap<[u8; 16], BootTimes> = HashMap::new(); - for reader in &self.files { - let header = reader.effective_header(); - if reader.recovery_status().incomplete() && header.n_entries == 0 { - continue; - } - merge_boot_times( - &mut boots, - BootTimes { - boot_id: header.tail_entry_boot_id, - first: header.head_entry_realtime, - last: header.tail_entry_realtime, - }, - ); - } - order_boots(boots.into_values().collect()) - } - pub fn add_match(&mut self, data: &[u8]) { self.invalidate_filter_candidates(); for reader in &mut self.files { diff --git a/rust/src/journal/src/facade.rs b/rust/src/journal/src/facade.rs index 64b7f31d..848ab2cb 100644 --- a/rust/src/journal/src/facade.rs +++ b/rust/src/journal/src/facade.rs @@ -1,8 +1,8 @@ #![allow(non_snake_case)] use crate::{ - BootInfo, DirectoryReader, Entry, EntryEncoder, EntryOutputOptions, FileReader, ReaderOptions, - SdkError, export_entry_bytes, format_entry_text, + BootInfo, BootListOptions, DirectoryReader, Entry, EntryEncoder, EntryOutputOptions, + FileReader, ReaderOptions, SdkError, export_entry_bytes, format_entry_text, }; use std::fmt; use std::path::Path; @@ -497,28 +497,21 @@ impl SdJournal { .map_err(map_error) } - /// Lists every boot, oldest first, as `journalctl --list-boots` does. If - /// the files cannot be read it falls back to one boot per file header. - pub fn list_boots(&self) -> Vec { - match &self.reader { - ReaderKind::File(reader) => reader.list_boots().unwrap_or_else(|_| { - let header = reader.cached_header().header; - vec![BootInfo { - index: 0, - boot_id: hex::encode(header.tail_entry_boot_id), - first_entry: header.head_entry_realtime as i64, - last_entry: header.tail_entry_realtime as i64, - }] - }), - ReaderKind::Directory(reader) => reader.list_boots(), - } + /// Lists every boot, oldest first, as `journalctl --list-boots` does. It + /// does not change matches or the read position. + pub fn list_boots(&self) -> std::result::Result, Error> { + self.list_boots_with_options(BootListOptions::default()) } - fn try_list_boots(&self) -> crate::Result> { + pub fn list_boots_with_options( + &self, + options: BootListOptions, + ) -> std::result::Result, Error> { match &self.reader { - ReaderKind::File(reader) => reader.list_boots(), - ReaderKind::Directory(reader) => reader.try_list_boots(), + ReaderKind::File(reader) => reader.list_boots_with_options(options), + ReaderKind::Directory(reader) => reader.list_boots_with_options(options), } + .map_err(map_error) } pub fn set_output_mode(&mut self, mode: OutputMode) { @@ -667,7 +660,14 @@ pub fn SdJournalEnumerateField(j: &mut SdJournal) -> std::result::Result std::result::Result, Error> { - j.try_list_boots().map_err(map_error) + j.list_boots() +} + +pub fn SdJournalListBootsWithOptions( + j: &mut SdJournal, + options: BootListOptions, +) -> std::result::Result, Error> { + j.list_boots_with_options(options) } pub fn SdJournalQueryUnique( diff --git a/rust/src/journal/src/lib.rs b/rust/src/journal/src/lib.rs index 228a87aa..602029d3 100644 --- a/rust/src/journal/src/lib.rs +++ b/rust/src/journal/src/lib.rs @@ -34,7 +34,9 @@ pub use export::{ is_printable_text, json_entry, }; pub use parse::{ParseError, ParsedCursor, parse_cursor, parse_match_bytes, parse_match_string}; -pub use sealed_verify::{verify_file, verify_file_with_key, verify_index}; +pub use sealed_verify::{ + VerificationKey, parse_verification_key, verify_file, verify_file_with_key, verify_index, +}; use ouroboros::self_referencing; use std::collections::HashMap; @@ -58,16 +60,17 @@ pub use facade::{ SdJournalClose, SdJournalEnumerateAvailableData, SdJournalEnumerateAvailableUnique, SdJournalEnumerateField, SdJournalEnumerateFields, SdJournalFlushMatches, SdJournalGetCursor, SdJournalGetData, SdJournalGetEntry, SdJournalGetMonotonicUsec, SdJournalGetRealtimeUsec, - SdJournalGetSeqnum, SdJournalListBoots, SdJournalNext, SdJournalNextSkip, SdJournalOpen, - SdJournalOpenDirectory, SdJournalOpenDirectoryPath, SdJournalOpenDirectoryPathWithOptions, - SdJournalOpenDirectoryWithOptions, SdJournalOpenFile, SdJournalOpenFilePath, - SdJournalOpenFilePathWithOptions, SdJournalOpenFilePaths, SdJournalOpenFilePathsWithOptions, - SdJournalOpenFileWithOptions, SdJournalOpenFiles, SdJournalOpenFilesWithOptions, - SdJournalOpenPath, SdJournalPrevious, SdJournalPreviousSkip, SdJournalProcessOutput, - SdJournalQueryUnique, SdJournalQueryUniqueState, SdJournalRecoveryStatuses, - SdJournalRestartData, SdJournalRestartFields, SdJournalRestartUnique, SdJournalSeekCursor, - SdJournalSeekCursorDirection, SdJournalSeekHead, SdJournalSeekRealtimeUsec, SdJournalSeekTail, - SdJournalSetOutputMode, SdJournalTestCursor, SdJournalVisitUniqueValues, + SdJournalGetSeqnum, SdJournalListBoots, SdJournalListBootsWithOptions, SdJournalNext, + SdJournalNextSkip, SdJournalOpen, SdJournalOpenDirectory, SdJournalOpenDirectoryPath, + SdJournalOpenDirectoryPathWithOptions, SdJournalOpenDirectoryWithOptions, SdJournalOpenFile, + SdJournalOpenFilePath, SdJournalOpenFilePathWithOptions, SdJournalOpenFilePaths, + SdJournalOpenFilePathsWithOptions, SdJournalOpenFileWithOptions, SdJournalOpenFiles, + SdJournalOpenFilesWithOptions, SdJournalOpenPath, SdJournalPrevious, SdJournalPreviousSkip, + SdJournalProcessOutput, SdJournalQueryUnique, SdJournalQueryUniqueState, + SdJournalRecoveryStatuses, SdJournalRestartData, SdJournalRestartFields, + SdJournalRestartUnique, SdJournalSeekCursor, SdJournalSeekCursorDirection, SdJournalSeekHead, + SdJournalSeekRealtimeUsec, SdJournalSeekTail, SdJournalSetOutputMode, SdJournalTestCursor, + SdJournalVisitUniqueValues, }; pub use journal_core::error::JournalError; use journal_core::file::ExperimentalMmapStrategy; @@ -303,6 +306,15 @@ fn split_raw_payload(payload: &[u8]) -> Option> { }) } +/// Selects how boots are listed. +#[derive(Debug, Clone, Copy, Default)] +pub struct BootListOptions { + /// Lists the boot IDs in ENTRY headers when the journal has no `_BOOT_ID` + /// DATA at all, which the core writer can produce. `journalctl + /// --list-boots` reports no boots for such a journal. + pub header_fallback: bool, +} + #[derive(Debug, Clone)] pub struct BootInfo { pub index: i64, @@ -510,9 +522,13 @@ impl FileReader { fn recover_missing_header_tail(file: &JournalFile) -> Result<()> { let header = file.journal_header_ref(); let extent = file.reader_file_size()?; - if header - .tail_object_offset - .is_some_and(|offset| offset.get().checked_add(16).is_none_or(|end| end > extent)) + // Without the main array, a filtered read could still reach entries + // through DATA postings; decide the visible prefix before any read. + let missing_main_array = header.entry_array_offset.is_none() && header.n_entries != 0; + if missing_main_array + || header + .tail_object_offset + .is_some_and(|offset| offset.get().checked_add(16).is_none_or(|end| end > extent)) { file.recover_reader_tail(&JournalError::ObjectExceedsFileBounds)?; } @@ -804,7 +820,12 @@ impl FileReader { let row = &mut self.row; let payload = self.inner.with_mut(|fields| { fields.reader.release_object_guards(); - row.read_next_payload(fields.file) + loop { + match row.read_next_payload(fields.file) { + Err(err) if recoverable_entry_data_error(&err) => continue, + other => break other, + } + } })?; Ok(payload.map(|payload| self.row.payload_slice(payload))) } diff --git a/rust/src/journal/src/netdata.rs b/rust/src/journal/src/netdata.rs index 18cda26d..29423e5a 100644 --- a/rust/src/journal/src/netdata.rs +++ b/rust/src/journal/src/netdata.rs @@ -3443,7 +3443,7 @@ fn journal_file_source_type(path: &Path) -> u64 { } fn local_namespace_source_name(path: &Path) -> Option { - let parent = path.parent()?.file_name()?.to_str()?; + let parent = path.parent()?.file_name()?.to_string_lossy(); let (_, namespace) = parent.rsplit_once('.')?; (!namespace.is_empty()).then(|| format!("namespace-{namespace}")) } diff --git a/rust/src/journal/src/reader_helpers.rs b/rust/src/journal/src/reader_helpers.rs index ccfd5c7a..309fbfed 100644 --- a/rust/src/journal/src/reader_helpers.rs +++ b/rust/src/journal/src/reader_helpers.rs @@ -73,7 +73,8 @@ pub(super) fn read_current_row_entry( let value = payload[eq + 1..].to_vec(); if let Ok(name) = std::str::from_utf8(raw_name) { let name = name.to_string(); - fields.insert(name.clone(), value.clone()); + // The first repeated value wins, as sd_journal_get_data returns it. + fields.entry(name.clone()).or_insert_with(|| value.clone()); field_values.entry(name).or_default().push(value); } } @@ -245,7 +246,8 @@ pub(super) fn recoverable_entry_data_error(err: &JournalError) -> bool { matches!( err, JournalError::InvalidOffset - | JournalError::InvalidObjectSize(0) + | JournalError::InvalidObjectSize(_) + | JournalError::InvalidObjectType | JournalError::ObjectExceedsFileBounds ) } diff --git a/rust/src/journal/src/sealed_verify.rs b/rust/src/journal/src/sealed_verify.rs index 7a3cc8a7..b80eae63 100644 --- a/rust/src/journal/src/sealed_verify.rs +++ b/rust/src/journal/src/sealed_verify.rs @@ -91,10 +91,11 @@ pub(crate) fn verify_file_with_key_options( let incompatible_flags = read_u32_for_verify(&source, 12, "incompatible_flags")?; let sealed = (compatible_flags & 1) != 0; if sealed { - let (seed, start_usec, interval_usec) = parse_verification_key(verification_key) - .map_err(|e| { - SdkError::VerificationError(format!("invalid verification key: {e}")) - })?; + let VerificationKey { + seed, + start_epoch: start_usec, + interval_usec, + } = parse_verification_key(verification_key)?; verify_sealed( &source, compatible_flags, @@ -122,74 +123,107 @@ fn verify_reader_object_graph(reader: &FileReader) -> Result<()> { }) } -fn parse_verification_key(key: &str) -> std::result::Result<([u8; 12], u64, u64), String> { +/// A parsed Forward Secure Sealing verification key. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct VerificationKey { + pub seed: [u8; 12], + pub start_epoch: u64, + pub interval_usec: u64, +} + +/// Parses `/-` with the grammar of systemd v262 +/// `journal_auth_load_key`, including its tolerance for content after the +/// interval. +pub fn parse_verification_key(key: &str) -> Result { + parse_verification_key_parts(key).map_err(|reason| { + SdkError::VerificationError(format!("invalid verification key: {reason}")) + }) +} + +fn parse_verification_key_parts(key: &str) -> std::result::Result { let bytes = key.as_bytes(); let (seed, slash_offset) = parse_verification_seed(bytes)?; - if slash_offset >= bytes.len() || bytes[slash_offset] != b'/' { - return Err("missing / separator".into()); + if bytes.get(slash_offset) != Some(&b'/') { + return Err("missing / separator"); } - let (start_usec, dash_offset) = parse_verification_hex_value(bytes, slash_offset + 1, "start")?; - if dash_offset >= bytes.len() || bytes[dash_offset] != b'-' { - return Err("bad start hex".into()); + let (start_epoch, dash_offset) = + parse_verification_number(bytes, slash_offset + 1).ok_or("bad start hex")?; + if bytes.get(dash_offset) != Some(&b'-') { + return Err("bad start hex"); } - let (interval_usec, end_offset) = - parse_verification_hex_value(bytes, dash_offset + 1, "interval")?; - if end_offset != bytes.len() { - return Err("trailing data".into()); + let (interval_usec, _) = + parse_verification_number(bytes, dash_offset + 1).ok_or("bad interval hex")?; + if start_epoch == 0 || interval_usec == 0 { + return Err("zero start or interval"); } - if interval_usec == 0 { - return Err("zero interval".into()); + if start_epoch.checked_mul(interval_usec).is_none() { + return Err("start time overflows"); } - Ok((seed, start_usec, interval_usec)) + Ok(VerificationKey { + seed, + start_epoch, + interval_usec, + }) } -fn parse_verification_seed(bytes: &[u8]) -> std::result::Result<([u8; 12], usize), String> { +fn parse_verification_seed(bytes: &[u8]) -> std::result::Result<([u8; 12], usize), &'static str> { let mut seed = [0u8; 12]; let mut i = 0; - for c in 0..12 { - let (next, val) = parse_verification_seed_byte(bytes, i)?; - seed[c] = val; - i = next; + for byte in &mut seed { + while bytes.get(i) == Some(&b'-') { + i += 1; + } + let high = hex_digit_value(bytes.get(i).copied()).ok_or("bad seed hex")?; + let low = hex_digit_value(bytes.get(i + 1).copied()).ok_or("bad seed hex")?; + *byte = high << 4 | low; + i += 2; } Ok((seed, i)) } -fn parse_verification_seed_byte( - bytes: &[u8], - start: usize, -) -> std::result::Result<(usize, u8), String> { +/// Follows sscanf's `%lx`, which systemd uses here: leading whitespace, a +/// sign, a `0x` prefix, and saturation on overflow. +fn parse_verification_number(bytes: &[u8], start: usize) -> Option<(u64, usize)> { let mut i = start; - while i < bytes.len() && bytes[i] == b'-' { + while bytes + .get(i) + .is_some_and(|b| *b == b' ' || (b'\t'..=b'\r').contains(b)) + { i += 1; } - if i + 2 > bytes.len() { - return Err("seed too short".into()); + let negative = bytes.get(i) == Some(&b'-'); + if matches!(bytes.get(i), Some(b'+' | b'-')) { + i += 1; } - let val = u8::from_str_radix(std::str::from_utf8(&bytes[i..i + 2]).unwrap_or("xx"), 16) - .map_err(|_| "bad seed hex".to_string())?; - Ok((i + 2, val)) -} - -fn parse_verification_hex_value( - bytes: &[u8], - start: usize, - label: &str, -) -> std::result::Result<(u64, usize), String> { - let (next, ok) = consume_hex(bytes, start); - if !ok { - return Err(format!("bad {label} hex")); + if bytes.get(i) == Some(&b'0') && matches!(bytes.get(i + 1), Some(b'x' | b'X')) { + i += 2; } - let value = u64::from_str_radix(std::str::from_utf8(&bytes[start..next]).unwrap_or("0"), 16) - .map_err(|_| format!("bad {label} hex"))?; - Ok((value, next)) -} - -fn consume_hex(bytes: &[u8], start: usize) -> (usize, bool) { - let mut i = start; - while i < bytes.len() && bytes[i].is_ascii_hexdigit() { + let digits = i; + let mut value = 0u64; + let mut overflow = false; + while let Some(digit) = hex_digit_value(bytes.get(i).copied()) { + overflow |= value >> 60 != 0; + value = value << 4 | u64::from(digit); i += 1; } - (i, i > start) + if i == digits { + return None; + } + if overflow { + return Some((u64::MAX, i)); + } + Some(( + if negative { + value.wrapping_neg() + } else { + value + }, + i, + )) +} + +fn hex_digit_value(byte: Option) -> Option { + char::from(byte?).to_digit(16).map(|digit| digit as u8) } pub(super) fn align8(v: u64) -> u64 { diff --git a/rust/src/journal/src/tests.rs b/rust/src/journal/src/tests.rs index 91ebc2df..4c1bf8e3 100644 --- a/rust/src/journal/src/tests.rs +++ b/rust/src/journal/src/tests.rs @@ -410,9 +410,11 @@ fn directory_reader_uses_sequential_path_for_non_overlapping_files() { mod boots; mod directory_files; +mod entry_damage; mod entry_output; mod facade; mod verification; +mod verification_key; mod indexed_snapshot; mod position_contract; diff --git a/rust/src/journal/src/tests/boots.rs b/rust/src/journal/src/tests/boots.rs index 970fb8b5..6a657868 100644 --- a/rust/src/journal/src/tests/boots.rs +++ b/rust/src/journal/src/tests/boots.rs @@ -92,8 +92,7 @@ fn list_boots_multi_boot() { boot(-1, 0xb0, 200, 220), boot(0, 0xa0, 100, 300), ]; - assert_eq!(rows(directory.try_list_boots().unwrap()), want); - assert_eq!(rows(directory.list_boots()), want); + assert_eq!(rows(directory.list_boots().unwrap()), want); let mut facade = SdJournalOpenDirectory(temp.path().to_str().unwrap(), 0).unwrap(); assert_eq!(rows(SdJournalListBoots(&mut facade).unwrap()), want); @@ -104,7 +103,8 @@ fn list_boots_multi_boot() { ); } -/// Journals without `_BOOT_ID` DATA fall back to ENTRY header boot IDs. +/// Journals without `_BOOT_ID` DATA list no boots, as v262 journalctl does, +/// unless the caller opts into the ENTRY header boot IDs. #[test] fn list_boots_falls_back_to_entry_headers() { let temp = tempfile::tempdir().unwrap(); @@ -116,8 +116,12 @@ fn list_boots_falls_back_to_entry_headers() { None, ); let reader = FileReader::open(&path).unwrap(); + assert!(reader.list_boots().unwrap().is_empty()); + let options = BootListOptions { + header_fallback: true, + }; assert_eq!( - rows(reader.list_boots().unwrap()), + rows(reader.list_boots_with_options(options).unwrap()), vec![boot(-1, 0xd0, 500, 510), boot(0, 0xe0, 600, 600)] ); } @@ -147,6 +151,83 @@ fn list_boots_ignores_non_canonical_boot_id_data() { ); } +/// Noncanonical `_BOOT_ID` DATA still counts as present, so the fallback must +/// not report boots a `_BOOT_ID` match would not find. +#[test] +fn list_boots_fallback_ignores_all_non_canonical_data() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("system.journal"); + write_boot_journal( + &path, + 0x51, + &[(0xbb, 200, 1)], + Some(|b| boot_hex(b).to_ascii_uppercase()), + ); + let reader = FileReader::open(&path).unwrap(); + let options = BootListOptions { + header_fallback: true, + }; + assert!(reader.list_boots_with_options(options).unwrap().is_empty()); +} + +/// Postings past the visible tail do not reach the boot list. +#[test] +fn list_boots_stops_at_the_visible_tail() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("system.journal"); + write_boot_journal( + &path, + 0x52, + &[(0xa0, 100, 1), (0xa0, 200, 2)], + Some(boot_hex), + ); + let repo = RepoFile::from_path(&path).unwrap(); + let mut offsets = Vec::new(); + JournalFile::::open(&repo, 4096) + .unwrap() + .entry_offsets(&mut offsets) + .unwrap(); + let mut bytes = std::fs::read(&path).unwrap(); + bytes[264..272].copy_from_slice(&offsets[0].get().to_le_bytes()); + std::fs::write(&path, bytes).unwrap(); + + let reader = FileReader::open(&path).unwrap(); + assert_eq!( + rows(reader.list_boots().unwrap()), + vec![boot(0, 0xa0, 100, 100)] + ); +} + +/// While a writer holds the file, the tail hint may lead `n_entries`. +#[test] +fn list_boots_ignores_a_tail_hint_ahead_of_the_entry_count() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("system.journal"); + write_boot_journal( + &path, + 0x53, + &[(0xa0, 100, 1), (0xb0, 200, 1)], + Some(boot_hex), + ); + let repo = RepoFile::from_path(&path).unwrap(); + let mut offsets = Vec::new(); + JournalFile::::open(&repo, 4096) + .unwrap() + .entry_offsets(&mut offsets) + .unwrap(); + let mut bytes = std::fs::read(&path).unwrap(); + bytes[16] = 1; + bytes[152..160].copy_from_slice(&1u64.to_le_bytes()); + bytes[264..272].copy_from_slice(&offsets[1].get().to_le_bytes()); + std::fs::write(&path, bytes).unwrap(); + + let reader = FileReader::open(&path).unwrap(); + assert_eq!( + rows(reader.list_boots().unwrap()), + vec![boot(0, 0xa0, 100, 100)] + ); +} + /// list_boots leaves the caller's matches and read position alone. #[test] fn list_boots_keeps_matches_and_position() { @@ -167,7 +248,7 @@ fn list_boots_keeps_matches_and_position() { let mut directory = DirectoryReader::open_files([&path]).unwrap(); directory.add_match(format!("_BOOT_ID={}", boot_hex(0xb0)).as_bytes()); assert!(directory.next().unwrap()); - assert_eq!(directory.list_boots().len(), 2); + assert_eq!(directory.list_boots().unwrap().len(), 2); assert!(directory.next().unwrap()); assert_eq!(directory.get_realtime_usec().unwrap(), 210); assert!(!directory.next().unwrap(), "match was lost"); @@ -205,7 +286,7 @@ fn list_boots_dirty_archives_fresh_reader() { ), ]; let directory = DirectoryReader::open(&dir).unwrap(); - assert_eq!(rows(directory.try_list_boots().unwrap()), want); + assert_eq!(rows(directory.list_boots().unwrap()), want); let mut facade = SdJournalOpenDirectory(dir.to_str().unwrap(), 0).unwrap(); assert_eq!(rows(SdJournalListBoots(&mut facade).unwrap()), want); } diff --git a/rust/src/journal/src/tests/entry_damage.rs b/rust/src/journal/src/tests/entry_damage.rs new file mode 100644 index 00000000..563235c5 --- /dev/null +++ b/rust/src/journal/src/tests/entry_damage.rs @@ -0,0 +1,141 @@ +//! Paired with Go `reader_entry_damage_test.go`. + +use super::*; + +const OBJECT_HEADER_TO_PAYLOAD: usize = 64; + +fn write_entry(path: &Path, payloads: &[&[u8]]) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + let repo_file = RepoFile::from_path(path).expect("journal path"); + let mut file = JournalFile::::create( + &repo_file, + JournalFileOptions::new(test_uuid(1), test_uuid(2), test_uuid(3)), + ) + .expect("create journal"); + let mut writer = JournalWriter::new(&mut file, 1, test_uuid(4)).expect("writer"); + writer + .add_entry(&mut file, payloads, 1_700_000_000_000_000, 1) + .expect("write entry"); + file.sync().expect("sync"); +} + +fn damage(path: &Path, patch: fn(&mut [u8], usize)) { + let mut bytes = std::fs::read(path).unwrap(); + let needle = b"BROKEN=1"; + let at = bytes + .windows(needle.len()) + .position(|w| w == needle) + .expect("payload in file"); + patch(&mut bytes, at); + std::fs::write(path, bytes).unwrap(); +} + +fn journal_path(dir: &Path) -> PathBuf { + dir.join(format!("{}/system.journal", test_uuid(1).simple())) +} + +#[test] +fn damaged_data_object_costs_the_field_not_the_entry() { + type Patch = fn(&mut [u8], usize); + let cases: [(&str, Patch, bool); 3] = [ + ("no-separator", |b, at| b[at + 6] = b'_', true), + ( + "wrong-type", + |b, at| b[at - OBJECT_HEADER_TO_PAYLOAD] = 3, + false, + ), + ( + "oversized", + |b, at| { + let size = at - OBJECT_HEADER_TO_PAYLOAD + 8; + b[size..size + 8].copy_from_slice(&(1u64 << 40).to_le_bytes()); + }, + false, + ), + ]; + for (name, patch, payload_kept) in cases { + let dir = tempfile::tempdir().unwrap(); + let path = journal_path(dir.path()); + write_entry(&path, &[b"MESSAGE=kept", b"BROKEN=1"]); + damage(&path, patch); + + let mut reader = DirectoryReader::open_files([&path]).unwrap(); + assert!(reader.next().unwrap(), "{name}"); + let entry = reader + .get_entry() + .unwrap_or_else(|e| panic!("{name}: {e:?}")); + assert_eq!(entry.fields["MESSAGE"], b"kept", "{name}"); + assert!(!entry.fields.contains_key("BROKEN"), "{name}"); + assert_eq!( + entry.payloads.len(), + 1 + usize::from(payload_kept), + "{name}" + ); + + let want = 1 + usize::from(payload_kept); + let check = |api: &str, got: &[Vec]| { + assert_eq!(got.len(), want, "{name} {api}"); + assert!( + got.iter().any(|p| p == b"MESSAGE=kept"), + "{name} {api}: MESSAGE missing" + ); + assert!( + !got.iter().any(|p| p.starts_with(b"BROKEN=")), + "{name} {api}: damaged field returned" + ); + }; + check("get_entry", &entry.payloads); + + let mut collected = Vec::new(); + reader.collect_entry_payloads(&mut collected).unwrap(); + check("collect_entry_payloads", &collected); + reader.entry_data_restart().unwrap(); + let mut enumerated = Vec::new(); + while let Some(payload) = reader.enumerate_entry_payload().unwrap() { + enumerated.push(payload.to_vec()); + } + check("enumerate_entry_payload", &enumerated); + } +} + +#[test] +fn repeated_field_keeps_the_first_value() { + let dir = tempfile::tempdir().unwrap(); + let path = journal_path(dir.path()); + write_entry(&path, &[b"KEY=a", b"KEY=b"]); + let mut reader = DirectoryReader::open_files([&path]).unwrap(); + assert!(reader.next().unwrap()); + let entry = reader.get_entry().unwrap(); + let first = entry.field_values["KEY"][0].clone(); + assert_eq!(entry.fields["KEY"], first); + assert_eq!(entry.field_values["KEY"].len(), 2); +} + +#[test] +fn open_files_opens_a_repeated_path_once() { + let dir = tempfile::tempdir().unwrap(); + let path = journal_path(dir.path()); + write_entry(&path, &[b"MESSAGE=once"]); + let mut reader = DirectoryReader::open_files([&path, &path]).unwrap(); + assert!(reader.next().unwrap()); + assert!(!reader.next().unwrap(), "the same file was read twice"); +} + +// Paired with Go TestMissingMainEntryArrayHidesEveryEntry. +#[test] +fn missing_main_entry_array_hides_every_entry() { + for filtered in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let path = journal_path(dir.path()); + write_entry(&path, &[b"MESSAGE=hidden"]); + let mut bytes = std::fs::read(&path).unwrap(); + bytes[176..184].fill(0); + std::fs::write(&path, bytes).unwrap(); + + let mut reader = DirectoryReader::open_files([&path]).unwrap(); + if filtered { + reader.add_match(b"MESSAGE=hidden"); + } + assert!(!reader.next().unwrap(), "filtered={filtered}"); + } +} diff --git a/rust/src/journal/src/tests/reader_hardening.rs b/rust/src/journal/src/tests/reader_hardening.rs index 36a72f12..81fe99c2 100644 --- a/rust/src/journal/src/tests/reader_hardening.rs +++ b/rust/src/journal/src/tests/reader_hardening.rs @@ -798,7 +798,11 @@ fn directory_repair_preserves_pending_merge_rows() { } assert_eq!(times, vec![2000, 3000]); assert!(directory.recovery_statuses().is_empty()); - let boots = directory.list_boots(); + let boots = directory + .list_boots_with_options(BootListOptions { + header_fallback: true, + }) + .unwrap(); assert_eq!(boots.len(), 1); assert_eq!(boots[0].last_entry, 3000); directory.seek_tail(); diff --git a/rust/src/journal/src/tests/verification_key.rs b/rust/src/journal/src/tests/verification_key.rs new file mode 100644 index 00000000..73043baa --- /dev/null +++ b/rust/src/journal/src/tests/verification_key.rs @@ -0,0 +1,46 @@ +use crate::parse_verification_key; + +const SEED: &str = "0123456789abcdef01234567"; + +// Accepted and rejected shapes were checked against the systemd v262 CLI. +#[test] +fn parse_verification_key_follows_systemd_grammar() { + let accepted = [ + (format!("{SEED}/1-f4240"), 1, 0xf4240), + ("-012345-6789ab--cdef01234567/1-2".to_string(), 1, 2), + (format!("{SEED}/1-2x"), 1, 2), + (format!("{SEED}/ 1- 2"), 1, 2), + (format!("{SEED}/+1-+2"), 1, 2), + (format!("{SEED}/0x1-0X2"), 1, 2), + (format!("{SEED}/1--1"), 1, u64::MAX), + (format!("{SEED}/1-1ffffffffffffffff"), 1, u64::MAX), + ]; + for (key, start, interval) in accepted { + let parsed = parse_verification_key(&key).unwrap_or_else(|err| panic!("{key}: {err}")); + assert_eq!( + parsed.seed, + [ + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67 + ], + "{key}" + ); + assert_eq!(parsed.start_epoch, start, "{key}"); + assert_eq!(parsed.interval_usec, interval, "{key}"); + } + + let rejected = [ + String::new(), + format!("+a{}/1-2", &SEED[2..]), + format!("{SEED}ab/1-2"), + SEED.to_string(), + format!("{SEED}/1"), + format!("{SEED}/1 -2"), + format!("{SEED}/0-2"), + format!("{SEED}/1-0"), + format!("{SEED}/0x-2"), + format!("{SEED}/ffffffffffffffff-2"), + ]; + for key in rejected { + assert!(parse_verification_key(&key).is_err(), "{key}"); + } +} diff --git a/tests/interoperability/run_journalctl_query_matrix.py b/tests/interoperability/run_journalctl_query_matrix.py index 9c0bb1a8..96bac49a 100755 --- a/tests/interoperability/run_journalctl_query_matrix.py +++ b/tests/interoperability/run_journalctl_query_matrix.py @@ -272,6 +272,7 @@ def make_fixtures() -> dict[str, Path]: source_realtime = FIXTURE_DIR / "source-realtime.journal" pager = FIXTURE_DIR / "pager.journal" implicit_boot = FIXTURE_DIR / "implicit-boot.journal" + no_boot_id = FIXTURE_DIR / "no-boot-id.journal" follow = FIXTURE_DIR / "follow" directory.mkdir(parents=True) follow.mkdir(parents=True) @@ -285,6 +286,7 @@ def make_fixtures() -> dict[str, Path]: write_source_realtime_journal(source_realtime) write_pager_journal(pager) write_implicit_boot_journal(implicit_boot) + write_no_boot_id_journal(no_boot_id) return { "directory": directory, "file": single_file, @@ -294,6 +296,7 @@ def make_fixtures() -> dict[str, Path]: "source_realtime": source_realtime, "pager": pager, "implicit_boot": implicit_boot, + "no_boot_id": no_boot_id, "follow": follow, } @@ -505,6 +508,23 @@ def write_pager_journal(path: Path) -> None: ) +def write_no_boot_id_journal(path: Path) -> None: + write_journal_file( + path, + machine_id=MACHINE_ID, + boot_id=BOOT_C, + seqnum_id=VACUUM_SEQNUM_ID, + entries=[ + { + "realtime_usec": 1_700_006_200_000_001, + "monotonic_usec": 1, + "boot_id": BOOT_C, + "fields": [("MESSAGE", "no-boot-id"), ("TEST_ID", "journalctl-no-boot-id")], + } + ], + ) + + def write_implicit_boot_journal(path: Path) -> None: entries = [] for idx, (boot_id, message) in enumerate(((BOOT_A, "implicit-old"), (BOOT_C, "implicit-current")), start=1): @@ -1623,6 +1643,31 @@ def run_utility_action_cases(tools: dict[str, str], fixtures: dict[str, Path]) - } ) + # Exit status and stdout only: the error wording is not part of this check. + for case_name, args in ( + ("list-boots-no-boot-id", ["--file", str(fixtures["no_boot_id"]), "--list-boots"]), + ("boot-zero-no-boot-id", ["--file", str(fixtures["no_boot_id"]), "-b", "0"]), + ): + stock = run(action_command("stock", tools, args), timeout=30) + if stock.returncode == 0: + raise RuntimeError(f"stock {case_name} unexpectedly succeeded") + for reader in ("go", "rust"): + cmd = action_command(reader, tools, args) + result = run(cmd, timeout=30) + ok = result.returncode == stock.returncode and result.stdout == stock.stdout + results.append( + { + "test": case_name, + "reader": reader, + "status": "PASS" if ok else "FAIL", + "command": " ".join(cmd), + "expected": stock.stdout, + "actual": result.stdout, + "stderr": result.stderr[-1000:], + "returncode": result.returncode, + } + ) + vacuum_root = fixtures["directory"].parent / "vacuum" vacuum_cases = [ ("vacuum-files-protect-active", ["--vacuum-files=2"], True),