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

High CVE's Present in karate-core #1834

Closed
packleader opened this issue Nov 12, 2021 · 17 comments
Closed

High CVE's Present in karate-core #1834

packleader opened this issue Nov 12, 2021 · 17 comments
Assignees
Labels
codequality dependencies Pull requests that update a dependency file fixed
Milestone

Comments

@packleader
Copy link
Contributor

Two high-severity CVE's are present in 1.2.0.RC1 of karate-core. My company has a strict policy that we are not permitted to use any open source projects that have vulnerabilities with a score >= 7.0. As a result, we will be unable to continue using Karate until the issue is resolved. I can't say with certainty, but I would image that many large corporations have similar policies in place.

Evidence

The official OWASP site recommends using the Dependency-Check Maven plugin to scan projects for vulnerabilities. This plugin can be applied two ways, as shown below.

Method 1: At compile-time, scan all of the dependencies used by Karate.

  1. Checkout this branch of Karate, which has been modified to include Dependency-Check.
  2. Build the project by running mvn clean verify from the root.
  3. Observe that the build fails with the message [ERROR] netty-transport-4.1.63.Final.jar: CVE-2021-37136, CVE-2021-37137

Method 2: Scan the karate-core library directly.

  1. Download the sample karate-dependency project.
  2. Unzip the pom.xml into a clean directory.
  3. Build the project by running mvn clean verify from the root.
  4. Observe that the build fails with the message [ERROR] karate-core-1.2.0.RC1.jar\META-INF/maven/io.netty/netty-transport/pom.xml: CVE-2021-37136, CVE-2021-37137

Analysis

Karate depends on io.netty:netty-transport:4.1.63.Final which is the subject of CVE-2021-37136 and CVE-2021-37137. Because karate-core is a shaded JAR, it inherits the CVE's of any JAR's which are packaged inside it. Thus, industry-standard scanners (such as Dependency-Check) will report that com.intuit.karate:karate-core:1.2.0.RC1 contains two CVE's.

Proposed Solution

Required: Update Karate's dependency list to include a newer version of Netty which does not have any open CVE's. More specifically, I propose updating com.linecorp.armeria:armeria from version 1.8.0 to 1.12.0 or higher in karate-core.

Optional: Build and publish two versions of the karate-core JAR - one standard, and one shaded. Providing a non-shaded version of karate-core will allow users (like me) to optionally specify our dependencies at runtime. This adds some overhead to the Karate project but provides a future-proof solution. With a shaded JAR, the dependencies are tightly-coupled. So when a new CVE is discovered in a dependency, the shaded JAR inherits that CVE and there's no way I can work around it. With a non-shaded JAR, I can mitigate the new CVE by explicitly declaring a newer version of the dependency in my POM.

Let me know your thoughts on the above, and I can start work on it.

@joelpramos
Copy link
Contributor

Great idea.

We could also set it up in the GitHub actions pipeline to get the results easily available.

https://github.com/snyk/actions/tree/master/maven

@ptrthomas
Copy link
Member

@packleader as a first step, please submit a PR for the armeria upgrade, lets confirm if the build passes

@ptrthomas ptrthomas added codequality dependencies Pull requests that update a dependency file labels Nov 13, 2021
@ptrthomas ptrthomas added this to the 1.2.0 milestone Nov 13, 2021
@ptrthomas ptrthomas self-assigned this Nov 24, 2021
ptrthomas added a commit that referenced this issue Nov 24, 2021
armeria became stricter which breaks mock + proxy functionality
taking the hit for now assuming hardly anyone uses that mode
@ptrthomas
Copy link
Member

build failed after armeria upgrade, just noting here for future reference:

05:52:17.181 [armeria-common-worker-epoll-2-1] DEBUG c.intuit.karate.http.RequestHandler - creating new session for '00': 1-1637733137181
05:52:17.573 [armeria-common-worker-epoll-2-1] DEBUG com.intuit.karate.http.RequestCycle - GET 00 200 [405 ms]
05:52:17.577 [armeria-common-worker-epoll-2-1] WARN  k.c.l.a.server.HttpServerHandler - [id: 0x4de95b12, L:/127.0.0.1:34525 - R:/127.0.0.1:52392][h1c] Unexpected exception:
java.lang.NoClassDefFoundError: com/fasterxml/jackson/core/JsonProcessingException
	at karate.com.linecorp.armeria.server.HttpServerHandler.handleRequest(HttpServerHandler.java:395)
	at karate.com.linecorp.armeria.server.HttpServerHandler.channelRead(HttpServerHandler.java:257)
	at karate.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at karate.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)

ptrthomas added a commit that referenced this issue Nov 24, 2021
ptrthomas added a commit that referenced this issue Nov 24, 2021
@ptrthomas
Copy link
Member

all: okay, I got the build to work, please validate. contributions for anything else welcome

@packleader
Copy link
Contributor Author

Thanks for proposing a fix. I had intended to work on this, but I've been having issues with my PC and haven't had time to fix them. I'll try to take a look at the PR between now and Saturday and provide any feedback.

@packleader
Copy link
Contributor Author

