davsclaus commented on code in PR #27429: URL: https://github.com/apache/camel/pull/27429#discussion_r4193499758
########## components/camel-spiffe/src/test/java/org/apache/camel/component/spiffe/integration/SpiffeMutualTlsIT.java: ########## @@ -0,0 +1,135 @@ +/* + * 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.camel.component.spiffe.integration; + +import java.io.IOException; +import java.net.InetAddress; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; + +import org.apache.camel.component.spiffe.SpiffeSSLContextParameters; +import org.apache.camel.support.jsse.ClientAuthentication; +import org.apache.camel.support.jsse.SSLContextServerParameters; +import org.apache.camel.test.infra.spiffe.services.SpiffeService; +import org.apache.camel.test.infra.spiffe.services.SpiffeServiceFactory; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Completes a real mutual-TLS handshake through {@link SpiffeSSLContextParameters} against SVIDs from a live SPIRE + * Workload API, asserting that an allow-listed peer connects and a non-allow-listed one is refused during the + * handshake. + */ +class SpiffeMutualTlsIT extends CamelTestSupport { + + @RegisterExtension + static SpiffeService service = SpiffeServiceFactory.createSingletonService(); + + private SpiffeSSLContextParameters serverSsl(String acceptedSpiffeIds) { + SpiffeSSLContextParameters ssl = new SpiffeSSLContextParameters(); + ssl.setSpiffeSocketPath(service.getWorkloadApiSocketPath()); + ssl.setAcceptedSpiffeIds(acceptedSpiffeIds); + SSLContextServerParameters serverParameters = new SSLContextServerParameters(); + serverParameters.setClientAuthentication(ClientAuthentication.REQUIRE.name()); + ssl.setServerParameters(serverParameters); + return ssl; + } + + private SpiffeSSLContextParameters clientSsl() { + SpiffeSSLContextParameters ssl = new SpiffeSSLContextParameters(); + ssl.setSpiffeSocketPath(service.getWorkloadApiSocketPath()); + ssl.setAcceptedSpiffeIds(service.getWorkloadSpiffeId()); + return ssl; + } + + @Test + void allowsAnAllowListedPeer() throws Exception { + SSLContext serverContext = serverSsl(service.getWorkloadSpiffeId()).createSSLContext(context); + SSLContext clientContext = clientSsl().createSSLContext(context); + + assertThatCode(() -> handshake(serverContext, clientContext)).doesNotThrowAnyException(); + } + + @Test + void refusesANonAllowListedPeer() throws Exception { + // the server accepts only an id the client does not have; its SVID is spiffe://example.org/workload + SSLContext serverContext = serverSsl("spiffe://example.org/not-allowed").createSSLContext(context); + SSLContext clientContext = clientSsl().createSSLContext(context); + + // a rejected mutual-TLS handshake surfaces as an IOException - a TLS alert (SSLHandshakeException) when the peer + // sends one, or a connection reset (SocketException) when it just drops the connection; both are acceptable, the + // point is that the handshake does not complete + assertThatThrownBy(() -> handshake(serverContext, clientContext)) + .as("a non-allow-listed peer must be refused during the handshake") + .isInstanceOf(IOException.class); Review Comment: Minor: `IOException` also accepts a `SocketTimeoutException` or a connect failure, so the deny test can pass for the wrong reason. Asserting the server-side `SSLHandshakeException` (ideally with the rejected SPIFFE ID in the message), or at least excluding `SocketTimeoutException`, would make it precise. ########## test-infra/camel-test-infra-spiffe/src/main/java/org/apache/camel/test/infra/spiffe/services/SpiffeLocalContainerInfraService.java: ########## @@ -0,0 +1,302 @@ +/* + * 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.camel.test.infra.spiffe.services; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; + +import com.sun.security.auth.module.UnixSystem; +import org.apache.camel.spi.annotations.InfraService; +import org.apache.camel.test.infra.common.LocalPropertyResolver; +import org.apache.camel.test.infra.common.services.ContainerService; +import org.apache.camel.test.infra.spiffe.common.SpiffeProperties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.BindMode; +import org.testcontainers.containers.Container; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.SelinuxContext; +import org.testcontainers.containers.startupcheck.IsRunningStartupCheckStrategy; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.MountableFile; + +/** + * Runs a SPIRE server and agent so a test can fetch SVIDs from a real Workload API. + * <p> + * The agent's Workload API Unix socket is bind-mounted to a short host path (the {@code AF_UNIX} {@code sun_path} limit + * is ~108 bytes, so a socket under a deep temp tree would fail to connect from the host). The agent runs in the host + * PID namespace because the {@code unix} workload attestor resolves the calling process through {@code /proc}, which a + * container cannot see otherwise; a single registration entry is created for {@code unix:uid:<the test process uid>} so + * the test JVM is issued {@code spiffe://<trustDomain>/workload}. + */ +@InfraService(service = SpiffeInfraService.class, + description = "SPIFFE/SPIRE server and agent exposing the Workload API", + serviceAlias = { "spiffe" }) +public class SpiffeLocalContainerInfraService implements SpiffeInfraService, ContainerService<GenericContainer<?>> { + + public static final String TRUST_DOMAIN = "example.org"; + public static final String AGENT_SPIFFE_ID = "spiffe://" + TRUST_DOMAIN + "/agent"; + public static final String WORKLOAD_SPIFFE_ID = "spiffe://" + TRUST_DOMAIN + "/workload"; + + private static final Logger LOG = LoggerFactory.getLogger(SpiffeLocalContainerInfraService.class); + + private static final String SERVER_ALIAS = "spire-server"; + private static final int SERVER_PORT = 8081; + private static final String SERVER_BIN = "/opt/spire/bin/spire-server"; + private static final String SERVER_CONF = "/opt/spire/conf/server/server.conf"; + private static final String AGENT_CONF = "/opt/spire/conf/agent/agent.conf"; + private static final String SOCKET_CONTAINER_DIR = "/tmp/spire-agent/public"; + private static final String SOCKET_FILE = "api.sock"; + private static final String SERVER_CONF_RESOURCE = "org/apache/camel/test/infra/spiffe/services/spire-server.conf"; + private static final String AGENT_CONF_RESOURCE = "org/apache/camel/test/infra/spiffe/services/spire-agent.conf"; + + private final String serverImage; + private final String agentImage; + + private Network network; + private GenericContainer<?> server; + private GenericContainer<?> agent; + private Path hostSocketDir; + private String socketPath; + + public SpiffeLocalContainerInfraService() { + this(LocalPropertyResolver.getProperty(SpiffeLocalContainerInfraService.class, + SpiffeProperties.SPIFFE_SERVER_CONTAINER), + LocalPropertyResolver.getProperty(SpiffeLocalContainerInfraService.class, + SpiffeProperties.SPIFFE_AGENT_CONTAINER)); + } + + public SpiffeLocalContainerInfraService(String serverImage, String agentImage) { + this.serverImage = serverImage; + this.agentImage = agentImage; + } + + @Override + public void initialize() { + try { + doInitialize(); + } catch (RuntimeException e) { + // a partial start would leak the server container, the network and the temp socket directory, and a + // TestServiceUtil.tryInitialize() retry would overwrite the fields and orphan them; tear down whatever + // came up before propagating + cleanup(); + throw e; + } + } + + private void doInitialize() { + createHostSocketDir(); + network = Network.newNetwork(); + + // the image entrypoint is "spire-server run", so only -config is passed; the config is copied in (no bind mount + // needed for it) and port 8081 is exposed purely so the listening-port wait can tell when the API is up + server = new GenericContainer<>(serverImage) + .withNetwork(network) + .withNetworkAliases(SERVER_ALIAS) + .withCopyFileToContainer(MountableFile.forClasspathResource(SERVER_CONF_RESOURCE), SERVER_CONF) + .withExposedPorts(SERVER_PORT) + .withCommand("-config", SERVER_CONF) + .waitingFor(Wait.forListeningPort()); + LOG.info("Starting the SPIRE server container"); + server.start(); + waitForServerHealthy(); + + String joinToken = generateJoinToken(); + createWorkloadEntry(); + + // the agent needs the host PID namespace so the unix workload attestor can resolve the host test process, and + // the socket directory bind-mounted (SELinux-shared) so the host can reach the Workload API socket it creates + agent = new GenericContainer<>(agentImage) + .withNetwork(network) + .withCopyFileToContainer(MountableFile.forClasspathResource(AGENT_CONF_RESOURCE), AGENT_CONF) + .withCreateContainerCmdModifier(cmd -> cmd.getHostConfig().withPidMode("host")) Review Comment: Host PID mode, the unix socket in the bind-mounted directory and the uid selector all assume a Linux Docker host; see the review summary for macOS/Windows Docker Desktop and rootless Podman. ########## components/camel-spiffe/src/test/java/org/apache/camel/component/spiffe/integration/SpiffeMutualTlsIT.java: ########## @@ -0,0 +1,135 @@ +/* + * 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.camel.component.spiffe.integration; + +import java.io.IOException; +import java.net.InetAddress; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; + +import org.apache.camel.component.spiffe.SpiffeSSLContextParameters; +import org.apache.camel.support.jsse.ClientAuthentication; +import org.apache.camel.support.jsse.SSLContextServerParameters; +import org.apache.camel.test.infra.spiffe.services.SpiffeService; +import org.apache.camel.test.infra.spiffe.services.SpiffeServiceFactory; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Completes a real mutual-TLS handshake through {@link SpiffeSSLContextParameters} against SVIDs from a live SPIRE + * Workload API, asserting that an allow-listed peer connects and a non-allow-listed one is refused during the + * handshake. + */ +class SpiffeMutualTlsIT extends CamelTestSupport { + + @RegisterExtension + static SpiffeService service = SpiffeServiceFactory.createSingletonService(); + + private SpiffeSSLContextParameters serverSsl(String acceptedSpiffeIds) { + SpiffeSSLContextParameters ssl = new SpiffeSSLContextParameters(); + ssl.setSpiffeSocketPath(service.getWorkloadApiSocketPath()); + ssl.setAcceptedSpiffeIds(acceptedSpiffeIds); + SSLContextServerParameters serverParameters = new SSLContextServerParameters(); + serverParameters.setClientAuthentication(ClientAuthentication.REQUIRE.name()); + ssl.setServerParameters(serverParameters); + return ssl; + } + + private SpiffeSSLContextParameters clientSsl() { + SpiffeSSLContextParameters ssl = new SpiffeSSLContextParameters(); + ssl.setSpiffeSocketPath(service.getWorkloadApiSocketPath()); + ssl.setAcceptedSpiffeIds(service.getWorkloadSpiffeId()); + return ssl; + } + + @Test + void allowsAnAllowListedPeer() throws Exception { + SSLContext serverContext = serverSsl(service.getWorkloadSpiffeId()).createSSLContext(context); + SSLContext clientContext = clientSsl().createSSLContext(context); + + assertThatCode(() -> handshake(serverContext, clientContext)).doesNotThrowAnyException(); + } + + @Test + void refusesANonAllowListedPeer() throws Exception { + // the server accepts only an id the client does not have; its SVID is spiffe://example.org/workload + SSLContext serverContext = serverSsl("spiffe://example.org/not-allowed").createSSLContext(context); + SSLContext clientContext = clientSsl().createSSLContext(context); + + // a rejected mutual-TLS handshake surfaces as an IOException - a TLS alert (SSLHandshakeException) when the peer + // sends one, or a connection reset (SocketException) when it just drops the connection; both are acceptable, the + // point is that the handshake does not complete + assertThatThrownBy(() -> handshake(serverContext, clientContext)) + .as("a non-allow-listed peer must be refused during the handshake") + .isInstanceOf(IOException.class); + } + + /** + * Drives a mutual-TLS handshake over a loopback socket: the server requires a client certificate and both sides + * present their SVID. Returns normally when the handshake and a one-byte exchange complete, and throws when either + * side rejects the peer. + */ + private void handshake(SSLContext serverContext, SSLContext clientContext) throws Exception { + ExecutorService executor = Executors.newSingleThreadExecutor(); + try (SSLServerSocket serverSocket = (SSLServerSocket) serverContext.getServerSocketFactory() + .createServerSocket(0, 1, InetAddress.getLoopbackAddress())) { + serverSocket.setNeedClientAuth(true); Review Comment: This forces client authentication directly on the socket, bypassing the `clientAuthentication=REQUIRE` configured on the `SSLContextServerParameters`. With this line the deny test would still pass if the SSL context decorator dropped the client authentication again (the CAMEL-24571 regression CAMEL-24733 asks to cover). Suggest removing it, so client auth only comes from `SpiffeSSLContextParameters`. ########## test-infra/camel-test-infra-spiffe/src/main/java/org/apache/camel/test/infra/spiffe/services/SpiffeLocalContainerInfraService.java: ########## @@ -0,0 +1,302 @@ +/* + * 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.camel.test.infra.spiffe.services; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; + +import com.sun.security.auth.module.UnixSystem; +import org.apache.camel.spi.annotations.InfraService; +import org.apache.camel.test.infra.common.LocalPropertyResolver; +import org.apache.camel.test.infra.common.services.ContainerService; +import org.apache.camel.test.infra.spiffe.common.SpiffeProperties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.BindMode; +import org.testcontainers.containers.Container; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.SelinuxContext; +import org.testcontainers.containers.startupcheck.IsRunningStartupCheckStrategy; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.MountableFile; + +/** + * Runs a SPIRE server and agent so a test can fetch SVIDs from a real Workload API. + * <p> + * The agent's Workload API Unix socket is bind-mounted to a short host path (the {@code AF_UNIX} {@code sun_path} limit + * is ~108 bytes, so a socket under a deep temp tree would fail to connect from the host). The agent runs in the host + * PID namespace because the {@code unix} workload attestor resolves the calling process through {@code /proc}, which a + * container cannot see otherwise; a single registration entry is created for {@code unix:uid:<the test process uid>} so + * the test JVM is issued {@code spiffe://<trustDomain>/workload}. + */ +@InfraService(service = SpiffeInfraService.class, + description = "SPIFFE/SPIRE server and agent exposing the Workload API", + serviceAlias = { "spiffe" }) +public class SpiffeLocalContainerInfraService implements SpiffeInfraService, ContainerService<GenericContainer<?>> { + + public static final String TRUST_DOMAIN = "example.org"; + public static final String AGENT_SPIFFE_ID = "spiffe://" + TRUST_DOMAIN + "/agent"; + public static final String WORKLOAD_SPIFFE_ID = "spiffe://" + TRUST_DOMAIN + "/workload"; + + private static final Logger LOG = LoggerFactory.getLogger(SpiffeLocalContainerInfraService.class); + + private static final String SERVER_ALIAS = "spire-server"; + private static final int SERVER_PORT = 8081; + private static final String SERVER_BIN = "/opt/spire/bin/spire-server"; + private static final String SERVER_CONF = "/opt/spire/conf/server/server.conf"; + private static final String AGENT_CONF = "/opt/spire/conf/agent/agent.conf"; + private static final String SOCKET_CONTAINER_DIR = "/tmp/spire-agent/public"; + private static final String SOCKET_FILE = "api.sock"; + private static final String SERVER_CONF_RESOURCE = "org/apache/camel/test/infra/spiffe/services/spire-server.conf"; + private static final String AGENT_CONF_RESOURCE = "org/apache/camel/test/infra/spiffe/services/spire-agent.conf"; + + private final String serverImage; + private final String agentImage; + + private Network network; + private GenericContainer<?> server; + private GenericContainer<?> agent; + private Path hostSocketDir; + private String socketPath; + + public SpiffeLocalContainerInfraService() { + this(LocalPropertyResolver.getProperty(SpiffeLocalContainerInfraService.class, + SpiffeProperties.SPIFFE_SERVER_CONTAINER), + LocalPropertyResolver.getProperty(SpiffeLocalContainerInfraService.class, + SpiffeProperties.SPIFFE_AGENT_CONTAINER)); + } + + public SpiffeLocalContainerInfraService(String serverImage, String agentImage) { + this.serverImage = serverImage; + this.agentImage = agentImage; + } + + @Override + public void initialize() { + try { + doInitialize(); + } catch (RuntimeException e) { + // a partial start would leak the server container, the network and the temp socket directory, and a + // TestServiceUtil.tryInitialize() retry would overwrite the fields and orphan them; tear down whatever + // came up before propagating + cleanup(); + throw e; + } + } + + private void doInitialize() { + createHostSocketDir(); + network = Network.newNetwork(); + + // the image entrypoint is "spire-server run", so only -config is passed; the config is copied in (no bind mount + // needed for it) and port 8081 is exposed purely so the listening-port wait can tell when the API is up + server = new GenericContainer<>(serverImage) + .withNetwork(network) + .withNetworkAliases(SERVER_ALIAS) + .withCopyFileToContainer(MountableFile.forClasspathResource(SERVER_CONF_RESOURCE), SERVER_CONF) + .withExposedPorts(SERVER_PORT) + .withCommand("-config", SERVER_CONF) + .waitingFor(Wait.forListeningPort()); + LOG.info("Starting the SPIRE server container"); + server.start(); + waitForServerHealthy(); + + String joinToken = generateJoinToken(); + createWorkloadEntry(); + + // the agent needs the host PID namespace so the unix workload attestor can resolve the host test process, and + // the socket directory bind-mounted (SELinux-shared) so the host can reach the Workload API socket it creates + agent = new GenericContainer<>(agentImage) + .withNetwork(network) + .withCopyFileToContainer(MountableFile.forClasspathResource(AGENT_CONF_RESOURCE), AGENT_CONF) + .withCreateContainerCmdModifier(cmd -> cmd.getHostConfig().withPidMode("host")) + .withStartupCheckStrategy(new IsRunningStartupCheckStrategy()) + .withCommand("-config", AGENT_CONF, "-joinToken", joinToken); + agent.addFileSystemBind(hostSocketDir.toString(), SOCKET_CONTAINER_DIR, BindMode.READ_WRITE, SelinuxContext.SHARED); + LOG.info("Starting the SPIRE agent container"); + agent.start(); + waitForSocket(); + + socketPath = "unix://" + hostSocketDir.resolve(SOCKET_FILE); + registerProperties(); + LOG.info("SPIRE Workload API available at {}", socketPath); + } + + private void createHostSocketDir() { + try { + // keep the socket path short (AF_UNIX sun_path is limited to ~108 bytes); /tmp keeps it well within that + hostSocketDir = Files.createTempDirectory(Path.of("/tmp"), "spiffe"); + hostSocketDir.toFile().setReadable(true, false); + hostSocketDir.toFile().setWritable(true, false); Review Comment: Nit: a short comment that the directory must be world-writable because the agent runs as a non-root uid would help, as this is a world-writable directory under `/tmp`. -- 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]
