Merge pull request #841 from karuturi/CLOUDSTACK-8868

CLOUDSTACK-8868: use PasswordGenerator.generateRandomPassword() to generate systemvm passwordsgenerateRandomPassword() is supposed to create root user passwords. Right now it is only used on the guest VMs. The format of the passwords it creates are of the form "random 3-character string with a lowercase character, uppercase character, and a digit" + random n-character string with only lowercase characters".

For whatever reason it was that we use generateRandomPassword() for guest VM root user passwords(maybe more secure?) we should use the same function for system VM root user passwords.

Tests:
manually tested that password is generated in proper format and am able to login to cpvm with the new password. ex: zD2ztm, tR8snbwhq

```
$ mvn -pl server test -Dtest=ConfigurationServerImplTest#testUpdateSystemvmPassword
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.cloud.server.ConfigurationServerImplTest
log4j:WARN No appenders could be found for logger (com.cloud.utils.crypt.EncryptionSecretKeyChecker).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.487 sec - in com.cloud.server.ConfigurationServerImplTest

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 7.781 s
[INFO] Finished at: 2015-09-16T14:17:07+05:30
[INFO] Final Memory: 60M/466M
[INFO] ------------------------------------------------------------------------
```

* pr/841:
  CLOUDSTACK-8868: change the default vm.password.length to 10
  CLOUDSTACK-8868: use same method to generate passwords for system/guest vms
  removed commented code

Signed-off-by: Remi Bergsma <github@remi.nl>
This commit is contained in:
Remi Bergsma 2015-12-03 09:58:51 +01:00
commit 17d5bfa32c
5 changed files with 97 additions and 36 deletions

View File

