Improve checksums for agent jar(s) download (#3884)

Before this commit the (md5) checksums for various jars are were encoded in base64 and hex encoding: curl -s https://example.com/go/admin/latest-agent.status -I | grep MD5 Agent-Content-MD5: xcOIJGRG6DVEndmh+ngL0w== Agent-Launcher-Content-MD5: YZZ/8vOLt9tme93YVwtoIg== Agent-Plugins-Content-MD5: 0fc9e52a2619dedfd7973959b8c92063 TFS-SDK-Content-MD5: FDZDpIK84xCKx0EO+HZu9A== This caused problems when checksums were verified because the agent was always verifying passwords using base64, and the plugins zip was encoded in hex format, causing the agent to re-download the plugins.zip every time it was started up. Additionally there was another bug with how the `agent-plugins.zip` checksum was computed. The checksum was computed was basically the equivalent of: `cat [list of plugin.jar] | md5sum`. Whereas it should have been: `zip [list of plugin.jar] | md5sum`
gocd · Oct 3, 2017 · a1f9945 · a1f9945
1 parent 2e1f325
commit a1f9945
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 281 deletions.
diff --git a/agent-common/src/com/thoughtworks/go/agent/launcher/DownloadableFile.java b/agent-common/src/com/thoughtworks/go/agent/launcher/DownloadableFile.java
@@ -1,30 +1,33 @@
-/*************************GO-LICENSE-START*********************************
- * Copyright 2016 ThoughtWorks, Inc.
+/*
+ * Copyright 2017 ThoughtWorks, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
- *************************GO-LICENSE-END***********************************/
+ */
 
 package com.thoughtworks.go.agent.launcher;
 
 import com.thoughtworks.go.agent.ServerUrlGenerator;
 import com.thoughtworks.go.agent.common.util.Downloader;
-import com.thoughtworks.go.util.FileDigester;
+import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.output.NullOutputStream;
 
+import javax.xml.bind.DatatypeConverter;
 import java.io.File;
 import java.io.FileInputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.security.DigestInputStream;
+import java.security.MessageDigest;
 
 public enum DownloadableFile {
     AGENT("admin/agent", Downloader.AGENT_BINARY),
@@ -62,9 +65,11 @@ public String toString() {
 
     protected static boolean matchChecksum(File localFile, String expectedSignature) {
         try (FileInputStream input = new FileInputStream(localFile)) {
-            FileDigester fileDigester = new FileDigester(input, new NullOutputStream());
-            fileDigester.copy();
-            return expectedSignature.equals(fileDigester.md5());
+            MessageDigest digester = MessageDigest.getInstance("MD5");
+            try (DigestInputStream digest = new DigestInputStream(input, digester)) {
+                IOUtils.copy(digest, new NullOutputStream());
+            }
+            return expectedSignature.equalsIgnoreCase(DatatypeConverter.printHexBinary(digester.digest()));
         } catch (Exception e) {
             throw new RuntimeException(e);
         }

diff --git a/agent-common/test/com/thoughtworks/go/agent/launcher/DownloadableFileTest.java b/agent-common/test/com/thoughtworks/go/agent/launcher/DownloadableFileTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 ThoughtWorks, Inc.
+ * Copyright 2017 ThoughtWorks, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,7 +36,7 @@ public class DownloadableFileTest {
     @Test
     public void shouldReturnTrueIfChecksumIsEqual() throws Exception {
         File inputFile = new File("test-resources/checksum.txt");
-        assertTrue(DownloadableFile.matchChecksum(inputFile, "FlCLOoC4KK/RMxgAO1hibg=="));
+        assertTrue(DownloadableFile.matchChecksum(inputFile, "16508b3a80b828afd13318003b58626e"));
     }
 
     /*

diff --git a/agent-common/test/com/thoughtworks/go/agent/launcher/ServerBinaryDownloaderTest.java b/agent-common/test/com/thoughtworks/go/agent/launcher/ServerBinaryDownloaderTest.java
@@ -22,6 +22,8 @@
 import com.thoughtworks.go.mothers.ServerUrlGeneratorMother;
 import com.thoughtworks.go.util.SslVerificationMode;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.output.NullOutputStream;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.impl.client.CloseableHttpClient;
@@ -30,12 +32,14 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import javax.xml.bind.DatatypeConverter;
 import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.net.UnknownHostException;
+import java.security.DigestInputStream;
+import java.security.MessageDigest;
 
-import static com.thoughtworks.go.util.FileDigester.md5DigestOfStream;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.any;
@@ -61,8 +65,12 @@ public void shouldSetMd5AndSSLPortHeaders() throws Exception {
         ServerBinaryDownloader downloader = new ServerBinaryDownloader(ServerUrlGeneratorMother.generatorFor("localhost", server.getPort()), null, SslVerificationMode.NONE);
         downloader.downloadIfNecessary(DownloadableFile.AGENT);
 
-        try(BufferedInputStream stream = new BufferedInputStream(new FileInputStream(DownloadableFile.AGENT.getLocalFile()))) {
-            assertThat(downloader.getMd5(), is(md5DigestOfStream(stream)));
+        MessageDigest digester = MessageDigest.getInstance("MD5");
+        try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(DownloadableFile.AGENT.getLocalFile()))) {
+            try (DigestInputStream digest = new DigestInputStream(stream, digester)) {
+                IOUtils.copy(digest, new NullOutputStream());
+            }
+            assertThat(downloader.getMd5(), is(DatatypeConverter.printHexBinary(digester.digest()).toLowerCase()));
         }
         assertThat(downloader.getSslPort(), is(String.valueOf(server.getSecurePort())));
     }

diff --git a/base/src/com/thoughtworks/go/util/FileDigester.java b/base/src/com/thoughtworks/go/util/FileDigester.java
diff --git a/base/test/com/thoughtworks/go/util/FileDigesterTest.java b/base/test/com/thoughtworks/go/util/FileDigesterTest.java
diff --git a/plugin-infra/go-plugin-infra/src/com/thoughtworks/go/plugin/infra/commons/PluginsZip.java b/plugin-infra/go-plugin-infra/src/com/thoughtworks/go/plugin/infra/commons/PluginsZip.java
@@ -23,19 +23,16 @@
 import com.thoughtworks.go.util.SystemEnvironment;
 import org.apache.commons.codec.binary.Hex;
 import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.commons.io.IOUtils;
-import org.apache.commons.io.output.NullOutputStream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 import java.io.BufferedOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.nio.file.Files;
-import java.security.DigestInputStream;
+import java.security.DigestOutputStream;
 import java.security.MessageDigest;
 import java.util.Comparator;
 import java.util.List;
@@ -93,7 +90,8 @@ public void create() {
         checkFilesAccessibility(bundledPlugins, externalPlugins);
         reset();
 
-        try (ZipOutputStream zos = new ZipOutputStream(new BufferedOutputStream(new FileOutputStream(destZipFile)))) {
+        MessageDigest md5Digest = DigestUtils.getMd5Digest();
+        try (ZipOutputStream zos = new ZipOutputStream(new DigestOutputStream(new BufferedOutputStream(new FileOutputStream(destZipFile)), md5Digest))) {
             for (GoPluginDescriptor taskPlugin : agentPlugins()) {
                 String zipEntryPrefix = "external/";
 
@@ -105,10 +103,11 @@ public void create() {
                 Files.copy(new File(taskPlugin.pluginFileLocation()).toPath(), zos);
                 zos.closeEntry();
             }
-            md5DigestOfPlugins = computeMd5DigestOfPlugins();
         } catch (Exception e) {
             LOG.error("Could not create zip of plugins for agent to download.", e);
         }
+
+        md5DigestOfPlugins = Hex.encodeHexString(md5Digest.digest());
     }
 
     private void reset() {
@@ -117,27 +116,9 @@ private void reset() {
     }
 
     public String md5() {
-        if (md5DigestOfPlugins == null) {
-            return computeMd5DigestOfPlugins();
-        }
         return md5DigestOfPlugins;
     }
 
-    private String computeMd5DigestOfPlugins() {
-        try {
-            MessageDigest md5Digest = DigestUtils.getMd5Digest();
-            List<GoPluginDescriptor> taskPlugins = agentPlugins();
-            for (GoPluginDescriptor taskPlugin : taskPlugins) {
-                try (DigestInputStream digest = new DigestInputStream(new FileInputStream(taskPlugin.pluginFileLocation()), md5Digest)) {
-                    IOUtils.copy(digest, new NullOutputStream());
-                }
-            }
-            return Hex.encodeHexString(md5Digest.digest());
-        } catch (Exception e) {
-            throw new RuntimeException("Could not compute md5 of plugins.", e);
-        }
-    }
-
     private List<GoPluginDescriptor> agentPlugins() {
         if (taskPlugins.isEmpty()) {
             List<GoPluginDescriptor> agentPlugins = pluginManager.plugins().stream().