CVE-2019-10348

@@ -101,6 +101,13 @@
       <artifactId>jenkins-client</artifactId>
       <version>0.3.7</version>
       <scope>test</scope>
+      <exclusions>
+        <!-- a forked version already included by the core -->
+        <exclusion>
+          <groupId>dom4j</groupId>
+          <artifactId>dom4j</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>

@@ -27,6 +27,7 @@ associated documentation files (the "Software"), to deal in the Software without
 import hudson.model.Job;
 import hudson.model.JobProperty;
 import hudson.model.JobPropertyDescriptor;
+import hudson.util.Secret;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
@@ -35,18 +36,23 @@ associated documentation files (the "Software"), to deal in the Software without
 
 @SuppressWarnings("ALL")
 public class GogsProjectProperty extends JobProperty<Job<?, ?>> {
-    private final String gogsSecret;
+    private final Secret gogsSecret;
     private final boolean gogsUsePayload;
     private final String gogsBranchFilter;
 
-    @DataBoundConstructor
+    @Deprecated
     public GogsProjectProperty(String gogsSecret, boolean gogsUsePayload, String gogsBranchFilter) {
+        this(Secret.fromString(gogsSecret), gogsUsePayload, gogsBranchFilter);
+    }
+
+    @DataBoundConstructor
+    public GogsProjectProperty(Secret gogsSecret, boolean gogsUsePayload, String gogsBranchFilter) {
         this.gogsSecret = gogsSecret;
         this.gogsUsePayload = gogsUsePayload;
         this.gogsBranchFilter = gogsBranchFilter;
     }
 
-    public String getGogsSecret() {
+    public Secret getGogsSecret() {
         return this.gogsSecret;
     }
 
@@ -74,12 +80,12 @@ public boolean filterBranch(String ref) {
     @Extension
     public static final class DescriptorImpl extends JobPropertyDescriptor {
         public static final String GOGS_PROJECT_BLOCK_NAME = "gogsProject";
-        private String gogsSecret;
+        private Secret gogsSecret;
         private boolean gogsUsePayload;
         private String gogsBranchFilter;
 
         public String getGogsSecret() {
-            return gogsSecret;
+            return Secret.toString(gogsSecret);
         }
 
         public boolean getGogsUsePayload() {
@@ -91,13 +97,16 @@ public String getGogsBranchFilter() {
         }
 
         public JobProperty<?> newInstance(StaplerRequest req, JSONObject formData) {
-            GogsProjectProperty tpp = req.bindJSON(
-                    GogsProjectProperty.class,
-                    formData.getJSONObject(GOGS_PROJECT_BLOCK_NAME)
-            );
+            GogsProjectProperty tpp = null;
+
+            if (req != null) {
+                tpp = req.bindJSON(
+                        GogsProjectProperty.class,
+                        formData.getJSONObject(GOGS_PROJECT_BLOCK_NAME)
+                );
+            }
             if (tpp != null) {
                 LOGGER.finest(formData.toString());
-                LOGGER.finest(tpp.gogsSecret);
                 LOGGER.finest(tpp.gogsBranchFilter);
 
                 gogsSecret = tpp.gogsSecret;

@@ -27,6 +27,7 @@ associated documentation files (the "Software"), to deal in the Software without
 import hudson.model.Job;
 import hudson.model.UnprotectedRootAction;
 import hudson.security.ACL;
+import hudson.util.Secret;
 import net.sf.json.JSONObject;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
@@ -187,7 +188,7 @@ public void doIndex(StaplerRequest req, StaplerResponse rsp) throws IOException
                     /* secret is stored in the properties of Job */
                     final GogsProjectProperty property = (GogsProjectProperty) job.getProperty(GogsProjectProperty.class);
                     if (property != null) { /* only if Gogs secret is defined on the job */
-                        jSecret = property.getGogsSecret(); /* Secret provided by Jenkins */
+                        jSecret = Secret.toString(property.getGogsSecret()); /* Secret provided by Jenkins */
                         isRefMatched = property.filterBranch(ref);
                     }
                 } else {
@@ -207,7 +208,7 @@ public void doIndex(StaplerRequest req, StaplerResponse rsp) throws IOException
                         /* secret is stored in the properties of Job */
                         final GogsProjectProperty property = (GogsProjectProperty) job.getProperty(GogsProjectProperty.class);
                         if (property != null) { /* only if Gogs secret is defined on the job */
-                            jSecret = property.getGogsSecret(); /* Secret provided by Jenkins */
+                            jSecret = Secret.toString(property.getGogsSecret()); /* Secret provided by Jenkins */
                             isRefMatched = property.filterBranch(ref);
                         }
                     }

@@ -1,4 +1,4 @@
-FROM gogs/gogs:0.11.4
+FROM gogs/gogs:0.11.86
 
 ARG FIRST_USER=dev
 

@@ -38,8 +38,11 @@ curl -v -X POST -s \
   -F "retype=${FIRST_USER}" \
   ${GITSERVER_URL}/user/sign_up
 
-cat /app/repos-to-mirror
+# Create Access token for first user
+TOKEN=$(curl -X POST -s -F "name=${FIRST_USER}" -u "${FIRST_USER}:${FIRST_USER}" \
+ ${GITSERVER_URL}/api/v1/users/${FIRST_USER}/tokens | sed 's/.*"sha1":"\(.*\)"}/\1/')
 
+echo $TOKEN
 while IFS= read -r LINE  || [[ -n "${LINE}" ]];
 do
   echo "==LINE: ${LINE}"
@@ -50,10 +53,10 @@ do
 
   # Create repo to migrate
   curl -v -X POST -s \
+    -H "Authorization: token ${TOKEN}" \
     -F "uid=1" \
     -F "clone_addr=${REMOTE_REPO_URL}" \
     -F "repo_name=${REPO_NAME}" \
-    -u "${FIRST_USER}:${FIRST_USER}" \
     ${GITSERVER_API_URL}/repos/migrate
 
 done < /app/repos-to-mirror

@@ -1,4 +1,4 @@
-FROM jenkinsci/jenkins:latest
+FROM jenkins/jenkins:lts-alpine
 
 # Load the required dependencies for the plugin under test
 RUN /usr/local/bin/install-plugins.sh git workflow-aggregator pipeline-model-extensions cloudbees-folder

@@ -1,13 +1,5 @@
 package org.jenkinsci.plugins.gogs;
 
-import net.sf.json.JSONObject;
-import net.sf.json.JSONSerializer;
-import org.apache.http.HttpHost;
-import org.apache.http.client.fluent.Executor;
-import org.apache.http.client.fluent.Form;
-import org.apache.http.client.fluent.Request;
-import org.apache.http.entity.ContentType;
-
 import java.io.File;
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -17,6 +9,15 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
+import net.sf.json.JSONArray;
+import net.sf.json.JSONObject;
+import net.sf.json.JSONSerializer;
+import org.apache.http.HttpHost;
+import org.apache.http.client.fluent.Executor;
+import org.apache.http.client.fluent.Form;
+import org.apache.http.client.fluent.Request;
+import org.apache.http.entity.ContentType;
+
 /**
  * A class to handle the configuration of the Gogs server used during the integration tests.
  */
@@ -28,7 +29,7 @@ public class GogsConfigHandler {
     private String gogsServer_password;
     private Executor executor = null;
     private String gogsServer_apiUrl = null;
-
+    private String gogsAccessToken;
 
     /**
      * Instantiates an object used to handle operations on a Gogs server.
@@ -46,6 +47,7 @@ public GogsConfigHandler(String gogsUrl, String user, String password) throws Ma
         this.gogsServer_port = aURL.getPort();
         this.gogsServer_user = user;
         this.gogsServer_password = password;
+        this.gogsAccessToken = getGogsAccessToken();
     }
 
     /**
@@ -60,10 +62,10 @@ void waitForServer(int retries, int retryDelay) throws TimeoutException, Interru
         String testUrl = this.getGogsUrl() + "/";
 
         for (int i = 0; i < retries; i++) {
-            int status = 0;
+            int status;
             try {
                 status = Request.Get(testUrl)
-                        .execute().returnResponse().getStatusLine().getStatusCode();
+                                .execute().returnResponse().getStatusLine().getStatusCode();
             } catch (IOException e) {
                 TimeUnit.SECONDS.sleep(retryDelay);
                 continue;
@@ -100,14 +102,17 @@ int createWebHook(String jsonCommand, String projectName) throws IOException {
                 + "/" + projectName + "/hooks";
 
         Executor executor = getExecutor();
+        Request request = Request.Post(gogsHooksConfigUrl);
+
+        if (gogsAccessToken != null) {
+            request.addHeader("Authorization", "token " + gogsAccessToken);
+        }
 
         String result = executor
-                .execute(Request.Post(gogsHooksConfigUrl).bodyString(jsonCommand, ContentType.APPLICATION_JSON))
+                .execute(request.bodyString(jsonCommand, ContentType.APPLICATION_JSON))
                 .returnContent().asString();
         JSONObject jsonObject = (JSONObject) JSONSerializer.toJSON( result );
-        int id = jsonObject.getInt("id");
-
-        return id;
+        return jsonObject.getInt("id");
     }
 
     /**
@@ -138,9 +143,14 @@ void removeHook(String projectName, int hookId) throws IOException {
                 + "/" + projectName + "/hooks/" + hookId;
 
         Executor executor = getExecutor();
+        Request request = Request.Delete(gogsHooksConfigUrl);
+
+        if (gogsAccessToken != null) {
+            request.addHeader("Authorization", "token " + gogsAccessToken);
+        }
 
         int result = executor
-                .execute(Request.Delete(gogsHooksConfigUrl))
+                .execute(request)
                 .returnResponse().getStatusLine().getStatusCode();
 
         if (result != 204) {
@@ -159,16 +169,20 @@ void createEmptyRepo(String projectName) throws IOException {
 
         Executor executor = getExecutor();
         String gogsHooksConfigUrl = getGogsServer_apiUrl() + "user/repos";
+        Request request = Request.Post(gogsHooksConfigUrl);
+
+        if (this.gogsAccessToken != null) {
+            request.addHeader("Authorization", "token " + this.gogsAccessToken);
+        }
 
         int result = executor
-                .execute(Request
-                        .Post(gogsHooksConfigUrl)
+                .execute(request
                         .bodyForm(Form.form()
-                                .add("name", projectName)
-                                .add("description", "API generated repository")
-                                .add("private", "true")
-                                .add("auto_init", "false")
-                                .build()
+                                      .add("name", projectName)
+                                      .add("description", "API generated repository")
+                                      .add("private", "true")
+                                      .add("auto_init", "false")
+                                      .build()
                         )
                 )
                 .returnResponse().getStatusLine().getStatusCode();
@@ -190,49 +204,37 @@ private Executor getExecutor() {
         if (this.executor == null) {
             HttpHost httpHost = new HttpHost(this.gogsServer_nodeName, this.gogsServer_port);
             this.executor = Executor.newInstance()
-                    .auth(httpHost, this.gogsServer_user, this.gogsServer_password)
-                    .authPreemptive(httpHost);
+                                    .auth(httpHost, this.gogsServer_user, this.gogsServer_password)
+                                    .authPreemptive(httpHost);
         }
         return this.executor;
     }
 
+    /**
+     * Get Access token of the user.
+     *
+     * @return an access token of the user
+     */
+    public String getGogsAccessToken() {
+        String resp;
+        String sha1 = null;
+        Executor executor = getExecutor();
+        try {
+            resp = executor.execute(
+                    Request.Get(this.getGogsUrl() + "/api/v1/users/" + this.gogsServer_user + "/tokens")
+            ).returnContent().toString();
+            JSONArray jsonArray = JSONArray.fromObject(resp);
+            if (!jsonArray.isEmpty()) {
+                sha1 = ((JSONObject) jsonArray.get(0)).getString("sha1");
+            }
+        } catch (IOException e) { }
+        return sha1;
+    }
+
     public String getGogsServer_apiUrl() {
         if (this.gogsServer_apiUrl == null) {
             this.gogsServer_apiUrl = this.getGogsUrl() + "/api/v1/";
         }
         return gogsServer_apiUrl;
     }
-
-    public String getGogsServer_nodeName() {
-        return gogsServer_nodeName;
-    }
-
-    public void setGogsServer_nodeName(String gogsServer_nodeName) {
-        this.gogsServer_nodeName = gogsServer_nodeName;
-    }
-
-    public int getGogsServer_port() {
-        return gogsServer_port;
-    }
-
-    public void setGogsServer_port(int gogsServer_port) {
-        this.gogsServer_port = gogsServer_port;
-    }
-
-    public String getGogsServer_user() {
-        return gogsServer_user;
-    }
-
-    public void setGogsServer_user(String gogsServer_user) {
-        this.gogsServer_user = gogsServer_user;
-    }
-
-    public String getGogsServer_password() {
-        return gogsServer_password;
-    }
-
-    public void setGogsServer_password(String gogsServer_password) {
-        this.gogsServer_password = gogsServer_password;
-    }
-
 }

@@ -0,0 +1,93 @@
+package org.jenkinsci.plugins.gogs;
+
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.model.FreeStyleProject;
+import hudson.util.Secret;
+import org.apache.commons.io.FileUtils;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+
+public class GogsWebHookJenkinsTest {
+    final Logger log = LoggerFactory.getLogger(GogsWebHookJenkinsTest.class);
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+    
+    @Test
+    public void whenJobBranchNotMatchMustReturnError() {
+        Secret secret = Secret.fromString(null);
+        String[][] test_vals = {
+            {null, "master", "true"},
+            {null, "dev", "true"},
+            {"", "master", "true"},
+            {"", "dev", "true"},
+            {"*", "master", "true"},
+            {"*", "dev", "true"},
+            {"dev", "master", "false"},
+            {"dev", "dev", "true"},
+            {"master", "master", "true"},
+            {"master", "dev", "false"},
+        };
+
+        for (int i = 0; i < test_vals.length; ++i) {
+            String filter = test_vals[i][0];
+            String ref = test_vals[i][1];
+            boolean ret = Boolean.parseBoolean(test_vals[i][2]);
+
+            GogsProjectProperty property = new GogsProjectProperty(secret, false, filter);
+            assertSame(String.format("branch filter check failed for [%s -> %s]", ref, filter), ret, property.filterBranch(ref));
+        }
+        
+        log.info("Test succeeded.");
+    }
+
+    @Test
+    @Issue("SECURITY-1438")
+    public void ensureTheSecretIsEncryptedOnDisk() throws Exception {
+        Secret secret = Secret.fromString("s3cr3t");
+        FreeStyleProject p = prepareProjectWithGogsProperty(secret);
+
+        File configFile = p.getConfigFile().getFile();
+        String configFileContent = FileUtils.readFileToString(configFile, StandardCharsets.UTF_8);
+        assertThat(configFileContent, not(containsString(secret.getPlainText())));
+        assertThat(configFileContent, containsString(secret.getEncryptedValue()));
+    }
+
+    @Test
+    @Issue("SECURITY-1438")
+    public void ensureTheSecretIsEncryptedInHtml() throws Exception {
+        Secret secret = Secret.fromString("s3cr3t");
+        FreeStyleProject p = prepareProjectWithGogsProperty(secret);
+
+        JenkinsRule.WebClient wc = j.createWebClient();
+        // there are some errors in the page and thus the status is 500 but the content is there
+        wc.getOptions().setThrowExceptionOnFailingStatusCode(false);
+        HtmlPage htmlPage = wc.goTo(p.getUrl() + "configure");
+        String pageContent = htmlPage.getWebResponse().getContentAsString();
+        assertThat(pageContent, not(containsString(secret.getPlainText())));
+        assertThat(pageContent, containsString(secret.getEncryptedValue()));
+    }
+
+    private FreeStyleProject prepareProjectWithGogsProperty(Secret secret) throws Exception {
+        FreeStyleProject p = j.createFreeStyleProject();
+
+        GogsProjectProperty prop = new GogsProjectProperty(secret, true, "master");
+        p.addProperty(prop);
+
+        p.save();
+
+        return p;
+    }
+}

@@ -1,5 +1,17 @@
 package org.jenkinsci.plugins.gogs;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import javax.servlet.ReadListener;
+import javax.servlet.ServletInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.junit.Before;
@@ -17,19 +29,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.servlet.ReadListener;
-import javax.servlet.ServletInputStream;
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.PrintWriter;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.fail;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
 public class GogsWebHookTest {
     final Logger log = LoggerFactory.getLogger(GogsWebHookTest.class);
 
@@ -233,33 +232,6 @@ public void whenUriDoesNotContainUrlNameMustReturnError() throws Exception {
         log.info("Test succeeded.");
     }
 
-    @Test
-    public void whenJobBranchNotMatchMustReturnError() throws Exception {
-        String[][] test_vals = {
-            {null, "master", "true"},
-            {null, "dev", "true"},
-            {"", "master", "true"},
-            {"", "dev", "true"},
-            {"*", "master", "true"},
-            {"*", "dev", "true"},
-            {"dev", "master", "false"},
-            {"dev", "dev", "true"},
-            {"master", "master", "true"},
-            {"master", "dev", "false"},
-        };
-
-        for (int i = 0; i < test_vals.length; ++i) {
-            String filter = test_vals[i][0];
-            String ref = test_vals[i][1];
-            boolean ret = Boolean.parseBoolean(test_vals[i][2]);
-
-            GogsProjectProperty property = new GogsProjectProperty(null, false, filter);
-            assertSame(String.format("branch filter check failed for [%s -> %s]", ref, filter), ret, property.filterBranch(ref));
-        }
-        
-        log.info("Test succeeded.");
-    }
-
     //
     // Helper methods
     //