Remove deprecated dependencies to satisfy security scanners

[ppkarwasz]

This is a PoC on how to remove deprecated libraries from the POM file (or move them to the test scope) to appease some primitive security scanners.

The trick is to extract classes/methods from the Avalon, LogKit and Log4j 1.x libraries that are used in the Commons Logging code and put them in an additional source code directory src/main/dummy.

Remark: The source files in src/main/dummy are not included in the any Commons Logging artifact. They are only used by the compiler to include the correct signatures in the class files.

Motivation

From a developer perspective the change is useless and the new artifacts should be identical to those before this change (except the embedded pom.xml, module-info.class and the aesthetic change in Log4JLog).

However many developers struggle to explain to their security experts that having log4j:log4j somewhere in a POM file is not a problem (cf. many questions on SO). This is also in line with #231.

[jochenw]

Question: Wouldn't it be sufficient to change the scope for those dependencies to "provided"?

[ppkarwasz]

@jochenw,

I don't have access to those so-called "security" scanners.

As far as I can test using commons-logging 1.3.0 does not even download the POM file of log4j:log4j, logkit:logkit or any other optional dependency. @garydgregory, IIRC you had problems with some security scanners (which motivated you to introduce #231). Can you check if removing logkit:logkit from the POM helps?

This PR certainly tricks the CycloneDX plugin to remove logkit:logkit from the SBOM, but moving it to provided and excluding the provided scope from the CycloneDX SBOM would have the same effect.

[garydgregory]

@ppkarwasz
Whatever I had noticed before was through a scan from at least one vendor we use at my day job and there is no way to test snapshot or local builds. Well, maybe there is but I don't want to take the day (or days) it would take to setup that scanning and static analysis stack locally or whatever it would take.

[ppkarwasz]

@garydgregory,

Let's wait for the 1.3.1 release then. If the work you did on log4j:log4j alleviates the problems you had with security scanners, we might consider this PR, otherwise I'll close it.

Remark: many "security" scanners treat log4j differently from other dependencies, e.g. GraalVM gives me this nice message, when I use a snapshot of Log4j:

Warning: The log4j library has been detected, but the version is unavailable. Due to Log4Shell, please ensure log4j is at version 2.17.1 or later.

It doesn't have the same problem with oro:oro that hasn't been maintained in two decades.

[garydgregory]

@ppkarwasz
FYI, the release vote for 1.3.1 is here: https://lists.apache.org/thread/j4kfrdopkv8okroymbv86m1w40czgzys

[ppkarwasz]

@garydgregory,

Does you security scanner at work still complain about Log4j 1, Avalon or LogKit?

[slachiewicz]

if someone would fork this repo and then on the personal fork, enable security/dependabot scanners - more is visible.
Here in this repo - the security tab is empty unfortunately.

[raboof]

if someone would fork this repo and then on the personal fork, enable security/dependabot scanners - more is visible. Here in this repo - the security tab is empty unfortunately.

Hmm, that is probably a rights issue: I do see the dependabot results. I'm not sure if GitHub has fine-grained controls to expose that, though.

Dependabot is currently only reporting on the logback issues for which #329, #332 and #335 are open.

[ppkarwasz]

I am not sure what permission someone needs to have, but there are 5 closed Dependabot alerts regarding log4j:log4j. I think we should take these head on and:

• Instead of tricking scanners to believe we don't depend on Log4j 1, restore the optional dependency of commons-logging on log4j:log4j.
• Dismiss all security alerts with a meaningful message.

All such alerts are bogus because:

• From our perspective (unit tests), we don't use any Log4j 1 component covered by a CVE. The vulnerable code is not reachable.
• From a consumer perspective the dependency is optional, so it is up to the consumer to choose if he adds it or not.
• The simple presence of log4j:log4j on the classpath of an application is not enough for Commons Logging to use it. To enable Log4j 1 support the user must set the org.apache.commons.logging.Log system property to an appropriate value. Only these legacy implementation are detected using the classpath:
  

  commons-logging/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java

  Lines 142 to 153 in 26e7dc2

  	/**
  	* The names of classes that will be tried (in order) as logging
  	* adapters. Each class is expected to implement the Log interface,
  	* and to throw NoClassDefFound or ExceptionInInitializerError when
  	* loaded if the underlying logging library is not available. Any
  	* other error indicates that the underlying logging library is available
  	* but broken/unusable for some reason.
  	*/
  	private static final String[] classesToDiscover = {
  	LOGGING_IMPL_JDK14_LOGGER,
  	LOGGING_IMPL_SIMPLE_LOGGER
  	};

I am closing this PR, since we should rather stand our ground and ask Security Scanners to improve their code. The commons-logging artifact can not possible expose the user to any danger, regardless of the dependency manager (Gradle, SBT, Ivy, Maven) they use.

The CycloneDX SBOM distributed with commons-logging clearly states that all dependencies are optional and might not be present at runtime.