This is an automated email from the ASF dual-hosted git repository. FreeAndNil pushed a commit to branch Feature/security-audit-hardening in repository https://gitbox.apache.org/repos/asf/logging-log4net.git
commit 9cc34d29e31b985e8e7d028220d9235c83fc9c7a Author: Jan Friedrich <[email protected]> AuthorDate: Mon Aug 17 23:30:16 2026 +0200 make the release verification scripts fail closed verify-release.ps1 reported a SHA-512 mismatch with -ErrorAction Continue, which overrides the script level $ErrorActionPreference, and never checked the exit code of gpg, because $ErrorActionPreference does not apply to native commands. A tampered artifact or a broken signature therefore still reached Expand-Archive and the script exited 0. build-release.ps1 already sets $PSNativeCommandUseErrorActionPreference with a comment explaining this, so the trap was known. Both scripts looped over whichever .asc files happened to be present, so deleting them left nothing to verify and the scripts succeeded. The .sha512 files travel with the artifacts and can be regenerated, so they add nothing on their own. Verification is now driven from the artifacts: everything that is not a hash, a signature or KEYS must have both, and an empty directory is an error. That also catches a file added to the release. Both scripts now import KEYS into a key ring of their own. Importing into the default one accepted a signature from any key the machine already trusted rather than only from a key in the Logging Services KEYS file. Checked against a synthetic release signed with a throwaway key. Before, the PowerShell script accepted a tampered artifact, a corrupted signature, a deleted signature, an added unsigned file and a signature from an untrusted key, and the shell script accepted the last three. After, all of them are rejected and an untampered release still passes. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- scripts/verify-release.ps1 | 78 +++++++++++++++------- scripts/verify-release.sh | 50 ++++++++++++-- .../3.4.0/309-verify-release-fails-closed.xml | 16 +++++ 3 files changed, 113 insertions(+), 31 deletions(-) diff --git a/scripts/verify-release.ps1 b/scripts/verify-release.ps1 index f99e4276..46e0ca02 100644 --- a/scripts/verify-release.ps1 +++ b/scripts/verify-release.ps1 @@ -5,52 +5,82 @@ Param ( Set-StrictMode -Version Latest $ErrorActionPreference = 'Stop' +# $ErrorActionPreference alone does not apply to native commands: gpg only sets $LASTEXITCODE, so +# without this a failed signature check would still reach the extraction at the end and the script +# would exit 0. Requires PowerShell 7.3+. +$PSNativeCommandUseErrorActionPreference = $true + if (!$Directory) { $Directory = $PSScriptRoot } -function Verify-Hash +function Assert-Hash { param ( - [Parameter(Mandatory=$true, HelpMessage='The file containing the hash.')] + [Parameter(Mandatory=$true, HelpMessage='The artifact to check.')] [System.IO.FileInfo]$File - ) - $Line = @(Get-Content $File.FullName)[0] - $Fields = $Line -split '\s+' - $Hash = $Fields[0].Trim().ToUpper() - $Filename = $Fields[1].Trim() - if ($Filename.StartsWith("*")) - { - $Filename = $Filename.Substring(1).Trim() - } - - $ComputedHash = (Get-FileHash -Algorithm 'SHA512' "$($File.DirectoryName)/$Filename").Hash.ToUpperInvariant() + ) - if($Hash -eq $ComputedHash) + $HashFile = "$($File.FullName).sha512" + if (!(Test-Path $HashFile)) { - "$($Filename): Passed" + throw "$($File.Name): no $($File.Name).sha512 to check it against" } - else + + $Hash = (@(Get-Content $HashFile)[0] -split '\s+')[0].Trim().ToUpperInvariant() + $ComputedHash = (Get-FileHash -Algorithm 'SHA512' $File.FullName).Hash.ToUpperInvariant() + if ($Hash -ne $ComputedHash) { - Write-Error "$($Filename): Not Passed" -ErrorAction Continue - Write-Error "Read from file: $Hash" -ErrorAction Continue - Write-Error "Computed: $ComputedHash" -ErrorAction Continue + throw "$($File.Name): SHA-512 mismatch, read $Hash but computed $ComputedHash" } + + "$($File.Name): hash ok" +} + +# Everything that is not a hash, a signature or the key file has to be covered by both. Driving the +# checks from the artifacts, rather than from the .sha512 and .asc files that happen to be present, +# is what turns a missing signature into a failure instead of one loop iteration fewer. +$Artifacts = @(Get-ChildItem $Directory -File | + Where-Object { $_.Extension -notin '.asc', '.sha512' -and $_.Name -ne 'KEYS' }) + +if ($Artifacts.Count -eq 0) +{ + throw "No artifacts to verify in $Directory" } -foreach ($File in Get-ChildItem $Directory *.sha512) +foreach ($Artifact in $Artifacts) { - Verify-Hash $File + Assert-Hash $Artifact } Invoke-WebRequest https://downloads.apache.org/logging/KEYS -OutFile $Directory/KEYS -gpg --import -q $Directory/KEYS -foreach ($File in Get-ChildItem $Directory *.asc) +# 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 +{ + $Keyring = Join-Path $KeyringDirectory 'logging-keys.gpg' + gpg --no-default-keyring --keyring $Keyring --batch --quiet --import $Directory/KEYS + + foreach ($Artifact in $Artifacts) + { + $Signature = "$($Artifact.FullName).asc" + if (!(Test-Path $Signature)) + { + throw "$($Artifact.Name): no $($Artifact.Name).asc to verify it with" + } + + gpg --no-default-keyring --keyring $Keyring --batch --verify $Signature $Artifact.FullName + "$($Artifact.Name): signature ok" + } +} +finally { - gpg --verify $File + Remove-Item $KeyringDirectory -Recurse -Force -ErrorAction SilentlyContinue } Expand-Archive $Directory/*source*.zip -DestinationPath $Directory/src diff --git a/scripts/verify-release.sh b/scripts/verify-release.sh index d1b3b9b8..70bdd1ac 100644 --- a/scripts/verify-release.sh +++ b/scripts/verify-release.sh @@ -1,26 +1,62 @@ #!/bin/bash -set -e +set -euo pipefail if ! which unzip >/dev/null 2>&1; then echo "The 'unzip' utility is required, but was not found in your path" >&2 exit 1 fi -TARGET_DIR="$1"; +TARGET_DIR="${1:-}" if test -z "$TARGET_DIR"; then TARGET_DIR="$(pwd)" fi +cd "$TARGET_DIR" -sha512sum --check *.sha512 +# Everything that is not a hash, a signature or the key file has to be covered by both. Driving the +# checks from the artifacts, rather than from the .sha512 and .asc files that happen to be present, +# is what turns a missing signature into a failure instead of one loop iteration fewer. +shopt -s nullglob +artifacts=() +for file in *; do + case "$file" in + *.asc|*.sha512|KEYS) continue ;; + esac + test -f "$file" || continue + artifacts+=("$file") +done + +if test ${#artifacts[@]} -eq 0; then + echo "No artifacts to verify in $TARGET_DIR" >&2 + exit 1 +fi + +for file in "${artifacts[@]}"; do + if test ! -f "$file.sha512"; then + echo "$file: no $file.sha512 to check it against" >&2 + exit 1 + fi + sha512sum --check "$file.sha512" +done wget https://downloads.apache.org/logging/KEYS -gpg --import -q KEYS -for f in $(find "$TARGET_DIR" -iname '*.asc'); do - gpg --verify "$f" + +# 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. +keyring_dir="$(mktemp -d)" +trap 'rm -rf "$keyring_dir"' EXIT +gpg --no-default-keyring --keyring "$keyring_dir/logging-keys.gpg" --batch --quiet --import KEYS + +for file in "${artifacts[@]}"; do + if test ! -f "$file.asc"; then + echo "$file: no $file.asc to verify it with" >&2 + exit 1 + fi + gpg --no-default-keyring --keyring "$keyring_dir/logging-keys.gpg" --batch --verify "$file.asc" "$file" done mkdir -p src cd src unzip -q -o ../*source*.zip -cd src \ No newline at end of file +cd src diff --git a/src/changelog/3.4.0/309-verify-release-fails-closed.xml b/src/changelog/3.4.0/309-verify-release-fails-closed.xml new file mode 100644 index 00000000..8f6f02a0 --- /dev/null +++ b/src/changelog/3.4.0/309-verify-release-fails-closed.xml @@ -0,0 +1,16 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xmlns="https://logging.apache.org/xml/ns"; + xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"; + type="fixed"> + <issue id="309" link="https://github.com/apache/logging-log4net/pull/309"/> + <description format="asciidoc"> + make the release verification scripts fail closed. `verify-release.ps1` reported a SHA-512 + mismatch with `-ErrorAction Continue` and never checked the exit code of `gpg`, so it extracted the + archive and exited 0 for a tampered artifact or a broken signature. Both scripts looped over + whichever `.asc` files were present, so deleting them left nothing to verify and the scripts + succeeded. The checks are now driven from the artifacts, each of which must have a `.sha512` and a + `.asc`, and the keys are imported into a key ring of their own so that a signature from any other + key this machine trusts is no longer accepted (audit 1231d72-f009, 1231d72-f010) + </description> +</entry>
