Skip to content

Commit 896d869

Browse files
committed
fix(plc4j): Stabilization of OPC UA driver tests.
First and foremost - this commit switches OPC-UA tests to rely on dynamic port. This implies also slight change to endpoint selection method and possibility to override desired endpoint port and host which might differ from host/port reported by OPC-UA server. Because server is not aware of actual tcp port and host used by client, it is client duty to make proper decission when looking for endpoint. Related to #1764. Signed-off-by: Łukasz Dywicki <[email protected]>
1 parent a159e7e commit 896d869

File tree

8 files changed

+152
-143
lines changed

8 files changed

+152
-143
lines changed

plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/config/OpcuaConfiguration.java

+17
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ public class OpcuaConfiguration implements PlcConnectionConfiguration {
135135
@Description("TCP encoding options")
136136
private Limits limits;
137137

138+
@ConfigurationParameter("endpoint-host")
139+
@Description("Endpoint host used to establish secure channel.")
140+
private String endpointHost;
141+
142+
@ConfigurationParameter("endpoint-port")
143+
@Description("Endpoint port used to establish secure channel")
144+
private Integer endpointPort;
145+
138146
public String getProtocolCode() {
139147
return protocolCode;
140148
}
@@ -228,6 +236,14 @@ public long getNegotiationTimeout() {
228236
return negotiationTimeout;
229237
}
230238

239+
public String getEndpointHost() {
240+
return endpointHost;
241+
}
242+
243+
public Integer getEndpointPort() {
244+
return endpointPort;
245+
}
246+
231247
@Override
232248
public String toString() {
233249
return "OpcuaConfiguration{" +
@@ -240,5 +256,6 @@ public String toString() {
240256
", limits=" + limits +
241257
'}';
242258
}
259+
243260
}
244261

plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/context/OpcuaDriverContext.java

+7-8
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,6 @@ public String getHost() {
111111
return host;
112112
}
113113

114-
public void setHost(String host) {
115-
this.host = host;
116-
}
117-
118114
public String getPort() {
119115
return port;
120116
}
@@ -126,10 +122,6 @@ public String getEndpoint() {
126122
public String getTransportEndpoint() {
127123
return transportEndpoint;
128124
}
129-
130-
public void setTransportEndpoint(String transportEndpoint) {
131-
this.transportEndpoint = transportEndpoint;
132-
}
133125

134126
public X509Certificate getServerCertificate() {
135127
return serverCertificate;
@@ -147,6 +139,13 @@ public void setConfiguration(OpcuaConfiguration configuration) {
147139
port = matcher.group("transportPort");
148140
transportEndpoint = matcher.group("transportEndpoint");
149141

142+
if (configuration.getEndpointHost() != null) {
143+
host = configuration.getEndpointHost();
144+
}
145+
if (configuration.getEndpointPort() != null) {
146+
port = String.valueOf(configuration.getEndpointPort());
147+
}
148+
150149
String portAddition = port != null ? ":" + port : "";
151150
endpoint = "opc." + code + "://" + host + portAddition + transportEndpoint;
152151

plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/context/SecureChannel.java

+75-85
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,24 @@
2828
import java.security.cert.CertificateEncodingException;
2929
import java.security.cert.CertificateFactory;
3030
import java.security.cert.X509Certificate;
31+
import java.util.Collection;
32+
import java.util.Comparator;
33+
import java.util.HashSet;
3134
import java.util.Map.Entry;
35+
import java.util.Set;
3236
import java.util.concurrent.ScheduledExecutorService;
3337
import java.util.concurrent.ScheduledFuture;
3438
import java.util.concurrent.TimeUnit;
3539
import java.util.function.Function;
3640
import java.util.function.Supplier;
41+
import java.util.stream.Collectors;
3742
import org.apache.commons.lang3.RandomStringUtils;
38-
import org.apache.commons.lang3.StringUtils;
3943
import org.apache.plc4x.java.api.authentication.PlcAuthentication;
4044
import org.apache.plc4x.java.api.authentication.PlcUsernamePasswordAuthentication;
4145
import org.apache.plc4x.java.api.exceptions.PlcRuntimeException;
4246
import org.apache.plc4x.java.opcua.config.OpcuaConfiguration;
4347
import org.apache.plc4x.java.opcua.readwrite.*;
48+
import org.apache.plc4x.java.opcua.security.MessageSecurity;
4449
import org.apache.plc4x.java.opcua.security.SecurityPolicy;
4550
import org.apache.plc4x.java.opcua.security.SecurityPolicy.SignatureAlgorithm;
4651
import org.apache.plc4x.java.spi.generation.*;
@@ -56,11 +61,8 @@
5661
import java.util.ArrayList;
5762
import java.util.List;
5863
import java.util.concurrent.CompletableFuture;
59-
import java.util.regex.Matcher;
6064
import java.util.regex.Pattern;
6165

62-
import static java.util.concurrent.Executors.newSingleThreadExecutor;
63-
6466
public class SecureChannel {
6567

6668
private static final Logger LOGGER = LoggerFactory.getLogger(SecureChannel.class);
@@ -91,7 +93,7 @@ public class SecureChannel {
9193
private final OpcuaDriverContext driverContext;
9294
private final Conversation conversation;
9395
private ScheduledFuture<?> keepAlive;
94-
private final List<String> endpoints = new ArrayList<>();
96+
private final Set<String> endpoints = new HashSet<>();
9597
private double sessionTimeout;
9698
private long revisedLifetime;
9799

@@ -117,9 +119,9 @@ public SecureChannel(Conversation conversation, RequestTransactionManager tm, Op
117119
// Generate a list of endpoints we can use.
118120
try {
119121
InetAddress address = InetAddress.getByName(driverContext.getHost());
120-
this.endpoints.add(address.getHostAddress());
121-
this.endpoints.add(address.getHostName());
122-
this.endpoints.add(address.getCanonicalHostName());
122+
this.endpoints.add("opc.tcp://" + address.getHostAddress() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint());
123+
this.endpoints.add("opc.tcp://" + address.getHostName() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint());
124+
this.endpoints.add("opc.tcp://" + address.getCanonicalHostName() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint());
123125
} catch (UnknownHostException e) {
124126
LOGGER.warn("Unable to resolve host name. Using original host from connection string which may cause issues connecting to server");
125127
this.endpoints.add(driverContext.getHost());
@@ -313,23 +315,24 @@ private CompletableFuture<ActivateSessionResponse> onConnectActivateSessionReque
313315
conversation.setRemoteCertificate(getX509Certificate(sessionResponse.getServerCertificate().getStringValue()));
314316
conversation.setRemoteNonce(sessionResponse.getServerNonce().getStringValue());
315317

316-
String[] endpoints = new String[3];
318+
List<String> contactPoints = new ArrayList<>(3);
317319
try {
318320
InetAddress address = InetAddress.getByName(driverContext.getHost());
319-
endpoints[0] = "opc.tcp://" + address.getHostAddress() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint();
320-
endpoints[1] = "opc.tcp://" + address.getHostName() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint();
321-
endpoints[2] = "opc.tcp://" + address.getCanonicalHostName() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint();
321+
contactPoints.add("opc.tcp://" + address.getHostAddress() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint());
322+
contactPoints.add("opc.tcp://" + address.getHostName() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint());
323+
contactPoints.add("opc.tcp://" + address.getCanonicalHostName() + ":" + driverContext.getPort() + driverContext.getTransportEndpoint());
322324
} catch (UnknownHostException e) {
323325
LOGGER.debug("error getting host", e);
324326
}
325327

326-
Entry<EndpointDescription, UserTokenPolicy> endpointAndAuthPolicy = selectEndpoint(sessionResponse);
327-
if (endpointAndAuthPolicy == null) {
328-
throw new PlcRuntimeException("Unable to find endpoint - " + endpoints[1]);
328+
Entry<EndpointDescription, UserTokenPolicy> selectedEndpoint = selectEndpoint(sessionResponse.getServerEndpoints(), contactPoints,
329+
configuration.getSecurityPolicy(), configuration.getMessageSecurity());
330+
if (selectedEndpoint == null) {
331+
throw new PlcRuntimeException("Unable to find endpoint matching - " + contactPoints.get(1));
329332
}
330333

331-
PascalString policyId = endpointAndAuthPolicy.getValue().getPolicyId();
332-
UserTokenType tokenType = endpointAndAuthPolicy.getValue().getTokenType();
334+
PascalString policyId = selectedEndpoint.getValue().getPolicyId();
335+
UserTokenType tokenType = selectedEndpoint.getValue().getTokenType();
333336
ExtensionObject userIdentityToken = getIdentityToken(tokenType, policyId.getStringValue());
334337
RequestHeader requestHeader = conversation.createRequestHeader();
335338
SignatureData clientSignature = new SignatureData(NULL_STRING, NULL_BYTE_STRING);
@@ -421,27 +424,19 @@ public CompletableFuture<EndpointDescription> onDiscoverGetEndpointsRequest() {
421424

422425
return conversation.submit(endpointsRequest, GetEndpointsResponse.class).thenApply(response -> {
423426
List<ExtensionObjectDefinition> endpoints = response.getEndpoints();
424-
MessageSecurityMode effectiveMode = this.configuration.getSecurityPolicy() == SecurityPolicy.NONE ? MessageSecurityMode.messageSecurityModeNone : this.configuration.getMessageSecurity().getMode();
425-
for (ExtensionObjectDefinition endpoint : endpoints) {
426-
EndpointDescription endpointDescription = (EndpointDescription) endpoint;
427-
428-
boolean urlMatch = endpointDescription.getEndpointUrl().getStringValue().equals(this.endpoint.getStringValue());
429-
boolean policyMatch = endpointDescription.getSecurityPolicyUri().getStringValue().equals(this.configuration.getSecurityPolicy().getSecurityPolicyUri());
430-
boolean msgSecurityMatch = endpointDescription.getSecurityMode().equals(effectiveMode);
431-
432-
LOGGER.debug("Validate OPC UA endpoint {} during discovery phase."
433-
+ "Expected {}. Endpoint policy {} looking for {}. Message security {}, looking for {}", endpointDescription.getEndpointUrl().getStringValue(), this.endpoint.getStringValue(),
434-
endpointDescription.getSecurityPolicyUri().getStringValue(), configuration.getSecurityPolicy().getSecurityPolicyUri(),
435-
endpointDescription.getSecurityMode(), configuration.getMessageSecurity().getMode());
436-
437-
if (urlMatch && policyMatch && msgSecurityMatch) {
438-
LOGGER.info("Found OPC UA endpoint {}", this.endpoint.getStringValue());
439-
return endpointDescription;
440-
}
427+
Entry<EndpointDescription, UserTokenPolicy> entry = selectEndpoint(response.getEndpoints(), this.endpoints, this.configuration.getSecurityPolicy(), this.configuration.getMessageSecurity());
428+
429+
if (entry == null) {
430+
Set<String> endpointUris = endpoints.stream()
431+
.filter(EndpointDescription.class::isInstance)
432+
.map(EndpointDescription.class::cast)
433+
.map(EndpointDescription::getEndpointUrl)
434+
.map(PascalString::getStringValue)
435+
.collect(Collectors.toSet());
436+
throw new IllegalArgumentException("Could not find endpoint matching client configuration. Tested " + endpointUris + ". "
437+
+ "Was looking for " + this.endpoint.getStringValue() + " " + this.configuration.getSecurityPolicy().getSecurityPolicyUri() + " " + this.configuration.getMessageSecurity().getMode());
441438
}
442-
443-
throw new IllegalArgumentException("Could not find endpoint matching client configuration. Tested " + endpoints.size() + " endpoints. "
444-
+ "None matched " + this.endpoint.getStringValue() + " " + this.configuration.getSecurityPolicy().getSecurityPolicyUri() + " " + this.configuration.getMessageSecurity().getMode());
439+
return entry.getKey();
445440
});
446441
}
447442

@@ -503,32 +498,49 @@ private static ReadBufferByteBased toBuffer(Supplier<Payload> supplier) {
503498
/**
504499
* Selects the endpoint and authentication policy based on client settings.
505500
*
506-
* @param sessionResponse - The CreateSessionResponse message returned by the server
507-
* @return Entry representing desired server endpoint and user token policy to access it.
501+
* @param extensionObjects Endpoint descriptions returned by the server.
502+
* @param contactPoints Contact points expected by client.
503+
* @param securityPolicy Security policy searched in endpoints.
504+
* @param messageSecurity Message security needed by client.
505+
* @return Endpoint matching given.
508506
*/
509-
private Entry<EndpointDescription, UserTokenPolicy> selectEndpoint(CreateSessionResponse sessionResponse) {
507+
private Entry<EndpointDescription, UserTokenPolicy> selectEndpoint(List<ExtensionObjectDefinition> extensionObjects, Collection<String> contactPoints,
508+
SecurityPolicy securityPolicy, MessageSecurity messageSecurity) throws PlcRuntimeException {
510509
// Get a list of the endpoints which match ours.
511-
EndpointDescription selectedEndpoint = null;
512-
for (ExtensionObjectDefinition endpoint : sessionResponse.getServerEndpoints()) {
513-
if (!(endpoint instanceof EndpointDescription)) {
510+
MessageSecurityMode effectiveMessageSecurity = SecurityPolicy.NONE == securityPolicy ? MessageSecurityMode.messageSecurityModeNone : messageSecurity.getMode();
511+
List<Entry<EndpointDescription, UserTokenPolicy>> serverEndpoints = new ArrayList<>();
512+
513+
for (ExtensionObjectDefinition extensionObject : extensionObjects) {
514+
if (!(extensionObject instanceof EndpointDescription)) {
514515
continue;
515516
}
516-
if (isEndpoint((EndpointDescription) endpoint)) {
517-
selectedEndpoint = (EndpointDescription) endpoint;
518-
break;
517+
518+
EndpointDescription endpointDescription = (EndpointDescription) extensionObject;
519+
if (isMatchingEndpoint(endpointDescription, contactPoints)) {
520+
boolean policyMatch = endpointDescription.getSecurityPolicyUri().getStringValue().equals(securityPolicy.getSecurityPolicyUri());
521+
boolean msgSecurityMatch = endpointDescription.getSecurityMode().equals(effectiveMessageSecurity);
522+
523+
if (!policyMatch && !msgSecurityMatch) {
524+
continue;
525+
}
526+
527+
for (ExtensionObjectDefinition objectDefinition : endpointDescription.getUserIdentityTokens()) {
528+
if (objectDefinition instanceof UserTokenPolicy) {
529+
UserTokenPolicy userTokenPolicy = (UserTokenPolicy) objectDefinition;
530+
if (isUserTokenPolicyCompatible(userTokenPolicy, this.username)) {
531+
serverEndpoints.add(entry(endpointDescription, userTokenPolicy));
532+
}
533+
}
534+
}
519535
}
520536
}
521537

522-
for (ExtensionObjectDefinition tokenPolicy : selectedEndpoint.getUserIdentityTokens()) {
523-
if (!(tokenPolicy instanceof UserTokenPolicy)) {
524-
continue;
525-
}
526-
if (hasIdentity((UserTokenPolicy) tokenPolicy)) {
527-
return entry(selectedEndpoint, (UserTokenPolicy) tokenPolicy);
528-
}
538+
if (serverEndpoints.isEmpty()) {
539+
return null;
529540
}
530541

531-
return null;
542+
serverEndpoints.sort(Comparator.comparing(e -> e.getKey().getSecurityLevel()));
543+
return serverEndpoints.get(0);
532544
}
533545

534546
/**
@@ -539,36 +551,14 @@ private Entry<EndpointDescription, UserTokenPolicy> selectEndpoint(CreateSession
539551
* @return true if this endpoint matches our configuration
540552
* @throws PlcRuntimeException - If the returned endpoint string doesn't match the format expected
541553
*/
542-
private boolean isEndpoint(EndpointDescription endpoint) throws PlcRuntimeException {
554+
private static boolean isMatchingEndpoint(EndpointDescription endpoint, Collection<String> contactPoints) throws PlcRuntimeException {
543555
// Split up the connection string into it's individual segments.
544-
String endpointUri = endpoint.getEndpointUrl().getStringValue();
545-
Matcher matcher = URI_PATTERN.matcher(endpointUri);
546-
if (!matcher.matches()) {
547-
throw new PlcRuntimeException(
548-
"Endpoint " + endpointUri + " returned from the server doesn't match the format '{protocol-code}:({transport-code})?//{transport-host}(:{transport-port})(/{transport-endpoint})'");
549-
}
550-
LOGGER.trace("Using Endpoint {} {} {}", matcher.group("transportHost"), matcher.group("transportPort"), matcher.group("transportEndpoint"));
551-
552-
//When the parameter discovery=false is configured, prefer using the custom address. If the transportEndpoint is empty,
553-
// directly replace it with the TransportEndpoint returned by the server.
554-
if (!configuration.isDiscovery() && StringUtils.isBlank(driverContext.getTransportEndpoint())) {
555-
driverContext.setTransportEndpoint(matcher.group("transportEndpoint"));
556-
return true;
557-
}
558-
559-
if (configuration.isDiscovery() && !this.endpoints.contains(matcher.group("transportHost"))) {
560-
return false;
561-
}
562-
563-
if (!driverContext.getPort().equals(matcher.group("transportPort"))) {
564-
return false;
565-
}
566-
567-
if (!driverContext.getTransportEndpoint().equals(matcher.group("transportEndpoint"))) {
568-
return false;
556+
for (String contactPoint : contactPoints) {
557+
if (endpoint.getEndpointUrl().getStringValue().startsWith(contactPoint)) {
558+
return true;
559+
}
569560
}
570-
571-
return true;
561+
return false;
572562
}
573563

574564
/**
@@ -577,11 +567,11 @@ private boolean isEndpoint(EndpointDescription endpoint) throws PlcRuntimeExcept
577567
* @param policy - UserTokenPolicy configured for server endpoint.
578568
* @return True if given token policy matches client configuration.
579569
*/
580-
private boolean hasIdentity(UserTokenPolicy policy) {
581-
if ((policy.getTokenType() == UserTokenType.userTokenTypeAnonymous) && this.username == null) {
570+
private static boolean isUserTokenPolicyCompatible(UserTokenPolicy policy, String username) {
571+
if ((policy.getTokenType() == UserTokenType.userTokenTypeAnonymous) && username == null) {
582572
return true;
583573
}
584-
return policy.getTokenType() == UserTokenType.userTokenTypeUserName && this.username != null;
574+
return policy.getTokenType() == UserTokenType.userTokenTypeUserName && username != null;
585575
}
586576

587577
/**

plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/ChunkFactory.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030

3131
public class ChunkFactory {
3232

33-
public static int SYMMETRIC_SECURITY_HEADER_SIZE = 4;
33+
public static final int ASYMMETRIC_SECURITY_HEADER_SIZE = 59;
34+
public static final int SYMMETRIC_SECURITY_HEADER_SIZE = 4;
3435

3536
public Chunk create(boolean asymmetric, Conversation conversation) {
3637
return create(asymmetric,
@@ -48,7 +49,7 @@ public Chunk create(boolean asymmetric, boolean encrypted, boolean signed, Secur
4849

4950
if (securityPolicy == SecurityPolicy.NONE) {
5051
return new Chunk(
51-
asymmetric ? 59 : SYMMETRIC_SECURITY_HEADER_SIZE,
52+
asymmetric ? ASYMMETRIC_SECURITY_HEADER_SIZE : SYMMETRIC_SECURITY_HEADER_SIZE,
5253
1,
5354
1,
5455
securityPolicy.getSymmetricSignatureSize(),

plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/MiloTestContainer.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@ public class MiloTestContainer extends GenericContainer<MiloTestContainer> {
3131

3232
private final static Logger logger = LoggerFactory.getLogger(MiloTestContainer.class);
3333

34+
private final static ImageFromDockerfile IMAGE = inlineImage();
35+
3436
public MiloTestContainer() {
35-
super(inlineImage());
37+
super(IMAGE);
3638

3739
waitingFor(Wait.forLogMessage("Server started\\s*", 1));
38-
addFixedExposedPort(12686, 12686);
40+
addExposedPort(12686);
3941
}
4042

4143
private static ImageFromDockerfile inlineImage() {

0 commit comments

Comments
 (0)