search

tada / pljava

PL/Java is a free add-on module that brings Java™ Stored Procedures, Triggers, Functions, Aggregates, Operators, Types, etc., to the PostgreSQL™ backend.
http://tada.github.io/pljava/
Other
238 stars 77 forks source link

Vulnerability scan: multiple issues #449

Closed beargiles closed 2 months ago

beargiles commented 10 months ago

A routine vulnerability scan of my docker image (for official 15.3 release) shows 38 outdated jars with fixed vulnerabilities.

See https://hub.docker.com/layers/beargiles/postgres-pljava/latest/images/sha256-e887c997e32cd1769e9bde68ab4cbfb75e9bb39292a855a53b5592f4c99d0e79?context=repo&tab=vulnerabilities

You can find the details, including fixed version, by drilling down the list on the right. E.g., image

beargiles commented 10 months ago

Looking at this a bit deeper I can see that many of the jars listed do not appear in the results of mvn dependency:list. I think some are dependencies for maven itself, and in a few cases those dependencies are affecting the version chosen when running mvn install.

Looking at mvn dependency:tree a good place to start is the reporting stanza:

org.apache.maven.reporting:maven-reporting-api:3.0 -> 3.1.`
org.apache.maven.reporting:maven-reporting-impl:3.0.0 -> 3.2.0

Changes to mvn dependency list

These are the changes in the maven dependency list (pljava_pgxs only). The docker scan may still show the vulnerable versions but at least a handful of vulnerabilities in the extension itself will be closed.

Additional changes may be possible - I just wanted to start with these two dependencies since it's not obvious that they would affect the Apache commons libraries.

commons-collections:common-collections:3.2.1 -> 3.2.2

org.apache.maven.shared/maven-shared-utils:* -> 3.3.4

org.apache.httpcomponents:httpclient:[4.0.2,4.5.8] -> 4.5.13

commons-io/commons-io:2.5 -> 2.6

Note: 'scout' also reports versions 2.2 and 2.4 present but they did not appear in the list of maven dependencies.

beargiles commented 10 months ago

The OWASP "dependency check" plugin will help identify the rest.

The stanza to add to build -> plugins is

        <plugin>
                <groupId>org.owasp</groupId>
                <artifactId>dependency-check-maven</artifactId>
                <version>8.3.1</version>
                <configuration>
                        <prettyPrint>true</prettyPrint>
                </configuration>
                <executions>
                        <execution>
                                <id>owasp-dependency-check</id>
                                <goals>
                                        <goal>check</goal>
                                </goals>
                        </execution>
                </executions>
        </plugin>

This will add a 'dependency-check-report.html` (and/or .xml, .json, etc.) file in the target directory.

The results (after prior modifications) for pljava-pgxs are:

Dependency Vulnerability IDs Package Highest Severity CVE Count Confidence Evidence Count
commons-beanutils-1.7.0.jar cpe:2.3:a:apache:commons_beanutils:1.7.0:::::::* pkg:maven/commons-beanutils/commons-beanutils@1.7.0 HIGH 2 Highest 21
commons-io-2.6.jar cpe:2.3:a:apache:commons_io:2.6:::::::* pkg:maven/commons-io/commons-io@2.6 MEDIUM 1 Highest 117
dom4j-1.1.jar cpe:2.3:a:dom4j_project:dom4j:1.1:::::::* pkg:maven/dom4j/dom4j@1.1 CRITICAL 2 Highest 17
graal-sdk-20.1.0.jar   pkg:maven/org.graalvm.sdk/graal-sdk@20.1.0 LOW 1   24
maven-core-3.1.0.jar cpe:2.3:a:apache:maven:3.1.0:::::::* pkg:maven/org.apache.maven/maven-core@3.1.0 CRITICAL 1 Highest 23
maven-settings-3.1.0.jar   pkg:maven/org.apache.maven/maven-settings@3.1.0 CRITICAL 1   25
velocity-1.7.jar cpe:2.3:a:apache:velocity_engine:1.7:::::::* pkg:maven/org.apache.velocity/velocity@1.7 HIGH 1 Low 76
velocity-tools-2.0.jar cpe:2.3:a:apache:velocity_tools:2.0:::::::* pkg:maven/org.apache.velocity/velocity-tools@2.0 MEDIUM 1 Highest 76

The vulnerable versions of domj, apache commons, etc. were brought in through the velocity dependencies.

https://owasp.org/www-project-dependency-check/

beargiles commented 10 months ago

Here's a bit of progress:

Dependency Vulnerability IDs Package Highest Severity CVE Count Confidence Evidence Count
dom4j-1.6.1.jar cpe:2.3:a:dom4j_project:dom4j:1.6.1:::::::* pkg:maven/dom4j/dom4j@1.6.1 CRITICAL 2 Highest 120
graal-sdk-23.0.1.jar cpe:2.3:a:sun:sdk:23.0.1:::::::* pkg:maven/org.graalvm.sdk/graal-sdk@23.0.1 LOW 1 Highest 27
guava-25.1-android.jar cpe:2.3:a:google:guava:25.1:::::::* pkg:maven/com.google.guava/guava@25.1-android HIGH 2 Highest 23
velocity-1.7.jar cpe:2.3:a:apache:velocity_engine:1.7:::::::* pkg:maven/org.apache.velocity/velocity@1.7 HIGH 1 Low 76
velocity-tools-2.0.jar cpe:2.3:a:apache:velocity_tools:2.0:::::::* pkg:maven/org.apache.velocity/velocity-tools@2.0 MEDIUM 1 Highest 76

