github-advanced-security[bot] commented on code in PR #1069: URL: https://github.com/apache/tomcat/pull/1069#discussion_r4120339951
########## modules/manager2/src/test/java/org/apache/tomcat/manager2/Manager2ConfigTestBase.java: ########## @@ -0,0 +1,639 @@ +/* + * 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 org.apache.tomcat.manager2; + +import java.io.File; +import java.io.PrintWriter; +import java.net.ServerSocket; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.security.cert.X509Certificate; +import java.util.List; +import java.util.Map; + +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; + +import org.junit.After; +import org.junit.Assert; + +import static org.apache.catalina.startup.SimpleHttpClient.CRLF; +import org.apache.catalina.Context; +import org.apache.catalina.core.StandardEngine; +import org.apache.catalina.realm.MemoryRealm; +import org.apache.catalina.servlets.DefaultServlet; +import org.apache.catalina.startup.ExpandWar; +import org.apache.catalina.startup.SimpleHttpClient; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.json.JSONParser; + + +/** + * Shared base for the integration tests of the manager2 configuration API ({@code /api/config/*}). The tests deploy + * the {@code manager2.war} built by this module (via the {@code deploy} target) into a throw-away Tomcat instance and + * drive it over HTTP with {@link SimpleHttpClient}, exercising the component tree, attribute updates, structural + * add/remove of child components, lifecycle operations (start / stop / restart), and persistence to + * {@code server.xml} through storeconfig. + * + * <p> + * The test methods are split over the concrete subclasses so that they can run in parallel: the Ant JUnit task + * parallelizes at the granularity of a test class (the methods of one class always run sequentially in a single + * thread), so one large class would serialize the whole configuration suite. The name of this base class deliberately + * does not start with {@code Test}: the batch test of the module build only picks up {@code Test*.java} (the + * {@code skipNonTests} option is a second line of defense). + * + * <p> + * All mutable state lives under {@link #getTemporaryDirectory()} (unique per test class): the webapp is deployed from + * a private copy so that the store tests can rewrite the manager's own context file without restarting the contexts + * of the other test classes, and the {@code manager2.store.base} system property is set per method and cleared in the + * {@code @After} tear down. + */ +public abstract class Manager2ConfigTestBase extends TomcatBaseTest { + + protected static final String MANAGER2 = "/manager2"; + + private String storeBaseProp = null; + + protected File manager2DocBase = null; + + + @After + public void clearStoreBase() { + if (storeBaseProp != null) { + System.clearProperty("manager2.store.base"); + storeBaseProp = null; + } + } + + + /** + * Fetch the node detail of the component with the given id. + * + * @param client The client to use + * @param id The component id + * + * @return The parsed node detail + */ + protected Map<String, Object> fetchNode(SimpleHttpClient client, String id) throws Exception { + request(client, "GET", MANAGER2 + "/api/config/node/" + id, null, null, 200); + return parseObject(client.getResponseBody()); + } + + + /** + * Count the direct children of a node with the given type. + * + * @param node The node + * @param type The child type to count + * + * @return The number of direct children of the given type + */ + protected static long countChildrenOfType(Map<String, Object> node, String type) { + List<Object> children = getList(node, "children"); + if (children == null) { + return 0; + } + long count = 0; + for (Object child : children) { + @SuppressWarnings("unchecked") + Map<String, Object> cm = (Map<String, Object>) child; + if (type.equals(cm.get("type"))) { + count++; + } + } + return count; + } + + + // ----------------------------------------------------------------------- + + + protected void setStoreBase(File storeBase) { + System.setProperty("manager2.store.base", storeBase.getAbsolutePath()); + storeBaseProp = storeBase.getAbsolutePath(); + } + + + /** + * Best effort cleanup for the add/remove test if a removal failed midway: remove any components that are still + * present so the shared instance and subsequent tests are not affected. + * + * @param client The client to use + * @param token The CSRF token + * @param serviceId Id of the service to remove, if any + * @param hostId Id of the host to remove, if any + * @param contextId Id of the context to remove, if any + * @param wrapperId Id of the wrapper to remove, if any + * @param valveId Id of the valve to remove, if any + * @param connectorId Id of the connector to remove, if any + * @param executorId Id of the executor to remove, if any + * @param aliasId Id of the alias to remove, if any + * + * @throws Exception If the cleanup requests cannot be exchanged + */ + protected void cleanup(SimpleHttpClient client, String token, String serviceId, String hostId, String contextId, + String wrapperId, String valveId, String connectorId, String executorId, String aliasId) throws Exception { + for (String id : new String[] { connectorId, executorId, wrapperId, valveId, aliasId, contextId, hostId, + serviceId }) { + if (id == null) { + continue; + } + try { + request(client, "DELETE", MANAGER2 + "/api/config/child", token, + "{\"id\":\"" + id + "\",\"confirm\":\"confirm\"}", 200); + } catch (AssertionError e) { + // Already removed (or never created): ignore. + } + } + } + + + protected static int freePort() throws Exception { + try (ServerSocket socket = new ServerSocket(0)) { + return socket.getLocalPort(); + } + } + + + /** + * Issue an HTTPS GET against the given port with a trust-all trust manager (the test certificate is self signed). + * Returns the HTTP status code; any status proves that the TLS handshake succeeded and the connector served the + * request. + * + * @param port The port to connect to + * @param path The request path + * + * @return The HTTP status code + * + * @throws Exception If the request fails + */ + protected static int httpsGet(int port, String path) throws Exception { + TrustManager[] trustAll = new TrustManager[] { new X509TrustManager() { + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) { + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) { + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + } }; + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, trustAll, new SecureRandom()); + HttpsURLConnection connection = (HttpsURLConnection) URI.create("https://localhost:"; + port + path).toURL() + .openConnection(); + connection.setSSLSocketFactory(sslContext.getSocketFactory()); + connection.setHostnameVerifier((hostname, session) -> true); Review Comment: ## CodeQL / Unsafe hostname verification The [hostname verifier](1) defined by [this type](2) always accepts any certificate, even if the hostname does not match. [Show more details](https://github.com/apache/tomcat/security/code-scanning/978) ########## modules/manager2/src/test/java/org/apache/tomcat/manager2/Manager2ConfigTestBase.java: ########## @@ -0,0 +1,639 @@ +/* + * 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 org.apache.tomcat.manager2; + +import java.io.File; +import java.io.PrintWriter; +import java.net.ServerSocket; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.security.cert.X509Certificate; +import java.util.List; +import java.util.Map; + +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; + +import org.junit.After; +import org.junit.Assert; + +import static org.apache.catalina.startup.SimpleHttpClient.CRLF; +import org.apache.catalina.Context; +import org.apache.catalina.core.StandardEngine; +import org.apache.catalina.realm.MemoryRealm; +import org.apache.catalina.servlets.DefaultServlet; +import org.apache.catalina.startup.ExpandWar; +import org.apache.catalina.startup.SimpleHttpClient; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.json.JSONParser; + + +/** + * Shared base for the integration tests of the manager2 configuration API ({@code /api/config/*}). The tests deploy + * the {@code manager2.war} built by this module (via the {@code deploy} target) into a throw-away Tomcat instance and + * drive it over HTTP with {@link SimpleHttpClient}, exercising the component tree, attribute updates, structural + * add/remove of child components, lifecycle operations (start / stop / restart), and persistence to + * {@code server.xml} through storeconfig. + * + * <p> + * The test methods are split over the concrete subclasses so that they can run in parallel: the Ant JUnit task + * parallelizes at the granularity of a test class (the methods of one class always run sequentially in a single + * thread), so one large class would serialize the whole configuration suite. The name of this base class deliberately + * does not start with {@code Test}: the batch test of the module build only picks up {@code Test*.java} (the + * {@code skipNonTests} option is a second line of defense). + * + * <p> + * All mutable state lives under {@link #getTemporaryDirectory()} (unique per test class): the webapp is deployed from + * a private copy so that the store tests can rewrite the manager's own context file without restarting the contexts + * of the other test classes, and the {@code manager2.store.base} system property is set per method and cleared in the + * {@code @After} tear down. + */ +public abstract class Manager2ConfigTestBase extends TomcatBaseTest { + + protected static final String MANAGER2 = "/manager2"; + + private String storeBaseProp = null; + + protected File manager2DocBase = null; + + + @After + public void clearStoreBase() { + if (storeBaseProp != null) { + System.clearProperty("manager2.store.base"); + storeBaseProp = null; + } + } + + + /** + * Fetch the node detail of the component with the given id. + * + * @param client The client to use + * @param id The component id + * + * @return The parsed node detail + */ + protected Map<String, Object> fetchNode(SimpleHttpClient client, String id) throws Exception { + request(client, "GET", MANAGER2 + "/api/config/node/" + id, null, null, 200); + return parseObject(client.getResponseBody()); + } + + + /** + * Count the direct children of a node with the given type. + * + * @param node The node + * @param type The child type to count + * + * @return The number of direct children of the given type + */ + protected static long countChildrenOfType(Map<String, Object> node, String type) { + List<Object> children = getList(node, "children"); + if (children == null) { + return 0; + } + long count = 0; + for (Object child : children) { + @SuppressWarnings("unchecked") + Map<String, Object> cm = (Map<String, Object>) child; + if (type.equals(cm.get("type"))) { + count++; + } + } + return count; + } + + + // ----------------------------------------------------------------------- + + + protected void setStoreBase(File storeBase) { + System.setProperty("manager2.store.base", storeBase.getAbsolutePath()); + storeBaseProp = storeBase.getAbsolutePath(); + } + + + /** + * Best effort cleanup for the add/remove test if a removal failed midway: remove any components that are still + * present so the shared instance and subsequent tests are not affected. + * + * @param client The client to use + * @param token The CSRF token + * @param serviceId Id of the service to remove, if any + * @param hostId Id of the host to remove, if any + * @param contextId Id of the context to remove, if any + * @param wrapperId Id of the wrapper to remove, if any + * @param valveId Id of the valve to remove, if any + * @param connectorId Id of the connector to remove, if any + * @param executorId Id of the executor to remove, if any + * @param aliasId Id of the alias to remove, if any + * + * @throws Exception If the cleanup requests cannot be exchanged + */ + protected void cleanup(SimpleHttpClient client, String token, String serviceId, String hostId, String contextId, + String wrapperId, String valveId, String connectorId, String executorId, String aliasId) throws Exception { + for (String id : new String[] { connectorId, executorId, wrapperId, valveId, aliasId, contextId, hostId, + serviceId }) { + if (id == null) { + continue; + } + try { + request(client, "DELETE", MANAGER2 + "/api/config/child", token, + "{\"id\":\"" + id + "\",\"confirm\":\"confirm\"}", 200); + } catch (AssertionError e) { + // Already removed (or never created): ignore. + } + } + } + + + protected static int freePort() throws Exception { + try (ServerSocket socket = new ServerSocket(0)) { + return socket.getLocalPort(); + } + } + + + /** + * Issue an HTTPS GET against the given port with a trust-all trust manager (the test certificate is self signed). + * Returns the HTTP status code; any status proves that the TLS handshake succeeded and the connector served the + * request. + * + * @param port The port to connect to + * @param path The request path + * + * @return The HTTP status code + * + * @throws Exception If the request fails + */ + protected static int httpsGet(int port, String path) throws Exception { + TrustManager[] trustAll = new TrustManager[] { new X509TrustManager() { + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) { + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) { + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + } }; + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, trustAll, new SecureRandom()); Review Comment: ## CodeQL / `TrustManager` that accepts all certificates This uses [TrustManager](1), which is defined in [Manager2ConfigTestBase$](2) and trusts any certificate. [Show more details](https://github.com/apache/tomcat/security/code-scanning/979) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
