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
Validate checksums for plugins if available #12888
Conversation
When a plugin is downloaded, this change additionally tries to download `${pluginurl}.sha1` and verify the SHA1 checksum for the file. If no .sha1 file is found, it tries `${pluginurl}.md5`. Note that if neither checksum file is found, a notice is printed but the plugin can still be installed. If the checksum check fails, the plugin install is aborted. Example output if no checksums are available: ``` bin/plugin install elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT -> Installing elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT... Trying http://download.elastic.co/elasticsearch/elasticsearch-analysis-icu/elasticsearch-analysis-icu-2.6.0-SNAPSHOT.zip ... Trying http://search.maven.org/remotecontent?filepath=elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT/elasticsearch-analysis-icu-2.6.0-SNAPSHOT.zip ... Trying https://oss.sonatype.org/service/local/repositories/releases/content/elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT/elasticsearch-analysis-icu-2.6.0-SNAPSHOT.zip ... Trying https://github.com/elasticsearch/elasticsearch-analysis-icu/archive/2.6.0-SNAPSHOT.zip ... Trying https://github.com/elasticsearch/elasticsearch-analysis-icu/archive/master.zip ... Downloading .....................................DONE Verifying https://github.com/elasticsearch/elasticsearch-analysis-icu/archive/master.zip checksums if available ... NOTE: Unable to verify checksum for downloaded plugin (unable to find .sha1 or .md5 file to verify) ``` Example output if checksums are available: ``` bin/plugin install elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT -> Installing elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT... Trying http://download.elastic.co/elasticsearch/elasticsearch-analysis-icu/elasticsearch-analysis-icu-2.6.0-SNAPSHOT.zip ... Trying http://search.maven.org/remotecontent?filepath=elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT/elasticsearch-analysis-icu-2.6.0-SNAPSHOT.zip ... Trying https://oss.sonatype.org/service/local/repositories/releases/content/elasticsearch/elasticsearch-analysis-icu/2.6.0-SNAPSHOT/elasticsearch-analysis-icu-2.6.0-SNAPSHOT.zip ... Trying https://github.com/elasticsearch/elasticsearch-analysis-icu/archive/2.6.0-SNAPSHOT.zip ... Trying https://github.com/elasticsearch/elasticsearch-analysis-icu/archive/master.zip ... Downloading .....................................DONE Verifying https://github.com/elasticsearch/elasticsearch-analysis-icu/archive/master.zip checksums if available ... Downloading .DONE ``` Example output if checksums fail: ``` bin/plugin install elasticsearch/elasticsearch-analysis-kuromoji/2.5.0 -url http://localhost:8000/elasticsearch-analysis-kuromoji-2.5.0.zip -> Installing elasticsearch/elasticsearch-analysis-kuromoji/2.5.0... Trying http://localhost:8000/elasticsearch-analysis-kuromoji-2.5.0.zip ... Downloading .............................................DONE Verifying http://localhost:8000/elasticsearch-analysis-kuromoji-2.5.0.zip checksums if available ... Downloading .DONE ERROR: incorrect hash, file hash: [dbdc9c2cd32782054497a21fbdcae3ca1ff23c80], expected: [dbdc9c2cd32782054497a21fbdcae3ca1ff23c80-bad] ``` Resolves elastic#12750
I was wondering if we can do the MD5/SHA1 analysis on the fly while downloading the zip file. Would this make sense in such a case? |
I'm not sure what that would buy us? It would mean that we don't have to |
String checksumHex = checksumLines.get(0); | ||
String fileHex = hashFunc.checksum(fileBytes); | ||
if (fileHex.equals(checksumHex) == false) { | ||
throw new ElasticsearchCorruptionException("incorrect hash, file hash: [" + |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
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
Not a big deal. Just that I found that way of doing it on the fly is super nice. The PluginManager runs in its own JVM so memory constraint is less important here I guess. As for the biggest plugin I've seen it's probably the mapper attachment which is more than 20mb IIRC. Not that big indeed. |
} | ||
return true; | ||
} | ||
} catch (FileNotFoundException e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also catch NoSuchFileException
please
There was a problem hiding this comment.
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.
left minor comments otherwise LGTM |
LGTM |
When a plugin is downloaded, this change additionally tries to download
${pluginurl}.sha1
and verify the SHA1 checksum for the file. If no.sha1 file is found, it tries
${pluginurl}.md5
.Note that if neither checksum file is found, a notice is printed but the
plugin can still be installed. If the checksum check fails, the plugin
install is aborted.
Example output if no checksums are available:
Example output if checksums are available:
Example output if checksums fail:
Resolves #12750