You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "FreeAndNil (via GitHub)" <gi...@apache.org> on 2026/09/01 18:27:50 UTC
[PR] Harden the release scripts and connection string redaction (logging-log4net)
FreeAndNil opened a new pull request, #313:
URL: https://github.com/apache/logging-log4net/pull/313
Follow-ups from the second security scan, on top of 3.4.0.
- verify-release.sh/.ps1: KEYS was downloaded into the directory being
verified, so a planted KEYS file was the one imported and its keys were
accepted as release keys. Both scripts now download into a temporary
keyring of their own. Not a regression, the bare wget line dates from 3.2.0.
- The release scripts relied on $PSNativeCommandUseErrorActionPreference,
which only exists from PowerShell 7.4. On 5.1 the assignment was a silent
no-op and a failing gpg --verify was ignored. Added #Requires -Version 7.4,
and release-review.adoc now installs PowerShell 7 and uses pwsh.
- AdoNetAppender: connection string redaction hid password-bearing keywords,
which missed Extended Properties and AccessToken. Replaced with an
allowlist of diagnostic keywords.
- Removed graphify and its 26 MB of generated output from the tree.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden the release scripts and connection string redaction (logging-log4net)
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 commented on code in PR #313:
URL: https://github.com/apache/logging-log4net/pull/313#discussion_r3911781450
##########
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:
I recieved the following warning when using the script to verfiy 3.4.0
`gpg: Note: Specified keyrings are ignored due to option "use-keyboxd"`
It seems gpg then ignores the --keyring file
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden the release scripts and connection string redaction (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden the release scripts and connection string redaction (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil merged PR #313:
URL: https://github.com/apache/logging-log4net/pull/313
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden the release scripts and connection string redaction (logging-log4net)
Posted by "swebb2066 (via GitHub)" <gi...@apache.org>.
swebb2066 commented on code in PR #313:
URL: https://github.com/apache/logging-log4net/pull/313#discussion_r3911781450
##########
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:
I recieved the following wrning when using the script to verfiy 3.4.0
`gpg: Note: Specified keyrings are ignored due to option "use-keyboxd"`
It seems gpg then ignores the --keyring file
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden the release scripts and connection string redaction (logging-log4net)
Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
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:
Thanks a lot.
I fixed it 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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org