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 @@ -276,12 +276,12 @@ private void addRouteToInternalIpOrCidr(String localgw, String eth1ip, String et
s_logger.debug("addRouteToInternalIp: destIp is null");
return;
}
if (!NetUtils.isValidIp(destIpOrCidr) && !NetUtils.isValidCIDR(destIpOrCidr)) {
if (!NetUtils.isValidIp4(destIpOrCidr) && !NetUtils.isValidIp4Cidr(destIpOrCidr)) {
s_logger.warn(" destIp is not a valid ip address or cidr destIp=" + destIpOrCidr);
return;
}
boolean inSameSubnet = false;
if (NetUtils.isValidIp(destIpOrCidr)) {
if (NetUtils.isValidIp4(destIpOrCidr)) {
if (eth1ip != null && eth1mask != null) {
inSameSubnet = NetUtils.sameSubnet(eth1ip, destIpOrCidr, eth1mask);
} else {
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/apache/cloudstack/api/ApiServerService.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import com.cloud.exception.CloudAuthenticationException;

public interface ApiServerService {
public boolean verifyRequest(Map<String, Object[]> requestParameters, Long userId) throws ServerApiException;
public boolean verifyRequest(Map<String, Object[]> requestParameters, Long userId, InetAddress remoteAddress) throws ServerApiException;

public Long fetchDomainId(String domainUUID);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,10 @@ public void create() {
String guestCidr = _networkService.getNetwork(getNetworkId()).getCidr();

for (String cidr : getSourceCidrList()) {
if (!NetUtils.isValidCIDR(cidr)) {
if (!NetUtils.isValidIp4Cidr(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Source cidrs formatting error " + cidr);
}
if (cidr.equals(NetUtils.ALL_CIDRS)) {
if (cidr.equals(NetUtils.ALL_IP4_CIDRS)) {
continue;
}
if (!NetUtils.isNetworkAWithinNetworkB(cidr, guestCidr)) {
Expand All @@ -261,7 +261,7 @@ public void create() {
//Destination CIDR formatting check. Since it's optional param, no need to set a default as in the case of source.
if(destCidrList != null){
for(String cidr : destCidrList){
if(!NetUtils.isValidCIDR(cidr)) {
if(!NetUtils.isValidIp4Cidr(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Destination cidrs formatting error" + cidr);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public List<String> getSourceCidrList() {
return cidrlist;
} else {
List<String> oneCidrList = new ArrayList<String>();
oneCidrList.add(NetUtils.ALL_CIDRS);
oneCidrList.add(NetUtils.ALL_IP4_CIDRS);
return oneCidrList;
}

Expand Down Expand Up @@ -242,7 +242,7 @@ public long getDomainId() {
public void create() {
if (getSourceCidrList() != null) {
for (String cidr : getSourceCidrList()) {
if (!NetUtils.isValidCIDR(cidr)) {
if (!NetUtils.isValidIp4Cidr(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Source CIDRs formatting error " + cidr);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ public void create() {

Ip privateIp = getVmSecondaryIp();
if (privateIp != null) {
if (!NetUtils.isValidIp(privateIp.toString())) {
if (!NetUtils.isValidIp4(privateIp.toString())) {
throw new InvalidParameterValueException("Invalid vm ip address");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public Map<Long, List<String>> getVmIdIpListMap() {
}

//check wether the given ip is valid ip or not
if (vmIp == null || !NetUtils.isValidIp(vmIp)) {
if (vmIp == null || !NetUtils.isValidIp4(vmIp)) {
throw new InvalidParameterValueException("Invalid ip address "+ vmIp +" passed in vmidipmap for " +
"vmid " + vmId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public List<String> getSourceCidrList() {
return cidrlist;
} else {
List<String> oneCidrList = new ArrayList<String>();
oneCidrList.add(NetUtils.ALL_CIDRS);
oneCidrList.add(NetUtils.ALL_IP4_CIDRS);
return oneCidrList;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public void create() throws ResourceAllocationException {
NicSecondaryIp result;
String secondaryIp = null;
if ((ip = getIpaddress()) != null) {
if (!NetUtils.isValidIp(ip)) {
if (!NetUtils.isValidIp4(ip)) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Invalid ip address " + ip);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void execute() throws ResourceUnavailableException, ResourceAllocationExc
CallContext.current().setEventDetails("Nic Id: " + getNicId() );
String ip;
if ((ip = getIpaddress()) != null) {
if (!NetUtils.isValidIp(ip)) {
if (!NetUtils.isValidIp4(ip)) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Invalid ip address " + ip);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ public class ApiServiceConfiguration implements Configurable {
"API end point. Can be used by CS components/services deployed remotely, for sending CS API requests", true);
public static final ConfigKey<Long> DefaultUIPageSize = new ConfigKey<Long>("Advanced", Long.class, "default.ui.page.size", "20",
"The default pagesize to be used by UI and other clients when making list* API calls", true, ConfigKey.Scope.Global);

public static final ConfigKey<Boolean> ApiSourceCidrChecksEnabled = new ConfigKey<>("Advanced", Boolean.class, "api.source.cidr.checks.enabled",
"true", "Are the source checks on API calls enabled (true) or not (false)? See api.allowed.source.cidr.list", true, ConfigKey.Scope.Global);
public static final ConfigKey<String> ApiAllowedSourceCidrList = new ConfigKey<String>("Advanced", String.class, "api.allowed.source.cidr.list",
"0.0.0.0/0,::/0", "Comma separated list of IPv4/IPv6 CIDRs from which API calls can be performed. Can be set on Global and Account levels.", true, ConfigKey.Scope.Account);
@Override
public String getConfigComponentName() {
return ApiServiceConfiguration.class.getSimpleName();
}

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {ManagementHostIPAdr, ApiServletPath, DefaultUIPageSize};
return new ConfigKey<?>[] {ManagementHostIPAdr, ApiServletPath, DefaultUIPageSize, ApiSourceCidrChecksEnabled, ApiAllowedSourceCidrList};
}

}
10 changes: 4 additions & 6 deletions core/src/com/cloud/network/HAProxyConfigurator.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public String[] generateConfiguration(final List<PortForwardingRuleTO> fwRules)
private List<String> getRulesForPool(final String poolName, final List<PortForwardingRuleTO> fwRules) {
final PortForwardingRuleTO firstRule = fwRules.get(0);
final String publicIP = firstRule.getSrcIp();
final String publicPort = Integer.toString(firstRule.getSrcPortRange()[0]);
final int publicPort = firstRule.getSrcPortRange()[0];
// FIXEME: String algorithm = firstRule.getAlgorithm();

final List<String> result = new ArrayList<String>();
Expand All @@ -108,9 +108,7 @@ private List<String> getRulesForPool(final String poolName, final List<PortForwa
sb = new StringBuilder();
// FIXME sb.append("\t").append("balance ").append(algorithm);
result.add(sb.toString());
if (publicPort.equals(NetUtils.HTTP_PORT)
// && global option httpclose set (or maybe not in this spot???)
) {
if (publicPort == NetUtils.HTTP_PORT) {
sb = new StringBuilder();
sb.append("\t").append("mode http");
result.add(sb.toString());
Expand Down Expand Up @@ -473,7 +471,7 @@ private List<String> getRulesForPool(final LoadBalancerTO lbTO, final boolean ke
StringBuilder sb = new StringBuilder();
final String poolName = sb.append(lbTO.getSrcIp().replace(".", "_")).append('-').append(lbTO.getSrcPort()).toString();
final String publicIP = lbTO.getSrcIp();
final String publicPort = Integer.toString(lbTO.getSrcPort());
final int publicPort = lbTO.getSrcPort();
final String algorithm = lbTO.getAlgorithm();

final List<String> result = new ArrayList<String>();
Expand Down Expand Up @@ -544,7 +542,7 @@ private List<String> getRulesForPool(final LoadBalancerTO lbTO, final boolean ke
if (stickinessSubRule != null && !destsAvailable) {
s_logger.warn("Haproxy stickiness policy for lb rule: " + lbTO.getSrcIp() + ":" + lbTO.getSrcPort() + ": Not Applied, cause: backends are unavailable");
}
if (publicPort.equals(NetUtils.HTTP_PORT) && !keepAliveEnabled || httpbasedStickiness) {
if (publicPort == NetUtils.HTTP_PORT && !keepAliveEnabled || httpbasedStickiness) {

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.

why this? equals() seems more what is intended then ==
is making it an int really the best? as opposed to Integer (or String for that matter)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is a port number so it should be a Int. It's a part of the NetUtils refactor. I found that only HTTP/HTTPS port were Strings.

I wanted to make the change in other files as small as possible.

sb = new StringBuilder();
sb.append("\t").append("mode http");
result.add(sb.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private boolean validateIpAddresses() {
if (ipAddress.trim().equalsIgnoreCase("localhost")) {
continue;
}
if (!NetUtils.isValidIp(ipAddress)) {
if (!NetUtils.isValidIp4(ipAddress)) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private boolean validateIpAddresses() {
if (ip.equalsIgnoreCase("localhost")) {
continue;
}
if (!NetUtils.isValidIp(ip)) {
if (!NetUtils.isValidIp4(ip)) {
return false;
}
} else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin
String ipAddr = Script.runSimpleBashScript(new StringBuilder().append("virt-cat ").append(command.getVmName())
.append(" /var/lib/dhclient/" + leaseFile + " | tail -16 | grep 'fixed-address' | awk '{print $2}' | sed -e 's/;//'").toString());
// Check if the IP belongs to the network
if((ipAddr != null) && NetUtils.isIpWithtInCidrRange(ipAddr, networkCidr)){
if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)){
ip = ipAddr;
break;
}
Expand All @@ -65,7 +65,7 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin
String[] ips = ipList.split("\n");
for (String ipAddr : ips){
// Check if the IP belongs to the network
if((ipAddr != null) && NetUtils.isIpWithtInCidrRange(ipAddr, networkCidr)){
if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)){
ip = ipAddr;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private void validatePoolAndCluster() {
LOGGER.debug("Clustering requires a pool, setting pool to true");
agentInOvm3Pool = true;
}
if (!NetUtils.isValidIp(ovm3PoolVip)) {
if (!NetUtils.isValidIp4(ovm3PoolVip)) {
LOGGER.debug("No VIP, Setting ovm3pool and ovm3cluster to false");
agentInOvm3Pool = false;
agentInOvm3Cluster = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public Answer execute(final GetVmIpAddressCommand command, final CitrixResourceB
Map<String, String> vmIpsMap = rec.networks;

for (String ipAddr: vmIpsMap.values()) {
if (NetUtils.isIpWithtInCidrRange(ipAddr, networkCidr)) {
if (NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)) {
vmIp = ipAddr;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2574,7 +2574,7 @@ private void deleteServersInGuestVlan(final long vlanTag, final String vlanSelfI
}

private String getNetScalerProtocol(final LoadBalancerTO loadBalancer) throws ExecutionException {
final String port = Integer.toString(loadBalancer.getSrcPort());
final int port = loadBalancer.getSrcPort();
String lbProtocol = loadBalancer.getLbProtocol();
final StickinessPolicyTO[] stickyPolicies = loadBalancer.getStickinessPolicies();
String nsProtocol = "TCP";
Expand All @@ -2596,7 +2596,7 @@ private String getNetScalerProtocol(final LoadBalancerTO loadBalancer) throws Ex
return lbProtocol.toUpperCase();
}

if (port.equals(NetUtils.HTTP_PORT)) {
if (port == NetUtils.HTTP_PORT) {
nsProtocol = "HTTP";
} else if (NetUtils.TCP_PROTO.equalsIgnoreCase(lbProtocol)) {
nsProtocol = "TCP";
Expand Down
62 changes: 41 additions & 21 deletions server/src/com/cloud/api/ApiServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import com.cloud.utils.Pair;
import com.cloud.utils.ReflectUtil;
import com.cloud.utils.StringUtils;
import com.cloud.utils.net.NetUtils;
import com.cloud.utils.component.ComponentContext;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.component.PluggableService;
Expand Down Expand Up @@ -97,6 +98,7 @@
import org.apache.cloudstack.api.response.ExceptionResponse;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.LoginCmdResponse;
import org.apache.cloudstack.config.ApiServiceConfiguration;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
Expand Down Expand Up @@ -795,7 +797,7 @@ private void buildAuditTrail(final StringBuilder auditTrailSb, final String comm
}

@Override
public boolean verifyRequest(final Map<String, Object[]> requestParameters, final Long userId) throws ServerApiException {
public boolean verifyRequest(final Map<String, Object[]> requestParameters, final Long userId, InetAddress remoteAddress) throws ServerApiException {
try {
String apiKey = null;
String secretKey = null;
Expand All @@ -814,21 +816,18 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
if (userId != null) {
final User user = ApiDBUtils.findUserById(userId);

try {
checkCommandAvailable(user, commandName);
} catch (final RequestLimitException ex) {
s_logger.debug(ex.getMessage());
throw new ServerApiException(ApiErrorCode.API_LIMIT_EXCEED, ex.getMessage());
} catch (final PermissionDeniedException ex) {
s_logger.debug("The user with id:" + userId + " is not allowed to request the API command or the API command does not exist: " + commandName);
throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "The user is not allowed to request the API command or the API command does not exist");
if (!commandAvailable(remoteAddress, commandName, user)) {
return false;
}

return true;
} else {
// check against every available command to see if the command exists or not
if (!s_apiNameCmdClassMap.containsKey(commandName) && !commandName.equals("login") && !commandName.equals("logout")) {
s_logger.debug("The user with id:" + userId + " is not allowed to request the API command or the API command does not exist: " + commandName);
throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "The user is not allowed to request the API command or the API command does not exist");
final String errorMessage = "The given command " + commandName + " either does not exist, is not available" +
" for user, or not available from ip address '" + remoteAddress.getHostAddress() + "'.";
s_logger.debug(errorMessage);
return false;
}
}

Expand Down Expand Up @@ -916,15 +915,8 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
return false;
}

try {
checkCommandAvailable(user, commandName);
} catch (final RequestLimitException ex) {
s_logger.debug(ex.getMessage());
throw new ServerApiException(ApiErrorCode.API_LIMIT_EXCEED, ex.getMessage());
} catch (final PermissionDeniedException ex) {
s_logger.debug("The given command:" + commandName + " does not exist or it is not available for user");
throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "The given command:" + commandName + " does not exist or it is not available for user with id:"
+ userId);
if (!commandAvailable(remoteAddress, commandName, user)) {
return false;
}

// verify secret key exists
Expand Down Expand Up @@ -959,6 +951,21 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
return false;
}

private boolean commandAvailable(final InetAddress remoteAddress, final String commandName, final User user) {
try {
checkCommandAvailable(user, commandName, remoteAddress);
} catch (final RequestLimitException ex) {
s_logger.debug(ex.getMessage());
throw new ServerApiException(ApiErrorCode.API_LIMIT_EXCEED, ex.getMessage());
} catch (final PermissionDeniedException ex) {
final String errorMessage = "The given command '" + commandName + "' either does not exist, is not available" +
" for user, or not available from ip address '" + remoteAddress + "'.";
s_logger.debug(errorMessage);
return false;
}
return true;
}

@Override
public Long fetchDomainId(final String domainUUID) {
final Domain domain = domainMgr.getDomain(domainUUID);
Expand Down Expand Up @@ -1113,11 +1120,24 @@ public boolean verifyUser(final Long userId) {
return true;
}

private void checkCommandAvailable(final User user, final String commandName) throws PermissionDeniedException {
private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException {
if (user == null) {
throw new PermissionDeniedException("User is null for role based API access check for command" + commandName);
}

final Account account = accountMgr.getAccount(user.getAccountId());
final String accessAllowedCidrs = ApiServiceConfiguration.ApiAllowedSourceCidrList.valueIn(account.getId()).replaceAll("\\s","");
final Boolean apiSourceCidrChecksEnabled = ApiServiceConfiguration.ApiSourceCidrChecksEnabled.value();

if (apiSourceCidrChecksEnabled) {
s_logger.debug("CIDRs from which account '" + account.toString() + "' is allowed to perform API calls: " + accessAllowedCidrs);
if (!NetUtils.isIpInCidrList(remoteAddress, accessAllowedCidrs.split(","))) {
s_logger.warn("Request by account '" + account.toString() + "' was denied since " + remoteAddress + " does not match " + accessAllowedCidrs);
throw new PermissionDeniedException("Calls for domain '" + account.getAccountName() + "' are not allowed from ip address '" + remoteAddress.getHostAddress());
}
}


for (final APIChecker apiChecker : apiAccessCheckers) {
apiChecker.checkAccess(user, commandName);
}
Expand Down
Loading