fix(signing): check the file bsign produced instead of its exit code - #336
Merged
Merged
Conversation
bsign returns 0 for a file it rewrote but can no longer hash, which happens once another section sits behind its own signature section, so a binary that fails its own hash check shipped silently. Run the check after signing and fail the build when it does not pass. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
Without -E, bsign also checks absent xattr and detached hashes and returns 64 even when the ELF hash is good. Assert the exact sign/check command sequence and report exit 73 as a tampered signature section. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
The stub needs POSIX sh and executable-script semantics, which !windows does not guarantee. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
A GPG signature shorter than the section bsign reserves leaves the remaining bytes non-zero, and bsign reports that instead of ever checking the hash, for a sound file as much as for a corrupt one. Every e2e build with an RSA key failed on it. Log that case and keep failing when bsign does report the hash. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
bsign rewrites the signature on every run, so an image that imports an already signed binary produced different bytes on every build. Skip signing when bsign reports a sound hash signed by the key about to sign it. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
A suffix of the fingerprint is not enough: any shorter string could match one. Document why a signature bsign cannot describe is signed again instead. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
This was referenced Sep 22, 2026
alexey-igrychev
added a commit
that referenced
this pull request
Sep 22, 2026
Backport only the bsign protections from #336 to v2.74.2-dk. This line uses the older objcopy-based in-house signer, whose repeated signing is already byte-stable, so no SDK or elfedit update is included. - skip bsign when the current hash is sound and the signer matches - verify the ELF hash after bsign writes the file Validation: - `go mod verify` - `go test -count=1 -tags="dfrunsecurity dfrunnetwork dfrunmount dfssh containers_image_openpgp" ./pkg/build/signing/` - `go vet -tags="dfrunsecurity dfrunnetwork dfrunmount dfssh containers_image_openpgp" ./pkg/build/signing/` Prepared for review; do not merge automatically. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
alexey-igrychev
added a commit
that referenced
this pull request
Sep 22, 2026
Backport the ELF signing guards from #336 to the v2 line and update delivery-kit-sdk to v1.4.2. - skip bsign when the current hash is sound and the signer matches - verify the ELF hash after bsign writes the file - make in-house signing a no-op for an equivalent existing signature - use elfedit v0.1.1 for safe same-sized section replacement Validation: - `go mod verify` - `go test -count=1 -tags="dfrunsecurity dfrunnetwork dfrunmount dfssh containers_image_openpgp" ./pkg/build/signing/` - `go vet -tags="dfrunsecurity dfrunnetwork dfrunmount dfssh containers_image_openpgp" ./pkg/build/signing/` Prepared for review; do not merge automatically. --------- Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
alexey-igrychev
pushed a commit
that referenced
this pull request
Sep 22, 2026
🤖 I have created a release *beep* *boop* --- ## [3.4.0-dk.3](v3.4.0-dk.2...v3.4.0-dk.3) (2026-09-22) ### Bug Fixes * **sbom, build:** report scanner exit code and output when SBOM scan fails ([#334](#334)) ([f58b505](f58b505)) * **sbom:** report the pm package catalogue version the build installed with ([#320](#320)) ([7384b6a](7384b6a)) * **signing:** check the file bsign produced instead of its exit code ([#336](#336)) ([5515529](5515529)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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.
signELFFileonly checked bsign's signing exit code. Astra bsign 1.4.2 can return 0 after rewriting a file that immediately fails its own hash check, so a corrupt ELF shipped silently.After
bsign -N -s, delivery-kit now runsbsign -cEand fails the build when the ELF-section hash does not pass.-Eis required: without it bsign also probes absent xattr/detached stores and returns 64 even for a good ELF hash.Measured on the incident binaries:
Exit 73 (non-zero unused signature-section bytes) now has a descriptive error instead of
unknown exit code. That condition is intentionally build-fatal; bsign also cannot verify those files.Stub tests assert the exact ordered argv sequence and same path for signing/checking, plus failures 66 and 73. Package unit/vet pass. Related root-cause protections: deckhouse/delivery-kit-sdk#109 and deckhouse/elfedit#4.