URGENT Security Advisory 2021-12-11 (re Apache Log4j 2)

A full release is now available for Platform 2.4.2.

2 Likes

Thanks for addressing this @grace

@ibacher can we do some penetrative tests on 2.5.0-beta.4 ? as at Log4Shell: RCE 0-day exploit found in log4j 2, a popular Java logging package | LunaSec

I can also just find the release at [maven-release-plugin] prepare for next development iteration · openmrs/openmrs-core@c4c182f · GitHub , but can’t find the specific changes. Can you point me to the commit ?

The commit that fixes this is this one. Log4J 2.15.0 disables the problematic part of code by default and adds some guards which should prevent the same exploit from re-appearing.

1 Like

Thanks @ibacher Did you try to do Exploits on the previous server versions including 2.5.0-beta.3 ?

No, but it should absolutely be regarded as vulnerable (the problem with being a “platform” in these circumstances is even if there were no way to exploit anything in core, any module might expose the vulnerability).

1 Like

Thanks, was basically asking to see if you have than that bit testing so i just proceed with final release.

Update: We are aware that a second Log4J vulnerability has been discovered (article link). However, this one is deemed much narrower than the original CVE and far less critical.

The plan is to upgrade 2.5.0, update 2.4.x and release 2.4.3.

@ibacher will keep us posted here.

@ibacher sure of the log4j version we need to upgrade to before we do another release ?

@tendomart I already committed the update to the 2.5.x core branch, so it should just be a matter of creating the releases.

4 Likes

Thanks Ian.

Alright, Core 2.4.3 (WAR only) is now available. Platform 2.4.3 (full release) is now available. RefApp 2.12.2 is now available.

2 Likes

Hi @ibacher Can I request you to make a release on Core 2.1.1(WAR only) with the latest log4j 2.17.1 version. Core 2.1.1 is being used for 0.92 and 0.93 versions of Bahmni and this release is critical for us to upgrade the log4j on all Bahmni repo’s. We will raise the PR with latest log4j 2.17.1 version if it helps.

@angshuonline @gsluthra @n0man @rohit.yawalkar @buvaneswariarun @tarunshettygari @swedhan @grace @supriyam @sivareddy

Hi @binduak!

Unfortunately, it’s not quite as easy as that. The changes to get OpenMRS to be able to use Log4J2 in place of Log4J1 were fairly extensive and need re-evaluation for how they apply to Core 2.1.x. I.e., this wasn’t simply a matter of swapping out libraries.

At the heart of it there are two issues:

  1. OpenMRS allows (some) logging configuration to be driven by global properties (log.level, log.layout, and log.location). Support for this is implemented in a somewhat invasive way. Replicating this with Log4J2, which uses a fundamentally different configuration system was a challenge and, in retrospect, I didn’t do things quite correctly hence, this PR I’m actively working on.
  2. OpenMRS uses an appender that stores log messages in-memory. This needed to be completely re-written. (This is called the MemoryAppender)

So the actual implementation of Log4J2 in OpenMRS is spread across several commits:

  • a78c140 (Base implementation)
  • 6e9d0ca (Bugfix)
  • 1a659a8 (Liquibase for some reason embedded Logback)
  • 9d2489b (Re-implementation of MemoryAppender)
  • 6b0651b (Re-wiring the StartupFilter to use the new MemoryAppender)

Additionally, new commits were needed to the various modules that leverage the memory appender, notable the REST module:

  • ecc2edd (Initial implementation)
  • 3e05209 (A more reliable way of getting the MemoryAppender)
  • 7a1be48 (Leveraging changes in 6b0651b to more easily get the MemoryAppender)

And the Legacy UI module:

  • a926a70 (Added a hack to be able to load messages from the MemoryAppender via reflection on appropriate versions of OpenMRS)

I don’t think these changes can just be backported and released as an existing version, because we’d break things for anyone who was using a non-updated 2.1.1. Alternatively, I suppose, alternative pathways could be devised to load the MemoryAppender in 2.1.1 depending on the presence of a getMemoryAppender() method, but that’s not built into the existing PRs and would need to be new development work.

The easiest path forward I can see is to get Bahmni running against Core 2.1.4 (the changes for which can be found here… I haven’t extensively reviewed them). Then we could maybe work on backporting the Log4J2 patches to the 2.1.x branch and release that as 2.1.5 at least that way we can make suitable changes to at least the REST API to be able to load the server log or, if that’s not a feature Bahmni uses, I suppose we could skip it altogether.

I’m, of course, willing to supply whatever support and guidance I can on backporting these patches, but I likely don’t have enough time to take it on as a project myself. It might also be easier to look at the state of things once TRUNK-6052 gets merged in as I think that makes a lot of useful changes that should’ve been part of the initial implementation (hopefully it will be ready by the end of the week).

2 Likes

I’ve merged TRUNK-6052 in this commit. As I said above, this is kind of a key feature that really should’ve been in 2.4.0 from the beginning.

3 Likes

Thanks much @ibacher for the detailed commits list. This did help us a lot to start with the log4j changes on 2.1.4 OMRS version. We are still working on the log4j changes for both legacy UI and REST modules.

1 Like

Thanks so much @ibacher for all the effort and the detailed & well-reasoned description.

If the Bahmni team can get these changes adapted for and applied to 2.1.x, do we think it might make it a little easier to forward-port those changes to 2.2 & 2.3?

1 Like

@burke Yes… forward-porting is usually more straight-forward that backporting.

The bigger issue around this is that this change will definitely break code (which is why I didn’t initially backport it). E.g., this code from Iniz, or this code from PIH. IIRC, calls to setLevel() should continue to work, but anything that attempts to create an appender is either a no-op (like logger.addAppender()) or will throw an exception, e.g., by attempting to create a new non-console appender.

1 Like

Hi @ibacher We have raised PR’s with log4j2 changes on both openmrs-core (on 2.1.x) and LegacyUI (on 1.9.0) repositories. Please find below the PR links

openmrs-core TRUNK-6057: log4j2.x changes in 2.1.x by sivareddytw · Pull Request #3979 · openmrs/openmrs-core · GitHub

LegacyUI LUI-184 : Changes for log4j2.x by sivareddytw · Pull Request #177 · openmrs/openmrs-module-legacyui · GitHub

Requesting you to review the PR’s. Let us know if it requires any changes. Thanks !

@angshuonline @gsluthra @n0man @mksd @sivareddy @buvaneswariarun @rohit.yawalkar @deepthi

1 Like

@binduak @sivareddy Thanks for all the work on this. I’ve merged the PR for core and updated the LegacyUI PR with my suggested changes. I’m hoping to release 2.1.5 today, but there’s an additional security-related change I’d like to get in before we do so.

I’d also recommend taking a look at TRUNK-6052 at some point in the future. This allows OpenMRS’s logging to be configured via an external log4j2.xml file, which is somewhat challenging the way the log4j2 (and the log4j stuff before it) is written. But this is absolutely not critical.

2 Likes

Core 2.1.5 has been released with the Log4J2 changes. Thanks to the Bahmni team for the work on this.

@binduak @angshuonline @gsluthra @n0man @mksd @sivareddy @buvaneswariarun @rohit.yawalkar @deepthi

6 Likes