Say which shape of key would not sign a release - #226
Merged
Conversation
Refs #41. The signing step hands the RELEASE_SIGNING_KEY secret to ssh-keygen and, until now, let ssh-keygen's own refusal end the step. Two shapes of private key produce that refusal and neither of its messages names the cause: a key carrying a passphrase reports an incorrect passphrase, having read an empty one from a runner with no terminal to ask at, and a key whose line endings are CRLF reports a public key that does not exist, which points at a second file rather than at the bytes that are wrong. Both return 255, both mean the secret was set wrongly rather than the release built wrongly, and this step is the only place that distinction can be drawn. I measured both with throwaway keys against the step's own block rather than reasoning about them: Enter passphrase for ".../signing-key": Load key ".../signing-key": incorrect passphrase supplied to decrypt private key exit=255 Couldn't load public key .../signing-key: No such file or directory exit=255 The step now captures the code, names both shapes and returns 1. It also removes the key file on the failing path, which set -e skipped before: the same passphrase key run against the step as the default branch has it leaves the private half written to the runner's temporary directory. Nothing changes for a release whose secret is right. The same block signed a stand-in checksum file with a well-formed key and the signature verified with the two commands docs/release-notes-preamble.md hands a reader. Signed-off-by: Nils Lehnen <30603423+iderex@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
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.
Refs #41.
What was wrong
The signing step writes the
RELEASE_SIGNING_KEYsecret to a file and hands itto
ssh-keygen -Y sign. Wheressh-keygenrefuses,set -eended the step andthe log carried
ssh-keygen's own message and nothing else. Two shapes ofprivate key produce that refusal and neither message names its cause.
A key carrying a passphrase reports an incorrect passphrase, because a runner
has no terminal to ask at and an empty one is read instead:
A key whose line endings are CRLF, which is what a private half pasted through a
Windows editor carries, reports a public key that does not exist:
That second message is the expensive one. It names a second file and a missing
path, and the thing that is wrong is one byte per line in the file it was given.
Both are the secret set wrongly rather than the release built wrongly, and this
step is the only place in the run that can tell those two apart.
What the change does
It captures the exit code instead of letting
set -eend the step, removes thekey file on both paths, and refuses with a message naming the two shapes.
The key file removal is a second defect rather than a tidy-up. Against the step
as the default branch has it, the same passphrase key leaves the private half
written to the runner's temporary directory, because
rm -fsat after thecommand that failed:
With this change, on every one of the four cases below:
Proved by running it, not by reading it
I extracted the step's own
run:block from this file and from the defaultbranch and ran both against throwaway keys generated for the purpose. Nothing
here touched the account key, and no key material is in this change.
The passing row is the one that matters most, so it was checked end to end
rather than by its exit code. The block signed a stand-in
SHA256SUMSand thesignature verified with the two commands
docs/release-notes-preamble.mdhandsa reader:
The rehearsal that found it
I found this while walking the whole release path at
8d4c281, which had neverbeen run with a key present. Every step of
release.ymland both jobs ofsmoke.ymlwere executed against a throwaway local tag, deleted afterwards andnever pushed:
That is the first run in which the release side produced a signature and the
smoke side consumed the same one. The two had only ever been exercised
separately, against bytes written by hand.
The means
The change is a shell block inside a workflow file, because that is what the
thing being repaired is. No language is added and nothing new is depended on:
the step already ran
ssh-keygenand already tested an exit condition, and whatchanges is which condition it tests and what it says.
What this does not do
It does not finish #41. That issue is left on its first leg, a tag producing the
artefacts, and nothing in this tree reaches it.
It refuses no new class. What it repairs is a message, and the failure it
prevents is a person reading
Couldn't load public key ... No such file or directoryand looking for a missing file.Nothing here runs on a pull request. Both legs of this workflow reach a tag
only, so no check on this change executes the block it edits, and the evidence
above is two shells on this machine standing in for two steps on a runner. It
reproduces the working directory and the absence of a terminal, and nothing else
about one.
The rehearsal ran on Windows and the workflow runs on
ubuntu-latest. What thatbounds is the exact wording
ssh-keygenprints; the exit code and the shape ofthe failure are the same on both, and the message this step adds is written here
rather than read from
ssh-keygen.Reading
No second person read this change. It is one file, one step and 22 added lines,
and the evidence above stands in place of a reader rather than being offered as
equivalent to one.