Skip to content

Commit 8f40876

Browse files
authored
Fix unordered server endpoint validation (#4029)
### Summary This PR makes `Session.ValidateServerEndpoints()` compare `ServerEndpoints` and `UserIdentityTokens` as unordered sets and adds regression tests that verify `OpenAsync()` no longer rejects legal servers that return the same endpoint data in a different order. ### Problem According to OPC UA Part 4, the client validates the endpoint set returned by `CreateSessionResponse.ServerEndpoints` against the endpoint set observed during discovery. The specification describes these values as a set of endpoint descriptions filtered by the relevant transport profile, not as an ordered array that must preserve the exact discovery-time enumeration order. Previously, `Session.ValidateServerEndpoints()` first checked the endpoint count and then compared `m_discoveryServerEndpoints[ii]` against `serverEndpoints[ii]` by index. It also compared `UserIdentityTokens[jj]` by index within each endpoint. As a result, a server that returned the same legal endpoints in a different order, or returned the same token policies in a different order within an endpoint, could be rejected with `BadSecurityChecksFailed` even though the endpoint data itself had not changed. ### Changes - Replace index-based `ServerEndpoints` validation with unordered matching based on the same endpoint fields already validated by the client. - Compare `UserIdentityTokens` as an unordered multiset instead of requiring the server to preserve the original token enumeration order. - Extend the client session test scaffolding so discovery endpoints can be supplied explicitly. - Add regression tests that verify `OpenAsync()` accepts reordered `ServerEndpoints` and reordered `UserIdentityTokens`.
1 parent 399747f commit 8f40876

3 files changed

Lines changed: 244 additions & 49 deletions

File tree

src/Opc.Ua.Client/Session/Session.cs

Lines changed: 81 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4761,43 +4761,13 @@ private void ValidateServerEndpoints(ArrayOf<EndpointDescription> serverEndpoint
47614761
"Server did not return a number of ServerEndpoints that matches the one from GetEndpoints.");
47624762
}
47634763

4764-
for (int ii = 0; ii < expectedServerEndpoints.Count; ii++)
4764+
if (!HaveEquivalentServerEndpoints(
4765+
expectedServerEndpoints,
4766+
m_discoveryServerEndpoints))
47654767
{
4766-
EndpointDescription serverEndpoint = expectedServerEndpoints[ii];
4767-
EndpointDescription expectedServerEndpoint = m_discoveryServerEndpoints[ii];
4768-
4769-
if (serverEndpoint.SecurityMode != expectedServerEndpoint.SecurityMode ||
4770-
serverEndpoint.SecurityPolicyUri != expectedServerEndpoint
4771-
.SecurityPolicyUri ||
4772-
serverEndpoint.TransportProfileUri != expectedServerEndpoint
4773-
.TransportProfileUri ||
4774-
serverEndpoint.SecurityLevel != expectedServerEndpoint.SecurityLevel)
4775-
{
4776-
throw ServiceResultException.Create(
4777-
StatusCodes.BadSecurityChecksFailed,
4778-
"The list of ServerEndpoints returned at CreateSession does not match the list from GetEndpoints.");
4779-
}
4780-
4781-
if (serverEndpoint.UserIdentityTokens.Count != expectedServerEndpoint
4782-
.UserIdentityTokens
4783-
.Count)
4784-
{
4785-
throw ServiceResultException.Create(
4786-
StatusCodes.BadSecurityChecksFailed,
4787-
"The list of ServerEndpoints returned at CreateSession does not match the one from GetEndpoints.");
4788-
}
4789-
4790-
for (int jj = 0; jj < serverEndpoint.UserIdentityTokens.Count; jj++)
4791-
{
4792-
if (!serverEndpoint
4793-
.UserIdentityTokens[jj]
4794-
.IsEqual(expectedServerEndpoint.UserIdentityTokens[jj]))
4795-
{
4796-
throw ServiceResultException.Create(
4797-
StatusCodes.BadSecurityChecksFailed,
4798-
"The list of ServerEndpoints returned at CreateSession does not match the one from GetEndpoints.");
4799-
}
4800-
}
4768+
throw ServiceResultException.Create(
4769+
StatusCodes.BadSecurityChecksFailed,
4770+
"The list of ServerEndpoints returned at CreateSession does not match the list from GetEndpoints.");
48014771
}
48024772
}
48034773

