Spinnaker: halyard 1.22.0 validatingFileDecrypt does not read binary files properly

[eaceaser]

https://github.com/spinnaker/halyard/commit/7bfa5ce9315aaca90d62b2a17c533505da6e0bee starts reading files in validators via the validatingFileDecrypt function. For example:

https://github.com/spinnaker/halyard/commit/7bfa5ce9315aaca90d62b2a17c533505da6e0bee#diff-6d740a77e7f6a34c5ba873b1d3b5bdb4R82

This file used to be read via a FileInputStream, but now is loaded into a string in validatingFileDecrypt.

validatingFileDecrypt ends up reading non-encrypted files here: https://github.com/spinnaker/halyard/blob/master/halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/model/v1/util/ValidatingFileReader.java#L45

using import com.amazonaws.util.IOUtils. Reading a file via IOUtils.toString is NOT equivalent to reading the file bytewise via FileInputStream if the file is a binary file, which the JKS for SAML is.

Reproduction

This is probably reproducable with any binary file that is not parse in a byte-equivalent manner with whatever encoding IOUtils.toString uses, but here are the exact steps I've used to reproduce the issue:

1. gen a jks: keytool -genkey -v -keystore saml.jks -alias saml -keyalg RSA -keysize 2048 -validity 10000
2. Set that file as the keystore hal security authn saml edit --keystore <path-to->/saml.jks
3. Try to enable SAML hal config security authn saml enable

Problems in default.security.authn.saml:
! ERROR Keystore validation problem: Invalid keystore format

Confirmation

executing the following test program:

```javaimport com.amazonaws.util.IOUtils;
import java.io.FileInputStream;
import com.google.common.hash.Hashing;
import java.nio.file.Files;
import java.nio.file.Paths;

public class Derp {
public static void main(String[] args) throws Exception {
String ksbad = IOUtils.toString(new FileInputStream("/saml.jks"));
byte[] ksgood = Files.readAllBytes(Paths.get("/saml.jks"));
System.out.println("IOUtils: " + Hashing.sha256().hashBytes(ksbad.getBytes()));
System.out.println("raw bytes: " + Hashing.sha256().hashBytes(ksgood));
}
}
```

outputs

IOUtils: c2b34838a5d72758925c2009eab3bccb10da98a042141def1ce0bc2bb040d5fc
raw bytes: 1d38807cc32365c236d47915b73f053e6631bfebf5b49cb9c592b2d077e84e3f

showing that the two representations of the file are not equivalent. Furthermore, trying to read ksbad.toBytes() as a JKS (which is what halyard master does atm) fails, but reading ksgood works.

[AnatoliyDutchak]

Just want to confirm that we're facing out equal issues with halyard 1.22.0

[scottfrederick]

@eaceaser I'll take a look at this.

[scottfrederick]

@eaceaser This was a case of over-eager refactoring, trying to encapsulate all the file reading logic.

This is probably reproducable with any binary file that is not parse in a byte-equivalent manner with whatever encoding IOUtils.toString uses

The SamlValidator is the only validator that reads external files as bytes; all other validators expect an external file to contain text that can be returned as a String. The fact that SamlValidator needs bytes was masked by this bit of previous code, which used secretSessionManager.decrypt() to read the value into a String and then get the bytes from the string.

  if (EncryptedSecret.isEncryptedSecret(saml.getKeyStore())) {
    is = new ByteArrayInputStream(secretSessionManager.decrypt(saml.getKeyStore()).getBytes());
  } else { 
  ...

This would have broken in a similar manner if anyone tried to use the secretsSessionManager to retrieve a keystore.

A fix is in the works.