Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions libs/entitlement/asm-provider/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ apply plugin: 'elasticsearch.build'
dependencies {
compileOnly project(':libs:entitlement')
compileOnly project(':libs:core')
compileOnly project(':libs:logging')
implementation 'org.ow2.asm:asm:9.7.1'
testImplementation project(":test:framework")
testImplementation project(":libs:entitlement:bridge")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
requires org.elasticsearch.entitlement;

requires static org.elasticsearch.base; // for SuppressForbidden
requires static org.elasticsearch.logging;

provides InstrumentationService with InstrumentationServiceImpl;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.elasticsearch.entitlement.instrumentation.CheckMethod;
import org.elasticsearch.entitlement.instrumentation.Instrumenter;
import org.elasticsearch.entitlement.instrumentation.MethodKey;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
Expand All @@ -36,6 +38,7 @@
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;

public class InstrumenterImpl implements Instrumenter {
private static final Logger logger = LogManager.getLogger(InstrumenterImpl.class);

private final String getCheckerClassMethodDescriptor;
private final String handleClass;
Expand Down Expand Up @@ -155,10 +158,10 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str
var key = new MethodKey(className, name, Stream.of(Type.getArgumentTypes(descriptor)).map(Type::getInternalName).toList());
var instrumentationMethod = checkMethods.get(key);
if (instrumentationMethod != null) {
// System.out.println("Will instrument method " + key);
logger.debug("Will instrument {}", key);
Copy link
Member

Choose a reason for hiding this comment

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

Can you please split this out? We've talked about it across several PRs, let's do it, but as distinct commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the controversy was using exceptions to get a stack trace.

Copy link
Member

Choose a reason for hiding this comment

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

There's no controversy, I just think adding debug logging to instrumentation is unrelated to adding miscellaneous file entitlements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

return new EntitlementMethodVisitor(Opcodes.ASM9, mv, isStatic, isCtor, descriptor, instrumentationMethod);
} else {
// System.out.println("Will not instrument method " + key);
logger.trace("Will not instrument {}", key);
}
}
return mv;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.UserPrincipal;
import java.nio.file.spi.FileSystemProvider;
import java.security.KeyStore;
import java.security.Provider;
import java.security.cert.CertStoreParameters;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -621,12 +623,50 @@ public interface EntitlementChecker {

void check$java_io_RandomAccessFile$(Class<?> callerClass, File file, String mode);

void check$java_security_KeyStore$$getInstance(Class<?> callerClass, File file, char[] password);

void check$java_security_KeyStore$$getInstance(Class<?> callerClass, File file, KeyStore.LoadStoreParameter param);

void check$java_security_KeyStore$Builder$$newInstance(Class<?> callerClass, File file, KeyStore.ProtectionParameter protection);

void check$java_security_KeyStore$Builder$$newInstance(
Class<?> callerClass,
String type,
Provider provider,
File file,
KeyStore.ProtectionParameter protection
);

void check$java_util_Scanner$(Class<?> callerClass, File source);

void check$java_util_Scanner$(Class<?> callerClass, File source, String charsetName);

void check$java_util_Scanner$(Class<?> callerClass, File source, Charset charset);

void check$java_util_jar_JarFile$(Class<?> callerClass, String name);

void check$java_util_jar_JarFile$(Class<?> callerClass, String name, boolean verify);

void check$java_util_jar_JarFile$(Class<?> callerClass, File file);

void check$java_util_jar_JarFile$(Class<?> callerClass, File file, boolean verify);

void check$java_util_jar_JarFile$(Class<?> callerClass, File file, boolean verify, int mode);

void check$java_util_jar_JarFile$(Class<?> callerClass, File file, boolean verify, int mode, Runtime.Version version);

void check$java_util_zip_ZipFile$(Class<?> callerClass, String name);

void check$java_util_zip_ZipFile$(Class<?> callerClass, String name, Charset charset);

void check$java_util_zip_ZipFile$(Class<?> callerClass, File file);

void check$java_util_zip_ZipFile$(Class<?> callerClass, File file, int mode);

void check$java_util_zip_ZipFile$(Class<?> callerClass, File file, Charset charset);

void check$java_util_zip_ZipFile$(Class<?> callerClass, File file, int mode, Charset charset);

// nio
void check$java_nio_file_Files$$getOwner(Class<?> callerClass, Path path, LinkOption... options);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.io.File;
import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.FileWriter;
Expand All @@ -26,12 +25,12 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.UserPrincipal;
import java.util.Scanner;

import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.ALWAYS_DENIED;
import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.PLUGINS;

@SuppressForbidden(reason = "Explicitly checking APIs that are forbidden")
@SuppressWarnings("unused") // Called via reflection
class FileCheckActions {

static Path testRootDir = Paths.get(System.getProperty("es.entitlements.testdir"));
Expand Down Expand Up @@ -207,21 +206,6 @@ static void fileSetWritableOwner() throws IOException {
readWriteFile().toFile().setWritable(true, false);
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createScannerFile() throws FileNotFoundException {
Copy link
Member

Choose a reason for hiding this comment

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

Aren't all of the rest of the methods in this file also in java.base? I don't understand the distinction as to why they belong in a separate files vs in this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I didn't check. I just moved over the ones that were on the spreadsheet tab.

What would you like to do? Merge them together?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe split them based on some explainable attributes? I don't feel strongly, but it seems like having a test file that implies java.base classes should be tested in it, yet other test files contain java.base classes, will cause confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I can explain why these ended up in their own spreadsheet tab. Maybe I'll just merge them into FileCheckActions.

new Scanner(readFile().toFile());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createScannerFileWithCharset() throws IOException {
new Scanner(readFile().toFile(), StandardCharsets.UTF_8);
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createScannerFileWithCharsetName() throws FileNotFoundException {
new Scanner(readFile().toFile(), "UTF-8");
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createFileInputStreamFile() throws IOException {
new FileInputStream(readFile().toFile()).close();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/*
* 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.entitlement.qa.test;

import org.elasticsearch.core.CheckedRunnable;
import org.elasticsearch.core.SuppressForbidden;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.util.Scanner;
import java.util.jar.JarFile;
import java.util.zip.ZipException;
import java.util.zip.ZipFile;

import static java.nio.charset.Charset.defaultCharset;
import static java.util.zip.ZipFile.OPEN_DELETE;
import static java.util.zip.ZipFile.OPEN_READ;
import static org.elasticsearch.entitlement.qa.entitled.EntitledActions.createTempFileForWrite;
import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.PLUGINS;
import static org.elasticsearch.entitlement.qa.test.FileCheckActions.readFile;

@SuppressForbidden(reason = "Explicitly checking APIs that are forbidden")
@SuppressWarnings("unused") // Called via reflection
public class JavaBaseFileActions {
@EntitlementTest(expectedAccess = PLUGINS)
static void keystore_getInstance_1() throws IOException {
try {
KeyStore.getInstance(readFile().toFile(), new char[0]);
} catch (GeneralSecurityException expected) {
return;
}
throw new AssertionError("Expected an exception");
}

@EntitlementTest(expectedAccess = PLUGINS)
static void keystore_getInstance_2() throws IOException {
try {
KeyStore.LoadStoreParameter loadStoreParameter = () -> null;
KeyStore.getInstance(readFile().toFile(), loadStoreParameter);
} catch (GeneralSecurityException expected) {
return;
}
throw new AssertionError("Expected an exception");
}

@EntitlementTest(expectedAccess = PLUGINS)
static void keystoreBuilder_newInstance() {
try {
KeyStore.Builder.newInstance("", null, readFile().toFile(), null);
} catch (NullPointerException expected) {
return;
}
throw new AssertionError("Expected an exception");
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_1() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

In other tests we've named these logically based on the parameters. Could we do that here too?

expectZipException(() -> new ZipFile(readFile().toString()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_2() throws IOException {
expectZipException(() -> new ZipFile(readFile().toString(), defaultCharset()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_3() throws IOException {
expectZipException(() -> new ZipFile(readFile().toFile()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_4() throws IOException {
expectZipException(() -> new ZipFile(readFile().toFile(), defaultCharset()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_5_readOnly() throws IOException {
expectZipException(() -> new ZipFile(readFile().toFile(), OPEN_READ).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_5_readAndDelete() throws IOException {
expectZipException(() -> new ZipFile(createTempFileForWrite().toFile(), OPEN_READ | OPEN_DELETE).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void zipFile_6_readOnly() throws IOException {
expectZipException(() -> new ZipFile(readFile().toFile(), OPEN_READ, defaultCharset()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_1() throws IOException {
expectZipException(() -> new JarFile(readFile().toString()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_2() throws IOException {
expectZipException(() -> new JarFile(readFile().toString(), false).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_3_readOnly() throws IOException {
expectZipException(() -> new JarFile(readFile().toFile(), false, OPEN_READ).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_3_readAndDelete() throws IOException {
expectZipException(() -> new JarFile(createTempFileForWrite().toFile(), false, OPEN_READ | OPEN_DELETE).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_4_readOnly() throws IOException {
expectZipException(() -> new JarFile(readFile().toFile(), false, OPEN_READ, Runtime.version()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_4_readAndDelete() throws IOException {
expectZipException(() -> new JarFile(createTempFileForWrite().toFile(), false, OPEN_READ | OPEN_DELETE, Runtime.version()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_5() throws IOException {
expectZipException(() -> new JarFile(readFile().toFile()).close());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void jarFile_6() throws IOException {
expectZipException(() -> new JarFile(readFile().toFile(), false).close());
}

private static void expectZipException(CheckedRunnable<IOException> action) throws IOException {
try {
action.run();
} catch (ZipException expected) {
return;
}
throw new AssertionError("Expected an exception");
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createScannerFile() throws FileNotFoundException {
new Scanner(readFile().toFile());
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createScannerFileWithCharset() throws IOException {
new Scanner(readFile().toFile(), StandardCharsets.UTF_8);
}

@EntitlementTest(expectedAccess = PLUGINS)
static void createScannerFileWithCharsetName() throws FileNotFoundException {
new Scanner(readFile().toFile(), "UTF-8");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ static CheckAction alwaysDenied(CheckedRunnable<Exception> action) {
),
getTestEntries(FileCheckActions.class),
getTestEntries(FileStoreActions.class),
getTestEntries(JavaBaseFileActions.class),
getTestEntries(ManageThreadsActions.class),
getTestEntries(NativeActions.class),
getTestEntries(NioFileSystemActions.class),
Expand Down
Loading