From a0f6cb47f10a82686186a1f9cc51327e2e466272 Mon Sep 17 00:00:00 2001 From: Vijayendra Bhamidipati Date: Wed, 11 Jul 2012 19:13:23 -0700 Subject: [PATCH] CS-15217: Security: Malicious user is able to get the size of the cloud by enumerating IDs Description: Removing DB IDs from exception messages. --- .../cloud/network/rules/RulesManagerImpl.java | 167 +++++++++++------- .../security/SecurityGroupManagerImpl.java | 97 +++++----- 2 files changed, 153 insertions(+), 111 deletions(-) diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index a9edd268211..01c71abd5df 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -56,6 +56,7 @@ import com.cloud.user.AccountManager; import com.cloud.user.DomainManager; import com.cloud.user.UserContext; import com.cloud.uservm.UserVm; +import com.cloud.utils.IdentityProxy; import com.cloud.utils.Ternary; import com.cloud.utils.component.Inject; import com.cloud.utils.component.Manager; @@ -114,7 +115,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { @Override public void checkIpAndUserVm(IpAddress ipAddress, UserVm userVm, Account caller) { if (ipAddress == null || ipAddress.getAllocatedTime() == null || ipAddress.getAllocatedToAccountId() == null) { - throw new InvalidParameterValueException("Unable to create ip forwarding rule on address " + ipAddress + ", invalid IP address specified."); + throw new InvalidParameterValueException("Unable to create ip forwarding rule on address " + ipAddress + ", invalid IP address specified.", null); } if (userVm == null) { @@ -122,19 +123,19 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } if (userVm.getState() == VirtualMachine.State.Destroyed || userVm.getState() == VirtualMachine.State.Expunging) { - throw new InvalidParameterValueException("Invalid user vm: " + userVm.getId()); + throw new InvalidParameterValueException("Couldn't find user vm by id", null); } _accountMgr.checkAccess(caller, null, true, ipAddress, userVm); // validate that IP address and userVM belong to the same account if (ipAddress.getAllocatedToAccountId().longValue() != userVm.getAccountId()) { - throw new InvalidParameterValueException("Unable to create ip forwarding rule, IP address " + ipAddress + " owner is not the same as owner of virtual machine " + userVm.toString()); + throw new InvalidParameterValueException("Unable to create ip forwarding rule, IP address " + ipAddress + " owner is not the same as owner of virtual machine " + userVm.toString(), null); } // validate that userVM is in the same availability zone as the IP address if (ipAddress.getDataCenterId() != userVm.getDataCenterIdToDeployIn()) { - throw new InvalidParameterValueException("Unable to create ip forwarding rule, IP address " + ipAddress + " is not in the same availability zone as virtual machine " + userVm.toString()); + throw new InvalidParameterValueException("Unable to create ip forwarding rule, IP address " + ipAddress + " is not in the same availability zone as virtual machine " + userVm.toString(), null); } } @@ -148,11 +149,13 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { _accountMgr.checkAccess(caller, null, true, rule, userVm); if (userVm.getState() == VirtualMachine.State.Destroyed || userVm.getState() == VirtualMachine.State.Expunging) { - throw new InvalidParameterValueException("Invalid user vm: " + userVm.getId()); + throw new InvalidParameterValueException("Couldn't locate user vm by id", null); } if (rule.getAccountId() != userVm.getAccountId()) { - throw new InvalidParameterValueException("New rule " + rule + " and vm id=" + userVm.getId() + " belong to different accounts"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(userVm, userVm.getId(), "vmId")); + throw new InvalidParameterValueException("New rule " + rule + " and vm with specified vmId belong to different accounts", idList); } } @@ -170,9 +173,13 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // Validate ip address if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddrId, "ipId")); + throw new InvalidParameterValueException("Unable to create port forwarding rule; ip with specified ipId doesn't exist in the system", idList); } else if (ipAddress.isOneToOneNat()) { - throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddrId, "ipId")); + throw new InvalidParameterValueException("Unable to create port forwarding rule; ip with specified ipId has static nat enabled", idList); } Long networkId = rule.getNetworkId(); @@ -182,7 +189,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { //set networkId just for verification purposes ipAddress.setAssociatedWithNetworkId(networkId); _networkMgr.checkIpForService(ipAddress, Service.PortForwarding); - + s_logger.debug("The ip is not associated with the network id="+ networkId + " so assigning"); try { ipAddress = _networkMgr.associateIPToGuestNetwork(ipAddrId, networkId); @@ -194,7 +201,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } else { _networkMgr.checkIpForService(ipAddress, Service.PortForwarding); } - + try { _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding, FirewallRuleType.User); @@ -204,19 +211,19 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // start port can't be bigger than end port if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) { - throw new InvalidParameterValueException("Start port can't be bigger than end port"); + throw new InvalidParameterValueException("Start port can't be bigger than end port", null); } // check that the port ranges are of equal size if ((rule.getDestinationPortEnd() - rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - rule.getSourcePortStart())) { - throw new InvalidParameterValueException("Source port and destination port ranges should be of equal sizes."); + throw new InvalidParameterValueException("Source port and destination port ranges should be of equal sizes.", null); } // validate user VM exists UserVm vm = _vmDao.findById(vmId); if (vm == null) { throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + - ", invalid virtual machine id specified (" + vmId + ")."); + ", couldn't locate virtual machine by id; (invalid id specified)", null); } else { checkRuleAndUserVm(rule, vm, caller); } @@ -226,7 +233,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { Ip dstIp = rule.getDestinationIpAddress(); Nic guestNic = _networkMgr.getNicInNetwork(vmId, networkId); if (guestNic == null || guestNic.getIp4Address() == null) { - throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress"); + throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress", null); } else { dstIp = new Ip(guestNic.getIp4Address()); } @@ -268,7 +275,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (e instanceof NetworkRuleConflictException) { throw (NetworkRuleConflictException) e; } - + throw new CloudRuntimeException("Unable to add rule for the ip id=" + ipAddrId, e); } } finally { @@ -282,8 +289,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } } } - - + + } @Override @@ -298,7 +305,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // Validate ip address if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to create static nat rule; ip id=" + ipAddrId + " doesn't exist in the system"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddrId, "ipId")); + throw new InvalidParameterValueException("Unable to create static nat rule; ip with specified ipId doesn't exist in the system", idList); } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || ipAddress.getAssociatedWithVmId() == null) { throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); } @@ -314,7 +323,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { Network network = _networkMgr.getNetwork(networkId); NetworkOffering off = _configMgr.getNetworkOffering(network.getNetworkOfferingId()); if (off.getElasticIp()) { - throw new InvalidParameterValueException("Can't create ip forwarding rules for the network where elasticIP service is enabled"); + throw new InvalidParameterValueException("Can't create ip forwarding rules for the network where elasticIP service is enabled", null); } String dstIp = _networkMgr.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); @@ -371,32 +380,34 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // Verify input parameters UserVmVO vm = _vmDao.findById(vmId); if (vm == null) { - throw new InvalidParameterValueException("Can't enable static nat for the address id=" + ipId + - ", invalid virtual machine id specified (" + vmId + ")."); + List idList = new ArrayList(); + idList.add(new IdentityProxy("user_ip_address", ipId, "ipId")); + throw new InvalidParameterValueException("Can't enable static nat for the address with specified ipId " + + ", invalid virtual machine id specified", idList); } IPAddressVO ipAddress = _ipAddressDao.findById(ipId); if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to find ip address by id " + ipId); + throw new InvalidParameterValueException("Unable to find ip address by id", null); } // Verify input parameters boolean setNetworkId = false; if (!isSystemVm) { - //associate ip address to network (if needed) - if (ipAddress.getAssociatedWithNetworkId() == null) { - s_logger.debug("The ip is not associated with the network id="+ networkId + " so assigning"); - try { - ipAddress = _networkMgr.associateIPToGuestNetwork(ipId, networkId); - } catch (Exception ex) { - s_logger.warn("Failed to associate ip id=" + ipId + " to network id=" + networkId + " as " + - "a part of enable static nat"); - return false; + //associate ip address to network (if needed) + if (ipAddress.getAssociatedWithNetworkId() == null) { + s_logger.debug("The ip is not associated with the network id="+ networkId + " so assigning"); + try { + ipAddress = _networkMgr.associateIPToGuestNetwork(ipId, networkId); + } catch (Exception ex) { + s_logger.warn("Failed to associate ip id=" + ipId + " to network id=" + networkId + " as " + + "a part of enable static nat"); + return false; + } + setNetworkId = true; } - setNetworkId = true; - } - - _networkMgr.checkIpForService(ipAddress, Service.StaticNat); + + _networkMgr.checkIpForService(ipAddress, Service.StaticNat); // Check permissions checkIpAndUserVm(ipAddress, vm, caller); @@ -405,13 +416,20 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // Check that vm has a nic in the network Nic guestNic = _networkMgr.getNicInNetwork(vmId, networkId); if (guestNic == null) { - throw new InvalidParameterValueException("Vm doesn't belong to the network " + networkId); + List idList = new ArrayList(); + idList.add(new IdentityProxy(vm, vmId, "vmId")); + throw new InvalidParameterValueException("Vm doesn't belong to the network with specified id", idList); } Network network = _networkMgr.getNetwork(networkId); + if (network == null) { + throw new InvalidParameterValueException("Unable to find network by id", null); + } if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.StaticNat)) { + List idList = new ArrayList(); + idList.add(new IdentityProxy(network, networkId, "networkId")); throw new InvalidParameterValueException("Unable to create static nat rule; StaticNat service is not " + - "supported in network id=" + networkId); + "supported in network with specified id", idList); } // Verify ip address parameter @@ -443,7 +461,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { protected void isIpReadyForStaticNat(long vmId, IPAddressVO ipAddress, Account caller, long callerUserId) throws NetworkRuleConflictException, ResourceUnavailableException { if (ipAddress.isSourceNat()) { - throw new InvalidParameterValueException("Can't enable static, ip address " + ipAddress + " is a sourceNat ip address"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddress.getId(), "ipId")); + throw new InvalidParameterValueException("Can't enable static, ip address with specified id is a sourceNat ip address", idList); } if (!ipAddress.isOneToOneNat()) { // Dont allow to enable static nat if PF/LB rules exist for the IP @@ -477,7 +497,12 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // If there is public ip address already associated with the vm, throw an exception if (!reassignStaticNat) { - throw new InvalidParameterValueException("Failed to enable static nat for the ip address id=" + ipAddress.getId() + " as vm id=" + vmId + " is already associated with ip id=" + oldIP.getId()); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddress.getId(), "ipId")); + idList.add(new IdentityProxy("user_vm", vmId, "vmId")); + idList.add(new IdentityProxy(oldIP, oldIP.getId(), "oldipId")); + throw new InvalidParameterValueException("Failed to enable static nat for the ip address with specified ipId" + + " as vm with specified vmId is already associated with ip with specified oldipId", idList); } // unassign old static nat rule s_logger.debug("Disassociating static nat for ip " + oldIP); @@ -495,7 +520,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { PortForwardingRuleVO rule = _portForwardingDao.findById(ruleId); if (rule == null) { - throw new InvalidParameterValueException("Unable to find " + ruleId); + throw new InvalidParameterValueException("Unable to find rule by id", null); } _accountMgr.checkAccess(caller, null, true, rule); @@ -530,7 +555,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { FirewallRuleVO rule = _firewallDao.findById(ruleId); if (rule == null) { - throw new InvalidParameterValueException("Unable to find " + ruleId); + throw new InvalidParameterValueException("Unable to find rule by id", null); } _accountMgr.checkAccess(caller, null, true, rule); @@ -642,8 +667,13 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (ipId != null) { IPAddressVO ipAddressVO = _ipAddressDao.findById(ipId); - if (ipAddressVO == null || !ipAddressVO.readyToUse()) { - throw new InvalidParameterValueException("Ip address id=" + ipId + " not ready for port forwarding rules yet"); + if (ipAddressVO == null) { + throw new InvalidParameterValueException("Unable to find ip address by id", null); + } + if (!ipAddressVO.readyToUse()) { + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddressVO, ipId, "ipId")); + throw new InvalidParameterValueException("Ip address with specified ipId not ready for port forwarding rules yet", idList); } _accountMgr.checkAccess(caller, null, true, ipAddressVO); } @@ -661,7 +691,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { sb.and("id", sb.entity().getId(), Op.EQ); sb.and("ip", sb.entity().getSourceIpAddressId(), Op.EQ); sb.and("purpose", sb.entity().getPurpose(), Op.EQ); - + if (tags != null && !tags.isEmpty()) { SearchBuilder tagSearch = _resourceTagDao.createSearchBuilder(); for (int count=0; count < tags.size(); count++) { @@ -681,7 +711,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (id != null) { sc.setParameters("id", id); } - + if (tags != null && !tags.isEmpty()) { int count = 0; sc.setJoinParameters("tagSearch", "resourceType", TaggedResourceType.PortForwardingRule.toString()); @@ -855,7 +885,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (ipId != null) { IPAddressVO ipAddressVO = _ipAddressDao.findById(ipId); if (ipAddressVO == null || !ipAddressVO.readyToUse()) { - throw new InvalidParameterValueException("Ip address id=" + ipId + " not ready for port forwarding rules yet"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddressVO, ipId, "ipId")); + throw new InvalidParameterValueException("Ip address with specified ipId not ready for port forwarding rules yet", idList); } _accountMgr.checkAccess(caller, null, true, ipAddressVO); } @@ -1109,16 +1141,16 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { checkIpAndUserVm(ipAddress, null, caller); if (ipAddress.getSystem()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Can't disable static nat for system IP address with specified id"); - ex.addProxyObject(ipAddress, ipId, "ipId"); - throw ex; + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipId, "ipId")); + throw new InvalidParameterValueException("Can't disable static nat for system IP address with specified id", idList); } Long vmId = ipAddress.getAssociatedWithVmId(); if (vmId == null) { - InvalidParameterValueException ex = new InvalidParameterValueException("Specified IP address id is not associated with any vm Id"); - ex.addProxyObject(ipAddress, ipId, "ipId"); - throw ex; + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipId, "ipId")); + throw new InvalidParameterValueException("Specified IP address id is not associated with any vm Id", idList); } // if network has elastic IP functionality supported, we first have to disable static nat on old ip in order to @@ -1142,9 +1174,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { checkIpAndUserVm(ipAddress, null, caller); if (!ipAddress.isOneToOneNat()) { - InvalidParameterValueException ex = new InvalidParameterValueException("One to one nat is not enabled for the specified ip id"); - ex.addProxyObject(ipAddress, ipId, "ipId"); - throw ex; + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipId, "ipId")); + throw new InvalidParameterValueException("One to one nat is not enabled for the specified ip id", idList); } // Revoke all firewall rules for the ip @@ -1204,12 +1236,15 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { IpAddress ip = _ipAddressDao.findById(rule.getSourceIpAddressId()); FirewallRuleVO ruleVO = _firewallDao.findById(rule.getId()); - if (ip == null || !ip.isOneToOneNat() || ip.getAssociatedWithVmId() == null) { - InvalidParameterValueException ex = new InvalidParameterValueException("Source ip address of the specified firewall rule id is not static nat enabled"); - ex.addProxyObject(ruleVO, rule.getId(), "ruleId"); - throw ex; + if (ip == null) { + throw new InvalidParameterValueException("Unable to find ip by id", null); } - + if (!ip.isOneToOneNat() || ip.getAssociatedWithVmId() == null) { + List idList = new ArrayList(); + idList.add(new IdentityProxy(ruleVO, rule.getId(), "ruleId")); + throw new InvalidParameterValueException("Source ip address of the specified firewall rule id is not static nat enabled", idList); + } + String dstIp; if (forRevoke) { dstIp = _networkMgr.getIpInNetworkIncludingRemoved(ip.getAssociatedWithVmId(), rule.getNetworkId()); @@ -1239,8 +1274,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { UserVmVO vm = _vmDao.findById(sourceIp.getAssociatedWithVmId()); Network network = _networkMgr.getNetwork(networkId); if (network == null) { - CloudRuntimeException ex = new CloudRuntimeException("Unable to find an ip address to map to specified vm id"); - ex.addProxyObject(vm, vm.getId(), "vmId"); + CloudRuntimeException ex = new CloudRuntimeException("Unable to find an ip address to map to specified vm id"); + ex.addProxyObject(vm, vm.getId(), "vmId"); throw ex; } @@ -1304,11 +1339,11 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { success = enableStaticNat(ip.getId(), vm.getId(), guestNetwork.getId(), isSystemVM); } catch (NetworkRuleConflictException ex) { s_logger.warn("Failed to enable static nat as a part of enabling elasticIp and staticNat for vm " + - vm + " in guest network " + guestNetwork + " due to exception ", ex); + vm + " in guest network " + guestNetwork + " due to exception ", ex); success = false; } catch (ResourceUnavailableException ex) { s_logger.warn("Failed to enable static nat as a part of enabling elasticIp and staticNat for vm " + - vm + " in guest network " + guestNetwork + " due to exception ", ex); + vm + " in guest network " + guestNetwork + " due to exception ", ex); success = false; } @@ -1322,7 +1357,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } } } - + @DB protected void removePFRule(PortForwardingRuleVO rule) { Transaction txn = Transaction.currentTxn(); @@ -1333,7 +1368,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (ip != null && ip.getVpcId() != null && _firewallDao.listByIp(ip.getId()).isEmpty()) { _networkMgr.unassignIPFromVpcNetwork(ip.getId()); } - + txn.commit(); } } diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index 7113cb87ba1..fcba445cd32 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -82,6 +82,7 @@ import com.cloud.user.DomainManager; import com.cloud.user.UserContext; import com.cloud.user.dao.AccountDao; import com.cloud.uservm.UserVm; +import com.cloud.utils.IdentityProxy; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; @@ -157,7 +158,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG UsageEventDao _usageEventDao; @Inject ResourceTagDao _resourceTagDao; - + ScheduledExecutorService _executorPool; ScheduledExecutorService _cleanupExecutor; @@ -454,7 +455,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG List groupsForVm = _securityGroupVMMapDao.listByInstanceId(vm.getId()); // For each group, find the security rules that allow the group for (SecurityGroupVMMapVO mapVO : groupsForVm) {// FIXME: use custom sql in the dao - //Add usage events for security group assign + //Add usage events for security group assign UsageEventVO usageEvent = new UsageEventVO(EventTypes.EVENT_SECURITY_GROUP_ASSIGN, vm.getAccountId(), vm.getDataCenterIdToDeployIn(), vm.getId(), mapVO.getSecurityGroupId()); _usageEventDao.persist(usageEvent); @@ -470,7 +471,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG List groupsForVm = _securityGroupVMMapDao.listByInstanceId(vm.getId()); // For each group, find the security rules rules that allow the group for (SecurityGroupVMMapVO mapVO : groupsForVm) {// FIXME: use custom sql in the dao - //Add usage events for security group remove + //Add usage events for security group remove UsageEventVO usageEvent = new UsageEventVO(EventTypes.EVENT_SECURITY_GROUP_REMOVE, vm.getAccountId(), vm.getDataCenterIdToDeployIn(), vm.getId(), mapVO.getSecurityGroupId()); _usageEventDao.persist(usageEvent); @@ -574,26 +575,26 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG Map groupList = cmd.getUserSecurityGroupList(); return authorizeSecurityGroupRule(securityGroupId,protocol,startPort,endPort,icmpType,icmpCode,cidrList,groupList,SecurityRuleType.IngressRule); } - + private List authorizeSecurityGroupRule(Long securityGroupId,String protocol,Integer startPort,Integer endPort,Integer icmpType,Integer icmpCode,List cidrList,Map groupList,SecurityRuleType ruleType) { Integer startPortOrType = null; Integer endPortOrCode = null; - + // Validate parameters SecurityGroup securityGroup = _securityGroupDao.findById(securityGroupId); if (securityGroup == null) { - throw new InvalidParameterValueException("Unable to find security group by id " + securityGroupId); + throw new InvalidParameterValueException("Unable to find security group by id ", null); } if (cidrList == null && groupList == null) { - throw new InvalidParameterValueException("At least one cidr or at least one security group needs to be specified"); + throw new InvalidParameterValueException("At least one cidr or at least one security group needs to be specified", null); } Account caller = UserContext.current().getCaller(); Account owner = _accountMgr.getAccount(securityGroup.getAccountId()); if (owner == null) { - throw new InvalidParameterValueException("Unable to find security group owner by id=" + securityGroup.getAccountId()); + throw new InvalidParameterValueException("Unable to find security group owner by id", null); } // Verify permissions @@ -605,45 +606,45 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG } if (!NetUtils.isValidSecurityGroupProto(protocol)) { - throw new InvalidParameterValueException("Invalid protocol " + protocol); + throw new InvalidParameterValueException("Invalid protocol " + protocol, null); } if ("icmp".equalsIgnoreCase(protocol)) { if ((icmpType == null) || (icmpCode == null)) { - throw new InvalidParameterValueException("Invalid ICMP type/code specified, icmpType = " + icmpType + ", icmpCode = " + icmpCode); + throw new InvalidParameterValueException("Invalid ICMP type/code specified, icmpType = " + icmpType + ", icmpCode = " + icmpCode, null); } if (icmpType == -1 && icmpCode != -1) { - throw new InvalidParameterValueException("Invalid icmp code"); + throw new InvalidParameterValueException("Invalid icmp code", null); } if (icmpType != -1 && icmpCode == -1) { - throw new InvalidParameterValueException("Invalid icmp code: need non-negative icmp code "); + throw new InvalidParameterValueException("Invalid icmp code: need non-negative icmp code ", null); } if (icmpCode > 255 || icmpType > 255 || icmpCode < -1 || icmpType < -1) { - throw new InvalidParameterValueException("Invalid icmp type/code "); + throw new InvalidParameterValueException("Invalid icmp type/code ", null); } startPortOrType = icmpType; endPortOrCode = icmpCode; } else if (protocol.equals(NetUtils.ALL_PROTO)) { if ((startPort != null) || (endPort != null)) { - throw new InvalidParameterValueException("Cannot specify startPort or endPort without specifying protocol"); + throw new InvalidParameterValueException("Cannot specify startPort or endPort without specifying protocol", null); } startPortOrType = 0; endPortOrCode = 0; } else { if ((startPort == null) || (endPort == null)) { - throw new InvalidParameterValueException("Invalid port range specified, startPort = " + startPort + ", endPort = " + endPort); + throw new InvalidParameterValueException("Invalid port range specified, startPort = " + startPort + ", endPort = " + endPort, null); } if (startPort == 0 && endPort == 0) { endPort = 65535; } if (startPort > endPort) { - throw new InvalidParameterValueException("Invalid port range " + startPort + ":" + endPort); + throw new InvalidParameterValueException("Invalid port range " + startPort + ":" + endPort, null); } if (startPort > 65535 || endPort > 65535 || startPort < -1 || endPort < -1) { - throw new InvalidParameterValueException("Invalid port numbers " + startPort + ":" + endPort); + throw new InvalidParameterValueException("Invalid port numbers " + startPort + ":" + endPort, null); } if (startPort < 0 || endPort < 0) { - throw new InvalidParameterValueException("Invalid port range " + startPort + ":" + endPort); + throw new InvalidParameterValueException("Invalid port range " + startPort + ":" + endPort, null); } startPortOrType = startPort; endPortOrCode = endPort; @@ -662,18 +663,22 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG if ((group == null) || (authorizedAccountName == null)) { throw new InvalidParameterValueException( - "Invalid user group specified, fields 'group' and 'account' cannot be null, please specify groups in the form: userGroupList[0].group=XXX&userGroupList[0].account=YYY"); + "Invalid user group specified, fields 'group' and 'account' cannot be null, please specify groups in the form: userGroupList[0].group=XXX&userGroupList[0].account=YYY", null); } Account authorizedAccount = _accountDao.findActiveAccount(authorizedAccountName, domainId); if (authorizedAccount == null) { - throw new InvalidParameterValueException("Nonexistent account: " + authorizedAccountName + " when trying to authorize security group rule for " + securityGroupId + ":" + protocol + ":" - + startPortOrType + ":" + endPortOrCode); + List idList = new ArrayList(); + idList.add(new IdentityProxy(securityGroup, securityGroupId, "secGrpId")); + throw new InvalidParameterValueException("Nonexistent account: " + authorizedAccountName + " when trying to authorize security group rule for security group with specified id:" + protocol + ":" + + startPortOrType + ":" + endPortOrCode, idList); } SecurityGroupVO groupVO = _securityGroupDao.findByAccountAndName(authorizedAccount.getId(), group); if (groupVO == null) { - throw new InvalidParameterValueException("Nonexistent group " + group + " for account " + authorizedAccountName + "/" + domainId + " is given, unable to authorize security group rule."); + List idList = new ArrayList(); + idList.add(new IdentityProxy("domain", domainId, "domainId")); + throw new InvalidParameterValueException("Nonexistent group " + group + " for account " + authorizedAccountName + "/ is given, unable to authorize security group rule.", idList); } // Check permissions @@ -746,7 +751,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG } } } - + @Override @DB @ActionEvent(eventType = EventTypes.EVENT_SECURITY_GROUP_REVOKE_EGRESS, eventDescription = "Revoking Egress Rule ", async = true) @@ -754,7 +759,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG Long id = cmd.getId(); return revokeSecurityGroupRule(id, SecurityRuleType.EgressRule); } - + @Override @DB @ActionEvent(eventType = EventTypes.EVENT_SECURITY_GROUP_REVOKE_INGRESS, eventDescription = "Revoking Ingress Rule ", async = true) @@ -763,23 +768,25 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG Long id = cmd.getId(); return revokeSecurityGroupRule(id, SecurityRuleType.IngressRule); } - + private boolean revokeSecurityGroupRule(Long id, SecurityRuleType type) { // input validation Account caller = UserContext.current().getCaller(); - + SecurityGroupRuleVO rule = _securityGroupRuleDao.findById(id); if (rule == null) { s_logger.debug("Unable to find security rule with id " + id); - throw new InvalidParameterValueException("Unable to find security rule with id " + id); + throw new InvalidParameterValueException("Unable to find security rule byid ", null); } // check type if (type != rule.getRuleType()) { s_logger.debug("Mismatch in rule type for security rule with id " + id ); - throw new InvalidParameterValueException("Mismatch in rule type for security rule with id " + id); + List idList = new ArrayList(); + idList.add(new IdentityProxy(rule, id, "ruleId")); + throw new InvalidParameterValueException("Mismatch in rule type for security rule with specified id ", idList); } - + // Check permissions SecurityGroup securityGroup = _securityGroupDao.findById(rule.getSecurityGroupId()); _accountMgr.checkAccess(caller, null, true, securityGroup); @@ -824,7 +831,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG Account owner = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId()); if (_securityGroupDao.isNameInUse(owner.getId(), owner.getDomainId(), cmd.getSecurityGroupName())) { - throw new InvalidParameterValueException("Unable to create security group, a group with name " + name + " already exisits."); + throw new InvalidParameterValueException("Unable to create security group, a group with name " + name + " already exists.", null); } return createSecurityGroup(cmd.getSecurityGroupName(), cmd.getDescription(), owner.getDomainId(), owner.getAccountId(), owner.getAccountName()); @@ -861,12 +868,12 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG _serverId = ((ManagementServer) ComponentLocator.getComponent(ManagementServer.Name)).getId(); s_logger.info("SecurityGroupManager: num worker threads=" + _numWorkerThreads + - ", time between cleanups=" + _timeBetweenCleanups + " global lock timeout=" + _globalWorkLockTimeout); + ", time between cleanups=" + _timeBetweenCleanups + " global lock timeout=" + _globalWorkLockTimeout); createThreadPools(); return true; } - + protected void createThreadPools() { _executorPool = Executors.newScheduledThreadPool(_numWorkerThreads, new NamedThreadFactory("NWGRP")); _cleanupExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("NWGRP-Cleanup")); @@ -963,7 +970,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG s_logger.debug("Unable to send ingress rules updates for vm: " + userVmId + "(agentid=" + agentId + ")"); _workDao.updateStep(work.getInstanceId(), seqnum, Step.Done); } - + } } } finally { @@ -1026,10 +1033,10 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG @Override @DB public void removeInstanceFromGroups(long userVmId) { - if (_securityGroupVMMapDao.countSGForVm(userVmId) < 1) { - s_logger.trace("No security groups found for vm id=" + userVmId + ", returning"); - return; - } + if (_securityGroupVMMapDao.countSGForVm(userVmId) < 1) { + s_logger.trace("No security groups found for vm id=" + userVmId + ", returning"); + return; + } final Transaction txn = Transaction.currentTxn(); txn.start(); UserVm userVm = _userVMDao.acquireInLockTable(userVmId); // ensures that duplicate entries are not created in @@ -1053,7 +1060,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG SecurityGroupVO group = _securityGroupDao.findById(groupId); if (group == null) { - throw new InvalidParameterValueException("Unable to find network group: " + groupId + "; failed to delete group."); + throw new InvalidParameterValueException("Unable to find network group by id; failed to delete group.", null); } // check permissions @@ -1064,11 +1071,11 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG group = _securityGroupDao.lockRow(groupId, true); if (group == null) { - throw new InvalidParameterValueException("Unable to find security group by id " + groupId); + throw new InvalidParameterValueException("Unable to find security group by id ", null); } if (group.getName().equalsIgnoreCase(SecurityGroupManager.DEFAULT_GROUP_NAME)) { - throw new InvalidParameterValueException("The network group default is reserved"); + throw new InvalidParameterValueException("The network group default is reserved", null); } List allowingRules = _securityGroupRuleDao.listByAllowedSecurityGroupId(groupId); @@ -1100,27 +1107,27 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG if (instanceId != null) { UserVmVO userVM = _userVMDao.findById(instanceId); if (userVM == null) { - throw new InvalidParameterValueException("Unable to list network groups for virtual machine instance " + instanceId + "; instance not found."); + throw new InvalidParameterValueException("Unable to list network groups for virtual machine instance; unable to locate virtual machine instance by id", null); } _accountMgr.checkAccess(caller, null, true, userVM); return listSecurityGroupRulesByVM(instanceId.longValue()); } List securityRulesList = new ArrayList(); - + Ternary domainIdRecursiveListProject = new Ternary(cmd.getDomainId(), cmd.isRecursive(), null); _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedAccounts, domainIdRecursiveListProject, cmd.listAll(), false); Long domainId = domainIdRecursiveListProject.first(); Boolean isRecursive = domainIdRecursiveListProject.second(); ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); - + Filter searchFilter = new Filter(SecurityGroupVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); SearchBuilder sb = _securityGroupDao.createSearchBuilder(); _accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); - + if (tags != null && !tags.isEmpty()) { SearchBuilder tagSearch = _resourceTagDao.createSearchBuilder(); for (int count=0; count < tags.size(); count++) { @@ -1139,7 +1146,7 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG if (id != null) { sc.setParameters("id", id); } - + if (tags != null && !tags.isEmpty()) { int count = 0; sc.setJoinParameters("tagSearch", "resourceType", TaggedResourceType.SecurityGroup.toString());