CVE-2019-10459

@@ -29,9 +29,8 @@
         <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
         <workflow.version>1.11</workflow.version>
         <hamcrest.version>1.3</hamcrest.version>
-        <powermock.version>1.6.2</powermock.version>
         <jenkins.version>2.0</jenkins.version>
-        <java.level>7</java.level>
+        <java.level>8</java.level>
         <jenkins-test-harness.version>2.13</jenkins-test-harness.version>
     </properties>
 
@@ -157,34 +156,20 @@
             <scope>test</scope>
         </dependency>
         <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <version>1.10.19</version>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.powermock</groupId>
-            <artifactId>powermock-core</artifactId>
-            <version>${powermock.version}</version>
-            <scope>test</scope>
+          <groupId>org.mockito</groupId>
+          <artifactId>mockito-all</artifactId>
+          <version>1.10.19</version>
+          <scope>test</scope>
         </dependency>
         <dependency>
-            <groupId>org.powermock</groupId>
-            <artifactId>powermock-module-junit4</artifactId>
-            <version>${powermock.version}</version>
-            <scope>test</scope>
+          <groupId>org.powermock</groupId>
+          <artifactId>powermock-module-junit4</artifactId>
+          <version>1.6.2</version>
         </dependency>
         <dependency>
-            <groupId>org.powermock</groupId>
-            <artifactId>powermock-api-mockito</artifactId>
-            <version>${powermock.version}</version>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.powermock</groupId>
-            <artifactId>powermock-reflect</artifactId>
-            <version>${powermock.version}</version>
-            <scope>test</scope>
+          <groupId>org.powermock</groupId>
+          <artifactId>powermock-api-mockito</artifactId>
+          <version>1.6.2</version>
         </dependency>
     </dependencies>
 
@@ -205,6 +190,13 @@
                     <compatibleSinceVersion>2.4.0</compatibleSinceVersion>
                 </configuration>
             </plugin>
+            <plugin>
+              <groupId>org.apache.maven.plugins</groupId>
+              <artifactId>maven-surefire-plugin</artifactId>
+              <configuration>
+                <trimStackTrace>false</trimStackTrace>
+              </configuration>
+            </plugin>
         </plugins>
     </build>
 

@@ -20,6 +20,7 @@
 import hudson.tasks.Publisher;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
+import hudson.util.Secret;
 import jenkins.model.Jenkins;
 import jenkins.model.JenkinsLocationConfiguration;
 import net.sf.json.JSONObject;
