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

Patch Shell class in hdfs to not execute #119189

Merged
merged 11 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ if (providers.systemProperty('idea.active').getOrNull() == 'true') {
tasks.register('buildDependencyArtifacts') {
group = 'ide'
description = 'Builds artifacts needed as dependency for IDE modules'
dependsOn([':plugins:repository-hdfs:hadoop-client-api:shadowJar',
dependsOn([':plugins:repository-hdfs:hadoop-client-api:jar',
':x-pack:plugin:esql:compute:ann:jar',
':x-pack:plugin:esql:compute:gen:jar',
':server:generateModulesList',
Expand Down
2 changes: 1 addition & 1 deletion plugins/repository-hdfs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ configurations {
}

dependencies {
api project(path: 'hadoop-client-api', configuration: 'shadow')
api project(path: 'hadoop-client-api', configuration: 'default')
if (isEclipse) {
/*
* Eclipse can't pick up the shadow dependency so we point it at *something*
Expand Down
47 changes: 39 additions & 8 deletions plugins/repository-hdfs/hadoop-client-api/build.gradle
Original file line number Diff line number Diff line change
@@ -1,16 +1,47 @@
apply plugin: 'elasticsearch.build'
apply plugin: 'com.gradleup.shadow'
apply plugin: 'elasticsearch.java'

sourceSets {
patcher
}

configurations {
thejar {
canBeResolved = true
}
}

dependencies {
implementation "org.apache.hadoop:hadoop-client-api:${project.parent.versions.hadoop}"
thejar("org.apache.hadoop:hadoop-client-api:${project.parent.versions.hadoop}") {
transitive = false
}

patcherImplementation 'org.ow2.asm:asm:9.7.1'
patcherImplementation 'org.ow2.asm:asm-tree:9.7.1'
}

tasks.named('shadowJar').configure {
exclude 'org/apache/hadoop/util/ShutdownHookManager$*.class'
def outputDir = layout.buildDirectory.dir("patched-classes")

def patchTask = tasks.register("patchClasses", JavaExec) {
inputs.files(configurations.thejar).withPathSensitivity(PathSensitivity.RELATIVE)
inputs.files(sourceSets.patcher.output).withPathSensitivity(PathSensitivity.RELATIVE)
outputs.dir(outputDir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We define outputs but no inputs here. A dependsOn doesn't imply an input, but an input does imply a dependency.

Sounds like configurations.thejar and sourcesets.patcher.output should be inputs to this task.

classpath = sourceSets.patcher.runtimeClasspath
mainClass = 'org.elasticsearch.hdfs.patch.HdfsClassPatcher'
doFirst {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to call args() with a Provider to avoid having to do this in a task action.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, I've kept this as doFirst for now.

args(configurations.thejar.singleFile, outputDir.get().asFile)
}
}

['jarHell', 'thirdPartyAudit', 'forbiddenApisMain', 'splitPackagesAudit'].each {
tasks.named(it).configure {
enabled = false
tasks.named('jar').configure {
dependsOn(patchTask)
dependsOn(configurations.thejar)

from(outputDir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can actually just become from(patchTask) which will automatically use the task's outputs and you can remove the dependsOn above.

from({ project.zipTree(configurations.thejar.singleFile) }) {
eachFile {
if (outputDir.get().file(it.relativePath.pathString).asFile.exists()) {
it.exclude()
}
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.hdfs.patch;

import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Map;
import java.util.function.Function;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;

public class HdfsClassPatcher {
static final Map<String, Function<ClassWriter, ClassVisitor>> patchers = Map.of(
"org/apache/hadoop/util/ShutdownHookManager.class",
ShutdownHookManagerPatcher::new,
"org/apache/hadoop/util/Shell.class",
ShellPatcher::new
);

public static void main(String[] args) throws Exception {
String jarPath = args[0];
Path outputDir = Paths.get(args[1]);

try (JarFile jarFile = new JarFile(new File(jarPath))) {
for (var patcher : patchers.entrySet()) {
JarEntry jarEntry = jarFile.getJarEntry(patcher.getKey());
if (jarEntry == null) {
throw new IllegalArgumentException("path [" + patcher.getKey() + "] not found in [" + jarPath + "]");
}
byte[] classToPatch = jarFile.getInputStream(jarEntry).readAllBytes();

ClassReader classReader = new ClassReader(classToPatch);
ClassWriter classWriter = new ClassWriter(classReader, 0);
classReader.accept(patcher.getValue().apply(classWriter), 0);

Path outputFile = outputDir.resolve(patcher.getKey());
Files.createDirectories(outputFile.getParent());
Files.write(outputFile, classWriter.toByteArray());
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.hdfs.patch;

import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;

public class MethodReplacement extends MethodVisitor {
private final MethodVisitor delegate;
private final Runnable bodyWriter;

MethodReplacement(MethodVisitor delegate, Runnable bodyWriter) {
super(Opcodes.ASM9);
this.delegate = delegate;
this.bodyWriter = bodyWriter;
}

@Override
public void visitCode() {
// delegate.visitCode();
bodyWriter.run();
// delegate.visitEnd();
}

@Override
public void visitMaxs(int maxStack, int maxLocals) {
delegate.visitMaxs(maxStack, maxLocals);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.hdfs.patch;

import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;

class ShellPatcher extends ClassVisitor {

ShellPatcher(ClassWriter classWriter) {
super(Opcodes.ASM9, classWriter);
}

@Override
public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) {
MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions);
if (name.equals("isSetsidSupported")) {
return new MethodReplacement(mv, () -> {
mv.visitInsn(Opcodes.ICONST_0);
mv.visitInsn(Opcodes.IRETURN);
});
}
return mv;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.hdfs.patch;

import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;

import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;

class ShutdownHookManagerPatcher extends ClassVisitor {
private static final String CLASSNAME = "org/apache/hadoop/util/ShutdownHookManager";
private static final Set<String> VOID_METHODS = Set.of("addShutdownHook", "clearShutdownHooks");
private static final Set<String> BOOLEAN_METHODS = Set.of("removeShutdownHook", "hasShutdownHook", "isShutdownInProgress");

ShutdownHookManagerPatcher(ClassWriter classWriter) {
super(Opcodes.ASM9, classWriter);
}

@Override
public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) {
MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions);
if (VOID_METHODS.contains(name)) {
// make void methods noops
return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.RETURN); });
} else if (BOOLEAN_METHODS.contains(name)) {
// make boolean methods always return false
return new MethodReplacement(mv, () -> {
mv.visitInsn(Opcodes.ICONST_0);
mv.visitInsn(Opcodes.IRETURN);
});
} else if (name.equals("<clinit>")) {
return new MethodReplacement(mv, () -> {
// just initialize the statics, don't actually get runtime to add shutdown hook

var classType = Type.getObjectType(CLASSNAME);
mv.visitTypeInsn(Opcodes.NEW, CLASSNAME);
mv.visitInsn(Opcodes.DUP);
mv.visitMethodInsn(Opcodes.INVOKESPECIAL, CLASSNAME, "<init>", "()V", false);
mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "MGR", classType.getDescriptor());

var timeUnitType = Type.getType(TimeUnit.class);
mv.visitFieldInsn(Opcodes.GETSTATIC, timeUnitType.getInternalName(), "SECONDS", timeUnitType.getDescriptor());
mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "TIME_UNIT_DEFAULT", timeUnitType.getDescriptor());

var executorServiceType = Type.getType(ExecutorService.class);
mv.visitInsn(Opcodes.ACONST_NULL);
mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "EXECUTOR", executorServiceType.getDescriptor());

mv.visitInsn(Opcodes.RETURN);
});
}
return mv;
}
}