CVEs not fixed in 2.x branch

[JanWesterkamp-iJUG]

Hi, reviewed the 2.x branch and the reported CVEs are not fixed in the branch 2.x completely, only one got fixed with the TestNG update to 7.5.1.

References to the reported CVEs:

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=581579 (Asciidoctor Maven Plugin related)
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=581586 (TestNG related)

For fixing the first one, cherry picking need to be done from the main (3.x) branch. Versions are not fully up to date there, but the CVEs are fixed.
To keep it up to date, the following versions need to be configured:

<!-- Asciidoctor support versions --> <asciidoctor.maven.plugin.version>2.2.4</asciidoctor.maven.plugin.version> <asciidoctorj.version>2.5.10</asciidoctorj.version> <asciidoctorj.pdf.version>**2.3.9**</asciidoctorj.pdf.version> <jruby.version>**9.4.3.0**</jruby.version>

For the 2nd fix, it can not be done completely without introducing a breaking change and so violating semver, as the fixed version of TestNG requires Java SE 11 instead of Java SE 8 - only one CVE got fixed with the 7.5.1 Patch Release still compatible with Java SE 8.
All the other Component Specs are required to swtich to version 3.2+ or introduce a workaround, if they depend on TestNG within themselfs. This might require a Major Release in some cases be be compliant with semver, especially when they still depend on Java SE 8 instead of Java SE 11 (!).

The comment about being TestNG version in sync with the Arquillian version is invalid too here, so I recoment adding an additional note to it, that this deviates now to workaround CVEs, that are not fixed in Arquillian yet.

[JanWesterkamp-iJUG]

@Emily-Jiang, as requested here are my findings in the 2.x branch, that need to be fixed before all the MP component specs depending on it need to do at least a Patch Release with it.

[Emily-Jiang]

Thank you @JanWesterkamp-iJUG for the analysis. In light of this, I don't think it can be easily done with a patch release. I think a wider org communication is needed to discuss the severity and impact of these CVEs.

[JanWesterkamp-iJUG]

Thank you @JanWesterkamp-iJUG for the analysis. In light of this, I don't think it can be easily done with a patch release. I think a wider org communication is needed to discuss the severity and impact of these CVEs.

These CVE findings are about a half year old and most of them (including all of the AsciiDoctor related ones) could be fixed easily. The remaining ones are a little bit tricky, because it requires a review on the affected component specs - the ones on Java SE 11+ already might be fixed via a workaround - the others could only switch to a different test framework or switch to Java SE 11 - both might not be done in a Patch Release.

This is a perfect example why it is a very bad idea to have a MicroProfile Release with component specs based on different environments (MicroProfile Parent Major versions).
Doing something like this reduces maintainability significantly - therefore I voted against doing such things.

Of course downplaying the security issues is another option to go for, but why you do not ask your IBM Security Team to evaluate this first and how to handle such potential support of supply chain attacks?

[Azquelt]

Hi @JanWesterkamp-iJUG, thanks for this issue but I'm not sure I completely follow what you're reporting.

1. For the TestNG issue: I thought all CVEs in TestNG itself were fixed as of 7.5.1 (the latest release still supporting Java 8). I see a few CVEs listed for optional dependencies, but I don't think that matters because they won't be included unless one of the component TCKs also specifies a dependency on that package.

   If there are any other specific CVEs for TestNG 7.5.1 that I'm missing, can you list them in the linked eclipse issue please?

2. For the asciidoctor plugin, it sounds like this just needs an update of the plugin dependencies? I can see the update of the plugin to 2.2.4 is needed, and a corresponding update to asciidoctor-pdf makes sense, though I'm not sure why we need to additionally specify asciidoctorj.version or jruby.version?

   If there's something here that I'm missing, could you explain why those are needed (either here or in the linked eclispe issue)

In general I am in favour of keeping all of our dependencies up to date, though I'm much more concerned about the dependencies of our APIs since those are used by all application developers, as opposed to our plugin and TCK dependencies which are only used by spec contributors and integrators and should be running in a safe environment against trusted inputs. Anyone with a strict security policy in this area can force the use of newer dependencies in their pom when they run the TCK.

The choice to allow specs to continue to support running the TCK on Java 8 is not something we can discard without community agreement so for the moment we're stuck maintaining two versions of the parent pom, and that does mean we should be monitoring this branch for dependency vulnerabilities as well as the main branch.

[JanWesterkamp-iJUG]

Hi @JanWesterkamp-iJUG, thanks for this issue but I'm not sure I completely follow what you're reporting.

1. For the TestNG issue: I thought all CVEs in TestNG itself were fixed as of 7.5.1 (the latest release still supporting Java 8). I see a few CVEs listed for optional dependencies, but I don't think that matters because they won't be included unless one of the component TCKs also specifies a dependency on that package.
   If there are any other specific CVEs for TestNG 7.5.1 that I'm missing, can you list them in the linked eclipse issue please?

