[SECURITY-1527] Protect GerritServer.DescriptorImpl.testConnection

Author: rsandell

Diff for: pom.xml

@@ -57,7 +57,7 @@
         <java.level>8</java.level>
         <no-test-jar>false</no-test-jar>
         <findbugs.failOnError>false</findbugs.failOnError>
-        <concurrency>1C</concurrency>
+        <concurrency>0.5C</concurrency>
         <powermock.version>1.6.2</powermock.version>
         <checkstyle.version>2.13</checkstyle.version>
         <mockito.version>1.10.19</mockito.version>

Diff for: src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java

@@ -711,27 +711,15 @@ public String getDisplayName() {
          * @return {@link FormValidation#ok() } if can be done,
          *         {@link FormValidation#error(java.lang.String) } otherwise.
          */
+        @RequirePOST
         public FormValidation doTestConnection(
                 @QueryParameter("gerritHostName") final String gerritHostName,
                 @QueryParameter("gerritSshPort") final int gerritSshPort,
                 @QueryParameter("gerritProxy") final String gerritProxy,
                 @QueryParameter("gerritUserName") final String gerritUserName,
                 @QueryParameter("gerritAuthKeyFile") final String gerritAuthKeyFile,
                 @QueryParameter("gerritAuthKeyFilePassword") final String gerritAuthKeyFilePassword) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("gerritHostName = {}\n"
-                        + "gerritSshPort = {}\n"
-                        + "gerritProxy = {}\n"
-                        + "gerritUserName = {}\n"
-                        + "gerritAuthKeyFile = {}\n"
-                        + "gerritAuthKeyFilePassword = {}",
-                        new Object[]{gerritHostName,
-                            gerritSshPort,
-                            gerritProxy,
-                            gerritUserName,
-                            gerritAuthKeyFile,
-                            gerritAuthKeyFilePassword,  });
-            }
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
 
             File file = new File(gerritAuthKeyFile);
             String password = null;
@@ -790,11 +778,12 @@ public Integer call() throws Exception {
          * @param gerritHttpPassword the password
          * @return {@link FormValidation#ok()} if it works.
          */
+        @RequirePOST
         public FormValidation doTestRestConnection(
                 @QueryParameter("gerritFrontEndUrl") final String gerritFrontEndUrl,
                 @QueryParameter("gerritHttpUserName") final String gerritHttpUserName,
                 @QueryParameter("gerritHttpPassword") final String gerritHttpPassword) {
-
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
             String password = Secret.fromString(gerritHttpPassword).getPlainText();
 
             String restUrl = gerritFrontEndUrl;

Diff for: src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/LockedDownGerritEventTest.java

@@ -206,7 +206,7 @@ public void testUserCanConfigureAJob() throws Exception {
         assertEquals("theOtherServer", serverName);
     }
 
-    // CS IGNORE MagicNumber FOR NEXT 32 LINES. REASON: test data.
+    // CS IGNORE MagicNumber FOR NEXT 200 LINES. REASON: test data.
 
     /**
      * Tests that only an admin can read server configuration and manipulate server state.
@@ -223,11 +223,7 @@ public void testOnlyAdminCanPerformServerConfigurationActions() throws Exception
         gerritServer.start();
 
         Setup.lockDown(j);
-        j.getInstance().setAuthorizationStrategy(
-                new MockAuthorizationStrategy().grant(Item.READ, Item.DISCOVER).everywhere().toAuthenticated()
-                        .grant(Jenkins.READ, Item.DISCOVER).everywhere().toEveryone()
-                        .grant(Item.CONFIGURE).everywhere().to("bob")
-                        .grant(Jenkins.ADMINISTER).everywhere().to("alice"));
+        grantsToAliceBobAndEveryone();
         j.jenkins.setCrumbIssuer(null); //Not really testing csrf right now
         JenkinsRule.WebClient webClient = j.createWebClient().login("alice", "alice");
         HtmlPage page = webClient.goTo("plugin/gerrit-trigger/servers/0/");
@@ -240,6 +236,145 @@ public void testOnlyAdminCanPerformServerConfigurationActions() throws Exception
         post(webClient, "plugin/gerrit-trigger/servers/0/wakeup", null, 403);
     }
 
+    /**
+     * Tests that you can't do an http GET request to
+     * ${@link GerritServer.DescriptorImpl#doTestConnection(String, int, String, String, String, String)}.
+     *
+     * @throws Exception if so
+     */
+    @Test @Issue("SECURITY-1527")
+    public void testGetTestConnectionNotWorking() throws Exception {
+        GerritServer gerritServer = new GerritServer(PluginImpl.DEFAULT_SERVER_NAME);
+        SshdServerMock.configureFor(sshd, gerritServer);
+        PluginImpl.getInstance().addServer(gerritServer);
+        gerritServer.getConfig().setNumberOfSendingWorkerThreads(NUMBEROFSENDERTHREADS);
+        ((Config)gerritServer.getConfig()).setGerritAuthKeyFile(sshKey.getPrivateKey());
+        gerritServer.start();
+
+        Setup.lockDown(j);
+        grantsToAliceBobAndEveryone();
+        j.jenkins.setCrumbIssuer(null); //Not really testing csrf right now
+        JenkinsRule.WebClient webClient = j.createWebClient(); //No login this time
+
+        webClient.assertFails("descriptorByName/"
+                        + "com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer/"
+                        + "testConnection?"
+                        + "gerritHostName=foo&"
+                        + "gerritSshPort=29418&"
+                        + "gerritProxy=&"
+                        + "gerritUserName=foo"
+                        + "gerritAuthKeyFile=/tmp/foo"
+                        + "gerritAuthKeyFilePassword=bar"
+                        + "&foo=",
+                405);
+
+        webClient.assertFails("descriptorByName/"
+                        + "com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer/"
+                        + "testRestConnection?"
+                        + "gerritHttpUserName=foo&"
+                        + "gerritHttpPassword=bar&"
+                        + "gerritFrontEndUrl=http://localhost:8000/?foo=",
+                405);
+
+    }
+
+    /**
+     * Tests that you can do an http POST request to
+     * ${@link GerritServer.DescriptorImpl#doTestConnection(String, int, String, String, String, String)}.
+     *
+     * @throws Exception if so
+     */
+    @Test @Issue("SECURITY-1527")
+    public void testPostTestConnectionNotWorking() throws Exception {
+        GerritServer gerritServer = new GerritServer(PluginImpl.DEFAULT_SERVER_NAME);
+        SshdServerMock.configureFor(sshd, gerritServer);
+        PluginImpl.getInstance().addServer(gerritServer);
+        gerritServer.getConfig().setNumberOfSendingWorkerThreads(NUMBEROFSENDERTHREADS);
+        ((Config)gerritServer.getConfig()).setGerritAuthKeyFile(sshKey.getPrivateKey());
+        gerritServer.start();
+
+        Setup.lockDown(j);
+        grantsToAliceBobAndEveryone();
+        j.jenkins.setCrumbIssuer(null); //Not really testing csrf right now
+        JenkinsRule.WebClient webClient = j.createWebClient(); //No login this time
+
+        post(webClient, "descriptorByName/"
+                        + "com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer/"
+                        + "testConnection?"
+                        + "gerritHostName=foo&"
+                        + "gerritSshPort=29418&"
+                        + "gerritProxy=&"
+                        + "gerritUserName=foo"
+                        + "gerritAuthKeyFile=/tmp/foo"
+                        + "gerritAuthKeyFilePassword=bar",
+                null, 403);
+
+        post(webClient, "descriptorByName/"
+                        + "com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer/"
+                        + "testRestConnection?"
+                        + "gerritHttpUserName=foo&"
+                        + "gerritHttpPassword=bar&"
+                        + "gerritFrontEndUrl=http://localhost:8000/?foo=",
+                null, 403);
+
+    }
+
+    /**
+     * Tests that you can do an http POST request to
+     * ${@link GerritServer.DescriptorImpl#doTestConnection(String, int, String, String, String, String)}.
+     *
+     * @throws Exception if so
+     */
+    @Test @Issue("SECURITY-1527")
+    public void testPostTestConnectionWorking() throws Exception {
+        GerritServer gerritServer = new GerritServer(PluginImpl.DEFAULT_SERVER_NAME);
+        SshdServerMock.configureFor(sshd, gerritServer);
+        PluginImpl.getInstance().addServer(gerritServer);
+        gerritServer.getConfig().setNumberOfSendingWorkerThreads(NUMBEROFSENDERTHREADS);
+        ((Config)gerritServer.getConfig()).setGerritAuthKeyFile(sshKey.getPrivateKey());
+        gerritServer.start();
+
+        Setup.lockDown(j);
+        grantsToAliceBobAndEveryone();
+        j.jenkins.setCrumbIssuer(null); //Not really testing csrf right now
+        JenkinsRule.WebClient webClient = j.createWebClient().login("alice");
+
+        post(webClient, "descriptorByName/"
+                        + "com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer/"
+                        + "testConnection?"
+                        + "gerritHostName=foo&"
+                        + "gerritSshPort=29418&"
+                        + "gerritProxy=&"
+                        + "gerritUserName=foo&"
+                        + "gerritAuthKeyFile=/tmp/foo&"
+                        + "gerritAuthKeyFilePassword=bar",
+                null, null);
+
+        post(webClient, "descriptorByName/"
+                        + "com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer/"
+                        + "testRestConnection?"
+                        + "gerritHttpUserName=foo&"
+                        + "gerritHttpPassword=bar&"
+                        + "gerritFrontEndUrl=http://localhost:8000/",
+                null, null);
+
+    }
+
+    /**
+     * Grants some permissions.
+     *
+     * Read and discover to everyone and authenticated.
+     * Configure everywhere to bob.
+     * Administer everywhere to alice.
+     */
+    private void grantsToAliceBobAndEveryone() {
+        j.getInstance().setAuthorizationStrategy(
+                new MockAuthorizationStrategy().grant(Item.READ, Item.DISCOVER).everywhere().toAuthenticated()
+                        .grant(Jenkins.READ, Item.DISCOVER).everywhere().toEveryone()
+                        .grant(Item.CONFIGURE).everywhere().to("bob")
+                        .grant(Jenkins.ADMINISTER).everywhere().to("alice"));
+    }
+
     /**
      * Performs an HTTP POST request to the relative url.
      *