Neither velocity nor dom4j has been updated in a very long time. I know that dom4j:dom4j has been replaced by org.dom4j:dom4j but I don't know what the status is for velocity.

I haven't investigated guava.

poms.zip

One known issue is that mvn site:site fails:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.12.1:site (default-cli) on project pljava.app: Error generating maven-project-info-reports-plugin:3.4.5:mailing-lists report: Illegal character in path at index 31: gmane.comp.db.postgresql.pljava on news.gmane.io -> [Help 1]

BTW I'm on REL1_6_STABLE branch and explicitly set maven-compiler-plulgin 'source' and 'target' to 11. (I commented out 'release'). The official image does not provide (or even offer!) anything other than java-17.

jcflack commented 10 months ago

Hi,

BTW I'm on REL1_6_STABLE branch and explicitly set maven-compiler-plulgin 'source' and 'target' to 11. (I commented out 'release'). The official image does not provide (or even offer!) anything other than java-17.

I am curious why you are changing those settings for the 1.6 series. The <release>9</release> in the POM is determined by the supported version document for PL/Java 1.6.x:

In the PL/Java 1.6.x series, the build can be done with Java 9 or newer. Once built, PL/Java is able to use another Java 9 or later JVM at run time, simply by setting the pljava.libjvm_location variable to the desired version’s library.

PL/Java can run application code written for a later Java version than PL/Java itself was built with, as long as that later JRE version is used at run time. That also allows PL/Java to take advantage of recent Java implementation advances ...

I have Java 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, and 20 here, and I do check now and then that JAVA_HOME can be pointed at any of those and mvn run and PL/Java gets built, with no need to change options in the POM. (Except on 20, where they broke regular expressions. They say that gets fixed in 21.) And once it has been built, regardless of what Java version you built it with, you can install it into PostgreSQL and freely change pljava.libjvm_location to point to any of those Javas and the PL/Java that you built will happily run, and the user code jars you install with your own logic in them can use whatever modern Java features you felt like using, as long as you have pljava.libjvm_location set to a Java version recent enough to have those.

The release compiler option is key to ensuring that. The older source and target options are not equivalent to it, as only release also checks for usage of a wrong version's API.

I am glad that you are looking at the versions of dependencies used in the Maven build; that is a thicket that has no siren call to me. It may be that you are working on updating some of those dependencies to newer versions, and then finding that those newer versions force you to abrogate the supported-Java-versions doc for PL/Java 1.6.x. That kind of thing can happen when venturing into that thicket.

It may help to keep in mind that those dependencies are used only during the build process: they are dependencies of Maven and the build process, not of PL/Java once it is built, and PL/Java relies on no other libraries to run, only what the Java runtime environment itself supplies.

(The PL/Java examples include one that can depend on Saxon, if you choose to install Saxon, and the testing harness built into the PL/Java installer jar, used in the CI testing scripts, can depend on pgjdbc-ng, if you install that.)

As for the dependencies of the Maven build process, while I would not say their vulnerabilities are of no concern, I would point out the build is something getting run in narrow circumstances and on fixed known inputs, not bombarded with unknown stuff from a potential adversary. (I mean the known local inputs, of course; the vast number of jars Maven downloads to do the work are coming from remote repos, and if just one of those has been Trojaned to do something nefarious while the build runs, it probably does not even need to find a vulnerability in some other module to accomplish it. That just seems to be life with Maven. I certainly do not run my Maven builds as root, or even as me.)

As for releases after 1.6.x, I would not shed tears if Maven were replaced completely as the build system. It was adopted back when PL/Java was building on older versions of Java with no inherent support for scripting a build, but all Java versions supported by 1.6.x come with jshell, which is already used to good advantage in PL/Java's CI scripting.

It really makes little sense to use Maven and have it download 107 MB of stuff to build PL/Java, which comes out at 2 MB and has no dependencies. The actual building of its Java classes and javadoc could probably be done in jshell in fewer lines than this comment, and the building of the native portion—which is already done in JavaScript thanks to Karthik's work a few years back—would look a little less elegant in jshell but probably not too bad.

The piece that would leave undone would be the markdown⇨HTML documentation and website generation, now being done with Velocity. Nothing for that is batteries-included in Java, so it would have to remain an external dependency. Other than that, the build process is really far less complicated than Maven makes it.

beargiles commented 10 months ago

Welcome to the world of Enterprise Supply Chain Security. :-)

I agree that there's low risk in a build-time dependency. But companies are starting to look more closely at everything that goes into their final artifacts and that may include rebuilding everything from source. That includes the build-time tools since it's easy for a compromised/malicious plugin to insert vulnerabilities.

