Mitigate potential Remote-Code-Execution caused by CVE-2021-44228 #1343
Conversation
Signed-off-by: Flole <[email protected]>
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/log4j-vulnerability/129863/27 |
|
@openhab/architecture-council I'd like to move the discussion from the forum to here. @rkoshak, I understand for the forum discussions that you have concerns about it, so I don't want to overrule you, but hope that we can come to a joint view on the topic in the AC. If I got you right, you see the risk that some fundamental logging features might break, right? Could this be checked by testing it in various setups? If so, we could decide to merge it in the snapshot, roll it out and if issues turn up to revert it again, wdyt? |
|
That is my primary concern. However, I think the risk of breaking the logging in the very short term is lower than the risk posed by the vulnerability. If nothing else, as shown in that thread, we should apply the fix to our Internet exposed demo server ASAP. We probably don't need logging there anyway. I think at least @Flole998 has tested the mitigation and not found the logging to have broken so we have at least one test under our belt. It's probably low risk that anything that will impact day-to-day users is pretty low. I plan on testing it myself when I can get to where I can actually see my logs. There might be some issues in DEBUG or TRACE logging hanging around that might crop up but I see no reason why we can't handle those as they arise. My biggest concern was that something in PAX or Karaf might break all logging if we disable all log4j2 Lookups. In the long term, once we have the patched library available, I would recommend reversing the parameter that disables all lookups and apply some of the new parameters that Apache has implemented to limit what protocols and what machines it can reach out to. There is no reason OH needs to be using these features by default. But long term I do have some concerns disabling all lookups forever as we don't really have full control over the logging config from end to end and upstream may start using them without our knowledge and input. |
|
I'd like to point out that I was able to do the first step of the exploit that connects to an LDAP Server on the openHAB Demo Installation at http://demo.openhab.org. While there is more involved into getting remote code execution in the currently published way (and modern java versions should block that) and while I am not disclosing yet how I've done it (and I haven't tried to do the other following steps as my intention was just to see if it could potentially be done and not to actually run code on that host), all internet-facing OH Installations that don't enforce authentication are potentially at risk currently. If someone comes up with a better way I wouldn't be mad at all if this doesn't get merged, it was the fastest fix I could come up with. In fact I'd prefer an updated karaf version, but there hasn't even be a timeframe announced yet as far as I can see, so maybe merge this, ignore the possible broken logging and delay the new release until karaf can be updated and make a new milestone and backport this. Or if someone wants to search through openHABs codebase it would even be possible to check if (and which) logging messages would break (something like Or maybe @wborn can do some of his magic and come up with a way to install the fixed version using bundle: install (and I believe there are config files to do that automagically), someone mentioned that it should be possible in the forum thread. |
I just did that! |
|
Ok, thanks @rkoshak, than I guess we are fine to merge. I'll wait for another member of the @openhab/architecture-council to comment, but if there's no veto, let's go for this as a short-term fix and remove it again once we have a proper fix at hand. Wdyt about doing a 3.1.1 patch release? Is it worth it or shall we just ask users to update to 3.2, once it is released on Dec 20? |
|
Given how slowly people upgrade I think w need a 3.1.1 patch. I wouldn't even rule out a 3.0.1 patch for something like this. It only take a quick browsing of Shodan to realize how many people foolishly have a naked openHAB exposed to the Internet. |
I can confirm that it no longer tries to connect to me when I do what I have done before, so it is fixed/mitigated there aswell. Maybe it's worth to run a quick "grep {" on the logs there, just to see if it broke anything there aswell.
Yes that is correct, I have verified that on my setup no "{" or "}" are in the log files and that I can no longer use the karaf-based test I described on the forum. So I have basically verified that I actually fixed what I wanted to fix and that I didn't break anything that I am using.
I agree with that, this was not designed as a "fix" but as a temporary "mitigation" (that's why it's named "Mitigate..." and not "Fix..."). The PAX Logging update was pushed out incredibly fast, in fact so fast that there was a solution a few hours before we even started discussing about it on the forum, so I was hoping that karaf would do such a fast update aswell as that would give a whole bunch of additional options on how to proceed (including my favourite: mitigate now, update karaf for the new release on december 20 and revert the mitigation again as then we have a proper fix). |
I checked shodan aswell but look at the versions there..... 2.5, 3.0M1..... You wouldn't reach those with a 3.1.1 or 3.0.1, I think they just leave their instances running and forget about them. I don't think they are afraid of moving from 3.0 to 3.1 and would prefer to move to 3.0.1 instead, so I am not sure if 3.0.1 would actually be installed. It's definitely a great approach to want to reach as many people as possible and patch as many instances at possible, but if someone doesn't want to update or simply forgets that they have a publicly available openHAB then they will not update, no matter how much effort you put into backporting the fix. I'd rather make an annoucement on the forum for 3.0 and 2.X runners that they should manually change their config files, as that can be done on ANY openHAB version, even if they are running some old milestone and don't want to upgrade for whatever reason. |
|
I'm not sure i'm seeing the downside to enabling "formatMsgNoLookups" ? What features would we be loosing ? It's a bit hard to google it right now, the internet is screaming about it as a fix, but not talking about what it disables exactly . |
That's definitely least effort, so I could go with this. Whoever wants to update only has to wait until Dec 20 and whoever wants to fix what they have right now, they can manually fix it themselves. |
That was not meant as an "instead of backporting to 3.1" but as an "and backporting to 3.1". The idea was to group users in 2 groups:
|
|
to answer my own question about what it disables |
|
Well, the non-tech group is usually also rather slow in doing updates, and if they blindly do the update, they will receive 3.2.0 on Dec 20 anyhow - so for just 10 days, the effort to do a 3.1.1 inbetween does not really seem justified. |
If (and I am not sure that exists in openHAB) you have something like |
|
I'm very certain that we don't use any such log constructs in openHAB... |
|
Yeah, this looks fine to me. |
|
Alright, shall we merge then? |
If that's the way to go then I would leave this open and hope for a Karaf update.... Merging this only makes sense if this is supposed to be backported aswell IMO. Unless you don't are about merging now and reverting in a week or so. On the other hand this would give additional testing before the release.... which might not be needed at all if Karaf is updated. I think there first needs to be a clear plan on what to do now, if forum annoucement plus fix on december 20 it is then merging this now doesn't really make sense (I'd wait and hope for a karaf update and then merge this in 2 days on the 13th if there isn't one until then). If that plus backporting is the plan then it would make sense to merge (and revert later on if Karaf is updated). Maybe @digitaldan and @rkoshak as AC members should vote on how to proceed? |
|
I do not really count on the Karaf update - as @wborn mentioned, the 4.3.4 seems to have some regressions, so even if the fix is added, the upgrade might be a risky one for us in the last minute before our release. We should imho rather look if we can do the Pax logging update with this magic exclusion mechanism. If we succeed with that, we can still rollback this PR. |
|
Since this is the fix that is available , i vote to merge it now. If karaf does implement a proper fix, then we can always revert this, but karaf upgrades always need testing so that could take time, |
|
As I understood @wborn he tested it with OH and it seems to work well. His post was:
|
2 AC members against me who doesn't have to say anything ;) Fair enough, feel free to merge :) I am working on writing a forum post to sum up the manual fix described in the forum right now. |
Oh, I read it as "does not seem to work well" as this is what @wborn usually writes after testing Karaf updates. 😆 |
Hey, you are the one who created the PR. I didn't yet see anyone complaining that his PR is getting accepted. 😝 |
|
I'd vote too to at least merge it now. If a better fix comes along before 3.2 so much the better. Based on my experience on the forum I'm not sure those who are slow to upgrade operate with quite the same motivations and decision making on when and how to upgrade. Indeed some user won't even know they are upgrading. Others are lazy about it. But there is a large contingent of users who deliberately do not update. Those users though would update to a 3.1.1 patch release for something like this though. What I can't say is how many of such users we have to make a judgement on how much extra work it's worth to back port it. |
I know 😂 Anyways, it's merged now and there is a plan on how to move forward, so too late to complain anyways and I am happy with that plan, I just wanted it clearly written down so everyone knows how this is handled now, before there were suggestions from you and me but no clear "this is how we do it". |
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/log4j-vulnerability/129863/44 |
|
I'm also OK with this quick fix! The vote for Karaf 4.3.4 was cancelled so the new release will also contain the Pax Logging fix. Yes so far Karaf 4.3.4 didn't have any issues while testing it. But it also doesn't have that many changes compared to 4.3.3. |
|
Sounds indeed as if we get Karaf 4.3.4 with the fix in time for our 3.2 release. 👍 |
Reverts openhab#1343 because Karaf 4.3.4 uses Pax Logging 2.0.11 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
|
I've created a few PRs so we can immediately update to Karaf 4.3.4 when it is released, see #1344. |
|
Sorry I missed that train - I'm all for the fix, and more generally actively tackling security vulnerabilities. 👍 On the patch update, I was more of the opinion that we should do them and have an easy official remediation for people still on the current supported stable release (upgrade to 3.1.1) instead of relying on people making the fix themselves - but I'm not personally making the effort so... :) In any case, I believe we should issue a proper advisory/CVE whatever the fix is. We should even do it ASAP, for instance as soon as 3.2RC1 is released and not wait until 3.2 GA as it'll be the first non-snapshot that is not affected. |
…enhab#1343) Signed-off-by: Flole <[email protected]>
Ok, I'm trying to do a 3.0.3 and 3.1.1 release with that fix - let's see how it works out... |
) Signed-off-by: Flole <[email protected]>
|
I succeeded. I cherry-picked this PR to 3.0.x and 3.1.x branches and created new releases 3.0.3 and 3.1.1 from it. |
) Signed-off-by: Flole <[email protected]>
Only half-succeeded. 😢 |
|
I see you updated the version to 3.0.4 in the advisory GHSA-j99j-qp89-pcfq, but the description still references 3.0.3:
|
|
Thx, fixed. |
Reverts openhab#1343 because Karaf 4.3.4 uses Pax Logging 2.0.11 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
Reverts openhab#1343 because Karaf 4.3.4 uses Pax Logging 2.0.11 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
Reverts openhab#1343 because Karaf 4.3.4 uses Pax Logging 2.0.11 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
Reverts openhab#1343 because Karaf 4.3.4 uses Pax Logging 2.0.11 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
Reverts openhab#1343 because Karaf 4.3.4 uses Pax Logging 2.0.12 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
* Reverts openhab#1343 because Pax Logging 2.0.12 is not vulnerable. * Excludes Pax Logging 2.0.10 to reduce archive size and to prevent scanner false positives. * Adds missing new line. Signed-off-by: Wouter Born <[email protected]>
* Reverts openhab#1343 because Pax Logging 2.0.12 is not vulnerable. * Excludes Pax Logging 2.0.10 to reduce archive size and to prevent scanner false positives. * Adds missing new line. Signed-off-by: Wouter Born <[email protected]>
* Upgrade to Karaf 4.3.4 * Syncs distro customizations with Karaf 4.3.4 * Resolves app runbundles for the new dependencies For release notes, see: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311140&version=12350547 Karaf 4.3.4 uses Pax Logging 2.0.12 which fixes CVE-2021-44228 and CVE-2021-45046. See also these advisories: * Log4j: GHSA-jfh8-c2jp-5v3q, GHSA-7rjr-3q55-vv33 * Pax Logging: GHSA-xxfh-x98p-j8fr Fixes #1334 * Revert CVE-2021-44228 quick fix Reverts #1343 because Karaf 4.3.4 uses Pax Logging 2.0.12 which is not vulnerable. Signed-off-by: Wouter Born <[email protected]>
This is a mitigation for the issue mentioned in https://community.openhab.org/t/log4j-vulnerability/129863 until Karaf has been updated. However, we could leave this in "forever" since I don't see any reason why openHAB should ever use JNDI in logging-messages, so this adds an additional protection in case there are more shady things going on in the jndi code in log4j.