[java-identity-provider] 02/05: IDP-1545 Blacklist property replacement if properties are going into secrets.properties

Rod Widdowson rdw at steadingsoftware.com
Fri Feb 21 11:20:07 EST 2020


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch master
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=911b6d96fafdef06da83926dc9534acd52dd8085

commit 911b6d96fafdef06da83926dc9534acd52dd8085
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Feb 21 16:11:39 2020 +0000

    IDP-1545 Blacklist property replacement if properties are going into secrets.properties
    
    https://issues.shibboleth.net/jira/browse/IDP-1545
---
 .../idp/installer/PropertiesWithComments.java      | 16 ++++++++++++
 .../net/shibboleth/idp/installer/V4Install.java    | 12 +++++++--
 .../idp/installer/TestPropertiesWithComments.java  | 30 +++++++++++++++++++++-
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
index f71ea31..ee6febc 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
@@ -26,14 +26,17 @@ import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 
 import javax.annotation.Nonnull;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 /**
@@ -56,6 +59,9 @@ public final class PropertiesWithComments {
     /** Name Replacement info. */
     private final Properties nameReplacement;
 
+    /**  BlackListed property names. */
+    @Nonnull private final Set<String> blacklistedNames;
+
     /** Have we loaded data?.
      *
      * We cannot load the replacement names after the file load.
@@ -64,6 +70,14 @@ public final class PropertiesWithComments {
 
     /** Legacy Constructor. */
     public PropertiesWithComments() {
+        this(Collections.emptySet());
+    }
+
+    /** Constructor.
+     * @param blacklist names to warn on.
+     */
+    public PropertiesWithComments(@Nonnull final Set<String> blacklist) {
+        blacklistedNames = Set.copyOf(blacklist);
         nameReplacement = new Properties();
     }
 
@@ -218,6 +232,8 @@ public final class PropertiesWithComments {
      */
     public boolean replaceProperty(final String propName, final String newPropValue) {
 
+        Constraint.isFalse(blacklistedNames.contains(propName),
+                "property '" + propName + "' cannot be replaced");
         CommentedProperty p = properties.get(propName);
         if (null != p) {
             p.setValue(newPropValue);
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
index 63f61a4..e2802b0 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
@@ -28,6 +28,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.Properties;
+import java.util.Set;
 
 import javax.annotation.Nonnull;
 
@@ -205,6 +206,13 @@ public class V4Install extends AbstractInitializableComponent {
      */
     // CheckStyle: CyclomaticComplexity|MethodLength OFF
     protected void populatePropertyFiles(final boolean sealerCreated) throws BuildException {
+        final Set<String> blackList = Set.of(
+                "idp.sealer.storePassword",
+                "idp.sealer.keyPassword",
+                "idp.authn.LDAP.bindDNCredential",
+                "idp.attribute.resolver.LDAP.bindDNCredential",
+                "idp.persistentId.salt");
+
         final Path conf = installerProps.getTargetDir().resolve("conf");
         final Path dstConf = installerProps.getTargetDir().resolve("dist").resolve("conf");
         if (!currentState.isIdPPropertiesPresent()) {
@@ -219,7 +227,7 @@ public class V4Install extends AbstractInitializableComponent {
                 if (!Files.exists(source)) {
                     throw new BuildException("missing idp.properties in dist");
                 }
-                final PropertiesWithComments propertiesToReWrite = new PropertiesWithComments();
+                final PropertiesWithComments propertiesToReWrite = new PropertiesWithComments(blackList);
                 final Properties replacements;
                 if (mergePath != null) {
                     log.debug("Creating {} from {} and {}", target, source, mergePath);
@@ -260,7 +268,7 @@ public class V4Install extends AbstractInitializableComponent {
                     throw new BuildException("missing ldap.properties in dist");
                 }
                 log.debug("Creating {} from {} and {}", target, source, ldapMergePath);
-                final PropertiesWithComments propertiesToReWrite = new PropertiesWithComments();
+                final PropertiesWithComments propertiesToReWrite = new PropertiesWithComments(blackList);
                 final Properties replacements = new Properties();
                 final File mergeFile = ldapMergePath.toFile();
                 if (!installerProps.isNoTidy()) {
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/TestPropertiesWithComments.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/TestPropertiesWithComments.java
index 0cd3d4c..8c71504 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/TestPropertiesWithComments.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/TestPropertiesWithComments.java
@@ -26,15 +26,19 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Properties;
+import java.util.Set;
 
 import org.testng.Assert;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
+
 /**
  * test for {@link PropertiesWithComments}.
  */
+ at SuppressWarnings("javadoc")
 public class TestPropertiesWithComments {
 
     // We use a File to aid scrutabiloty in testing
@@ -58,7 +62,7 @@ public class TestPropertiesWithComments {
 
 
     @Test public void testReplaceValues() throws FileNotFoundException, IOException {
-        final PropertiesWithComments pwc = new PropertiesWithComments();
+        final PropertiesWithComments pwc = new PropertiesWithComments(Set.of("a", "b"));
 
         pwc.load(getInputStream());
 
@@ -81,6 +85,30 @@ public class TestPropertiesWithComments {
         
     }
 
+    @Test public void testBlackList() throws IOException {
+        final PropertiesWithComments pwc = new PropertiesWithComments(Set.of("x", "a", "b"));
+
+        pwc.load(getInputStream());
+
+        pwc.replaceProperty("c", "new C");
+        try {
+            pwc.replaceProperty("a", "new C");
+            fail("Property Replacement with black listed name worked");
+        } catch (ConstraintViolationException e) {
+            // OK
+        }
+
+        Properties p = new Properties(1);
+        p.setProperty("b", "new b");
+        try {
+            pwc.replaceProperties(p);
+            fail("Property Replacement with black listed name failed");
+        } catch (ConstraintViolationException e) {
+            // OK
+        }
+
+    }
+
     @Test public void testReplaceNamesFail() throws FileNotFoundException, IOException {
         final PropertiesWithComments pwc = new PropertiesWithComments();
         pwc.load(getInputStream());

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list