Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add host-key verification strategy check-static #483

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* [Strategies](#strategies)
* [Check New Hard](#check-new-hard)
* [Check New Soft](#check-new-soft)
* [Check Static](#check-static)
* [Accept New](#accept-new)
* [Off](#off)
* [New AMIs](#new-amis)
Expand Down Expand Up @@ -515,6 +516,18 @@ when upgrading from a previous version of the plugin. _Check New Hard_ is the sa
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.

#### Check Static

This strategy checks the SSH host key provided by the `Static Host Keys` field in the slave template.
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.

The expected format of the `Static Host Keys` field is `algorithm base64-public-key`. For example:
```
ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHm0sVqkjSuaPg8e7zfaKXt3b1hE1tBwFsB18NOWv5ow
```

#### 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@
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.CheckStaticStrategy;
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()),
CHECK_STATIC("check-static", "yes", new CheckStaticStrategy()),
ACCEPT_NEW("accept-new", "accept-new", new AcceptNewStrategy()),
OFF("off", "no", new NonVerifyingKeyVerificationStrategy());

Expand Down
168 changes: 68 additions & 100 deletions src/main/java/hudson/plugins/ec2/SlaveTemplate.java

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion src/main/java/hudson/plugins/ec2/ssh/EC2UnixLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,6 @@ private Connection connectToSsh(EC2Computer computer, TaskListener listener, Sla
conn.setProxyData(proxyData);
logInfo(computer, listener, "Using HTTP Proxy Configuration");
}

conn.connect(new ServerHostKeyVerifierImpl(computer, listener), slaveConnectTimeout, slaveConnectTimeout);
logInfo(computer, listener, "Connected via SSH.");
return conn; // successfully connected
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* 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.ssh.verifiers;

import hudson.model.TaskListener;
import hudson.plugins.ec2.EC2Cloud;
import hudson.plugins.ec2.EC2Computer;
import hudson.plugins.ec2.SlaveTemplate;
import hudson.slaves.OfflineCause;

import java.util.ArrayList;
import java.util.Base64;

import java.io.IOException;
import java.util.Objects;
import java.util.Scanner;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* This strategy checks the key presented by the host with the key in the Static Host Keys` field in the slave template.
* If the key is not found, the connection is not trusted. If it's found, the key is stored in the ssh-host-key.xml file
* of the node directory and checked on every further connection.
* @author Christian Groschupp
* @since TODO
*/
public class CheckStaticStrategy extends SshHostKeyVerificationStrategy {
private static final Logger LOGGER = Logger.getLogger(CheckStaticStrategy.class.getName());

private ArrayList<HostKey> getStaticHostKeys(EC2Computer computer) {
ArrayList<HostKey> hostKeys;
hostKeys = new ArrayList<>();

SlaveTemplate computerSlaveTemplate = computer.getSlaveTemplate();
if (computerSlaveTemplate == null) {
EC2Cloud.log(LOGGER, Level.WARNING, computer.getListener(), "No compute slave template found, return empty hostKeys list");
return hostKeys;
}

Scanner scanner = new Scanner(computerSlaveTemplate.getStaticHostKeys());

while (scanner.hasNextLine()) {
String hostKeyString = scanner.nextLine();
String[] hostKeyParts = hostKeyString.split(" ");
if (hostKeyParts.length < 2 || hostKeyParts.length > 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (hostKeyParts.length < 2 || hostKeyParts.length > 3) {
if (hostKeyParts.length != 2) {

EC2Cloud.log(LOGGER, Level.WARNING, computer.getListener(), "The provided static SSH key is invalid");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add the reason it would be appreciated, like:

the format is algorithm base64-public-key, example: ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHm0sVqkjSuaPg8e7zfaKXt3b1hE1tBwFsB18NOWv5ow

continue;
}
HostKey hostKey = new HostKey(hostKeyParts[0], Base64.getDecoder().decode(hostKeyParts[1]));
hostKeys.add(hostKey);
}
scanner.close();
return hostKeys;
}

@Override
public boolean verify(EC2Computer computer, HostKey hostKey, TaskListener listener) throws IOException {
HostKey existingHostKey = HostKeyHelper.getInstance().getHostKey(computer);
ArrayList<HostKey> staticHostKeys = getStaticHostKeys(computer);

if (staticHostKeys.size() < 1) {
EC2Cloud.log(LOGGER, Level.WARNING, computer.getListener(), "No configured static SSH key or none of the statically configured SSH keys are valid");
// To avoid reconnecting continuously
computer.setTemporarilyOffline(true, OfflineCause.create(Messages._OfflineCause_SSHKeyCheckFailed()));
return false;
}

if (null == existingHostKey) {
for (HostKey staticHostKey : staticHostKeys) {
if (hostKey.equals(staticHostKey)) {
HostKeyHelper.getInstance().saveHostKey(computer, hostKey);
EC2Cloud.log(LOGGER, Level.INFO, computer.getListener(), String.format("The SSH key %s %s has been successfully checked against the instance console for connections to %s", hostKey.getAlgorithm(), hostKey.getFingerprint(), computer.getName()));
return true;
}
}
// To avoid reconnecting continuously
computer.setTemporarilyOffline(true, OfflineCause.create(Messages._OfflineCause_SSHKeyCheckFailed()));
return false;

} else if (existingHostKey.equals(hostKey)) {
EC2Cloud.log(LOGGER, Level.INFO, computer.getListener(), "Connection allowed after the host key has been verified");
return true;
} else {
EC2Cloud.log(LOGGER, Level.WARNING, computer.getListener(), String.format("The SSH key (%s) presented by the instance has changed since first saved (%s). The connection to %s is closed to prevent a possible man-in-the-middle attack", hostKey.getFingerprint(), existingHostKey.getFingerprint(), computer.getName()));
// To avoid reconnecting continuously
computer.setTemporarilyOffline(true, OfflineCause.create(Messages._OfflineCause_SSHKeyCheckFailed()));
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ THE SOFTWARE.
<f:entry title="${%Host Key Verification Strategy}" field="hostKeyVerificationStrategy">
<f:select default="${descriptor.defaultHostKeyVerificationStrategy}"/>
</f:entry>

<f:entry title="${%Static Host Keys}" field="staticHostKeys">
<f:textarea />
</f:entry>

<f:entry title="${%Maximum Total Uses}" field="maxTotalUses">
<f:textbox default="-1"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<ul>
<li><b><i>check-new-hard</i></b>: Check the key presented by the instance against the instance console and stores it to check subsequent connections. If the key is not printed on the console, the connection is not trusted. This is the default behavior for new AMIs.</li>
<li><b><i>check-new-soft</i></b>: Check the key against the instance console and stores it to check subsequent connections. If the key is not printed on the console, the connection is trusted anyway. This is the default behavior for existing AMIs (upgrading from a previous plugin version). This avoids future attacks but cannot guarantee the instance is the right one if a man-in-the-middle attack has already been committed.</i></b></li>
<li><b><i>check-static</i></b>: Check the key against the static configured SSH keys in the cloud template.</i></b></li>
<li><b><i>accept-new</i></b>: Accept the key on first connection and stores it to check subsequent connections. This doesn't try to check the key against the console as the <i>check-new-soft</i> strategy does.</li>
<li><b><i>off</i></b>: Don't check the host key on any connection.</li>
</ul>
Expand All @@ -34,6 +35,7 @@
<ul>
<li><b><i>check-new-hard = yes</i></b></li>
<li><b><i>check-new-soft = accept-new</i></b></li>
<li><b><i>check-static = yes</i></b></li>
<li><b><i>accept-new = accept-new</i></b></li>
<li><b><i>off = no</i></b></li>
</ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div>
List of static ssh host keys, seperated by new line. Only used when Host Key Verification Strategy is check-static.
</div>
2 changes: 1 addition & 1 deletion src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public String getEc2Type() {
@Test
public void testMaxUsesBackwardCompat() throws Exception {
final String description = "description";
SlaveTemplate orig = new SlaveTemplate("ami-123", EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, description, "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet 456", null, null, null, "", true, false, false, "", false, "", false, false, false, ConnectionStrategy.PUBLIC_IP, -1);
SlaveTemplate orig = new SlaveTemplate("ami-123", EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, description, "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet 456", null, null, null, "", true, false, false, "", false, "", false, false, false, ConnectionStrategy.PUBLIC_IP, -1, "");
List<SlaveTemplate> templates = new ArrayList<>();
templates.add(orig);
AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "us-east-1", "ghi", "3", templates, null, null);
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public InstanceState getState() {

@Override
public SlaveTemplate getSlaveTemplate() {
return new SlaveTemplate("ami-123", EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "AMI description", "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet-123 subnet-456", null, null, true, null, "", false, false, "", false, "");
return new SlaveTemplate("ami-123", EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "AMI description", "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet-123 subnet-456", null, null, true, null, "", false, false, "", false, "", "");
}
};
assertTrue(computer.isIdle());
Expand Down Expand Up @@ -209,7 +209,7 @@ public void testRetentionDespiteIdleWithMinimumInstances() throws Exception {
SlaveTemplate template = new SlaveTemplate("ami1", EC2AbstractSlave.TEST_ZONE, null, "default", "foo",
InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null,
"-Xmx1g", false, "subnet 456", null, null, 2, 0, "10", null, true, true, false, "", false, "", false, false,
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList());
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList(), "");
AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", PrivateKeyHelper.generate(), "3",
Collections
.singletonList(template), "roleArn", "roleSessionName");
Expand Down Expand Up @@ -268,7 +268,7 @@ public void testRetentionDespiteIdleWithMinimumInstanceActiveTimeRange() throws
SlaveTemplate template = new SlaveTemplate("ami1", EC2AbstractSlave.TEST_ZONE, null, "default", "foo",
InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null,
"-Xmx1g", false, "subnet 456", null, null, 2, 0, "10", null, true, true, false, "", false, "", false, false,
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList());
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList(), "");

MinimumNumberOfInstancesTimeRangeConfig minimumNumberOfInstancesTimeRangeConfig = new MinimumNumberOfInstancesTimeRangeConfig();
minimumNumberOfInstancesTimeRangeConfig.setMinimumNoInstancesActiveTimeRangeFrom("11:00");
Expand Down Expand Up @@ -324,7 +324,7 @@ public void testRetentionIdleWithMinimumInstanceInactiveTimeRange() throws Excep
SlaveTemplate template = new SlaveTemplate("ami1", EC2AbstractSlave.TEST_ZONE, null, "default", "foo",
InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null,
"-Xmx1g", false, "subnet 456", null, null, 2, 0, "10", null, true, true, false, "", false, "", false, false,
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList());
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList(), "");

MinimumNumberOfInstancesTimeRangeConfig minimumNumberOfInstancesTimeRangeConfig = new MinimumNumberOfInstancesTimeRangeConfig();
minimumNumberOfInstancesTimeRangeConfig.setMinimumNoInstancesActiveTimeRangeFrom("11:00");
Expand Down Expand Up @@ -365,7 +365,7 @@ public void testRetentionDespiteIdleWithMinimumInstanceActiveTimeRangeAfterMidni
SlaveTemplate template = new SlaveTemplate("ami1", EC2AbstractSlave.TEST_ZONE, null, "default", "foo",
InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null,
"-Xmx1g", false, "subnet 456", null, null, 2, 0, "10", null, true, true, false, "", false, "", false, false,
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList());
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList(), "");

MinimumNumberOfInstancesTimeRangeConfig minimumNumberOfInstancesTimeRangeConfig = new MinimumNumberOfInstancesTimeRangeConfig();
minimumNumberOfInstancesTimeRangeConfig.setMinimumNoInstancesActiveTimeRangeFrom("15:00");
Expand Down Expand Up @@ -420,7 +420,7 @@ public void testRetentionStopsAfterActiveRangeEnds() throws Exception {
SlaveTemplate template = new SlaveTemplate("ami1", EC2AbstractSlave.TEST_ZONE, null, "default", "foo",
InstanceType.M1Large, false, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "bbb", "aaa", "10", "fff", null,
"-Xmx1g", false, "subnet 456", null, null, 2, 0, "10", null, true, true, false, "", false, "", false, false,
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList());
true, ConnectionStrategy.PRIVATE_IP, 0, Collections.emptyList(), "");

MinimumNumberOfInstancesTimeRangeConfig minimumNumberOfInstancesTimeRangeConfig = new MinimumNumberOfInstancesTimeRangeConfig();
minimumNumberOfInstancesTimeRangeConfig.setMinimumNoInstancesActiveTimeRangeFrom("11:00");
Expand Down
Loading