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.

Sign up for GitHub

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

Jump to bottom

Validate checksums for plugins if available #12888

Merged
merged 3 commits into from
Aug 14, 2015
Merged

Validate checksums for plugins if available #12888

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41d8b55
Validate checksums for plugins if available
dakrone Aug 13, 2015
33f118e
Print out the name of the sum that failed
dakrone Aug 14, 2015
d35a3a3
Also catch NoSuchFileException
dakrone Aug 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 69 additions & 2 deletions core/src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@

import com.google.common.base.Charsets;
import com.google.common.base.Strings;
import com.google.common.hash.Hashing;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchTimeoutException;
import org.elasticsearch.Version;
import org.elasticsearch.*;
import org.elasticsearch.common.Base64;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.ByteArray;

import java.io.*;
import java.net.HttpURLConnection;
Expand All @@ -35,6 +36,9 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Callable;

/**
*
Expand Down Expand Up @@ -83,6 +87,69 @@ public boolean download(URL source, Path dest, @Nullable DownloadProgress progre
return getThread.wasSuccessful();
}

public interface Checksummer {
/** Return the hex string for the given byte array */
String checksum(byte[] filebytes);
}

/** Checksummer for SHA1 */
public static Checksummer SHA1_CHECKSUM = new Checksummer() {
@Override
public String checksum(byte[] filebytes) {
return Hashing.sha1().hashBytes(filebytes).toString();
}
};

/** Checksummer for MD5 */
public static Checksummer MD5_CHECKSUM = new Checksummer() {
@Override
public String checksum(byte[] filebytes) {
return Hashing.md5().hashBytes(filebytes).toString();
}
};

/**
* Download the given checksum URL to the destination and check the checksum
* @param checksumURL URL for the checksum file
* @param originalFile original file to calculate checksum of
* @param checksumFile destination to download the checksum file to
* @param hashFunc class used to calculate the checksum of the file
* @return true if the checksum was validated, false if it did not exist
* @throws Exception if the checksum failed to match
*/
public boolean downloadAndVerifyChecksum(URL checksumURL, Path originalFile, Path checksumFile,
@Nullable DownloadProgress progress,
TimeValue timeout, Checksummer hashFunc) throws Exception {
try {
if (download(checksumURL, checksumFile, progress, timeout)) {
byte[] fileBytes = Files.readAllBytes(originalFile);
List<String> checksumLines = Files.readAllLines(checksumFile);
if (checksumLines.size() != 1) {
throw new ElasticsearchCorruptionException("invalid format for checksum file, expected 1 line, got: " +
checksumLines.size());
}
String checksumHex = checksumLines.get(0);
String fileHex = hashFunc.checksum(fileBytes);
if (fileHex.equals(checksumHex) == false) {
throw new ElasticsearchCorruptionException("incorrect hash, file hash: [" +
Copy link
Member

Choose a reason for hiding this comment

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

can we add the hash function that was used into the message?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated this to print the hash function that failed

fileHex + "], expected: [" + checksumHex + "]");
}
return true;
}
} catch (FileNotFoundException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

also catch NoSuchFileException please

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, changed this to catch both.

// checksum file doesn't exist
return false;
} catch (IOException e) {
if (ExceptionsHelper.unwrapCause(e) instanceof FileNotFoundException) {
// checksum file didn't exist
return false;
}
throw e;
} finally {
IOUtils.deleteFilesIgnoringExceptions(checksumFile);
}
return false;
}

