You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by sk...@apache.org on 2020/12/01 22:32:33 UTC

[ignite] branch master updated: IGNITE-13716 Fixed an issue where control utility did not hide sensitive information. Fixes #8471

This is an automated email from the ASF dual-hosted git repository.

sk0x50 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git


The following commit(s) were added to refs/heads/master by this push:
     new f5dfac2  IGNITE-13716 Fixed an issue where control utility did not hide sensitive information. Fixes #8471
f5dfac2 is described below

commit f5dfac22bd79fd0f13cce9840fe20114ab718976
Author: Slava Koptilin <sl...@gmail.com>
AuthorDate: Wed Dec 2 01:32:03 2020 +0300

    IGNITE-13716 Fixed an issue where control utility did not hide sensitive information. Fixes #8471
---
 .../internal/commandline/CommandHandler.java       | 33 +++++++++++-
 .../internal/commandline/CommonArgParser.java      | 15 ++++++
 .../GridCommandHandlerSslWithSecurityTest.java     | 59 +++++++++++++++++++++-
 3 files changed, 104 insertions(+), 3 deletions(-)

diff --git a/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommandHandler.java b/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommandHandler.java
index a612e1c..14c8799 100644
--- a/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommandHandler.java
+++ b/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommandHandler.java
@@ -46,6 +46,7 @@ import org.apache.ignite.internal.client.impl.connection.GridClientConnectionRes
 import org.apache.ignite.internal.client.ssl.GridSslBasicContextFactory;
 import org.apache.ignite.internal.util.typedef.F;
 import org.apache.ignite.internal.util.typedef.X;
+import org.apache.ignite.internal.util.typedef.internal.SB;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.logger.java.JavaLoggerFileHandler;
 import org.apache.ignite.logger.java.JavaLoggerFormatter;
@@ -266,7 +267,7 @@ public class CommandHandler {
                     }
 
                     logger.info("Command [" + commandName + "] started");
-                    logger.info("Arguments: " + String.join(" ", rawArgs));
+                    logger.info("Arguments: " + argumentsToString(rawArgs));
                     logger.info(DELIM);
 
                     lastOperationRes = command.execute(clientCfg, logger, args.verbose());
@@ -455,6 +456,36 @@ public class CommandHandler {
     }
 
     /**
+     * Joins user's arguments and hides sensitive information.
+     *
+     * @param rawArgs Arguments which user has provided.
+     * @return String which could be shown in console and pritned to log.
+     */
+    private String argumentsToString(List<String> rawArgs) {
+        boolean hide = false;
+
+        SB sb = new SB();
+
+        for (int i = 0; i < rawArgs.size(); i++) {
+            if (hide) {
+                sb.a("***** ");
+
+                hide = false;
+
+                continue;
+            }
+
+            String arg = rawArgs.get(i);
+
+            sb.a(arg).a(' ');
+
+            hide = CommonArgParser.isSensitiveArgument(arg);
+        }
+
+        return sb.toString();
+    }
+
+    /**
      * Does one of three things:
      * <ul>
      *     <li>returns user name from connection parameters if it is there;</li>
diff --git a/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommonArgParser.java b/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommonArgParser.java
index 52e95f4..e27179d 100644
--- a/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommonArgParser.java
+++ b/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/CommonArgParser.java
@@ -103,6 +103,9 @@ public class CommonArgParser {
     /** List of optional auxiliary commands. */
     private static final Set<String> AUX_COMMANDS = new HashSet<>();
 