@@ -4837,6 +4807,81 @@ private void ValidateServerEndpoints(ArrayOf<EndpointDescription> serverEndpoint
48374807
}
48384808
}
48394809

4810+
private static bool HaveEquivalentServerEndpoints(
4811+
ArrayOf<EndpointDescription> serverEndpoints,
4812+
ArrayOf<EndpointDescription> discoveryEndpoints)
4813+
{
4814+
if (serverEndpoints.Count != discoveryEndpoints.Count)
4815+
{
4816+
return false;
4817+
}
4818+
4819+
var unmatchedDiscoveryEndpoints = discoveryEndpoints.ToList();
4820+
4821+
foreach (EndpointDescription serverEndpoint in serverEndpoints)
4822+
{
4823+
int matchIndex = unmatchedDiscoveryEndpoints.FindIndex(
4824+
discoveryEndpoint => AreEquivalentServerEndpoints(
4825+
serverEndpoint,
4826+
discoveryEndpoint));
4827+
4828+
if (matchIndex < 0)
4829+
{
4830+
return false;
4831+
}
4832+
4833+
unmatchedDiscoveryEndpoints.RemoveAt(matchIndex);
4834+
}
4835+
4836+
return unmatchedDiscoveryEndpoints.Count == 0;
4837+
}
4838+
4839+
private static bool AreEquivalentServerEndpoints(
4840+
EndpointDescription serverEndpoint,
4841+
EndpointDescription discoveryEndpoint)
4842+
{
4843+
return serverEndpoint.SecurityMode == discoveryEndpoint.SecurityMode &&
4844+
string.Equals(
4845+
serverEndpoint.SecurityPolicyUri,
4846+
discoveryEndpoint.SecurityPolicyUri,
4847+
StringComparison.Ordinal) &&
4848+
string.Equals(
4849+
serverEndpoint.TransportProfileUri,
4850+
discoveryEndpoint.TransportProfileUri,
4851+
StringComparison.Ordinal) &&
4852+
serverEndpoint.SecurityLevel == discoveryEndpoint.SecurityLevel &&
4853+
HaveEquivalentUserIdentityTokens(
4854+
serverEndpoint.UserIdentityTokens,
4855+
discoveryEndpoint.UserIdentityTokens);
4856+
}
4857+
4858+
private static bool HaveEquivalentUserIdentityTokens(
4859+
ArrayOf<UserTokenPolicy> serverTokens,
4860+
ArrayOf<UserTokenPolicy> discoveryTokens)
4861+
{
4862+
if (serverTokens.Count != discoveryTokens.Count)
4863+
{
4864+
return false;
4865+
}
4866+
4867+
var unmatchedDiscoveryTokens = discoveryTokens.ToList();
4868+
4869+
foreach (UserTokenPolicy serverToken in serverTokens)
4870+
{
4871+
int matchIndex = unmatchedDiscoveryTokens.FindIndex(
4872+
discoveryToken => serverToken.IsEqual(discoveryToken));
4873+
4874+
if (matchIndex < 0)
4875+
{
4876+
return false;
4877+
}
4878+
4879+
unmatchedDiscoveryTokens.RemoveAt(matchIndex);
4880+
}
4881+
4882+
return unmatchedDiscoveryTokens.Count == 0;
4883+
}
4884+
48404885
/// <summary>
48414886
/// Find and return matching application description
48424887
/// </summary>

tests/Opc.Ua.Client.TestFramework/SessionMock.cs

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,33 @@ public SessionMock(
6868
Channel = channel;
6969
}
7070

