fix(core): Fix reading of external files as binary instead of text (#…

…1380) (#1381)

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/model/v1/canary/google/GoogleCanaryAccount.java

@@ -16,24 +16,16 @@
 
 package com.netflix.spinnaker.halyard.config.model.v1.canary.google;
 
-import com.fasterxml.jackson.annotation.JsonIgnore;
-import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
 import com.netflix.spinnaker.halyard.config.model.v1.canary.AbstractCanaryAccount;
 import com.netflix.spinnaker.halyard.config.model.v1.canary.AbstractCanaryServiceIntegration;
 import com.netflix.spinnaker.halyard.config.model.v1.node.LocalFile;
 import com.netflix.spinnaker.halyard.config.model.v1.node.SecretFile;
-import com.netflix.spinnaker.halyard.config.model.v1.util.ValidatingFileReader;
-import com.netflix.spinnaker.halyard.config.problem.v1.ConfigProblemSetBuilder;
-import com.netflix.spinnaker.halyard.core.problem.v1.Problem;
-import com.netflix.spinnaker.halyard.core.secrets.v1.SecretSessionManager;
-import com.netflix.spinnaker.kork.secrets.EncryptedSecret;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import lombok.Data;
 import lombok.EqualsAndHashCode;
 import lombok.ToString;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.commons.lang3.StringUtils;
 
 @Data
 @EqualsAndHashCode(callSuper = true)
@@ -47,45 +39,4 @@ public class GoogleCanaryAccount extends AbstractCanaryAccount implements Clonea
   private String rootFolder = "kayenta";
   private SortedSet<AbstractCanaryServiceIntegration.SupportedTypes> supportedTypes =
       new TreeSet<>();
-
-  @JsonIgnore
-  public GoogleNamedAccountCredentials getNamedAccountCredentials(
-      String version, SecretSessionManager secretSessionManager, ConfigProblemSetBuilder p) {
-    String jsonKey = null;
-    if (!StringUtils.isEmpty(getJsonPath())) {
-      if (EncryptedSecret.isEncryptedSecret(getJsonPath())) {
-        jsonKey = secretSessionManager.decrypt(getJsonPath());
-      } else {
-        jsonKey = ValidatingFileReader.contents(p, getJsonPath());
-      }
-
-      if (jsonKey == null) {
-        return null;
-      } else if (jsonKey.isEmpty()) {
-        p.addProblem(Problem.Severity.WARNING, "The supplied credentials file is empty.");
-      }
-    }
-
-    if (StringUtils.isEmpty(getProject())) {
-      p.addProblem(Problem.Severity.ERROR, "No google project supplied.");
-      return null;
-    }
-
-    try {
-      return new GoogleNamedAccountCredentials.Builder()
-          .name(getName())
-          .jsonKey(jsonKey)
-          .project(getProject())
-          .applicationName("halyard " + version)
-          .liveLookupsEnabled(false)
-          .build();
-    } catch (Exception e) {
-      p.addProblem(
-              Problem.Severity.ERROR,
-              "Error instantiating Google credentials: " + e.getMessage() + ".")
-          .setRemediation(
-              "Do the provided credentials have access to project " + getProject() + "?");
-      return null;
-    }
-  }
 }

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/model/v1/node/Validator.java

@@ -19,7 +19,6 @@
 import com.netflix.spinnaker.halyard.config.model.v1.util.ValidatingFileReader;
 import com.netflix.spinnaker.halyard.config.problem.v1.ConfigProblemSetBuilder;
 import com.netflix.spinnaker.halyard.core.secrets.v1.SecretSessionManager;
