SECURITY-1408, SECURITY-381, SECURITY-1528

    Co-Authored-By: Jeff Thompson <37345299+jeffret-b@users.noreply.github.com>
    Co-Authored-By: Matt Sicker <boards@gmail.com>
    Co-Authored-By: rsandell <rsandell@cloudbees.com>

.gitignore

@@ -11,3 +11,5 @@ target
 /bin
 /work
 /src_generated
+.history
+.factorypath

README.md

@@ -4,6 +4,31 @@
 [![GitHub release](https://img.shields.io/github/release/jenkinsci/ec2-plugin.svg?label=changelog)](https://github.com/jenkinsci/ec2-plugin/releases/latest)
 [![Gitter](https://badges.gitter.im/ec2-plugin/Lobby.svg)](https://gitter.im/ec2-plugin/Lobby?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
 
+# Table of contents
+   * [Introduction](#introduction)
+   * [Usage](#usage)
+      * [Spot Instances](#spot-instances)
+         * [Enable Spot Request](#enable-spot-request)
+         * [Configure Jenkins for Spot Support](#configure-jenkins-for-spot-support)
+         * [Configure AMI for Spot Support](#configure-ami-for-spot-support)
+      * [IAM setup](#iam-setup)
+   * [Configure plugin via Groovy script](#configure-plugin-via-groovy-script)
+   * [Security](#security)
+      * [Securing the connection to Unix AMIs](#securing-the-connection-to-unix-amis)
+         * [Strategies](#strategies)
+            * [Check New Hard](#check-new-hard)
+            * [Check New Soft](#check-new-soft)
+            * [Accept New](#accept-new)
+            * [Off](#off)
+         * [New AMIs](#new-amis)
+         * [Upgrade - Existing AMIs](#upgrade---existing-amis)
+      * [Securing the connection to Windows AMIs](#securing-the-connection-to-windows-amis)
+         * [AMI Set Up](#ami-set-up)
+   * [Known Issues](#known-issues)
+      * [Authentication Timeout](#authentication-timeout)
+      * [Amazon Linux build/connectivity issues](#amazon-linux-buildconnectivity-issues)
+   * [Change Log](#change-log)
+
 # Introduction
 
 Allow Jenkins to start agents on
@@ -419,6 +444,131 @@ jenkins.clouds.add(amazonEC2Cloud)
 jenkins.save()
 ```
 
+# Security
+## Securing the connection to Unix AMIs
+When you set up a template for a *Unix* instance (`Type AMI` field), you can select the strategy used to guarantee the
+instance you're connecting to is the expected one. You should use a strong strategy to guarantee that a 
+_[man-in-the-middle attack](https://en.wikipedia.org/wiki/Man-in-the-middle_attack)_ cannot be performed.
+
+You can select your strategy under the _Advanced..._ configuration, on the _Host Key Verification Strategy_ field of 
+every configured AMI. 
+
+The plugin provides several strategies because each one has its own requirements. So providing more than one allows
+ administrators to use the one best fits to their environment. These strategies are:
+
+### Strategies
+#### Check New Hard
+This strategy checks the SSH host key provided by the instance with the key printed out in the instance console during
+the instance initialization. If the key is not found, the plugin **doesn't allow** the connection to the instance to 
+guarantee the instance is the right one. If the key is found and it is the same as the one presented by the instance,
+then it's saved to be used on future connections, so the console is only checked once.
+
+Requirements:
+
+* The AMI used should print the key used. It's a common behaviour, for example the _Amazon Linux 2_ AMI prints it 
+out. You can consult the AMI documentation to figure it out.
+* The launch timeout should be long enough to allow the plugin to check the instance console. With this strategy, the
+plugin waits for the console to be available, which can take a few minutes. The _Launch Timeout in seconds_ field should
+have a number to allow that, for example 600 (10 minutes). By default there is no timeout, so it's safe.
+
+The expected format on the instance console is `algorithm base64-public-key` at the beginning of a line. For example:
+```
+ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNFNGfKpPS/UT2jAEa0+9aZneku2a7TVwN+MjGesm65DDGnXPcM9TM9BsiOE+s4Vo6aCT9L/TVrtDFa0hqbnqc8= 
+ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHm0sVqkjSuaPg8e7zfaKXt3b1hE1tBwFsB18NOWv5ow 
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDNTngsAxOCpZwt+IBqJSQ9MU2qVNYzP4D5i1OHfIRXCrnAuJ54GtFzZEZqqo4e1e/JqBQOX3ZPsaegbkzl2uq5FzfFcFoYYXg5gL7htlZ1I2k6/2iIBv7CHAjbpXMkH8WoF2C3vZFRMWLs20ikQpED+9m11VejE19+kqJwLMopyAtq+/mCgiv4nw5QWh3rrrEcbgzuxYoMD0t9daqBq1V0lzRqL36ALVySy7oDjr3YzCN+wMXe1I36kv3lSeCHXnhc53ubrBIsRakWLBndHhPqyyAOMEjdby/O/EQ2PR7vBpH5MaseaJwvRRDPQ6qt4sV8lk0tEt9qbdb1prFRB4W1 
+```
+Recommended for:
+
+This strategy is the most secure. It's recommended for every instance if you can meet the requirements. We recommend,
+whenever possible, configuring each AMI with _Stop/Disconnect on Idle Timeout_ to take advantage of the ssh host key
+cache allowing next connections to be done faster. 
+
+#### Check New Soft
+This strategy checks the SSH host key provided by the instance with the key printed out in the instance console during
+the instance initialization. If the key is not found, the plugin **allows** the connection to the instance in order to 
+guarantee the instance is the right one. If the key is found and it is the same as the one presented by the instance,
+then it's saved to be used on future connections, so the console is only checked once.
+
+Requirements:
+
+* The AMI used may print the key used to guarantee the instance is the right one, but **it's not mandatory**.
+* The launch timeout should be long enough to allow the plugin to check the instance console. With this strategy, the
+plugin waits for the console to be available, which can take a few minutes. The _Launch Timeout in seconds_ field should
+have a number to allow that. For example 600 (10 minutes). By default there is no timeout, so it's safe. If the timeout
+expires, the connection is not done.
+
+The expected format on the instance console is `algorithm base64-public-key` at the beginning of a line. For example:
+```
+ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNFNGfKpPS/UT2jAEa0+9aZneku2a7TVwN+MjGesm65DDGnXPcM9TM9BsiOE+s4Vo6aCT9L/TVrtDFa0hqbnqc8= 
+ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHm0sVqkjSuaPg8e7zfaKXt3b1hE1tBwFsB18NOWv5ow 
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDNTngsAxOCpZwt+IBqJSQ9MU2qVNYzP4D5i1OHfIRXCrnAuJ54GtFzZEZqqo4e1e/JqBQOX3ZPsaegbkzl2uq5FzfFcFoYYXg5gL7htlZ1I2k6/2iIBv7CHAjbpXMkH8WoF2C3vZFRMWLs20ikQpED+9m11VejE19+kqJwLMopyAtq+/mCgiv4nw5QWh3rrrEcbgzuxYoMD0t9daqBq1V0lzRqL36ALVySy7oDjr3YzCN+wMXe1I36kv3lSeCHXnhc53ubrBIsRakWLBndHhPqyyAOMEjdby/O/EQ2PR7vBpH5MaseaJwvRRDPQ6qt4sV8lk0tEt9qbdb1prFRB4W1 
+```
+Recommended for:
+
+This strategy is the default one for AMIs created with a former version of the plugin. It doesn't break any connection
+because the plugin connects to the instance even when the key is not found on the console. The only point to take into
+ account is you need to have the right timeout to allow the plugin to get the instance console. This strategy is recommended
+when upgrading from a previous version of the plugin. _Check New Hard_ is the safest strategy, so you should
+consider migrating to it. We recommend, whenever possible, configuring each AMI with _Stop/Disconnect on Idle Timeout_
+ to take advantage of the ssh host key cache allowing next connections to be done faster.
+
+#### Accept New
+This strategy doesn't check any key on the console. It accepts the key provided by the instance on the first
+connection. Then, the key is saved to be used on future connections to detect a Man-in-the-Middle attack (the host
+key has changed).
+
+Requirements:
+* N/A
+
+Recommended for:
+
+This strategy is recommended when your AMIs don't print out the host keys on the console. The _Check New Soft_ cannot be
+ used, but at least, you can catch a man-in-the-middle attack on further connections to the same instance. If the attack
+ was already perpetrated you cannot detect that. Again, the _Check New Hard_ is the safest strategy.
+
+#### Off
+This strategy neither checks any key on the console, nor checks future connections to the same instance with a saved
+key. It accepts blindly the key provided by the instance on the first and further connections.
+
+Requirements:
+* N/A
+
+Recommended for:
+
+This strategy is not recommended because of its lack of security. It is the strategy used for prior versions of the plugin.
+
+### New AMIs
+The default strategy for every new instance is the _Check New Hard_ one. You can select a strategy per AMI. It's under 
+the _Advanced..._ configuration, on the _Host Key Verification Strategy_ field. 
+
+### Upgrade - Existing AMIs
+You may upgrade from a Jenkins installation with a former plugin version without this security mechanism. The default
+ strategy for every existing instance is the _Check New Soft_ strategy. This guarantees your jobs are not going to stop
+ working and improves the situation. We recommend, if possible, upgrading to the _Check New Hard_ strategy to be safer
+ against a _Man in the Middle attack_. 
+
+## Securing the connection to Windows AMIs
+When you configure a template for a *Windows* instance (`Type AMI` field), you can use HTTPS and disallow 
+self-signed certificates. This guarantees the instance you're connecting to is the expected one and a 
+[man-in-the-middle attack](https://en.wikipedia.org/wiki/Man-in-the-middle_attack) cannot be performed.
+
+### AMI Set Up
+Before securely connecting to the instance, you need to 1) configure the AMI, 2)install the 
+certificate, 3) configure WinRM properly and 4) set the firewall rules to allow the connection. You can find some 
+guidance at the `AMI Type` field help, under the template configuration on your Jenkins instance.
+
+Tips:
+* When the `Allow Self Signed Certificate` field is checked, the plugin checks the CA which issued the 
+certificate and verifies the host it is connecting to is present on the certificate. If the field is not checked, both checks are skipped.
+* The EC2 plugin connects to the instance using either an IP address. It does not use the DNS name. You must configure WinRM with a certificate which includes 
+the **IP** of the instance. Something like: 
+```
+#3: ObjectId: 2.5.29.17 Criticality=false
+SubjectAlternativeName [
+  DNSName: myhostname.com
+  IPAddress: 111.222.333.444  <--------------
+]
+```
 # Known Issues
 
 ## Authentication Timeout

pom.xml

@@ -246,6 +246,12 @@ THE SOFTWARE.
             <version>${jcasc.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.testcontainers</groupId>
+            <artifactId>testcontainers</artifactId>
+            <version>1.8.3</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <dependencyManagement>
@@ -420,6 +426,7 @@ THE SOFTWARE.
                 <configuration>
                     <compatibleSinceVersion>1.45</compatibleSinceVersion>
                     <pluginFirstClassLoader>true</pluginFirstClassLoader>
+                    <minimumJavaVersion>8</minimumJavaVersion>
                 </configuration>
             </plugin>
             <plugin>

src/main/java/hudson/plugins/ec2/AmazonEC2Cloud.java

@@ -162,12 +162,14 @@ public FormValidation doCheckCloudName(@QueryParameter String value) {
             return FormValidation.ok();
         }
 
+        @RequirePOST
         public ListBoxModel doFillRegionItems(
                 @QueryParameter String altEC2Endpoint,
                 @QueryParameter boolean useInstanceProfileForCredentials,
                 @QueryParameter String credentialsId)
 
                 throws IOException, ServletException {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
 
             ListBoxModel model = new ListBoxModel();
 

src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java

@@ -735,6 +735,10 @@ public int getBootDelay() {
     public boolean isSpecifyPassword() {
         return amiType.isWindows() && ((WindowsData) amiType).isSpecifyPassword();
     }
+
+    public boolean isAllowSelfSignedCertificate() {
+        return amiType.isWindows() && ((WindowsData) amiType).isAllowSelfSignedCertificate();
+    }
 
     public static ListBoxModel fillZoneItems(AWSCredentialsProvider credentialsProvider, String region) {
         ListBoxModel model = new ListBoxModel();

src/main/java/hudson/plugins/ec2/EC2Cloud.java

@@ -116,6 +116,7 @@
 import hudson.util.HttpResponses;
 import hudson.util.Secret;
 import hudson.util.StreamTaskListener;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 /**
  * Hudson's view of EC2.
@@ -325,6 +326,7 @@ public synchronized KeyPair getKeyPair() throws AmazonClientException, IOExcepti
     /**
      * Debug command to attach to a running instance.
      */
+    @RequirePOST
     public void doAttach(StaplerRequest req, StaplerResponse rsp, @QueryParameter String id)
             throws ServletException, IOException, AmazonClientException {
         checkPermission(PROVISION);
@@ -338,6 +340,7 @@ public void doAttach(StaplerRequest req, StaplerResponse rsp, @QueryParameter St
         rsp.sendRedirect2(req.getContextPath() + "/computer/" + node.getNodeName());
     }
 
+    @RequirePOST
     public HttpResponse doProvision(@QueryParameter String template) throws ServletException, IOException {
         checkPermission(PROVISION);
         if (template == null) {
@@ -959,6 +962,21 @@ public FormValidation doCheckPrivateKey(@QueryParameter String value) throws IOE
             return FormValidation.ok();
         }
 
+        /**
+         * Tests the connection settings.
+         *
+         * Overriding needs to {@code @RequirePOST}
+         * @param ec2endpoint
+         * @param useInstanceProfileForCredentials
+         * @param credentialsId
+         * @param privateKey
+         * @param roleArn
+         * @param roleSessionName
+         * @param region
+         * @return the validation result
+         * @throws IOException
+         * @throws ServletException
+         */
         protected FormValidation doTestConnection(URL ec2endpoint, boolean useInstanceProfileForCredentials, String credentialsId, String privateKey, String roleArn, String roleSessionName, String region)
                 throws IOException, ServletException {
             Jenkins.get().checkPermission(Jenkins.ADMINISTER);
@@ -986,7 +1004,9 @@ protected FormValidation doTestConnection(URL ec2endpoint, boolean useInstancePr
             }
         }
 
+        @RequirePOST
         public ListBoxModel doFillCredentialsIdItems() {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
             return new StandardListBoxModel()
                     .withEmptySelection()
                     .withMatching(

src/main/java/hudson/plugins/ec2/EC2Computer.java

@@ -97,6 +97,16 @@ public String getConsoleOutput() throws AmazonClientException {
         return ec2.getConsoleOutput(request).getOutput();
     }
 
+    /**
+     * Gets the EC2 decoded console output.
+     * @since TODO
+     */
+    public String getDecodedConsoleOutput() throws AmazonClientException {
+        AmazonEC2 ec2 = getCloud().connect();
+        GetConsoleOutputRequest request = new GetConsoleOutputRequest(getInstanceId());
+        return ec2.getConsoleOutput(request).getDecodedOutput();
+    }
+
     /**
      * Obtains the instance state description in EC2.
      *

src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java

@@ -174,7 +174,7 @@ private long internalCheck(EC2Computer computer) {
                     return 1;
                 }
                 long launchTimeout = node.getLaunchTimeoutInMillis();
-                if(uptime > launchTimeout){
+                if (launchTimeout > 0 && uptime > launchTimeout){
                     // Computer is offline and startup time has expired
                     LOGGER.info("Startup timeout of " + computer.getName() + " after "
                     + uptime +

src/main/java/hudson/plugins/ec2/HostKeyVerificationStrategyEnum.java

@@ -0,0 +1,67 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2020-, M Ramon Leon, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package hudson.plugins.ec2;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.plugins.ec2.ssh.verifiers.AcceptNewStrategy;
+import hudson.plugins.ec2.ssh.verifiers.CheckNewHardStrategy;
+import hudson.plugins.ec2.ssh.verifiers.CheckNewSoftStrategy;
+import hudson.plugins.ec2.ssh.verifiers.NonVerifyingKeyVerificationStrategy;
+import hudson.plugins.ec2.ssh.verifiers.SshHostKeyVerificationStrategy;
+
+public enum HostKeyVerificationStrategyEnum {
+    CHECK_NEW_HARD("check-new-hard", "yes", new CheckNewHardStrategy()),
+    CHECK_NEW_SOFT("check-new-soft", "accept-new", new CheckNewSoftStrategy()),
+    ACCEPT_NEW("accept-new", "accept-new", new AcceptNewStrategy()),
+    OFF("off", "off", new NonVerifyingKeyVerificationStrategy());
+
+    private final String displayText;
+    private final SshHostKeyVerificationStrategy strategy;
+    private final String sshCommandEquivalentFlag;
+
+    HostKeyVerificationStrategyEnum(@NonNull String displayText, @NonNull String sshCommandEquivalentFlag, @NonNull SshHostKeyVerificationStrategy strategy) {
+        this.displayText = displayText;
+        this.sshCommandEquivalentFlag = sshCommandEquivalentFlag;
+        this.strategy = strategy;
+    }
+
+    @NonNull
+    public SshHostKeyVerificationStrategy getStrategy() {
+        return strategy;
+    }
+
+    public boolean equalsDisplayText(String other) {
+        return this.displayText.equals(other);
+    }
+
+    @NonNull
+    public String getDisplayText() {
+        return displayText;
+    }
+
+    @NonNull
+    public String getSshCommandEquivalentFlag() {
+        return sshCommandEquivalentFlag;
+    }
+}