search

karatelabs / karate

Test Automation Made Simple
https://karatelabs.github.io/karate
MIT License
8.33k stars 1.95k forks source link

upgrade to apache httpclient 5 #2296

Open ptrthomas opened 1 year ago

ptrthomas commented 1 year ago

low priority CVE: commons-codec:commons-codec @ 1.11 vulnerability reported by @kdefives

description:

Summary

Apache commons-codec before 1.13 is vulnerable to information exposure. The Base32 and Base64 implementation blindly decode invalid string, which can be re-encoded again using the same implementation. This can result in a security exploitation such as tunneling additional information via seemingly valid base 32 strings.

How to fix

Migrate from usage of org.apache.httpcomponents:httpclient to org.apache.httpcomponents.client5:httpclient5

migration is non-trivial: https://hc.apache.org/httpcomponents-client-5.2.x/migration-guide/migration-to-classic.html and requires refactoring of the ApacheHttpClient

currently tagging as help wanted

pru-namikaze commented 1 year ago

Hello @ptrthomas , I would like to contribute to this issue. Can I work on this?

ptrthomas commented 1 year ago

@pru-namikaze sure

milikkan commented 1 year ago

I hope this upgrade ships for the next release. We have recently updated to Spring Boot 3.1 in which support for Apache HttpClient4 is dropped (so we have only HttpClient5 on the classpath). Since Karate is using HttpClient4, transitive dependencies are causing a problem. We are not having problem running Karate tests on CI/CD, but when we run individual Karate tests during development (inside IntelliJ using the Karate plugin) tests are failing during initialization.

the error message is:

org.graalvm.polyglot.polyglotexception: 'java.lang.string org apache.http.client.utils.urlencodedutils.formatsegments(java.lang.iterable, java.nio.charset.charset)'
com.intuit.karate.core.ScenarioBridge.callSingle(ScenarioBridge.java:238)

We fixed it adding Apache HttpClient4 dependency explicitly in our projects dependency management. This helps ensuring Karate uses HttpClient4.

ptrthomas commented 1 year ago

@milikkan can you confirm that if you use <classifier>all</classifier> for your karate-core dependency, it should solve this problem. for an example, refer: https://github.com/karatelabs/karate-examples/blob/main/quarkus/README.md

pru-namikaze commented 1 year ago

Hi @ptrthomas,

I have been swamped with personal commitments and will not be able to complete this issue. However, I have pushed the majority of the changes which have passed all but 3 test cases (related to cookies, which should be an easy fix to just find the right implementation from hc5).

I apologize for not letting you know sooner.

rajeshsrinivas1991 commented 1 year ago

Hi Team, I am new to Karate and seeing this warning in my pom.xml file. Can someone suggest a fix? I went through the conversations and didn't understand. Thanks in advance.

Cxeb68d52e-5509 3.7 Exposure of Sensitive Information to an Unauthorized Actor vulnerability

OS: Mac OS Ventura 13.6 OpenJDK 17 Intellij IDE

Using Karate-core as dependency in my pom.xml file with version 1.4.0 and scope

ptrthomas commented 1 year ago

@rajeshsrinivas1991 see if this helps: https://stackoverflow.com/a/76399959/143475

else we have to wait for the apache http client to be upgraded. if anyone else has thoughts, please chime in

f-delahaye commented 11 months ago

I will try to have a look at this one, if it's ok.

Any requirements in terms of perfomance? I read somewhere that karate-gatling was using a special pooling/connection/whatever, so anything to check specifically?

ptrthomas commented 11 months ago

@f-delahaye thanks ! no particular requirements, karate-gatling would actually use this automatically

f-delahaye commented 11 months ago

Thanks.

It looks like apache http client 5.3 no longer supports ntlm: https://hc.apache.org/httpcomponents-client-5.3.x/current/httpclient5/apidocs/org/apache/hc/client5/http/impl/auth/NTLMScheme.html

Should we upgrade to client 5.3 and remove ntlm support altogether? Or upgrade to 5.2.1 and issue a warning, something like 'Will be removed in a future release"?

ptrthomas commented 11 months ago

tagging @dvargas46 who implemented this in https://github.com/karatelabs/karate/pull/2343

@f-delahaye I propose removing ntlm support. not many people use this, and supporting Apache and solving the CVE issues we have is a priority. hopefully the community can find a way to add ntlm back, perhaps the code in the apache project can be used as a reference

dvargas46 commented 11 months ago

Thanks for looping me in on this @ptrthomas . I agree that removing ntlm support is best here to be able to upgrade the apache client and resolve further CVEs. It's a bummer that apache deprecated support for ntlm, but it's understandable given the vulnerabilities around that auth scheme. If I find another way to bring ntlm support back to Karate then I'll create a PR.