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 Karaf from 4.4.5 to 4.4.6 #4181

Merged
merged 2 commits into from
May 20, 2024
Merged

Upgrade Karaf from 4.4.5 to 4.4.6 #4181

merged 2 commits into from
May 20, 2024

Conversation

holgerfriedrich
Copy link
Member

@holgerfriedrich holgerfriedrich commented Apr 12, 2024

  • Sync runtime dependencies with Karaf 4.4.6, most notably:
    • Jetty 9.4.54.v20240208, addresses CVE-2024-22201
    • Pax Logging 2.2.7
    • Pax Web 8.0.27
    • ASM 9.7
    • BouncyCastle 1.77
  • Resolve itest runbundles

Karaf 4.4.6 has just been released, see changelog:
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311140&version=12354057

It includes the fixes for Jetty, but relies on ASM 9.7 (which does not match xtext release, which is still at 9.6).
As this is a security topic, I do not want to wait until xtext is released with the matching version of ASM.

This matches a recently published milestone release of xtext 2.35.0.M0M1.

Looking at the changelog of ASM, there might be a chance to replace the dependency.
I tried to exclude ASM 9.6 form all imported dependencies and put in ASM 9.7 instead. mvn dependency:tree does not show any 9.6 version anymore. Compilation itself succeeds.

Open points:

  • feature verification fails (conflicting ASM versions)
  • more testing, especially as xtext is a M0M1 milestone release

@wborn Do you have a recommendation how to pass the feature verification? I have not been successful, the docs are not very detailed and I did not find any matching post on the Karaf mail archive.

Fixes: openhab/openhab-distro#1641
Refs: openhab/openhab-addons#16676, openhab/openhab-webui#2547, openhab/openhab-distro#1649

@holgerfriedrich holgerfriedrich requested a review from a team as a code owner April 12, 2024 15:02
@andrewfg
Copy link
Contributor

@holgerfriedrich apropos Jetty..

We should consider ourselves fortunate that Jetty maintainers have patched Jetty 9.4.54.v20240208 to fix what is quite a dangerous security issue. However this may not be the case forever as Jetty 9.x.x has been deprecated since some years. Ideally we should migrate to Jetty 10 or 11. I think that in the past the reason for not migrating to Jetty 10 or 11 was that we had an older version of Karaf that would not support a newer version of Jetty; but I think that the Karaf version upgrade does now open the path towards a Jetty version upgrade too??

@J-N-K
Copy link
Member

J-N-K commented Apr 12, 2024

Jetty 10 will be part of Karaf 4.5, which will be released soon.

@wborn
Copy link
Member

wborn commented Apr 14, 2024

Do you have a recommendation how to pass the feature verification?

You can probably add the older ASM version to the xtext feature to fix the feature verification. But it will still cause issues for itests and devs using Eclipse because bnd has the limitation that it can only resolve one version of a bundle.

Xtext 2.35 will use ASM 9.7, see eclipse/xtext#2971

@holgerfriedrich
Copy link
Member Author

Thanks, @wborn for your comment.
I have seen the PR, but I do not expect xtext 2.35 very soon, see https://github.com/eclipse/xtext/milestone/25
If we cannot get around the problem with non-matching asm versions, we are caught with the Jetty issue openhab/openhab-distro#1641.

My idea was to force everything to asm 9.7 and it worked for the tests and mvn dependency:tree, but no idea how to get around feature verification and bnd as well.... Maybe we just have to wait.

@J-N-K
Copy link
Member

J-N-K commented Apr 14, 2024

Again Xtext is blocking progress...

@holgerfriedrich
Copy link
Member Author

I think I will stop the activities here until xtext 2.35 is released. It will contain the matching asm 9.7.
This might still take a while.
https://github.com/eclipse/xtext/milestone/25

@andrewfg
Copy link
Contributor

stop the activities here until xtext 2.35

Perhaps even wait until Karaf 10.5 .. so we can also upgrade from Jetty v9.x to 10.x

@holgerfriedrich
Copy link
Member Author

Great, xtext provided a first milestone release 2.35.0.M0.
All PR builds are completing 🎉🥳

@holgerfriedrich
Copy link
Member Author

It seems ready for a try.

image

@holgerfriedrich holgerfriedrich changed the title [WIP] Upgrade Karaf from 4.4.5 to 4.4.6 Upgrade Karaf from 4.4.5 to 4.4.6 Apr 22, 2024
@holgerfriedrich
Copy link
Member Author

holgerfriedrich commented Apr 30, 2024

@wborn Sorry to bother you again with a Karaf upgrade, but I think I need your help on this.

I thought I had everything prepared for 4.4.6, the PRs for all repos are up to date. Everything builds fine, tests are succeeding.
When I deploy the distro and my kar file, OH is starting.

But then I see an issue installing specific plugins, e.g. the mapdb binding:

18:10:41.139 [ERROR] [.core.karaf.internal.FeatureInstaller] - Failed installing 'openhab-persistence-mapdb': Unable to resolve root: missing requirement [root] osgi.identity; osgi.identity=openhab-runtime-base; type=karaf.feature; version="[4.2.0.SNAPSHOT,4.2.0.SNAPSHOT]"; filter:="(&(osgi.identity=openhab-runtime-base)(type=karaf.feature)(version>=4.2.0.SNAPSHOT)(version<=4.2.0.SNAPSHOT))" [caused by: Unable to resolve openhab-runtime-base/4.2.0.SNAPSHOT: missing requirement [openhab-runtime-base/4.2.0.SNAPSHOT] osgi.identity; osgi.identity=openhab-core-io-rest-sitemap; type=karaf.feature [caused by: Unable to resolve openhab-core-io-rest-sitemap/4.2.0.SNAPSHOT: missing requirement [openhab-core-io-rest-sitemap/4.2.0.SNAPSHOT] osgi.identity; osgi.identity=openhab-core-base; type=karaf.feature [caused by: Unable to resolve openhab-core-base/4.2.0.SNAPSHOT: missing requirement [openhab-core-base/4.2.0.SNAPSHOT] osgi.identity; osgi.identity=org.openhab.core.automation; type=osgi.bundle; version="[4.2.0.202404300434,4.2.0.202404300434]"; resolution:=mandatory [caused by: Unable to resolve org.openhab.core.automation/4.2.0.202404300434: missing requirement [org.openhab.core.automation/4.2.0.202404300434] osgi.wiring.package; filter:="(&(osgi.wiring.package=com.google.gson)(version>=2.10.0)(!(version>=3.0.0)))" [caused by: Unable to resolve com.google.gson/2.10.1.v20230109-0753: missing requirement [com.google.gson/2.10.1.v20230109-0753] osgi.ee; filter:="(&(osgi.ee=JavaSE)(version=1.7))"]]]]]

Others are working fine. I do not see where I missed something....

@J-N-K
Copy link
Member

J-N-K commented Apr 30, 2024

I think this is the same as (or at least very similar to) #4158:

filter:="(| (&(osgi.ee=JavaSE)(version=1.8)) (&(osgi.ee=JavaSE/compact1)(version=1.8)) )"
filter:="(&(osgi.ee=JavaSE)(version=1.7))"

The resolver seems to try to find an older (ancient) Java version and fails on that.

@holgerfriedrich
Copy link
Member Author

@J-N-K I thought that #4158 is a startup issue for new installs or after cleaning the cache.
The problem above persisted over several restarts, and prevented me from installing MapDB persistence.

Once I stopped OH, deleted cache and tmp folder, and finally restarted OH, it worked.

The test installation is running fine, and still working after I restarted OH.

Anyway, the resolver issue is annoying and still present in 4.4.6. I don't know how to debug and resolve this 😒

@jimtng
Copy link
Contributor

jimtng commented May 7, 2024

But then I see an issue installing specific plugins, e.g. the mapdb binding:

Seems like #4222

@holgerfriedrich
Copy link
Member Author

Local build is fine, distro works without any resolver issues. 🎉

The last verification step is an add-on build including all tests (which were excluded before).

@holgerfriedrich
Copy link
Member Author

holgerfriedrich commented May 12, 2024

@openhab/core-maintainers Karaf 4.4.6 upgrade seems ready. All itests now pass, including add-ons.

@holgerfriedrich
Copy link
Member Author

@wborn Do you think we should go for the Karaf upgrade before triggering the milestone build? It would make addon development easier, as anyone can run the current plugins with the last milestone. But it would increase the risk of further delays....

@kaikreuzer
Copy link
Member

@holgerfriedrich As such an upgrade always comes with the risk of regressions, I would do it right AFTER the milestone release. But very happy to hear that so far all looks good for Karaf 4.4.6, great job!

@lolodomo
Copy link
Contributor

Great, xtext provided a first milestone release 2.35.0.M0.

A 2.35.0.M1 was released
https://github.com/eclipse/xtext/releases/tag/v2.35.0.M1

@holgerfriedrich
Copy link
Member Author

Thanks, @lolodomo. It's already included in my latest update to this PR.

* Sync runtime dependencies with Karaf 4.4.6, most notably:
  * Jetty 9.4.54.v20240208, addresses CVE-2024-22201
  * Pax Logging 2.2.7
  * Pax Web 8.0.27
  * ASM 9.7
  * BouncyCastle 1.77
* Upgrade xtext to 2.35.0.M1
* Resolve itest runbundles

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
@holgerfriedrich
Copy link
Member Author

@kaikreuzer> @holgerfriedrich As such an upgrade always comes with the risk of regressions, I would do it right AFTER the milestone release. But very happy to hear that so far all looks good for Karaf 4.4.6, great job!

@openhab/core-maintainers
Should we attempt the Karaf update today? I have just rebased all PRs and did a full build including tests and install in Windows. I also installed a few of my favorite add-ons.
So far, it seems to work fine, at least on Windows.

We would need someone who can trigger the snapshot builds and merge in distro repo as well.....

pom.xml Outdated
<slf4j.version>2.0.7</slf4j.version>
<xtext.version>2.34.0</xtext.version>
<slf4j.version>2.0.12</slf4j.version>
<slf4j-for-model.version>2.0.12</slf4j-for-model.version>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is needed. We can just use slf4j.version instead or omit it completely (not sure if that works).

Copy link
Member

Choose a reason for hiding this comment

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

Did you see this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I did not.
Anyway, you are right, this can be removed since we have the xtext milestone release.
PR is adapted, let the CI check.

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks!

@J-N-K J-N-K merged commit 873bb53 into openhab:main May 20, 2024
5 checks passed
@J-N-K J-N-K added this to the 4.2 milestone May 20, 2024
@J-N-K
Copy link
Member

J-N-K commented May 20, 2024

I also triggered a new core snapshot: https://ci.openhab.org/job/openHAB-Core/1399/

@holgerfriedrich Can you re-build the PR in openhab-addons once this is finished?

@holgerfriedrich holgerfriedrich deleted the k446 branch May 20, 2024 15:45
@holgerfriedrich
Copy link
Member Author

holgerfriedrich commented May 20, 2024

@J-N-K could you please re-trigger the core snapshot build? One of the itests somehow failed...

florian-h05 pushed a commit to openhab/openhab-webui that referenced this pull request May 20, 2024
Refs openhab/openhab-core#4181.

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
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 of the Core security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Karaf / Jetty to address security issue
7 participants