I merged the latest changes from develop into my test branch and re-ran the Dependency-Check as outlined in Method 1 above. The good news is that the findings with Netty are resolved. The bad news is that there's a new finding with Thymeleaf, and the build fails with [ERROR] thymeleaf-3.0.12.RELEASE.jar: CVE-2021-43466. This is the most recent version of Thymeleaf, so I'm not sure how to resolve the finding.

This brings up my earlier point that it may not be possible to remove all the high-severity CVE's from karate-core. The simplest way around it would be to offer a non-shaded JAR so that end-users can exclude or upgrade transitive dependencies to suit their needs. Another option would be to split karate-core into two modules - one for reading and executing the feature files and another for running the mock server.

Our team uses Karate to run functional and performance tests against live servers in our test environment, so we don't need the mock server functionality. In my experience, most CVE's are related to dependencies that enable running a simple server (Armeria, Thymeleaf, Netty, etc.) Moving those dependencies and the mock server code into another module would help isolate the CVE's and decouple them from the core test-execution capabilities of Karate. I recognize that this would be a significant change, so I'm curious to hear your thoughts.

@ptrthomas
Copy link
Member

@packleader sure, I'm open to making this structural change. one addition is that thymeleaf is needed for the html reports, so we may be talking about 3 JARS - karate-core, karate-html and karate-server. the bad news is that there are a few inter-connects, for e.g. armeria+netty classes (cookies, some other utils) are used for http request building.

there is a proposed refactor to clean-up cookies that may be relevant: #1708

I have to say that I personally will not have the bandwidth to look at this over the next four or so months. perhaps the best thing is for someone to contribute the non-shading first (and the github action for CVEs is nice to have) and I can of course review and support. the pre-release and release maven profiles will need some work.

ptrthomas added a commit that referenced this issue Dec 8, 2021
@ptrthomas
Copy link
Member

@packleader note that thymeleaf just released a version 3.0.14 specifically for security fixes: https://www.thymeleaf.org/releasenotes.html#thymeleaf-3.0.14

I've updated, in case that helps.

@packleader
Copy link
Contributor Author

I've submitted a PR to fix another finding and add the Dependency Checker plugin.

I hope to have another PR for the un-shaded JAR later this month.

I can tackle the task of splitting up karate-core, but not until January.

ptrthomas added a commit that referenced this issue Dec 17, 2021
Issue #1834 - Fix for High Severity CVE's in karate-core
@ptrthomas
Copy link
Member

@packleader thank you. the build did seem to pass on the second try. I moved the plugin into a new maven profile, so to run it we can do:

mvn clean verify -P depcheck

I'll add it to our release process checklist

@ptrthomas
Copy link
Member

@packleader we released 1.2.0.RC2 so please see if that looks good in your env: https://twitter.com/getkarate/status/1471710785051103233

@packleader
Copy link
Contributor Author

Thanks. I have confirmed that karate-core and karate-junit5 are free of any high-severity CVE's as of 1.2.0.RC2.

If you're able to release karate-gatling:1.2.0.RC2, then I can also test #1794.

@ptrthomas
Copy link
Member

@packleader great ! karate-gatling is always released at the same time, let me know: https://search.maven.org/artifact/com.intuit.karate/karate-gatling/1.2.0.RC2/jar

@packleader
Copy link
Contributor Author

I finally got around to working on the non-shaded JAR, and here's what I found. If we want to publish both a shaded and non-shaded version of the JAR, the official documentation states that the plugin supports having the non-shaded JAR as the primary (or default) artifact in Maven Central. The shaded JAR would then be published as a secondary artifact.

So the non-shaded JAR would be

<dependency>
    <groupId>com.intuit.karate</groupId>
    <artifactId>karate-core</artifactId>
    <version>1.2.0</version>
</dependency>

And the shaded JAR would be

<dependency>
    <groupId>com.intuit.karate</groupId>
    <artifactId>karate-core</artifactId>
    <version>1.2.0</version>
    <classifier>standalone</classifier>
</dependency>

If that works for you, then I can submit a PR with the necessary changes.

@ptrthomas
Copy link
Member

@packleader inviting comments from others.

so for those who need the shaded JAR will the artifact-id be karate-core-standalone. I think we can go with all instead of standalone. second choice would be shaded

@packleader
Copy link
Contributor Author

I'm flexible on the name; I think all should work just fine.

The artifact-id will still be karate-core. Those who need the shaded JAR will need to specify the optional classifier. Maven allows us to publish multiple artifacts under the same artifact-id by using classifiers. It's kind of like trim levels on a car. You can buy different variations of a Ford F150 - 2WD, 4WD, etc. The artifact-id is F150, and the classifier is 2WD or 4WD.

Both JARs would be published to Maven Central under com/intuit/karate/karate-core/1.2.0, and the filenames would be karate-core-1.2.0.jar and karate-core-1.2.0-all.jar.

I've opened #1916 so that you can test out the changes if you want.

ptrthomas added a commit that referenced this issue Mar 3, 2022
Issue #1834 - Publish Non-Shaded JAR For karate-core
@ptrthomas
Copy link
Member

1.2.0 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codequality dependencies Pull requests that update a dependency file fixed
Projects
None yet
Development

No branches or pull requests

3 participants