davsclaus commented on code in PR #27824:
URL: https://github.com/apache/camel/pull/27824#discussion_r4269545143
##########
components/camel-milo/src/main/docs/milo-client-component.adoc:
##########
@@ -204,10 +204,45 @@ The known security policy URIs and enum literals are can
be seen here: https://g
NOTE: In any case security policies are considered case sensitive.
+When `allowedSecurityPolicies` is not set, the client accepts every security
policy the server offers and uses the
+endpoint with the highest security level reported by the server, which can be
`None`. To make sure the connection is
+secured, set `allowedSecurityPolicies` to the secured policies you expect the
server to offer.
+
+=== Validating the server certificate
+
+When the selected endpoint uses a security policy other than `None`, the
client validates the certificate of the
+server when it opens the secure channel and the session. Configure the trusted
certificates with one of these options:
+
+* `trustListDirectory`: a directory holding the trust list, in the layout used
by Eclipse Milo. Put the certificate of
+the server, or the certificate of the CA that issued it, in `trusted/certs`.
Certificates of intermediate CAs that are
+only used to build the chain go in `issuer/certs`, and certificate revocation
lists in `trusted/crl` and `issuer/crl`.
+The directories are created if they do not exist. A server certificate that
fails validation is written to
+`rejected/certs`, from where it can be moved to `trusted/certs` once it has
been checked.
+* `certificateValidator`: a custom
`org.eclipse.milo.opcua.stack.core.security.CertificateValidator`. It is used
+instead of `trustListDirectory` when both are set.
+
+The trust list checks that the certificate chains up to a trusted certificate.
The optional checks of Eclipse Milo,
Review Comment:
With `DefaultClientCertificateValidator(manager, quarantine)` Milo 1.1.8
uses `ValidationCheck.NO_OPTIONAL_CHECKS`, which is `Set.of(APPLICATION_URI)`.
So the application URI check is enforced (`Bad_CertificateUriInvalid`, and the
certificate is not written to `rejected/certs`). Revocation and validity are
only logged. Combined with the advice above to put CRLs in `trusted/crl` /
`issuer/crl`, readers will expect a revoked server certificate to be refused,
and it is not. Please enable `REVOCATION` (and maybe `VALIDITY`) in
`ServerCertificateTrust`, or describe the checks exactly here.
##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java:
##########
@@ -740,6 +744,9 @@ private Connected performConnect() throws Exception {
LOG.debug("Selected endpoint: {}", endpoint);
+ // the same validator checks the server certificate for the secure
channel, the session and the identity token
+ final CertificateValidator certificateValidator =
this.serverCertificateTrust.validatorFor(endpoint);
Review Comment:
When this throws, or when Milo rejects the certificate in
`client.connect()`, the reason ends up in `LOG.info("Failed to connect", e)`
and the client retries every 10 s. A producer only sees
`IllegalStateException("No connected")`. A configuration error that retrying
cannot fix should be visible at WARN. Carrying the last failure as the cause of
"No connected" would also give the producer a clear error.
##########
components/camel-milo/src/test/java/org/apache/camel/component/milo/ServerCertificateValidationTest.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.milo;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.cert.X509Certificate;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.EndpointInject;
+import org.apache.camel.Produce;
+import org.apache.camel.ProducerTemplate;
+import org.apache.camel.RoutesBuilder;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.milo.call.TestCertificateFactory;
+import org.apache.camel.component.milo.server.MiloServerComponent;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.eclipse.milo.opcua.stack.core.UaException;
+import org.eclipse.milo.opcua.stack.core.security.CertificateValidator;
+import org.eclipse.milo.opcua.stack.core.security.DefaultApplicationGroup;
+import org.eclipse.milo.opcua.stack.core.security.DefaultCertificateManager;
+import
org.eclipse.milo.opcua.stack.core.security.DefaultClientCertificateValidator;
+import org.eclipse.milo.opcua.stack.core.security.MemoryCertificateQuarantine;
+import org.eclipse.milo.opcua.stack.core.security.MemoryCertificateStore;
+import org.eclipse.milo.opcua.stack.core.security.MemoryTrustListManager;
+import org.eclipse.milo.opcua.stack.core.security.SecurityPolicy;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.awaitility.Awaitility.await;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * The client validates the certificate of a server that only offers a secured
endpoint.
+ */
+class ServerCertificateValidationTest extends AbstractMiloServerTest {
+
+ private static final String DIRECT_START = "direct:start";
+
+ private static final String MILO_SERVER_ITEM = "milo-server:myitem1";
+
+ private static final String MILO_CLIENT =
"milo-client:opc.tcp://foo:bar@localhost:@@port@@?node="
+ +
NodeIds.nodeValue(MiloServerComponent.DEFAULT_NAMESPACE_URI, "myitem1")
+ +
"&keyStoreUrl=file:src/test/resources/keystore&keyStorePassword=testtest"
+ +
"&keyPassword=test&keyAlias=test&allowedSecurityPolicies=Basic256Sha256"
+ +
"&discoveryEndpointSuffix=/discovery&overrideHost=true";
+
+ private static final String SERVER_APPLICATION_URI =
"urn:eclipse:milo:test";
+
+ @TempDir
+ static Path pki;
+
+ private final RecordingCertificateValidator recordingValidator = new
RecordingCertificateValidator();
+
+ @EndpointInject("mock:trustList")
+ MockEndpoint trustListEndpoint;
+
+ @EndpointInject("mock:emptyTrustList")
+ MockEndpoint emptyTrustListEndpoint;
+
+ @EndpointInject("mock:notConfigured")
+ MockEndpoint notConfiguredEndpoint;
+
+ @EndpointInject("mock:trustAll")
+ MockEndpoint trustAllEndpoint;
+
+ @EndpointInject("mock:certificateValidator")
+ MockEndpoint certificateValidatorEndpoint;
+
+ @Produce(DIRECT_START)
+ ProducerTemplate producer;
+
+ @Override
+ protected void configureContext(final CamelContext context) throws
Exception {
+ super.configureContext(context);
+ context.getRegistry().bind("recordingValidator",
this.recordingValidator);
+ }
+
+ @Override
+ protected void configureMiloServer(final MiloServerComponent server)
throws Exception {
+ super.configureMiloServer(server);
+
+ // the server uses a self-signed certificate, and accepts any client:
the client is under test here
+ final DefaultApplicationGroup certificateGroup =
DefaultApplicationGroup.createAndInitialize(
+ new MemoryTrustListManager(), new MemoryCertificateStore(),
new TestCertificateFactory(),
+ new CertificateValidator.InsecureCertificateValidator());
+ final X509Certificate serverCertificate =
certificateGroup.getCertificateEntries().get(0).certificateChain[0];
+
+ server.setApplicationUri(SERVER_APPLICATION_URI);
+ server.setCertificateManager(new DefaultCertificateManager(new
MemoryCertificateQuarantine(), certificateGroup));
+ server.setCertificate(serverCertificate);
+ server.setSecurityPolicies(EnumSet.of(SecurityPolicy.Basic256Sha256));
+ server.setUsernameSecurityPolicyUri(SecurityPolicy.Basic256Sha256);
+
+ final Path trustedCerts =
Files.createDirectories(pki.resolve("trusted").resolve("trusted").resolve("certs"));
+ Files.write(trustedCerts.resolve("server.der"),
serverCertificate.getEncoded());
+
+ this.recordingValidator.trust(serverCertificate);
+ }
+
+ @Override
+ protected RoutesBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ from(DIRECT_START).to(MILO_SERVER_ITEM);
+
+ from(resolve(MILO_CLIENT + "&trustListDirectory=RAW(" +
pki.resolve("trusted") + ")"))
+ .to("mock:trustList");
+ from(resolve(MILO_CLIENT + "&trustListDirectory=RAW(" +
pki.resolve("empty") + ")"))
+ .to("mock:emptyTrustList");
+ from(resolve(MILO_CLIENT)).to("mock:notConfigured");
+ from(resolve(MILO_CLIENT +
"&trustAllCertificates=true")).to("mock:trustAll");
+ from(resolve(MILO_CLIENT +
"&certificateValidator=#recordingValidator")).to("mock:certificateValidator");
+ }
+ };
+ }
+
+ @Test
+ void onlyClientsThatCanValidateTheServerCertificateConnect() throws
Exception {
+ this.trustListEndpoint.setExpectedCount(1);
+ this.trustAllEndpoint.setExpectedCount(1);
+ this.certificateValidatorEndpoint.setExpectedCount(1);
+
+ this.emptyTrustListEndpoint.setExpectedCount(0);
+ this.emptyTrustListEndpoint.setSleepForEmptyTest(5_000);
+ this.notConfiguredEndpoint.setExpectedCount(0);
Review Comment:
Expecting zero messages also passes when the client fails for a reason
unrelated to the trust configuration, for example a key store problem. Can this
case assert the reason too, e.g. the `validatorFor` message through a producer,
or a captured log line?
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -97,6 +97,24 @@ now read as a local date-time without a zone (`getDate()`
returns a `LocalDateTi
definition of an earlier message, if there was one. A calendar that defines a
`TZID` an earlier message has defined
differently is now read with its own definition.
+=== camel-milo - the OPC UA client validates the server certificate
+
+When the endpoint it selects uses a security policy other than `None`, the
`milo-client` and `milo-browse` components
+now validate the certificate of the OPC UA server. Before, any server
certificate was accepted, and no option allowed
+to validate it.
+
+Configure the trusted certificates with the new `trustListDirectory` option:
put the certificate of the server, or of
+the CA that issued it, in the `trusted/certs` directory of the trust list. A
rejected server certificate is written to
+`rejected/certs`, from where it can be moved to `trusted/certs` once it has
been checked. A custom validator can be
+configured with the new `certificateValidator` option instead.
+
+With neither option, the client does not connect to such an endpoint, and logs
that the server certificate cannot be
Review Comment:
Today this is an INFO "Failed to connect" log, repeated every 10 s, and a
producer gets `No connected`. Once the level is settled, please name the
symptom here so users who upgrade can recognise it.
--
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]