@@ -39,7 +40,7 @@ public class MattermostNotifier extends Notifier {
 
   private static final Logger logger = Logger.getLogger(MattermostNotifier.class.getName());
 
-  private String endpoint;
+  private Secret endpoint;
   private String buildServerUrl;
   private String room;
   private String icon;
@@ -65,7 +66,7 @@ public DescriptorImpl getDescriptor() {
     return (DescriptorImpl)super.getDescriptor();
   }
 
-  public String getEndpoint() {
+  public Secret getEndpoint() {
     return endpoint;
   }
 
@@ -148,8 +149,8 @@ public String getCustomMessage() {
     return customMessage;
   }
 
-  public void setEndpoint(@CheckForNull String endpoint) {
-    this.endpoint = fixNull(endpoint);
+  public void setEndpoint(String endpoint) {
+    this.endpoint = Secret.fromString(endpoint);
   }
 
   @DataBoundSetter
@@ -243,7 +244,7 @@ public void setCustomMessage(@CheckForNull String customMessage) {
   }
 
   @DataBoundConstructor
-  public MattermostNotifier(final String endpoint, final String room, final String icon, final String buildServerUrl,
+  public MattermostNotifier(final Secret endpoint, final String room, final String icon, final String buildServerUrl,
       final String sendAs, final boolean startNotification, final boolean notifyAborted, final boolean notifyFailure,
       final boolean notifyNotBuilt, final boolean notifySuccess, final boolean notifyUnstable, final boolean notifyBackToNormal,
       final boolean notifyRepeatedFailure, final boolean includeTestSummary, CommitInfoChoice commitInfoChoice,
@@ -284,9 +285,9 @@ public BuildStepMonitor getRequiredMonitorService() {
   }
 
   public MattermostService newMattermostService(AbstractBuild r, BuildListener listener) {
-    String endpoint = this.endpoint;
+    String endpoint = Secret.toString(this.getEndpoint());
     if (StringUtils.isEmpty(endpoint)) {
-      endpoint = getDescriptor().getEndpoint();
+      endpoint = Secret.toString(getDescriptor().getEndpoint());
     }
     String room = this.room;
     if (StringUtils.isEmpty(room)) {
@@ -334,7 +335,7 @@ public boolean prebuild(AbstractBuild<?, ?> build, BuildListener listener) {
   @Extension @Symbol("mattermostNotifier")
   public static class DescriptorImpl extends BuildStepDescriptor<Publisher> {
 
-    private String endpoint;
+    private Secret endpoint;
     private String room;
     private String icon;
     private String buildServerUrl;
@@ -344,12 +345,20 @@ public DescriptorImpl() {
       load();
     }
 
-    @DataBoundSetter
     public void setEndpoint(String endpoint) {
+      if (endpoint == null) {
+        this.endpoint = null;
+        return;
+      }
+      this.setEndpoint(Secret.fromString(endpoint));
+    }
+
+    @DataBoundSetter
+    public void setEndpoint(Secret endpoint) {
       this.endpoint = endpoint;
     }
 
-    public String getEndpoint() {
+    public Secret getEndpoint() {
       return endpoint;
     }
 
@@ -430,7 +439,7 @@ public FormValidation doTestConnection(@QueryParameter("endpoint") final String
         try {
           String targetEndpoint = endpoint;
           if (StringUtils.isEmpty(targetEndpoint)) {
-            targetEndpoint = this.endpoint;
+            targetEndpoint = Secret.toString(this.getEndpoint());
           }
           String targetRoom = room;
           if (StringUtils.isEmpty(targetRoom)) {
@@ -635,8 +644,8 @@ public void onLoaded() {
         } else {
           logger.info(String.format("Starting migration for \"%s\"", p.getName()));
           //map settings
-          if (StringUtils.isBlank(mattermostNotifier.endpoint)) {
-            mattermostNotifier.endpoint = mattermostJobProperty.getEndpoint();
+          if (StringUtils.isBlank(Secret.toString(mattermostNotifier.getEndpoint()))) {
+            mattermostNotifier.setEndpoint(mattermostJobProperty.getEndpoint());
           }
           if (StringUtils.isBlank(mattermostNotifier.icon)) {
             mattermostNotifier.icon = mattermostJobProperty.getIcon();

@@ -3,6 +3,7 @@
 import hudson.AbortException;
 import hudson.Extension;
 import hudson.Util;
+import hudson.util.Secret;
 import hudson.model.TaskListener;
 import jenkins.model.Jenkins;
 import jenkins.plugins.mattermost.*;
@@ -136,7 +137,7 @@ protected Void run() throws Exception {
                 return null;
             }
             MattermostNotifier.DescriptorImpl mattermostDesc = jenkins.getDescriptorByType(MattermostNotifier.DescriptorImpl.class);
-            String team = step.endpoint != null ? step.endpoint : mattermostDesc.getEndpoint();
+            String team = step.getEndpoint() != null ? step.getEndpoint() : Secret.toString(mattermostDesc.getEndpoint());
             String channel = step.channel != null ? step.channel : mattermostDesc.getRoom();
             String icon = step.icon != null ? step.icon : mattermostDesc.getIcon();
             String color = step.color != null ? step.color : "";

@@ -1,13 +1,15 @@
 package jenkins.plugins.mattermost;
 
+import hudson.util.Secret;
+
 public class MattermostNotifierStub extends MattermostNotifier {
 
 	public MattermostNotifierStub(String host, String room, String icon, String buildServerUrl,
 			String sendAs, boolean startNotification, boolean notifyAborted, boolean notifyFailure,
 			boolean notifyNotBuilt, boolean notifySuccess, boolean notifyUnstable, boolean notifyBackToNormal,
 			boolean notifyRepeatedFailure, boolean includeTestSummary, CommitInfoChoice commitInfoChoice,
 			boolean includeCustomAttachmentMessage, String customAttachmentMessage,boolean includeCustomMessage,String customMessage) {
-		super(host, room, icon, buildServerUrl, sendAs, startNotification, notifyAborted, notifyFailure,
+    super(host != null ? Secret.fromString(host) : null, room, icon, buildServerUrl, sendAs, startNotification, notifyAborted, notifyFailure,
 				notifyNotBuilt, notifySuccess, notifyUnstable, notifyBackToNormal, notifyRepeatedFailure,
 				includeTestSummary, commitInfoChoice, includeCustomAttachmentMessage, customAttachmentMessage, includeCustomMessage, customMessage);
 	}

@@ -1,16 +1,20 @@
 package jenkins.plugins.mattermost.workflow;
 
 import hudson.model.TaskListener;
+import hudson.util.Secret;
 import jenkins.model.Jenkins;
 import jenkins.plugins.mattermost.MattermostNotifier;
 import jenkins.plugins.mattermost.MattermostService;
+import jenkins.security.ConfidentialStore;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Matchers;
 import org.mockito.Mock;
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 import java.io.PrintStream;
@@ -25,7 +29,8 @@
  * Traditional Unit tests, allows testing null Jenkins,getInstance()
  */
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({Jenkins.class, MattermostSendStep.class})
+@PrepareForTest({Jenkins.class, ConfidentialStore.class, MattermostSendStep.class})
+@PowerMockIgnore({"javax.crypto.*" }) // https://github.com/powermock/powermock/issues/294
 public class MattermostSendStepTest {
 
     @Mock
@@ -88,7 +93,13 @@ public void testValuesForGlobalConfig() throws Exception {
 
         stepExecution.listener = taskListenerMock;
 
-        when(mattermostDescMock.getEndpoint()).thenReturn("globalEndpoint");
+        PowerMockito.mockStatic(ConfidentialStore.class);
+        ConfidentialStore csMock = mock(ConfidentialStore.class);
+        when(ConfidentialStore.get()).thenReturn(csMock);
+        when(csMock.randomBytes(Matchers.anyInt())).thenAnswer( it -> new byte[ (Integer)(it.getArguments()[0])] );
+
+        Secret encryptedEndpoint = Secret.fromString("globalEndpoint");
+        when(mattermostDescMock.getEndpoint()).thenReturn(encryptedEndpoint);
         when(mattermostDescMock.getIcon()).thenReturn("globalIcon");
         when(mattermostDescMock.getRoom()).thenReturn("globalChannel");
 
@@ -128,7 +139,7 @@ public void testNonNullEmptyColor() throws Exception {
         verify(mattermostServiceMock, times(1)).publish("message", "", "");
         assertNull(stepExecution.step.getColor());
     }
-    
+
     @Test
     public void testNonNullPretext() throws Exception {