Hence adding the OWASP plugin and addressing everything it reports. This lets users update their vulnerability scan reports with an explanation of why a vulnerability is present.

The separate BOM is more of a growing convention than anything else but it can be valuable in larger projects. E.g., at my last employer our final artifacts had over 1000 dependencies. Many of the components (separate classloaders) had to use specific versions of a dependency. Without a BOM individual subprojects tended to use whatever was the most recent version when it was written and the vulnerability scans were a nightmare. Adding a BOM simplified that - and in the few cases where the update broke functionality we could override it in the individual pom.xml file and add an explanation for why we needed to stick to that version.

A lot like your comments about several dependencies being tied to a specific version of the JVM!

jcflack commented 10 months ago

Sure, I'd just be frustrated spending that much time sorting through what all amount to build dependencies of the build system, when strictly speaking PL/Java hasn't got any.*

Given that what we need is

compile and jar the Java stuff; compile and link the C stuff; exit 0

and what we have instead is Maven ... if I had the time to spend and my choice of how to spend it, I think I'd spend it making the build not use Maven.

* ok, I lied slightly. There are some (very few, at the moment) JUnit tests happening during the build. So JUnit is one build dependency that's really there because we want it, not just because Maven does. One build dependency. That I could live with.

And of course there's processing the Markdown into HTML. Needs to be some story for that, if only for people who want to build their own docs.

Even for that, I'm pretty sure there are nicer Markdown processors than what comes in Maven, though the dialect differences might mean a big flag day messing with the syntax in the doc files.

fmbiete commented 9 months ago

I have the same requirement: I cannot use maven plugins or dependencies with critical vulnerabilities, even if it's only for the build process and the final artifact doesn't contain the vulnerable package.

I have solved my issue updating the versions of the maven plugins hardcoded in the main pom.xml. That's enough to avoid the vulnerable build dependencies. I know that some of those packages will make the build require a version higher than Java 7, but even RHEL7 has Java 11 available, so I don't think it's a major issue, and certainly not for my use case...

These are the changes required. If you think this would be an acceptable request, I can open a pull request.

--- pom.xml 2023-09-20 13:42:00.937364034 +0000
+++ pom.xml.modified    2023-09-20 13:24:39.788806424 +0000
@@ -132,17 +132,17 @@
                <plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-install-plugin</artifactId>
-                   <version>2.5.2</version><!-- later versions are Java 7+ -->
+                   <version>3.1.0</version><!-- later versions are Java 7+ -->
                </plugin>
                <plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-resources-plugin</artifactId>
-                   <version>3.0.1</version><!-- later versions are Java 7+ -->
+                   <version>3.3.0</version><!-- later versions are Java 7+ -->
                </plugin>
                <plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-compiler-plugin</artifactId>
-                   <version>3.8.1</version>
+                   <version>3.10.1</version>
                </plugin>
            </plugins>
        </pluginManagement>
@@ -160,24 +160,17 @@
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-jar-plugin</artifactId>
-               <version>3.0.2</version>
+               <version>3.3.0</version>
            </plugin>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
-               <version>3.0.0-M4</version>
+               <version>3.0.0-M7</version>
            </plugin>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-site-plugin</artifactId>
-               <version>3.9.1</version>
-               <dependencies>
-                   <dependency>
-                       <groupId>net.trajano.wagon</groupId>
-                       <artifactId>wagon-git</artifactId>
-                       <version>2.0.4</version>
-                   </dependency>
-               </dependencies>
+               <version>3.12.1</version>
                <configuration>
                    <relativizeDecorationLinks>false</relativizeDecorationLinks>
                </configuration>
jcflack commented 9 months ago

Thank you for that effort.

I noticed a couple of places in your patch where you changed a version number that had a "later versions are Java 7+" comment, to a later version, but left the "later versions are Java 7+" comment.

If you are basing your patch on REL1_6_STABLE, there is flexibility here, as 1.6.x only promises to build on Java 9 or later. If you are able to find a satisfactory configuration using plugin versions that will run on Java 9, then that's an easy call.

It becomes a harder call if it requires reneging on the Java 9 buildability commitment for PL/Java 1.6.x.

fmbiete commented 9 months ago

All those versions require Java 8, it would be ok for REL_1_6 if the minimum is Java 9. I fear that REL_1_5 should be impossible...

jcflack commented 9 months ago

If that's the case, then a PR based on REL1_6_STABLE (and with updated version comments:)) would be lovely.

This is the third year on the 1.6.x series; 1.5.x is pretty much left for masochists with very ancient requirements by this point anyway.

jcflack commented 9 months ago

... an ideal pull request would also make sure to change anything necessary in src/site/markdown/build/versions.md. I guess at the moment it only mentions the overall Maven version, so if that doesn't need a bump then maybe nothing needs to change there.

jcflack commented 2 months ago

Believed addressed (for the time being, anyway) by applying @fmbiete's changes in 1.6.7.

Note to self: those changes included a version bump for maven-site-plugin, which (it turned out, only after merging the changes) broke the documentation build. Something to remember to check in any future pull requests affecting maven-site-plugin!