71+
public SessionMock(
72+
Mock<ITransportChannel> channel,
73+
ApplicationConfiguration configuration,
74+
ConfiguredEndpoint endpoint,
75+
ArrayOf<EndpointDescription> availableEndpoints,
76+
ArrayOf<string> discoveryProfileUris = default)
77+
: base(
78+
channel.Object,
79+
configuration,
80+
endpoint,
81+
clientCertificate: null,
82+
clientCertificateChain: null,
83+
availableEndpoints: availableEndpoints,
84+
discoveryProfileUris: discoveryProfileUris,
85+
engineFactory: null)
86+
{
87+
Channel = channel;
88+
}
89+
7190
/// <summary>
7291
/// Create default mock
7392
/// </summary>
7493
/// <returns></returns>
75-
public static SessionMock Create(EndpointDescription endpoint = null)
94+
public static SessionMock Create(
95+
EndpointDescription endpoint = null,
96+
ArrayOf<EndpointDescription> availableEndpoints = default,
97+
ArrayOf<string> discoveryProfileUris = default)
7698
{
7799
ITelemetryContext telemetry = NUnitTelemetryContext.Create();
78100
var channel = new Mock<ITransportChannel>();
@@ -105,18 +127,30 @@ public static SessionMock Create(EndpointDescription endpoint = null)
105127
application.CheckApplicationInstanceCertificatesAsync(true).AsTask().GetAwaiter().GetResult();
106128
}
107129

108-
return new SessionMock(channel, configuration,
109-
new ConfiguredEndpoint(null, endpoint ??
110-
new EndpointDescription
111-
{
112-
SecurityMode = MessageSecurityMode.None,
113-
SecurityPolicyUri = SecurityPolicies.None,
114-
EndpointUrl = "opc.tcp://localhost:4840",
115-
UserIdentityTokens =
116-
[
117-
new UserTokenPolicy()
118-
]
119-
}));
130+
var configuredEndpoint = new ConfiguredEndpoint(
131+
null,
132+
endpoint ?? new EndpointDescription
133+
{
134+
SecurityMode = MessageSecurityMode.None,
135+
SecurityPolicyUri = SecurityPolicies.None,
136+
EndpointUrl = "opc.tcp://localhost:4840",
137+
UserIdentityTokens =
138+
[
139+
new UserTokenPolicy()
140+
]
141+
});
142+
143+
if (availableEndpoints.IsEmpty && discoveryProfileUris.IsEmpty)
144+
{
145+
return new SessionMock(channel, configuration, configuredEndpoint);
146+
}
147+
148+
return new SessionMock(
149+
channel,
150+
configuration,
151+
configuredEndpoint,
152+
availableEndpoints,
153+
discoveryProfileUris);
120154
}
121155

122156
public void SetConnected()

tests/Opc.Ua.Client.Tests/Session/SessionTests.cs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,6 +1582,78 @@ public async Task OpenAsyncShouldOpenSessionSuccessfullyAsync()
15821582
sut.Channel.Verify();
15831583
}
15841584

