Skip to content

Commit 2a1db65

Browse files
authored
Fix legacy password verification and JSON QualifiedName decoding (#3991)
# Description Fix two compatibility and decoding regressions: - Verify persisted PBKDF2 password hashes using the iteration count stored with each hash. This preserves authentication for user databases created with the previous 10,000-iteration policy when running on .NET 10. - Normalize malformed string-form JSON `QualifiedName` values through the decoder's existing strict/lenient contract. Lenient parsing now returns `QualifiedName.Null`, while strict parsing reports `BadDecodingError` instead of leaking `BadNodeIdInvalid`. The change adds deterministic legacy/current password-hash vectors and scalar, array, and Variant JSON decoder coverage. ## Related Issues - No related issue. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ## Validation - `LinqUserDatabaseTests`: 10/10 passed on `net10.0` and `net48`. - `JsonDecoderTests`: 262/262 passed on `net10.0` and `net48`. - Deterministic JSON fuzz regression tests: 1368/1368 passed on `net10.0` and `net48`. - Affected test projects build with zero warnings and errors on `net10.0` and `net48`.
1 parent aadba8b commit 2a1db65

4 files changed

Lines changed: 286 additions & 7 deletions

File tree

Libraries/Opc.Ua.Server/RoleBasedUserManagement/UserDatabase/LinqUserDatabase.cs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ public class LinqUserDatabase : IUserDatabase
5858
/// </summary>
5959
private const int kKeySize = 32;
6060

61+
/// <summary>
62+
/// Upper bound for the persisted PBKDF2 iteration count accepted during
63+
/// verification. Guards against corrupt or malicious hash records that would
64+
/// otherwise cause unbounded key-derivation work.
65+
/// </summary>
66+
private const int kMaxIterations = 10_000_000;
67+
6168
/// <summary>
6269
/// The representation of a user in the Linq database.
6370
/// </summary>
@@ -326,15 +333,33 @@ private static bool Check(string hash, ReadOnlySpan<byte> password)
326333
"Unexpected hash format. Should be formatted as `{iterations}.{salt}.{hash}`");
327334
}
328335

329-
int iterations = Convert.ToInt32(parts[0], CultureInfo.InvariantCulture.NumberFormat);
336+
if (!int.TryParse(
337+
parts[0],
338+
NumberStyles.Integer,
339+
CultureInfo.InvariantCulture,
340+
out int iterations) ||
341+
iterations is <= 0 or > kMaxIterations)
342+
{
343+
// A non-positive or excessively large iteration count cannot come from
344+
// Hash() and would either throw or cause unbounded CPU work below. Fail
345+
// closed so a corrupt or malicious hash record cannot authenticate.
346+
return false;
347+
}
348+
330349
byte[] salt = Convert.FromBase64String(parts[1]);
331350
byte[] key = Convert.FromBase64String(parts[2]);
351+
if (key.Length == 0)
352+
{
353+
// An empty derived key cannot come from Hash() and would make the
354+
// net10 span overload derive zero bytes; reject it explicitly.
355+
return false;
356+
}
332357
#if NET10_0_OR_GREATER
333358
// Use span overloads
334359
byte[] keyToCheck = Rfc2898DeriveBytes.Pbkdf2(
335360
password,
336361
salt,
337-
kIterations,
362+
iterations,
338363
HashAlgorithmName.SHA512,
339364
key.Length);
340365
return keyToCheck.SequenceEqual(key);

