diff --git a/.github/linters/.flake8 b/.github/linters/.flake8 index f250719ca198..c513b0e3c4b8 100644 --- a/.github/linters/.flake8 +++ b/.github/linters/.flake8 @@ -24,6 +24,7 @@ # E242 Tab after ',' # E273 Tab after keyword # E274 Tab before keyword +# E713 Test for membership should be 'not in' # E742 Do not define classes named 'I', 'O', or 'l' # E743 Do not define functions named 'I', 'O', or 'l' # E901 SyntaxError or IndentationError @@ -37,4 +38,4 @@ exclude = .git, venv -select = E112,E113,E133,E223,E224,E227,E242,E273,E274,E742,E743,E901,E902,W291,W292,W293,W391 +select = E112,E113,E133,E223,E224,E227,E242,E273,E274,E713,E742,E743,E901,E902,W291,W292,W293,W391 diff --git a/api/src/main/java/org/apache/cloudstack/ca/CAManager.java b/api/src/main/java/org/apache/cloudstack/ca/CAManager.java index 12a9d3d7b41c..b0fb1ac73c21 100644 --- a/api/src/main/java/org/apache/cloudstack/ca/CAManager.java +++ b/api/src/main/java/org/apache/cloudstack/ca/CAManager.java @@ -77,6 +77,14 @@ public interface CAManager extends CAService, Configurable, PluggableService { "15", "The number of days before expiry of a client certificate, the validations are checked. Admins are alerted when auto-renewal is not allowed, otherwise auto-renewal is attempted.", true, ConfigKey.Scope.Cluster); + + ConfigKey CertManagementCustomSubjectAlternativeName = new ConfigKey<>("Advanced", String.class, + "ca.framework.cert.management.custom.san", + "cloudstack.internal", + "The custom Subject Alternative Name that will be added to the management server certificate. " + + "The actual implementation will depend on the configured CA provider.", + false); + /** * Returns a list of available CA provider plugins * @return returns list of CAProvider diff --git a/core/src/main/java/com/cloud/resource/CommandWrapper.java b/core/src/main/java/com/cloud/resource/CommandWrapper.java index a839234117be..72d1348dfe70 100644 --- a/core/src/main/java/com/cloud/resource/CommandWrapper.java +++ b/core/src/main/java/com/cloud/resource/CommandWrapper.java @@ -19,10 +19,13 @@ package com.cloud.resource; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; public abstract class CommandWrapper { protected Logger logger = LogManager.getLogger(getClass()); @@ -33,4 +36,26 @@ public abstract class CommandWrapper*?![]{}~".indexOf(c) != -1) { + sanitized.append('\\'); + } + sanitized.append(c); + } + return sanitized.toString(); + } + + public void removeDpdkPort(String portToRemove) { + logger.debug("Removing DPDK port: " + portToRemove); + int port; + try { + port = Integer.valueOf(portToRemove); + } catch (NumberFormatException nfe) { + throw new CloudRuntimeException(String.format("Invalid DPDK port specified: '%s'", portToRemove)); + } + Script.executeCommand("ovs-vsctl", "del-port", String.valueOf(port)); + } } diff --git a/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAProvider.java b/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAProvider.java index 388cae7e0074..77b3ee27783c 100644 --- a/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAProvider.java +++ b/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAProvider.java @@ -22,6 +22,7 @@ import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.KeyStoreException; +import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; import java.util.List; import java.util.Map; @@ -45,6 +46,7 @@ public interface CAProvider { /** * Issues certificate with provided options + * * @param domainNames * @param ipAddresses * @param validityDays @@ -104,4 +106,6 @@ public interface CAProvider { * @return returns description */ String getDescription(); + + boolean isManagementCertificate(java.security.cert.Certificate certificate) throws CertificateParsingException; } diff --git a/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAService.java b/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAService.java index facf13a5cb68..721c88bee507 100644 --- a/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAService.java +++ b/framework/ca/src/main/java/org/apache/cloudstack/framework/ca/CAService.java @@ -21,6 +21,7 @@ import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.KeyStoreException; +import java.security.cert.CertificateParsingException; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; @@ -47,4 +48,6 @@ public interface CAService { * @return returns char[] passphrase */ char[] getKeyStorePassphrase(); + + boolean isManagementCertificate(java.security.cert.Certificate certificate) throws CertificateParsingException; } diff --git a/framework/cluster/src/main/java/com/cloud/cluster/ClusterManager.java b/framework/cluster/src/main/java/com/cloud/cluster/ClusterManager.java index 1b1406c1cecc..54f575830e4a 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/ClusterManager.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/ClusterManager.java @@ -16,8 +16,8 @@ // under the License. package com.cloud.cluster; -import org.apache.cloudstack.management.ManagementServerHost; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.management.ManagementServerHost; import com.cloud.utils.component.Manager; @@ -77,6 +77,8 @@ public interface ClusterManager extends Manager { */ String getSelfPeerName(); + String getSelfNodeIP(); + long getManagementNodeId(); /** diff --git a/framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java b/framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java index e4e55eb9348a..050c2a7a1aa8 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java @@ -40,16 +40,16 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.cluster.dao.ManagementServerStatusDao; -import org.apache.cloudstack.management.ManagementServerHost; 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.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.management.ManagementServerHost; import org.apache.cloudstack.utils.identity.ManagementServerNode; import com.cloud.cluster.dao.ManagementServerHostDao; import com.cloud.cluster.dao.ManagementServerHostPeerDao; +import com.cloud.cluster.dao.ManagementServerStatusDao; import com.cloud.utils.DateUtil; import com.cloud.utils.Profiler; import com.cloud.utils.component.ComponentLifecycle; @@ -128,7 +128,7 @@ public ClusterManagerImpl() { // recursive remote calls between nodes // _executor = Executors.newCachedThreadPool(new NamedThreadFactory("Cluster-Worker")); - setRunLevel(ComponentLifecycle.RUN_LEVEL_FRAMEWORK); + setRunLevel(ComponentLifecycle.RUN_LEVEL_COMPONENT); } private void registerRequestPdu(final ClusterServiceRequestPdu pdu) { @@ -473,6 +473,7 @@ public String getSelfPeerName() { return Long.toString(_msId); } + @Override public String getSelfNodeIP() { return _clusterNodeIP; } diff --git a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceAdapter.java b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceAdapter.java index 735de5bdac9d..e073a28a6221 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceAdapter.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceAdapter.java @@ -28,7 +28,5 @@ public interface ClusterServiceAdapter extends Adapter { public ClusterService getPeerService(String strPeer) throws RemoteException; - public String getServiceEndpointName(String strPeer); - public int getServicePort(); } diff --git a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletAdapter.java b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletAdapter.java index 937ef4a62491..3e498b1fbec3 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletAdapter.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletAdapter.java @@ -23,6 +23,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.framework.config.ConfigDepot; import com.cloud.cluster.dao.ManagementServerHostDao; @@ -42,6 +43,8 @@ public class ClusterServiceServletAdapter extends AdapterBase implements Cluster @Inject private ManagementServerHostDao _mshostDao; @Inject + private CAManager caService; + @Inject protected ConfigDepot _configDepot; private ClusterServiceServletContainer _servletContainer; @@ -49,7 +52,7 @@ public class ClusterServiceServletAdapter extends AdapterBase implements Cluster private int _clusterServicePort = DEFAULT_SERVICE_PORT; public ClusterServiceServletAdapter() { - setRunLevel(ComponentLifecycle.RUN_LEVEL_FRAMEWORK); + setRunLevel(ComponentLifecycle.RUN_LEVEL_COMPONENT); } @Override @@ -64,12 +67,10 @@ public ClusterService getPeerService(String strPeer) throws RemoteException { String serviceUrl = getServiceEndpointName(strPeer); if (serviceUrl == null) return null; - - return new ClusterServiceServletImpl(serviceUrl); + return new ClusterServiceServletImpl(serviceUrl, caService); } - @Override - public String getServiceEndpointName(String strPeer) { + protected String getServiceEndpointName(String strPeer) { try { init(); } catch (ConfigurationException e) { @@ -93,7 +94,7 @@ public int getServicePort() { private String composeEndpointName(String nodeIP, int port) { StringBuffer sb = new StringBuffer(); - sb.append("http://").append(nodeIP).append(":").append(port).append("/clusterservice"); + sb.append("https://").append(nodeIP).append(":").append(port).append("/clusterservice"); return sb.toString(); } @@ -106,7 +107,8 @@ public boolean configure(String name, Map params) throws Configu @Override public boolean start() { _servletContainer = new ClusterServiceServletContainer(); - _servletContainer.start(new ClusterServiceServletHttpHandler(_manager), _clusterServicePort); + _servletContainer.start(new ClusterServiceServletHttpHandler(_manager), _manager.getSelfNodeIP(), + _clusterServicePort, caService); return true; } diff --git a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletContainer.java b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletContainer.java index ac468089f473..e8c3de980168 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletContainer.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletContainer.java @@ -17,11 +17,23 @@ package com.cloud.cluster; import java.io.IOException; -import java.net.ServerSocket; +import java.net.InetAddress; import java.net.Socket; +import java.security.GeneralSecurityException; +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLServerSocketFactory; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; + +import org.apache.cloudstack.framework.ca.CAService; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.http.ConnectionClosedException; import org.apache.http.HttpException; import org.apache.http.impl.DefaultConnectionReuseStrategy; @@ -41,12 +53,12 @@ import org.apache.http.protocol.ResponseContent; import org.apache.http.protocol.ResponseDate; import org.apache.http.protocol.ResponseServer; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; -import org.apache.cloudstack.managed.context.ManagedContextRunnable; - +import com.cloud.utils.StringUtils; import com.cloud.utils.concurrency.NamedThreadFactory; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; +import com.cloud.utils.nio.Link; public class ClusterServiceServletContainer { @@ -55,9 +67,9 @@ public class ClusterServiceServletContainer { public ClusterServiceServletContainer() { } - public boolean start(HttpRequestHandler requestHandler, int port) { + public boolean start(HttpRequestHandler requestHandler, String ip, int port, CAService caService) { - listenerThread = new ListenerThread(requestHandler, port); + listenerThread = new ListenerThread(requestHandler, ip, port, caService); listenerThread.start(); return true; @@ -69,26 +81,46 @@ public void stop() { } } + + protected static SSLServerSocket getSecuredServerSocket(SSLContext sslContext, String ip, int port) + throws IOException { + SSLServerSocketFactory sslFactory = sslContext.getServerSocketFactory(); + SSLServerSocket serverSocket = null; + if (StringUtils.isNotEmpty(ip)) { + serverSocket = (SSLServerSocket) sslFactory.createServerSocket(port, 0, + InetAddress.getByName(ip)); + } else { + serverSocket = (SSLServerSocket) sslFactory.createServerSocket(port); + } + serverSocket.setNeedClientAuth(true); + return serverSocket; + } + static class ListenerThread extends Thread { private static Logger LOGGER = LogManager.getLogger(ListenerThread.class); - private HttpService _httpService = null; - private volatile ServerSocket _serverSocket = null; - private HttpParams _params = null; - private ExecutorService _executor; + private HttpService httpService = null; + private volatile SSLServerSocket serverSocket = null; + private HttpParams params = null; + private ExecutorService executor; + private CAService caService = null; - public ListenerThread(HttpRequestHandler requestHandler, int port) { - _executor = Executors.newCachedThreadPool(new NamedThreadFactory("Cluster-Listener")); + public ListenerThread(HttpRequestHandler requestHandler, String ip, int port, + CAService caService) { + this.executor = Executors.newCachedThreadPool(new NamedThreadFactory("Cluster-Listener")); + this.caService = caService; try { - _serverSocket = new ServerSocket(port); - } catch (IOException ioex) { - LOGGER.error("error initializing cluster service servlet container", ioex); + SSLContext sslContext = Link.initManagementSSLContext(caService); + serverSocket = getSecuredServerSocket(sslContext, ip, port); + } catch (IOException | GeneralSecurityException e) { + LOGGER.error("Error initializing cluster service servlet container for secure connection", + e); return; } - _params = new BasicHttpParams(); - _params.setIntParameter(CoreConnectionPNames.SO_TIMEOUT, 5000) + params = new BasicHttpParams(); + params.setIntParameter(CoreConnectionPNames.SO_TIMEOUT, 5000) .setIntParameter(CoreConnectionPNames.SOCKET_BUFFER_SIZE, 8 * 1024) .setBooleanParameter(CoreConnectionPNames.STALE_CONNECTION_CHECK, false) .setBooleanParameter(CoreConnectionPNames.TCP_NODELAY, true) @@ -106,35 +138,55 @@ public ListenerThread(HttpRequestHandler requestHandler, int port) { reqistry.register("/clusterservice", requestHandler); // Set up the HTTP service - _httpService = new HttpService(httpproc, new DefaultConnectionReuseStrategy(), new DefaultHttpResponseFactory()); - _httpService.setParams(_params); - _httpService.setHandlerResolver(reqistry); + httpService = new HttpService(httpproc, new DefaultConnectionReuseStrategy(), new DefaultHttpResponseFactory()); + httpService.setParams(params); + httpService.setHandlerResolver(reqistry); } public void stopRunning() { - if (_serverSocket != null) { + if (serverSocket != null) { try { - _serverSocket.close(); + serverSocket.close(); } catch (IOException e) { LOGGER.info("[ignored] error on closing server socket", e); } - _serverSocket = null; + serverSocket = null; } } + protected boolean isValidPeerConnection(Socket socket) throws SSLPeerUnverifiedException, + CertificateParsingException { + SSLSocket sslSocket = (SSLSocket) socket; + SSLSession session = sslSocket.getSession(); + if (session == null || !session.isValid()) { + return false; + } + Certificate[] certs = session.getPeerCertificates(); + if (certs == null || certs.length < 1) { + return false; + } + return caService.isManagementCertificate(certs[0]); + } + @Override public void run() { if (LOGGER.isInfoEnabled()) - LOGGER.info("Cluster service servlet container listening on port " + _serverSocket.getLocalPort()); + LOGGER.info(String.format("Cluster service servlet container listening on host: %s and port %d", + serverSocket.getInetAddress().getHostAddress(), serverSocket.getLocalPort())); - while (_serverSocket != null) { + while (serverSocket != null) { try { // Set up HTTP connection - Socket socket = _serverSocket.accept(); + Socket socket = serverSocket.accept(); final DefaultHttpServerConnection conn = new DefaultHttpServerConnection(); - conn.bind(socket, _params); - - _executor.execute(new ManagedContextRunnable() { + conn.bind(socket, params); + if (!isValidPeerConnection(socket)) { + LOGGER.warn(String.format("Failure during validating cluster request from %s", + socket.getInetAddress().getHostAddress())); + conn.shutdown(); + continue; + } + executor.execute(new ManagedContextRunnable() { @Override protected void runInContext() { HttpContext context = new BasicHttpContext(null); @@ -143,7 +195,7 @@ protected void runInContext() { if (LOGGER.isTraceEnabled()) LOGGER.trace("dispatching cluster request from " + conn.getRemoteAddress().toString()); - _httpService.handleRequest(conn, context); + httpService.handleRequest(conn, context); if (LOGGER.isTraceEnabled()) LOGGER.trace("Cluster request from " + conn.getRemoteAddress().toString() + " is processed"); @@ -178,7 +230,7 @@ protected void runInContext() { } } - _executor.shutdown(); + executor.shutdown(); if (LOGGER.isInfoEnabled()) LOGGER.info("Cluster service servlet container shutdown"); } diff --git a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletImpl.java b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletImpl.java index b60012dbeef1..d582538c31e0 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletImpl.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletImpl.java @@ -17,99 +17,144 @@ package com.cloud.cluster; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.rmi.RemoteException; +import java.security.GeneralSecurityException; +import java.util.ArrayList; +import java.util.List; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.HttpException; +import javax.net.ssl.SSLContext; + +import org.apache.cloudstack.framework.ca.CAService; import org.apache.commons.httpclient.HttpStatus; -import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; -import org.apache.commons.httpclient.methods.PostMethod; -import org.apache.commons.httpclient.params.HttpClientParams; -import org.apache.logging.log4j.Logger; +import org.apache.http.NameValuePair; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.message.BasicNameValuePair; +import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import com.cloud.utils.HttpUtils; import com.cloud.utils.Profiler; +import com.cloud.utils.nio.Link; +import com.google.gson.Gson; public class ClusterServiceServletImpl implements ClusterService { private static final long serialVersionUID = 4574025200012566153L; protected Logger logger = LogManager.getLogger(getClass()); - private String _serviceUrl; + private String serviceUrl; + + private CAService caService; + + private Gson gson = new Gson(); + + protected static CloseableHttpClient s_client = null; - protected static HttpClient s_client = null; + private void logPostParametersForFailedEncoding(List parameters) { + if (logger.isTraceEnabled()) { + logger.trace(String.format("%s encoding failed for POST parameters: %s", HttpUtils.UTF_8, + gson.toJson(parameters))); + } + } public ClusterServiceServletImpl() { } - public ClusterServiceServletImpl(final String serviceUrl) { - logger.info("Setup cluster service servlet. service url: " + serviceUrl + ", request timeout: " + ClusterServiceAdapter.ClusterMessageTimeOut.value() + - " seconds"); + public ClusterServiceServletImpl(final String serviceUrl, final CAService caService) { + logger.info(String.format("Setup cluster service servlet. service url: %s, request timeout: %d seconds", serviceUrl, + ClusterServiceAdapter.ClusterMessageTimeOut.value())); + this.serviceUrl = serviceUrl; + this.caService = caService; + } - _serviceUrl = serviceUrl; + protected List getClusterServicePduPostParameters(final ClusterServicePdu pdu) { + List postParameters = new ArrayList<>(); + postParameters.add(new BasicNameValuePair("method", Integer.toString(RemoteMethodConstants.METHOD_DELIVER_PDU))); + postParameters.add(new BasicNameValuePair("sourcePeer", pdu.getSourcePeer())); + postParameters.add(new BasicNameValuePair("destPeer", pdu.getDestPeer())); + postParameters.add(new BasicNameValuePair("pduSeq", Long.toString(pdu.getSequenceId()))); + postParameters.add(new BasicNameValuePair("pduAckSeq", Long.toString(pdu.getAckSequenceId()))); + postParameters.add(new BasicNameValuePair("agentId", Long.toString(pdu.getAgentId()))); + postParameters.add(new BasicNameValuePair("gsonPackage", pdu.getJsonPackage())); + postParameters.add(new BasicNameValuePair("stopOnError", pdu.isStopOnError() ? "1" : "0")); + postParameters.add(new BasicNameValuePair("pduType", Integer.toString(pdu.getPduType()))); + return postParameters; } @Override public String execute(final ClusterServicePdu pdu) throws RemoteException { - - final HttpClient client = getHttpClient(); - final PostMethod method = new PostMethod(_serviceUrl); - - method.addParameter("method", Integer.toString(RemoteMethodConstants.METHOD_DELIVER_PDU)); - method.addParameter("sourcePeer", pdu.getSourcePeer()); - method.addParameter("destPeer", pdu.getDestPeer()); - method.addParameter("pduSeq", Long.toString(pdu.getSequenceId())); - method.addParameter("pduAckSeq", Long.toString(pdu.getAckSequenceId())); - method.addParameter("agentId", Long.toString(pdu.getAgentId())); - method.addParameter("gsonPackage", pdu.getJsonPackage()); - method.addParameter("stopOnError", pdu.isStopOnError() ? "1" : "0"); - method.addParameter("pduType", Integer.toString(pdu.getPduType())); + if (logger.isDebugEnabled()) { + logger.debug(String.format("Executing ClusterServicePdu with service URL: %s", serviceUrl)); + } + final CloseableHttpClient client = getHttpClient(); + final HttpPost method = new HttpPost(serviceUrl); + final List postParameters = getClusterServicePduPostParameters(pdu); + try { + method.setEntity(new UrlEncodedFormEntity(postParameters, HttpUtils.UTF_8)); + } catch (UnsupportedEncodingException e) { + logger.error("Failed to encode request POST parameters", e); + logPostParametersForFailedEncoding(postParameters); + throw new RemoteException("Failed to encode request POST parameters", e); + } return executePostMethod(client, method); } + protected List getPingPostParameters(final String callingPeer) { + List postParameters = new ArrayList<>(); + postParameters.add(new BasicNameValuePair("method", Integer.toString(RemoteMethodConstants.METHOD_PING))); + postParameters.add(new BasicNameValuePair("callingPeer", callingPeer)); + return postParameters; + } + @Override public boolean ping(final String callingPeer) throws RemoteException { if (logger.isDebugEnabled()) { - logger.debug("Ping at " + _serviceUrl); + logger.debug("Ping at " + serviceUrl); } - final HttpClient client = getHttpClient(); - final PostMethod method = new PostMethod(_serviceUrl); + final CloseableHttpClient client = getHttpClient(); + final HttpPost method = new HttpPost(serviceUrl); - method.addParameter("method", Integer.toString(RemoteMethodConstants.METHOD_PING)); - method.addParameter("callingPeer", callingPeer); + List postParameters = getPingPostParameters(callingPeer); + try { + method.setEntity(new UrlEncodedFormEntity(postParameters, HttpUtils.UTF_8)); + } catch (UnsupportedEncodingException e) { + logger.error("Failed to encode ping request POST parameters", e); + logPostParametersForFailedEncoding(postParameters); + throw new RemoteException("Failed to encode ping request POST parameters", e); + } final String returnVal = executePostMethod(client, method); - if ("true".equalsIgnoreCase(returnVal)) { - return true; - } - return false; + return Boolean.TRUE.toString().equalsIgnoreCase(returnVal); } - private String executePostMethod(final HttpClient client, final PostMethod method) { - int response = 0; + private String executePostMethod(final CloseableHttpClient client, final HttpPost method) { String result = null; try { final Profiler profiler = new Profiler(); profiler.start(); - response = client.executeMethod(method); + CloseableHttpResponse httpResponse = client.execute(method); + int response = httpResponse.getStatusLine().getStatusCode(); if (response == HttpStatus.SC_OK) { - result = method.getResponseBodyAsString(); + result = EntityUtils.toString(httpResponse.getEntity()); profiler.stop(); if (logger.isDebugEnabled()) { - logger.debug("POST " + _serviceUrl + " response :" + result + ", responding time: " + profiler.getDurationInMillis() + " ms"); + logger.debug("POST " + serviceUrl + " response :" + result + ", responding time: " + profiler.getDurationInMillis() + " ms"); } } else { profiler.stop(); - logger.error("Invalid response code : " + response + ", from : " + _serviceUrl + ", method : " + method.getParameter("method") + " responding time: " + + logger.error("Invalid response code : " + response + ", from : " + serviceUrl + ", method : " + method.getParams().getParameter("method") + " responding time: " + profiler.getDurationInMillis()); } - } catch (final HttpException e) { - logger.error("HttpException from : " + _serviceUrl + ", method : " + method.getParameter("method")); - } catch (final IOException e) { - logger.error("IOException from : " + _serviceUrl + ", method : " + method.getParameter("method")); - } catch (final Throwable e) { - logger.error("Exception from : " + _serviceUrl + ", method : " + method.getParameter("method") + ", exception :", e); + } catch (IOException e) { + logger.error("Exception from : " + serviceUrl + ", method : " + method.getParams().getParameter("method") + ", exception :", e); } finally { method.releaseConnection(); } @@ -117,20 +162,25 @@ private String executePostMethod(final HttpClient client, final PostMethod metho return result; } - private HttpClient getHttpClient() { - + private CloseableHttpClient getHttpClient() { if (s_client == null) { - final MultiThreadedHttpConnectionManager mgr = new MultiThreadedHttpConnectionManager(); - mgr.getParams().setDefaultMaxConnectionsPerHost(4); - - // TODO make it configurable - mgr.getParams().setMaxTotalConnections(1000); + SSLContext sslContext = null; + try { + sslContext = Link.initManagementSSLContext(caService); + } catch (GeneralSecurityException | IOException e) { + throw new RuntimeException(e); + } - s_client = new HttpClient(mgr); - final HttpClientParams clientParams = new HttpClientParams(); - clientParams.setSoTimeout(ClusterServiceAdapter.ClusterMessageTimeOut.value() * 1000); + int timeout = ClusterServiceAdapter.ClusterMessageTimeOut.value() * 1000; + RequestConfig config = RequestConfig.custom() + .setConnectTimeout(timeout) + .setConnectionRequestTimeout(timeout) + .setSocketTimeout(timeout).build(); - s_client.setParams(clientParams); + s_client = HttpClientBuilder.create() + .setDefaultRequestConfig(config) + .setSSLContext(sslContext) + .build(); } return s_client; } diff --git a/framework/cluster/src/test/java/com/cloud/cluster/ClusterManagerImplTest.java b/framework/cluster/src/test/java/com/cloud/cluster/ClusterManagerImplTest.java new file mode 100644 index 000000000000..9b1854f73487 --- /dev/null +++ b/framework/cluster/src/test/java/com/cloud/cluster/ClusterManagerImplTest.java @@ -0,0 +1,38 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.cluster; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +@RunWith(MockitoJUnitRunner.class) +public class ClusterManagerImplTest { + @InjectMocks + ClusterManagerImpl clusterManager = new ClusterManagerImpl(); + + @Test + public void testGetSelfNodeIP() { + String ip = "1.2.3.4"; + ReflectionTestUtils.setField(clusterManager, "_clusterNodeIP", ip); + Assert.assertEquals(ip, clusterManager.getSelfNodeIP()); + } +} diff --git a/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletAdapterTest.java b/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletAdapterTest.java index 25266106f43f..6f4b7d6aa9ee 100644 --- a/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletAdapterTest.java +++ b/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletAdapterTest.java @@ -50,7 +50,7 @@ public void setup() throws IllegalArgumentException, IllegalAccessException, NoS @Test public void testRunLevel() { int runLevel = clusterServiceServletAdapter.getRunLevel(); - assertTrue(runLevel == ComponentLifecycle.RUN_LEVEL_FRAMEWORK); + assertTrue(runLevel == ComponentLifecycle.RUN_LEVEL_COMPONENT); assertTrue(runLevel == clusterManagerImpl.getRunLevel()); } } diff --git a/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletContainerTest.java b/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletContainerTest.java new file mode 100644 index 000000000000..baf4e5841bdb --- /dev/null +++ b/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletContainerTest.java @@ -0,0 +1,87 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.cluster; + +import java.io.IOException; +import java.net.InetAddress; +import java.util.ArrayList; +import java.util.List; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLContextSpi; +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLServerSocketFactory; + +import org.apache.commons.collections.CollectionUtils; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.utils.StringUtils; + +@RunWith(MockitoJUnitRunner.class) +public class ClusterServiceServletContainerTest { + + private void runGetSecuredServerSocket(String ip) { + SSLContext sslContext = Mockito.mock(SSLContext.class); + SSLContextSpi sslContextSpi = Mockito.mock(SSLContextSpi.class); + ReflectionTestUtils.setField(sslContext, "contextSpi", sslContextSpi); + SSLServerSocketFactory factory = Mockito.mock(SSLServerSocketFactory.class); + Mockito.when(sslContext.getServerSocketFactory()).thenReturn(factory); + int port = 9090; + final List socketNeedClientAuth = new ArrayList<>(); + try { + SSLServerSocket socketMock = Mockito.mock(SSLServerSocket.class); + if (StringUtils.isBlank(ip)) { + Mockito.when(factory.createServerSocket(port)).thenReturn(socketMock); + } else { + Mockito.when(factory.createServerSocket(Mockito.anyInt(), Mockito.anyInt(), + Mockito.any(InetAddress.class))).thenReturn(socketMock); + } + Mockito.doAnswer((Answer) invocationOnMock -> { + boolean needClientAuth = (boolean) invocationOnMock.getArguments()[0]; + socketNeedClientAuth.add(needClientAuth); + return null; + }).when(socketMock).setNeedClientAuth(Mockito.anyBoolean()); + SSLServerSocket socket = ClusterServiceServletContainer.getSecuredServerSocket(sslContext, ip, 9090); + if (StringUtils.isBlank(ip)) { + Mockito.verify(factory, Mockito.times(1)).createServerSocket(port); + } else { + Mockito.verify(factory, Mockito.times(1)).createServerSocket(port, 0, InetAddress.getByName(ip)); + } + Mockito.verify(socket, Mockito.times(1)).setNeedClientAuth(Mockito.anyBoolean()); + Assert.assertTrue(CollectionUtils.isNotEmpty(socketNeedClientAuth)); + Assert.assertTrue(socketNeedClientAuth.get(0)); + } catch (IOException e) { + Assert.fail("Exception occurred: " + e.getMessage()); + } + } + + @Test + public void testGetSecuredServerSocketNoIp() { + runGetSecuredServerSocket(""); + } + + @Test + public void testGetSecuredServerSocketIp() { + runGetSecuredServerSocket("1.2.3.4"); + } +} diff --git a/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletImplTest.java b/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletImplTest.java new file mode 100644 index 000000000000..361c77dbeff4 --- /dev/null +++ b/framework/cluster/src/test/java/com/cloud/cluster/ClusterServiceServletImplTest.java @@ -0,0 +1,64 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.cluster; + +import java.util.List; +import java.util.Optional; + +import org.apache.commons.collections.CollectionUtils; +import org.apache.http.NameValuePair; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class ClusterServiceServletImplTest { + + @InjectMocks + ClusterServiceServletImpl clusterServiceServlet = new ClusterServiceServletImpl(); + + @Test + public void testClusterServicePduPostParameters() { + List parameters = + clusterServiceServlet.getClusterServicePduPostParameters(Mockito.mock(ClusterServicePdu.class)); + Assert.assertTrue(CollectionUtils.isNotEmpty(parameters)); + Optional opt = parameters.stream().filter(x -> x.getName().equals("method")).findFirst(); + Assert.assertTrue(opt.isPresent()); + NameValuePair val = opt.get(); + Assert.assertEquals(Integer.toString(RemoteMethodConstants.METHOD_DELIVER_PDU), val.getValue()); + } + + @Test + public void testPingPostParameters() { + String peer = "1.2.3.4"; + List parameters = + clusterServiceServlet.getPingPostParameters(peer); + Assert.assertTrue(CollectionUtils.isNotEmpty(parameters)); + Optional opt = parameters.stream().filter(x -> x.getName().equals("method")).findFirst(); + Assert.assertTrue(opt.isPresent()); + NameValuePair val = opt.get(); + Assert.assertEquals(Integer.toString(RemoteMethodConstants.METHOD_PING), val.getValue()); + opt = parameters.stream().filter(x -> x.getName().equals("callingPeer")).findFirst(); + Assert.assertTrue(opt.isPresent()); + val = opt.get(); + Assert.assertEquals(peer, val.getValue()); + } +} diff --git a/packaging/systemd/cloudstack-management.default b/packaging/systemd/cloudstack-management.default index ca8ff628fc19..994a1ee86997 100644 --- a/packaging/systemd/cloudstack-management.default +++ b/packaging/systemd/cloudstack-management.default @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -JAVA_OPTS="-Djava.security.properties=/etc/cloudstack/management/java.security.ciphers -Djava.awt.headless=true -Dcom.sun.management.jmxremote=false -Xmx2G -XX:+UseParallelGC -XX:MaxGCPauseMillis=500 -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/var/log/cloudstack/management/ -XX:ErrorFile=/var/log/cloudstack/management/cloudstack-management.err --add-opens=java.base/java.lang=ALL-UNNAMED --add-exports=java.base/sun.security.x509=ALL-UNNAMED" +JAVA_OPTS="-Djava.security.properties=/etc/cloudstack/management/java.security.ciphers -Djava.awt.headless=true -Xmx2G -XX:+UseParallelGC -XX:MaxGCPauseMillis=500 -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/var/log/cloudstack/management/ -XX:ErrorFile=/var/log/cloudstack/management/cloudstack-management.err --add-opens=java.base/java.lang=ALL-UNNAMED --add-exports=java.base/sun.security.x509=ALL-UNNAMED" CLASSPATH="/usr/share/cloudstack-management/lib/*:/etc/cloudstack/management:/usr/share/cloudstack-common:/usr/share/cloudstack-management/setup:/usr/share/cloudstack-management:/usr/share/java/mysql-connector-java.jar:/usr/share/cloudstack-mysql-ha/lib/*" diff --git a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java index d7001ce941aa..25c45ed2a102 100644 --- a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java +++ b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java @@ -35,9 +35,11 @@ import java.security.SignatureException; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; +import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; @@ -53,7 +55,6 @@ import javax.net.ssl.TrustManagerFactory; import javax.xml.bind.DatatypeConverter; -import com.cloud.configuration.Config; import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.framework.ca.CAProvider; import org.apache.cloudstack.framework.ca.Certificate; @@ -62,6 +63,8 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.utils.security.CertUtils; import org.apache.cloudstack.utils.security.KeyStoreUtils; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.bouncycastle.asn1.pkcs.Attribute; import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers; import org.bouncycastle.asn1.x509.Extension; @@ -75,11 +78,11 @@ import org.bouncycastle.util.io.pem.PemReader; import com.cloud.certificate.dao.CrlDao; +import com.cloud.configuration.Config; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.db.GlobalLock; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; -import org.apache.commons.lang3.StringUtils; public final class RootCAProvider extends AdapterBase implements CAProvider, Configurable { @@ -130,6 +133,8 @@ public final class RootCAProvider extends AdapterBase implements CAProvider, Con "true", "When set to true, it will allow expired client certificate during SSL handshake.", true); + private static String managementCertificateCustomSAN; + /////////////////////////////////////////////////////////// /////////////// Root CA Private Methods /////////////////// @@ -371,8 +376,11 @@ private boolean loadManagementKeyStore() { List nicIps = NetUtils.getAllDefaultNicIps(); addConfiguredManagementIp(nicIps); nicIps = new ArrayList<>(new HashSet<>(nicIps)); + List domainNames = new ArrayList<>(); + domainNames.add(NetUtils.getHostName()); + domainNames.add(CAManager.CertManagementCustomSubjectAlternativeName.value()); - final Certificate serverCertificate = issueCertificate(Collections.singletonList(NetUtils.getHostName()), nicIps, getCaValidityDays()); + final Certificate serverCertificate = issueCertificate(domainNames, nicIps, getCaValidityDays()); if (serverCertificate == null || serverCertificate.getPrivateKey() == null) { throw new CloudRuntimeException("Failed to generate management server certificate and load management server keystore"); @@ -431,6 +439,7 @@ private boolean setupCA() { @Override public boolean start() { + managementCertificateCustomSAN = CAManager.CertManagementCustomSubjectAlternativeName.value(); return loadRootCAKeyPair() && loadRootCAKeyPair() && loadManagementKeyStore(); } @@ -485,4 +494,26 @@ public String getProviderName() { public String getDescription() { return "CloudStack's Root CA provider plugin"; } + + @Override + public boolean isManagementCertificate(java.security.cert.Certificate certificate) throws CertificateParsingException { + if (!(certificate instanceof X509Certificate)) { + return false; + } + X509Certificate x509Certificate = (X509Certificate) certificate; + + // Check for alternative names + Collection> altNames = x509Certificate.getSubjectAlternativeNames(); + if (CollectionUtils.isEmpty(altNames)) { + return false; + } + for (List altName : altNames) { + int type = (Integer) altName.get(0); + String name = (String) altName.get(1); + if (type == GeneralName.dNSName && managementCertificateCustomSAN.equals(name)) { + return true; + } + } + return false; + } } diff --git a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java index 15514b91c785..8311f4d45abc 100644 --- a/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java +++ b/plugins/ca/root-ca/src/test/java/org/apache/cloudstack/ca/provider/RootCAProviderTest.java @@ -26,8 +26,13 @@ import java.security.NoSuchProviderException; import java.security.SignatureException; import java.security.cert.CertificateException; +import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.UUID; import javax.net.ssl.SSLEngine; @@ -35,15 +40,16 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.utils.security.CertUtils; import org.apache.cloudstack.utils.security.SSLUtils; +import org.bouncycastle.asn1.x509.GeneralName; import org.joda.time.DateTime; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; - -import org.mockito.junit.MockitoJUnitRunner; import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; @RunWith(MockitoJUnitRunner.class) @@ -150,4 +156,56 @@ public void testGetProviderName() throws Exception { Assert.assertEquals(provider.getProviderName(), "root"); } + @Test + public void testIsManagementCertificateNotX509() { + try { + Assert.assertFalse(provider.isManagementCertificate(Mockito.mock(java.security.cert.Certificate.class))); + } catch (CertificateParsingException e) { + Assert.fail(String.format("Exception occurred: %s", e.getMessage())); + } + } + + @Test + public void testIsManagementCertificateNoAltNames() { + try { + X509Certificate certificate = Mockito.mock(X509Certificate.class); + Mockito.when(certificate.getSubjectAlternativeNames()).thenReturn(new ArrayList<>()); + Assert.assertFalse(provider.isManagementCertificate(certificate)); + } catch (CertificateParsingException e) { + Assert.fail(String.format("Exception occurred: %s", e.getMessage())); + } + } + + @Test + public void testIsManagementCertificateNoMatch() { + ReflectionTestUtils.setField(provider, "managementCertificateCustomSAN", "cloudstack"); + try { + X509Certificate certificate = Mockito.mock(X509Certificate.class); + List> altNames = new ArrayList<>(); + altNames.add(List.of(GeneralName.dNSName, UUID.randomUUID().toString())); + altNames.add(List.of(GeneralName.dNSName, UUID.randomUUID().toString())); + Collection> collection = new ArrayList<>(altNames); + Mockito.when(certificate.getSubjectAlternativeNames()).thenReturn(collection); + Assert.assertFalse(provider.isManagementCertificate(certificate)); + } catch (CertificateParsingException e) { + Assert.fail(String.format("Exception occurred: %s", e.getMessage())); + } + } + + @Test + public void testIsManagementCertificateMatch() { + String customSAN = "cloudstack"; + ReflectionTestUtils.setField(provider, "managementCertificateCustomSAN", customSAN); + try { + X509Certificate certificate = Mockito.mock(X509Certificate.class); + List> altNames = new ArrayList<>(); + altNames.add(List.of(GeneralName.dNSName, customSAN)); + altNames.add(List.of(GeneralName.dNSName, UUID.randomUUID().toString())); + Collection> collection = new ArrayList<>(altNames); + Mockito.when(certificate.getSubjectAlternativeNames()).thenReturn(collection); + Assert.assertTrue(provider.isManagementCertificate(certificate)); + } catch (CertificateParsingException e) { + Assert.fail(String.format("Exception occurred: %s", e.getMessage())); + } + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java index 4c80a99cb84f..cc955e86d8a4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java @@ -18,6 +18,24 @@ // package com.cloud.hypervisor.kvm.resource.wrapper; +import java.io.BufferedInputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URLEncoder; +import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; + +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.vm.UnmanagedInstanceTO; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.ConvertInstanceAnswer; import com.cloud.agent.api.ConvertInstanceCommand; @@ -34,27 +52,11 @@ import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import com.cloud.storage.Storage; +import com.cloud.utils.FileUtil; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.OutputInterpreter; import com.cloud.utils.script.Script; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; -import org.apache.cloudstack.vm.UnmanagedInstanceTO; -import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.io.IOUtils; -import org.apache.commons.lang3.StringUtils; - -import java.io.BufferedInputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.URLEncoder; -import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.List; -import java.util.UUID; -import java.util.stream.Collectors; @ResourceWrapper(handles = ConvertInstanceCommand.class) public class LibvirtConvertInstanceCommandWrapper extends CommandWrapper { @@ -161,7 +163,7 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve if (ovfExported && StringUtils.isNotBlank(ovfTemplateDirOnConversionLocation)) { String sourceOVFDir = String.format("%s/%s", temporaryConvertPath, ovfTemplateDirOnConversionLocation); logger.debug("Cleaning up exported OVA at dir " + sourceOVFDir); - Script.runSimpleBashScript("rm -rf " + sourceOVFDir); + FileUtil.deletePath(sourceOVFDir); } if (conversionTemporaryLocation instanceof NfsTO) { logger.debug("Cleaning up secondary storage temporary location"); @@ -253,7 +255,7 @@ private void cleanupDisksAndDomainFromTemporaryLocation(List di temporaryStoragePool.deletePhysicalDisk(disk.getName(), Storage.ImageFormat.QCOW2); } logger.info(String.format("Cleaning up temporary domain %s after conversion from temporary location", temporaryConvertUuid)); - Script.runSimpleBashScript(String.format("rm -f %s/%s*.xml", temporaryStoragePool.getLocalPath(), temporaryConvertUuid)); + FileUtil.deleteFiles(temporaryStoragePool.getLocalPath(), temporaryConvertUuid, ".xml"); } protected void sanitizeDisksPath(List disks) { @@ -356,7 +358,10 @@ protected List getUnmanagedInstanceDisks(List getNfsStoragePoolHostAndPath(KVMStoragePool storagePool) { String sourceHostIp = null; String sourcePath = null; - String storagePoolMountPoint = Script.runSimpleBashScript(String.format("mount | grep %s", storagePool.getLocalPath())); + List commands = new ArrayList<>(); + commands.add(new String[]{Script.getExecutableAbsolutePath("mount")}); + commands.add(new String[]{Script.getExecutableAbsolutePath("grep"), storagePool.getLocalPath()}); + String storagePoolMountPoint = Script.executePipedCommands(commands, 0).second(); logger.debug(String.format("NFS Storage pool: %s - local path: %s, mount point: %s", storagePool.getUuid(), storagePool.getLocalPath(), storagePoolMountPoint)); if (StringUtils.isNotEmpty(storagePoolMountPoint)) { String[] res = storagePoolMountPoint.strip().split(" "); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java index 45b0c179938c..58a74d6e0f61 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java @@ -19,6 +19,9 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.util.ArrayList; +import java.util.List; + import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.libvirt.Connect; @@ -35,8 +38,8 @@ import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; -import com.cloud.storage.Volume; import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.Volume; import com.cloud.utils.script.Script; @ResourceWrapper(handles = DeleteVMSnapshotCommand.class) @@ -94,12 +97,20 @@ public Answer execute(final DeleteVMSnapshotCommand cmd, final LibvirtComputingR PrimaryDataStoreTO primaryStore = (PrimaryDataStoreTO) rootVolume.getDataStore(); KVMPhysicalDisk rootDisk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), rootVolume.getPath()); - String qemu_img_snapshot = Script.runSimpleBashScript("qemu-img snapshot -l " + rootDisk.getPath() + " | tail -n +3 | awk -F ' ' '{print $2}' | grep ^" + cmd.getTarget().getSnapshotName() + "$"); + String qemuImgPath = Script.getExecutableAbsolutePath("qemu-img"); + List commands = new ArrayList<>(); + commands.add(new String[]{qemuImgPath, "snapshot", "-l", sanitizeBashCommandArgument(rootDisk.getPath())}); + commands.add(new String[]{Script.getExecutableAbsolutePath("tail"), "-n", "+3"}); + commands.add(new String[]{Script.getExecutableAbsolutePath("awk"), "-F", " ", "{print $2}"}); + commands.add(new String[]{Script.getExecutableAbsolutePath("grep"), "^" + sanitizeBashCommandArgument(cmd.getTarget().getSnapshotName()) + "$"}); + String qemu_img_snapshot = Script.executePipedCommands(commands, 0).second(); if (qemu_img_snapshot == null) { logger.info("Cannot find snapshot " + cmd.getTarget().getSnapshotName() + " in file " + rootDisk.getPath() + ", return true"); return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs()); } - int result = Script.runSimpleBashScriptForExitValue("qemu-img snapshot -d " + cmd.getTarget().getSnapshotName() + " " + rootDisk.getPath()); + int result = Script.executeCommandForExitValue(qemuImgPath, "snapshot", "-d", + sanitizeBashCommandArgument(cmd.getTarget().getSnapshotName()), + sanitizeBashCommandArgument(rootDisk.getPath())); if (result != 0) { return new DeleteVMSnapshotAnswer(cmd, false, "Delete VM Snapshot Failed due to can not remove snapshot from image file " + rootDisk.getPath() + " : " + result); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java index 227e68872dac..61c20f96bacc 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java @@ -19,6 +19,9 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.util.ArrayList; +import java.util.List; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.GetVmIpAddressCommand; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; @@ -35,31 +38,51 @@ public final class LibvirtGetVmIpAddressCommandWrapper extends CommandWrapper commands = new ArrayList<>(); + final String virt_ls_path = Script.getExecutableAbsolutePath("virt-ls"); + final String virt_cat_path = Script.getExecutableAbsolutePath("virt-cat"); + final String virt_win_reg_path = Script.getExecutableAbsolutePath("virt-win-reg"); + final String tail_path = Script.getExecutableAbsolutePath("tail"); + final String grep_path = Script.getExecutableAbsolutePath("grep"); + final String awk_path = Script.getExecutableAbsolutePath("awk"); + final String sed_path = Script.getExecutableAbsolutePath("sed"); if(!command.isWindows()) { //List all dhcp lease files inside guestVm - String leasesList = Script.runSimpleBashScript(new StringBuilder().append("virt-ls ").append(command.getVmName()) - .append(" /var/lib/dhclient/ | grep .*\\*.leases").toString()); + commands.add(new String[]{virt_ls_path, sanitizedVmName, "/var/lib/dhclient/"}); + commands.add(new String[]{grep_path, ".*\\*.leases"}); + String leasesList = Script.executePipedCommands(commands, 0).second(); if(leasesList != null) { String[] leasesFiles = leasesList.split("\n"); for(String leaseFile : leasesFiles){ - //Read from each dhclient lease file inside guest Vm using virt-cat libguestfs ulitiy - 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()); + //Read from each dhclient lease file inside guest Vm using virt-cat libguestfs utility + commands = new ArrayList<>(); + commands.add(new String[]{virt_cat_path, sanitizedVmName, "/var/lib/dhclient/" + leaseFile}); + commands.add(new String[]{tail_path, "-16"}); + commands.add(new String[]{grep_path, "fixed-address"}); + commands.add(new String[]{awk_path, "{print $2}"}); + commands.add(new String[]{sed_path, "-e", "s/;//"}); + String ipAddr = Script.executePipedCommands(commands, 0).second(); // Check if the IP belongs to the network - if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)){ + if((ipAddr != null) && NetUtils.isIpWithInCidrRange(ipAddr, networkCidr)) { ip = ipAddr; break; } - logger.debug("GetVmIp: "+command.getVmName()+ " Ip: "+ipAddr+" does not belong to network "+networkCidr); + logger.debug("GetVmIp: "+ vmName + " Ip: "+ipAddr+" does not belong to network "+networkCidr); } } } else { // For windows, read from guest Vm registry using virt-win-reg libguestfs ulitiy. Registry Path: HKEY_LOCAL_MACHINE\SYSTEM\ControlSet001\Services\Tcpip\Parameters\Interfaces\\DhcpIPAddress - String ipList = Script.runSimpleBashScript(new StringBuilder().append("virt-win-reg --unsafe-printable-strings ").append(command.getVmName()) - .append(" 'HKEY_LOCAL_MACHINE\\SYSTEM\\ControlSet001\\Services\\Tcpip\\Parameters\\Interfaces' | grep DhcpIPAddress | awk -F : '{print $2}' | sed -e 's/^\"//' -e 's/\"$//'").toString()); + commands = new ArrayList<>(); + commands.add(new String[]{virt_win_reg_path, "--unsafe-printable-strings", sanitizedVmName, "HKEY_LOCAL_MACHINE\\SYSTEM\\ControlSet001\\Services\\Tcpip\\Parameters\\Interfaces"}); + commands.add(new String[]{grep_path, "DhcpIPAddress"}); + commands.add(new String[]{awk_path, "-F", ":", "{print $2}"}); + commands.add(new String[]{sed_path, "-e", "s/^\"//", "-e", "s/\"$//"}); + String ipList = Script.executePipedCommands(commands, 0).second(); if(ipList != null) { - logger.debug("GetVmIp: "+command.getVmName()+ "Ips: "+ipList); + logger.debug("GetVmIp: "+ vmName + "Ips: "+ipList); String[] ips = ipList.split("\n"); for (String ipAddr : ips){ // Check if the IP belongs to the network @@ -67,13 +90,13 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin ip = ipAddr; break; } - logger.debug("GetVmIp: "+command.getVmName()+ " Ip: "+ipAddr+" does not belong to network "+networkCidr); + logger.debug("GetVmIp: "+ vmName + " Ip: "+ipAddr+" does not belong to network "+networkCidr); } } } if(ip != null){ result = true; - logger.debug("GetVmIp: "+command.getVmName()+ " Found Ip: "+ip); + logger.debug("GetVmIp: "+ vmName + " Found Ip: "+ip); } return new Answer(command, result, ip); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java index db07cc5291a0..923c44f48285 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapper.java @@ -19,7 +19,11 @@ package com.cloud.hypervisor.kvm.resource.wrapper; -import org.apache.commons.lang3.StringUtils; +import java.net.InetAddress; +import java.net.InterfaceAddress; +import java.net.NetworkInterface; +import java.net.SocketException; +import java.util.Enumeration; import com.cloud.agent.api.Answer; import com.cloud.agent.api.OvsFetchInterfaceAnswer; @@ -27,32 +31,72 @@ import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; -import com.cloud.utils.script.Script; +import com.cloud.utils.Ternary; @ResourceWrapper(handles = OvsFetchInterfaceCommand.class) public final class LibvirtOvsFetchInterfaceCommandWrapper extends CommandWrapper { + private String getSubnetMaskForAddress(NetworkInterface networkInterface, InetAddress inetAddress) { + for (InterfaceAddress address : networkInterface.getInterfaceAddresses()) { + if (!inetAddress.equals(address.getAddress())) { + continue; + } + int prefixLength = address.getNetworkPrefixLength(); + int mask = 0xffffffff << (32 - prefixLength); + return String.format("%d.%d.%d.%d", + (mask >>> 24) & 0xff, + (mask >>> 16) & 0xff, + (mask >>> 8) & 0xff, + mask & 0xff); + } + return ""; + } + + private String getMacAddress(NetworkInterface networkInterface) throws SocketException { + byte[] macBytes = networkInterface.getHardwareAddress(); + if (macBytes == null) { + return ""; + } + StringBuilder macAddress = new StringBuilder(); + for (byte b : macBytes) { + macAddress.append(String.format("%02X:", b)); + } + if (macAddress.length() > 0) { + macAddress.deleteCharAt(macAddress.length() - 1); // Remove trailing colon + } + return macAddress.toString(); + } + + public Ternary getInterfaceDetails(String interfaceName) throws SocketException { + NetworkInterface networkInterface = NetworkInterface.getByName(interfaceName); + if (networkInterface == null) { + logger.warn(String.format("Network interface: '%s' not found", interfaceName)); + return new Ternary<>(null, null, null); + } + Enumeration inetAddresses = networkInterface.getInetAddresses(); + while (inetAddresses.hasMoreElements()) { + InetAddress inetAddress = inetAddresses.nextElement(); + if (inetAddress instanceof java.net.Inet4Address) { + String ipAddress = inetAddress.getHostAddress(); + String subnetMask = getSubnetMaskForAddress(networkInterface, inetAddress); + String macAddress = getMacAddress(networkInterface); + return new Ternary<>(ipAddress, subnetMask, macAddress); + } + } + return new Ternary<>(null, null, null); + } + @Override public Answer execute(final OvsFetchInterfaceCommand command, final LibvirtComputingResource libvirtComputingResource) { - final String label = command.getLabel(); + final String label = "'" + command.getLabel() + "'"; logger.debug("Will look for network with name-label:" + label); try { - String ipadd = Script.runSimpleBashScript("ifconfig " + label + " | grep 'inet addr:' | cut -d: -f2 | awk '{ print $1}'"); - if (StringUtils.isEmpty(ipadd)) { - ipadd = Script.runSimpleBashScript("ifconfig " + label + " | grep ' inet ' | awk '{ print $2}'"); - } - String mask = Script.runSimpleBashScript("ifconfig " + label + " | grep 'inet addr:' | cut -d: -f4"); - if (StringUtils.isEmpty(mask)) { - mask = Script.runSimpleBashScript("ifconfig " + label + " | grep ' inet ' | awk '{ print $4}'"); - } - String mac = Script.runSimpleBashScript("ifconfig " + label + " | grep HWaddr | awk -F \" \" '{print $5}'"); - if (StringUtils.isEmpty(mac)) { - mac = Script.runSimpleBashScript("ifconfig " + label + " | grep ' ether ' | awk '{ print $2}'"); - } + Ternary interfaceDetails = getInterfaceDetails(label); return new OvsFetchInterfaceAnswer(command, true, "Interface " + label - + " retrieved successfully", ipadd, mask, mac); + + " retrieved successfully", interfaceDetails.first(), interfaceDetails.second(), + interfaceDetails.third()); } catch (final Exception e) { logger.warn("Caught execption when fetching interface", e); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java index c8b205113465..535494877043 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java @@ -46,7 +46,6 @@ import com.cloud.resource.ResourceWrapper; import com.cloud.storage.Volume; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; @ResourceWrapper(handles = PrepareForMigrationCommand.class) public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapper { @@ -127,9 +126,7 @@ public Answer execute(final PrepareForMigrationCommand command, final LibvirtCom } catch (final LibvirtException | CloudRuntimeException | InternalErrorException | URISyntaxException e) { if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { for (DpdkTO to : dpdkInterfaceMapping.values()) { - String cmd = String.format("ovs-vsctl del-port %s", to.getPort()); - logger.debug("Removing DPDK port: " + to.getPort()); - Script.runSimpleBashScript(cmd); + removeDpdkPort(to.getPort()); } } return new PrepareForMigrationAnswer(command, e.toString()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java index 0b0f69f3eed5..8f23e79e4a32 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java @@ -19,7 +19,9 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import com.cloud.agent.api.Answer; @@ -33,7 +35,6 @@ import com.cloud.resource.ResourceWrapper; import com.cloud.utils.script.Script; - @ResourceWrapper(handles = ReadyCommand.class) public final class LibvirtReadyCommandWrapper extends CommandWrapper { @@ -50,13 +51,18 @@ public Answer execute(final ReadyCommand command, final LibvirtComputingResource } private boolean hostSupportsUefi(boolean isUbuntuHost) { - String cmd = "rpm -qa | grep -i ovmf"; int timeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.AGENT_SCRIPT_TIMEOUT) * 1000; // Get property value & convert to milliseconds + int result; if (isUbuntuHost) { - cmd = "dpkg -l ovmf"; + logger.debug("Running command : [dpkg -l ovmf] with timeout : " + timeout + " ms"); + result = Script.executeCommandForExitValue(timeout, Script.getExecutableAbsolutePath("dpkg"), "-l", "ovmf"); + } else { + logger.debug("Running command : [rpm -qa | grep -i ovmf] with timeout : " + timeout + " ms"); + List commands = new ArrayList<>(); + commands.add(new String[]{Script.getExecutableAbsolutePath("rpm"), "-qa"}); + commands.add(new String[]{Script.getExecutableAbsolutePath("grep"), "-i", "ovmf"}); + result = Script.executePipedCommands(commands, timeout).first(); } - logger.debug("Running command : [" + cmd + "] with timeout : " + timeout + " ms"); - int result = Script.runSimpleBashScriptForExitValue(cmd, timeout, false); logger.debug("Got result : " + result); return result == 0; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java index 348151557110..9919689cf3b4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevokeDirectDownloadCertificateWrapper.java @@ -19,6 +19,14 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; + +import org.apache.cloudstack.agent.directdownload.RevokeDirectDownloadCertificateCommand; +import org.apache.cloudstack.utils.security.KeyStoreUtils; +import org.apache.commons.lang3.StringUtils; + import com.cloud.agent.api.Answer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; @@ -26,13 +34,6 @@ import com.cloud.utils.PropertiesUtil; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; -import org.apache.cloudstack.agent.directdownload.RevokeDirectDownloadCertificateCommand; -import org.apache.cloudstack.utils.security.KeyStoreUtils; -import org.apache.commons.lang3.StringUtils; - -import java.io.File; -import java.io.FileNotFoundException; -import java.io.IOException; @ResourceWrapper(handles = RevokeDirectDownloadCertificateCommand.class) public class LibvirtRevokeDirectDownloadCertificateWrapper extends CommandWrapper { @@ -82,17 +83,17 @@ public Answer execute(RevokeDirectDownloadCertificateCommand command, LibvirtCom } final String keyStoreFile = getKeyStoreFilePath(agentFile); - - String checkCmd = String.format("keytool -list -alias %s -keystore %s -storepass %s", - certificateAlias, keyStoreFile, privatePassword); - int existsCmdResult = Script.runSimpleBashScriptForExitValue(checkCmd); + String keyToolPath = Script.getExecutableAbsolutePath("keytool"); + int existsCmdResult = Script.executeCommandForExitValue(keyToolPath, "-list", "-alias", + sanitizeBashCommandArgument(certificateAlias), "-keystore", keyStoreFile, "-storepass", + privatePassword); if (existsCmdResult == 1) { logger.error("Certificate alias " + certificateAlias + " does not exist, no need to revoke it"); } else { - String revokeCmd = String.format("keytool -delete -alias %s -keystore %s -storepass %s", - certificateAlias, keyStoreFile, privatePassword); logger.debug("Revoking certificate alias " + certificateAlias + " from keystore " + keyStoreFile); - Script.runSimpleBashScriptForExitValue(revokeCmd); + Script.executeCommandForExitValue(keyToolPath, "-delete", "-alias", + sanitizeBashCommandArgument(certificateAlias), "-keystore", keyStoreFile, "-storepass", + privatePassword); } } catch (FileNotFoundException | CloudRuntimeException e) { logger.error("Error while setting up certificate " + certificateAlias, e); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java index eb4e6be7609e..fcca16ba6186 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java @@ -18,20 +18,25 @@ // package com.cloud.hypervisor.kvm.resource.wrapper; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand; +import org.apache.cloudstack.utils.security.KeyStoreUtils; +import org.apache.commons.lang3.StringUtils; + import com.cloud.agent.api.Answer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.FileUtil; import com.cloud.utils.PropertiesUtil; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; -import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand; -import org.apache.cloudstack.utils.security.KeyStoreUtils; -import org.apache.commons.lang3.StringUtils; - -import java.io.File; -import java.io.FileNotFoundException; -import java.io.IOException; @ResourceWrapper(handles = SetupDirectDownloadCertificateCommand.class) public class LibvirtSetupDirectDownloadCertificateCommandWrapper extends CommandWrapper { @@ -77,9 +82,10 @@ private String getKeyStoreFilePath(File agentFile) { */ private void importCertificate(String tempCerFilePath, String keyStoreFile, String certificateName, String privatePassword) { logger.debug("Importing certificate from temporary file to keystore"); - String importCommandFormat = "keytool -importcert -file %s -keystore %s -alias '%s' -storepass '%s' -noprompt"; - String importCmd = String.format(importCommandFormat, tempCerFilePath, keyStoreFile, certificateName, privatePassword); - int result = Script.runSimpleBashScriptForExitValue(importCmd); + String keyToolPath = Script.getExecutableAbsolutePath("keytool"); + int result = Script.executeCommandForExitValue(keyToolPath, "-importcert", "file", tempCerFilePath, + "-keystore", keyStoreFile, "-alias", sanitizeBashCommandArgument(certificateName), "-storepass", + privatePassword, "-noprompt"); if (result != 0) { logger.debug("Certificate " + certificateName + " not imported as it already exist on keystore"); } @@ -92,8 +98,7 @@ private String createTemporaryFile(File agentFile, String certificateName, Strin String tempCerFilePath = String.format("%s/%s-%s", agentFile.getParent(), temporaryCertFilePrefix, certificateName); logger.debug("Creating temporary certificate file into: " + tempCerFilePath); - int result = Script.runSimpleBashScriptForExitValue(String.format("echo '%s' > %s", certificate, tempCerFilePath)); - if (result != 0) { + if (!FileUtil.writeToFile(tempCerFilePath, certificate)) { throw new CloudRuntimeException("Could not create the certificate file on path: " + tempCerFilePath); } return tempCerFilePath; @@ -102,9 +107,24 @@ private String createTemporaryFile(File agentFile, String certificateName, Strin /** * Remove temporary file */ - private void cleanupTemporaryFile(String temporaryFile) { + + protected void cleanupTemporaryFile(String temporaryFile) { logger.debug("Cleaning up temporary certificate file"); - Script.runSimpleBashScript("rm -f " + temporaryFile); + if (StringUtils.isBlank(temporaryFile)) { + logger.debug("Provided temporary certificate file path is empty"); + return; + } + try { + Path filePath = Paths.get(temporaryFile); + if (!Files.exists(filePath)) { + logger.debug("Temporary certificate file does not exist: " + temporaryFile); + return; + } + Files.delete(filePath); + } catch (IOException e) { + logger.warn(String.format("Error while cleaning up temporary file: %s", temporaryFile)); + logger.debug(String.format("Error while cleaning up temporary file: %s", temporaryFile), e); + } } @Override diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java index 8b3942f5ebad..1be5a1e949e5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java @@ -23,27 +23,26 @@ import java.util.List; import java.util.Map; -import com.cloud.agent.api.to.DpdkTO; -import com.cloud.hypervisor.kvm.resource.LibvirtKvmAgentHook; -import com.cloud.utils.Pair; -import com.cloud.utils.script.Script; -import com.cloud.utils.ssh.SshHelper; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.DomainInfo.DomainState; +import org.libvirt.LibvirtException; import com.cloud.agent.api.Answer; import com.cloud.agent.api.StopAnswer; import com.cloud.agent.api.StopCommand; +import com.cloud.agent.api.to.DpdkTO; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtKvmAgentHook; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef; import com.cloud.hypervisor.kvm.resource.VifDriver; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; -import org.libvirt.LibvirtException; +import com.cloud.utils.Pair; +import com.cloud.utils.ssh.SshHelper; @ResourceWrapper(handles = StopCommand.class) public final class LibvirtStopCommandWrapper extends CommandWrapper { @@ -119,10 +118,7 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource Map dpdkInterfaceMapping = command.getDpdkInterfaceMapping(); if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { for (DpdkTO to : dpdkInterfaceMapping.values()) { - String portToRemove = to.getPort(); - String cmd = String.format("ovs-vsctl del-port %s", portToRemove); - logger.debug("Removing DPDK port: " + portToRemove); - Script.runSimpleBashScript(cmd); + removeDpdkPort(to.getPort()); } } } else { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java index 79cd55763db0..f0e94e59485d 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java @@ -18,6 +18,23 @@ // package com.cloud.hypervisor.kvm.resource.wrapper; +import java.util.Arrays; +import java.util.List; +import java.util.UUID; + +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.vm.UnmanagedInstanceTO; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockedConstruction; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.ConvertInstanceCommand; import com.cloud.agent.api.to.NfsTO; @@ -32,22 +49,6 @@ import com.cloud.storage.Storage; import com.cloud.utils.Pair; import com.cloud.utils.script.Script; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; -import org.apache.cloudstack.vm.UnmanagedInstanceTO; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.MockedConstruction; -import org.mockito.MockedStatic; -import org.mockito.Mockito; -import org.mockito.Spy; -import org.mockito.junit.MockitoJUnitRunner; - -import java.util.Arrays; -import java.util.List; -import java.util.UUID; @RunWith(MockitoJUnitRunner.class) public class LibvirtConvertInstanceCommandWrapperTest { @@ -186,26 +187,31 @@ public void testMoveTemporaryDisksToDestination() { @Test public void testGetUnmanagedInstanceDisks() { - String relativePath = UUID.randomUUID().toString(); - LibvirtVMDef.DiskDef diskDef = new LibvirtVMDef.DiskDef(); - LibvirtVMDef.DiskDef.DiskBus bus = LibvirtVMDef.DiskDef.DiskBus.IDE; - LibvirtVMDef.DiskDef.DiskFmtType type = LibvirtVMDef.DiskDef.DiskFmtType.QCOW2; - diskDef.defFileBasedDisk(relativePath, relativePath, bus, type); - - KVMPhysicalDisk sourceDisk = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(sourceDisk.getName()).thenReturn(UUID.randomUUID().toString()); - Mockito.when(sourceDisk.getPool()).thenReturn(destinationPool); - Mockito.when(destinationPool.getType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); - List disks = List.of(sourceDisk); - - LibvirtDomainXMLParser parser = Mockito.mock(LibvirtDomainXMLParser.class); - Mockito.when(parser.getDisks()).thenReturn(List.of(diskDef)); - Mockito.doReturn(new Pair(null, null)).when(convertInstanceCommandWrapper).getNfsStoragePoolHostAndPath(destinationPool); - - List unmanagedInstanceDisks = convertInstanceCommandWrapper.getUnmanagedInstanceDisks(disks, parser); - Assert.assertEquals(1, unmanagedInstanceDisks.size()); - UnmanagedInstanceTO.Disk disk = unmanagedInstanceDisks.get(0); - Assert.assertEquals(LibvirtVMDef.DiskDef.DiskBus.IDE.toString(), disk.getController()); + try (MockedStatic