/**
* Interface implemented for reporting
Expand Down
30 changes: 28 additions & 2 deletions core/src/main/java/org/elasticsearch/plugins/PluginManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
import com.google.common.collect.Iterators;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.Build;
import org.elasticsearch.ElasticsearchCorruptionException;
import org.elasticsearch.ElasticsearchTimeoutException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.Version;
import org.elasticsearch.bootstrap.JarHell;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.http.client.HttpDownloadHelper;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.unit.TimeValue;
Expand Down Expand Up @@ -125,6 +127,7 @@ private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOExc

HttpDownloadHelper downloadHelper = new HttpDownloadHelper();
boolean downloaded = false;
boolean verified = false;
HttpDownloadHelper.DownloadProgress progress;
if (outputMode == OutputMode.SILENT) {
progress = new HttpDownloadHelper.NullProgress();
Expand All @@ -145,7 +148,14 @@ private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOExc
try {
downloadHelper.download(pluginUrl, pluginFile, progress, this.timeout);
downloaded = true;
} catch (ElasticsearchTimeoutException e) {
terminal.println("Verifying %s checksums if available ...", pluginUrl.toExternalForm());
Tuple<URL, Path> sha1Info = pluginHandle.newChecksumUrlAndFile(environment, pluginUrl, "sha1");
verified = downloadHelper.downloadAndVerifyChecksum(sha1Info.v1(), pluginFile,
sha1Info.v2(), progress, this.timeout, HttpDownloadHelper.SHA1_CHECKSUM);
Tuple<URL, Path> md5Info = pluginHandle.newChecksumUrlAndFile(environment, pluginUrl, "md5");
verified = verified || downloadHelper.downloadAndVerifyChecksum(md5Info.v1(), pluginFile,
md5Info.v2(), progress, this.timeout, HttpDownloadHelper.MD5_CHECKSUM);
} catch (ElasticsearchTimeoutException | ElasticsearchCorruptionException e) {
throw e;
} catch (Exception e) {
// ignore
Expand All @@ -164,8 +174,15 @@ private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOExc
try {
downloadHelper.download(url, pluginFile, progress, this.timeout);
downloaded = true;
terminal.println("Verifying %s checksums if available ...", url.toExternalForm());
Tuple<URL, Path> sha1Info = pluginHandle.newChecksumUrlAndFile(environment, url, "sha1");
verified = downloadHelper.downloadAndVerifyChecksum(sha1Info.v1(), pluginFile,
sha1Info.v2(), progress, this.timeout, HttpDownloadHelper.SHA1_CHECKSUM);
Tuple<URL, Path> md5Info = pluginHandle.newChecksumUrlAndFile(environment, url, "md5");
verified = verified || downloadHelper.downloadAndVerifyChecksum(md5Info.v1(), pluginFile,
md5Info.v2(), progress, this.timeout, HttpDownloadHelper.MD5_CHECKSUM);
break;
} catch (ElasticsearchTimeoutException e) {
} catch (ElasticsearchTimeoutException | ElasticsearchCorruptionException e) {
throw e;
} catch (Exception e) {
terminal.println(VERBOSE, "Failed: %s", ExceptionsHelper.detailedMessage(e));
Expand All @@ -178,6 +195,10 @@ private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOExc
IOUtils.deleteFilesIgnoringExceptions(pluginFile);
throw new IOException("failed to download out of all possible locations..., use --verbose to get detailed information");
}

if (verified == false) {
terminal.println("NOTE: Unable to verify checksum for downloaded plugin (unable to find .sha1 or .md5 file to verify)");
}
return pluginFile;
}

Expand Down Expand Up @@ -469,6 +490,11 @@ Path newDistroFile(Environment env) throws IOException {
return Files.createTempFile(env.tmpFile(), name, ".zip");
}

Tuple<URL, Path> newChecksumUrlAndFile(Environment env, URL originalUrl, String suffix) throws IOException {
URL newUrl = new URL(originalUrl.toString() + "." + suffix);
Copy link
Contributor

Choose a reason for hiding this comment

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

any chance we can get rid of URL instead use URI? it might be baked in then I think we should open a followup to clean this up

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 to get rid of URL and use URI instead, as a follow-up. Opened #12896 for this.

return new Tuple<>(newUrl, Files.createTempFile(env.tmpFile(), name, ".zip." + suffix));
}

Path extractedDir(Environment env) {
return env.pluginsFile().resolve(name);
}
Expand Down
83 changes: 77 additions & 6 deletions core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.plugins;

import com.google.common.base.Charsets;
import com.google.common.hash.Hashing;
import org.apache.http.impl.client.HttpClients;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.Version;
Expand Down Expand Up @@ -50,7 +51,10 @@
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSocketFactory;
import java.io.BufferedWriter;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintStream;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileVisitResult;
Expand All @@ -73,8 +77,7 @@
import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.elasticsearch.plugins.PluginInfoTests.writeProperties;
import static org.elasticsearch.test.ESIntegTestCase.Scope;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertDirectoryExists;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileExists;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
import static org.hamcrest.Matchers.*;
import static org.jboss.netty.handler.codec.http.HttpVersion.HTTP_1_1;

Expand Down Expand Up @@ -105,7 +108,27 @@ public void setup() throws Exception {
public void clearPathHome() {
System.clearProperty("es.default.path.home");
}


private void writeSha1(Path file, boolean corrupt) throws IOException {
String sha1Hex = Hashing.sha1().hashBytes(Files.readAllBytes(file)).toString();
try (BufferedWriter out = Files.newBufferedWriter(file.resolveSibling(file.getFileName() + ".sha1"), Charsets.UTF_8)) {
out.write(sha1Hex);
if (corrupt) {
out.write("bad");
}
}
}

private void writeMd5(Path file, boolean corrupt) throws IOException {
String md5Hex = Hashing.md5().hashBytes(Files.readAllBytes(file)).toString();
try (BufferedWriter out = Files.newBufferedWriter(file.resolveSibling(file.getFileName() + ".md5"), Charsets.UTF_8)) {
out.write(md5Hex);
if (corrupt) {
out.write("bad");
}
}
}

/** creates a plugin .zip and returns the url for testing */
private String createPlugin(final Path structure, String... properties) throws IOException {
writeProperties(structure, properties);
Expand All @@ -120,9 +143,35 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
}
});
}
if (randomBoolean()) {
writeSha1(zip, false);
} else if (randomBoolean()) {
writeMd5(zip, false);
}
return zip.toUri().toURL().toString();
}

