Update snakeyaml dependency to resolve CVEs #480

mamccorm · 2022-09-22T12:30:03Z

Fixes: #479

There are a number of vulnerabilities in snakeyaml, which we inherit from our dependency on snakeyaml, namely:

Most of these have been resolved in snakeyaml v1.32 or 1.31.

The changes in this PR are to upgrade to the latest snakeyaml library to avail of the fix.

Local testing:

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for plugin-management-parent-pom 2.12.10-SNAPSHOT:
[INFO]
[INFO] plugin-management-parent-pom ....................... SUCCESS [  5.837 s]
[INFO] plugin-management-library .......................... SUCCESS [01:38 min]
[INFO] plugin-management-cli .............................. SUCCESS [ 11.598 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:56 min
[INFO] Finished at: 2022-09-22T13:10:49+01:00
[INFO] ------------------------------------------------------------------------

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

MarkEWaite · 2022-09-22T12:41:43Z

I don't think this change will resolve the issue that you're trying to resolve.

I think that you want the plugin installation manager tool to include snakeyaml 1.31 or even better, snakeyaml 1.32. The changes you've made will adjust the tests, but won't change the version of snakeyaml that is packaged as a transitive dependency of the Jackson dataformat library.

The maven dependency tree shows:

[INFO] -------< io.jenkins.plugin-management:plugin-management-library >-------
[INFO] Building plugin-management-library 2.12.10-SNAPSHOT                [2/3]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:3.3.0:tree (default-cli) @ plugin-management-library ---
[INFO] io.jenkins.plugin-management:plugin-management-library:jar:2.12.10-SNAPSHOT
[INFO] +- org.jenkins-ci:version-number:jar:1.10:compile
[INFO] +- commons-io:commons-io:jar:2.11.0:compile
[INFO] +- org.apache.commons:commons-lang3:jar:3.12.0:compile
...
[INFO] +- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.13.4:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.13.4:compile
[INFO] |  |  \- com.fasterxml.jackson.core:jackson-annotations:jar:2.13.4:compile
[INFO] |  +- org.yaml:snakeyaml:jar:1.31:compile
[INFO] |  \- com.fasterxml.jackson.core:jackson-core:jar:2.13.4:compile

I think that means that a new dependency on snakeyaml will need to be added to the plugin-management-library/pom.xml file.

mamccorm · 2022-09-22T13:46:49Z

Thanks @MarkEWaite. Still finding my way at contributions so my apologies. Added the v1.32 dependency to the bom.xml

timja · 2022-09-22T13:55:23Z

This should be in a dependencyManagement block in the https://github.com/jenkinsci/plugin-installation-manager-tool/blob/master/plugin-management-library/pom.xml file

With a TODO comment to remove it when jackson-dataformat-yaml is updated and includes a newer version.
and the TODO should also go next to

plugin-installation-manager-tool/plugin-management-library/pom.xml

Line 66 in 4e58736

MarkEWaite · 2022-09-23T14:24:56Z

I confirmed that the snakeyaml version bundled in plugin installation manager tool when built with this pull request is 1.32 as expected.

MarkEWaite

I would prefer that the two changes to test data be reverted because they are not needed for the objective of this pull request and they check that we retain compatibility with older releases.

However, I'm also OK if the decision is to update the test data.

MarkEWaite · 2022-09-23T14:25:47Z

...-library/src/test/java/io/jenkins/tools/pluginmanager/impl/PluginManagerIntegrationTest.java

@@ -298,7 +298,7 @@ public void verifyDownloads_smoke() throws Exception {

        // Second cycle, with plugin update and new plugin installation
        Plugin trileadAPI = new Plugin("trilead-api", "1.0.13", null, null);
-        Plugin snakeYamlAPI = new Plugin("snakeyaml-api", "1.27.0", null, null);
+        Plugin snakeYamlAPI = new Plugin("snakeyaml-api", "1.31-84.ve43da_fb_49d0b", null, null);


I think this setting can be retained to continue testing that behavior remains compatible even for older releases.

Suggested change

Plugin snakeYamlAPI = new Plugin("snakeyaml-api", "1.31-84.ve43da_fb_49d0b", null, null);

Plugin snakeYamlAPI = new Plugin("snakeyaml-api", "1.27.0", null, null);

I think fine to update some firewalls may get angry at vulnerable deps getting downloaded although could also be left

MarkEWaite · 2022-09-23T14:26:43Z

plugin-management-library/src/test/resources/io/jenkins/tools/pluginmanager/impl/plugins.txt

@@ -46,7 +46,7 @@ role-strategy:2.16
 run-condition:1.3
 scm-api:2.6.3
 security-inspector:0.5
-snakeyaml-api:1.26.4
+snakeyaml-api:1.31-84.ve43da_fb_49d0b


I think this data value can be retained to continue testing that behavior remains compatible even for older releases.

Suggested change

snakeyaml-api:1.31-84.ve43da_fb_49d0b

snakeyaml-api:1.26.4

MarkEWaite

I'm approving this because my comments are non-blocking. I plan to merge it within 24 hours unless someone raises an objection.

Thanks for the pull request!

timja · 2022-09-23T15:24:04Z

@MarkEWaite I would like #480 (comment) addressed

timja · 2022-09-23T15:24:42Z

i.e. I don't think it should be in the root pom it should be in the library

MarkEWaite · 2022-09-23T15:25:24Z

i.e. I don't think it should be in the root pom it should be in the library

Sorry that I missed that the dependency management entry was in the wrong directory. Agreed. Thanks for catching that!

Declare the dependency as narrowly as possible

MarkEWaite · 2022-09-23T15:36:13Z

I pushed ec27c39 too soon. That change is not correct. It still bundles snakeyaml 1.31 in the jar file. Will need more investigation.

The Jackson dataformat library depends on snakeyaml 1.31. The snakeyaml 1.31 library has a vulnerability that is resolved in snakeyaml 1.32. Include snakeyaml 1.32 as an explicit dependency until Jackson dataformat is updated to use a newer snakeyaml library.

MarkEWaite · 2022-09-23T15:53:20Z

Pushed 6239603 after verifying that the snakeyaml version included in the library is now 1.32. I'm open to suggestions of better ways to do it...

timja · 2022-09-23T15:59:08Z

sure it'll do for now

MarkEWaite · 2022-09-25T02:44:01Z

@timja I've labeled this as a "bug" rather than "chore". I think we're ready to merge and release with this change. Any objections if I merge it?

timja · 2022-09-25T06:42:55Z

@timja I've labeled this as a "bug" rather than "chore". I think we're ready to merge and release with this change. Any objections if I merge it?

no objections

upgrade snakeyaml-api plugin version

3065bd3

mamccorm mentioned this pull request Sep 22, 2022

Upgrade snakeyaml dependency to address various CVEs #479

Closed

Adding snakeyaml dependency

c4c8664

timja added the chore Project maintenance label Sep 22, 2022

MarkEWaite reviewed Sep 23, 2022

View reviewed changes

MarkEWaite approved these changes Sep 23, 2022

View reviewed changes

Add TODO for future snakeyaml dependency removal

a0d7361

Move snakeyaml dependency into library

ec27c39

Declare the dependency as narrowly as possible

timja approved these changes Sep 23, 2022

View reviewed changes

MarkEWaite added 2 commits September 23, 2022 09:36

Remove root pom change

04cfb26

MarkEWaite changed the title ~~Update snakeyaml-api-plugin dependency to resolve CVEs~~ Update snakeyaml dependency to resolve CVEs Sep 25, 2022

MarkEWaite added bug Something isn't working and removed chore Project maintenance labels Sep 25, 2022

MarkEWaite merged commit 5563515 into jenkinsci:master Sep 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update snakeyaml dependency to resolve CVEs #480

Update snakeyaml dependency to resolve CVEs #480

mamccorm commented Sep 22, 2022 •

edited by MarkEWaite

Loading

MarkEWaite commented Sep 22, 2022 •

edited

Loading

mamccorm commented Sep 22, 2022

timja commented Sep 22, 2022

MarkEWaite commented Sep 23, 2022

MarkEWaite left a comment

MarkEWaite Sep 23, 2022

timja Sep 23, 2022

MarkEWaite Sep 23, 2022

MarkEWaite left a comment

timja commented Sep 23, 2022

timja commented Sep 23, 2022

MarkEWaite commented Sep 23, 2022

MarkEWaite commented Sep 23, 2022

MarkEWaite commented Sep 23, 2022

timja commented Sep 23, 2022

MarkEWaite commented Sep 25, 2022

timja commented Sep 25, 2022

	Plugin snakeYamlAPI = new Plugin("snakeyaml-api", "1.31-84.ve43da_fb_49d0b", null, null);
	Plugin snakeYamlAPI = new Plugin("snakeyaml-api", "1.27.0", null, null);

Update snakeyaml dependency to resolve CVEs #480

Update snakeyaml dependency to resolve CVEs #480

Conversation

mamccorm commented Sep 22, 2022 • edited by MarkEWaite Loading

MarkEWaite commented Sep 22, 2022 • edited Loading

mamccorm commented Sep 22, 2022

timja commented Sep 22, 2022

MarkEWaite commented Sep 23, 2022

MarkEWaite left a comment

Choose a reason for hiding this comment

MarkEWaite Sep 23, 2022

Choose a reason for hiding this comment

timja Sep 23, 2022

Choose a reason for hiding this comment

MarkEWaite Sep 23, 2022

Choose a reason for hiding this comment

MarkEWaite left a comment

Choose a reason for hiding this comment

timja commented Sep 23, 2022

timja commented Sep 23, 2022

MarkEWaite commented Sep 23, 2022

MarkEWaite commented Sep 23, 2022

MarkEWaite commented Sep 23, 2022

timja commented Sep 23, 2022

MarkEWaite commented Sep 25, 2022

timja commented Sep 25, 2022

mamccorm commented Sep 22, 2022 •

edited by MarkEWaite

Loading

MarkEWaite commented Sep 22, 2022 •

edited

Loading