Cleaned-up log4j 1.2 that disables scary networking (base=1.2.17, fully binary compatible) #17
Conversation
git-svn-id: https://svn.apache.org/repos/asf/logging/log4j/tags/v1_2_17@1333769 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/logging/log4j/tags/v1_2_17_rc3@1334596 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/logging/log4j/tags/v1_2_17@1342873 13f79535-47bb-0310-9956-ffa450edef68
The new build requires JDK8 or later, and a JDK6 installed as a maven
toolchain.
Main changes include:
* switch to Maven 3, requiring JDK7 to invoke
* switch to latest (compatible) maven plugins, requiring JDK8 to invoke
* introduce two profiles: toolchain (default) and no-toolchain
* toolchain profile uses maven toolchains feature with JDK6
(can also use JDK7) and source/target version of 1.4,
for compatibility with JDK 1.4/5/6/7 and beyond
* no-toolchain profile uses default system JDK with
source/target version of 7,
for compatibility with JDK 8/9/10/11/17 and beyond
* stop building NTEventLogAppender.dll from maven by invoking ant
(instead use the checked-in version)
* replace some plugin use with resources section
* replace some ant tasks with maven clean plugin
* change all project metadata for correctness
* remove duplicate LICENSE and NOTICE files and use the main ones
* set default encoding to UTF-8 to silence some warnings
* enable running all unit tests instead of a whitelist
* change when and how site is generated
* customize site templating for modern site plugin
* remove unused and unsupported ant build files
* update INSTALL document to match changes
Sets up a matrix build using github actions that runs on every push. The following combinations all work: * Toolchain 1.6, JDK 11, OS ubuntu-18.04 * Toolchain 1.6, JDK 11, OS ubuntu-20.04 * Toolchain 1.6, JDK 11, OS windows-2019 * Toolchain 1.6, JDK 11, OS windows-2022 * Toolchain 1.7, JDK 11, OS ubuntu-18.04 * JDK 8, OS ubuntu-18.04 * JDK 11, OS ubuntu-18.04 * JDK 17, OS ubuntu-18.04 * JDK 8, OS ubuntu-20.04 * JDK 11, OS ubuntu-20.04 * JDK 17, OS ubuntu-20.04 * JDK 8, OS windows-2019 * JDK 11, OS windows-2019 * JDK 17, OS windows-2019 * JDK 8, OS windows-2022 * JDK 11, OS windows-2022 * JDK 17, OS windows-2022
Delete several integration-style tests that are broken. They are not being run by the main build (because they are broken).
Now that the main build runs all tests, remove the old way of running them with ant or bash.
MDCTestCase has some nasty reflection inside that is not allowed anymore on JDK >= 17. Detect and trap the error.
Changes most tests to use unique output files during their runs, so that when they run in parallel or out-of-order or interactively they do not cause test failures, reducing flakyness of test runs.
Hand-picked changes from trunk
* Remove outdated roadmap page * Update navigation to link to renamed mailing lists page * Remove link to non-existent wiki
These changes are incomplete because contributors cannot test the maven release plugin setup.
Disables: * JMSAppender * JMSSink * SimpleSocketServer * SocketHubAppender * SocketServer * SocketNode * TelnetAppender These classes now log an error instead of making network connections. Their external APIs remain intact to allow code using them to migrate without blowing up before runtime. Added tests for their remaining behavior. Adds warnings: * for SyslogAppender unless logging to a local loopback address * for SMTPAppender: * Does not warn about safe(r) use talking to the local loopback. * If using SMTPS, warn that host will not be verified (CVE-2020-9488). Note this commit does not actually fix the security issue. Instead it warns an upgrade is needed (or a change in configuration). * If using SMTP, warn about an unencrypted protocol. * deprecation warnings on disablec classes * deprecation for SocketAppender since it's the client side of the disabled server code
Disables: * jmx.Agent This class now throws an error instead of making network connections.
Disables: * JDBCAppender This class now logs an error instead of making network connections.
Disables networking in: * ExternallyRolledFileAppender Behavior reduces to be a normal non-rolling FileAppender.
Avoids trying to determine whether java >= 1.2 since if this version of the code loads java >= 1.4.
Disable a set of flaky integration tests that depend on specific JDK behavior (usually due to expecting a specific stack trace), when not running using the toolchain JDK.
|
A hard -1 Thank you for pitching in but this PR completely breaks major blocks of functionality. I see comments like "Changed in 1.2.18+ to complain about its use and do nothing else." for the JDBC Appender, Telnet Appender, ExternallyRolledFileAppender, JMS Appender, JMX, and more! 140 files changed? You might want to do one PR per CVE/issue. The PR says "binary compatible" but that's only because the code has been gutted which will break applications left and right. This PR is so far out-of-bounds that I have a hard time finding where to start. 1.x is End-Of-Life. If we do ANYTHING, it would be to address CVEs. We have CVE: CVE-2019-1757. We also have discussed security in the JMS Appender. So that would be two changes to do in two PRs IMO. This PR deals with neither in a meaningful manner. I see a disconnect between the email and the PR: "So as requested I've made a conservative fully binary compatible version of This PR contains ZERO security fixes, it just removes code. For me "conservative" means the least amount of changes, not gutting the code, changing 140 files, and breaking applications with no alternative. "Trying to have solid network code in 1.x while having it also be binary So you've gutted the code because doing real fixes is "too much work". There you have it I guess. I am not endorsing a 1.2.18 release but if there was consensus on the PMC for resurrection it should be ONLY to address IMO CVE-2019-1757 and the JMS Appender using the same techniques we used in 2.x. I'll leave it to each person to decide what is "too much work", just note that it took 6 of us working almost full time to work on 2.15.0, 2.16.0, 2.17.0, and 2.12.2. Keep in mind that this is a low-level library that must be a drop-in replacement in all settings from the largest enterprise to a hobbyist. Hence our choices of remedies in 2.x. |
| void warnInCaseOfInsecureSMTP() { | ||
| boolean local = false; | ||
| if (smtpHost != null) { | ||
| try { | ||
| if (InetAddress.getByName(smtpHost).isLoopbackAddress()) { | ||
| local = true; | ||
| } | ||
| } catch (UnknownHostException e) { | ||
| // hope it'll work out later | ||
| } | ||
| } | ||
| if (!local) { | ||
| if ("smtps".equals(smtpProtocol)) { | ||
| LogLog.warn("WARN-LOG4J-NETWORKING-REMOTE-SMTPS: logging to remote SMTPS host '" + | ||
| smtpHost + "'! Log4J 1.2 cannot verify the host, upgrade to Log4J 2!"); | ||
| } else { | ||
| LogLog.warn("WARN-LOG4J-NETWORKING-REMOTE-SMTP: logging to remote SMTP host '" + | ||
| smtpHost + "'! SMTP is an unencrypted protocol."); | ||
| } | ||
| } | ||
| } | ||
|
|
|
My $0.000002: Log4j 1.x reached end-of-life long ago - last updated a decade ago.
From 2019 in re CVE-2019-17571:
There's no rationale given here for bringing log4j 1.x back from the dead, and anyone who cares about security or project hygiene has long since moved on to log4j2 or logback. Assuming there are reasons, why not fork this and call it something else? Update: This applies to PR #18, too. I'm only a user — once upon a time of log4j 1.x — not a contributor, but I'm puzzled why there's not a clear statement on either PR justifying reversing the above-cited decisions. |
|
The rationale is not found here because most discussion about what should happen with apache projects happens on mailing lists. In this case, the discussion can be followed (and joined!) on dev@logging.apache.org, archives at https://lists.apache.org/list.html?dev@logging.apache.org , more info at https://logging.apache.org/log4j/2.x/mail-lists.html . |
|
Superceded by #18 |
|
See for a maintained/released fork with similar security fixes. |
Conservative version of earlier PR #16. This has full binary and source compatibility and starts of from 1.2.17, not including the non-released changes made many years ago after the release of 1.2.17.
Unlike PR16, this branch is neatly structured as a series of minimal mostly-idempotent patches, that could be accepted step by step:
cleanup-only patches
security fixes
bugfixes
Please note that while this PR is filed against trunk, it can't be merged directly because trunk has moved on from 1.2.17. Suggested approach is to make a bugfix 1.2.x branch starting from tag v1_2_17, and then apply these changes to it, and release 1.2.18 from there.