changeset 1797:7f2a3a806c77

Prevent infinite loops on short input for "setup -c". Reviewed-by: aazores, neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-September/016049.html Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-September/016171.html PR2626
author Severin Gehwolf <sgehwolf@redhat.com>
date Wed, 09 Sep 2015 10:00:43 +0200
parents 539a7e34b6b3
children e955837e95de
files setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/CLISetup.java setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReader.java setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReader.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/CLISetupTest.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReaderTest.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReaderTest.java
diffstat 6 files changed, 182 insertions(+), 7 deletions(-) [+]
line wrap: on
line diff
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/CLISetup.java	Wed Sep 16 10:45:57 2015 -0400
+++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/CLISetup.java	Wed Sep 09 10:00:43 2015 +0200
@@ -54,6 +54,7 @@
 
 public class CLISetup {
 
+    private static final int EOF = -1;
     private static final Logger logger = LoggingUtils.getLogger(CLISetup.class);
     private static final Translate<LocaleResources> t = LocaleResources.createLocalizer();
     private final ThermostatSetup thermostatSetup;
@@ -165,6 +166,9 @@
         int currTry = 0;
         do {
             input = readLine(in);
+            if (input == null) {
+                throw new IOException("Unexpected EOF while reading answer.");
+            }
             if (input.equals(localizedCancelToken)) {
                 return false;
             }
@@ -181,13 +185,17 @@
     private String readLine(InputStream in) throws IOException {
         int c;
         StringBuilder builder = new StringBuilder();
-        while ((c = in.read()) != -1) {
+        while ((c = in.read()) != EOF) {
             char token = (char) c;
             if (token == '\n') {
                 break;
             }
             builder.append(token);
         }
+        if (c == EOF) {
+            // reached EOF before we read any other character.
+            return null;
+        }
         return builder.toString();
     }
 
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReader.java	Wed Sep 16 10:45:57 2015 -0400
+++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReader.java	Wed Sep 09 10:00:43 2015 +0200
@@ -50,6 +50,7 @@
 
 class PasswordCredentialsReader {
 
+    private static final int MAX_TRIES = 100;
     private static final LocalizedString UNUSED = new LocalizedString("ignored");
     private static final Translate<LocaleResources> t = LocaleResources.createLocalizer();
     private final Console console;
@@ -70,11 +71,19 @@
         char[] password;
         char[] confirmation;
         boolean isValid = false;
+        int currTry = 0;
         do {
             StorageAuthInfoGetter getter = new StorageAuthInfoGetter(console, UNUSED, passwordPrompt);
             password = getter.getPassword(null);
+            if (password == null) {
+                throw new IOException("Unexpected EOF while reading password.");
+            }
             StorageAuthInfoGetter confirmGetter = new StorageAuthInfoGetter(console, UNUSED, confirmPasswordPrompt);
             confirmation = confirmGetter.getPassword(null);
+            if (confirmation == null) {
+                throw new IOException("Unexpected EOF while reading password confirmation.");
+            }
+            currTry++;
             try {
                 verifyPassword(password, confirmation);
                 Arrays.fill(confirmation, '\0');
@@ -87,13 +96,21 @@
                 printError(LocaleResources.CLI_SETUP_PASSWORD_MISMATCH);
                 clearPasswords(password, confirmation);
             }
-        } while (!isValid);
+        } while (!isValid && currTry < MAX_TRIES);
+        // If we have reached maximum tries then we might still be invalid
+        if (!isValid) {
+            throw new IOException("Tried " + MAX_TRIES + " times and got invalid input each time.");
+        }
         return password;
     }
     
     private void clearPasswords(char[] password, char[] confirmation) {
-        Arrays.fill(password, '\0');
-        Arrays.fill(confirmation, '\0');
+        if (password != null) {
+            Arrays.fill(password, '\0');
+        }
+        if (confirmation != null) {
+            Arrays.fill(confirmation, '\0');
+        }
         password = null;
         confirmation = null;
     }
@@ -104,7 +121,6 @@
     }
     
     private void verifyPassword(char[] password, char[] confirmation) throws InvalidPasswordException, PasswordMismatchException {
-        checkPasswordsMatch(password, confirmation);
         UserCredsValidator validator = new UserCredsValidator();
         try {
             validator.validatePassword(password);
@@ -112,6 +128,8 @@
         } catch (IllegalArgumentException e) {
             throw new InvalidPasswordException();
         }
+        // passwords are now non-null and not empty
+        checkPasswordsMatch(password, confirmation);
     }
 
     void checkPasswordsMatch(char[] first, char[] second) throws PasswordMismatchException {
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReader.java	Wed Sep 16 10:45:57 2015 -0400
+++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReader.java	Wed Sep 09 10:00:43 2015 +0200
@@ -48,6 +48,7 @@
 
 class UsernameCredentialsReader {
 
+    private static final int MAX_TRIES = 100;
     private static final LocalizedString UNUSED = new LocalizedString("ignored");
     private static final Translate<LocaleResources> t = LocaleResources.createLocalizer();
     private final Console console;
@@ -65,8 +66,13 @@
         boolean isValid = false;
         UserCredsValidator validator = new UserCredsValidator();
         String username = null;
-        while (!isValid) {
+        int currTry = 0;
+        while (!isValid && currTry < MAX_TRIES) {
             username = getter.getUserName(null);
+            if (username == null) {
+                throw new IOException("Unexpected EOF while reading username.");
+            }
+            currTry++;
             try {
                 validator.validateUsername(username);
                 isValid = true;
@@ -75,6 +81,10 @@
                 // continue loop
             }
         }
+        // If we have reached maximum tries then we might still be invalid
+        if (!isValid) {
+            throw new IOException("Tried " + MAX_TRIES + " times and got invalid input each time.");
+        }
         return username;
     }
 
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/CLISetupTest.java	Wed Sep 16 10:45:57 2015 -0400
+++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/CLISetupTest.java	Wed Sep 09 10:00:43 2015 +0200
@@ -136,7 +136,8 @@
             cliSetup.run();
             fail("Expected setup to cancel");
         } catch (CommandException e) {
-            assertEquals("Setup cancelled on user request.", e.getMessage());
+            IOException cause = (IOException)e.getCause();
+            assertEquals("Unexpected EOF while reading answer.", cause.getMessage());
         }
     }
     
@@ -216,6 +217,36 @@
         assertEquals("Expected no errors", "", new String(berr.toByteArray()));
     }
     