Stack/Opc.Ua.Types/Encoders/JsonDecoder.cs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2455,11 +2455,20 @@ private bool TryGetQualifiedNameFromElement(
24552455
value = QualifiedName.Null;
24562456
return true;
24572457
case JsonValueKind.String:
2458-
value = QualifiedName.Parse(
2459-
Context,
2460-
element.GetString()!,
2461-
m_options.UpdateNamespaceTable);
2462-
return true;
2458+
try
2459+
{
2460+
value = QualifiedName.Parse(
2461+
Context,
2462+
element.GetString()!,
2463+
m_options.UpdateNamespaceTable);
2464+
return true;
2465+
}
2466+
catch (ServiceResultException sre)
2467+
when (sre.StatusCode == StatusCodes.BadNodeIdInvalid)
2468+
{
2469+
value = QualifiedName.Null;
2470+
return false;
2471+
}
24632472
default:
24642473
value = QualifiedName.Null;
24652474
return false;

Tests/Opc.Ua.Server.Tests/LinqUserDatabaseTests.cs

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,32 @@
1+
/* ========================================================================
2+
* Copyright (c) 2005-2025 The OPC Foundation, Inc. All rights reserved.
3+
*
4+
* OPC Foundation MIT License 1.00
5+
*
6+
* Permission is hereby granted, free of charge, to any person
7+
* obtaining a copy of this software and associated documentation
8+
* files (the "Software"), to deal in the Software without
9+
* restriction, including without limitation the rights to use,
10+
* copy, modify, merge, publish, distribute, sublicense, and/or sell
11+
* copies of the Software, and to permit persons to whom the
12+
* Software is furnished to do so, subject to the following
13+
* conditions:
14+
*
15+
* The above copyright notice and this permission notice shall be
16+
* included in all copies or substantial portions of the Software.
17+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
18+
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
19+
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
20+
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
21+
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
22+
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
23+
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
24+
* OTHER DEALINGS IN THE SOFTWARE.
25+
*
26+
* The complete license agreement can be found here:
27+
* http://opcfoundation.org/License/MIT/1.00/
28+
* ======================================================================*/
29+
130
using System;
231
using System.Collections.Generic;
332
using System.Linq;
@@ -15,6 +44,23 @@ public sealed class LinqUserDatabaseTests
1544
{
1645
private static readonly string[] s_createdUsers = ["TestUser1", "TestUser2"];
1746

47+
/// <summary>
48+
/// Deterministic PBKDF2-SHA512 vector persisted with 10,000 iterations, generated
49+
/// offline for the password "Correct.Horse.Battery9!" using the persisted format
50+
/// `{iterations}.{saltBase64}.{keyBase64}`. Represents a legacy hash created before
51+
/// the default iteration count was raised to 100,000.
52+
/// </summary>
53+
private const string kPbkdf2Vector10000Iterations =
54+
"10000.AQIDBAUGBwgJCgsMDQ4PEBESExQ=.m11DHtGqddtFRM3j/jT625QRQ94Wr77OQ6JVDB3wkOM=";
55+
56+
/// <summary>
57+
/// Deterministic PBKDF2-SHA512 vector persisted with 100,000 iterations, generated
58+
/// offline for the password "Correct.Horse.Battery9!" using the persisted format
59+
/// `{iterations}.{saltBase64}.{keyBase64}`. Matches the current default iteration count.
60+
/// </summary>
61+
private const string kPbkdf2Vector100000Iterations =
62+
"100000.FRYXGBkaGxwdHh8gISIjJCUmJyg=.ZTa2BfHDODFt/dLxiIyZQECswP3Rzd3KCmu6TxeE3Wo=";
63+
1864
[Test]
1965
public void CreateInvalidUser()
2066
{
@@ -138,5 +184,131 @@ public void GetUsersReturnsCreatedUsers()
138184
Assert.That(users.All(user => user.UserConfiguration == (uint)UserConfigurationMask.None), Is.True);
139185
Assert.That(users.Select(user => user.Description), Is.All.Empty);
140186
}
187+
188+
[Test]
189+
public void CheckCredentialsSucceedsForPersistedPbkdf2VectorWith10000Iterations()
190+
{
191+
// Arrange
192+
var usersDb = new LinqUserDatabase
193+
{
194+
Users =
195+
[
196+
new LinqUserDatabase.User
197+
{
198+
ID = Guid.NewGuid(),
199+
UserName = "LegacyUser10000",
200+
Hash = kPbkdf2Vector10000Iterations,
201+
Roles = [Role.AuthenticatedUser]
202+
}
203+
]
204+
};
205+
206+
// Act
207+
bool correctPassword = usersDb.CheckCredentials(
208+
"LegacyUser10000",
209+
"Correct.Horse.Battery9!"u8);
210+
bool wrongPassword = usersDb.CheckCredentials(
211+
"LegacyUser10000",
212+
"Wrong.Horse.Battery9!"u8);
213+
214+
// Assert
215+
Assert.That(correctPassword, Is.True);
216+
Assert.That(wrongPassword, Is.False);
217+
}
218+
219+
[Test]
220+
public void CheckCredentialsSucceedsForPersistedPbkdf2VectorWith100000Iterations()
221+
{
222+
// Arrange
223+
var usersDb = new LinqUserDatabase
224+
{
225+
Users =
226+
[
227+
new LinqUserDatabase.User
228+
{
229+
ID = Guid.NewGuid(),
230+
UserName = "CurrentUser100000",
231+
Hash = kPbkdf2Vector100000Iterations,
232+
Roles = [Role.AuthenticatedUser]
233+
}
234+
]
235+
};
236+
237+
// Act
238+
bool correctPassword = usersDb.CheckCredentials(
239+
"CurrentUser100000",
240+
"Correct.Horse.Battery9!"u8);
241+
bool wrongPassword = usersDb.CheckCredentials(
242+
"CurrentUser100000",
243+
"Wrong.Horse.Battery9!"u8);
244+
245+
// Assert
246+
Assert.That(correctPassword, Is.True);
247+
Assert.That(wrongPassword, Is.False);
248+
}
249+
250+
[Theory]
251+
[TestCase("0")]
252+
[TestCase("-1")]
253+
[TestCase("100000000")]
254+
[TestCase("99999999999999999999")]
255+
[TestCase("notanumber")]
256+
public void CheckCredentialsReturnsFalseForHashWithInvalidIterationCount(string iterations)
257+
{
258+
// Arrange - reuse the valid salt/key segments from a known-good vector but
259+
// replace the iteration count with an out-of-range or unparsable value.
260+
string[] parts = kPbkdf2Vector100000Iterations.Split('.');
261+
string corruptHash = $"{iterations}.{parts[1]}.{parts[2]}";
262+
var usersDb = new LinqUserDatabase
263+
{
264+
Users =
265+
[
266+
new LinqUserDatabase.User
267+
{
268+
ID = Guid.NewGuid(),
269+
UserName = "CorruptIterationsUser",
270+
Hash = corruptHash,
271+
Roles = [Role.AuthenticatedUser]
272+
}
273+
]
274+
};
275+
276+
// Act
277+
bool result = usersDb.CheckCredentials(
278+
"CorruptIterationsUser",
279+
"Correct.Horse.Battery9!"u8);
280+
281+
// Assert - fail closed, never throw.
282+
Assert.That(result, Is.False);
283+
}
284+
285+
[Test]
286+
public void CheckCredentialsReturnsFalseForHashWithEmptyKey()
287+
{
288+
// Arrange - a persisted hash with an empty derived key segment.
289+
string[] parts = kPbkdf2Vector100000Iterations.Split('.');
290+
string corruptHash = $"{parts[0]}.{parts[1]}.";
291+
var usersDb = new LinqUserDatabase
292+
{
293+
Users =
294+
[
295+
new LinqUserDatabase.User
296+
{
297+
ID = Guid.NewGuid(),
298+
UserName = "EmptyKeyUser",
299+
Hash = corruptHash,
300+
Roles = [Role.AuthenticatedUser]
301+
}
302+
]
303+
};
304+
305+
// Act
306+
bool result = usersDb.CheckCredentials(
307+
"EmptyKeyUser",
308+
"Correct.Horse.Battery9!"u8);
309+
310+
// Assert - fail closed, never throw.
311+
Assert.That(result, Is.False);
312+
}
141313
}
142314
}

