Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Karaf 4.3.4 #1344

Merged
merged 2 commits into from
Dec 18, 2021
Merged

Upgrade to Karaf 4.3.4 #1344

merged 2 commits into from
Dec 18, 2021

Conversation

wborn
Copy link
Member

@wborn wborn commented Dec 11, 2021

  • 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:

Fixes #1334


Depends on:

@kaikreuzer
Copy link
Member

Thanks for the preparation - let's hope we see the release soon :-)

@wborn wborn added awaiting external dependency Depends on a new version of an external dependency awaiting other PR Depends on another PR security labels Dec 11, 2021
@wborn wborn added the enhancement An enhancement or new feature label Dec 13, 2021
@Flole998
Copy link
Member

Just for information and maybe this will be relevant for release planning aswell: Looking at http://mail-archives.apache.org/mod_mbox/karaf-dev/202112.mbox/browser the release is currently up for vote now and will stay in this state for at least 72 hours. That means the vote will close on the (GMT) afternoon of the 16th at earliest.

@kaikreuzer
Copy link
Member

Sounds tight, but I hope we'll make it. If not, we'll have to stick to the workaround.

@wborn
Copy link
Member Author

wborn commented Dec 14, 2021

It will be even tighter now the vote is cancelled again. 😉

http://mail-archives.apache.org/mod_mbox/karaf-dev/202112.mbox/%3Cd530560e-a7c0-0e09-0c9e-ef5cb95a3d0f%40nanthrax.net%3E

@J-N-K
Copy link
Member

J-N-K commented Dec 14, 2021

Since it also fixes #1334 it's probably worth waiting.

@Flole998
Copy link
Member

Waiting almost certainly means the release of OH will be delayed at this point.

The reason for the canceled vote is a new log4j version that (finally) disabled the jndi lookups completely.....

@wborn wborn force-pushed the karaf-4.3.4 branch 2 times, most recently from 94447d7 to 241656d Compare December 15, 2021 10:11
@J-N-K
Copy link
Member

J-N-K commented Dec 15, 2021

With regard to CVE-2021-45046 (and Karaf 4.3.4 fixing that issue, as it includes log4j 2.0.16), IMO 3.2.0 should not be shipped without this PR merged.

* 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 openhab#1334

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]>
@Flole998
Copy link
Member

With regard to CVE-2021-45046 (and Karaf 4.3.4 fixing that issue, as it includes log4j 2.0.16), IMO 3.2.0 should not be shipped without this PR merged.

I tend to agree, it looks like our mitigation is not enough at this point.... It seems to prevent most attacks but not all. To be honest I kinda expected that more will be uncovered once researchers dig deeper. Also while initially some Java versions were assumed to be safe, other attack methods have been discovered since then. Long story short: OH should still be safe from script kiddies that copy their exploit from GitHub, but not from someone who invests more time into a well-planned and well-targeted attack.

So at this point it might actually make sense to postpone this release to get the new karaf version in. Or if the timeframe between RC build and release can be shortened by a day then do that and hope that the new karaf release will be there on-time then. But I'm just saying this as someone who doesn't know what effort a new release actually means. It could very well be that Kai considered his own time schedule aswell when setting up the timeframe...

@Flole998
Copy link
Member

Just saw that a new vote has been started earlier today: http://mail-archives.apache.org/mod_mbox/karaf-dev/202112.mbox/%[email protected]%3e

It will (at earliest) be closed on the morning (GMT) of the 18th. Unless there are reasons to not do so I'd suggest making a RC later, making it available through the testing repository (so everyone who tests the milestones can also easily test the RC) and then ideally still before the 24th make the release.

@wborn
Copy link
Member Author

wborn commented Dec 15, 2021

Hey I made a distro build using the Karaf 4.3.4 thats currently on vote, so you can already check for issues while we wait for the official release!

Hopefully the Karaf committee also has a sense of urgency like they had at the Apache Log4j project where they shortened their normal 3 day voting period. 🙂

@kaikreuzer
Copy link
Member

Let's hope they don't cancel the vote once again 😬 : http://mail-archives.apache.org/mod_mbox/karaf-dev/202112.mbox/%3C588F464E-D546-4606-8DF7-21A2440A1E94%40gmail.com%3E

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-3-2-milestone-builds/125096/6

@J-N-K
Copy link
Member

J-N-K commented Dec 18, 2021