+    /*
+     * Verifies that if webapp is installed but only mongodb credentials are
+     * provided, setup fails and does not loop forever. 
+     */
+    @Test
+    public void setupDoesNotLoopOnShortInput() throws CommandException {
+        String input = "yes\nmongodb-user\nfoo\nfoo\n";
+        doShortInputTest(input);
+    }
+    
+    @Test
+    public void setupDoesNotLoopOnShortInput2() throws CommandException {
+        String input = "yes\nmongodb-user\nfoo\nfoo\nsomeUser\n";
+        doShortInputTest(input);
+    }
+
+    private void doShortInputTest(String input) {
+        // simulate webapp being installed
+        when(thermostatSetup.isWebAppInstalled()).thenReturn(true);
+        
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            cliSetup.run();
+            fail("Expected exception due to invalid - too short - input");
+        } catch (CommandException e) {
+            IOException cause = (IOException)e.getCause();
+            assertTrue(cause.getMessage().startsWith("Unexpected EOF while reading"));
+        }
+    }
+    
     private CharArrayMatcher matchesPassword(char[] array) {
         return new CharArrayMatcher(array);
     }
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReaderTest.java	Wed Sep 16 10:45:57 2015 -0400
+++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReaderTest.java	Wed Sep 09 10:00:43 2015 +0200
@@ -46,6 +46,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -100,4 +101,75 @@
         assertEquals("Expected no errors", "", new String(berr.toByteArray()));
         assertEquals("tell me the password: \nrepeat the password: \n", new String(bout.toByteArray()));
     }