1585+
[Test]
1586+
public async Task OpenAsyncAcceptsServerEndpointsWithDifferentOrderingAsync()
1587+
{
1588+
EndpointDescription primaryEndpoint = CreateSessionEndpointDescription(
1589+
"opc.tcp://localhost:4840");
1590+
primaryEndpoint.SecurityLevel = 1;
1591+
primaryEndpoint.TransportProfileUri = Profiles.UaTcpTransport;
1592+
1593+
EndpointDescription secondaryEndpoint = CreateSessionEndpointDescription(
1594+
"opc.tcp://localhost:4841");
1595+
secondaryEndpoint.SecurityMode = MessageSecurityMode.Sign;
1596+
secondaryEndpoint.SecurityPolicyUri = SecurityPolicies.Basic256Sha256;
1597+
secondaryEndpoint.SecurityLevel = 2;
1598+
secondaryEndpoint.TransportProfileUri = Profiles.UaTcpTransport;
1599+
1600+
using var sut = SessionMock.Create(
1601+
primaryEndpoint,
1602+
[primaryEndpoint, secondaryEndpoint],
1603+
[Profiles.UaTcpTransport]);
1604+
1605+
ConfigureSuccessfulOpenResponses(
1606+
sut.Channel,
1607+
[secondaryEndpoint, primaryEndpoint],
1608+
ByteString.From([1, 2, 3, 4]),
1609+
NodeId.Parse("s=cookie"));
1610+
1611+
await sut.OpenAsync("test", new UserIdentity(), CancellationToken.None)
1612+
.ConfigureAwait(false);
1613+
1614+
Assert.That(sut.ServerNonce, Is.EqualTo(ByteString.From([1, 2, 3, 4])));
1615+
sut.Channel.Verify();
1616+
}
1617+
1618+
[Test]
1619+
public async Task OpenAsyncAcceptsUserTokenPoliciesWithDifferentOrderingAsync()
1620+
{
1621+
EndpointDescription discoveryEndpoint = CreateSessionEndpointDescription(
1622+
"opc.tcp://localhost:4840");
1623+
discoveryEndpoint.TransportProfileUri = Profiles.UaTcpTransport;
1624+
discoveryEndpoint.UserIdentityTokens =
1625+
[
1626+
CreateUserTokenPolicy("anonymous", UserTokenType.Anonymous),
1627+
CreateUserTokenPolicy("username", UserTokenType.UserName)
1628+
];
1629+
1630+
EndpointDescription responseEndpoint = CreateSessionEndpointDescription(
1631+
"opc.tcp://localhost:4840");
1632+
responseEndpoint.TransportProfileUri = Profiles.UaTcpTransport;
1633+
responseEndpoint.UserIdentityTokens =
1634+
[
1635+
CreateUserTokenPolicy("username", UserTokenType.UserName),
1636+
CreateUserTokenPolicy("anonymous", UserTokenType.Anonymous)
1637+
];
1638+
1639+
using var sut = SessionMock.Create(
1640+
discoveryEndpoint,
1641+
[discoveryEndpoint],
1642+
[Profiles.UaTcpTransport]);
1643+
1644+
ConfigureSuccessfulOpenResponses(
1645+
sut.Channel,
1646+
[responseEndpoint],
1647+
ByteString.From([1, 2, 3, 4]),
1648+
NodeId.Parse("s=cookie"));
1649+
1650+
await sut.OpenAsync("test", new UserIdentity(), CancellationToken.None)
1651+
.ConfigureAwait(false);
1652+
1653+
Assert.That(sut.ServerNonce, Is.EqualTo(ByteString.From([1, 2, 3, 4])));
1654+
sut.Channel.Verify();
1655+
}
1656+
15851657
[Test]
15861658
public void OpenAsyncShouldHandleCreateSessionSuccessButActivationError()
15871659
{
@@ -2036,6 +2108,18 @@ private static EndpointDescription CreateSessionEndpointDescription(string endpo
20362108
};
20372109
}
20382110

2111+
private static UserTokenPolicy CreateUserTokenPolicy(
2112+
string policyId,
2113+
UserTokenType tokenType)
2114+
{
2115+
return new UserTokenPolicy
2116+
{
2117+
PolicyId = policyId,
2118+
TokenType = tokenType,
2119+
SecurityPolicyUri = SecurityPolicies.None
2120+
};
2121+
}
2122+
20392123
private static Mock<ITransportChannel> CreateReconnectChannelMock(
20402124
SessionMock session,
20412125
EndpointDescription endpointDescription)
@@ -2051,6 +2135,38 @@ private static Mock<ITransportChannel> CreateReconnectChannelMock(
20512135
return channel;
20522136
}
20532137

2138+
private static void ConfigureSuccessfulOpenResponses(
2139+
Mock<ITransportChannel> channel,
2140+
ArrayOf<EndpointDescription> serverEndpoints,
2141+
ByteString serverNonce,
2142+
NodeId authToken)
2143+
{
2144+
channel
2145+
.Setup(c => c.SendRequestAsync(
2146+
It.IsAny<CreateSessionRequest>(),
2147+
It.IsAny<CancellationToken>()))
2148+
.Returns(new ValueTask<IServiceResponse>(new CreateSessionResponse
2149+
{
2150+
ServerNonce = serverNonce,
2151+
SessionId = NodeId.Parse("s=connected"),
2152+
AuthenticationToken = authToken,
2153+
ServerEndpoints = serverEndpoints
2154+
}));
2155+
2156+
channel
2157+
.Setup(c => c.SendRequestAsync(
2158+
It.Is<ActivateSessionRequest>(r => r.RequestHeader.AuthenticationToken == authToken),
2159+
It.IsAny<CancellationToken>()))
2160+
.Returns(new ValueTask<IServiceResponse>(new ActivateSessionResponse
2161+
{
2162+
ServerNonce = serverNonce,
2163+
Results = [],
2164+
DiagnosticInfos = []
2165+
}));
2166+
2167+
ConfigureOpenAsyncReadResponses(channel);
2168+
}
2169+
20542170
private static void ConfigureOpenAsyncReadResponses(Mock<ITransportChannel> channel)
20552171
{
20562172
channel

0 commit comments

Comments
 (0)