2. For the asciidoctor plugin, it sounds like this just needs an update of the plugin dependencies? I can see the update of the plugin to 2.2.4 is needed, and a corresponding update to asciidoctor-pdf makes sense, though I'm not sure why we need to additionally specify `asciidoctorj.version` or `jruby.version`?
   If there's something here that I'm missing, could you explain why those are needed (either here or in the linked eclispe issue)

In general I am in favour of keeping all of our dependencies up to date, though I'm much more concerned about the dependencies of our APIs since those are used by all application developers, as opposed to our plugin and TCK dependencies which are only used by spec contributors and integrators and should be running in a safe environment against trusted inputs. Anyone with a strict security policy in this area can force the use of newer dependencies in their pom when they run the TCK.

The choice to allow specs to continue to support running the TCK on Java 8 is not something we can discard without community agreement so for the moment we're stuck maintaining two versions of the parent pom, and that does mean we should be monitoring this branch for dependency vulnerabilities as well as the main branch.

@Azquelt, regarding the TestNG CVEs I will update the MicroProfile and Jakarta security isssues as suggested be you, when there is a little bit more time - for a shortcut, you can simply look at the sec scanner results on this:

• https://mvnrepository.com/artifact/org.testng/testng/7.5.0 (Arquillian 1.7.0.Final)
• https://mvnrepository.com/artifact/org.testng/testng/7.5.1 (MP Parent 2.x)
• https://mvnrepository.com/artifact/org.testng/testng/7.8.0 (MP Parent 3.2)

NOTE: The side confuses a little bit, as looking at the versions overview site (https://mvnrepository.com/artifact/org.testng/testng) it shows less results.

For patching the AsciiDoctor CVEs issues:
You can find the long story in the issues and PRs that have been made to fix them in the plugin. Short, Asciidoctor is implemented in Ruby and can be run in a JVM via JRuby.
The release cadence of the plugin (and included dependencies) differs from the maintenance on the other Ruby/JRuby artifacts. These dependencies need to match to have a well tested environment. The conservative approach is to orient on the default dependencies of the plugin (sometimes outdated and having issues), a better tested definition on the official AsciiDoctor Maven Plugin Examples (they overwrite a lot of the dependencies) or my well tested set of dependencies, which are derived from my current projects with complex use of AsciiDoctor features (including Asciidoctor Diagram, which I wanted to integrate into the MP Parent when finding these issues originally).

So for fixing the CVEs only, as far as I know, it should be sufficent to update the plugin version itself. But for preventing issues, I suggest to use a well tested combination - especially in this MP Parent Use Case, as it's hard to check the correct creation on all the dependent component specs...

I can create a PR for the 3.x branch for my latest configuration (without diagram support), which you can use in the 2.x branch. Or you take the current 3.x config here now. Unfortunately I can not create a similar PR to the 2.x brach easily because the new branch is not forked (I selceted to fork all of them originally). It looks like GitHub add new branches to the forks not automatically or stops doing this because since the renaming of the master to the main branch it's not a simple same branch name mapping anymore. @Emily-Jiang was the creation of the 2.x branch before or after the renaming?

I aggree that a 2.x branch is required, when support is offered to older versions - but in this case for supporting one umbrella spec version (MP 6.0, 6.1) with two MP Parent versions is only technical debt that increases maintenance effort - for saving some effort on releasing separate versions (with the new MP Parent version) for component specs, that then got quailfied in a different environment before umbrella spec CI vendors start testing it in the new environment - and have to solve issues late in the release process.

I also aggree that there are different categories on threats, that should be handled with different priorities - thing that affect the majority of the end users directly must be fixed immediatly. But there are special end users in critical environments, who are creating everything they run from source (that they can analyse) and they are also affected by supply chain issues like this. Also we ourself and our organistaions are affected by this and before updating some documentation to workaround these it's much better to fix them. Also end users are affected with potentially false positives (for their common use cases), which results in increased maintenance effort, example: Build fails because of the finding, finding need to be analysed, issue is filed, exception from the security team is requested, another threat analysis will be done by them, exception might be granted from 0 to 90 days to solve it and until solved, the process is triggered again on that time base.

So I think we need to solve them as much as possible for any new release and in some cases we need to trigger extra Patch Releases too. To fix these in MiroProfile 6.1 all of the component specs need an update.

BTW, keeping the versions up to date helps a lot to prevent these issues, as some of them might be fixed already, but not reported at this time. Or they will be fixed while not even recognised by the maintainers - this happend on Jakarta Concurrency through good maintenance practice ;-)

[Emily-Jiang]