+    
+    @Test
+    public void canGetPasswordNoEOLAfterConfirmation() throws IOException {
+        String input = "bar\nbar";
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        char[] password = credsReader.readPassword();
+        assertArrayEquals(new char[] { 'b', 'a', 'r' }, password);
+        assertEquals("Expected no errors", "", new String(berr.toByteArray()));
+        assertEquals("tell me the password: \nrepeat the password: \n", new String(bout.toByteArray()));
+    }
+    
+    /*
+     * Insufficient input should not loop forever.
+     */
+    @Test
+    public void testShortReadNoConfirmation() throws IOException {
+        String input = "bar\n"; // expected password + confirmation
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            credsReader.readPassword();
+            fail("should not reach here");
+        } catch (IOException e) {
+            assertEquals("Unexpected EOF while reading password confirmation.", e.getMessage());
+        }
+    }
+    
+
+    @Test
+    public void testShortReadNothing() throws IOException {
+        String input = ""; // expected password + confirmation
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            credsReader.readPassword();
+            fail("should not reach here");
+        } catch (IOException e) {
+            assertEquals("Unexpected EOF while reading password.", e.getMessage());
+        }
+    }
+    
+    @Test
+    public void testShortReadNoMatch() throws IOException {
+        String input = "foo\nbar"; // no EOL after bar. next iteration is null password
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            credsReader.readPassword();
+            fail("should not reach here");
+        } catch (IOException e) {
+            assertEquals("Unexpected EOF while reading password.", e.getMessage());
+        }
+    }
+    
+    @Test
+    public void noMatchAfter100TriesBreaksLoop() {
+        String input = build101NoMatchingPasswords("mypassword");
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            credsReader.readPassword();
+            fail("should not reach here");
+        } catch (IOException e) {
+            assertEquals("Tried 100 times and got invalid input each time.", e.getMessage());
+        }
+    }
+    
+    private String build101NoMatchingPasswords(String password) {
+        StringBuilder builder = new StringBuilder();
+        for (int i = 0; i < 101; i++) {
+            builder.append(password + "\n");
+            builder.append(password + "-no-match\n");
+        }
+        return builder.toString();
+    }
 }
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReaderTest.java	Wed Sep 16 10:45:57 2015 -0400
+++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReaderTest.java	Wed Sep 09 10:00:43 2015 +0200
@@ -37,6 +37,7 @@
 package com.redhat.thermostat.setup.command.internal.cli;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -88,4 +89,39 @@
         assertEquals("Chosen username '' invalid!\n", new String(berr.toByteArray()));
         assertEquals("tell me the username: \ntell me the username: try-second-time\n", new String(bout.toByteArray()));
     }
+    
+    /*
+     * Insufficient input should not loop forever.
+     */
+    @Test
+    public void testShortRead() throws IOException {
+        String input = "";
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            credsReader.read();
+            fail("should not reach here");
+        } catch (IOException e) {
+            assertEquals("Unexpected EOF while reading username.", e.getMessage());
+        }
+    }
+    
+    @Test
+    public void tooManyInvalidInputsBreaksLoop() throws IOException {
+        String input = build101EmptyInputs();
+        when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes()));
+        try {
+            credsReader.read();
+            fail("should not reach here");
+        } catch (IOException e) {
+            assertEquals("Tried 100 times and got invalid input each time.", e.getMessage());
+        }
+    }
+
+    private String build101EmptyInputs() {
+        StringBuilder builder = new StringBuilder();
+        for (int i = 0; i < 101; i++) {
+            builder.append("\n");
+        }
+        return builder.toString();
+    }
 }