+    /** Set of sensitive arguments */
+    private static final Set<String> SENSITIVE_ARGUMENTS = new HashSet<>();
+
     static {
         AUX_COMMANDS.add(CMD_HOST);
         AUX_COMMANDS.add(CMD_PORT);
@@ -127,6 +130,18 @@ public class CommonArgParser {
         AUX_COMMANDS.add(CMD_TRUSTSTORE);
         AUX_COMMANDS.add(CMD_TRUSTSTORE_PASSWORD);
         AUX_COMMANDS.add(CMD_TRUSTSTORE_TYPE);
+
+        SENSITIVE_ARGUMENTS.add(CMD_PASSWORD);
+        SENSITIVE_ARGUMENTS.add(CMD_KEYSTORE_PASSWORD);
+        SENSITIVE_ARGUMENTS.add(CMD_TRUSTSTORE_PASSWORD);
+    }
+
+    /**
+     * @param arg To check.
+     * @return True if provided argument is among sensitive one and not should be displayed.
+     */
+    public static boolean isSensitiveArgument(String arg) {
+        return SENSITIVE_ARGUMENTS.contains(arg);
     }
 
     /**
diff --git a/modules/control-utility/src/test/java/org/apache/ignite/internal/processors/security/GridCommandHandlerSslWithSecurityTest.java b/modules/control-utility/src/test/java/org/apache/ignite/internal/processors/security/GridCommandHandlerSslWithSecurityTest.java
index 18a60ca..a34c619 100644
--- a/modules/control-utility/src/test/java/org/apache/ignite/internal/processors/security/GridCommandHandlerSslWithSecurityTest.java
+++ b/modules/control-utility/src/test/java/org/apache/ignite/internal/processors/security/GridCommandHandlerSslWithSecurityTest.java
@@ -17,22 +17,29 @@
 
 package org.apache.ignite.internal.processors.security;
 
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Handler;
+import java.util.logging.Logger;
 import org.apache.ignite.configuration.ConnectorConfiguration;
 import org.apache.ignite.configuration.IgniteConfiguration;
 import org.apache.ignite.internal.IgniteEx;
 import org.apache.ignite.internal.commandline.CommandHandler;
 import org.apache.ignite.internal.commandline.NoopConsole;
 import org.apache.ignite.internal.processors.security.impl.TestSecurityPluginProvider;
+import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
 import org.junit.Test;
 
+import static org.apache.ignite.cluster.ClusterState.ACTIVE;
 import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_OK;
 import static org.apache.ignite.internal.commandline.CommandList.DEACTIVATE;
 import static org.apache.ignite.plugin.security.SecurityPermissionSetBuilder.ALLOW_ALL;
+import static org.apache.ignite.testframework.GridTestUtils.assertContains;
 import static org.apache.ignite.testframework.GridTestUtils.keyStorePassword;
 import static org.apache.ignite.testframework.GridTestUtils.keyStorePath;
 import static org.apache.ignite.testframework.GridTestUtils.sslTrustedFactory;
@@ -47,13 +54,51 @@ public class GridCommandHandlerSslWithSecurityTest extends GridCommonAbstractTes
     /** Password. */
     private final String pwd = "testPwd";
 
+    /** System out. */
+    protected static PrintStream sysOut;
+
+    /**
+     * Test out - can be injected via {@link #injectTestSystemOut()} instead of System.out and analyzed in test.
+     * Will be as well passed as a handler output for an anonymous logger in the test.
+     */
+    protected static ByteArrayOutputStream testOut;
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTestsStarted() throws Exception {
+        super.beforeTestsStarted();
+
+        testOut = new ByteArrayOutputStream(16 * 1024);
+
+        sysOut = System.out;
+    }
+
     /** {@inheritDoc} */
     @Override protected void afterTest() throws Exception {
         super.afterTest();
 
+        System.setOut(sysOut);
+
+        testOut.reset();
+
         stopAllGrids();
     }
 
+    /**
+     * Sets test output stream.
+     */
+    protected void injectTestSystemOut() {
+        System.setOut(new PrintStream(testOut));
+    }
+
+    /**
+     * Flushes all Logger handlers to make log data available to test.
+     * @param hnd Command handler.
+     */
+    private void flushCommandOutput(CommandHandler hnd) {
+        Logger log = U.field(hnd, "logger");
+        Arrays.stream(log.getHandlers()).forEach(Handler::flush);
+    }
+
     /** {@inheritDoc} */
     @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
         return super.getConfiguration(igniteInstanceName)
@@ -77,7 +122,7 @@ public class GridCommandHandlerSslWithSecurityTest extends GridCommonAbstractTes
     public void testInputKeyTrustStorePwdOnlyOnce() throws Exception {
         IgniteEx crd = startGrid();
 
-        crd.cluster().active(true);
+        crd.cluster().state(ACTIVE);
 
         CommandHandler cmd = new CommandHandler();
 
@@ -129,7 +174,9 @@ public class GridCommandHandlerSslWithSecurityTest extends GridCommonAbstractTes
     public void testConnector() throws Exception {
         IgniteEx crd = startGrid();
 
-        crd.cluster().active(true);
+        crd.cluster().state(ACTIVE);
+
+        injectTestSystemOut();
 
         CommandHandler hnd = new CommandHandler();
 
@@ -143,5 +190,13 @@ public class GridCommandHandlerSslWithSecurityTest extends GridCommonAbstractTes
             "--truststore-password", keyStorePassword()));
 
         assertEquals(EXIT_CODE_OK, exitCode);
+
+        flushCommandOutput(hnd);
+
+        // Make sure all sensitive information is masked.
+        String testOutput = testOut.toString();
+        assertContains(log, testOutput, "--password *****");
+        assertContains(log, testOutput, "--keystore-password *****");
+        assertContains(log, testOutput, "--truststore-password *****");
     }
 }