@wborn wborn marked this pull request as ready for review December 18, 2021 08:21
@wborn wborn requested a review from a team as a code owner December 18, 2021 08:21
@ghys
Copy link
Member

ghys commented Dec 18, 2021

Would it make sense to make a RC2 with Karaf 4.3.4 to make sure there aren't obvious regressions (or #1344 (comment) is enough)?

@wborn wborn removed the awaiting external dependency Depends on a new version of an external dependency label Dec 18, 2021
@kaikreuzer
Copy link
Member

@ghys I'd say we do a snapshot build with it asap and have a few people test it - that should imho be enough as I don't expect bigger issues through the Karaf patch update.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray 🎉

@kaikreuzer kaikreuzer merged commit 4ccfcbe into openhab:main Dec 18, 2021
@kaikreuzer kaikreuzer added this to the 3.2 milestone Dec 18, 2021
@kaikreuzer kaikreuzer removed the awaiting other PR Depends on another PR label Dec 18, 2021
@wborn wborn deleted the karaf-4.3.4 branch December 18, 2021 10:00
@J-N-K
Copy link
Member

J-N-K commented Dec 18, 2021

Let me know when it is available.

@kaikreuzer
Copy link
Member

https://ci.openhab.org/job/openHAB3-Distribution/2639/ should now include everything!

@J-N-K
Copy link
Member

J-N-K commented Dec 18, 2021

Looks good. Upgrade was working w/o issues on both, Mac and Debian. Also karaf related features work, no bundle reloading issues (expected because of openhab/openhab-core#2599, just checked to be sure it doesn't re-appear after the Karaf upgrade).

@jlaur
Copy link
Contributor

jlaur commented Dec 19, 2021

Just checking: What about CVE-2021-45105? Fixed in log4j 2.17.

@kaikreuzer
Copy link
Member

kaikreuzer commented Dec 19, 2021

The condition is

"When the logging configuration uses a non-default Pattern Layout with a Context Lookup (for example, $${ctx:loginId}), attackers with control over Thread Context Map (MDC) input data"

which I say does not apply in our case - so I'd say we should be fine with 2.16 and there is no need to postpone our release.
Let me know, if you have a different opinion!

@Flole998
Copy link
Member

The condition is

"When the logging configuration uses a non-default Pattern Layout with a Context Lookup (for example, $${ctx:loginId}), attackers with control over Thread Context Map (MDC) input data"

which I say does not apply in our case - so I'd say we should be fine with 2.16 and there is no need to postpone our release. Let me know, if you have a different opinion!

I'm looking into it right now. I'll report back what I found/haven't found later on.

@J-N-K
Copy link
Member

J-N-K commented Dec 19, 2021

I don't think this is blocking for openHAB. As @kaikreuzer said: that's a very special case that is not our standard configuration. It is required that someone changes the standard logging configuration to something I don't see a use-case for in openHAB. And then this instance needs to be exposed to the internet. I highly doubt that someone who tweaks the logging config in that way would do that.

@wborn
Copy link
Member Author

wborn commented Dec 19, 2021

Or just merge #1354. 😉

@kaikreuzer
Copy link
Member

Alright, that was quick, @wborn, thanks!
Let's honor @splatch's work and use this to directly update to log4j 2.17.

@Flole998
Copy link
Member

I've checked the CVE, understood the attack and what it requires in order to work. You need a log instruction with a string like ${ctx:version} in order to trigger it. OH does not use that context feature at all.

I wouldn't update log4j/PAX Logging at this point as we have a RC and fixing a bug that isn't even a bug for us is not necessary.

@kaikreuzer
Copy link
Member

Thanks for your analysis, @Flole998!
While I agree, I also think that there's currently a huge focus of everyone out there on log4j - most people won't go into details though and would rather come up with questions, why we only ship with 2.16 and not 2.17. So purely from a "marketing" perspective, I think it is better that we ship with the latest log4j version.
Wrt RC: We had some other fixes in it meanwhile, so this isn't the only last minute change anyhow...

@Flole998
Copy link
Member

I thought this would be the only change between RC and Release. If that's not the case then it might make perfect sense to include this as I don't expect the log4j CVEs to stop soon-ish and now with those changes merged updating it again will be super easy in case that's needed after the release (where a karaf update would be a rather huge change that might not be wanted).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature karaf security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Karaf client take a very long time/hangs if you to execute multiple commands on 3.2M3
7 participants