dohertj2/lmxopcua
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): a driver name may not claim another driver's credential segment, nor be renamed off its own (#848)

Browse Source 
RawTreeService.CreateDriverAsync / RenameDriverAsync / UpdateDriverAsync run
DriverConfigSaveGate.ValidateNameAsync -> DriverInstanceIdentities.DescribeNameChangeProblemsAsync:
- claim: the new name may not be the instance part (secret: segment or env: part)
  of a reference STORED on another same-type driver's DriverConfig or DeviceConfig -
  naming a driver 'gw' used to fail every sharer's next deploy;
- pin: a driver may not be renamed away from a name its own references are named
  after (unless they also name its id or the new name) - the rename would free that
  name for every other driver's save / Test Connect / browse. Re-pointing to the id
  in the same config-modal save is allowed; a case-only rename is never refused.
Messages name the other driver, id, device and field path, never a value. The
driver + device reads are pinned on SQL Server by a [SqlServerFact].

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-11-11 06:24:59 -05:00
parent 4a6229e4d2
commit d08e59eea7
5 changed files with 347 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DriverInstanceIdentities.cs
+97 -1
View File
@@ -86,9 +86,105 @@ public static class DriverInstanceIdentities
];
}
/// <summary>
/// lmxopcua#848: the problems of giving a driver the name <paramref name="newName"/> (create, or a rename via the
/// /raw rename or the driver config modal). Two rules, both about a name being the instance part of a stored
/// credential reference (<see cref="DriverCredentialPolicy.ReferenceNames"/>):
/// <list type="bullet">
/// <item>
/// <b>Claim</b> — the new name may not be named by a reference STORED on another same-type driver (its
/// DriverConfig or any DeviceConfig). Otherwise that reference would start naming "another instance" and the
/// other driver's next deploy fails (naming a driver <c>gw</c> when others share <c>secret:galaxy/gw/…</c>).
/// </item>
/// <item>
/// <b>Pin</b> — a driver may not be renamed AWAY from a name its own references are named after (unless they
/// also name its immutable id or the new name): renaming would free the old name, and a reference to it would
/// then look like an unclaimed shared name to every other driver's Test Connect, browse and save.
/// </item>
/// </list>
/// A case-only rename changes nothing the rule matches, so it is never refused. Messages name drivers, devices and
/// field paths — never a credential value.
/// </summary>
/// <param name="db">The config DB.</param>
/// <param name="driverType">The driver's type.</param>
/// <param name="ownDriverInstanceId">The driver being renamed; null when it is being created.</param>
/// <param name="currentName">Its stored name; null when it is being created.</param>
/// <param name="newName">The name about to be stored.</param>
/// <param name="ownDriverConfig">
/// The DriverConfig being saved with the rename (the config modal saves both at once); null ⇒ the stored one.
/// </param>
/// <param name="ct">Cancellation.</param>
/// <returns>The problems; empty when the name may be stored.</returns>
public static async Task<IReadOnlyList<string>> DescribeNameChangeProblemsAsync(
OtOpcUaConfigDbContext db, string? driverType, string? ownDriverInstanceId, string? currentName, string newName,
string? ownDriverConfig, CancellationToken ct)
{
ArgumentNullException.ThrowIfNull(db);
if (!DriverCredentialPolicy.TryGet(driverType, out var rules)) return [];
if (currentName is not null && string.Equals(currentName, newName, StringComparison.OrdinalIgnoreCase)) return [];
// Same shape as LoadAsync: the type filter stays client-side (case-insensitive), the device read is a plain IN.
var drivers = (await db.DriverInstances.AsNoTracking()
.Select(d => new { d.DriverInstanceId, d.Name, d.DriverType, d.DriverConfig })
.ToListAsync(ct).ConfigureAwait(false))
.Where(d => string.Equals(d.DriverType, rules.DriverType, StringComparison.OrdinalIgnoreCase))
.ToList();
var ids = drivers.Select(d => d.DriverInstanceId).ToList();
var devices = ids.Count == 0
? []
: await db.Devices.AsNoTracking()
.Where(d => ids.Contains(d.DriverInstanceId))
.Select(d => new { d.DriverInstanceId, d.Name, d.DeviceConfig })
.ToListAsync(ct).ConfigureAwait(false);
var problems = new List<string>();
foreach (var driver in drivers)
{
var own = string.Equals(driver.DriverInstanceId, ownDriverInstanceId, StringComparison.Ordinal);
var configs = new List<(string? Device, string? Json)>
{
(null, own && ownDriverConfig is not null ? ownDriverConfig : driver.DriverConfig),
};
configs.AddRange(devices
.Where(d => string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal))
.Select(d => ((string?)d.Name, (string?)d.DeviceConfig)));
foreach (var (device, json) in configs)
{
var where = device is null ? "" : $" device '{device}'";
foreach (var reference in DriverCredentialPolicy.CollectInstanceReferences(driverType, json))
{
if (!own && DriverCredentialPolicy.ReferenceNames(reference, newName))
{
problems.Add(
$"The name '{newName}' is the instance part of the credential reference {reference.FieldPath} on "
+ $"{rules.Scope.Label} driver '{driver.Name}' ({driver.DriverInstanceId}){where} — naming this driver "
+ "so would make that reference name another driver instance and fail that driver's next deploy. "
+ "Choose another name, or move that reference first (lmxopcua#848).");
}
else if (own && currentName is not null
&& DriverCredentialPolicy.ReferenceNames(reference, currentName)
&& !DriverCredentialPolicy.ReferenceNames(reference, ownDriverInstanceId)
&& !DriverCredentialPolicy.ReferenceNames(reference, newName))
{
problems.Add(
$"This driver's credential reference {reference.FieldPath}{where} is named after its current name "
+ $"'{currentName}' — renaming would free that name, and any other {rules.Scope.Label} driver could then "
+ "reference the same credential. Move the reference to this driver's id first "
+ $"(secret:{rules.Scope.SecretNamespaces[0]}{ownDriverInstanceId}/… or "
+ $"env:{rules.Scope.EnvVariablePrefixes[0]}{DriverCredentialPolicy.NormalizeEnvironmentIdentifier(ownDriverInstanceId)}_…), "
+ "then rename (lmxopcua#848).");
}
}
}
}
return problems;
}
/// <summary>
/// The cross-instance problems of <paramref name="configJson"/> — reading the ConfigDb only when the config carries
/// an in-scope <c>secret:</c> reference at all.
/// an in-scope <c>secret:</c> or <c>env:</c> reference at all.
/// </summary>
/// <param name="db">The config DB.</param>
/// <param name="driverType">The owning driver's type.</param>
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs
+25 -1
View File
@@ -34,7 +34,7 @@ public static class DriverConfigSaveGate
/// plus the cross-instance rule (lmxopcua#835 — a <c>secret:galaxy/&lt;X&gt;/…</c> reference may not name ANOTHER
/// existing instance of the same driver type), which needs the stored instance list.
/// </summary>
/// <param name="db">The config DB (read only when the config carries an in-scope <c>secret:</c> reference).</param>
/// <param name="db">The config DB (read only when the config carries an in-scope <c>secret:</c>/<c>env:</c> reference).</param>
/// <param name="driverType">The owning driver's type.</param>
/// <param name="configJson">The DriverConfig or DeviceConfig JSON about to be stored.</param>
/// <param name="ownDriverInstanceId">The owning driver; null when it is being created.</param>
@@ -48,4 +48,28 @@ public static class DriverConfigSaveGate
.DescribeOtherInstanceProblemsAsync(db, driverType, configJson, ownDriverInstanceId, ct).ConfigureAwait(false);
return problems.Count == 0 ? null : string.Join(" ", problems);
}
/// <summary>
/// The authority check <see cref="RawTreeService"/> runs on every driver create and rename (lmxopcua#848,
/// <see cref="DriverInstanceIdentities.DescribeNameChangeProblemsAsync"/>): the new name may not be the instance part
/// of another same-type driver's stored credential reference, and a rename may not free a name the driver's own
/// references are named after.
/// </summary>
/// <param name="db">The config DB.</param>
/// <param name="driverType">The driver's type.</param>
/// <param name="ownDriverInstanceId">The driver being renamed; null on create.</param>
/// <param name="currentName">Its stored name; null on create.</param>
/// <param name="newName">The name about to be stored.</param>
/// <param name="ownDriverConfig">The DriverConfig saved with it; null ⇒ the stored one.</param>
/// <param name="ct">Cancellation.</param>
/// <returns>The refusal (never containing a credential value), or null.</returns>
public static async Task<string?> ValidateNameAsync(
OtOpcUaConfigDbContext db, string? driverType, string? ownDriverInstanceId, string? currentName, string newName,
string? ownDriverConfig, CancellationToken ct)
{
var problems = await DriverInstanceIdentities
.DescribeNameChangeProblemsAsync(db, driverType, ownDriverInstanceId, currentName, newName, ownDriverConfig, ct)
.ConfigureAwait(false);
return problems.Count == 0 ? null : string.Join(" ", problems);
}
}
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs
+15
View File
@@ -492,6 +492,10 @@ public sealed class RawTreeService(IDbContextFactory<OtOpcUaConfigDbContext> dbF
if (await DriverConfigSaveGate.ValidateAsync(db, driverType, driverConfigJson, null, ct) is { } instanceRefusal)
return new UnsMutationResult(false, instanceRefusal);
// lmxopcua#848: nor may its NAME be the instance part of another driver's stored reference.
if (await DriverConfigSaveGate.ValidateNameAsync(db, driverType, null, null, name, driverConfigJson, ct) is { } nameRefusal)
return new UnsMutationResult(false, nameRefusal);
if (!await db.ServerClusters.AnyAsync(c => c.ClusterId == clusterId, ct))
return new UnsMutationResult(false, $"Cluster '{clusterId}' not found.");
@@ -549,6 +553,12 @@ public sealed class RawTreeService(IDbContextFactory<OtOpcUaConfigDbContext> dbF
&& d.Name == newName && d.DriverInstanceId != driverInstanceId, ct))
return RawRenameResult.Failure($"A driver named '{newName}' already exists here.");
// lmxopcua#848: the new name may not be another driver's credential segment, nor may this rename free a name
// the driver's own credential references are named after.
if (await DriverConfigSaveGate.ValidateNameAsync(
db, entity.DriverType, entity.DriverInstanceId, entity.Name, newName, null, ct) is { } nameRefusal)
return RawRenameResult.Failure(nameRefusal);
var warnings = await BuildRenameWarningsAsync(db, await CollectTagsBeneathDriverAsync(db, driverInstanceId, ct), ct);
db.Entry(entity).Property(e => e.RowVersion).OriginalValue = rowVersion;
@@ -1129,6 +1139,11 @@ public sealed class RawTreeService(IDbContextFactory<OtOpcUaConfigDbContext> dbF
&& d.Name == name && d.DriverInstanceId != driverInstanceId, ct))
return new UnsMutationResult(false, $"A driver named '{name}' already exists here.");
// lmxopcua#848 — as RenameDriverAsync, judged against the config being saved with the rename.
if (await DriverConfigSaveGate.ValidateNameAsync(
db, entity.DriverType, entity.DriverInstanceId, entity.Name, name, driverConfigJson, ct) is { } nameRefusal)
return new UnsMutationResult(false, nameRefusal);
db.Entry(entity).Property(e => e.RowVersion).OriginalValue = rowVersion;
entity.Name = name;
entity.DriverConfig = string.IsNullOrWhiteSpace(driverConfigJson) ? "{}" : driverConfigJson;