fix(saml): With the upgrade to spring saml off of the DSL signing saml requests were broken. This restores that capability (#1898) (#1901)

(cherry picked from commit 0fb605c)

Co-authored-by: Jason <jason.mcintosh@harness.io>

Author: mergify[bot]

gate-saml/src/main/java/com/netflix/spinnaker/gate/security/saml/SAMLConfiguration.java

@@ -87,6 +87,13 @@ public RelyingPartyRegistrationRepository relyingPartyRegistrationRepository() {
       if (decryptionCredential != null) {
         builder.decryptionX509Credentials(credentials -> credentials.add(decryptionCredential));
       }
+      // This is used in some identity providers to sign the request.  NOT the response - response
+      // is handled via the certs in metadata or up above.  This is keycloak and some others
+      // TO USE THIS:  The certificate should be uploaded to the IDP to allow it to decrypt these
+      // requests
+      if (properties.isSignRequests()) {
+        builder.signingX509Credentials(c -> c.addAll(properties.getSigningCredentials()));
+      }
       RelyingPartyRegistration registration = builder.build();
       return new InMemoryRelyingPartyRegistrationRepository(registration);
     }

gate-saml/src/main/java/com/netflix/spinnaker/gate/security/saml/SecuritySamlProperties.java

@@ -20,23 +20,33 @@
 import com.netflix.spinnaker.kork.annotations.NullableByDefault;
 import com.netflix.spinnaker.kork.exceptions.ConfigurationException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.GeneralSecurityException;
 import java.security.KeyStore;
 import java.security.PrivateKey;
+import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
+import java.security.interfaces.RSAPrivateKey;
 import java.util.Enumeration;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 import javax.annotation.Nonnull;
 import javax.annotation.PostConstruct;
 import javax.validation.constraints.NotEmpty;
+import lombok.Data;
 import lombok.Getter;
+import lombok.NoArgsConstructor;
 import lombok.Setter;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.NestedConfigurationProperty;
+import org.springframework.core.io.DefaultResourceLoader;
+import org.springframework.core.io.Resource;
+import org.springframework.core.io.ResourceLoader;
+import org.springframework.security.converter.RsaKeyConverters;
 import org.springframework.security.saml2.core.Saml2X509Credential;
 import org.springframework.util.StringUtils;
 import org.springframework.validation.annotation.Validated;
@@ -47,11 +57,37 @@
 @ConfigurationProperties("saml")
 @NullableByDefault
 public class SecuritySamlProperties {
+  public static final String FILE_PREFIX = "file:";
   private Path keyStore;
   private String keyStoreType = "PKCS12";
   private String keyStorePassword;
   private String keyStoreAliasName = "mykey"; // default alias for keytool
 
+  // the privatekey/cert location files can be generated via
+  // openssl req -new -x509 -nodes -keyout private_key.pem -out certificate.pem -subj
+  // "/CN=Spinnaker" -days 3650
+  @Data
+  @NoArgsConstructor
+  public static class Credential {
+    private String privateKeyLocation;
+    private String certificateLocation;
+
+    public Credential(String privateKeyLocation, String certificateLocation) {
+      setCertificateLocation(certificateLocation);
+      setPrivateKeyLocation(privateKeyLocation);
+    }
+
+    public void setCertificateLocation(String certificateLocation) {
+      this.certificateLocation = addFilePrefixIfNeeded(certificateLocation);
+    }
+
+    public void setPrivateKeyLocation(String privateKeyLocation) {
+      this.privateKeyLocation = addFilePrefixIfNeeded(privateKeyLocation);
+    }
+  }
+
+  private List<Credential> signingCredentials = new LinkedList<>();
+
   public Saml2X509Credential getDecryptionCredential()
       throws IOException, GeneralSecurityException {
     if (keyStore == null) {
@@ -71,6 +107,82 @@ public Saml2X509Credential getDecryptionCredential()
     return Saml2X509Credential.decryption(privateKey, certificate);
   }
 
+  private static String addFilePrefixIfNeeded(String property) {
+    if (StringUtils.hasLength(property) && property.startsWith("/")) {
+      return FILE_PREFIX + property;
+    }
+    return property;
+  }
+  // @formatter:off
+  /**
+   * Try to match the standard spring saml config LIKE the below. This would be a keylcoak
+   * configuration example. Note that this does NOT match, but at least the private key suff does. *
+   * <br>
+   * <!-- @formatter:off -->
+   *
+   * <pre>
+   * spring:
+   *   security:
+   *     saml2:
+   *     ## We ARE a relying party.
+   *       relyingparty:
+   *         registration:
+   *           SSO:
+   *           ## This would be the SSO provider information.  Asserting party would be who's sending the SAML payload (e.g. okta/keycloak/etc)
+   *             assertingparty:
+   *               metadata-uri: http://192.168.1.2:8080/realms/master/protocol/saml/descriptor
+   *               entityId: Spinnaker
+   *               ## This uses the signing credentials in a separate block.  Why it's there vs. here is strange
+   *               singlesignon:
+   *                 sign-request: true
+   *                 url: http://192.168.1.2:8080/realms/master/protocol/saml
+   *               ## This IF metadata-uri is set SHOULD NOT be needed as it's PARSED as part of the operation
+   *               ## this is the decryptionCredentials
+   *               verification:
+   *               ## Nominally this could be in a JKS or a P12 keystore as well.  REMINDER:  this is basically
+   *               ## coming from the metadata in NORMAL cases.
+   *                 credentials:
+   *                 - certificate-location: encryptedFile:k8s!n:saml-verification-secret!k:certificate.pem
+   *                   private-key-location: encryptedFile:k8s!n:saml-verification-secret!k:private_key.pem
+   *             signing:
+   *               credentials:
+   *               - certificate-location: encryptedFile:k8s!n:saml-signing-secret!k:certificate.pem
+   *                 private-key-location: encryptedFile:k8s!n:saml-signing-secret!k:private_key.pem
+   *
+   * </pre>
+   *
+   * *
+   * <!-- @formatter:on -->
+   * NOTE: We ONLY support a single credential with this config.
+   *
+   * @return the credentials piece based upon privateKey/certificate
+   */
+  // @formatter:on
+  // TODO:  Replace this entire thing with standard spring security saml loading vs. doing all this
+  // ourselves
+  // See:
+  // https://docs.spring.io/spring-boot/api/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyProperties.Registration.html for the auto wiring based approach
+  @Nonnull
+  public List<Saml2X509Credential> getSigningCredentials() {
+    if (this.signingCredentials != null && !signingCredentials.isEmpty()) {
+      return this.signingCredentials.stream()
+          .map(
+              each ->
+                  getSaml2Credential(
+                      each.getPrivateKeyLocation(),
+                      each.getCertificateLocation(),
+                      Saml2X509Credential.Saml2X509CredentialType.SIGNING))
+          .toList();
+    }
+    return new LinkedList<>();
+  }
+
+  /**
+   * Sign requests via the WantAuthnRequestsSigned XML flag. Defaults to false. Keycloak defaults to
+   * true, okta DOES NOT support true. Uses the signing credentials
+   */
+  private boolean signRequests = false;
+
   /** URL pointing to the SAML metadata to use. */
   private String metadataUrl;
 
@@ -112,9 +224,7 @@ public String getAssertionConsumerServiceLocation() {
 
   @PostConstruct
   public void validate() throws IOException, GeneralSecurityException {
-    if (StringUtils.hasLength(metadataUrl) && metadataUrl.startsWith("/")) {
-      metadataUrl = "file:" + metadataUrl;
-    }
+    metadataUrl = addFilePrefixIfNeeded(metadataUrl);
     if (keyStore != null) {
       if (keyStoreType == null) {
         keyStoreType = "PKCS12";
@@ -159,4 +269,38 @@ public static class UserAttributeMapping {
     private String username;
     private String email;
   }
+
+  /// Filched DIRECTLY from spring-security private temporarily until all of this is replaced
+  // directly with the autowired spring configuration stuff
+  ///
+  // https://github.com/spring-projects/spring-security/blob/main/config/src/main/java/org/springframework/security/config/saml2/RelyingPartyRegistrationsBeanDefinitionParser.java
+  private static final ResourceLoader resourceLoader = new DefaultResourceLoader();
+
+  private static Saml2X509Credential getSaml2Credential(
+      String privateKeyLocation,
+      String certificateLocation,
+      Saml2X509Credential.Saml2X509CredentialType credentialType) {
+    RSAPrivateKey privateKey = readPrivateKey(privateKeyLocation);
+    X509Certificate certificate = readCertificate(certificateLocation);
+    return new Saml2X509Credential(privateKey, certificate, credentialType);
+  }
+
+  private static RSAPrivateKey readPrivateKey(String privateKeyLocation) {
+    Resource privateKey = resourceLoader.getResource(privateKeyLocation);
+    try (InputStream inputStream = privateKey.getInputStream()) {
+      return RsaKeyConverters.pkcs8().convert(inputStream);
+    } catch (Exception ex) {
+      throw new IllegalArgumentException(ex);
+    }
+  }
+
+  private static X509Certificate readCertificate(String certificateLocation) {
+    Resource certificate = resourceLoader.getResource(certificateLocation);
+    try (InputStream inputStream = certificate.getInputStream()) {
+      return (X509Certificate)
+          CertificateFactory.getInstance("X.509").generateCertificate(inputStream);
+    } catch (Exception ex) {
+      throw new IllegalArgumentException(ex);
+    }
+  }
 }

gate-saml/src/test/java/com/netflix/spinnaker/gate/security/saml/SecuritySamlPropertiesTest.java

@@ -0,0 +1,36 @@
+package com.netflix.spinnaker.gate.security.saml;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.springframework.security.saml2.core.Saml2X509Credential;
+
+class SecuritySamlPropertiesTest {
+
+  @Test
+  public void verifyCanLoadCerts() {
+    SecuritySamlProperties properties = new SecuritySamlProperties();
+    properties.setSigningCredentials(
+        List.of(
+            new SecuritySamlProperties.Credential(
+                "classpath:private_key.pem", "classpath:certificate.pem")));
+    List<Saml2X509Credential> signingCredentials = properties.getSigningCredentials();
+    assertThat(signingCredentials.get(0).getPrivateKey().getAlgorithm()).isEqualTo("RSA");
+  }
+
+  @Test
+  public void verifyCanLoadCertsFromAFileLocation() {
+    SecuritySamlProperties properties = new SecuritySamlProperties();
+    Path currentDir = Paths.get("");
+    properties.setSigningCredentials(
+        List.of(
+            new SecuritySamlProperties.Credential(
+                currentDir.toAbsolutePath() + "/src/test/resources/private_key.pem",
+                currentDir.toAbsolutePath() + "/src/test/resources/certificate.pem")));
+    List<Saml2X509Credential> signingCredentials = properties.getSigningCredentials();
+    assertThat(signingCredentials.get(0).getPrivateKey().getAlgorithm()).isEqualTo("RSA");
+  }
+}

gate-saml/src/test/resources/certificate.pem

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDCTCCAfGgAwIBAgIUOevToAcJr3FsI/MsoEq75EwMt5QwDQYJKoZIhvcNAQEL
+BQAwFDESMBAGA1UEAwwJU3Bpbm5ha2VyMB4XDTI1MDUwOTIwNTE0OFoXDTM1MDUw
+NzIwNTE0OFowFDESMBAGA1UEAwwJU3Bpbm5ha2VyMIIBIjANBgkqhkiG9w0BAQEF
+AAOCAQ8AMIIBCgKCAQEAryEo1o+YyVssgFuI5N5U3eQyvAoGbtXCeaCoR1LltAqq
+SrDGyrzJnB3kXtd0RieGK0AxBg+FgL/Xl8kw+p+72h7gW8XYi9gS92fGYVyoY9nz
+c9WueQpazzfuQjx5UuDqZn59f6EsAEAwzIAep9YHwysAuma8PYlNzqnhTWTu5tJG
+SvCD5kv11njYHJJ1ntIaUHGNlpHH2rAPWKcf8XeqBfvFgBHDLRpuj+2XOijwY6uz
+iOgnsViOG/NbC7zBbpkwELoWcxsh3msLs4HIs6DRULtQq6EUnvjx72pwM14GlQ+9
+ZrcnxtX7qQ4ODf2rNpobmoBkE2mfV72vNS4J5XpltwIDAQABo1MwUTAdBgNVHQ4E
+FgQU9I5Nl3n/D1gdrOapVfpRUTUSfPwwHwYDVR0jBBgwFoAU9I5Nl3n/D1gdrOap
+VfpRUTUSfPwwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAfW58
+p9z8T+OUbi284Eejvy+k4hDvCB3+90GoiXrVQvhBlfXr9eO4rzkuOQe9Qw4jVf47
+RBwvUuen+gOMo6zx+PK7wKTZqmluKzPf5ic1XOl7M9wy14g84bBWP5EniJdqzpup
+WQK58sYWZc0QrpwGbWIkr7QQGju0F4nte8at/MvK9uHmtK5j6Rg3axPjLInIAuXq
+d/+H+CKRbybwS/ZCp37rjD9pyVYYIQ7ZZt/0EZCAaAKPeZ6RFhIguIpx1Ww5mMrF
+JK1LnWPuzr8R30SoH/BRQwU/sM7cvYsmhVsKNPfaaEbakqfo+LtSD+yrtRvbCo69
+c1ZqBjsV5B1736V+CQ==
+-----END CERTIFICATE-----

gate-saml/src/test/resources/private_key.pem

@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCvISjWj5jJWyyA
+W4jk3lTd5DK8CgZu1cJ5oKhHUuW0CqpKsMbKvMmcHeRe13RGJ4YrQDEGD4WAv9eX
+yTD6n7vaHuBbxdiL2BL3Z8ZhXKhj2fNz1a55ClrPN+5CPHlS4Opmfn1/oSwAQDDM
+gB6n1gfDKwC6Zrw9iU3OqeFNZO7m0kZK8IPmS/XWeNgcknWe0hpQcY2WkcfasA9Y
+px/xd6oF+8WAEcMtGm6P7Zc6KPBjq7OI6CexWI4b81sLvMFumTAQuhZzGyHeawuz
+gcizoNFQu1CroRSe+PHvanAzXgaVD71mtyfG1fupDg4N/as2mhuagGQTaZ9Xva81
+LgnlemW3AgMBAAECggEAAOs3ZIFN+b6p13ZMxTXacJWmyK9CVCfDA5pYqbyNgfdf
+15sfDUyrCf0xYqpBnnZIwKmpMwW/TwPHTCS7BB5gS0EWaPZmelm8ilF/KRz2h+op
+fz4+f6ynXLvLteyK+ibCEPkUVoMuQEXT4OBWsA+0NqhGfN0pccvqdTOZnOlFt/0A
+yv2GfYLlP13nreU+u4efE7/i+E8xvUxdwH4sLSuKMIB/0aYI6LbdwJbqh5KMBLVt
+u+ooXRk8atsK7OucpSuo5Wj5jzUJV31PfYtpcnVnatLMjPAA+LQAYDrZkHLMXvv8
+opuyHExZlJ5iWx6tViQz0B1hrWcCIHS7cvx5+D/a6QKBgQDb9Crz3AOeZH3IR6Ha
+Zws4rhlqtq5Co8DAyTHE62XWkdTY4hhtfhvQ+8YH9jzU+fIBS39QVlM73MmPR4ga
+GtSTtsRNN1Oukouq+QvsXx9qR0XEoRBofVmo5VQilosZmqlAjc1vGCyDHpszCUOG
+pMVP/2lEiknheSCZobK9XfhQlQKBgQDL1HUPKQaR2iBHPnri7tZOD2K4c8jrwwVX
+YjGuUxyzyrnQq8XPkNolatPXxRoqIyqDDfXCafIC8g/BkAY9TxLhoyQEnN6aRODV
+ds3wKy/Dn09YaH4BtPbXaNv5cz6JkHtgtl6z6EXXaInb5GATGaHEw9KpH4/eWwIH
+A92iZyrOGwKBgQCaleSKNxsj+ySb2hxazwkH8PRUF8gpdcVGuSCNcZPFVgDt3Rml
++ne6TPlFJz5hwLjhSBpWcBVXgTj3xiJVln3Iwy77xeK+UqhupVJH8iK2IxlZtIk/
+prmZBnQ3Su7AM/64K/EyHx9Jl/0jxWL8Alnae3uUfEyoduT+lLJ2fNDEcQKBgAnc
+vsk7//BgsH0h/corKj1eqzUnjQozRnfi7Wp05QeiAHmjRg/z/0oeMB/ZjpmJWA49
+R63feHFCCxcfg93FjLFUNnLusCqguIw7kl1TiZ0agTlS3P3yJptnnHUmaVk4n2+f
+g1eLHo38peb41tk1vUkK/I9oUoq8to1mV3v7J+wPAoGBALbIYJ4O8FfclXR+63TK
+36FYlcHL2u0JjGXyByLM8x6xv9eHeFfij66k4OxWC9vpr3fO/AkbGWnceiztjca5
+lejCMjQkU7qbNRAr+XUuk9d6qD+sHd1XLYeXNY9WW9Ki8xbms9C7q/C/nq0uWyHu
+KyrlMN7DaB1PfNSRFDPBIEgv
+-----END PRIVATE KEY-----