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

Explicitly declaring xstream to override a transitively imported version affected by CVE #3785

Merged
merged 6 commits into from
Nov 20, 2024

Conversation

yesamer
Copy link
Contributor

@yesamer yesamer commented Nov 19, 2024

Temporary declaring xstream dependency, a version (1.4.20) is transitively imported by Quarkus 3.8 affected by CVE

@yesamer yesamer changed the title exclude_xstream Exclude xstream version affected by CVE Nov 19, 2024
gitgabrio
gitgabrio previously approved these changes Nov 19, 2024
Copy link
Contributor

@gitgabrio gitgabrio left a comment

Choose a reason for hiding this comment

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

Thanks @yesamer LGTM!

@kie-ci3
Copy link
Contributor

kie-ci3 commented Nov 19, 2024

PR job #2 was: UNSTABLE
Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-runtimes -u #3785 --skipParallelCheckout

NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-runtimes-pr/job/PR-3785/2/display/redirect

Test results:

  • PASSED: 3369
  • FAILED: 23

Those are the test failures:

org.kie.kogito.addon.quarkus.messaging.common.message.http.CloudEventHttpOutgoingDecoratorTest.verifyOutgoingHttpMetadataIsSet java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.process.dynamic.DynamicCallResourceTest.testConcurrentDynamicCalls java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.events.mongodb.QuarkusMongoDBEventPublisherTest.userTasksEventsCollection java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.DeploymentUtilsTest.testDeploymentWithServiceWithCustomPortName java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.Fabric8KubernetesServiceCatalogTest.getServiceAddress(KubernetesProtocol, String, String)[1] java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.IngressUtilsTest.testIngressWithTLS java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.KnativeServiceDiscoveryTest.queryService java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.KubernetesResourceDiscoveryTest.testServiceNodePort java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.OpenShiftServiceDiscoveryTest.testDeploymentConfigWithoutService java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.PodUtilsTest.testPodWithNoService java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.fabric8.k8s.service.catalog.StatefulSetUtilsTest.testStatefulSetNoService java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.jbpm.usertask.jpa.quarkus.H2QuarkusJPAUserTaskInstancesTest.testFindByIdentityByPotentialGroups java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.jbpm.usertask.jpa.quarkus.PostgreSQLQuarkusJPAUserTaskInstancesTest.testFindByIdentityByPotentialGroups java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.k8s.KnativeRouteEndpointDiscoveryTest.testBaseCase java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.k8s.KubernetesServiceEndpointDiscoveryTest.testGetURLOnRandomPort java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.k8s.config.KubeDiscoveryConfigCacheUpdaterTest.knativeResource java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.mail.QuarkusMailSenderTest.testMail java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addons.quarkus.microprofile.config.service.catalog.MicroProfileConfigServiceCatalogTest.getServiceAddress(KubernetesProtocol, String, String)[1] java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.infinispan.health.InfinispanHealthCheckIT.testCall java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.process.definitions.ProcessDefinitionsResourceTest.testAddDefinition java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.addon.source.files.SourceFilesResourceTest.getSourceFilesByProcessIdTest java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.quarkus.drools.RuleUnitMetaDataContextSerializationTest.ensureRuleUnitMetaDataSerializable java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter
org.kie.kogito.legacy.rules.TmsEndpointTest.testHelloEndpoint java.lang.NoClassDefFoundError: com/thoughtworks/xstream/converters/Converter

@gitgabrio
Copy link
Contributor

gitgabrio commented Nov 20, 2024

Thanks @yesamer , but I'm still doubtful about this modification.

Here's from the build logs

2024-11-19T17:57:42.1468110Z Downloading from central: https://repo.maven.apache.org/maven2/com/thoughtworks/xstream/xstream/1.4.20/xstream-1.4.20.pom        
2024-11-19T17:57:42.1471968Z Downloaded from central: https://repo.maven.apache.org/maven2/com/thoughtworks/xstream/xstream/1.4.20/xstream-1.4.20.pom (25 kB at 1.6 MB/s)
2024-11-19T17:57:42.1474434Z Downloading from central: https://repo.maven.apache.org/maven2/com/thoughtworks/xstream/xstream-parent/1.4.20/xstream-parent-1.4.20.pom
2024-11-19T17:57:42.2456693Z Downloaded from central: https://repo.maven.apache.org/maven2/com/thoughtworks/xstream/xstream-parent/1.4.20/xstream-parent-1.4.20.pom (44 kB at 2.6 MB/s)
2024-11-19T17:57:42.3465718Z Downloading from central: https://repo.maven.apache.org/maven2/com/thoughtworks/xstream/xstream/1.4.20/xstream-1.4.20.jar

To be clear, I'm not saying that the fix does not work as expected

But IMO, this solution is inherently frail, since, without explicitly excluding xstream from quarkus-junit5:

  1. the xstream version from quarkus-junit5 it is anyway added to maven context (maybe demonstrated by the downloading messages)
  2. the override of that version with the declared one strongly depends on the order used by maven to override dependencies (the "bottom" ones overrides the upper ones): it would require just a small reorganization of the pom, or some analogous modification in some children, to break it

At the same time, I see that, with the exclude, we would require another modification somewhere else...

@gitgabrio gitgabrio dismissed their stale review November 20, 2024 09:05

Doubtfull about proposed solution

@yesamer
Copy link
Contributor Author

yesamer commented Nov 20, 2024

@gitgabrio I understand your concern and I agree this is not the ideal solution and in normal conditions it should be avoided, in my opinion, could make sense as a temporary solution but let's wait for other opinions.

@yesamer yesamer changed the title Exclude xstream version affected by CVE Explicitly declaring xstream to override a transitively imported version affected by CVE Nov 20, 2024
@porcelli porcelli merged commit 4d1a3d7 into apache:main Nov 20, 2024
6 checks passed
@yesamer yesamer deleted the exclude_xstream branch November 20, 2024 19:10
rgdoliveira pushed a commit to rgdoliveira/kogito-runtimes that referenced this pull request Nov 25, 2024
…ion affected by CVE (apache#3785)

* exclude_xstream

* Updated comment

* change

* change

* change

* change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants