oscerd commented on code in PR #27824:
URL: https://github.com/apache/camel/pull/27824#discussion_r4281191069
##########
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:
Fixed in 0b7c25a9a20c. You were right that `NO_OPTIONAL_CHECKS` is
`Set.of(APPLICATION_URI)`. The trust list validator is now built with
`APPLICATION_URI`, `VALIDITY` and `REVOCATION`, and the doc lists exactly what
is refused.
One result differs from the review. In Milo 1.1.8, `REVOCATION` without
`REVOCATION_LISTS` does not soft-fail when the trust list has no CRL for a CA:
a CA-issued server certificate is then refused with
`Bad_CertificateRevocationUnknown`
(`trustListRefusesServerCertificateOfCaWithoutRevocationList`). We kept
`REVOCATION`, as OPC UA requires. The doc now says that a CA-issued certificate
needs a CRL of its CA in the trust list (an empty one is enough), and that a
self-signed certificate of the trust list is not checked for revocation.
On validity:
- An expired CA-issued certificate is already refused while Milo builds the
chain (`Bad_SecurityChecksFailed`), and it is written to `rejected/certs`.
- `VALIDITY` is what refuses an expired self-signed certificate of the trust
list (`Bad_CertificateTimeInvalid`).
- The doc says that only a certificate without a chain up to a trusted
certificate is written to `rejected/certs`, and that one refused for another
reason, such as the application URI, is not.
New unit tests, with certificates issued by Bouncy Castle:
- revoked: `Bad_CertificateRevoked`;
- CA without CRL: `Bad_CertificateRevocationUnknown`;
- CA with an empty CRL: accepted;
- expired self-signed: `Bad_CertificateTimeInvalid`;
- another application URI: `Bad_CertificateUriInvalid`, not written to
`rejected/certs`.
With Milo's default check set put back, the revoked, expired and CRL-less
cases fail because the certificate is accepted.
_Claude Code on behalf of Andrea Cosentino_
##########
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:
Fixed in 0b7c25a9a20c.
These two failures are now logged at WARN as one line, with the stack trace
at DEBUG:
- a missing trust configuration (now a dedicated `NotConfiguredException`):
`Failed to connect: The certificate of the OPC UA server cannot be validated:
...`;
- a `UaException` with a certificate or security status code: `Failed to
connect, because a certificate or security check failed: UaException:
status=...`.
The second wording does not blame the server certificate, because the same
status codes come back when the server refuses the client certificate. Other
failures stay at INFO.
The client keeps retrying every 10 s, since the trust list watches its
directories and a certificate moved to `trusted/certs` is used at the next
attempt.
The last connect failure is now the cause of `No connected`, and it is
cleared after a successful connect.
_Claude Code on behalf of Andrea Cosentino_
##########
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:
Fixed in 0b7c25a9a20c. After the mocks are satisfied, the test writes
through a producer that shares the connection of each failing client
(`defaultAwaitWrites=true`), and asserts the cause chain:
- for the client without trust configuration: `No connected`, caused by `The
certificate of the OPC UA server cannot be validated`;
- for the empty trust list: `No connected`, caused by
`Bad_SecurityChecksFailed`.
With the cause removed from `No connected`, the test fails on that assertion.
_Claude Code on behalf of Andrea Cosentino_
##########
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:
Done in 0b7c25a9a20c. The entry now names:
- the WARN message `Failed to connect: The certificate of the OPC UA server
cannot be validated`;
- the retry every 10 seconds;
- the producer failure `IllegalStateException: No connected`, with that
cause;
- the WARN logged for a certificate refused by the trust list or by a
`certificateValidator`.
It also lists what the trust list refuses.
_Claude Code on behalf of Andrea Cosentino_
##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/MiloClientConfiguration.java:
##########
@@ -464,11 +517,27 @@ public MiloClientConfiguration clone() {
}
public String toCacheId() {
+ final String cacheId;
if (this.clientId != null && !this.clientId.isEmpty()) {
- return this.endpointUri + "|" + this.clientId + "|" +
this.requestedPublishingInterval;
+ cacheId = this.endpointUri + "|" + this.clientId + "|" +
this.requestedPublishingInterval;
} else {
- return this.endpointUri + "|" + this.requestedPublishingInterval;
+ cacheId = this.endpointUri + "|" +
this.requestedPublishingInterval;
+ }
+ // endpoints that validate the server certificate differently must not
share a connection
+ return cacheId + serverCertificateTrustCacheId();
+ }
+
+ private String serverCertificateTrustCacheId() {
+ if (this.certificateValidator != null) {
+ return "|certificateValidator=" +
Integer.toHexString(System.identityHashCode(this.certificateValidator));
Review Comment:
Done in 0b7c25a9a20c. The cache id now uses the validator's class name
together with its identity hash, and the `certificateValidator` option
documents that endpoints share a connection only when they use the same
validator instance.
The id only picks a cached connection inside one JVM, so it does not need to
be stable across restarts.
_Claude Code on behalf of Andrea Cosentino_
--
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]
