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

Remove JndiLookup.class from the SQL command line tool #81776

Closed
ChrisHegarty opened this issue Dec 15, 2021 · 4 comments · Fixed by #81879
Closed

Remove JndiLookup.class from the SQL command line tool #81776

ChrisHegarty opened this issue Dec 15, 2021 · 4 comments · Fixed by #81879
Labels
:Analytics/SQL SQL querying >bug :Core/Infra/CLI CLI utilities, scripts, and infrastructure Team:Core/Infra Meta label for core/infra team Team:QL (Deprecated) Meta label for query languages team

Comments

@ChrisHegarty
Copy link
Contributor

ChrisHegarty commented Dec 15, 2021

Elasticsearch ships with a SQL CLI Client in its bin directory. This client tool is standalone (NOT part of the server), for running ad-hoc SQL interactions. The jar containing the client is not on the servers class path. As a client tool, it does NOT accept external user input from the network, in a way that a server would typically do.

The jar containing the SQL CLI is a stand alone Java application, and can be run as such. Alternatively, the jar could also be used as a client.

This issue has been filed to consider the possible removal of JndiLookup.class from the SQL CLI jar. The mere presence of the JndiLookup.class is not problematic here, but it looks interesting and could lead to confusion for scanners, e.g.

unzip -l build/distributions/elasticsearch-sql-cli-8.1.0-SNAPSHOT.jar | grep JndiLookup
     2937  07-22-2018 20:45   org/apache/logging/log4j/core/lookup/JndiLookup.class

Alternatively, or additionally, upgrade the version of log4j that is bundled in the uber executable jar.

@ChrisHegarty ChrisHegarty added >bug needs:triage Requires assignment of a team area label labels Dec 15, 2021
@ChrisHegarty
Copy link
Contributor Author

Check if the version of log4j used by the tool the same as the server, and affected by the upgrade in #81709

@pgomulka
Copy link
Contributor

I think the version of log4j in this CLI tool will be the same as we use in ES. So given we will upgrade to 2.16 in master we might not need this? Or do you want to remove JndiLookup in versions <8 ?

I tested with an upgrade to 2.16 and I got this
./gradlew -q :x-pack:plugin:sql:sql-cli:dependencies

compileClasspath - Compile classpath for source set 'main'.
...
+--- project :x-pack:plugin:sql:sql-action
...
|    +--- project :x-pack:plugin:core
|    |    +--- project :libs:elasticsearch-ssl-config
|    |    |    \--- project :libs:elasticsearch-core
...
|    |    +--- org.apache.logging.log4j:log4j-1.2-api:2.16.0
...
runtimeClasspath - Runtime classpath of source set 'main'.
...
|    \--- com.fasterxml.jackson.core:jackson-core:2.10.4
+--- project :x-pack:plugin:sql:sql-action
...
|    +--- project :x-pack:plugin:core
...
|    |    +--- org.apache.logging.log4j:log4j-1.2-api:2.16.0
...
|    +--- org.apache.logging.log4j:log4j-api:2.16.0
|    \--- org.apache.logging.log4j:log4j-core:2.16.0
...

When I extract the elasticsearch-sql-cli-8.1.0-SNAPSHOT.jar I also see 2.16 in pom files in META-INF

@gwbrown gwbrown added :Core/Infra/CLI CLI utilities, scripts, and infrastructure :Analytics/SQL SQL querying and removed needs:triage Requires assignment of a team area label labels Dec 16, 2021
@elasticmachine elasticmachine added Team:Core/Infra Meta label for core/infra team Team:QL (Deprecated) Meta label for query languages team labels Dec 16, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying >bug :Core/Infra/CLI CLI utilities, scripts, and infrastructure Team:Core/Infra Meta label for core/infra team Team:QL (Deprecated) Meta label for query languages team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants