Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,17 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
sshConnection = new Connection(agentIp, 22);

sshConnection.connect(null, 60000, 60000);
if (!sshConnection.authenticateWithPassword(username, password)) {
s_logger.debug("Failed to authenticate");
throw new DiscoveredWithErrorException("Authentication error");

final String privateKey = _configDao.getValue("ssh.privatekey");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@weizhouapache I think, it is better to try with ssh key or password (based on the authentication mode), not with other when one of it fails. You can determine authentication mode, either from the user input or may be from password/privatekey config.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@sureshanaparti
isn't it better to support both ? hosts are still manageable if one of the authentication fails.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if one authentication fails, try other may not be good.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

if one authentication fails, try other may not be good.

@sureshanaparti ok. let's ask other guys.

@rhtyd @DaanHoogland what's your opinion ?

if (!SSHCmdHelper.acquireAuthorizedConnectionWithPublicKey(sshConnection, username, privateKey)) {
s_logger.error("Failed to authenticate with ssh key");
if (org.apache.commons.lang3.StringUtils.isEmpty(password)) {
throw new DiscoveredWithErrorException("Authentication error with ssh private key");
}
if (!sshConnection.authenticateWithPassword(username, password)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit - maybe do a check if password was even passed (empty?)

s_logger.error("Failed to authenticate with password");
throw new DiscoveredWithErrorException("Authentication error with host password");
}
}

if (!SSHCmdHelper.sshExecuteCmd(sshConnection, "ls /dev/kvm")) {
Expand Down
33 changes: 20 additions & 13 deletions server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.utils.Pair;
import com.cloud.utils.Ternary;
import com.cloud.utils.StringUtils;
import com.cloud.utils.UriUtils;
import com.cloud.utils.component.Manager;
Expand Down Expand Up @@ -200,7 +200,6 @@
import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
import com.google.common.base.Strings;
import com.google.gson.Gson;

@Component
Expand Down Expand Up @@ -696,9 +695,16 @@ private List<HostVO> discoverHostsFull(final Long dcId, final Long podId, Long c
throw new InvalidParameterValueException("Can't specify cluster without specifying the pod");
}
List<String> skipList = Arrays.asList(HypervisorType.VMware.name().toLowerCase(Locale.ROOT), Type.SecondaryStorage.name().toLowerCase(Locale.ROOT));
if (!skipList.contains(hypervisorType.toLowerCase(Locale.ROOT)) &&
(Strings.isNullOrEmpty(username) || Strings.isNullOrEmpty(password))) {
throw new InvalidParameterValueException("Username and Password need to be provided.");
if (!skipList.contains(hypervisorType.toLowerCase(Locale.ROOT))) {
if (HypervisorType.KVM.toString().equalsIgnoreCase(hypervisorType)) {
if (org.apache.commons.lang3.StringUtils.isBlank(username)) {
throw new InvalidParameterValueException("Username need to be provided.");
}
} else {
if (org.apache.commons.lang3.StringUtils.isBlank(username) || org.apache.commons.lang3.StringUtils.isBlank(password)) {
throw new InvalidParameterValueException("Username and Password need to be provided.");
}
}
}

if (clusterId != null) {
Expand Down Expand Up @@ -2732,8 +2738,8 @@ protected void handleAgentIfNotConnected(HostVO host, boolean vmsMigrating) {
}
final boolean sshToAgent = Boolean.parseBoolean(_configDao.getValue(KvmSshToAgentEnabled.key()));
if (sshToAgent) {
Pair<String, String> credentials = getHostCredentials(host);
connectAndRestartAgentOnHost(host, credentials.first(), credentials.second());
Ternary<String, String, String> credentials = getHostCredentials(host);
connectAndRestartAgentOnHost(host, credentials.first(), credentials.second(), credentials.third());
} else {
throw new CloudRuntimeException("SSH access is disabled, cannot cancel maintenance mode as " +
"host agent is not connected");
Expand All @@ -2744,22 +2750,23 @@ protected void handleAgentIfNotConnected(HostVO host, boolean vmsMigrating) {
* Get host credentials
* @throws CloudRuntimeException if username or password are not found
*/
protected Pair<String, String> getHostCredentials(HostVO host) {
protected Ternary<String, String, String> getHostCredentials(HostVO host) {
_hostDao.loadDetails(host);
final String password = host.getDetail("password");
final String username = host.getDetail("username");
if (password == null || username == null) {
throw new CloudRuntimeException("SSH to agent is enabled, but username/password credentials are not found");
final String privateKey = _configDao.getValue("ssh.privatekey");
if ((password == null && privateKey == null) || username == null) {
throw new CloudRuntimeException("SSH to agent is enabled, but username and password or private key are not found");
}
return new Pair<>(username, password);
return new Ternary<>(username, password, privateKey);
}

/**
* True if agent is restarted via SSH. Assumes kvm.ssh.to.agent = true and host status is not Up
*/
protected void connectAndRestartAgentOnHost(HostVO host, String username, String password) {
protected void connectAndRestartAgentOnHost(HostVO host, String username, String password, String privateKey) {
final com.trilead.ssh2.Connection connection = SSHCmdHelper.acquireAuthorizedConnection(
host.getPrivateIpAddress(), 22, username, password);
host.getPrivateIpAddress(), 22, username, password, privateKey);
if (connection == null) {
throw new CloudRuntimeException(String.format("SSH to agent is enabled, but failed to connect to %s via IP address [%s].", host, host.getPrivateIpAddress()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.storage.StorageManager;
import com.cloud.utils.Pair;
import com.cloud.utils.Ternary;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;
import com.cloud.utils.ssh.SSHCmdHelper;
Expand Down Expand Up @@ -125,6 +125,7 @@ public class ResourceManagerImplTest {
private static long hostId = 1L;
private static final String hostUsername = "user";
private static final String hostPassword = "password";
private static final String hostPrivateKey = "privatekey";
private static final String hostPrivateIp = "192.168.1.10";

private static long vm1Id = 1L;
Expand All @@ -148,6 +149,7 @@ public void setup() throws Exception {
when(hostDao.findById(hostId)).thenReturn(host);
when(host.getDetail("username")).thenReturn(hostUsername);
when(host.getDetail("password")).thenReturn(hostPassword);
when(configurationDao.getValue("ssh.privatekey")).thenReturn(hostPrivateKey);
when(host.getStatus()).thenReturn(Status.Up);
when(host.getPrivateIpAddress()).thenReturn(hostPrivateIp);
when(vm1.getId()).thenReturn(vm1Id);
Expand All @@ -171,7 +173,7 @@ public void setup() throws Exception {

PowerMockito.mockStatic(SSHCmdHelper.class);
BDDMockito.given(SSHCmdHelper.acquireAuthorizedConnection(eq(hostPrivateIp), eq(22),
eq(hostUsername), eq(hostPassword))).willReturn(sshConnection);
eq(hostUsername), eq(hostPassword), eq(hostPrivateKey))).willReturn(sshConnection);
BDDMockito.given(SSHCmdHelper.sshExecuteCmdOneShot(eq(sshConnection),
eq("service cloudstack-agent restart"))).
willReturn(new SSHCmdHelper.SSHCmdResult(0,"",""));
Expand Down Expand Up @@ -292,50 +294,52 @@ public void testConfigureVncAccessForKVMHostFailedMigrations() {
@Test(expected = CloudRuntimeException.class)
public void testGetHostCredentialsMissingParameter() {
when(host.getDetail("password")).thenReturn(null);
when(configurationDao.getValue("ssh.privatekey")).thenReturn(null);
resourceManager.getHostCredentials(host);
}

@Test
public void testGetHostCredentials() {
Pair<String, String> credentials = resourceManager.getHostCredentials(host);
Ternary<String, String, String> credentials = resourceManager.getHostCredentials(host);
Assert.assertNotNull(credentials);
Assert.assertEquals(hostUsername, credentials.first());
Assert.assertEquals(hostPassword, credentials.second());
Assert.assertEquals(hostPrivateKey, credentials.third());
}

@Test(expected = CloudRuntimeException.class)
public void testConnectAndRestartAgentOnHostCannotConnect() {
BDDMockito.given(SSHCmdHelper.acquireAuthorizedConnection(eq(hostPrivateIp), eq(22),
eq(hostUsername), eq(hostPassword))).willReturn(null);
resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword);
eq(hostUsername), eq(hostPassword), eq(hostPrivateKey))).willReturn(null);
resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword, hostPrivateKey);
}

@Test(expected = CloudRuntimeException.class)
public void testConnectAndRestartAgentOnHostCannotRestart() throws Exception {
BDDMockito.given(SSHCmdHelper.sshExecuteCmdOneShot(eq(sshConnection),
eq("service cloudstack-agent restart"))).willThrow(new SshException("exception"));
resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword);
resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword, hostPrivateKey);
}

@Test
public void testConnectAndRestartAgentOnHost() {
resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword);
resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword, hostPrivateKey);
}

@Test
public void testHandleAgentSSHEnabledNotConnectedAgent() {
when(host.getStatus()).thenReturn(Status.Disconnected);
resourceManager.handleAgentIfNotConnected(host, false);
verify(resourceManager).getHostCredentials(eq(host));
verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey));
}

@Test
public void testHandleAgentSSHEnabledConnectedAgent() {
when(host.getStatus()).thenReturn(Status.Up);
resourceManager.handleAgentIfNotConnected(host, false);
verify(resourceManager, never()).getHostCredentials(eq(host));
verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey));
}

@Test(expected = CloudRuntimeException.class)
Expand All @@ -351,14 +355,14 @@ public void testHandleAgentSSHDisabledConnectedAgent() {
when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false");
resourceManager.handleAgentIfNotConnected(host, false);
verify(resourceManager, never()).getHostCredentials(eq(host));
verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey));
}