/** creates a plugin .zip and bad checksum file and returns the url for testing */
private String createPluginWithBadChecksum(final Path structure, String... properties) throws IOException {
writeProperties(structure, properties);
Path zip = createTempDir().resolve(structure.getFileName() + ".zip");
try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(zip))) {
Files.walkFileTree(structure, new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
stream.putNextEntry(new ZipEntry(structure.relativize(file).toString()));
Files.copy(file, stream);
return FileVisitResult.CONTINUE;
}
});
}
if (randomBoolean()) {
writeSha1(zip, true);
} else {
writeMd5(zip, true);
}
return zip.toUri().toURL().toString();
}
@Test
public void testThatPluginNameMustBeSupplied() throws IOException {
Path pluginDir = createTempDir().resolve("fake-plugin");
Expand Down Expand Up @@ -342,15 +391,30 @@ public void testInstallSitePlugin() throws IOException {
Files.createDirectories(pluginDir.resolve("_site"));
Files.createFile(pluginDir.resolve("_site").resolve("somefile"));
String pluginUrl = createPlugin(pluginDir,
"description", "fake desc",
"version", "1.0",
"site", "true");
"description", "fake desc",
"version", "1.0",
"site", "true");
assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl));
assertThatPluginIsListed(pluginName);
// We want to check that Plugin Manager moves content to _site
assertFileExists(initialSettings.v2().pluginsFile().resolve(pluginName).resolve("_site"));
}

@Test
public void testInstallPluginWithBadChecksum() throws IOException {
String pluginName = "fake-plugin";
Path pluginDir = createTempDir().resolve(pluginName);
Files.createDirectories(pluginDir.resolve("_site"));
Files.createFile(pluginDir.resolve("_site").resolve("somefile"));
String pluginUrl = createPluginWithBadChecksum(pluginDir,
"description", "fake desc",
"version", "1.0",
"site", "true");
assertStatus(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl),
ExitStatus.IO_ERROR);
assertThatPluginIsNotListed(pluginName);
assertFileNotExists(initialSettings.v2().pluginsFile().resolve(pluginName).resolve("_site"));
}

private void singlePluginInstallAndRemove(String pluginDescriptor, String pluginName, String pluginCoordinates) throws IOException {
logger.info("--> trying to download and install [{}]", pluginDescriptor);
Expand Down Expand Up @@ -592,4 +656,11 @@ private void assertThatPluginIsListed(String pluginName) {
String message = String.format(Locale.ROOT, "Terminal output was: %s", terminal.getTerminalOutput());
assertThat(message, terminal.getTerminalOutput(), hasItem(containsString(pluginName)));
}

private void assertThatPluginIsNotListed(String pluginName) {
terminal.getTerminalOutput().clear();
assertStatusOk("list");
String message = String.format(Locale.ROOT, "Terminal output was: %s", terminal.getTerminalOutput());
assertFalse(message, terminal.getTerminalOutput().contains(pluginName));
}
}
11 changes: 11 additions & 0 deletions core/src/test/java/org/elasticsearch/plugins/PluginManagerUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.io.Files;
import org.elasticsearch.Build;
import org.elasticsearch.Version;
import org.elasticsearch.common.http.client.HttpDownloadHelper;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
Expand All @@ -30,6 +31,7 @@

import java.io.IOException;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.util.Iterator;
import java.util.Locale;
Expand Down Expand Up @@ -129,4 +131,13 @@ public void testTrimmingElasticsearchFromGithubPluginName() throws IOException {
URL expected = new URL("https", "github.com", "/" + user + "/" + pluginName + "/" + "archive/master.zip");
assertThat(handle.urls().get(0), is(expected));
}

@Test
public void testDownloadHelperChecksums() throws Exception {
// Sanity check to make sure the checksum functions never change how they checksum things
assertEquals("0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33",
HttpDownloadHelper.SHA1_CHECKSUM.checksum("foo".getBytes(Charset.forName("UTF-8"))));
assertEquals("acbd18db4cc2f85cedef654fccc4a4d8",
HttpDownloadHelper.MD5_CHECKSUM.checksum("foo".getBytes(Charset.forName("UTF-8"))));
}
}