Repository navigation
fix(deps): move rustls to 0.23.45 for RUSTSEC-2026-0285 - #1785
Merged
Merged
Conversation
rustls 0.23.42 accepted a TLS 1.3 handshake message sent at the wrong encryption level when it shared a record with a key-changing message. podup reaches rustls only through ureq, client side, in the self-updater, and verifies every downloaded asset against the release signature, so the exposure is low. The audit gate is red for every pull request that changes the lockfile until this moves. Lockfile only: rustls 0.23.42 -> 0.23.45, rustls-webpki 0.103.13 -> 0.103.15. Refs #1783. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added
type:security
develop builds to 7,863,856 bytes, 464 under the 7.5 MiB ceiling set on 2026-08-03 (#1320), so any change that adds code goes over it. The rustls security patch in this branch is the first: 7,864,880 bytes, 1,024 more than develop and 560 over. That growth is explained, a patched TLS library, and is not a regression. 8 MiB leaves 523,728 bytes of room over this branch, which still fails a dependency that brings a few hundred KiB with it while no longer failing every ordinary fix. Refs #1783. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added a commit
that referenced
this pull request
Sep 19, 2026
…1789) ## Summary - A warm `up` asked Podman for the same image tag once per replica: N replicas of one service that pass the config-hash check made N identical `GET /images/{tag}/json`. It now makes one per service. - The answer lives for one service's replica loop and nothing longer, which is the difference from the attempt that #1755 withdrew. Refs #1759. ## Changes - `lifecycle::ResolvedImage`, a `tokio::sync::OnceCell<Option<String>>`, created in `up_one_service` right after `acquire_service_image` returns and dropped with that service's replica fan-out. `unchanged` reads the tag's id through `get_or_try_init`. - The cell serialises initialisation, so the replicas `join_bounded` polls at once produce one request, not N. It is lazy: a cold `up`, `--force-recreate`, `--no-recreate` and a changed hash still make zero inspects. - A failed inspect is not remembered. `get_or_try_init` leaves the cell empty on `Err`, the replica that asked gets the same `ComposeError::Podman` as before, and the next replica asks again. - Not shared between services. Two services on one image cost two inspects, deliberately: a cell that outlives one service spans a sibling's pull or build of the same tag, which is the #1755 cache again one size down. ## Why the earlier attempt failed and this one should not #1755's squash message still claims this saving, but its diff touches only `scale.rs`, `query/images.rs` and `query/inspect.rs`. The `up` half cached the id the prefetch stage had fetched, and prefetch runs before acquisition (`up.rs` prefetch, then `acquire_service_image` in `up_one.rs`), so the cache spanned the pull or build of the very tag it was about. `recreate_on_image::a_retagged_image_recreates_an_image_service` and `x_podman_autoupdate::up_with_autoupdate_registry_recreates_after_the_tag_moved_without_pull_flag` refused it. Here the lookup is taken after this service's acquisition is over, and `create_and_start` neither pulls nor builds, so nothing this service does moves its tag while the answer is alive. Both live tests retag between invocations, and a cell never spans two. ## What this is allowed to miss The comment on the issue asks that somebody define what `unchanged` may miss before this lands. Decided on 2026-09-18: a tag moved by a sibling service in the same level, or from outside, while one service's replica loop is running. That race already exists today and gives a mixed service: replicas that inspected before the move are kept, the ones after are recreated. With this change all of them see it or none does, and the next `up` catches it. Nothing else is narrowed. ## Correction after a second review A second review found that my first commit's doc comment said the cell does not span a sibling service's pull or build of the same tag. It can: services in one level start concurrently. The behaviour is the one described in the previous section; the second commit makes the comment say so, and I corrected the same sentence where I had written it on #1759. ## Test plan - [x] `cargo fmt --all --check` is clean - [x] `cargo clippy --locked --all-targets --all-features -- -D warnings` is clean - [x] `cargo test --locked --lib --all-features lifecycle::`: 162 passed. The nine new ones are in `lifecycle/up_one_tests.rs` and assert request counts against the recording fake: three replicas inspect once, two services sharing an image once each, two images once each, a moved tag still recreates every replica, only the replica bound to a stale image is recreated, a failed inspect is asked again by the next replica, a cold `up` inspects nothing, a failing inspect fails `up` with the Podman error - [x] New or changed checks were verified by deleting the control and watching them fail: with only the `get_or_try_init` replaced by the old direct `image_id` call (signature kept so it compiles, `diff -q` confirmed the file changed), 7 of the 9 fail with the old numbers, `left: 3 right: 1` for three replicas, and the two controls stay green - [ ] Podman lane: I could not run the two live tests on my machine. They fail on the first `up`, before any retag, because this Podman 5.7.0 answers 404 on the libpod image inspect for a locally built image and podup then tries to pull `localhost/...`. The same two fail the same way on `develop` without this change, so it says nothing either way. This pull request's own Podman 5 and Podman 6 lanes are the proof, and those two tests are the ones to read - [ ] The two behaviours I argued but did not sabotage, because undoing them is a restructure and not a revert: caching the error, and widening the cell across services. `a_failed_inspect_is_not_remembered_for_the_next_replica` and `two_services_sharing_an_image_inspect_it_once_each` are the tests that would catch them ## Checklist - [x] Targets `develop` - [x] Commits are signed off (DCO, `git commit -s`) - [x] Commits are signed - [x] Labels applied - [x] No secrets, keys or credentials in code, logs or fixtures - [x] `bench/binary-budget-mib`: rebased on `develop` after #1785 raised the ceiling to 8 MiB ## Related issues Refs #1759. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added a commit
that referenced
this pull request
Sep 19, 2026
…s up (#1790) ## Summary - `podup cp` of a directory was reported as failed against Podman 6 whether or not it had landed. Podman 6 applies the archive and closes without a response, and the only confirmation podup had was one file's size, which a directory does not have. - A directory upload is now confirmed entry by entry: every regular file of the archive that was sent is at the destination with its size, and every directory is there as a directory. Refs #1777. ## Root cause In `put_archive_verified`, after an `IncompleteMessage` on the PUT, the only arm that could say "landed" was `(Some(path), Some(expected))`. `uploaded_entry_size` filters on `Metadata::is_file`, so a directory source gave `None` and fell into `_ => false`, always. On Podman 5 the PUT answers 200 and the function returns before any of this, which is why it never showed there. ## Changes - `internal/engine/copy/verify.rs`, new: `sent_entries` reads the expectation back out of the gzipped tar that went over the wire, not from a second walk of the source, so it cannot describe another tree. `tree_landed` stats each entry with the same archive HEAD the single-file path uses. - Anything short of a full match is a failure: an archive that cannot be read back, one with nothing to ask about, a stat that errors, an entry that is absent, a file of another size, a file where a directory was sent. - Links are sent and not asked about. Measured against Podman 5.7.0 on 2026-09-18: a dangling symlink answers the stat with 404 although it is there, so asking would turn a landed copy into a failure. - Entries are asked about last to first, because extraction is sequential and a cut stream loses its tail, and one at a time, because Podman 6 dropped responses under concurrency (#1339) and a dropped stat here is the same false failure over again. - The single-file path keeps its function, its comparison and its arm. On a 200 nothing new runs. - `FakeReply::Headers` in the fake Podman, because the archive stat lives in a response header and the fake could not send one. Cost: one HEAD per entry, about 10 ms each measured locally, paid only after a dropped response. The one request that lists a destination recursively is the archive GET, which streams the contents back too and so costs what was already there rather than what was sent. Residual false positive, the same one the single-file check documents: a failed upload over a tree whose entries already had these sizes. ## Correction after a second review A second review found three gaps, fixed in the third commit: - A FIFO of size 0 passed as an empty regular file, because a file only had to be "not a directory". It now needs none of Go's `os.ModeType` bits. - Links were left out of the expectation because a dangling one answers 404, so an archive cut after its last directory could be confirmed. On Podman 5.7.0 that 404 still carries the stat header, so links are now asked about through a separate client method that reads it; `head_path_stat` keeps treating 404 as absent for its other callers. Hard links, FIFOs and devices are still not asked about, and the module says so. - `to_string_lossy` rewrote a non-UTF-8 name before the stat. Such a tree is now unverifiable, reported as a failure. The live test copies a relative link and a dangling one too and reads them back with `readlink`. Sabotage on the fix: "not a directory" put back fells the FIFO cases, a link always accepted fells 3, the ordinary HEAD for links fells the 404-with-stat case, lossy names fell the non-UTF-8 case. ## Test plan - [x] `cargo fmt --all --check` is clean - [x] `cargo clippy --locked --all-targets --all-features -- -D warnings` is clean - [x] `cargo test --locked --lib --all-features engine::copy`: 62 passed. The upload tests drive `put_archive_verified` end to end against the fake with the PUT dropped the Podman 6 way; the stat JSON is in the shape captured from Podman 5.7.0 - [x] Written first and watched red: on the unfixed code `upload_tests` was 6 passed, 2 failed, the two being the directory that landed and the renamed directory, with the message from the issue - [x] `cargo test --locked --test engine_integration --features test-helpers cp_flags` against local Podman 5.7.0: 5 passed. That lane answers 200 and never reaches the new path, so it proves nothing broke, not that the fix works - [x] Podman 6: the first green lane on this pull request proved nothing, and I wrote that it was the proof, which was wrong. The only live test that copied a directory, `engine_cp_uploads_a_directory_holding_a_sparse_file`, stored `cp`'s result and deliberately never asserted on it, because before this fix a directory copy against Podman 6 failed closed whatever it did on disk. That lane was green with or without this change. The second commit adds `engine_cp_reports_a_directory_copy_as_landed`: a plain directory, no sparse file, no early return, asserting that the bytes arrived and that `cp` returned `Ok`; the sparse-directory test asserts the return value too now. Against my local Podman 5.7.0 the six `cp_flags` tests pass, which shows the test is sound and nothing else, since Podman 5 answers 200 and never reaches the new path. The lane sets `PODUP_REQUIRE_PODMAN`, so an unreachable Podman is a failure there and not a silent pass. The proof is this push's Podman 6 lane, measured on 2026-09-19: `Podman 6.1.2: 206 passed, 0 failed`, 0 ignored, 0 filtered out, where the run before this commit had 205. I cannot sabotage it locally for the same reason I cannot prove it locally, so what stands behind it is that the same assertion could not be written before this fix: the test next to it carried a comment saying why - [x] New or changed checks were verified by deleting the control and watching them fail, each edit confirmed with `diff -q`: | Control removed | What fell | |---|---| | the arm put back to `_ => false` | only the two directory tests | | the file size comparison | `a_directory_whose_entries_do_not_match_what_was_sent_is_a_failure` and the unit test of `entry_landed` | | an empty expectation counted as landed | only `an_upload_with_nothing_to_ask_about_stays_unconfirmed` | | a failed stat skipped instead of failing | only `a_directory_with_an_entry_that_cannot_be_read_back_is_a_failure` | ## Checklist - [x] Targets `develop` - [x] Commits are signed off (DCO, `git commit -s`) - [x] Commits are signed - [x] Labels applied - [x] No secrets, keys or credentials in code, logs or fixtures - [x] Line limit: `copy.rs` goes from 323 to 328 code lines, already over the 300 warning before this and under 500; the new code lives in `copy/verify.rs` (96) - [x] `bench/binary-budget-mib`: rebased on `develop` after #1785 raised the ceiling to 8 MiB ## Related issues Refs #1777. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added a commit
that referenced
this pull request
Sep 19, 2026
…1791) ## Summary - `podup cp SERVICE:PATH HOSTPATH` refused a destination that is a symlink, but looked at the last component only. `symlink_metadata` is `lstat`: it does not follow the last component and follows every earlier one, so `real/link/out` was classified as whatever `elsewhere/out` is and the bytes landed outside the path I named. - The guard now walks the destination one component at a time and refuses a symlink at any of them. Refs #1764. ## What I found beyond the issue - A second bypass of the old check: a trailing separator. `lstat("link/")` follows `link`, so `podup cp svc:/x ./link/` passed the last-component check too. The walk rebuilds each prefix from `components()`, which drops it. - Who can plant the link is not only a local user. `extract_tar_guarded` creates symlink entries from the archive, it only refuses to write through them in the same extraction. A hostile container can leave `backup/data -> ~/.ssh` in one copy and wait for my next `cp svc:/y ./backup/data/keys`. ## Changes - `internal/engine/copy/destination.rs`, new: `destination_refusal` and `refusal_for`. Both gates call it, `cp_destination_kind` (routing, `copy.rs`) and `extract_archive` (buffered branch, `copy/archive.rs`). - `..` is never collapsed lexically; every prefix before it was already seen to be a real directory, so the kernel's physical `..` is safe. The walk stops at the first missing component or at a regular file in the middle, and the existing "no such directory" error still comes from the extraction. A component that cannot be inspected for any other reason is refused. - The message is unchanged when the link is the destination itself, and names the offending component otherwise. - No `cfg` in the guard. On Windows std reports junctions as symlinks too; I read that in std's source and did not run it on a Windows host. - `docs/security-model.md`, row "Container filesystem": one sentence saying what the guard does and that it is a path check, not a pinned directory handle. ## What this does not close The time-of-check/time-of-use gap. The guard is a path check and every later operation walks the path again, so the issue's acceptance line "resolved once and the result is used" is not met, and the module doc says so. Who can win that race: a local process that can write in an ancestor of the destination, and a container with a writable bind mount over an ancestor of it. Closing it means replacing the extractor with one that works from an `O_NOFOLLOW` directory handle (`tar::Entry::unpack_in` takes a path), plus a Windows equivalent. That is its own issue. ## The system's own links Refusing every symlink refuses the system's too: `/tmp` on macOS (`/tmp -> /private/tmp`) and `/home` on Fedora Atomic (`/home -> var/home`). The first commit of this branch did exactly that, and `podup cp svc:/x /tmp/out` stopped working there. The second commit adds one narrow exception. A symlink is let through, and the walk continues below it with the path as written, only when all of these hold: - it sits directly in the root directory; - it is owned by root; - the root directory is owned by root and has no group or other write bit. A container cannot plant a link directly in `/` unless I already extracted into `/` as root, and then there is nothing left to protect. This is deliberately not the broader "trust any root-owned link in a root-owned directory": when podup runs as root, links extracted from a hostile archive are root-owned inside root-owned directories, and that rule would trust them. Off Unix the exception does not exist and every symlink is refused. Two things came out of reviewing it. `podup cp svc:/x /tmp`, the link being the destination itself, was still refused because both gates looked at the last component again on their own; they now read it through one function, `destination_metadata`, that follows a link only under the same three conditions. And with a single trusted uid in the policy the two owner checks could not be told apart in a test, since a test cannot `chown`: deleting the link-owner check left every test green. The policy now carries `root_uid` and `link_uid` separately, both 0 in production, so each check falls alone. ## Correction after a second review A second review found that the exception decided "directly in the root" by comparing spellings: `/../tmp/out`, or `tmp/out` from a working directory of `/`, were refused while `/tmp/out` passed, although all name the same link. Fail-closed, so a false refusal. The third commit compares canonicalised directories. Three new tests cover `R/sub/../link`, `R/real/../link` and a relative path; with the comparison put back to spellings exactly those three fall. The bare relative case from `/` (an empty parent) is handled but not tested, because a test cannot change the process's working directory without racing the others. ## Test plan - [x] `cargo fmt --all --check` is clean - [x] `cargo clippy --locked --all-targets --all-features -- -D warnings` is clean - [x] `cargo test --locked --lib --all-features engine::copy`: 77 passed, 12 in `copy/destination_tests.rs` and 21 in `copy/destination_trusted_tests.rs`, on real temp trees with real symlinks. The exception is tested with a temp directory as the trusted root and the test process's own uid, since nobody can create a link in `/` without being root - [x] Written first and watched red: on the unfixed code 3 passed and 7 failed. The table test counts: of 7 destinations that cross a symlink the old guard refused 1 - [x] New or changed checks were verified by deleting the control and watching them fail, each edit confirmed with `diff -q`: | Control removed | What fell | |---|---| | the walk cut back to the last component only | 6: first component, intermediate, both extraction cases, the table, the message. The last-component tests stay green | | an uninspectable component accepted | only `a_component_that_cannot_be_inspected_is_refused` | | "directly in the root" not required | 4: the link one level down (walk and metadata), the table, the walk below a trusted link | | the link's owner not checked | only the two `link_uid` tests | | the root's owner not checked | only the two `root_uid` tests | | the root's mode not checked | only `a_trusted_root_link_in_a_group_writable_root_is_refused` | | the exception removed altogether | 3: the link let through, the table, the walk below it | | `destination_metadata` following any symlink | the two tests that expect an untrusted link to still read as a link | - [ ] Windows: a cross `cargo check` needs a mingw C compiler I do not have installed. This pull request's Windows job is the proof that it compiles there ## Checklist - [x] Targets `develop` - [x] Commits are signed off (DCO, `git commit -s`) - [x] Commits are signed - [x] Labels applied - [x] No secrets, keys or credentials in code, logs or fixtures - [x] Docs updated (`docs/security-model.md`) - [x] `bench/binary-budget-mib`: rebased on `develop` after #1785 raised the ceiling to 8 MiB ## Related issues Refs #1764. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rustlsfrom 0.23.42 to 0.23.45, which is the first version patched for RUSTSEC-2026-0285 (GHSA-2mjx-qc3c-rqvc).rustls-webpkimoves with it, 0.103.13 to 0.103.15, because the resolver takes them together.bench/binary-budget-mibfrom 7.5 to 8, in its own commit, because this patch is what tips the binary over the old ceiling.Refs #1783.
Why
cargo auditandcargo denywent red on #1781, a pull request that does not touch rustls, because any pull request that changesCargo.lockgets the lockfile audited anddevelophas carried the affected version since before the advisory was published on 2026-09-14.maincarries it too, so v5.9.3 shipped with it.The exposure here is low: rustls is reached only through
ureq, client side, by the self-updater, the advisory leaves the handshake transcript authenticated, and every asset the updater downloads is checked against the release signature afterwards. The reason to move now is that the audit gate stays red for every dependency pull request until the lockfile does.Changes
cargo update -p rustls, nothing else:The binary budget
developbuilds to 7,863,856 bytes, 464 under the 7.5 MiB ceiling (7,864,320) that has not moved since #1320 on 2026-08-03. With rustls 0.23.45 the binary is 7,864,880 bytes: 1,024 more thandevelopand 560 over. The first run of this pull request failedRelease binary fits its budgeton exactly that.The growth is explained, it is a patched TLS library and nothing else changed, so by the check's own wording the ceiling moves with the reason in the commit message. I took it to 8 MiB rather than 7.6:
developwas already sitting against the ceiling, so every fix that adds code would have met it next, and 8 MiB still leaves only about 512 KiB, which a heavyweight dependency does not fit in.Test plan
cargo auditlocally against the advisory database of today (1251 advisories): no vulnerability foundcargo deny check advisories:advisories okcargo test --lib update::passes locally, which is the module that reaches rustlsChecklist
developgit commit -s)Related issues
Refs #1783. I will close it by hand once this lands on
develop.Signed-off-by: Jaro-c 75870284+Jaro-c@users.noreply.github.com