@Test
public void testHandleAgentVMsMigrating() {
resourceManager.handleAgentIfNotConnected(host, true);
verify(resourceManager, never()).getHostCredentials(eq(host));
verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword));
verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey));
}

private void setupNoPendingMigrationRetries() {
Expand Down
3 changes: 3 additions & 0 deletions ui/public/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@
"label.associatednetworkid": "Associated Network ID",
"label.associatednetworkname": "Network Name",
"label.asyncbackup": "Async Backup",
"label.authentication.method": "Authentication Method",
"label.authentication.sshkey": "System SSH Key",
"label.author.email": "Author e-mail",
"label.author.name": "Author name",
"label.auto.assign.diskoffering.disk.size": "Automatically assign offering matching the disk size",
Expand Down Expand Up @@ -2588,6 +2590,7 @@
"message.add.firewall.rule.processing": "Adding new Firewall rule...",
"message.add.guest.network": "Please confirm that you would like to add a guest network",
"message.add.host": "Please specify the following parameters to add a new host",
"message.add.host.sshkey": "WARNING: In order to add a host with SSH key, you must ensure your hypervisor host has been configured correctly.",
"message.add.ip.range": "Add an IP range to public network in zone",
"message.add.ip.range.direct.network": "Add an IP range to direct network <b><span id=\"directnetwork_name\"></span></b> in zone <b><span id=\"zone_name\"></span></b>",
"message.add.ip.range.to.pod": "<p>Add an IP range to pod: <b><span id=\"pod_name_label\"></span></b></p>",
Expand Down
33 changes: 32 additions & 1 deletion ui/src/views/infra/HostAdd.vue
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,30 @@
<a-input :placeholder="placeholder.username" v-model="username"></a-input>
</div>

<div class="form__item required-field" v-if="selectedClusterHyperVisorType !== 'VMware'">
<div class="form__item" v-if="selectedClusterHyperVisorType !== 'VMware'">
<div class="form__label"><span class="required">* </span>{{ $t('label.authentication.method') }}</div>
<a-radio-group
v-decorator="['authmethod', {
initialValue: authMethod
}]"
buttonStyle="solid"
:defaultValue="authMethod"
@change="selected => { handleAuthMethodChange(selected.target.value) }">
<a-radio-button value="password">
{{ $t('label.password') }}
</a-radio-button>
<a-radio-button value="sshkey" v-if="selectedClusterHyperVisorType === 'KVM'">
{{ $t('label.authentication.sshkey') }}
</a-radio-button>
</a-radio-group>
<span v-if="authMethod === 'sshkey'">
<a-alert type="warning">
<span style="display:block;width:300px;word-wrap:break-word;" slot="message" v-html="$t('message.add.host.sshkey')" />
</a-alert>
</span>
</div>