@ -899,14 +899,6 @@ public enum Config {
"0",
"Default disk I/O read rate in requests per second allowed in User vm's disk.",
null),
VmPasswordLength(
"Advanced",
ManagementServer.class,
Integer.class,
"vm.password.length",
"6",
"Specifies the length of a randomly generated password",
null),
VmDiskThrottlingIopsWriteRate(
"Advanced",
ManagementServer.class,

View File

@ -773,7 +773,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
throw new InvalidParameterValueException("Please enter a positive value for the configuration parameter:" + name);
}
//TODO - better validation for all password pamameters
if ("vm.password.length".equalsIgnoreCase(name) && val < 6) {
if ("vm.password.length".equalsIgnoreCase(name) && val < 10) {
throw new InvalidParameterValueException("Please enter a value greater than 6 for the configuration parameter:" + name);
}
if ("remote.access.vpn.psk.length".equalsIgnoreCase(name)) {

View File

@ -155,6 +155,8 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio
protected ConfigDepot _configDepot;
@Inject
protected ConfigurationManager _configMgr;
@Inject
protected ManagementService _mgrService;
public ConfigurationServerImpl() {
@ -416,30 +418,6 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio
});
}
/*
private void updateUuids() {
_identityDao.initializeDefaultUuid("disk_offering");
_identityDao.initializeDefaultUuid("network_offerings");
_identityDao.initializeDefaultUuid("vm_template");
_identityDao.initializeDefaultUuid("user");
_identityDao.initializeDefaultUuid("domain");
_identityDao.initializeDefaultUuid("account");
_identityDao.initializeDefaultUuid("guest_os");
_identityDao.initializeDefaultUuid("guest_os_category");
_identityDao.initializeDefaultUuid("hypervisor_capabilities");
_identityDao.initializeDefaultUuid("snapshot_policy");
_identityDao.initializeDefaultUuid("security_group");
_identityDao.initializeDefaultUuid("security_group_rule");
_identityDao.initializeDefaultUuid("physical_network");
_identityDao.initializeDefaultUuid("physical_network_traffic_types");
_identityDao.initializeDefaultUuid("physical_network_service_providers");
_identityDao.initializeDefaultUuid("virtual_router_providers");
_identityDao.initializeDefaultUuid("networks");
_identityDao.initializeDefaultUuid("user_ip_address");
_identityDao.initializeDefaultUuid("counter");
}
*/
private String getMountParent() {
return getEnvironmentProperty("mount.parent");
}
@ -692,7 +670,7 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio
if (already == null) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
try {
String rpassword = PasswordGenerator.generatePresharedKey(8);
String rpassword = _mgrService.generateRandomPassword();
String wSql = "INSERT INTO `cloud`.`configuration` (category, instance, component, name, value, description) "
+ "VALUES ('Secure','DEFAULT', 'management-server','system.vm.password', ?,'randmon password generated each management server starts for system vm')";
PreparedStatement stmt = txn.prepareAutoCloseStatement(wSql);

View File

@ -503,6 +503,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
import org.apache.cloudstack.framework.config.ConfigDepot;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
@ -670,9 +671,11 @@ import com.cloud.vm.dao.SecondaryStorageVmDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
public class ManagementServerImpl extends ManagerBase implements ManagementServer {
public class ManagementServerImpl extends ManagerBase implements ManagementServer, Configurable {
public static final Logger s_logger = Logger.getLogger(ManagementServerImpl.class.getName());
static final ConfigKey<Integer> vmPasswordLength = new ConfigKey<Integer>("Advanced", Integer.class, "vm.password.length", "10",
"Specifies the length of a randomly generated password", false);
@Inject
public AccountManager _accountMgr;
@Inject
@ -904,7 +907,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
@Override
public String generateRandomPassword() {
final Integer passwordLength = Integer.parseInt(_configDao.getValue("vm.password.length"));
final Integer passwordLength = vmPasswordLength.value();
return PasswordGenerator.generateRandomPassword(passwordLength);
}
@ -3019,6 +3022,16 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
return cmdList;
}
@Override
public String getConfigComponentName() {
return ManagementServer.class.getSimpleName();
}
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {vmPasswordLength};
}
protected class EventPurgeTask extends ManagedContextRunnable {
@Override
protected void runInContext() {

View File

@ -19,14 +19,74 @@ package com.cloud.server;
import java.io.File;
import java.io.IOException;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.configuration.dao.ResourceCountDao;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.dc.dao.HostPodDao;
import com.cloud.dc.dao.VlanDao;
import com.cloud.domain.dao.DomainDao;
import com.cloud.network.dao.NetworkDao;
import com.cloud.offerings.dao.NetworkOfferingDao;
import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.db.TransactionLegacy;
import org.apache.cloudstack.framework.config.ConfigDepot;
import org.apache.cloudstack.framework.config.ConfigDepotAdmin;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.io.FileUtils;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.runners.MockitoJUnitRunner;
@RunWith(MockitoJUnitRunner.class)
public class ConfigurationServerImplTest {
@Mock
private ConfigurationDao _configDao;
@Mock
private DataCenterDao _zoneDao;
@Mock
private HostPodDao _podDao;
@Mock
private DiskOfferingDao _diskOfferingDao;
@Mock
private ServiceOfferingDao _serviceOfferingDao;
@Mock
private NetworkOfferingDao _networkOfferingDao;
@Mock
private DataCenterDao _dataCenterDao;
@Mock
private NetworkDao _networkDao;
@Mock
private VlanDao _vlanDao;
@Mock
private DomainDao _domainDao;
@Mock
private AccountDao _accountDao;
@Mock
private ResourceCountDao _resourceCountDao;
@Mock
private NetworkOfferingServiceMapDao _ntwkOfferingServiceMapDao;
@Mock
private ConfigDepotAdmin _configDepotAdmin;
@Mock
private ConfigDepot _configDepot;
@Mock
private ConfigurationManager _configMgr;
@Mock
private ManagementService _mgrService;
@InjectMocks
private ConfigurationServerImpl configurationServer;
@Spy
ConfigurationServerImpl windowsImpl = new ConfigurationServerImpl() {
protected boolean isOnWindows() {
@ -84,4 +144,22 @@ public class ConfigurationServerImplTest {
Assert.assertFalse(linuxImpl.isOnWindows());
Assert.assertEquals("scripts/vm/systemvm/injectkeys.sh", linuxImpl.getInjectScript());
}
@Test
public void testUpdateSystemvmPassword() {
//setup
String realusername = System.getProperty("user.name");
System.setProperty("user.name", "cloud");
Mockito.when(_configDao.getValue("system.vm.random.password")).thenReturn(String.valueOf(true));
TransactionLegacy.open("cloud");
Mockito.when(_mgrService.generateRandomPassword()).thenReturn("randomPassword");
//call the method to test
configurationServer.updateSystemvmPassword();
//verify that generateRandomPassword() is called
Mockito.verify(_mgrService, Mockito.times(1)).generateRandomPassword();
//teardown
System.setProperty("user.name", realusername);
}
}