-import com.netflix.spinnaker.kork.secrets.EncryptedSecret;
 import org.springframework.beans.factory.annotation.Autowired;
 
 public abstract class Validator<T extends Node> {
@@ -28,10 +27,10 @@ public abstract class Validator<T extends Node> {
   public abstract void validate(ConfigProblemSetBuilder p, T n);
 
   protected String validatingFileDecrypt(ConfigProblemSetBuilder p, String filePath) {
-    if (EncryptedSecret.isEncryptedSecret(filePath)) {
-      return secretSessionManager.decrypt(filePath);
-    } else {
-      return ValidatingFileReader.contents(p, filePath);
-    }
+    return ValidatingFileReader.contents(p, filePath, secretSessionManager);
+  }
+
+  protected byte[] validatingFileDecryptBytes(ConfigProblemSetBuilder p, String filePath) {
+    return ValidatingFileReader.contentBytes(p, filePath, secretSessionManager);
   }
 }

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/model/v1/util/ValidatingFileReader.java

@@ -21,6 +21,8 @@
 import com.netflix.spinnaker.halyard.config.problem.v1.ConfigProblemBuilder;
 import com.netflix.spinnaker.halyard.config.problem.v1.ConfigProblemSetBuilder;
 import com.netflix.spinnaker.halyard.core.problem.v1.Problem;
+import com.netflix.spinnaker.halyard.core.secrets.v1.SecretSessionManager;
+import com.netflix.spinnaker.kork.secrets.EncryptedSecret;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -32,17 +34,33 @@ public class ValidatingFileReader {
           + System.getProperty("user.name")
           + ". Make sure that user can read the requested file.";
 
-  public static String contents(ConfigProblemSetBuilder ps, String path) {
+  public static String contents(
+      ConfigProblemSetBuilder ps, String path, SecretSessionManager secretSessionManager) {
+
+    byte[] contentBytes = contentBytes(ps, path, secretSessionManager);
+    if (contentBytes == null) {
+      return null;
+    }
+    return new String(contentBytes);
+  }
+
+  public static byte[] contentBytes(
+      ConfigProblemSetBuilder ps, String path, SecretSessionManager secretSessionManager) {
+
     if (PropertyUtils.isConfigServerResource(path)) {
       return null;
-    } else {
-      return readFromLocalFilesystem(ps, path);
     }
+
+    if (EncryptedSecret.isEncryptedSecret(path)) {
+      return secretSessionManager.decryptAsBytes(path);
+    }
+
+    return readFromLocalFilesystem(ps, path);
   }
 
-  private static String readFromLocalFilesystem(ConfigProblemSetBuilder ps, String path) {
+  private static byte[] readFromLocalFilesystem(ConfigProblemSetBuilder ps, String path) {
     try {
-      return IOUtils.toString(new FileInputStream(path));
+      return IOUtils.toByteArray(new FileInputStream(path));
     } catch (FileNotFoundException e) {
       buildProblem(ps, "Cannot find provided path: " + e.getMessage() + ".", e);
     } catch (IOException e) {

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/validate/v1/canary/CanaryAccountValidator.java

@@ -24,11 +24,11 @@
 import org.springframework.stereotype.Component;
 
 @Component
-public class CanaryAccountValidator extends Validator<AbstractCanaryAccount> {
+public class CanaryAccountValidator<T extends AbstractCanaryAccount> extends Validator<T> {
   private static final String namePattern = "^[a-z0-9]+([-a-z0-9_]*[a-z0-9])?$";
 
   @Override
-  public void validate(ConfigProblemSetBuilder p, AbstractCanaryAccount n) {
+  public void validate(ConfigProblemSetBuilder p, T n) {
     if (n.getName() == null) {
       p.addProblem(Severity.FATAL, "Canary account name must be specified");
     } else if (!Pattern.matches(namePattern, n.getName())) {

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/validate/v1/canary/google/GoogleCanaryAccountValidator.java

@@ -20,20 +20,21 @@
 import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
 import com.netflix.spinnaker.front50.model.GcsStorageService;
 import com.netflix.spinnaker.front50.model.StorageService;
-import com.netflix.spinnaker.halyard.config.model.v1.canary.AbstractCanaryAccount;
 import com.netflix.spinnaker.halyard.config.model.v1.canary.google.GoogleCanaryAccount;
 import com.netflix.spinnaker.halyard.config.problem.v1.ConfigProblemSetBuilder;
 import com.netflix.spinnaker.halyard.config.validate.v1.canary.CanaryAccountValidator;
+import com.netflix.spinnaker.halyard.core.problem.v1.Problem;
 import com.netflix.spinnaker.halyard.core.problem.v1.Problem.Severity;
 import com.netflix.spinnaker.halyard.core.secrets.v1.SecretSessionManager;
 import com.netflix.spinnaker.halyard.core.tasks.v1.DaemonTaskHandler;
 import lombok.Data;
 import lombok.EqualsAndHashCode;
+import org.apache.commons.lang3.StringUtils;
 import org.springframework.scheduling.TaskScheduler;
 
 @Data
 @EqualsAndHashCode(callSuper = false)
-public class GoogleCanaryAccountValidator extends CanaryAccountValidator {
+public class GoogleCanaryAccountValidator extends CanaryAccountValidator<GoogleCanaryAccount> {
 
   private String halyardVersion;
 
@@ -53,33 +54,30 @@ public class GoogleCanaryAccountValidator extends CanaryAccountValidator {
   }
 
   @Override
-  public void validate(ConfigProblemSetBuilder p, AbstractCanaryAccount n) {
+  public void validate(ConfigProblemSetBuilder p, GoogleCanaryAccount n) {
     super.validate(p, n);
 
-    GoogleCanaryAccount canaryAccount = (GoogleCanaryAccount) n;
-
     DaemonTaskHandler.message(
         "Validating "
             + n.getNodeName()
             + " with "
             + GoogleCanaryAccountValidator.class.getSimpleName());
 
-    GoogleNamedAccountCredentials credentials =
-        canaryAccount.getNamedAccountCredentials(halyardVersion, secretSessionManager, p);
+    GoogleNamedAccountCredentials credentials = getNamedAccountCredentials(p, n);
 
     if (credentials == null) {
       return;
     }
 
-    String jsonPath = canaryAccount.getJsonPath();
+    String jsonPath = n.getJsonPath();
 
     try {
       StorageService storageService =
           new GcsStorageService(
-              canaryAccount.getBucket(),
-              canaryAccount.getBucketLocation(),
-              canaryAccount.getRootFolder(),
-              canaryAccount.getProject(),
+              n.getBucket(),
+              n.getBucketLocation(),
+              n.getRootFolder(),
+              n.getProject(),
               jsonPath != null ? secretSessionManager.decryptAsFile(jsonPath) : "",
               "halyard",
               connectTimeoutSec,
@@ -96,9 +94,47 @@ public void validate(ConfigProblemSetBuilder p, AbstractCanaryAccount n) {
       p.addProblem(
           Severity.ERROR,
           "Failed to ensure the required bucket \""
-              + canaryAccount.getBucket()
+              + n.getBucket()
               + "\" exists: "
               + e.getMessage());
     }
   }
+
+  private GoogleNamedAccountCredentials getNamedAccountCredentials(
+      ConfigProblemSetBuilder p, GoogleCanaryAccount canaryAccount) {
+    String jsonKey = null;
+    if (!StringUtils.isEmpty(canaryAccount.getJsonPath())) {
+      jsonKey = validatingFileDecrypt(p, canaryAccount.getJsonPath());
+
+      if (jsonKey == null) {
+        return null;
+      } else if (jsonKey.isEmpty()) {
+        p.addProblem(Problem.Severity.WARNING, "The supplied credentials file is empty.");
+      }
+    }
+
+    if (StringUtils.isEmpty(canaryAccount.getProject())) {
+      p.addProblem(Problem.Severity.ERROR, "No google project supplied.");
+      return null;
+    }
+
+    try {
+      return new GoogleNamedAccountCredentials.Builder()
+          .name(canaryAccount.getName())
+          .jsonKey(jsonKey)
+          .project(canaryAccount.getProject())
+          .applicationName("halyard " + halyardVersion)
+          .liveLookupsEnabled(false)
+          .build();
+    } catch (Exception e) {
+      p.addProblem(
+              Problem.Severity.ERROR,
+              "Error instantiating Google credentials: " + e.getMessage() + ".")
+          .setRemediation(
+              "Do the provided credentials have access to project "
+                  + canaryAccount.getProject()
+                  + "?");
+      return null;
+    }
+  }
 }

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/validate/v1/canary/prometheus/PrometheusCanaryAccountValidator.java

@@ -18,7 +18,6 @@
 
 import static com.netflix.spinnaker.halyard.core.problem.v1.Problem.Severity.ERROR;
 
-import com.netflix.spinnaker.halyard.config.model.v1.canary.AbstractCanaryAccount;
 import com.netflix.spinnaker.halyard.config.model.v1.canary.prometheus.PrometheusCanaryAccount;
 import com.netflix.spinnaker.halyard.config.problem.v1.ConfigProblemSetBuilder;
 import com.netflix.spinnaker.halyard.config.validate.v1.canary.CanaryAccountValidator;
@@ -33,23 +32,22 @@
 @Data
 @EqualsAndHashCode(callSuper = false)
 @Component
-public class PrometheusCanaryAccountValidator extends CanaryAccountValidator {
+public class PrometheusCanaryAccountValidator
+    extends CanaryAccountValidator<PrometheusCanaryAccount> {
 
   @Autowired private SecretSessionManager secretSessionManager;
 
   @Override
-  public void validate(ConfigProblemSetBuilder p, AbstractCanaryAccount n) {
+  public void validate(ConfigProblemSetBuilder p, PrometheusCanaryAccount n) {
     super.validate(p, n);
 
-    PrometheusCanaryAccount canaryAccount = (PrometheusCanaryAccount) n;
-
     DaemonTaskHandler.message(
         "Validating "
             + n.getNodeName()
             + " with "
             + PrometheusCanaryAccountValidator.class.getSimpleName());
 
-    String usernamePasswordFile = canaryAccount.getUsernamePasswordFile();
+    String usernamePasswordFile = n.getUsernamePasswordFile();
 
     if (StringUtils.isNotEmpty(usernamePasswordFile)) {
       String usernamePassword = validatingFileDecrypt(p, usernamePasswordFile);

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/validate/v1/security/SamlValidator.java

@@ -79,13 +79,13 @@ public void validate(ConfigProblemSetBuilder p, Saml saml) {
     }
 
     try {
-      String keyStore = validatingFileDecrypt(p, saml.getKeyStore());
+      byte[] keyStore = validatingFileDecryptBytes(p, saml.getKeyStore());
       if (keyStore != null) {
         val keystore = KeyStore.getInstance(KeyStore.getDefaultType());
 
         // will throw an exception if `keyStorePassword` is invalid
         keystore.load(
-            new ByteArrayInputStream(keyStore.getBytes()),
+            new ByteArrayInputStream(keyStore),
             secretSessionManager.decrypt(saml.getKeyStorePassword()).toCharArray());
 
         Collections.list(keystore.aliases()).stream()