<div class="form__item required-field" v-if="selectedClusterHyperVisorType !== 'VMware' && authMethod === 'password'">
<div class="form__label"><span class="required">* </span>{{ $t('label.password') }}</div>
<span class="required required-label">{{ $t('label.required') }}</span>
<a-input :placeholder="placeholder.password" type="password" v-model="password"></a-input>
Expand Down Expand Up @@ -190,6 +213,7 @@ export default {
agentusername: null,
agentpassword: null,
agentport: null,
authMethod: 'password',
selectedCluster: null,
selectedClusterHyperVisorType: null,
showDedicated: false,
Expand Down Expand Up @@ -280,6 +304,9 @@ export default {
this.dedicatedAccount = null
this.showDedicated = !this.showDedicated
},
handleAuthMethodChange (val) {
this.authMethod = val
},
handleSubmitForm () {
if (this.loading) return
const requiredFields = document.querySelectorAll('.required-field')
Expand All @@ -306,6 +333,10 @@ export default {
this.url = this.hostname
}

if (this.authMethod !== 'password') {
this.password = ''
}

const args = {
zoneid: this.zoneId,
podid: this.podId,
Expand Down
26 changes: 26 additions & 0 deletions utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.InputStream;

import org.apache.cloudstack.utils.security.KeyStoreUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;

import com.google.common.base.Strings;
Expand Down Expand Up @@ -77,8 +78,33 @@ public static com.trilead.ssh2.Connection acquireAuthorizedConnection(String ip,
}

public static com.trilead.ssh2.Connection acquireAuthorizedConnection(String ip, int port, String username, String password) {
return acquireAuthorizedConnection(ip, 22, username, password, null);
}

public static boolean acquireAuthorizedConnectionWithPublicKey(final com.trilead.ssh2.Connection sshConnection, final String username, final String privateKey) {
if (StringUtils.isNotBlank(privateKey)) {
try {
if (!sshConnection.authenticateWithPublicKey(username, privateKey.toCharArray(), null)) {
s_logger.warn("Failed to authenticate with ssh key");
return false;
}
return true;
} catch (IOException e) {
s_logger.warn("An exception occurred when authenticate with ssh key");
return false;
}
}
return false;
}

public static com.trilead.ssh2.Connection acquireAuthorizedConnection(String ip, int port, String username, String password, String privateKey) {
com.trilead.ssh2.Connection sshConnection = new com.trilead.ssh2.Connection(ip, port);
try {
sshConnection.connect(null, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT);
if (acquireAuthorizedConnectionWithPublicKey(sshConnection, username, privateKey)) {
return sshConnection;
};
sshConnection = new com.trilead.ssh2.Connection(ip, port);
sshConnection.connect(null, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT);
if (!sshConnection.authenticateWithPassword(username, password)) {
String[] methods = sshConnection.getRemainingAuthMethods(username);
Expand Down