Tests/Opc.Ua.Types.Tests/Encoders/JsonDecoderTests.cs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,6 +1415,79 @@ public void ReadQualifiedNameWithInvalidTypeValue()
14151415
Assert.That(result, Is.EqualTo(QualifiedName.Null));
14161416
}
14171417

1418+
[Test]
1419+
public void ReadQualifiedNameWithMalformedNamespaceUri()
1420+
{
1421+
// "nsu=urn:test" is missing the mandatory ';' separator before the name.
1422+
using JsonDecoder reader = NewDecoder(Body(@"""nsu=urn:test"""));
1423+
QualifiedName result = reader.ReadQualifiedName(JsonProperties.Value);
1424+
Assert.That(result, Is.EqualTo(QualifiedName.Null));
1425+
}
1426+
1427+
[Test]
1428+
public void ReadQualifiedNameWithMalformedNamespaceUriThrowsWhenStrict()
1429+
{
1430+
using JsonDecoder reader = NewDecoder(Body(@"""nsu=urn:test"""), true);
1431+
try
1432+
{
1433+
reader.ReadQualifiedName(JsonProperties.Value);
1434+
}
1435+
catch (ServiceResultException sre)
1436+
{
1437+
Assert.That(sre.StatusCode, Is.EqualTo(StatusCodes.BadDecodingError));
1438+
return;
1439+
}
1440+
Assert.Fail("Exception not thrown");
1441+
}
1442+
1443+
[Test]
1444+
public void ReadQualifiedNameArrayWithMalformedNamespaceUri()
1445+
{
1446+
using JsonDecoder reader = NewDecoder(Body(/*lang=json,strict*/ """[ "nsu=urn:test" ]"""));
1447+
ArrayOf<QualifiedName> result = reader.ReadQualifiedNameArray(JsonProperties.Value);
1448+
Assert.That(result, Is.EqualTo(ArrayOf.Null<QualifiedName>()));
1449+
}
1450+
1451+
[Test]
1452+
public void ReadQualifiedNameArrayWithMalformedNamespaceUriThrowsWhenStrict()
1453+
{
1454+
using JsonDecoder reader = NewDecoder(Body(/*lang=json,strict*/ """[ "nsu=urn:test" ]"""), true);
1455+
try
1456+
{
1457+
reader.ReadQualifiedNameArray(JsonProperties.Value);
1458+
}
1459+
catch (ServiceResultException sre)
1460+
{
1461+
Assert.That(sre.StatusCode, Is.EqualTo(StatusCodes.BadDecodingError));
1462+
return;
1463+
}
1464+
Assert.Fail("Exception not thrown");
1465+
}
1466+
1467+
[Test]
1468+
public void ReadVariantWithQualifiedNameMalformedNamespaceUri()
1469+
{
1470+
using JsonDecoder reader = NewDecoder(Body(/*lang=json,strict*/ """{"UaType":20, "Value":"nsu=urn:test"}"""));
1471+
Variant result = reader.ReadVariant(JsonProperties.Value);
1472+
Assert.That(result, Is.EqualTo(Variant.Null));
1473+
}
1474+
1475+
[Test]
1476+
public void ReadVariantWithQualifiedNameMalformedNamespaceUriThrowsWhenStrict()
1477+
{
1478+
using JsonDecoder reader = NewDecoder(Body(/*lang=json,strict*/ """{"UaType":20, "Value":"nsu=urn:test"}"""), true);
1479+
try
1480+
{
1481+
reader.ReadVariant(JsonProperties.Value);
1482+
}
1483+
catch (ServiceResultException sre)
1484+
{
1485+
Assert.That(sre.StatusCode, Is.EqualTo(StatusCodes.BadDecodingError));
1486+
return;
1487+
}
1488+
Assert.Fail("Exception not thrown");
1489+
}
1490+
14181491
[Test]
14191492
public void ReadSByteArrayWithBadStringValue()
14201493
{

0 commit comments

Comments
 (0)