I can create a PR for the 3.x branch for my latest configuration (without diagram support), which you can use in the 2.x branch. Or you take the current 3.x config here now. Unfortunately I can not create a similar PR to the 2.x brach easily because the new branch is not forked (I selceted to fork all of them originally). It looks like GitHub add new branches to the forks not automatically or stops doing this because since the renaming of the master to the main branch it's not a simple same branch name mapping anymore. @Emily-Jiang was the creation of the 2.x branch before or after the renaming?

I don't understand how is to do with the branch renaming from master to main. The branch 2.x was untouched. You can simply resync.

For patching the AsciiDoctor CVEs issues:
You can find the long story in the issues and PRs that have been made to fix them in the plugin. Short, Asciidoctor is implemented in Ruby and can be run in a JVM via JRuby.
The release cadence of the plugin (and included dependencies) differs from the maintenance on the other Ruby/JRuby artifacts. These dependencies need to match to have a well tested environment. The conservative approach is to orient on the default dependencies of the plugin (sometimes outdated and having issues), a better tested definition on the official AsciiDoctor Maven Plugin Examples (they overwrite a lot of the dependencies) or my well tested set of dependencies, which are derived from my current projects with complex use of AsciiDoctor features (including Asciidoctor Diagram, which I wanted to integrate into the MP Parent when finding these issues originally).

MicroProfile is to release api jar, spec doc and tck jars. The AsciiDoctor features were used to produce these artifacts. As for the released artifacts, none of them plugins surfaced out. It is a good practice to adopt the latest version but it has nothing to do with the released docs produced. I think it is sufficient to fix the main branch. Since component specs uses difference parent branches, it makes sense to fix them there. I still think fixing parent poms is a lot better than going through to each component specs to perform the same fixes. Having said that, I think for MP 7.0 release, we should move up all component specifications to EE 10 and be done with it. Again, since previously we agreed each component spec can make their own choices, we need to agree on the scope.

I also aggree that there are different categories on threats, that should be handled with different priorities - thing that affect the majority of the end users directly must be fixed immediatly. But there are special end users in critical environments, who are creating everything they run from source (that they can analyse) and they are also affected by supply chain issues like this. Also we ourself and our organistaions are affected by this and before updating some documentation to workaround these it's much better to fix them. Also end users are affected with potentially false positives (for their common use cases), which results in increased maintenance effort, example: Build fails because of the finding, finding need to be analysed, issue is filed, exception from the security team is requested, another threat analysis will be done by them, exception might be granted from 0 to 90 days to solve it and until solved, the process is triggered again on that time base.

I am not sure what Eclipse Foundation has any policiy about this as EF treats security supply chain very seriously as well.

So I think we need to solve them as much as possible for any new release and in some cases we need to trigger extra Patch Releases too. To fix these in MiroProfile 6.1 all of the component specs need an update.

MP 6.1 just aggregates the component specs. All component specs are free to do patch releases and the implementation of MP 6.1 should consume them and the spec doc encourages this practice. It is unnecessary to stop MP 6.1 from going. CVEs could come any day, we have to design a flexible and sustainable pattern to address the CVEs and encourage the affected specs to perform patch releases.

[JanWesterkamp-iJUG]

@Emily-Jiang , the CVEs are reported months ago and we are missing a process (ressources and/or will) to apply the necessary changes.

Having two different major versions of MP Parent involved here in a single MP umbrella spec release increases the maintenance significantly. A majority of the members accepted these technical debts without having the will to pay the interest rate on it ;-)
The MicroProfile umbrella spec und MP Parent (and corresponding teams) have also the responsibility to ensure, that the combind/derived component specs do the necessary fixes, so issues will be fixed competely and not partly on some specs only.
Who else should do this?

BTW, this is similarity to Jakarta, where the technical debt is even higher as there is no equivaltent to the MP Parent yet and much less will to do maintenance like this.
And most of these fixes need to be done there too, to get rid of them completely, but I assume this is out of scope here (but discussed in the Jakarta Platform Mainanence addon meeting).

I think we need to have something/somebody to force us doing it, as it looks like mentioning it in the MicroProfile Release Plan in not sufficent to ensure it will be done.

[Emily-Jiang]

@JanWesterkamp-iJUG Thanks for your input! We will discuss this in the technical calls and find a sustainable way to handle it. I have a few thoughts on this:

1. Who will provide the list of CVEs on the ongoing basis? I think it is not feasible to rely on a particular person to do so. I expect an automated process to check CVEs and bring our attentions.
2. What CVEs to be fixed asap? What CVEs can be fixed in the next release? What CVEs can be fixed on the master branch without doing a release?
3. Do we need to maintain the previous versions?
4. Who should be responsible to deal with the CVEs (e.g. each spec needs to release a micro release)?