Merge pull request #55 from jenkinsci/SECURITY-1438

sanderv32 · web-flow · commit 34de11fe0822 · 2019-07-07T01:48:40.000+02:00
[SECURITY-1438] Plaintext password of Gogs is stored in job config.xml
diff --git a/pom.xml b/pom.xml
@@ -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>
diff --git a/src/main/java/org/jenkinsci/plugins/gogs/GogsProjectProperty.java b/src/main/java/org/jenkinsci/plugins/gogs/GogsProjectProperty.java
@@ -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;
diff --git a/src/main/java/org/jenkinsci/plugins/gogs/GogsWebHook.java b/src/main/java/org/jenkinsci/plugins/gogs/GogsWebHook.java
@@ -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);
                         }
                     }
diff --git a/src/test/docker/gitserver-dockerfile/Dockerfile b/src/test/docker/gitserver-dockerfile/Dockerfile
@@ -1,4 +1,4 @@
-FROM gogs/gogs:0.11.4
+FROM gogs/gogs:0.11.86
 
 ARG FIRST_USER=dev
 
diff --git a/src/test/docker/gitserver-dockerfile/setup-gogs.sh b/src/test/docker/gitserver-dockerfile/setup-gogs.sh
@@ -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
diff --git a/src/test/docker/jenkins-dockerfile/Dockerfile b/src/test/docker/jenkins-dockerfile/Dockerfile
@@ -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
diff --git a/src/test/java/org/jenkinsci/plugins/gogs/GogsConfigHandler.java b/src/test/java/org/jenkinsci/plugins/gogs/GogsConfigHandler.java
@@ -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;
-    }
-
 }
diff --git a/src/test/java/org/jenkinsci/plugins/gogs/GogsWebHookJenkinsTest.java b/src/test/java/org/jenkinsci/plugins/gogs/GogsWebHookJenkinsTest.java
diff --git a/src/test/java/org/jenkinsci/plugins/gogs/GogsWebHookTest.java b/src/test/java/org/jenkinsci/plugins/gogs/GogsWebHookTest.java

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-FROM gogs/gogs:0.11.4`
	`1`	`+FROM gogs/gogs:0.11.86`
`2`	`2`
`3`	`3`	`ARG FIRST_USER=dev`
`4`	`4`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-FROM jenkinsci/jenkins:latest`
	`1`	`+FROM jenkins/jenkins:lts-alpine`
`2`	`2`
`3`	`3`	`# Load the required dependencies for the plugin under test`
`4`	`4`	`RUN /usr/local/bin/install-plugins.sh git workflow-aggregator pipeline-model-extensions cloudbees-folder`