FreeAndNil commented on code in PR #313: URL: https://github.com/apache/logging-log4net/pull/313#discussion_r3917815946
########## scripts/verify-release.ps1: ########## @@ -55,16 +59,20 @@ foreach ($Artifact in $Artifacts) Assert-Hash $Artifact } -Invoke-WebRequest https://downloads.apache.org/logging/KEYS -OutFile $Directory/KEYS - # A key ring of its own, holding only the downloaded KEYS. Importing into the default key ring # would accept a signature from any key this machine already has, not only from a key in the # Logging Services KEYS file. $KeyringDirectory = New-Item -ItemType Directory -Path (Join-Path ([System.IO.Path]::GetTempPath()) ([guid]::NewGuid())) try { + # Downloaded into that directory, and never read from the one being verified: a KEYS file sitting + # next to the artifacts is not covered by any of the checks above, so importing it would let + # anyone who can place a file there have their own key accepted as a release key. + $Keys = Join-Path $KeyringDirectory 'KEYS' + Invoke-WebRequest https://downloads.apache.org/logging/KEYS -OutFile $Keys + $Keyring = Join-Path $KeyringDirectory 'logging-keys.gpg' - gpg --no-default-keyring --keyring $Keyring --batch --quiet --import $Directory/KEYS + gpg --no-default-keyring --keyring $Keyring --batch --quiet --import $Keys Review Comment: Confirmed, and it is a bypass rather than a warning. With keyboxd enabled and a decoy signed by a key already in the keybox: the old script accepted it, exit 0, and the downloaded KEYS went into the real key store. With a throwaway GNUPGHOME it is rejected, exit 2. Fixed with GNUPGHOME instead of --keyring. keyboxd is per homedir too, so nothing needs disabling. Both defects you found in the change are fixed: the export masking a failed mktemp, and gpgconf throwing inside the finally. One correction: the trap is not safe by accident. Under set -e a failing command in a trap aborts the rest of it, so the shell script had the same defect, exit 127 with the cleanup skipped. Guarded with || true. Also added Pester tests for the stages needing neither network nor gpg, run in CI. Fixed with https://github.com/apache/logging-log4net/pull/313/commits/2e5cc8319de32ddbdb9b46ca1dbd7e1389e8d443 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
