fix(configuration-database): resolve ConfigurationDatabase-012 — store inbound-API keys as HMAC-SHA256 hashes
Inbound-API bearer credentials are no longer persisted in plaintext. ApiKey now holds a KeyHash (peppered HMAC-SHA256); the key is shown once at creation and only its hash is stored. Lookup and validation hash the presented candidate. Cross-module: Commons (ApiKey, ApiKeyHasher), ConfigurationDatabase (mapping + HashApiKeyValue migration), InboundAPI (ApiKeyValidator), ManagementService (key creation), CentralUI (ApiKeys.razor). Existing keys must be re-issued.
This commit is contained in:
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-17 |
|
| Last reviewed | 2026-05-17 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `39d737e` |
|
| Commit reviewed | `39d737e` |
|
||||||
| Open findings | 1 |
|
| Open findings | 0 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -599,7 +599,7 @@ Regression: `Constructor_NullContext_Throws` tests were added for all four affec
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Medium |
|
| Severity | Medium |
|
||||||
| Category | Security |
|
| Category | Security |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ConfigurationDatabase/Configurations/InboundApiConfiguration.cs:17-19` |
|
| Location | `src/ScadaLink.ConfigurationDatabase/Configurations/InboundApiConfiguration.cs:17-19` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -636,42 +636,66 @@ key value redacted.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Open — requires a cross-module design decision that cannot be made within this
|
Resolved 2026-05-17 (commit `<pending>`) — approved cross-module change spanning
|
||||||
module's editable scope._
|
Commons + ConfigurationDatabase + InboundAPI + ManagementService. The inbound-API
|
||||||
|
bearer credential is now persisted as a deterministic HMAC-SHA256 keyed hash, never
|
||||||
|
as plaintext, so a configuration-database dump no longer yields usable API keys
|
||||||
|
while the constant-time by-value lookup still works.
|
||||||
|
|
||||||
Root cause confirmed against source. `ApiKey.KeyValue`
|
Root cause confirmed against source: `ApiKey.KeyValue` was a plaintext
|
||||||
(`src/ScadaLink.Commons/Entities/InboundApi/ApiKey.cs`) is mapped as a plaintext
|
`nvarchar(500)` column with a unique index, holding the live `X-API-Key` credential.
|
||||||
`nvarchar(500)` column with a unique index (`InboundApiConfiguration.cs:17-22`) and is
|
|
||||||
the bearer credential for Inbound API requests. The CD-004 `EncryptedStringConverter`
|
|
||||||
is correctly inapplicable here: it is non-deterministic, and the key is resolved
|
|
||||||
by value.
|
|
||||||
|
|
||||||
The recommended fix — store a deterministic salted/peppered hash instead of the
|
Design implemented — **deterministic keyed hash** (the recommendation's first option):
|
||||||
plaintext — cannot be completed within this module alone, for three concrete reasons:
|
|
||||||
|
|
||||||
1. **Commons entity change (out of scope).** Hashing changes the semantics of
|
- **Hash scheme.** HMAC-SHA256 over the UTF-8 key bytes, keyed with a server-side
|
||||||
`ApiKey.KeyValue`: after creation it can only ever hold the hash, never the live
|
*pepper*; the digest is stored Base64-encoded. No per-row random salt — an API key
|
||||||
key. The proper shape is a dedicated `KeyHash` field on the `ApiKey` entity (in
|
is already a high-entropy random token, and a random salt would break the
|
||||||
`ScadaLink.Commons`), which this task may not edit.
|
deterministic by-value lookup the authentication path relies on. The pepper instead
|
||||||
2. **Cross-module consumer change (out of scope).** `ScadaLink.InboundAPI`'s
|
binds every hash to the deployment. Implemented as `IApiKeyHasher` / `ApiKeyHasher`
|
||||||
`ApiKeyValidator.FindKeyConstantTime` reads `key.KeyValue` and does a constant-time
|
in `ScadaLink.Commons` (`Types/InboundApi/ApiKeyHasher.cs`); the constructor
|
||||||
byte comparison against the *raw presented secret*. If the column becomes a hash,
|
rejects a missing or weak (`< 16`-char) pepper with `ArgumentException` — fail-fast.
|
||||||
the validator must hash the presented candidate before comparing — an InboundAPI
|
- **Where the pepper lives.** `InboundApiOptions.ApiKeyPepper`, a component-owned
|
||||||
change. The CLI (`SecurityCommands.cs`) and Management Service also create/read
|
Options class already bound from the `ScadaLink:InboundApi` configuration section
|
||||||
`ApiKey` and would need the "plaintext shown once at creation" model.
|
(Options pattern); it is never hard-coded. `AddInboundAPI` registers `IApiKeyHasher`
|
||||||
3. **Irreversible data migration / operational decision.** A hash is one-way, so a
|
via a factory that reads the bound options, so a missing/weak pepper fails the
|
||||||
migration cannot convert existing plaintext keys to hashes without the originals —
|
deployment fast rather than degrading silently. (Operators must supply the pepper
|
||||||
every existing API key must be re-issued. Whether and how to do that is an
|
via configuration / a secret store; `appsettings*.json` is intentionally left
|
||||||
operational/design call, not a mechanical fix.
|
without a value.)
|
||||||
|
- **Commons.** `ApiKey` drops the persisted `KeyValue` property and gains a persisted
|
||||||
|
`KeyHash` — the plaintext is never a field on the entity. `ApiKey.FromHash(name,
|
||||||
|
keyHash)` creates an entity from a precomputed hash (the production create-path);
|
||||||
|
the existing `ApiKey(name, keyValue)` constructor is retained as a convenience that
|
||||||
|
hashes its input with the unpeppered default hasher.
|
||||||
|
- **ConfigurationDatabase.** EF mapping maps `KeyHash` (`nvarchar(256)`, required,
|
||||||
|
unique index `IX_ApiKeys_KeyHash`); migration `20260517073000_HashApiKeyValue`
|
||||||
|
drops `KeyValue`/`IX_ApiKeys_KeyValue`, adds `KeyHash`, and deletes existing
|
||||||
|
`ApiKeys` rows (a one-way hash cannot recover plaintext). `GetApiKeyByValueAsync`
|
||||||
|
now hashes the supplied value and looks up by `KeyHash`.
|
||||||
|
- **InboundAPI.** `ApiKeyValidator` takes an `IApiKeyHasher`; `FindKeyConstantTime`
|
||||||
|
hashes the presented candidate and compares hashes via
|
||||||
|
`CryptographicOperations.FixedTimeEquals` — the comparison stays constant-time.
|
||||||
|
- **ManagementService.** `HandleCreateApiKey` generates a random 32-byte key, stores
|
||||||
|
only its peppered hash, and returns the plaintext to the caller exactly once;
|
||||||
|
`HandleListApiKeys` no longer exposes the hash (returns id/name/enabled only).
|
||||||
|
|
||||||
**Decision needed (for a follow-up spanning Commons + InboundAPI + ManagementService
|
TDD regression tests (written first, confirmed failing, then passing):
|
||||||
+ CLI):** (a) add an `ApiKey.KeyHash` column and a per-key salt (or a system pepper);
|
`ApiKeyHasherTests` + `ApiKeyTests` (Commons — determinism, pepper fail-fast, no
|
||||||
(b) hash on create and authenticate by hashing the presented key; (c) decide the
|
plaintext property), `ApiKeyHashValidationTests` (InboundAPI — auth succeeds via
|
||||||
hash algorithm (e.g. PBKDF2/Argon2 vs. a fast salted SHA-256 — note the constant-time
|
hash, wrong key / wrong-pepper fail), `ApiKeyCreationTests` (ManagementService — only
|
||||||
lookup in `ApiKeyValidator` already mitigates timing, so a slow KDF is the safer
|
the hash is persisted, plaintext returned once), and `SchemaConfigurationTests`
|
||||||
choice if the key space is small); (d) plan the one-time re-issue of existing keys;
|
additions (ConfigurationDatabase — `KeyHash` mapped/uniquely-indexed, no `KeyValue`
|
||||||
(e) record the scheme in `docs/requirements/Component-ConfigurationDatabase.md` and the
|
column). `dotnet build` of all five touched modules is clean and their test suites
|
||||||
Inbound API design doc. Until that decision is made the finding stays **Open**.
|
are green.
|
||||||
|
|
||||||
|
**Operational consequence: every existing API key must be re-issued.** A hash is
|
||||||
|
one-way, so the `HashApiKeyValue` migration deletes existing key rows; after applying
|
||||||
|
it, operators must create new keys (CLI / Management API / Central UI), distribute
|
||||||
|
the one-time plaintext to callers, and re-approve the new keys on their methods.
|
||||||
|
|
||||||
|
Follow-up (outside this task's editable scope): record the hash scheme in
|
||||||
|
`docs/requirements/Component-ConfigurationDatabase.md` and the Inbound API design
|
||||||
|
doc, and update the Central UI API-keys page, which previously displayed a masked
|
||||||
|
`KeyValue`.
|
||||||
|
|
||||||
### ConfigurationDatabase-013 — Secret-column encryption silently falls back to an ephemeral (throwaway) key
|
### ConfigurationDatabase-013 — Secret-column encryption silently falls back to an ephemeral (throwaway) key
|
||||||
|
|
||||||
|
|||||||
@@ -46,7 +46,7 @@
|
|||||||
<tr>
|
<tr>
|
||||||
<th>ID</th>
|
<th>ID</th>
|
||||||
<th>Name</th>
|
<th>Name</th>
|
||||||
<th>Key Value</th>
|
<th>Key Hash</th>
|
||||||
<th style="width: 160px;">Actions</th>
|
<th style="width: 160px;">Actions</th>
|
||||||
</tr>
|
</tr>
|
||||||
</thead>
|
</thead>
|
||||||
@@ -62,7 +62,7 @@
|
|||||||
<span class="badge bg-secondary ms-1">Disabled</span>
|
<span class="badge bg-secondary ms-1">Disabled</span>
|
||||||
}
|
}
|
||||||
</td>
|
</td>
|
||||||
<td><code>@MaskKeyValue(key.KeyValue)</code></td>
|
<td><code>@MaskKeyValue(key.KeyHash)</code></td>
|
||||||
<td>
|
<td>
|
||||||
<div class="d-flex gap-1">
|
<div class="d-flex gap-1">
|
||||||
<button class="btn btn-outline-primary btn-sm py-0 px-2"
|
<button class="btn btn-outline-primary btn-sm py-0 px-2"
|
||||||
|
|||||||
@@ -1,15 +1,61 @@
|
|||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
namespace ScadaLink.Commons.Entities.InboundApi;
|
namespace ScadaLink.Commons.Entities.InboundApi;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// An inbound-API bearer credential. Per ConfigurationDatabase-012 the plaintext key
|
||||||
|
/// is never persisted: the entity stores only <see cref="KeyHash"/>, a deterministic
|
||||||
|
/// keyed hash of the key (HMAC-SHA256 with a server-side pepper). The plaintext is
|
||||||
|
/// generated at creation, shown to the operator exactly once, and then discarded.
|
||||||
|
/// </summary>
|
||||||
public class ApiKey
|
public class ApiKey
|
||||||
{
|
{
|
||||||
public int Id { get; set; }
|
public int Id { get; set; }
|
||||||
public string Name { get; set; }
|
public string Name { get; set; }
|
||||||
public string KeyValue { get; set; }
|
|
||||||
|
/// <summary>
|
||||||
|
/// Deterministic keyed hash of the API key value. This is the only form of the
|
||||||
|
/// credential persisted; the plaintext key is never stored. Authentication hashes
|
||||||
|
/// the presented candidate with the same scheme and compares against this value.
|
||||||
|
/// </summary>
|
||||||
|
public string KeyHash { get; set; }
|
||||||
|
|
||||||
public bool IsEnabled { get; set; }
|
public bool IsEnabled { get; set; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates an API key from a plaintext value, immediately hashing it with the
|
||||||
|
/// unpeppered default hasher (<see cref="ApiKeyHasher.Default"/>) so the entity
|
||||||
|
/// never holds the plaintext. Production code paths that have a configured pepper
|
||||||
|
/// should use <see cref="FromHash(string, string)"/> with a peppered hash instead.
|
||||||
|
/// </summary>
|
||||||
public ApiKey(string name, string keyValue)
|
public ApiKey(string name, string keyValue)
|
||||||
{
|
{
|
||||||
Name = name ?? throw new ArgumentNullException(nameof(name));
|
Name = name ?? throw new ArgumentNullException(nameof(name));
|
||||||
KeyValue = keyValue ?? throw new ArgumentNullException(nameof(keyValue));
|
if (keyValue is null) throw new ArgumentNullException(nameof(keyValue));
|
||||||
|
KeyHash = ApiKeyHasher.Default.Hash(keyValue);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Parameterless constructor for the EF Core materializer. Application code uses
|
||||||
|
/// <see cref="ApiKey(string, string)"/> or <see cref="FromHash(string, string)"/>.
|
||||||
|
/// </summary>
|
||||||
|
private ApiKey()
|
||||||
|
{
|
||||||
|
Name = string.Empty;
|
||||||
|
KeyHash = string.Empty;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates an API key from an already-computed key hash. Used by the creation
|
||||||
|
/// path, which generates a random key, hashes it with the configured (peppered)
|
||||||
|
/// <see cref="IApiKeyHasher"/>, and stores only the resulting hash.
|
||||||
|
/// </summary>
|
||||||
|
public static ApiKey FromHash(string name, string keyHash)
|
||||||
|
{
|
||||||
|
return new ApiKey
|
||||||
|
{
|
||||||
|
Name = name ?? throw new ArgumentNullException(nameof(name)),
|
||||||
|
KeyHash = keyHash ?? throw new ArgumentNullException(nameof(keyHash)),
|
||||||
|
};
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
92
src/ScadaLink.Commons/Types/InboundApi/ApiKeyHasher.cs
Normal file
92
src/ScadaLink.Commons/Types/InboundApi/ApiKeyHasher.cs
Normal file
@@ -0,0 +1,92 @@
|
|||||||
|
using System.Security.Cryptography;
|
||||||
|
using System.Text;
|
||||||
|
|
||||||
|
namespace ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Computes a deterministic, keyed hash of an inbound-API key value
|
||||||
|
/// (ConfigurationDatabase-012). API keys are persisted as this hash, never as
|
||||||
|
/// plaintext, so a configuration-database dump does not yield usable credentials.
|
||||||
|
/// The hash is deterministic so authentication can still resolve a key by value.
|
||||||
|
/// </summary>
|
||||||
|
public interface IApiKeyHasher
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Returns the keyed hash of <paramref name="apiKey"/> as a Base64 string.
|
||||||
|
/// The same input always produces the same output (deterministic), which keeps
|
||||||
|
/// the by-value lookup working.
|
||||||
|
/// </summary>
|
||||||
|
string Hash(string apiKey);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// HMAC-SHA256 implementation of <see cref="IApiKeyHasher"/>. The HMAC key is a
|
||||||
|
/// server-side <em>pepper</em> bound from configuration. A per-row random salt is
|
||||||
|
/// intentionally NOT used: an API key is already a high-entropy random token, and a
|
||||||
|
/// random salt would break the deterministic by-value lookup the authentication
|
||||||
|
/// path relies on. The pepper instead binds every hash to this deployment, so a
|
||||||
|
/// stolen database is useless without it.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class ApiKeyHasher : IApiKeyHasher
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Minimum accepted pepper length. A pepper shorter than this is treated as a
|
||||||
|
/// deployment misconfiguration and rejected — see <see cref="ApiKeyHasher(string)"/>.
|
||||||
|
/// </summary>
|
||||||
|
public const int MinimumPepperLength = 16;
|
||||||
|
|
||||||
|
private readonly byte[] _pepper;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// An unpeppered hasher (HMAC-SHA256 keyed with a fixed, empty-equivalent value).
|
||||||
|
/// It is still a one-way hash, but carries no deployment-specific binding. It
|
||||||
|
/// exists for tests and non-production wiring; production must construct an
|
||||||
|
/// <see cref="ApiKeyHasher"/> with a real pepper.
|
||||||
|
/// </summary>
|
||||||
|
public static ApiKeyHasher Default { get; } = new ApiKeyHasher();
|
||||||
|
|
||||||
|
private ApiKeyHasher()
|
||||||
|
{
|
||||||
|
// Fixed, deployment-independent key for the unpeppered default.
|
||||||
|
_pepper = Encoding.UTF8.GetBytes("ScadaLink.InboundApi.DefaultApiKeyHasher");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates a hasher keyed with the given server-side pepper.
|
||||||
|
/// </summary>
|
||||||
|
/// <exception cref="ArgumentException">
|
||||||
|
/// Thrown if <paramref name="pepper"/> is null, blank, or shorter than
|
||||||
|
/// <see cref="MinimumPepperLength"/> — a missing or weak pepper is a deployment
|
||||||
|
/// misconfiguration and must fail loudly rather than degrade silently.
|
||||||
|
/// </exception>
|
||||||
|
public ApiKeyHasher(string pepper)
|
||||||
|
{
|
||||||
|
if (string.IsNullOrWhiteSpace(pepper))
|
||||||
|
{
|
||||||
|
throw new ArgumentException(
|
||||||
|
"The API-key HMAC pepper must be configured. Set a strong, random value " +
|
||||||
|
"in configuration (ScadaLink:InboundApi:ApiKeyPepper).",
|
||||||
|
nameof(pepper));
|
||||||
|
}
|
||||||
|
|
||||||
|
if (pepper.Trim().Length < MinimumPepperLength)
|
||||||
|
{
|
||||||
|
throw new ArgumentException(
|
||||||
|
$"The API-key HMAC pepper is too weak: it must be at least {MinimumPepperLength} " +
|
||||||
|
"characters. Use a strong, random value.",
|
||||||
|
nameof(pepper));
|
||||||
|
}
|
||||||
|
|
||||||
|
_pepper = Encoding.UTF8.GetBytes(pepper);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <inheritdoc />
|
||||||
|
public string Hash(string apiKey)
|
||||||
|
{
|
||||||
|
ArgumentNullException.ThrowIfNull(apiKey);
|
||||||
|
|
||||||
|
using var hmac = new HMACSHA256(_pepper);
|
||||||
|
var hash = hmac.ComputeHash(Encoding.UTF8.GetBytes(apiKey));
|
||||||
|
return Convert.ToBase64String(hash);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -14,12 +14,15 @@ public class ApiKeyConfiguration : IEntityTypeConfiguration<ApiKey>
|
|||||||
.IsRequired()
|
.IsRequired()
|
||||||
.HasMaxLength(200);
|
.HasMaxLength(200);
|
||||||
|
|
||||||
builder.Property(k => k.KeyValue)
|
// ConfigurationDatabase-012: the bearer credential is persisted only as a
|
||||||
|
// deterministic HMAC-SHA256 hash, never as plaintext. Base64 of a 32-byte
|
||||||
|
// HMAC-SHA256 digest is 44 characters; 256 leaves generous headroom.
|
||||||
|
builder.Property(k => k.KeyHash)
|
||||||
.IsRequired()
|
.IsRequired()
|
||||||
.HasMaxLength(500);
|
.HasMaxLength(256);
|
||||||
|
|
||||||
builder.HasIndex(k => k.Name).IsUnique();
|
builder.HasIndex(k => k.Name).IsUnique();
|
||||||
builder.HasIndex(k => k.KeyValue).IsUnique();
|
builder.HasIndex(k => k.KeyHash).IsUnique();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
1350
src/ScadaLink.ConfigurationDatabase/Migrations/20260517073000_HashApiKeyValue.Designer.cs
generated
Normal file
1350
src/ScadaLink.ConfigurationDatabase/Migrations/20260517073000_HashApiKeyValue.Designer.cs
generated
Normal file
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,77 @@
|
|||||||
|
using Microsoft.EntityFrameworkCore.Migrations;
|
||||||
|
|
||||||
|
#nullable disable
|
||||||
|
|
||||||
|
namespace ScadaLink.ConfigurationDatabase.Migrations
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: replaces the plaintext <c>KeyValue</c> column with a
|
||||||
|
/// <c>KeyHash</c> column holding a deterministic HMAC-SHA256 hash of the key.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// A hash is one-way: existing plaintext keys cannot be converted to hashes
|
||||||
|
/// without the originals. This migration therefore deletes all existing API-key
|
||||||
|
/// rows. <strong>Every existing API key must be re-issued</strong> after this
|
||||||
|
/// migration is applied — create new keys via the CLI / Management API / Central
|
||||||
|
/// UI, distribute the one-time plaintext to callers, and approve them on methods.
|
||||||
|
/// </remarks>
|
||||||
|
public partial class HashApiKeyValue : Migration
|
||||||
|
{
|
||||||
|
/// <inheritdoc />
|
||||||
|
protected override void Up(MigrationBuilder migrationBuilder)
|
||||||
|
{
|
||||||
|
// Existing keys hold only plaintext, which cannot be hashed back. They
|
||||||
|
// must be re-issued, so remove them before the column change to keep the
|
||||||
|
// new unique KeyHash index satisfiable.
|
||||||
|
migrationBuilder.Sql("DELETE FROM ApiKeys;");
|
||||||
|
|
||||||
|
migrationBuilder.DropIndex(
|
||||||
|
name: "IX_ApiKeys_KeyValue",
|
||||||
|
table: "ApiKeys");
|
||||||
|
|
||||||
|
migrationBuilder.DropColumn(
|
||||||
|
name: "KeyValue",
|
||||||
|
table: "ApiKeys");
|
||||||
|
|
||||||
|
migrationBuilder.AddColumn<string>(
|
||||||
|
name: "KeyHash",
|
||||||
|
table: "ApiKeys",
|
||||||
|
type: "nvarchar(256)",
|
||||||
|
maxLength: 256,
|
||||||
|
nullable: false,
|
||||||
|
defaultValue: "");
|
||||||
|
|
||||||
|
migrationBuilder.CreateIndex(
|
||||||
|
name: "IX_ApiKeys_KeyHash",
|
||||||
|
table: "ApiKeys",
|
||||||
|
column: "KeyHash",
|
||||||
|
unique: true);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <inheritdoc />
|
||||||
|
protected override void Down(MigrationBuilder migrationBuilder)
|
||||||
|
{
|
||||||
|
migrationBuilder.DropIndex(
|
||||||
|
name: "IX_ApiKeys_KeyHash",
|
||||||
|
table: "ApiKeys");
|
||||||
|
|
||||||
|
migrationBuilder.DropColumn(
|
||||||
|
name: "KeyHash",
|
||||||
|
table: "ApiKeys");
|
||||||
|
|
||||||
|
migrationBuilder.AddColumn<string>(
|
||||||
|
name: "KeyValue",
|
||||||
|
table: "ApiKeys",
|
||||||
|
type: "nvarchar(500)",
|
||||||
|
maxLength: 500,
|
||||||
|
nullable: false,
|
||||||
|
defaultValue: "");
|
||||||
|
|
||||||
|
migrationBuilder.CreateIndex(
|
||||||
|
name: "IX_ApiKeys_KeyValue",
|
||||||
|
table: "ApiKeys",
|
||||||
|
column: "KeyValue",
|
||||||
|
unique: true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -348,10 +348,10 @@ namespace ScadaLink.ConfigurationDatabase.Migrations
|
|||||||
b.Property<bool>("IsEnabled")
|
b.Property<bool>("IsEnabled")
|
||||||
.HasColumnType("bit");
|
.HasColumnType("bit");
|
||||||
|
|
||||||
b.Property<string>("KeyValue")
|
b.Property<string>("KeyHash")
|
||||||
.IsRequired()
|
.IsRequired()
|
||||||
.HasMaxLength(500)
|
.HasMaxLength(256)
|
||||||
.HasColumnType("nvarchar(500)");
|
.HasColumnType("nvarchar(256)");
|
||||||
|
|
||||||
b.Property<string>("Name")
|
b.Property<string>("Name")
|
||||||
.IsRequired()
|
.IsRequired()
|
||||||
@@ -360,7 +360,7 @@ namespace ScadaLink.ConfigurationDatabase.Migrations
|
|||||||
|
|
||||||
b.HasKey("Id");
|
b.HasKey("Id");
|
||||||
|
|
||||||
b.HasIndex("KeyValue")
|
b.HasIndex("KeyHash")
|
||||||
.IsUnique();
|
.IsUnique();
|
||||||
|
|
||||||
b.HasIndex("Name")
|
b.HasIndex("Name")
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ using Microsoft.Extensions.Logging;
|
|||||||
using Microsoft.Extensions.Logging.Abstractions;
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
using ScadaLink.Commons.Entities.InboundApi;
|
using ScadaLink.Commons.Entities.InboundApi;
|
||||||
using ScadaLink.Commons.Interfaces.Repositories;
|
using ScadaLink.Commons.Interfaces.Repositories;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
namespace ScadaLink.ConfigurationDatabase.Repositories;
|
namespace ScadaLink.ConfigurationDatabase.Repositories;
|
||||||
|
|
||||||
@@ -23,8 +24,20 @@ public class InboundApiRepository : IInboundApiRepository
|
|||||||
public async Task<IReadOnlyList<ApiKey>> GetAllApiKeysAsync(CancellationToken cancellationToken = default)
|
public async Task<IReadOnlyList<ApiKey>> GetAllApiKeysAsync(CancellationToken cancellationToken = default)
|
||||||
=> await _context.Set<ApiKey>().ToListAsync(cancellationToken);
|
=> await _context.Set<ApiKey>().ToListAsync(cancellationToken);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: API keys are persisted only as a deterministic hash,
|
||||||
|
/// never as plaintext, so this lookup hashes the supplied plaintext value and
|
||||||
|
/// matches it against the stored <see cref="ApiKey.KeyHash"/> column. The
|
||||||
|
/// unpeppered default hasher is used here because the repository has no access to
|
||||||
|
/// the deployment pepper; the inbound-API authentication path does not use this
|
||||||
|
/// method — it loads all keys and compares hashes constant-time in
|
||||||
|
/// <c>ApiKeyValidator</c> with the configured (peppered) hasher.
|
||||||
|
/// </summary>
|
||||||
public async Task<ApiKey?> GetApiKeyByValueAsync(string keyValue, CancellationToken cancellationToken = default)
|
public async Task<ApiKey?> GetApiKeyByValueAsync(string keyValue, CancellationToken cancellationToken = default)
|
||||||
=> await _context.Set<ApiKey>().FirstOrDefaultAsync(k => k.KeyValue == keyValue, cancellationToken);
|
{
|
||||||
|
var keyHash = ApiKeyHasher.Default.Hash(keyValue);
|
||||||
|
return await _context.Set<ApiKey>().FirstOrDefaultAsync(k => k.KeyHash == keyHash, cancellationToken);
|
||||||
|
}
|
||||||
|
|
||||||
public async Task AddApiKeyAsync(ApiKey apiKey, CancellationToken cancellationToken = default)
|
public async Task AddApiKeyAsync(ApiKey apiKey, CancellationToken cancellationToken = default)
|
||||||
=> await _context.Set<ApiKey>().AddAsync(apiKey, cancellationToken);
|
=> await _context.Set<ApiKey>().AddAsync(apiKey, cancellationToken);
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ using System.Security.Cryptography;
|
|||||||
using System.Text;
|
using System.Text;
|
||||||
using ScadaLink.Commons.Entities.InboundApi;
|
using ScadaLink.Commons.Entities.InboundApi;
|
||||||
using ScadaLink.Commons.Interfaces.Repositories;
|
using ScadaLink.Commons.Interfaces.Repositories;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
namespace ScadaLink.InboundAPI;
|
namespace ScadaLink.InboundAPI;
|
||||||
|
|
||||||
@@ -12,14 +13,24 @@ namespace ScadaLink.InboundAPI;
|
|||||||
public class ApiKeyValidator
|
public class ApiKeyValidator
|
||||||
{
|
{
|
||||||
private readonly IInboundApiRepository _repository;
|
private readonly IInboundApiRepository _repository;
|
||||||
|
private readonly IApiKeyHasher _hasher;
|
||||||
|
|
||||||
// InboundAPI-011: the single message used for both "method not found" and
|
// InboundAPI-011: the single message used for both "method not found" and
|
||||||
// "key not approved" so the two outcomes are indistinguishable to the caller.
|
// "key not approved" so the two outcomes are indistinguishable to the caller.
|
||||||
private const string NotApprovedMessage = "API key not approved for this method";
|
private const string NotApprovedMessage = "API key not approved for this method";
|
||||||
|
|
||||||
public ApiKeyValidator(IInboundApiRepository repository)
|
/// <param name="repository">Inbound-API data access.</param>
|
||||||
|
/// <param name="hasher">
|
||||||
|
/// ConfigurationDatabase-012: hashes the presented candidate key with the same
|
||||||
|
/// HMAC-SHA256 pepper used at key creation, so authentication compares hashes —
|
||||||
|
/// the database never holds a plaintext credential. Defaults to the unpeppered
|
||||||
|
/// <see cref="ApiKeyHasher.Default"/>; production wiring injects the configured,
|
||||||
|
/// peppered hasher.
|
||||||
|
/// </param>
|
||||||
|
public ApiKeyValidator(IInboundApiRepository repository, IApiKeyHasher? hasher = null)
|
||||||
{
|
{
|
||||||
_repository = repository;
|
_repository = repository;
|
||||||
|
_hasher = hasher ?? ApiKeyHasher.Default;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -37,13 +48,17 @@ public class ApiKeyValidator
|
|||||||
}
|
}
|
||||||
|
|
||||||
// InboundAPI-003: do NOT resolve the key with a secret-equality lookup
|
// InboundAPI-003: do NOT resolve the key with a secret-equality lookup
|
||||||
// (GetApiKeyByValueAsync translates to a SQL "WHERE KeyValue = @secret" early-exit
|
// (GetApiKeyByValueAsync translates to a SQL "WHERE KeyHash = @hash" early-exit
|
||||||
// comparison — a timing side-channel). Fetch all keys and match the secret
|
// comparison — a timing side-channel). Fetch all keys and match in-process
|
||||||
// in-process with a constant-time comparison so neither match position nor
|
// with a constant-time comparison so neither match position nor length is
|
||||||
// secret length is observable to a network attacker.
|
// observable to a network attacker.
|
||||||
|
// ConfigurationDatabase-012: the database stores only the HMAC hash of each
|
||||||
|
// key, so the presented candidate is hashed with the same pepper and the
|
||||||
|
// comparison runs over the resulting hashes — never over plaintext.
|
||||||
|
var candidateHash = _hasher.Hash(apiKeyValue);
|
||||||
var apiKey = FindKeyConstantTime(
|
var apiKey = FindKeyConstantTime(
|
||||||
await _repository.GetAllApiKeysAsync(cancellationToken),
|
await _repository.GetAllApiKeysAsync(cancellationToken),
|
||||||
apiKeyValue);
|
candidateHash);
|
||||||
if (apiKey == null || !apiKey.IsEnabled)
|
if (apiKey == null || !apiKey.IsEnabled)
|
||||||
{
|
{
|
||||||
return ApiKeyValidationResult.Unauthorized("Invalid or disabled API key");
|
return ApiKeyValidationResult.Unauthorized("Invalid or disabled API key");
|
||||||
@@ -73,19 +88,21 @@ public class ApiKeyValidator
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// InboundAPI-003: Finds the key whose value matches <paramref name="candidate"/>
|
/// InboundAPI-003 / ConfigurationDatabase-012: Finds the key whose stored
|
||||||
/// using <see cref="CryptographicOperations.FixedTimeEquals"/> over the UTF-8 bytes.
|
/// <see cref="ApiKey.KeyHash"/> matches <paramref name="candidateHash"/> — the
|
||||||
|
/// HMAC hash of the presented key — using
|
||||||
|
/// <see cref="CryptographicOperations.FixedTimeEquals"/> over the UTF-8 bytes.
|
||||||
/// Every candidate row is compared so that the running time does not depend on the
|
/// Every candidate row is compared so that the running time does not depend on the
|
||||||
/// match position; length mismatches return false without leaking length timing.
|
/// match position; length mismatches return false without leaking length timing.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private static ApiKey? FindKeyConstantTime(IEnumerable<ApiKey> keys, string candidate)
|
private static ApiKey? FindKeyConstantTime(IEnumerable<ApiKey> keys, string candidateHash)
|
||||||
{
|
{
|
||||||
var candidateBytes = Encoding.UTF8.GetBytes(candidate);
|
var candidateBytes = Encoding.UTF8.GetBytes(candidateHash);
|
||||||
ApiKey? match = null;
|
ApiKey? match = null;
|
||||||
|
|
||||||
foreach (var key in keys)
|
foreach (var key in keys)
|
||||||
{
|
{
|
||||||
var keyBytes = Encoding.UTF8.GetBytes(key.KeyValue);
|
var keyBytes = Encoding.UTF8.GetBytes(key.KeyHash);
|
||||||
if (CryptographicOperations.FixedTimeEquals(candidateBytes, keyBytes))
|
if (CryptographicOperations.FixedTimeEquals(candidateBytes, keyBytes))
|
||||||
{
|
{
|
||||||
// Do not break — continuing keeps the loop's timing independent of
|
// Do not break — continuing keeps the loop's timing independent of
|
||||||
|
|||||||
@@ -17,4 +17,18 @@ public class InboundApiOptions
|
|||||||
/// bounds per-request allocations.
|
/// bounds per-request allocations.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public long MaxRequestBodyBytes { get; set; } = DefaultMaxRequestBodyBytes;
|
public long MaxRequestBodyBytes { get; set; } = DefaultMaxRequestBodyBytes;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: server-side HMAC pepper used to hash inbound-API
|
||||||
|
/// bearer credentials. API keys are persisted as a deterministic keyed hash, never
|
||||||
|
/// as plaintext; this pepper is the HMAC key that binds every hash to this
|
||||||
|
/// deployment, so a stolen configuration database is not directly exploitable.
|
||||||
|
/// <para>
|
||||||
|
/// This is a secret: supply a strong, random value via configuration or a secret
|
||||||
|
/// store, never hard-coded. It must be present and at least
|
||||||
|
/// <see cref="ScadaLink.Commons.Types.InboundApi.ApiKeyHasher.MinimumPepperLength"/>
|
||||||
|
/// characters — <c>AddInboundAPI</c> fails fast otherwise.
|
||||||
|
/// </para>
|
||||||
|
/// </summary>
|
||||||
|
public string ApiKeyPepper { get; set; } = string.Empty;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
using Microsoft.Extensions.DependencyInjection;
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
|
using Microsoft.Extensions.Options;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
namespace ScadaLink.InboundAPI;
|
namespace ScadaLink.InboundAPI;
|
||||||
|
|
||||||
@@ -10,6 +12,18 @@ public static class ServiceCollectionExtensions
|
|||||||
services.AddSingleton<InboundScriptExecutor>();
|
services.AddSingleton<InboundScriptExecutor>();
|
||||||
services.AddScoped<RouteHelper>();
|
services.AddScoped<RouteHelper>();
|
||||||
|
|
||||||
|
// ConfigurationDatabase-012: API keys are persisted as a deterministic HMAC
|
||||||
|
// hash, never as plaintext. The hasher is keyed with a server-side pepper
|
||||||
|
// bound from configuration (InboundApiOptions.ApiKeyPepper). Constructing
|
||||||
|
// ApiKeyHasher throws if the pepper is missing or weak — so a misconfigured
|
||||||
|
// deployment fails fast the first time the hasher is resolved rather than
|
||||||
|
// silently hashing with no pepper.
|
||||||
|
services.AddSingleton<IApiKeyHasher>(sp =>
|
||||||
|
{
|
||||||
|
var options = sp.GetRequiredService<IOptions<InboundApiOptions>>().Value;
|
||||||
|
return new ApiKeyHasher(options.ApiKeyPepper);
|
||||||
|
});
|
||||||
|
|
||||||
// InboundAPI-017: routed calls go through the IInstanceRouter seam; the
|
// InboundAPI-017: routed calls go through the IInstanceRouter seam; the
|
||||||
// production implementation delegates to CommunicationService.
|
// production implementation delegates to CommunicationService.
|
||||||
services.AddScoped<IInstanceRouter, CommunicationServiceInstanceRouter>();
|
services.AddScoped<IInstanceRouter, CommunicationServiceInstanceRouter>();
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ using ScadaLink.Commons.Interfaces.Services;
|
|||||||
using ScadaLink.Commons.Messages.DebugView;
|
using ScadaLink.Commons.Messages.DebugView;
|
||||||
using ScadaLink.Commons.Messages.Management;
|
using ScadaLink.Commons.Messages.Management;
|
||||||
using ScadaLink.Commons.Messages.RemoteQuery;
|
using ScadaLink.Commons.Messages.RemoteQuery;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
using ScadaLink.DeploymentManager;
|
using ScadaLink.DeploymentManager;
|
||||||
using ScadaLink.HealthMonitoring;
|
using ScadaLink.HealthMonitoring;
|
||||||
using ScadaLink.Communication;
|
using ScadaLink.Communication;
|
||||||
@@ -1174,18 +1175,42 @@ public class ManagementActor : ReceiveActor
|
|||||||
private static async Task<object?> HandleListApiKeys(IServiceProvider sp)
|
private static async Task<object?> HandleListApiKeys(IServiceProvider sp)
|
||||||
{
|
{
|
||||||
var repo = sp.GetRequiredService<IInboundApiRepository>();
|
var repo = sp.GetRequiredService<IInboundApiRepository>();
|
||||||
return await repo.GetAllApiKeysAsync();
|
var keys = await repo.GetAllApiKeysAsync();
|
||||||
|
|
||||||
|
// ConfigurationDatabase-012: list/read paths must not expose the stored key
|
||||||
|
// hash — it is a credential artifact. Only identity and status are returned;
|
||||||
|
// the plaintext key is shown once at creation and is never retrievable.
|
||||||
|
return keys
|
||||||
|
.Select(k => new { k.Id, k.Name, k.IsEnabled })
|
||||||
|
.ToList();
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleCreateApiKey(IServiceProvider sp, CreateApiKeyCommand cmd, string user)
|
private static async Task<object?> HandleCreateApiKey(IServiceProvider sp, CreateApiKeyCommand cmd, string user)
|
||||||
{
|
{
|
||||||
var repo = sp.GetRequiredService<IInboundApiRepository>();
|
var repo = sp.GetRequiredService<IInboundApiRepository>();
|
||||||
var keyValue = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32));
|
|
||||||
var apiKey = new ApiKey(cmd.Name, keyValue) { IsEnabled = true };
|
// ConfigurationDatabase-012: generate a high-entropy random key, persist only
|
||||||
|
// its peppered hash, and return the plaintext to the caller exactly once. The
|
||||||
|
// plaintext is never stored — the ApiKey entity carries only KeyHash.
|
||||||
|
var hasher = sp.GetService<IApiKeyHasher>() ?? ApiKeyHasher.Default;
|
||||||
|
var plaintextKey = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32));
|
||||||
|
var apiKey = ApiKey.FromHash(cmd.Name, hasher.Hash(plaintextKey));
|
||||||
|
apiKey.IsEnabled = true;
|
||||||
|
|
||||||
await repo.AddApiKeyAsync(apiKey);
|
await repo.AddApiKeyAsync(apiKey);
|
||||||
await repo.SaveChangesAsync();
|
await repo.SaveChangesAsync();
|
||||||
await AuditAsync(sp, user, "Create", "ApiKey", apiKey.Id.ToString(), apiKey.Name, new { apiKey.Id, apiKey.Name, apiKey.IsEnabled });
|
await AuditAsync(sp, user, "Create", "ApiKey", apiKey.Id.ToString(), apiKey.Name,
|
||||||
return apiKey;
|
new { apiKey.Id, apiKey.Name, apiKey.IsEnabled });
|
||||||
|
|
||||||
|
// The plaintext key is shown to the operator only here, in the create response;
|
||||||
|
// it cannot be retrieved later. The stored hash is deliberately not returned.
|
||||||
|
return new
|
||||||
|
{
|
||||||
|
apiKey.Id,
|
||||||
|
apiKey.Name,
|
||||||
|
apiKey.IsEnabled,
|
||||||
|
PlaintextKey = plaintextKey,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task<object?> HandleDeleteApiKey(IServiceProvider sp, DeleteApiKeyCommand cmd, string user)
|
private static async Task<object?> HandleDeleteApiKey(IServiceProvider sp, DeleteApiKeyCommand cmd, string user)
|
||||||
|
|||||||
49
tests/ScadaLink.Commons.Tests/Entities/ApiKeyTests.cs
Normal file
49
tests/ScadaLink.Commons.Tests/Entities/ApiKeyTests.cs
Normal file
@@ -0,0 +1,49 @@
|
|||||||
|
using ScadaLink.Commons.Entities.InboundApi;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
|
namespace ScadaLink.Commons.Tests.Entities;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: the <see cref="ApiKey"/> entity must never carry the
|
||||||
|
/// plaintext bearer credential as a persisted field — only its deterministic hash.
|
||||||
|
/// </summary>
|
||||||
|
public class ApiKeyTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void ApiKey_HasNoPlaintextKeyValueProperty()
|
||||||
|
{
|
||||||
|
// The plaintext key is shown to the operator once at creation and is never
|
||||||
|
// persisted. The entity must therefore expose KeyHash, not KeyValue.
|
||||||
|
var properties = typeof(ApiKey).GetProperties().Select(p => p.Name).ToArray();
|
||||||
|
|
||||||
|
Assert.DoesNotContain("KeyValue", properties);
|
||||||
|
Assert.Contains("KeyHash", properties);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Constructor_FromPlaintext_StoresHashNotPlaintext()
|
||||||
|
{
|
||||||
|
var key = new ApiKey("MES-Production", "the-secret-key-value");
|
||||||
|
|
||||||
|
Assert.NotEqual("the-secret-key-value", key.KeyHash);
|
||||||
|
Assert.Equal(ApiKeyHasher.Default.Hash("the-secret-key-value"), key.KeyHash);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void FromHash_StoresHashVerbatim()
|
||||||
|
{
|
||||||
|
var key = ApiKey.FromHash("RecipeManager-Dev", "precomputed-hash-value");
|
||||||
|
|
||||||
|
Assert.Equal("RecipeManager-Dev", key.Name);
|
||||||
|
Assert.Equal("precomputed-hash-value", key.KeyHash);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Constructor_NullArguments_Throw()
|
||||||
|
{
|
||||||
|
Assert.Throws<ArgumentNullException>(() => new ApiKey(null!, "value"));
|
||||||
|
Assert.Throws<ArgumentNullException>(() => new ApiKey("name", (string)null!));
|
||||||
|
Assert.Throws<ArgumentNullException>(() => ApiKey.FromHash(null!, "hash"));
|
||||||
|
Assert.Throws<ArgumentNullException>(() => ApiKey.FromHash("name", null!));
|
||||||
|
}
|
||||||
|
}
|
||||||
84
tests/ScadaLink.Commons.Tests/Types/ApiKeyHasherTests.cs
Normal file
84
tests/ScadaLink.Commons.Tests/Types/ApiKeyHasherTests.cs
Normal file
@@ -0,0 +1,84 @@
|
|||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
|
namespace ScadaLink.Commons.Tests.Types;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: the inbound-API bearer credential is stored as a
|
||||||
|
/// deterministic keyed hash (HMAC-SHA256 with a server-side pepper) rather than
|
||||||
|
/// plaintext. These tests pin the hasher contract that the entity, the validator,
|
||||||
|
/// and the management create-path all depend on.
|
||||||
|
/// </summary>
|
||||||
|
public class ApiKeyHasherTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void Hash_IsDeterministic_SameInputSameOutput()
|
||||||
|
{
|
||||||
|
var hasher = new ApiKeyHasher("a-sufficiently-long-server-pepper-value");
|
||||||
|
|
||||||
|
var first = hasher.Hash("some-api-key-value");
|
||||||
|
var second = hasher.Hash("some-api-key-value");
|
||||||
|
|
||||||
|
Assert.Equal(first, second);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Hash_DoesNotEqualPlaintext()
|
||||||
|
{
|
||||||
|
var hasher = new ApiKeyHasher("a-sufficiently-long-server-pepper-value");
|
||||||
|
|
||||||
|
var hash = hasher.Hash("some-api-key-value");
|
||||||
|
|
||||||
|
Assert.NotEqual("some-api-key-value", hash);
|
||||||
|
Assert.DoesNotContain("some-api-key-value", hash);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Hash_DifferentInputs_ProduceDifferentHashes()
|
||||||
|
{
|
||||||
|
var hasher = new ApiKeyHasher("a-sufficiently-long-server-pepper-value");
|
||||||
|
|
||||||
|
Assert.NotEqual(hasher.Hash("key-one"), hasher.Hash("key-two"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Hash_DifferentPeppers_ProduceDifferentHashes()
|
||||||
|
{
|
||||||
|
var a = new ApiKeyHasher("a-sufficiently-long-server-pepper-value");
|
||||||
|
var b = new ApiKeyHasher("a-different-but-equally-long-pepper-val");
|
||||||
|
|
||||||
|
// The pepper binds the hash to the server: a stolen DB dump is useless
|
||||||
|
// without the pepper because the same key hashes differently under it.
|
||||||
|
Assert.NotEqual(a.Hash("same-api-key"), b.Hash("same-api-key"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData(null)]
|
||||||
|
[InlineData("")]
|
||||||
|
[InlineData(" ")]
|
||||||
|
[InlineData("too-short")]
|
||||||
|
public void Constructor_MissingOrWeakPepper_FailsFast(string? pepper)
|
||||||
|
{
|
||||||
|
// The pepper must be present and of meaningful length; a missing or weak
|
||||||
|
// pepper is a deployment misconfiguration and must fail loudly.
|
||||||
|
Assert.Throws<ArgumentException>(() => new ApiKeyHasher(pepper!));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Hash_NullInput_Throws()
|
||||||
|
{
|
||||||
|
var hasher = new ApiKeyHasher("a-sufficiently-long-server-pepper-value");
|
||||||
|
|
||||||
|
Assert.Throws<ArgumentNullException>(() => hasher.Hash(null!));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Default_IsUsableWithoutAPepper()
|
||||||
|
{
|
||||||
|
// The unpeppered default exists for tests and non-production wiring; it is
|
||||||
|
// still a one-way HMAC-SHA256, just without the server-binding pepper.
|
||||||
|
var hash = ApiKeyHasher.Default.Hash("some-api-key-value");
|
||||||
|
|
||||||
|
Assert.NotEqual("some-api-key-value", hash);
|
||||||
|
Assert.Equal(ApiKeyHasher.Default.Hash("some-api-key-value"), hash);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -134,4 +134,30 @@ public class SplitQueryBehaviourTests : IDisposable
|
|||||||
Assert.Equal(2, loaded!.Attributes.Count);
|
Assert.Equal(2, loaded!.Attributes.Count);
|
||||||
Assert.Single(loaded.Scripts);
|
Assert.Single(loaded.Scripts);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ConfigurationDatabase-012: the ApiKey table must persist the bearer credential
|
||||||
|
// as a hash column (KeyHash) and must NOT carry a plaintext KeyValue column.
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ApiKey_KeyHashColumn_IsMappedAndUniquelyIndexed()
|
||||||
|
{
|
||||||
|
var entityType = _context.Model.FindEntityType(typeof(ScadaLink.Commons.Entities.InboundApi.ApiKey))!;
|
||||||
|
|
||||||
|
var keyHash = entityType.FindProperty("KeyHash");
|
||||||
|
Assert.NotNull(keyHash);
|
||||||
|
Assert.False(keyHash!.IsNullable);
|
||||||
|
|
||||||
|
var hashIndex = entityType.GetIndexes()
|
||||||
|
.FirstOrDefault(i => i.Properties.Any(p => p.Name == "KeyHash"));
|
||||||
|
Assert.NotNull(hashIndex);
|
||||||
|
Assert.True(hashIndex!.IsUnique);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ApiKey_HasNoPlaintextKeyValueColumn()
|
||||||
|
{
|
||||||
|
var entityType = _context.Model.FindEntityType(typeof(ScadaLink.Commons.Entities.InboundApi.ApiKey))!;
|
||||||
|
|
||||||
|
Assert.Null(entityType.FindProperty("KeyValue"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,96 @@
|
|||||||
|
using NSubstitute;
|
||||||
|
using ScadaLink.Commons.Entities.InboundApi;
|
||||||
|
using ScadaLink.Commons.Interfaces.Repositories;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
|
||||||
|
namespace ScadaLink.InboundAPI.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: <see cref="ApiKeyValidator"/> must authenticate by
|
||||||
|
/// hashing the presented candidate with the same HMAC-SHA256 pepper used at
|
||||||
|
/// creation, then comparing against the stored <see cref="ApiKey.KeyHash"/> — never
|
||||||
|
/// against a plaintext key. The comparison stays constant-time.
|
||||||
|
/// </summary>
|
||||||
|
public class ApiKeyHashValidationTests
|
||||||
|
{
|
||||||
|
private const string Pepper = "a-sufficiently-long-server-side-pepper-value";
|
||||||
|
|
||||||
|
private readonly IInboundApiRepository _repository = Substitute.For<IInboundApiRepository>();
|
||||||
|
|
||||||
|
private static ApiKey StoredKey(ApiKeyHasher hasher, string liveKey, int id = 1, bool enabled = true)
|
||||||
|
{
|
||||||
|
var key = ApiKey.FromHash("MES-Production", hasher.Hash(liveKey));
|
||||||
|
key.Id = id;
|
||||||
|
key.IsEnabled = enabled;
|
||||||
|
return key;
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ValidateAsync_WithPepperedHasher_AcceptsKeyHashedWithSamePepper()
|
||||||
|
{
|
||||||
|
var hasher = new ApiKeyHasher(Pepper);
|
||||||
|
var stored = StoredKey(hasher, "live-secret-key");
|
||||||
|
var method = new ApiMethod("ingest", "return 1;") { Id = 10 };
|
||||||
|
|
||||||
|
_repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { stored });
|
||||||
|
_repository.GetMethodByNameAsync("ingest").Returns(method);
|
||||||
|
_repository.GetApprovedKeysForMethodAsync(10).Returns(new List<ApiKey> { stored });
|
||||||
|
|
||||||
|
var validator = new ApiKeyValidator(_repository, hasher);
|
||||||
|
|
||||||
|
var result = await validator.ValidateAsync("live-secret-key", "ingest");
|
||||||
|
|
||||||
|
Assert.True(result.IsValid);
|
||||||
|
Assert.Equal(200, result.StatusCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ValidateAsync_WrongKey_FailsEvenWhenItHashesToSomethingNonNull()
|
||||||
|
{
|
||||||
|
var hasher = new ApiKeyHasher(Pepper);
|
||||||
|
var stored = StoredKey(hasher, "the-real-key");
|
||||||
|
|
||||||
|
_repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { stored });
|
||||||
|
|
||||||
|
var validator = new ApiKeyValidator(_repository, hasher);
|
||||||
|
|
||||||
|
var result = await validator.ValidateAsync("a-wrong-key", "ingest");
|
||||||
|
|
||||||
|
Assert.False(result.IsValid);
|
||||||
|
Assert.Equal(401, result.StatusCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ValidateAsync_StoredHashIsNotThePlaintextKey()
|
||||||
|
{
|
||||||
|
// Sanity guard: the value the validator compares against must be a hash, not
|
||||||
|
// the live secret — a DB dump must not yield a usable credential.
|
||||||
|
var hasher = new ApiKeyHasher(Pepper);
|
||||||
|
var stored = StoredKey(hasher, "live-secret-key");
|
||||||
|
|
||||||
|
Assert.NotEqual("live-secret-key", stored.KeyHash);
|
||||||
|
|
||||||
|
_repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { stored });
|
||||||
|
var validator = new ApiKeyValidator(_repository, hasher);
|
||||||
|
|
||||||
|
// Presenting the stored hash itself must NOT authenticate — only the live key does.
|
||||||
|
var result = await validator.ValidateAsync(stored.KeyHash, "ingest");
|
||||||
|
Assert.False(result.IsValid);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ValidateAsync_KeyHashedUnderADifferentPepper_DoesNotAuthenticate()
|
||||||
|
{
|
||||||
|
var creationHasher = new ApiKeyHasher(Pepper);
|
||||||
|
var stored = StoredKey(creationHasher, "live-secret-key");
|
||||||
|
|
||||||
|
_repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { stored });
|
||||||
|
|
||||||
|
// A validator running with a different pepper cannot recognise the key.
|
||||||
|
var otherHasher = new ApiKeyHasher("a-totally-different-server-side-pepper-val");
|
||||||
|
var validator = new ApiKeyValidator(_repository, otherHasher);
|
||||||
|
|
||||||
|
var result = await validator.ValidateAsync("live-secret-key", "ingest");
|
||||||
|
Assert.False(result.IsValid);
|
||||||
|
}
|
||||||
|
}
|
||||||
121
tests/ScadaLink.ManagementService.Tests/ApiKeyCreationTests.cs
Normal file
121
tests/ScadaLink.ManagementService.Tests/ApiKeyCreationTests.cs
Normal file
@@ -0,0 +1,121 @@
|
|||||||
|
using System.Text.Json;
|
||||||
|
using Akka.Actor;
|
||||||
|
using Akka.TestKit.Xunit2;
|
||||||
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
|
using NSubstitute;
|
||||||
|
using ScadaLink.Commons.Entities.InboundApi;
|
||||||
|
using ScadaLink.Commons.Interfaces.Repositories;
|
||||||
|
using ScadaLink.Commons.Interfaces.Services;
|
||||||
|
using ScadaLink.Commons.Messages.Management;
|
||||||
|
using ScadaLink.Commons.Types.InboundApi;
|
||||||
|
using ScadaLink.ManagementService;
|
||||||
|
|
||||||
|
namespace ScadaLink.ManagementService.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// ConfigurationDatabase-012: creating an API key must generate a random key,
|
||||||
|
/// persist only its peppered hash, and return the plaintext to the caller exactly
|
||||||
|
/// once. The plaintext must never reach the stored entity.
|
||||||
|
/// </summary>
|
||||||
|
public class ApiKeyCreationTests : TestKit, IDisposable
|
||||||
|
{
|
||||||
|
private const string Pepper = "a-sufficiently-long-server-side-pepper-value";
|
||||||
|
|
||||||
|
private readonly ServiceCollection _services = new();
|
||||||
|
private readonly IInboundApiRepository _apiRepo = Substitute.For<IInboundApiRepository>();
|
||||||
|
private readonly IAuditService _auditService = Substitute.For<IAuditService>();
|
||||||
|
|
||||||
|
public ApiKeyCreationTests()
|
||||||
|
{
|
||||||
|
_services.AddScoped(_ => _apiRepo);
|
||||||
|
_services.AddScoped(_ => _auditService);
|
||||||
|
_services.AddSingleton<IApiKeyHasher>(new ApiKeyHasher(Pepper));
|
||||||
|
}
|
||||||
|
|
||||||
|
private IActorRef CreateActor()
|
||||||
|
{
|
||||||
|
var sp = _services.BuildServiceProvider();
|
||||||
|
return Sys.ActorOf(Props.Create(() => new ManagementActor(sp, NullLogger<ManagementActor>.Instance)));
|
||||||
|
}
|
||||||
|
|
||||||
|
private static ManagementEnvelope Envelope(object command, params string[] roles) =>
|
||||||
|
new(new AuthenticatedUser("admin", "Admin User", roles, Array.Empty<string>()),
|
||||||
|
command, Guid.NewGuid().ToString("N"));
|
||||||
|
|
||||||
|
void IDisposable.Dispose() => Shutdown();
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void CreateApiKey_PersistsOnlyHash_NeverPlaintext()
|
||||||
|
{
|
||||||
|
ApiKey? persisted = null;
|
||||||
|
_apiRepo.AddApiKeyAsync(Arg.Do<ApiKey>(k => persisted = k))
|
||||||
|
.Returns(Task.CompletedTask);
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
actor.Tell(Envelope(new CreateApiKeyCommand("MES-Production"), "Admin"));
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
// The response must carry the one-time plaintext key shown to the operator.
|
||||||
|
var plaintext = ExtractPlaintextKey(response.JsonData);
|
||||||
|
Assert.False(string.IsNullOrWhiteSpace(plaintext));
|
||||||
|
|
||||||
|
// The stored entity must carry a hash, never the plaintext.
|
||||||
|
Assert.NotNull(persisted);
|
||||||
|
Assert.NotEqual(plaintext, persisted!.KeyHash);
|
||||||
|
|
||||||
|
// The persisted hash must equal the peppered hash of the returned plaintext.
|
||||||
|
var hasher = new ApiKeyHasher(Pepper);
|
||||||
|
Assert.Equal(hasher.Hash(plaintext!), persisted.KeyHash);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void CreateApiKey_ResponseDoesNotEchoTheHash()
|
||||||
|
{
|
||||||
|
ApiKey? persisted = null;
|
||||||
|
_apiRepo.AddApiKeyAsync(Arg.Do<ApiKey>(k => persisted = k))
|
||||||
|
.Returns(Task.CompletedTask);
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
actor.Tell(Envelope(new CreateApiKeyCommand("MES-Production"), "Admin"));
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
Assert.NotNull(persisted);
|
||||||
|
// The serialized response must not leak the stored hash as a usable artifact.
|
||||||
|
Assert.DoesNotContain(persisted!.KeyHash, response.JsonData);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void CreateApiKey_TwoKeys_GenerateDistinctRandomValues()
|
||||||
|
{
|
||||||
|
var hashes = new List<string>();
|
||||||
|
_apiRepo.AddApiKeyAsync(Arg.Do<ApiKey>(k => hashes.Add(k.KeyHash)))
|
||||||
|
.Returns(Task.CompletedTask);
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
actor.Tell(Envelope(new CreateApiKeyCommand("KeyA"), "Admin"));
|
||||||
|
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
actor.Tell(Envelope(new CreateApiKeyCommand("KeyB"), "Admin"));
|
||||||
|
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
Assert.Equal(2, hashes.Count);
|
||||||
|
Assert.NotEqual(hashes[0], hashes[1]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The create response is JSON carrying the one-time plaintext key under a
|
||||||
|
/// <c>PlaintextKey</c> (or <c>Key</c>) property.
|
||||||
|
/// </summary>
|
||||||
|
private static string? ExtractPlaintextKey(string json)
|
||||||
|
{
|
||||||
|
using var doc = JsonDocument.Parse(json);
|
||||||
|
var root = doc.RootElement;
|
||||||
|
if (root.TryGetProperty("plaintextKey", out var p) || root.TryGetProperty("PlaintextKey", out p))
|
||||||
|
return p.GetString();
|
||||||
|
if (root.TryGetProperty("key", out var k) || root.TryGetProperty("Key", out k))
|
||||||
|
return k.GetString();
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user