Add option to JarInfer to deal with signed jars. (#345)
This makes two changes to the JarInfer CLI tool:
1) By default, the tool will error out when it encounters a signed jar
(as per the `jarsigner` tool), since modifying a signed jar's bytecode
and having it still verify is impossible. Users can then exclude such
jars from the list of jars sent the tool runs on.
2) We add an optional flag `--strip-jar-signatures` for users who
explicitly want to run JarInfer on such jars, removing the signature
in the process.
This change includes a test, but it might be fragile outside of JDK8
due to the fact that the `jarsigner` tool's programatic API is not
stable. This instability does not affect the JarInfer tool, just the test.
diff --git a/jar-infer/jar-infer-cli/src/main/java/com/uber/nullaway/jarinfer/JarInfer.java b/jar-infer/jar-infer-cli/src/main/java/com/uber/nullaway/jarinfer/JarInfer.java
index 1ca01f9..3146a72 100644
--- a/jar-infer/jar-infer-cli/src/main/java/com/uber/nullaway/jarinfer/JarInfer.java
+++ b/jar-infer/jar-infer-cli/src/main/java/com/uber/nullaway/jarinfer/JarInfer.java
@@ -67,6 +67,12 @@
.desc("annotate bytecode")
.build());
options.addOption(
+ Option.builder("s")
+ .argName("strip-jar-signatures")
+ .longOpt("strip-jar-signatures")
+ .desc("handle signed jars by removing signature information from META-INF/")
+ .build());
+ options.addOption(
Option.builder("h")
.argName("help")
.longOpt("help")
@@ -90,13 +96,14 @@
String pkgName = line.getOptionValue('p', "");
String outPath = line.getOptionValue('o');
boolean annotateBytecode = line.hasOption('b');
+ boolean stripJarSignatures = line.hasOption('s');
boolean debug = line.hasOption('d');
boolean verbose = line.hasOption('v');
if (!pkgName.isEmpty()) {
pkgName = "L" + pkgName.replaceAll("\\.", "/");
}
DefinitelyDerefedParamsDriver driver = new DefinitelyDerefedParamsDriver();
- driver.run(jarPath, pkgName, outPath, annotateBytecode, debug, verbose);
+ driver.run(jarPath, pkgName, outPath, annotateBytecode, stripJarSignatures, debug, verbose);
if (!new File(outPath).exists()) {
System.out.println("Could not write jar file: " + outPath);
}
diff --git a/jar-infer/jar-infer-lib/build.gradle b/jar-infer/jar-infer-lib/build.gradle
index 4bf1109..fcc6082 100644
--- a/jar-infer/jar-infer-lib/build.gradle
+++ b/jar-infer/jar-infer-lib/build.gradle
@@ -24,6 +24,7 @@
}
testCompile project(":jar-infer:test-java-lib-jarinfer")
testCompile project(path: ":jar-infer:test-android-lib-jarinfer", configuration: "default")
+ testCompile files("${System.properties['java.home']}/../lib/tools.jar") // is there a better way?
testRuntime deps.build.errorProneCheckApi
}
diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java
index bd9f1a5..675a6a7 100644
--- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java
+++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java
@@ -15,9 +15,11 @@
*/
package com.uber.nullaway.jarinfer;
+import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InputStreamReader;
import java.io.OutputStream;
import java.util.Enumeration;
import java.util.List;
@@ -48,6 +50,19 @@
public static final String javaxNullableDesc = "Ljavax/annotation/Nullable;";
public static final String javaxNonnullDesc = "Ljavax/annotation/Nonnull;";
+ // Constants used for signed jar processing
+ private static final String SIGNED_JAR_ERROR_MESSAGE =
+ "JarInfer will not process signed jars by default. "
+ + "Please take one of the following actions:\n"
+ + "\t1) Remove the signature from the original jar before passing it to jarinfer,\n"
+ + "\t2) Pass the --strip-jar-signatures flag to JarInfer and the tool will remove signature "
+ + "metadata for you, or\n"
+ + "\t3) Exclude this jar from those being processed by JarInfer.";
+ private static final String BASE64_PATTERN =
+ "(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?";
+ private static final String DIGEST_ENTRY_PATTERN =
+ "Name: [A-Za-z0-9/\\$\\n\\s\\-\\.]+[A-Za-z0-9]\\nSHA-256-Digest: " + BASE64_PATTERN;
+
private static void addAnnotationIfNotPresent(
List<AnnotationNode> annotationList, String annotation) {
for (AnnotationNode node : annotationList) {
@@ -131,6 +146,49 @@
annotateBytecode(is, os, nonnullParams, nullableReturns);
}
+ private static void copyAndAnnotateJarEntry(
+ JarEntry jarEntry,
+ InputStream is,
+ JarOutputStream jarOS,
+ MethodParamAnnotations nonnullParams,
+ MethodReturnAnnotations nullableReturns,
+ boolean stripJarSignatures)
+ throws IOException {
+ String entryName = jarEntry.getName();
+ if (entryName.endsWith(".class")) {
+ jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
+ annotateBytecode(is, jarOS, nonnullParams, nullableReturns);
+ } else if (entryName.equals("META-INF/MANIFEST.MF")) {
+ // Read full file
+ StringBuilder stringBuilder = new StringBuilder();
+ BufferedReader br = new BufferedReader(new InputStreamReader(is, "UTF-8"));
+ String currentLine;
+ while ((currentLine = br.readLine()) != null) {
+ stringBuilder.append(currentLine + "\n");
+ }
+ String manifestText = stringBuilder.toString();
+ // Check for evidence of jar signing, note that lines can be split if too long so regex
+ // matching line by line will have false negatives.
+ String manifestMinusDigests = manifestText.replaceAll(DIGEST_ENTRY_PATTERN, "");
+ if (!manifestText.equals(manifestMinusDigests) && !stripJarSignatures) {
+ throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);
+ }
+ jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
+ jarOS.write(manifestMinusDigests.getBytes("UTF-8"));
+ } else if (entryName.startsWith("META-INF/")
+ && (entryName.endsWith(".DSA")
+ || entryName.endsWith(".RSA")
+ || entryName.endsWith(".SF"))) {
+ if (!stripJarSignatures) {
+ throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);
+ } // the case where stripJarSignatures==true is handled by default by skipping these files
+ } else {
+ jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
+ jarOS.write(IOUtils.toByteArray(is));
+ }
+ jarOS.closeEntry();
+ }
+
/**
* Annotates the methods and method parameters in the classes in the given jar with the specified
* annotations.
@@ -147,6 +205,7 @@
JarOutputStream jarOS,
MethodParamAnnotations nonnullParams,
MethodReturnAnnotations nullableReturns,
+ boolean stripJarSignatures,
boolean debug)
throws IOException {
BytecodeAnnotator.debug = debug;
@@ -158,13 +217,8 @@
for (Enumeration<JarEntry> entries = inputJar.entries(); entries.hasMoreElements(); ) {
JarEntry jarEntry = entries.nextElement();
InputStream is = inputJar.getInputStream(jarEntry);
- jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
- if (jarEntry.getName().endsWith(".class")) {
- annotateBytecode(is, jarOS, nonnullParams, nullableReturns);
- } else {
- jarOS.write(IOUtils.toByteArray(is));
- }
- jarOS.closeEntry();
+ copyAndAnnotateJarEntry(
+ jarEntry, is, jarOS, nonnullParams, nullableReturns, stripJarSignatures);
}
}
@@ -184,6 +238,7 @@
ZipOutputStream zipOS,
MethodParamAnnotations nonnullParams,
MethodReturnAnnotations nullableReturns,
+ boolean stripJarSignatures,
boolean debug)
throws IOException {
BytecodeAnnotator.debug = debug;
@@ -201,16 +256,11 @@
ByteArrayOutputStream byteArrayOS = new ByteArrayOutputStream();
JarOutputStream jarOS = new JarOutputStream(byteArrayOS);
while (inputJarEntry != null) {
-
- jarOS.putNextEntry(new ZipEntry(inputJarEntry.getName()));
- if (inputJarEntry.getName().endsWith(".class")) {
- annotateBytecode(jarIS, jarOS, nonnullParams, nullableReturns);
- } else {
- jarOS.write(IOUtils.toByteArray(jarIS));
- }
- jarOS.closeEntry();
+ copyAndAnnotateJarEntry(
+ inputJarEntry, jarIS, jarOS, nonnullParams, nullableReturns, stripJarSignatures);
inputJarEntry = jarIS.getNextJarEntry();
}
+ jarOS.close();
zipOS.write(byteArrayOS.toByteArray());
} else {
zipOS.write(IOUtils.toByteArray(is));
diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java
index 8bc88d0..7857026 100644
--- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java
+++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java
@@ -83,6 +83,7 @@
private MethodReturnAnnotations nullableReturns = new MethodReturnAnnotations();
private boolean annotateBytecode = false;
+ private boolean stripJarSignatures = false;
private static final String DEFAULT_ASTUBX_LOCATION = "META-INF/nullaway/jarinfer.astubx";
private static final String ASTUBX_JAR_SUFFIX = ".astubx.jar";
@@ -123,12 +124,18 @@
} else if (new File(firstInPath).exists()) {
outPath = FilenameUtils.getFullPath(firstInPath) + DEFAULT_ASTUBX_LOCATION;
}
- return run(inPaths, pkgName, outPath, false, DEBUG, VERBOSE);
+ return run(inPaths, pkgName, outPath, false, false, DEBUG, VERBOSE);
+ }
+
+ MethodParamAnnotations runAndAnnotate(
+ String inPaths, String pkgName, String outPath, boolean stripJarSignatures)
+ throws IOException, ClassHierarchyException {
+ return run(inPaths, pkgName, outPath, true, stripJarSignatures, DEBUG, VERBOSE);
}
MethodParamAnnotations runAndAnnotate(String inPaths, String pkgName, String outPath)
throws IOException, ClassHierarchyException {
- return run(inPaths, pkgName, outPath, true, DEBUG, VERBOSE);
+ return runAndAnnotate(inPaths, pkgName, outPath, false);
}
/**
@@ -151,12 +158,14 @@
String pkgName,
String outPath,
boolean annotateBytecode,
+ boolean stripJarSignatures,
boolean dbg,
boolean vbs)
throws IOException, ClassHierarchyException {
DEBUG = dbg;
VERBOSE = vbs;
this.annotateBytecode = annotateBytecode;
+ this.stripJarSignatures = stripJarSignatures;
Set<String> setInPaths = new HashSet<>(Arrays.asList(inPaths.split(",")));
analysisStartTime = System.currentTimeMillis();
for (String inPath : setInPaths) {
@@ -407,12 +416,14 @@
if (inPath.endsWith(".jar")) {
JarFile jar = new JarFile(inPath);
JarOutputStream jarOS = new JarOutputStream(new FileOutputStream(outFile));
- BytecodeAnnotator.annotateBytecodeInJar(jar, jarOS, nonnullParams, nullableReturns, DEBUG);
+ BytecodeAnnotator.annotateBytecodeInJar(
+ jar, jarOS, nonnullParams, nullableReturns, stripJarSignatures, DEBUG);
jarOS.close();
} else if (inPath.endsWith(".aar")) {
ZipFile zip = new ZipFile(inPath);
ZipOutputStream zipOS = new ZipOutputStream(new FileOutputStream(outFile));
- BytecodeAnnotator.annotateBytecodeInAar(zip, zipOS, nonnullParams, nullableReturns, DEBUG);
+ BytecodeAnnotator.annotateBytecodeInAar(
+ zip, zipOS, nonnullParams, nullableReturns, stripJarSignatures, DEBUG);
zipOS.close();
} else {
InputStream is = new FileInputStream(inPath);
diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/SignedJarException.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/SignedJarException.java
new file mode 100644
index 0000000..5e4e385
--- /dev/null
+++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/SignedJarException.java
@@ -0,0 +1,8 @@
+package com.uber.nullaway.jarinfer;
+
+public class SignedJarException extends SecurityException {
+
+ public SignedJarException(String message) {
+ super(message);
+ }
+}
diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/EntriesComparator.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/EntriesComparator.java
index c8fa9a0..38878d5 100644
--- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/EntriesComparator.java
+++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/EntriesComparator.java
@@ -16,7 +16,9 @@
package com.uber.nullaway.jarinfer;
import com.google.common.base.Preconditions;
+import java.io.BufferedReader;
import java.io.IOException;
+import java.io.InputStreamReader;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;
@@ -104,4 +106,41 @@
}
return jar1Entries.equals(jar2Entries);
}
+
+ private static String readManifestFromJar(String jarfile) throws IOException {
+ JarFile jar = new JarFile(jarfile);
+ ZipEntry manifestEntry = jar.getEntry("META-INF/MANIFEST.MF");
+ if (manifestEntry == null) {
+ throw new IllegalArgumentException("Jar does not contain a manifest at META-INF/MANIFEST.MF");
+ }
+ StringBuilder stringBuilder = new StringBuilder();
+ BufferedReader bufferedReader =
+ new BufferedReader(new InputStreamReader(jar.getInputStream(manifestEntry), "UTF-8"));
+ String currentLine;
+ while ((currentLine = bufferedReader.readLine()) != null) {
+ // Ignore empty new lines
+ if (currentLine.trim().length() > 0) {
+ stringBuilder.append(currentLine + "\n");
+ }
+ }
+ return stringBuilder.toString();
+ }
+
+ /**
+ * Compares the META-INF/MANIFEST.MF file in the given 2 jar files. We ignore empty newlines.
+ *
+ * @param jarFile1 Path to the first jar file.
+ * @param jarFile2 Path to the second jar file.
+ * @return True iff the MANIFEST.MF files in the two jar files exist and are the same.
+ * @throws IOException
+ * @throws IllegalArgumentException
+ */
+ public static boolean compareManifestContents(String jarFile1, String jarFile2)
+ throws IOException {
+ Preconditions.checkArgument(jarFile1.endsWith(".jar"), "invalid jar file: " + jarFile1);
+ Preconditions.checkArgument(jarFile2.endsWith(".jar"), "invalid jar file: " + jarFile2);
+ String manifest1 = readManifestFromJar(jarFile1);
+ String manifest2 = readManifestFromJar(jarFile2);
+ return manifest1.equals(manifest2);
+ }
}
diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java
index 6ccba9f..d39b120 100644
--- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java
+++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java
@@ -24,6 +24,9 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
import java.security.MessageDigest;
import java.util.Arrays;
import java.util.HashMap;
@@ -433,6 +436,63 @@
Assert.assertArrayEquals(checksumBytes1, checksumBytes2);
}
+ @Test
+ public void testSignedJars() throws Exception {
+ // Set test configuration paths / options
+ final String baseJarPath = "../test-java-lib-jarinfer/build/libs/test-java-lib-jarinfer.jar";
+ final String pkg = "com.uber.nullaway.jarinfer.toys.unannotated";
+ final String baseJarName = FilenameUtils.getBaseName(baseJarPath);
+ final String workingFolderPath = outputFolder.newFolder("signed_" + pkg).getAbsolutePath();
+ final String inputJarPath = workingFolderPath + "/" + baseJarName + ".jar";
+ final String outputJarPath = workingFolderPath + "/" + baseJarName + "-annotated.jar";
+
+ // Copy Jar to workspace, and sign it
+ Files.copy(
+ Paths.get(baseJarPath), Paths.get(inputJarPath), StandardCopyOption.REPLACE_EXISTING);
+ String ksPath =
+ Thread.currentThread().getContextClassLoader().getResource("testKeyStore.jks").getPath();
+ // JDK 8 only?
+ sun.security.tools.jarsigner.Main jarSignerTool = new sun.security.tools.jarsigner.Main();
+ jarSignerTool.run(
+ new String[] {
+ "-keystore", ksPath, "-storepass", "testPassword", inputJarPath, "testKeystore"
+ });
+
+ // Test that this new jar fails if not run in --strip-jar-signatures mode
+ boolean signedJarExceptionThrown = false;
+ try {
+ DefinitelyDerefedParamsDriver driver1 = new DefinitelyDerefedParamsDriver();
+ driver1.runAndAnnotate(inputJarPath, "", outputJarPath);
+ } catch (SignedJarException sje) {
+ signedJarExceptionThrown = true;
+ }
+ Assert.assertTrue(signedJarExceptionThrown);
+
+ // And that it succeeds if run in --strip-jar-signatures mode
+ DefinitelyDerefedParamsDriver driver2 = new DefinitelyDerefedParamsDriver();
+ driver2.runAndAnnotate(inputJarPath, "", outputJarPath, true);
+
+ Assert.assertTrue(
+ "Annotated jar after signature stripping does not match the expected jar!",
+ AnnotationChecker.checkMethodAnnotationsInJar(
+ outputJarPath,
+ ImmutableMap.of(
+ "Lcom/uber/nullaway/jarinfer/toys/unannotated/ExpectNullable;",
+ BytecodeAnnotator.javaxNullableDesc,
+ "Lcom/uber/nullaway/jarinfer/toys/unannotated/ExpectNonnull;",
+ BytecodeAnnotator.javaxNonnullDesc)));
+ // Files should match the base jar, not the one to which we added META-INF, since we are
+ // stripping those files
+ Assert.assertTrue(
+ "Annotated jar after signature stripping does not have all the entries present in the input "
+ + "jar (before signing), or contains extra (e.g. META-INF) entries!",
+ EntriesComparator.compareEntriesInJars(outputJarPath, baseJarPath));
+ // Check that META-INF/Manifest.MF is content-identical to the original unsigned jar
+ Assert.assertTrue(
+ "Annotated jar after signature stripping does not preserve the info inside META-INF/MANIFEST.MF",
+ EntriesComparator.compareManifestContents(outputJarPath, baseJarPath));
+ }
+
private byte[] sha1sum(String path) throws Exception {
File file = new File(path);
MessageDigest digest = MessageDigest.getInstance("SHA-1");
diff --git a/jar-infer/jar-infer-lib/src/test/resources/testKeyStore.jks b/jar-infer/jar-infer-lib/src/test/resources/testKeyStore.jks
new file mode 100644
index 0000000..820906f
--- /dev/null
+++ b/jar-infer/jar-infer-lib/src/test/resources/testKeyStore.jks
Binary files differ
diff --git a/jar-infer/test-java-lib-jarinfer/build.gradle b/jar-infer/test-java-lib-jarinfer/build.gradle
index 0122a0d..f372840 100644
--- a/jar-infer/test-java-lib-jarinfer/build.gradle
+++ b/jar-infer/test-java-lib-jarinfer/build.gradle
@@ -25,6 +25,17 @@
def astubxPath = "META-INF/nullaway/jarinfer.astubx"
+jar {
+ manifest {
+ attributes(
+ 'Build-Timestamp': new java.text.SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date()),
+ 'Created-By' : "Gradle ${gradle.gradleVersion}",
+ 'Build-Jdk' : "${System.properties['java.version']} (${System.properties['java.vendor']} ${System.properties['java.vm.version']})",
+ 'Build-OS' : "${System.properties['os.name']} ${System.properties['os.arch']} ${System.properties['os.version']}"
+ )
+ }
+}
+
jar.doLast {
javaexec {
classpath = files("${rootProject.projectDir}/jar-infer/jar-infer-cli/build/libs/jar-infer-cli-${VERSION_NAME}.jar")