From 57d208429073f5147ca175ae97a5cddb744edcf4 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 27 Sep 2026 19:23:02 -0400 Subject: [PATCH 1/3] feat(certification): give an operator a way to turn pinning on TrustedEd25519PublicKeys was always settable and PinningEnabled always read it, but nothing outside tests ever assigned it. Every production verifier therefore ran unpinned and checked each signature against the public key the RECORD carries -- which a record signed with an attacker's own keypair satisfies. The options file says so in its own words. CertificationTrustPolicy is the missing assignment: it reads a set of trusted PUBLIC keys (ASHLAR_CERT_TRUSTED_ED25519_KEYS, or any configuration reader) and hands verifiers Strict with that set applied. The file-backed record store, the certified brick registry, the composition constituent checker, the hot-swap host, the self-extend admission edge, the adaptation wiring and the state-log verifier all take it; with nothing configured each of them resolves to the Strict preset instance itself, so an unconfigured host is unchanged. No private key is configurable here. The minting key stays with the certifier (SPEC-006); publishing the public half is what pinning is for. Configuration fails closed and loud. Absent is not malformed: absent leaves the preset alone, but a value that is present and unusable -- naming no key, not Base64, not 32 bytes -- throws at resolution rather than resolving to an EMPTY set, because an empty set reads as pinning OFF at every call site that tests PinningEnabled. ASHLAR_CERT_PINNING_REQUIRED lets an operator assert that pinning is in effect, and an unrecognized value there is refused rather than read as false. Co-Authored-By: Claude Opus 5 --- .../SelfProducedBrickCertificationPolicy.cs | 20 +- .../CertificationTrustPolicy.cs | 322 ++++++++++++++++ .../CertificationVerifyOptions.cs | 9 + .../StateLogVerifier.cs | 11 +- .../AdaptationServiceCollectionExtensions.cs | 5 +- .../Certification/CertifiedBrickRegistry.cs | 15 +- .../CompositionConstituentChecker.cs | 14 +- .../FileCertificationRecordStore.cs | 18 +- .../HotSwap/CertifiedBrickHotSwapHost.cs | 16 +- .../TrustedKeyPinningConfigurationTests.cs | 360 ++++++++++++++++++ 10 files changed, 772 insertions(+), 18 deletions(-) create mode 100644 src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs create mode 100644 src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs diff --git a/src/Ashlar.BackgroundAgents/Security/SelfProducedBrickCertificationPolicy.cs b/src/Ashlar.BackgroundAgents/Security/SelfProducedBrickCertificationPolicy.cs index 79c047623..3ef9afc23 100644 --- a/src/Ashlar.BackgroundAgents/Security/SelfProducedBrickCertificationPolicy.cs +++ b/src/Ashlar.BackgroundAgents/Security/SelfProducedBrickCertificationPolicy.cs @@ -13,10 +13,24 @@ namespace Ashlar.BackgroundAgents.Security; public sealed class SelfProducedBrickCertificationPolicy : IPolicy { private readonly ICertificationRecordStore _certificationStore; - - public SelfProducedBrickCertificationPolicy(ICertificationRecordStore certificationStore) + private readonly CertificationVerifyOptions _verifyOptions; + + /// Initializes the policy. + /// Store consulted for the written brick's record. + /// + /// Operator trust configuration supplying the pinned signer set. Defaults to + /// , so a host that configured + /// ASHLAR_CERT_TRUSTED_ED25519_KEYS pins this edge as well; with nothing configured + /// this is the Strict preset exactly as before. Without a pinning set, "certified" + /// here means only that the record is self-consistent, which a brick the agent signed with + /// its own keypair also is. + /// + public SelfProducedBrickCertificationPolicy( + ICertificationRecordStore certificationStore, + CertificationTrustPolicy? trustPolicy = null) { _certificationStore = certificationStore ?? throw new ArgumentNullException(nameof(certificationStore)); + _verifyOptions = (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict; } public bool Approve(ToolCall call, WorldSnapshot s, out string reason) @@ -57,7 +71,7 @@ public bool Approve(ToolCall call, WorldSnapshot s, out string reason) var trust = CertificationTrustVerifier.Verify( CertificationRecordMapper.ToData(record), content, - options: CertificationVerifyOptions.Strict); + options: _verifyOptions); if (!trust.Trusted) { reason = $"Self-produced brick admission refused for '{brickId}': {trust.FailureCode} — {trust.Reason}"; diff --git a/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs b/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs new file mode 100644 index 000000000..6ea0ebd68 --- /dev/null +++ b/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs @@ -0,0 +1,322 @@ +namespace Ashlar.Certification.Contracts; + +/// +/// Raised when trust configuration is present but cannot be turned into a usable pinning set. +/// +/// +/// This exception exists so that malformed configuration cannot be mistaken for absent +/// configuration. Absent configuration means "this operator has not pinned anything" and leaves +/// verification exactly where it was; malformed configuration means "this operator tried to pin +/// something and we do not know what", and the only safe answer to that is to stop. Degrading it +/// to an empty pinning set would turn pinning OFF — the direction that makes a record signed with +/// an attacker's own keypair verify — so nothing in this file catches it on the operator's behalf. +/// +public sealed class CertificationTrustConfigurationException : Exception +{ + /// Initializes the exception with a message. + /// What was wrong with the configuration. + public CertificationTrustConfigurationException(string message) + : base(message) + { + } + + /// Initializes the exception with a message and the parse failure beneath it. + /// What was wrong with the configuration. + /// The underlying parse failure. + public CertificationTrustConfigurationException(string message, Exception innerException) + : base(message, innerException) + { + } +} + +/// +/// The operator-facing way to turn pinning on: reads a set of trusted Ed25519 PUBLIC keys from +/// configuration and hands verifiers the that carry it. +/// +/// +/// Why this type exists. +/// has always been settable, and has always +/// read it — but nothing outside tests ever assigned it, so every production verifier ran with +/// pinning off and checked each record's signature against the public key the RECORD carries. A +/// record signed with an attacker's own keypair is self-consistent and passes that check. This type +/// is the missing assignment: a host resolves one policy and every verifier it composes applies the +/// same pinning set. +/// +/// Public keys only. holds Base64 raw 32-byte +/// Ed25519 PUBLIC keys. There is deliberately no configuration path here for a private key — that +/// is the minting capability, it belongs to the certifier, and SPEC-006 keeps it out of the repo +/// and out of CI secrets. Publishing the public half is what pinning is for. +/// +/// The failure directions are not symmetric. Configuration that is absent leaves the +/// basis preset untouched: the host runs as it did before pinning was configurable, which is a +/// weaker posture the operator already had. Configuration that is PRESENT but unusable — no keys +/// in it, a value that is not Base64, a key that is not 32 bytes — throws +/// at resolution, before any record is +/// verified. An operator who wants "pinning must be in effect" to be checkable sets +/// , and then an absent or empty key set is a startup failure +/// too. +/// +public sealed class CertificationTrustPolicy +{ + /// + /// Environment variable holding the trusted signer set: Base64 raw 32-byte Ed25519 PUBLIC + /// keys, separated by commas, semicolons or whitespace (newlines included, so a deployment can + /// mount the list as a multi-line secret). + /// + public const string TrustedKeysVariable = "ASHLAR_CERT_TRUSTED_ED25519_KEYS"; + + /// + /// Environment variable that asserts pinning must be in effect. Accepts 1/true/yes/on + /// and 0/false/no/off, case-insensitively. When it asserts pinning and + /// yields no keys, resolution throws rather than running + /// unpinned. A value that is neither also throws: reading an unrecognized value as "not + /// required" would let a typo silently switch pinning off. + /// + public const string PinningRequiredVariable = "ASHLAR_CERT_PINNING_REQUIRED"; + + private static readonly char[] KeySeparators = { ',', ';', ' ', '\t', '\r', '\n' }; + + private static readonly string[] TruthyValues = { "1", "true", "yes", "on" }; + + private static readonly string[] FalsyValues = { "0", "false", "no", "off" }; + + private static readonly Lazy AmbientPolicy = + new Lazy(FromEnvironment, LazyThreadSafetyMode.ExecutionAndPublication); + + private CertificationTrustPolicy(string[]? trustedKeys) + { + TrustedEd25519PublicKeys = trustedKeys; + Strict = Apply(CertificationVerifyOptions.Strict); + Default = Apply(CertificationVerifyOptions.Default); + } + + /// + /// The policy this process runs under, resolved once from the environment on first use. + /// + /// + /// Cached because a verifier must not change its mind about which signers it accepts halfway + /// through a process, and because the alternative — re-reading the environment per + /// verification — makes the trusted set writable by anything that can set a variable in this + /// process. If resolution throws, every access rethrows the same failure: a host whose trust + /// configuration is unusable does not get to run verifications with pinning off. + /// + public static CertificationTrustPolicy Ambient => AmbientPolicy.Value; + + /// + /// A policy that pins nothing, for hosts and tests that want the presets verbatim. Every + /// Apply on it returns its argument. + /// + public static CertificationTrustPolicy Unpinned { get; } = new CertificationTrustPolicy(null); + + /// + /// The trusted signer set, or null when the operator configured none. Never empty: an empty + /// set would read as "pinning off" at every call site that tests + /// , so it is refused at parse time. + /// + public IReadOnlyCollection? TrustedEd25519PublicKeys { get; } + + /// True when this policy carries a signer set, i.e. when pinning is on. + public bool PinningConfigured => TrustedEd25519PublicKeys is not null; + + /// + /// with this policy's pinning set applied. + /// With nothing configured this is the Strict preset itself, unchanged. + /// + public CertificationVerifyOptions Strict { get; } + + /// + /// with this policy's pinning set applied. + /// With nothing configured this is the Default preset itself, unchanged. + /// + public CertificationVerifyOptions Default { get; } + + /// + /// Resolves the policy from process environment variables. Prefer in + /// hosts; this overload exists for a host that wants to resolve (and fail) at a point it + /// chooses. + /// + public static CertificationTrustPolicy FromEnvironment() => + FromConfiguration(Environment.GetEnvironmentVariable); + + /// + /// Resolves the policy from an arbitrary configuration reader, so a host can source the same + /// settings from IConfiguration, a secrets mount or a command line rather than the + /// environment. + /// + /// + /// Reads a configuration key, returning null when it is not set. Called with + /// and . + /// + /// + /// The configuration is present but unusable, or asserts pinning it does not supply keys for. + /// + public static CertificationTrustPolicy FromConfiguration(Func read) + { + if (read is null) + throw new ArgumentNullException(nameof(read)); + + var pinningRequired = ParsePinningRequired(read(PinningRequiredVariable)); + var keys = ParseTrustedKeys(read(TrustedKeysVariable)); + + if (pinningRequired && keys is null) + { + throw new CertificationTrustConfigurationException( + $"{PinningRequiredVariable} asserts that certification pinning is in effect, but " + + $"{TrustedKeysVariable} supplies no trusted Ed25519 public key. Refusing to start " + + "unpinned: without a pinning set, verification accepts the key each record carries, " + + "so a record signed with any keypair verifies."); + } + + return keys is null && !pinningRequired + ? Unpinned + : new CertificationTrustPolicy(keys); + } + + /// + /// Builds a policy from an explicit set of Base64 raw Ed25519 public keys, for a host that + /// already holds them (a mounted trust bundle, a parsed manifest) rather than a raw setting. + /// The same validation applies: an empty or malformed set throws. + /// + /// Base64 raw 32-byte Ed25519 public keys. + public static CertificationTrustPolicy FromTrustedKeys(IEnumerable trustedEd25519PublicKeys) + { + if (trustedEd25519PublicKeys is null) + throw new ArgumentNullException(nameof(trustedEd25519PublicKeys)); + + return new CertificationTrustPolicy(Validate(trustedEd25519PublicKeys, TrustedKeysVariable)); + } + + /// + /// Returns carrying this policy's pinning set. With nothing + /// configured, returns itself — this method never removes a pinning + /// set, because config that says nothing must not undo a decision the code made. + /// + /// The strictness preset to pin, e.g. . + /// + /// already pins a different set of keys. Silently replacing it would + /// mean configuration could redirect trust that was chosen in code, and merging the two would + /// widen the accepted signer set beyond either — so a conflict is reported, not resolved. + /// + public CertificationVerifyOptions Apply(CertificationVerifyOptions basis) + { + if (basis is null) + throw new ArgumentNullException(nameof(basis)); + + var keys = TrustedEd25519PublicKeys; + if (keys is null) + return basis; + + if (basis.PinningEnabled && !SameKeys(basis.TrustedEd25519PublicKeys!, keys)) + { + throw new CertificationTrustConfigurationException( + "The supplied verification options already pin a different set of Ed25519 signers. " + + $"Configured pinning ({TrustedKeysVariable}) will not override a pinning set chosen " + + "in code, and will not widen it by merging: resolve the two deliberately."); + } + + return new CertificationVerifyOptions + { + MinimumSchemaVersion = basis.MinimumSchemaVersion, + // Pinning implies the signature it pins: an unsigned record cannot be pinned, and the + // verifier already treats PinningEnabled as requiring a signature. Making it explicit + // here keeps the options object self-describing rather than relying on that coupling. + RequireEd25519Signature = true, + TrustedEd25519PublicKeys = keys, + RequireGateEmittedArtifact = basis.RequireGateEmittedArtifact, + RequireCertifierIdentity = basis.RequireCertifierIdentity, + }; + } + + /// + /// Parses a configured signer set: null when the setting is absent, otherwise a non-empty, + /// validated set. Present-but-unusable throws. + /// + /// The raw setting value, or null when it is not set. + internal static string[]? ParseTrustedKeys(string? configured) + { + if (configured is null) + return null; + + var entries = configured.Split(KeySeparators, StringSplitOptions.RemoveEmptyEntries); + return Validate(entries, TrustedKeysVariable); + } + + private static string[] Validate(IEnumerable entries, string settingName) + { + var keys = new List(); + foreach (var entry in entries) + { + var candidate = entry?.Trim(); + if (string.IsNullOrEmpty(candidate)) + continue; + + byte[] bytes; + try + { + bytes = Convert.FromBase64String(candidate!); + } + catch (FormatException ex) + { + throw new CertificationTrustConfigurationException( + $"{settingName} contains an entry that is not valid Base64, so it cannot be an " + + "Ed25519 public key. Refusing to start with a pinning set that dropped an entry: " + + "a set that loses keys silently refuses honest records, and a set that ends up " + + "empty stops pinning altogether.", + ex); + } + + if (bytes.Length != 32) + { + throw new CertificationTrustConfigurationException( + $"{settingName} contains an entry that decodes to {bytes.Length} bytes; an Ed25519 " + + "public key is 32 raw bytes. Refusing rather than pinning a value no signature " + + "can ever match."); + } + + if (!keys.Contains(candidate!, StringComparer.Ordinal)) + keys.Add(candidate!); + } + + if (keys.Count == 0) + { + throw new CertificationTrustConfigurationException( + $"{settingName} is set but names no Ed25519 public key. An empty pinning set turns " + + "pinning OFF, which makes verification accept whatever key a record carries — the " + + "opposite of what setting this asks for. Unset it to run unpinned deliberately."); + } + + return keys.ToArray(); + } + + private static bool ParsePinningRequired(string? configured) + { + if (configured is null) + return false; + + var value = configured.Trim(); + if (value.Length == 0 || FalsyValues.Contains(value, StringComparer.OrdinalIgnoreCase)) + return false; + + if (TruthyValues.Contains(value, StringComparer.OrdinalIgnoreCase)) + return true; + + throw new CertificationTrustConfigurationException( + $"{PinningRequiredVariable} is set to a value that is neither true nor false. It is " + + "refused rather than read as false, because a typo in a switch that demands pinning " + + "must not be the thing that silently disables it."); + } + + private static bool SameKeys(IReadOnlyCollection left, IReadOnlyCollection right) + { + if (left.Count != right.Count) + return false; + + foreach (var key in left) + { + if (!right.Contains(key, StringComparer.Ordinal)) + return false; + } + + return true; + } +} diff --git a/src/Ashlar.Certification.Contracts/CertificationVerifyOptions.cs b/src/Ashlar.Certification.Contracts/CertificationVerifyOptions.cs index beb2067c5..f1b02939d 100644 --- a/src/Ashlar.Certification.Contracts/CertificationVerifyOptions.cs +++ b/src/Ashlar.Certification.Contracts/CertificationVerifyOptions.cs @@ -95,6 +95,15 @@ public sealed class CertificationVerifyOptions /// self-consistent and verifies. Pinning is what makes "signed" mean "signed by someone /// we accept". Setting this implies : an unsigned /// record cannot be pinned. + /// + /// Who sets this. is the production path: + /// an operator lists trusted PUBLIC keys in ASHLAR_CERT_TRUSTED_ED25519_KEYS (or any + /// configuration source), and the hosts that verify records — the record store, the certified + /// registry, the hot-swap host, the self-extend admission edge, the state-log verifier — apply + /// the resulting options instead of the bare preset. Being settable was + /// never the gap; being assigned outside tests was. Configuration that is present but unusable + /// throws there rather than resolving to an empty set, because an empty set reads as pinning + /// off at every call site that tests . /// public IReadOnlyCollection? TrustedEd25519PublicKeys { get; init; } diff --git a/src/Ashlar.Certification.State/StateLogVerifier.cs b/src/Ashlar.Certification.State/StateLogVerifier.cs index 113ee0aef..9718b1577 100644 --- a/src/Ashlar.Certification.State/StateLogVerifier.cs +++ b/src/Ashlar.Certification.State/StateLogVerifier.cs @@ -9,9 +9,12 @@ public static class StateLogVerifier private static readonly CertifiedTransitionBuilder TransitionBuilder = new(); /// - /// Verifies under : - /// every behavior certificate must carry a verifying Ed25519 signature, a gate-emitted - /// artifact and a certifier identity. This is the production default and is unchanged. + /// Verifies under + /// plus the signer set the operator pinned through + /// : every behavior certificate must carry a + /// verifying Ed25519 signature, a gate-emitted artifact and a certifier identity, and — once + /// keys are configured — a signature from one of those keys. With nothing configured this is + /// the Strict preset itself, so the production default is unchanged. /// /// /// Strict requires an Ed25519 signature, which the netstandard2.0 asset of @@ -31,7 +34,7 @@ public static StateLogTrustResult Verify( ICertificateResolver resolver, string? hmacKey = null, ITransitionReplayer? replayer = null) => - Verify(log, schema, resolver, hmacKey, replayer, Contracts.CertificationVerifyOptions.Strict); + Verify(log, schema, resolver, hmacKey, replayer, Contracts.CertificationTrustPolicy.Ambient.Strict); /// /// Verifies , applying to every behavior diff --git a/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs b/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs index c0d9e2b42..268016902 100644 --- a/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs +++ b/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs @@ -98,7 +98,10 @@ public static IServiceCollection AddAdaptationInfrastructure( var store = sp.GetRequiredService(); var signer = sp.GetRequiredService(); var record = store.Get("observation.context"); - if (record is { Admitted: true, Signed: true } && signer.Verify(record, CertificationVerifyOptions.Strict)) + // Strict plus whatever signer set the operator pinned; with nothing configured this + // is the Strict preset unchanged. + if (record is { Admitted: true, Signed: true } + && signer.Verify(record, CertificationTrustPolicy.Ambient.Strict)) { var contextAssembler = sp.GetRequiredService(); bricks.Add(new ObservationContextBrick(contextAssembler)); diff --git a/src/Ashlar.Infrastructure/Certification/CertifiedBrickRegistry.cs b/src/Ashlar.Infrastructure/Certification/CertifiedBrickRegistry.cs index c33aa4a22..c784e2f93 100644 --- a/src/Ashlar.Infrastructure/Certification/CertifiedBrickRegistry.cs +++ b/src/Ashlar.Infrastructure/Certification/CertifiedBrickRegistry.cs @@ -15,16 +15,27 @@ public sealed class CertifiedBrickRegistry : Ashlar.Core.Domain.Execution.IBrick private readonly ICertificationRecordStore _store; private readonly CertificationRecordSigner _signer; private readonly ILogger? _logger; + private readonly CertificationVerifyOptions _verifyOptions; /// Initializes a new certified brick registry. + /// Record store admissions are persisted to. + /// Signer used to verify a record before admitting its brick. + /// Optional logger. + /// + /// Operator trust configuration supplying the pinned signer set. Defaults to + /// ; with nothing configured this is the + /// Strict preset exactly as before. + /// public CertifiedBrickRegistry( ICertificationRecordStore store, CertificationRecordSigner signer, - ILogger? logger = null) + ILogger? logger = null, + CertificationTrustPolicy? trustPolicy = null) { _store = store ?? throw new ArgumentNullException(nameof(store)); _signer = signer ?? throw new ArgumentNullException(nameof(signer)); _logger = logger; + _verifyOptions = (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict; } /// Gets brick. @@ -41,7 +52,7 @@ public IReadOnlyList GetAllBricks() => internal bool TryAdmit(DomainBrick brick, CertificationRecord record) { - if (!record.Admitted || !record.Signed || !_signer.Verify(record, CertificationVerifyOptions.Strict)) + if (!record.Admitted || !record.Signed || !_signer.Verify(record, _verifyOptions)) { _logger?.LogWarning("Rejected ungated brick admission attempt for {BrickId}", brick.Id); return false; diff --git a/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs b/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs index 065928561..fdda73a63 100644 --- a/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs +++ b/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs @@ -8,12 +8,22 @@ namespace Ashlar.Infrastructure.Certification.Composition; internal static class CompositionConstituentChecker { /// Check. + /// Composition whose constituents are checked. + /// Store holding each constituent's record. + /// Signer used to verify each record. + /// + /// Operator trust configuration supplying the pinned signer set. Defaults to + /// ; with nothing configured this is the + /// Strict preset exactly as before. + /// public static ConstituentCheckResult Check( CompositionSpec spec, ICertificationRecordStore brickCertificationStore, - CertificationRecordSigner signer) + CertificationRecordSigner signer, + CertificationTrustPolicy? trustPolicy = null) { var violations = new List(); + var verifyOptions = (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict; foreach (var node in spec.Nodes) { @@ -30,7 +40,7 @@ public static ConstituentCheckResult Check( continue; } - if (!signer.Verify(record, CertificationVerifyOptions.Strict)) + if (!signer.Verify(record, verifyOptions)) { violations.Add($"Constituent brick '{node.BrickId}' has invalid certification signature"); } diff --git a/src/Ashlar.Infrastructure/Certification/FileCertificationRecordStore.cs b/src/Ashlar.Infrastructure/Certification/FileCertificationRecordStore.cs index 6037a07db..9326d05a4 100644 --- a/src/Ashlar.Infrastructure/Certification/FileCertificationRecordStore.cs +++ b/src/Ashlar.Infrastructure/Certification/FileCertificationRecordStore.cs @@ -12,12 +12,20 @@ public sealed class FileCertificationRecordStore : ICertificationRecordStore { private readonly string _directory; private readonly CertificationRecordSigner _signer; + private readonly CertificationVerifyOptions _verifyOptions; /// Initializes a new file certification record store. /// Directory holding one JSON record per brick. /// /// Signer used to RE-VERIFY every record on load. Defaults to the standard signer. /// + /// + /// Operator trust configuration, which supplies the pinned signer set re-verification applies. + /// Defaults to , so a host that configured + /// ASHLAR_CERT_TRUSTED_ED25519_KEYS pins this store without threading anything through. + /// Resolved HERE rather than per load, so a host whose trust configuration is unusable fails + /// when the store is built instead of quietly verifying unpinned for the process's lifetime. + /// /// /// Verification on load is the whole reason this store can be trusted. Records live /// as plain JSON on disk, so anything that can write the directory can set @@ -26,10 +34,14 @@ public sealed class FileCertificationRecordStore : ICertificationRecordStore /// including its ContentHash, so editing either the admission flags or the /// certified content hash invalidates it. /// - public FileCertificationRecordStore(string directory, CertificationRecordSigner? signer = null) + public FileCertificationRecordStore( + string directory, + CertificationRecordSigner? signer = null, + CertificationTrustPolicy? trustPolicy = null) { _directory = directory ?? throw new ArgumentNullException(nameof(directory)); _signer = signer ?? new CertificationRecordSigner(); + _verifyOptions = (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict; Directory.CreateDirectory(_directory); } @@ -108,7 +120,9 @@ record = JsonSerializer.Deserialize(File.ReadAllText(path), if (record is null) return null; - return _signer.Verify(record, CertificationVerifyOptions.Strict) ? record : null; + // Strict PLUS whatever the operator pinned. Without the pinning set this check asks only + // "is this record self-consistent", which any keypair can satisfy. + return _signer.Verify(record, _verifyOptions) ? record : null; } /// Whether admitted. diff --git a/src/Ashlar.Infrastructure/Certification/HotSwap/CertifiedBrickHotSwapHost.cs b/src/Ashlar.Infrastructure/Certification/HotSwap/CertifiedBrickHotSwapHost.cs index afecb78ab..078f68460 100644 --- a/src/Ashlar.Infrastructure/Certification/HotSwap/CertifiedBrickHotSwapHost.cs +++ b/src/Ashlar.Infrastructure/Certification/HotSwap/CertifiedBrickHotSwapHost.cs @@ -20,7 +20,8 @@ namespace Ashlar.Infrastructure.Certification.HotSwap; /// /// Verify-at-load. Every brick's certification record is re-verified /// against the exact source bytes being loaded ( -/// with ). When a supplied PE matches +/// with , plus the signer set the operator pinned +/// through ). When a supplied PE matches /// the record's gate-emitted-artifact hash, the artifact-bytes overload binds /// those bytes; otherwise the host rematerializes from wrapped source. /// Fail-closed swap. Any verification, compile, load, or @@ -60,6 +61,7 @@ public sealed class CertifiedBrickHotSwapHost : IDisposable private readonly Ashlar.Core.Application.Autonomy.LoopPauseControl? _pauseControl; private readonly TimeSpan? _cadenceFloor; private readonly TimeProvider _clock; + private readonly CertificationVerifyOptions _verifyOptions; private BrickGeneration? _current; private int _generationCounter; @@ -110,6 +112,7 @@ public sealed class CertifiedBrickHotSwapHost : IDisposable /// Global pause (R6.2): while paused, autonomous swaps are refused; human-driven swaps proceed. /// Minimum interval between autonomous swaps (R6.1) so the runtime never absorbs changes faster than watch windows clear. /// Clock for cadence decisions; system time when null. + /// Operator trust configuration supplying the pinned signer set; defaults to , which is the Strict preset itself when nothing is configured. public CertifiedBrickHotSwapHost( ICertifiedBrickSwapProvenanceSink? provenanceSink = null, ILogger? logger = null, @@ -121,8 +124,13 @@ public CertifiedBrickHotSwapHost( Ashlar.Core.Application.Autonomy.ILineageAuthority? lineageAuthority = null, Ashlar.Core.Application.Autonomy.LoopPauseControl? pauseControl = null, TimeSpan? cadenceFloor = null, - TimeProvider? clock = null) + TimeProvider? clock = null, + CertificationTrustPolicy? trustPolicy = null) { + // Strict plus whatever signer set the operator pinned. Resolved once, at construction: a + // host must not change its mind about which signers it accepts between generations, and a + // trust configuration it cannot parse must stop it here rather than at its first swap. + _verifyOptions = (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict; _provenanceSink = provenanceSink; _logger = logger; _hmacKey = hmacKey; @@ -843,12 +851,12 @@ private List VerifyAll(IReadOnlyList +/// Pinning as an operator can actually turn it on. +/// was always settable and always read it, +/// but nothing outside tests ever assigned it: every production verifier therefore ran with pinning +/// off and checked each signature against the public key the RECORD carries, which a record signed +/// with an attacker's own keypair satisfies. is the assignment. +/// These tests drive it THROUGH the production consumers — the file-backed record store and the +/// self-extend admission edge — rather than handing a verifier options built in the test, because +/// options built in a test prove only that the verifier's pinning branch works, which was never the +/// part that was missing. +/// +/// The second direction matters as much as the first: configuration that is present but +/// unusable must not resolve to an EMPTY pinning set. An empty set reads as pinning off at every +/// call site that tests PinningEnabled, so silently degrading to it would turn an operator's +/// attempt to pin into the weakest posture available, exactly when they believed they had the +/// strongest. +/// +/// Joins the non-parallel EnvironmentVariables collection. This class writes no +/// environment variable, but the self-extend edge verifies with no explicit HMAC key, so it READS +/// ASHLAR_CERT_DEV_HMAC_KEY through CertificationRecordSigning; a class that rewrote +/// that variable beside it would invalidate signatures minted here between the mint and the +/// verification. +/// +[Trait("Category", "Certification")] +[Collection("EnvironmentVariables")] +public sealed class TrustedKeyPinningConfigurationTests : IDisposable +{ + private const string BrickRelativePath = "src/Ashlar.Bricks.Pinning/PinnedProbeBrick.cs"; + private const string BrickSource = "namespace Ashlar.Bricks.Pinning; public sealed class PinnedProbeBrick { }"; + private const string StoreHmacKey = "pinning-tests-explicit-hmac-key"; + + private readonly string _directory; + + /// Creates the record directory this class's file store reads. + public TrustedKeyPinningConfigurationTests() + { + _directory = Path.Combine( + Path.GetTempPath(), + "ashlar-pinning-config-" + Guid.NewGuid().ToString("N")[..12]); + Directory.CreateDirectory(_directory); + } + + /// Removes the record directory. + public void Dispose() + { + try { Directory.Delete(_directory, recursive: true); } catch { /* best effort */ } + } + + /// + /// Fact (a) on the load path: a configured pinning set makes the store that re-verifies records + /// on load refuse one signed by a key the operator did not list — and the same store, pinned to + /// the signer that actually minted it, still returns it. The positive control is what makes this + /// load-bearing: Get reports an unverifiable record as ABSENT, so a refusal alone could + /// have come from anything. + /// + [Fact] + public void A_configured_pinning_set_makes_the_record_store_refuse_a_foreign_signer() + { + var ours = CreateEd25519Key(); + var theirs = CreateEd25519Key(); + var brickId = SaveRecordSignedBy(theirs.PrivateKeyBase64); + + Store(Policy(ours.PublicKeyBase64)).Get(brickId).Should().BeNull( + "the record is signed by a key this operator did not pin, and verification checks the " + + "signature against the key the record itself carries, so pinning is the only thing " + + "that can tell those two keys apart"); + + Store(Policy(theirs.PublicKeyBase64)).Get(brickId).Should().NotBeNull( + "pinned to the key that actually minted the record, the same store must still load it — " + + "otherwise the refusal above proves nothing about pinning"); + + Store(CertificationTrustPolicy.Unpinned).Get(brickId).Should().NotBeNull( + "this is the defect being closed: with no pinning set configured, a record signed by " + + "any keypair at all is self-consistent and loads"); + } + + /// + /// Fact (a) on the admission edge: the same configuration refuses a self-produced brick whose + /// certificate was signed by a key the operator did not list, with the verifier's own failure + /// code in the denial reason. + /// + [Fact] + public void A_configured_pinning_set_makes_the_self_extend_edge_refuse_a_foreign_signer() + { + var ours = CreateEd25519Key(); + var theirs = CreateEd25519Key(); + var store = new InMemoryCertificationRecordStore(); + var brickId = BrickAdmissionPathHelper.InferBrickId(BrickRelativePath, BrickSource)!; + store.Save(new CertificationRecordSigner(ed25519PrivateKeyBase64: theirs.PrivateKeyBase64) + .SignRecord(UnsignedRecord(brickId))); + + var call = new ToolCall( + "repo.fs.write", + JsonSerializer.SerializeToElement(new { path = BrickRelativePath, content = BrickSource })); + var snapshot = new WorldSnapshot(0, new Dictionary + { + ["RepoRoot"] = "/workspace", + ["selfExtendAdmission"] = true, + }); + + new SelfProducedBrickCertificationPolicy(store, Policy(ours.PublicKeyBase64)) + .Approve(call, snapshot, out var refused) + .Should().BeFalse("the certificate is signed by a key this operator did not pin"); + refused.Should().Contain("ed25519-key-not-trusted"); + + new SelfProducedBrickCertificationPolicy(store, Policy(theirs.PublicKeyBase64)) + .Approve(call, snapshot, out var admitted) + .Should().BeTrue("pinned to the minting key, the same write must still be admitted"); + admitted.Should().Be("OK"); + } + + /// + /// Fact (a), multi-signer: an operator listing several keys — commas, semicolons, newlines, + /// duplicates — gets a set that accepts each of them and nothing else. + /// + [Fact] + public void A_configured_pinning_set_accepts_every_signer_it_lists_and_no_other() + { + var first = CreateEd25519Key(); + var second = CreateEd25519Key(); + var stranger = CreateEd25519Key(); + var policy = CertificationTrustPolicy.FromConfiguration( + Config($" {first.PublicKeyBase64};{second.PublicKeyBase64}\n\t{first.PublicKeyBase64} ")); + + policy.TrustedEd25519PublicKeys.Should().BeEquivalentTo( + new[] { first.PublicKeyBase64, second.PublicKeyBase64 }, + "separators are commas, semicolons and whitespace, and a repeated key is one key"); + + var store = Store(policy); + store.Get(SaveRecordSignedBy(first.PrivateKeyBase64)).Should().NotBeNull(); + store.Get(SaveRecordSignedBy(second.PrivateKeyBase64)).Should().NotBeNull(); + store.Get(SaveRecordSignedBy(stranger.PrivateKeyBase64)).Should().BeNull( + "a key absent from the list stays absent from the trusted set"); + } + + /// + /// Fact (b): configuration that is present but names no key is refused at resolution. Returning + /// a policy whose set is empty would read as "pinning off" everywhere downstream, so there is + /// no such policy to inspect — the refusal is the whole assertion. + /// + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("\n\t ")] + [InlineData(",")] + [InlineData(" ; , ")] + public void Configuration_that_names_no_key_is_refused_rather_than_disabling_pinning(string configured) + { + var resolve = () => CertificationTrustPolicy.FromConfiguration(Config(configured)); + + resolve.Should().Throw( + "an empty pinning set turns pinning OFF, which is the dangerous direction: " + + "verification would accept whatever key the record carries") + .WithMessage($"*{CertificationTrustPolicy.TrustedKeysVariable}*"); + } + + /// + /// Fact (b), malformed rather than empty: an entry that cannot be an Ed25519 public key is + /// refused, including when it sits beside a good one. Dropping the bad entry would leave a set + /// that is quietly narrower than the operator wrote, and dropping the only entry would leave no + /// set at all. + /// + [Theory] + [InlineData("this-is-not-base64!!")] + [InlineData("c2hvcnQ=")] + // 44 unpadded Base64 characters decode to 33 bytes: one byte too long for Ed25519, and the + // kind of near-miss a truncated or concatenated secret produces. 43 characters plus '=' would + // decode to a structurally valid 32-byte key, which is why the length is spelled out here. + [InlineData("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")] + public void Configuration_that_cannot_be_a_key_is_refused(string configured) + { + var alone = () => CertificationTrustPolicy.FromConfiguration(Config(configured)); + alone.Should().Throw(); + + var good = CreateEd25519Key().PublicKeyBase64; + var beside = () => CertificationTrustPolicy.FromConfiguration(Config($"{good},{configured}")); + beside.Should().Throw( + "a set that silently loses an entry is not the set the operator configured"); + } + + /// + /// Fact (b), the operator's own assertion: a host told that pinning must be in effect refuses to + /// start without keys, and a switch value it cannot read is refused rather than taken as "not + /// required" — a typo must not be the thing that disables pinning. + /// + [Fact] + public void Requiring_pinning_without_keys_refuses_to_start() + { + var missingKeys = () => CertificationTrustPolicy.FromConfiguration(Config(null, "true")); + missingKeys.Should().Throw() + .WithMessage($"*{CertificationTrustPolicy.TrustedKeysVariable}*"); + + var typo = () => CertificationTrustPolicy.FromConfiguration( + Config(CreateEd25519Key().PublicKeyBase64, "treu")); + typo.Should().Throw( + "an unrecognized value read as false would silently switch pinning off"); + + CertificationTrustPolicy + .FromConfiguration(Config(CreateEd25519Key().PublicKeyBase64, "yes")) + .PinningConfigured.Should().BeTrue(); + CertificationTrustPolicy + .FromConfiguration(Config(CreateEd25519Key().PublicKeyBase64, "off")) + .PinningConfigured.Should().BeTrue("keys configure pinning whether or not it is demanded"); + } + + /// + /// Absent configuration is not malformed configuration: it leaves every preset byte-for-byte + /// where it was, so wiring this policy into the production verifiers changed nothing for a host + /// that configures nothing. + /// + [Fact] + public void Absent_configuration_leaves_the_presets_exactly_as_they_were() + { + var policy = CertificationTrustPolicy.FromConfiguration(_ => null); + + policy.PinningConfigured.Should().BeFalse(); + policy.TrustedEd25519PublicKeys.Should().BeNull("empty and absent must not be the same value"); + policy.Strict.Should().BeSameAs(CertificationVerifyOptions.Strict); + policy.Default.Should().BeSameAs(CertificationVerifyOptions.Default); + policy.Apply(CertificationVerifyOptions.Legacy).Should().BeSameAs(CertificationVerifyOptions.Legacy); + } + + /// + /// Pinning is additive: it must not relax any other strictness the basis carried. The reflection + /// check is the guard against the copy in Apply rotting — a sixth strictness option added + /// to would be silently dropped on every pinned host, + /// which is a relaxation nobody would see. + /// + [Fact] + public void Pinning_adds_to_strictness_and_relaxes_nothing() + { + var key = CreateEd25519Key().PublicKeyBase64; + var pinned = Policy(key).Strict; + + pinned.PinningEnabled.Should().BeTrue(); + pinned.TrustedEd25519PublicKeys.Should().BeEquivalentTo(new[] { key }); + pinned.RequireEd25519Signature.Should().BeTrue("an unsigned record cannot be pinned"); + pinned.MinimumSchemaVersion.Should().Be(CertificationVerifyOptions.Strict.MinimumSchemaVersion); + pinned.RequireGateEmittedArtifact.Should().Be(CertificationVerifyOptions.Strict.RequireGateEmittedArtifact); + pinned.RequireCertifierIdentity.Should().Be(CertificationVerifyOptions.Strict.RequireCertifierIdentity); + + typeof(CertificationVerifyOptions) + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanWrite) + .Select(p => p.Name) + .Should().BeEquivalentTo( + new[] + { + nameof(CertificationVerifyOptions.MinimumSchemaVersion), + nameof(CertificationVerifyOptions.RequireEd25519Signature), + nameof(CertificationVerifyOptions.TrustedEd25519PublicKeys), + nameof(CertificationVerifyOptions.RequireGateEmittedArtifact), + nameof(CertificationVerifyOptions.RequireCertifierIdentity), + }, + "CertificationTrustPolicy.Apply copies these five by hand; a new settable option " + + "must be added there too, or every pinned host silently loses it"); + } + + /// + /// Configuration may turn pinning on; it may not redirect a pinning set that code already chose, + /// and merging the two would widen the accepted signer set beyond either. Applying the same set + /// twice is not a conflict. + /// + [Fact] + public void Applying_configuration_over_a_pinning_set_chosen_in_code_is_refused() + { + var mine = CreateEd25519Key().PublicKeyBase64; + var yours = CreateEd25519Key().PublicKeyBase64; + var inCode = new CertificationVerifyOptions { TrustedEd25519PublicKeys = new[] { mine } }; + + var redirect = () => Policy(yours).Apply(inCode); + redirect.Should().Throw(); + + Policy(mine).Apply(inCode).TrustedEd25519PublicKeys.Should().BeEquivalentTo(new[] { mine }); + } + + private static Func Config(string? trustedKeys, string? pinningRequired = null) => + name => name switch + { + CertificationTrustPolicy.TrustedKeysVariable => trustedKeys, + CertificationTrustPolicy.PinningRequiredVariable => pinningRequired, + _ => null, + }; + + private static CertificationTrustPolicy Policy(string publicKeyBase64) => + CertificationTrustPolicy.FromConfiguration(Config(publicKeyBase64)); + + private FileCertificationRecordStore Store(CertificationTrustPolicy policy) => + new(_directory, new CertificationRecordSigner(hmacKey: StoreHmacKey), policy); + + /// + /// Writes a record for a fresh brick id, signed by , and + /// returns that id. The store's own Save does the writing, so the bytes under test are + /// the bytes production writes. + /// + private string SaveRecordSignedBy(string privateKeyBase64) + { + var brickId = "pinned-probe-" + Guid.NewGuid().ToString("N")[..8]; + var signed = new CertificationRecordSigner( + hmacKey: StoreHmacKey, + ed25519PrivateKeyBase64: privateKeyBase64) + .SignRecord(UnsignedRecord(brickId)); + Store(CertificationTrustPolicy.Unpinned).Save(signed); + return brickId; + } + + /// + /// A record that satisfies every Strict requirement except the pinning set: trust-loop + /// schema, admitted PASS, content bound to , and both compile-authority + /// inputs. Pinning is then the only thing that can decide it. + /// + private static CertificationRecord UnsignedRecord(string brickId) + { + var hash = BrickContentHasher.ComputeSha256(BrickSource); + return new CertificationRecord + { + Status = "PASS", + Stage = "admit", + Admitted = true, + Signed = true, + Timestamp = DateTimeOffset.UtcNow, + BrickId = brickId, + ContentHash = hash, + SchemaVersion = CertificationRecordData.TrustLoopSchemaVersion, + Inputs = new[] + { + new CertificationInput + { + Kind = CertificationInputKinds.GateEmittedArtifact, + Id = brickId, + Hash = hash, + }, + CertifierIdentity.ToInput(), + }, + }; + } + + private static (string PrivateKeyBase64, string PublicKeyBase64) CreateEd25519Key() + { + using var key = Key.Create( + SignatureAlgorithm.Ed25519, + new KeyCreationParameters { ExportPolicy = KeyExportPolicies.AllowPlaintextExport }); + return ( + Convert.ToBase64String(key.Export(KeyBlobFormat.RawPrivateKey)), + Convert.ToBase64String(key.PublicKey.Export(KeyBlobFormat.RawPublicKey))); + } +} From cdbec619fb914212b1c4f58955bb613e7f12cd9b Mon Sep 17 00:00:00 2001 From: t Date: Sun, 27 Sep 2026 20:57:34 -0400 Subject: [PATCH 2/3] fix(certification): a blank pinning switch is not permission to run unpinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParsePinningRequired read a present-but-EMPTY ASHLAR_CERT_PINNING_REQUIRED as "pinning not required", which is the one failure that variable exists to catch. A deployment template rendering `ASHLAR_CERT_PINNING_REQUIRED=` with nothing substituted into it — a k8s configMap empty value, `export VAR=$UNSET`, a YAML empty string, an IConfiguration key present with no value — therefore started a host UNPINNED while the operator believed they had demanded pinning, with no error anywhere. The type's own XML doc and the exception message both promised the opposite, and the sibling keys variable throws for the identical input. Blank now falls through to the throw; `configured is null` already distinguishes absent ("no assertion made") from blank ("an assertion that arrived empty"). Three further things that were documented and not true: - Apply's summary promises configuration will not WIDEN a pinning set chosen in code. SameKeys compared counts and then checked one direction only, so basis {A, A} against configuration {A, B} reported "same set", raised no conflict, and returned options accepting B. It is set equality on de-duplicated sides now. - Validation decoded to check the length and then stored the operator's STRING, while enforcement compares ordinally against Convert.ToBase64String of the raw key. A 32-byte key has four valid 44-character encodings, so an operator could configure a value that decodes to exactly the right key and match no record at all — fail-closed, but a host refusing every honest record while reporting itself correctly pinned. The stored set is canonical now. - Validate SKIPPED a blank entry, two lines above a message refusing "a pinning set that dropped an entry". FromTrustedKeys passes a caller's bundle straight in, so a bundle with a blank line silently yielded a narrower set. It refuses. And the tests that were not testing the thing. The operator-facing contract was tautological: every fact reached FromConfiguration through a helper keyed on TrustedKeysVariable itself, so arrange and act read the same symbol and neither Ambient nor FromEnvironment — the only paths any production consumer uses — was executed at all. One fact now sets the REAL variables and calls FromEnvironment (not Ambient, which caches per process; it resolves Ambient first so no other collection can resolve it inside the write window). FromTrustedKeys had no test. Apply's hand-copy was unfalsifiable in the dangerous direction: every fact applied Strict and compared back to Strict, so hardcoding Strict's three values inside Apply stayed green while every pinned host asking for Default or Legacy would silently have been handed Strict; a configured policy's Default and Apply(Legacy) are now checked against their own presets. Five of the seven wirings were unfalsifiable at all, because with nothing configured Ambient.Strict IS the Strict instance; CertifiedBrickHotSwapHost (verify-at-load, the path a hot-swap attacker reaches) and CertifiedBrickRegistry now have facts that redden if either reverts to the bare preset. CompositionConstituentChecker, StateLogVerifier and the adaptation DI factory are still unverified, and still invisible to any mutation. Docs: ASHLAR_CERT_TRUSTED_ED25519_KEYS and ASHLAR_CERT_PINNING_REQUIRED get rows in Configuration.md, and certification-evidence.md stops saying pinning "needs the trust root that SPEC-006 §3 defers". It also now says what remains true: nothing in this repository or in CI sets either variable, so every deployment that has not configured one still verifies unpinned, and the packable contracts library takes trust input from ambient process environment its consumers did not opt into. VERIFIED in the devtest container. Filtered to the two touched classes (net8.0, Ashlar.Tests.Infrastructure): baseline before the change Failed 0, Passed 15, Total 15; after Failed 0, Passed 26, Total 26. Full cert-gate filter on the committed tree: Failed 0, Passed 1234, Skipped 0, Total 1234. The packable library builds clean on all three TFMs (netstandard2.0, net8.0, net10.0) under TreatWarningsAsErrors. Mutations, in two disjoint groups: - restore `value.Length == 0` + FromEnvironment -> FromConfiguration(_ => null) + FromTrustedKeys -> Unpinned: Failed 5, Passed 21. The three new blank switch cases reddened and "treu" did not; the environment fact and the bundle fact reddened; nothing else moved. - Apply ignores its basis for all three copied values + hot-swap host reverted to the bare preset + old SameKeys + store the raw candidate: Failed 4, Passed 22 — one failure per fact, and Pinning_adds_to_strictness_and_relaxes _nothing stayed green under the Apply mutation, which is the whole point. Co-Authored-By: Claude Opus 5 --- docs/Configuration.md | 4 +- docs/certification-evidence.md | 16 +- .../CertificationTrustPolicy.cs | 87 +++-- .../PinnedHotSwapVerifyAtLoadTests.cs | 174 +++++++++ .../TrustedKeyPinningConfigurationTests.cs | 335 +++++++++++++++++- 5 files changed, 579 insertions(+), 37 deletions(-) create mode 100644 src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedHotSwapVerifyAtLoadTests.cs diff --git a/docs/Configuration.md b/docs/Configuration.md index 417134a0e..bbe92c936 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -441,7 +441,9 @@ Bound from the `Ashlar:Autonomy` section by `AddAshlarAutonomy(configuration)` | Variable | Description | Default | |----------|-------------|---------| | `ASHLAR_CERT_DEV_HMAC_KEY` | HMAC key for signing **and verifying** brick and composition certification records (`CertificationRecordSigner`, `CompositionCertificationRecordSigner`, `Ashlar.Certification.Contracts.CertificationRecordSigning`). **Unset means the COMMITTED, PUBLIC dev key** `CertificationRecordSigning.DefaultDevKey`: anyone with the source can forge a record that verifies, so certificates then prove integrity against accident, not against an adversary. Both signers log a warning at construction while the dev key is in effect (`UsesDevKey`). Same-owner cross-project reuse works by sharing this value; cross-organization trust needs the Ed25519 key below or PKI | unset (dev key; warns) | -| `ASHLAR_CERT_ED25519_KEY` | Base64 Ed25519 private key; when set, records are dual-signed and carry the public key. **`CertificationVerifyOptions.Default` and `.Strict` require Ed25519 signatures** (`RequireEd25519Signature = true`) and trust-loop schema v2+ (`MinimumSchemaVersion = 2`), fail-closed; only `.Legacy` accepts HMAC-only records (see limitations 7–8 **CLOSED** 2026-09-06 in `docs/certification-evidence.md`). **Trusted-key pinning exists** via `TrustedEd25519PublicKeys` but is optional; **without pinning, a self-consistent record signed with an attacker's own keypair still verifies** (limitation 1: dev HMAC under committed key remains forgeable). Limitation 9 is CLOSED as of 2026-09-13: a `CertificationRecordSigner` holding a real key now keys the composition lane too, through the shipped DI registration. Production paths use Strict | unset (HMAC-only) | +| `ASHLAR_CERT_ED25519_KEY` | Base64 Ed25519 private key; when set, records are dual-signed and carry the public key. **`CertificationVerifyOptions.Default` and `.Strict` require Ed25519 signatures** (`RequireEd25519Signature = true`) and trust-loop schema v2+ (`MinimumSchemaVersion = 2`), fail-closed; only `.Legacy` accepts HMAC-only records (see limitations 7–8 **CLOSED** 2026-09-06 in `docs/certification-evidence.md`). **Trusted-key pinning is now configurable** — see `ASHLAR_CERT_TRUSTED_ED25519_KEYS` below — but it is still OPT-IN: **with no pinning set configured, a self-consistent record signed with an attacker's own keypair still verifies** (limitation 1: dev HMAC under committed key remains forgeable). Limitation 9 is CLOSED as of 2026-09-13: a `CertificationRecordSigner` holding a real key now keys the composition lane too, through the shipped DI registration. Production paths use Strict | unset (HMAC-only) | +| `ASHLAR_CERT_TRUSTED_ED25519_KEYS` | Base64 raw 32-byte Ed25519 **PUBLIC** keys this host accepts as record signers, separated by commas, semicolons or whitespace (newlines included, so the list can be mounted as a multi-line secret). This is the switch that makes "signed" mean "signed by someone we accept": `CertificationRecordEd25519.VerifySignature` checks the signature against the public key the RECORD carries, so requiring a signature without pinning only forces an attacker to sign rather than strip. Read once per process by `CertificationTrustPolicy.Ambient`, which the record store, the certified brick registry, the hot-swap host, the composition constituent checker, the self-extend admission edge, the state-log verifier and the adaptation wiring all apply in place of the bare `Strict` preset. **There is deliberately no configuration path here for a private key** — that is the minting capability and belongs to the certifier. Present-but-unusable configuration (no keys, a blank entry, a non-Base64 value, a key that is not 32 raw bytes) throws `CertificationTrustConfigurationException` at resolution rather than degrading to an empty set, because an empty set reads as pinning OFF everywhere downstream. Keys are normalized to their canonical Base64 encoding, since enforcement is an ordinal string comparison against what the record carries | unset (**unpinned**: any keypair verifies) | +| `ASHLAR_CERT_PINNING_REQUIRED` | Asserts that pinning must be in effect, so a deployment that expected it fails at startup instead of verifying unpinned for its lifetime. Accepts `1/true/yes/on` and `0/false/no/off`, case-insensitively. When it asserts pinning and `ASHLAR_CERT_TRUSTED_ED25519_KEYS` supplies no key, resolution throws. **A value that is neither — a typo, or a deployment template that rendered the variable with nothing substituted into it — also throws**, rather than being read as "not required": the switch that demands pinning must not be the thing that silently disables it. Only an ABSENT variable means "no assertion made" | unset (no assertion) | ## Workload scaling (`Ashlar:WorkloadScaling:*`, `ASHLAR_WORKLOAD_*`) diff --git a/docs/certification-evidence.md b/docs/certification-evidence.md index 2f52bfe2f..487899257 100644 --- a/docs/certification-evidence.md +++ b/docs/certification-evidence.md @@ -642,7 +642,7 @@ redundant guard is exactly what a careful proposer writes. ## Known v0 limitations -1. **Dev HMAC signer, not PKI.** `CertificationRecordSigner` uses a development HMAC key, not a public-key infrastructure. This becomes more load-bearing in the composition phase because trust chains from constituent atom signatures — a forged or weak constituent record undermines the whole composition admission path. Unless `ASHLAR_CERT_DEV_HMAC_KEY` is set, the key is the COMMITTED constant `CertificationRecordSigning.DefaultDevKey`, so every record verifiable here is forgeable by anyone with the source; both signers now warn at construction while that is the case (`UsesDevKey`), and `ASHLAR_CERT_ED25519_KEY` adds a real signature on top. **Ed25519 is now required on Default/Strict verification paths** (limitations 7–8 CLOSED as of 2026-09-06), but it is still not a complete operator trust root on its own: without `TrustedEd25519PublicKeys` pinning, a self-consistent attacker-signed record can verify; HMAC remains forgeable under the committed dev key (this row); and compositions ARE keyed by the operator as of 2026-09-13 (limitation 9 CLOSED — the composition signer takes the injected brick signer as its key holder and the shipped DI registration supplies it, so a host key reaches both lanes; see the CLOSING NOTE below). What remains in this row is the committed dev key itself, not composition reachability. +1. **Dev HMAC signer, not PKI.** `CertificationRecordSigner` uses a development HMAC key, not a public-key infrastructure. This becomes more load-bearing in the composition phase because trust chains from constituent atom signatures — a forged or weak constituent record undermines the whole composition admission path. Unless `ASHLAR_CERT_DEV_HMAC_KEY` is set, the key is the COMMITTED constant `CertificationRecordSigning.DefaultDevKey`, so every record verifiable here is forgeable by anyone with the source; both signers now warn at construction while that is the case (`UsesDevKey`), and `ASHLAR_CERT_ED25519_KEY` adds a real signature on top. **Ed25519 is now required on Default/Strict verification paths** (limitations 7–8 CLOSED as of 2026-09-06), but it is still not a complete operator trust root on its own: without `TrustedEd25519PublicKeys` pinning, a self-consistent attacker-signed record can verify — and pinning is now CONFIGURABLE (`ASHLAR_CERT_TRUSTED_ED25519_KEYS`, see `docs/Configuration.md`) but still opt-in, and nothing in this repository or in CI sets it; HMAC remains forgeable under the committed dev key (this row); and compositions ARE keyed by the operator as of 2026-09-13 (limitation 9 CLOSED — the composition signer takes the injected brick signer as its key holder and the shipped DI registration supplies it, so a host key reaches both lanes; see the CLOSING NOTE below). What remains in this row is the committed dev key itself, not composition reachability. 2. **Composition seam check is TYPE-level only.** The seam validator checks producer/consumer type compatibility (e.g. `string` vs `int`) but not semantic mismatches where types align (e.g. file path vs URL, both `string`). Graph-mutation teeth only partially compensate for this gap. @@ -746,6 +746,20 @@ redundant guard is exactly what a careful proposer writes. two-operator-identity split — **resolved 2026-08-27 to a single identity**, see `_handoff/readiness/DECISION-identity-split.md`). + *Update: the trust root is now the operator's own setting, not a deferred file.* + `CertificationTrustPolicy` reads `ASHLAR_CERT_TRUSTED_ED25519_KEYS` (Base64 raw PUBLIC + keys; any configuration source, not only the environment) and hands the resulting + `CertificationVerifyOptions` to every production verifier, and + `ASHLAR_CERT_PINNING_REQUIRED` makes "pinning is in effect" a checkable startup + assertion rather than a hope. This is a THIRD location for trust material, chosen over + `~/.ashlar/keys/trusted/` precisely because of the enumeration hazard below, and it + lives in a PACKABLE library (`Ashlar.Certification.Contracts`), so an external consumer + of that package has its trust decisions influenced by ambient process environment it + did not opt into — stated here because it is the kind of thing that should not be + discovered by surprise. **Nothing in this repository or in CI sets either variable**, so + every deployment that has not configured one still verifies unpinned: this row is + downgraded from an open hole to a *configurable* one, not closed. + **Pin against `operator.pub` only.** Do *not* enumerate `~/.ashlar/keys/trusted/`: `OperatorKey.Generate(rotate: true)` writes the *previous* public key into it (`src/Ashlar.Manifest/Signing/OperatorKey.cs:45-52`) and there is no revocation (`:28`), diff --git a/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs b/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs index 6ea0ebd68..9104ce418 100644 --- a/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs +++ b/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs @@ -50,7 +50,8 @@ public CertificationTrustConfigurationException(string message, Exception innerE /// The failure directions are not symmetric. Configuration that is absent leaves the /// basis preset untouched: the host runs as it did before pinning was configurable, which is a /// weaker posture the operator already had. Configuration that is PRESENT but unusable — no keys -/// in it, a value that is not Base64, a key that is not 32 bytes — throws +/// in it, a blank entry, a value that is not Base64, a key that is not 32 bytes, a pinning switch +/// this code cannot read (blank included) — throws /// at resolution, before any record is /// verified. An operator who wants "pinning must be in effect" to be checkable sets /// , and then an absent or empty key set is a startup failure @@ -70,7 +71,10 @@ public sealed class CertificationTrustPolicy /// and 0/false/no/off, case-insensitively. When it asserts pinning and /// yields no keys, resolution throws rather than running /// unpinned. A value that is neither also throws: reading an unrecognized value as "not - /// required" would let a typo silently switch pinning off. + /// required" would let a typo silently switch pinning off. Present but blank throws too + /// — a deployment template that rendered this variable without substituting a value has said + /// nothing, and must not be what decides pinning is optional. Only an ABSENT variable means + /// "no assertion made". /// public const string PinningRequiredVariable = "ASHLAR_CERT_PINNING_REQUIRED"; @@ -112,6 +116,9 @@ private CertificationTrustPolicy(string[]? trustedKeys) /// The trusted signer set, or null when the operator configured none. Never empty: an empty /// set would read as "pinning off" at every call site that tests /// , so it is refused at parse time. + /// Each key is the CANONICAL Convert.ToBase64String encoding of the 32 raw bytes, not + /// necessarily the string the operator wrote, because that is the encoding a record carries + /// and enforcement is an ordinal string comparison. /// public IReadOnlyCollection? TrustedEd25519PublicKeys { get; } @@ -175,7 +182,9 @@ public static CertificationTrustPolicy FromConfiguration(Func r /// /// Builds a policy from an explicit set of Base64 raw Ed25519 public keys, for a host that /// already holds them (a mounted trust bundle, a parsed manifest) rather than a raw setting. - /// The same validation applies: an empty or malformed set throws. + /// The same validation applies: an empty or malformed set throws, and so does a blank entry — + /// a bundle read line-by-line must have its blank lines removed by the caller rather than + /// silently yielding a narrower pinning set than the bundle lists. /// /// Base64 raw 32-byte Ed25519 public keys. public static CertificationTrustPolicy FromTrustedKeys(IEnumerable trustedEd25519PublicKeys) @@ -248,7 +257,21 @@ private static string[] Validate(IEnumerable entries, string settingName { var candidate = entry?.Trim(); if (string.IsNullOrEmpty(candidate)) - continue; + { + // Skipping it would hand back a set narrower than the one the caller listed, + // which is the failure the Base64 message below refuses by name. FromTrustedKeys + // passes a caller's bundle straight in, and a bundle with a blank line is the + // ordinary case. It is reachable from ParseTrustedKeys too, though rarer: + // RemoveEmptyEntries drops the separators in KeySeparators, but whitespace + // outside that set — a vertical tab, a form feed, a non-breaking space — arrives + // as an entry that Trim() empties. + throw new CertificationTrustConfigurationException( + $"{settingName} contains a blank entry. Refusing rather than skipping it: a " + + "pinning set that silently loses an entry is not the set that was " + + "configured, and it refuses honest records signed by the key that went " + + "missing. Remove the blank entry, or unset the whole setting to run " + + "unpinned deliberately."); + } byte[] bytes; try @@ -273,8 +296,17 @@ private static string[] Validate(IEnumerable entries, string settingName + "can ever match."); } - if (!keys.Contains(candidate!, StringComparer.Ordinal)) - keys.Add(candidate!); + // Store the CANONICAL encoding, not the operator's. Validation decodes; enforcement + // compares strings ordinally against `record.Ed25519PublicKey`, which is always + // `Convert.ToBase64String` of the raw key. A 32-byte key has four valid 44-character + // encodings, because the 43rd character carries two bits the decoder discards — so an + // operator can configure a value that decodes to exactly the right key and then match + // no record at all, with the host reporting itself correctly pinned. Normalizing here + // is also what makes the de-duplication below real: two encodings of one key would + // otherwise count as two keys. + var canonical = Convert.ToBase64String(bytes); + if (!keys.Contains(canonical, StringComparer.Ordinal)) + keys.Add(canonical); } if (keys.Count == 0) @@ -293,30 +325,39 @@ private static bool ParsePinningRequired(string? configured) if (configured is null) return false; + // Blank is NOT falsy. It is the commonest configuration accident there is — a deployment + // template rendering `ASHLAR_CERT_PINNING_REQUIRED=` with nothing substituted into it, an + // `export VAR=$UNSET`, a YAML empty string, an IConfiguration key present with no value — + // and reading it as "pinning not required" would start the host unpinned while the + // operator believed they had demanded pinning. That is the one failure this switch exists + // to catch, so it falls through to the throw below. Absent is handled above, by the null + // check: absent means "no assertion made", blank means "an assertion that arrived empty". + // The sibling keys variable already refuses the identical input. var value = configured.Trim(); - if (value.Length == 0 || FalsyValues.Contains(value, StringComparer.OrdinalIgnoreCase)) + if (FalsyValues.Contains(value, StringComparer.OrdinalIgnoreCase)) return false; if (TruthyValues.Contains(value, StringComparer.OrdinalIgnoreCase)) return true; throw new CertificationTrustConfigurationException( - $"{PinningRequiredVariable} is set to a value that is neither true nor false. It is " - + "refused rather than read as false, because a typo in a switch that demands pinning " - + "must not be the thing that silently disables it."); + $"{PinningRequiredVariable} is set to a value that is neither true nor false" + + (value.Length == 0 ? " (it is present but blank)" : $" ('{value}')") + + ". It is refused rather than read as false, because a typo — or a deployment " + + "template that rendered the variable without substituting a value — in a switch " + + "that demands pinning must not be the thing that silently disables it. Unset " + + $"{PinningRequiredVariable} to run unpinned deliberately."); } - private static bool SameKeys(IReadOnlyCollection left, IReadOnlyCollection right) - { - if (left.Count != right.Count) - return false; - - foreach (var key in left) - { - if (!right.Contains(key, StringComparer.Ordinal)) - return false; - } - - return true; - } + /// + /// Set equality, de-duplicated on BOTH sides. The de-duplication is the whole point: counting + /// and then checking one direction reports "same set" for a basis of {A, A} against a + /// configuration of {A, B} — matching counts, every left key present — and + /// would return options accepting B, which is exactly the widening its + /// summary promises never happens. de-duplicates the configured side, + /// but comes from a caller-supplied options object and nothing + /// de-duplicates that. + /// + private static bool SameKeys(IReadOnlyCollection left, IReadOnlyCollection right) => + new HashSet(left, StringComparer.Ordinal).SetEquals(right); } diff --git a/src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedHotSwapVerifyAtLoadTests.cs b/src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedHotSwapVerifyAtLoadTests.cs new file mode 100644 index 000000000..896470a84 --- /dev/null +++ b/src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedHotSwapVerifyAtLoadTests.cs @@ -0,0 +1,174 @@ +using Ashlar.Certification.Contracts; +using Ashlar.Infrastructure.Certification; +using Ashlar.Infrastructure.Certification.HotSwap; +using FluentAssertions; +using NSec.Cryptography; +using Xunit; + +namespace Ashlar.Tests.Infrastructure.Tests.Certification; + +/// +/// The hot-swap host's verify-at-load path under a configured pinning set. +/// +/// Why this fact exists separately. Seven production sites now apply +/// CertificationTrustPolicy.Ambient.Strict in place of the bare Strict preset, and +/// five of them were unfalsifiable: with nothing configured Ambient.Strict IS the +/// CertificationVerifyOptions.Strict instance, reference-identically, so reverting a call +/// site to the preset is invisible to every test that does not configure a policy. A reviewer +/// reverted all five at once and the suite stayed green. CertifiedBrickHotSwapHost is the +/// largest of the five and the one a hot-swap attacker actually reaches: it is the path that +/// decides whether freshly supplied bytes get loaded into the running process. +/// +/// Joins hot-swap-host like every other class that commits a generation: a committed +/// swap creates a real AssemblyLoadContext named BrickGeneration_*, and sibling +/// classes assert on the set of those contexts. +/// +[Trait("Category", "Certification")] +[Collection("hot-swap-host")] +public sealed class PinnedHotSwapVerifyAtLoadTests +{ + private const string HmacKey = "pinned-hot-swap-test-hmac"; + private const string ProbeBrickId = "pinned-hot-swap-probe"; + + private const string ProbeBrickSource = """ +using Ashlar.Core.Domain.Bricks; +using Ashlar.Core.Domain.Execution; + +namespace Ashlar.Tests.PinnedHotSwapProbe; + +public sealed class PinnedHotSwapProbeBrick : DomainBrick +{ + public PinnedHotSwapProbeBrick() + { + Id = "pinned-hot-swap-probe"; + Name = "Pinned Hot Swap Probe"; + Description = "Marker brick for pinned verify-at-load tests."; + } + + public override Task ExecuteAsync( + BrickInput input, + ImplementationType implementation, + IExecutionContext context, + CancellationToken cancellationToken = default) + { + var output = new BrickOutput { Summary = "pinned" }; + output.Set("marker", "pinned"); + return Task.FromResult(output); + } +} +"""; + + /// + /// A record signed by a key the operator did not pin is refused at load with the trust + /// verifier's own failure code, and the generation never commits. The positive control — the + /// SAME request against a host pinned to the key that actually minted the record — is what + /// makes the refusal load-bearing: this host refuses for a dozen other reasons, and a + /// mis-built request would produce a refusal that proves nothing about pinning. + /// + [Fact] + public async Task A_configured_pinning_set_makes_the_hot_swap_host_refuse_a_foreign_signer() + { + var ours = CreateEd25519Key(); + var theirs = CreateEd25519Key(); + var request = RequestSignedBy(theirs); + + using var pinnedElsewhere = HostPinnedTo(ours.PublicKeyBase64); + var refused = await pinnedElsewhere.SwapAsync(new[] { request }); + + refused.Swapped.Should().BeFalse( + "the certificate is signed by a key this operator did not pin, and verify-at-load " + + "checks the signature against the key the RECORD carries, so pinning is the only " + + "thing that can tell the two keys apart"); + refused.Refusals.Should().ContainSingle(r => + r.BrickId == ProbeBrickId + && r.Stage == BrickSwapRefusalStage.Verification + && r.FailureCode == "ed25519-key-not-trusted"); + pinnedElsewhere.CurrentBrickIds.Should().NotContain(ProbeBrickId); + + using var pinnedToTheSigner = HostPinnedTo(theirs.PublicKeyBase64); + var landed = await pinnedToTheSigner.SwapAsync(new[] { request }); + + landed.Refusals.Should().BeEmpty(Describe(landed)); + landed.Swapped.Should().BeTrue( + "pinned to the minting key, the identical request must still land — otherwise the " + + "refusal above could have come from anything this host checks"); + pinnedToTheSigner.CurrentBrickIds.Should().Contain(ProbeBrickId); + } + + /// + /// A host pinned to one signer. The policy comes from FromConfiguration rather than + /// FromTrustedKeys deliberately: the claim under test is the HOST's wiring, and a fact + /// about it should redden for a change to that wiring and for nothing else. + /// + private static CertifiedBrickHotSwapHost HostPinnedTo(string publicKeyBase64) => + new( + hmacKey: HmacKey, + drainTimeout: TimeSpan.FromSeconds(10), + trustPolicy: CertificationTrustPolicy.FromConfiguration( + name => name == CertificationTrustPolicy.TrustedKeysVariable ? publicKeyBase64 : null)); + + /// + /// A load request whose certificate is minted by , satisfying every + /// other Strict clause — trust-loop schema, admitted PASS, content hash over the exact + /// source, gate-emitted artifact over the exact PE, certifier identity — so the pinning set is + /// the only free variable. + /// + private static CertifiedBrickLoadRequest RequestSignedBy( + (string PrivateKeyBase64, string PublicKeyBase64) signer) + { + var pe = GateEmittedArtifactCompiler.Compile( + ProbeBrickSource, BrickCertificationProjectLoader.DefaultCompilationReferences()); + + var unsigned = new CertificationRecordData + { + Status = "PASS", + Stage = "S0-S2", + Admitted = true, + Signed = true, + Timestamp = DateTimeOffset.UtcNow, + BrickId = ProbeBrickId, + ContentHash = BrickContentHasher.ComputeSha256(ProbeBrickSource), + Gate = "pinned-hot-swap-test-harness", + SchemaVersion = CertificationRecordData.TrustLoopSchemaVersion, + Inputs = + [ + new CertificationInput + { + Kind = CertificationInputKinds.GateEmittedArtifact, + Id = pe.BrickTypeName, + Hash = pe.AssemblySha256, + }, + CertifierIdentity.ToInput(), + ], + Ed25519PublicKey = signer.PublicKeyBase64, + }; + + return new CertifiedBrickLoadRequest + { + BrickId = ProbeBrickId, + SourceCode = ProbeBrickSource, + Record = unsigned with + { + Signature = CertificationRecordSigning.Sign(unsigned, HmacKey), + Ed25519Signature = CertificationRecordEd25519.Sign( + unsigned, Convert.FromBase64String(signer.PrivateKeyBase64)), + }, + PrecompiledAssembly = pe.AssemblyBytes, + }; + } + + private static string Describe(CertifiedBrickSwapResult result) => + result.Swapped + ? "swapped" + : string.Join("; ", result.Refusals.Select(r => $"{r.BrickId} {r.Stage} {r.FailureCode}: {r.Reason}")); + + private static (string PrivateKeyBase64, string PublicKeyBase64) CreateEd25519Key() + { + using var key = Key.Create( + SignatureAlgorithm.Ed25519, + new KeyCreationParameters { ExportPolicy = KeyExportPolicies.AllowPlaintextExport }); + return ( + Convert.ToBase64String(key.Export(KeyBlobFormat.RawPrivateKey)), + Convert.ToBase64String(key.PublicKey.Export(KeyBlobFormat.RawPublicKey))); + } +} diff --git a/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs b/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs index 4c006679b..5edfd1a16 100644 --- a/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs +++ b/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs @@ -5,6 +5,7 @@ using Ashlar.Certification.Contracts; using Ashlar.Core.Application.Certification.Models; using Ashlar.Infrastructure.Certification; +using Ashlar.Tests.Infrastructure.Certification.Fixtures; using FluentAssertions; using NSec.Cryptography; using Xunit; @@ -28,11 +29,16 @@ namespace Ashlar.Tests.Infrastructure.Tests.Certification; /// attempt to pin into the weakest posture available, exactly when they believed they had the /// strongest. /// -/// Joins the non-parallel EnvironmentVariables collection. This class writes no -/// environment variable, but the self-extend edge verifies with no explicit HMAC key, so it READS -/// ASHLAR_CERT_DEV_HMAC_KEY through CertificationRecordSigning; a class that rewrote -/// that variable beside it would invalidate signatures minted here between the mint and the -/// verification. +/// Joins the non-parallel EnvironmentVariables collection, for two reasons now. +/// +/// WRITES ASHLAR_CERT_TRUSTED_ED25519_KEYS and ASHLAR_CERT_PINNING_REQUIRED, which +/// are process-global; and the self-extend edge verifies with no explicit HMAC key, so it READS +/// ASHLAR_CERT_DEV_HMAC_KEY through CertificationRecordSigning, and a class that +/// rewrote that variable beside it would invalidate signatures minted here between the mint and +/// the verification. The collection serializes the classes in it; what it cannot serialize is a +/// class in another collection resolving CertificationTrustPolicy.Ambient for the first +/// time inside the write window, so that test resolves Ambient itself before writing +/// anything. /// [Trait("Category", "Certification")] [Collection("EnvironmentVariables")] @@ -193,8 +199,7 @@ public void Configuration_that_cannot_be_a_key_is_refused(string configured) /// /// Fact (b), the operator's own assertion: a host told that pinning must be in effect refuses to - /// start without keys, and a switch value it cannot read is refused rather than taken as "not - /// required" — a typo must not be the thing that disables pinning. + /// start without keys. /// [Fact] public void Requiring_pinning_without_keys_refuses_to_start() @@ -203,11 +208,6 @@ public void Requiring_pinning_without_keys_refuses_to_start() missingKeys.Should().Throw() .WithMessage($"*{CertificationTrustPolicy.TrustedKeysVariable}*"); - var typo = () => CertificationTrustPolicy.FromConfiguration( - Config(CreateEd25519Key().PublicKeyBase64, "treu")); - typo.Should().Throw( - "an unrecognized value read as false would silently switch pinning off"); - CertificationTrustPolicy .FromConfiguration(Config(CreateEd25519Key().PublicKeyBase64, "yes")) .PinningConfigured.Should().BeTrue(); @@ -216,6 +216,134 @@ public void Requiring_pinning_without_keys_refuses_to_start() .PinningConfigured.Should().BeTrue("keys configure pinning whether or not it is demanded"); } + /// + /// Fact (b), the switch itself: a value this code cannot read refuses to start rather than + /// being taken as "not required". + /// + /// Blank is the case that matters, and it was a fail-open. + /// ParsePinningRequired treated a present-but-empty value as falsy, so a deployment + /// template that rendered ASHLAR_CERT_PINNING_REQUIRED= with nothing substituted into + /// it — a Kubernetes configMap empty value, export VAR=$UNSET, a YAML empty string, an + /// IConfiguration key present with no value — started the host UNPINNED while the + /// operator believed they had demanded pinning, which is the one failure this variable exists + /// to catch. The sibling keys variable throws for the identical input. Keys are supplied in + /// every case here, so the only thing that can throw is the switch — and the message + /// assertion names the switch's own variable, so the keys refusal cannot satisfy it. + /// + [Theory] + [InlineData("treu")] + [InlineData("")] + [InlineData(" ")] + [InlineData("\t")] + public void A_pinning_switch_value_that_cannot_be_read_refuses_to_start(string configured) + { + var keys = CreateEd25519Key().PublicKeyBase64; + + var resolve = () => CertificationTrustPolicy.FromConfiguration(Config(keys, configured)); + + resolve.Should().Throw( + "a value read as false would silently switch pinning off, and the operator would " + + "see no error at all") + .WithMessage($"*{CertificationTrustPolicy.PinningRequiredVariable}*"); + } + + /// + /// The path production actually uses, executed rather than described. + /// + /// Every wiring site resolves , whose + /// factory is . Every OTHER fact in this + /// class reaches FromConfiguration through , whose keys are + /// CertificationTrustPolicy.TrustedKeysVariable itself — so arrange and act read the + /// same symbol, and changing that constant's VALUE, or breaking the + /// Environment.GetEnvironmentVariable call, leaves them all green while no operator can + /// turn pinning on. This one sets the REAL variables. + /// + /// FromEnvironment and not Ambient: Ambient is a + /// resolved once per process, so it would answer whatever the first + /// caller anywhere in the run happened to see. For the same reason this test TOUCHES + /// Ambient before it writes anything — every store, registry and host built anywhere in + /// this assembly resolves it on construction, and a first resolution landing inside this + /// test's window would pin the whole process to a key only this test knows. + /// + [Fact] + public void The_environment_variables_an_operator_sets_are_the_ones_that_turn_pinning_on() + { + _ = CertificationTrustPolicy.Ambient; + + var key = CreateEd25519Key().PublicKeyBase64; + var keysBefore = Environment.GetEnvironmentVariable(CertificationTrustPolicy.TrustedKeysVariable); + var requiredBefore = Environment.GetEnvironmentVariable(CertificationTrustPolicy.PinningRequiredVariable); + try + { + Environment.SetEnvironmentVariable(CertificationTrustPolicy.TrustedKeysVariable, key); + Environment.SetEnvironmentVariable(CertificationTrustPolicy.PinningRequiredVariable, "true"); + + var configured = CertificationTrustPolicy.FromEnvironment(); + + configured.PinningConfigured.Should().BeTrue( + "this is the only path any production consumer uses: all seven wiring sites fall " + + "through to Ambient, whose factory is FromEnvironment"); + configured.TrustedEd25519PublicKeys.Should().BeEquivalentTo(new[] { key }); + configured.Strict.PinningEnabled.Should().BeTrue(); + + Store(configured).Get(SaveRecordSignedBy(CreateEd25519Key().PrivateKeyBase64)) + .Should().BeNull("a signer absent from the environment's list is not trusted"); + + Environment.SetEnvironmentVariable(CertificationTrustPolicy.TrustedKeysVariable, null); + var demandedWithoutKeys = () => CertificationTrustPolicy.FromEnvironment(); + demandedWithoutKeys.Should().Throw( + "the operator's assertion is checkable through the environment too, or it is not " + + "checkable at all"); + + Environment.SetEnvironmentVariable(CertificationTrustPolicy.PinningRequiredVariable, null); + CertificationTrustPolicy.FromEnvironment().Strict + .Should().BeSameAs(CertificationVerifyOptions.Strict, + "with neither variable set, the environment path leaves the preset alone"); + } + finally + { + Environment.SetEnvironmentVariable(CertificationTrustPolicy.TrustedKeysVariable, keysBefore); + Environment.SetEnvironmentVariable(CertificationTrustPolicy.PinningRequiredVariable, requiredBefore); + } + } + + /// + /// — new public surface on a security + /// type, documented as the path for "a mounted trust bundle, a parsed manifest", and until now + /// with no test at all. It pins like a setting does, and it refuses the entries a setting + /// refuses, including the blank line a bundle read line-by-line will contain. + /// + [Fact] + public void A_trust_bundle_handed_in_directly_pins_the_way_a_setting_does() + { + var first = CreateEd25519Key(); + var second = CreateEd25519Key(); + var stranger = CreateEd25519Key(); + + var policy = CertificationTrustPolicy.FromTrustedKeys( + new[] { first.PublicKeyBase64, second.PublicKeyBase64 }); + + policy.PinningConfigured.Should().BeTrue(); + var store = Store(policy); + store.Get(SaveRecordSignedBy(first.PrivateKeyBase64)).Should().NotBeNull(); + store.Get(SaveRecordSignedBy(second.PrivateKeyBase64)).Should().NotBeNull(); + store.Get(SaveRecordSignedBy(stranger.PrivateKeyBase64)).Should().BeNull( + "a bundle is a pinning set like any other, or it is decoration"); + + var withBlankLine = () => CertificationTrustPolicy.FromTrustedKeys( + new[] { first.PublicKeyBase64, " " }); + withBlankLine.Should().Throw( + "dropping it would return a set narrower than the bundle lists, which refuses honest " + + "records signed by the key that went missing"); + + var notAKey = () => CertificationTrustPolicy.FromTrustedKeys(new[] { "c2hvcnQ=" }); + notAKey.Should().Throw(); + + var nothingAtAll = () => CertificationTrustPolicy.FromTrustedKeys(Array.Empty()); + nothingAtAll.Should().Throw( + "an empty bundle reads as pinning off at every call site downstream"); + } + /// /// Absent configuration is not malformed configuration: it leaves every preset byte-for-byte /// where it was, so wiring this policy into the production verifiers changed nothing for a host @@ -269,6 +397,49 @@ public void Pinning_adds_to_strictness_and_relaxes_nothing() + "must be added there too, or every pinned host silently loses it"); } + /// + /// Apply must carry its ARGUMENT through, not a preset it was once handed. + /// + /// This is a hole a reviewer proved rather than argued: + /// applies Strict and + /// compares every value back to Strict, so hardcoding Strict's three copied + /// values inside Apply left the whole suite green — while every pinned host that asked + /// for Default or Legacy would silently have been handed Strict, which is + /// a strictness change nobody configured. A configured policy's Default is therefore + /// checked against the Default preset, and Apply(Legacy) against Legacy — + /// the two bases whose values DIFFER from Strict's. + /// + [Fact] + public void A_configured_policy_pins_each_basis_without_promoting_it_to_Strict() + { + var key = CreateEd25519Key().PublicKeyBase64; + var policy = Policy(key); + + var pinnedDefault = policy.Default; + pinnedDefault.PinningEnabled.Should().BeTrue(); + pinnedDefault.MinimumSchemaVersion.Should() + .Be(CertificationVerifyOptions.Default.MinimumSchemaVersion); + pinnedDefault.RequireGateEmittedArtifact.Should() + .Be(CertificationVerifyOptions.Default.RequireGateEmittedArtifact).And.BeFalse( + "Default demands no gate-emitted artifact and pinning adds none; if this turns " + + "true, Apply returned Strict's value in place of its basis's"); + pinnedDefault.RequireCertifierIdentity.Should() + .Be(CertificationVerifyOptions.Default.RequireCertifierIdentity).And.BeFalse( + "Default names no judge requirement either"); + + var pinnedLegacy = policy.Apply(CertificationVerifyOptions.Legacy); + pinnedLegacy.PinningEnabled.Should().BeTrue( + "a configured policy pins whatever basis it is given, Legacy included"); + pinnedLegacy.MinimumSchemaVersion.Should() + .Be(CertificationVerifyOptions.Legacy.MinimumSchemaVersion).And.Be(0, + "Legacy sets no schema floor, and configuration that says nothing about schema " + + "must not raise one"); + pinnedLegacy.RequireGateEmittedArtifact.Should().BeFalse(); + pinnedLegacy.RequireCertifierIdentity.Should().BeFalse(); + pinnedLegacy.RequireEd25519Signature.Should().BeTrue( + "the single value Apply sets on its own authority: an unsigned record cannot be pinned"); + } + /// /// Configuration may turn pinning on; it may not redirect a pinning set that code already chose, /// and merging the two would widen the accepted signer set beyond either. Applying the same set @@ -287,6 +458,146 @@ public void Applying_configuration_over_a_pinning_set_chosen_in_code_is_refused( Policy(mine).Apply(inCode).TrustedEd25519PublicKeys.Should().BeEquivalentTo(new[] { mine }); } + /// + /// The conflict check has to be SET equality on both sides, and a count plus one-directional + /// containment is not. + /// + /// Basis {A, A} against configuration {A, B}: the counts match and every + /// basis key is present, so the old check reported "same set", no conflict was raised, and + /// Apply returned options accepting B — a signer the basis did not accept. That is + /// precisely the widening Apply's own summary promises never happens, so the guarantee + /// was documented and false. The configured side is de-duplicated by Validate; the basis + /// side comes from a caller-supplied options object and nothing de-duplicates it. + /// + [Fact] + public void A_basis_that_repeats_one_key_is_not_the_same_set_as_one_that_adds_another() + { + var mine = CreateEd25519Key().PublicKeyBase64; + var yours = CreateEd25519Key().PublicKeyBase64; + var repeatedInCode = new CertificationVerifyOptions + { + TrustedEd25519PublicKeys = new[] { mine, mine }, + }; + + var widen = () => Policy($"{mine},{yours}").Apply(repeatedInCode); + widen.Should().Throw( + "the basis accepts one signer and the configuration two, so this is the widening the " + + "conflict check exists to refuse — equal counts are not equal sets"); + + Policy($"{mine} {mine}").Apply(repeatedInCode).TrustedEd25519PublicKeys + .Should().BeEquivalentTo( + new[] { mine }, + "one key on both sides, however often either side repeats it, is the same set and " + + "not a conflict — a count comparison would have refused this one instead"); + } + + /// + /// Validation decodes; enforcement compares strings. The two have to agree on the encoding, so + /// the stored set is canonical. + /// + /// A 32-byte key has four valid 44-character Base64 encodings, because the 43rd + /// character carries two bits the decoder discards. Storing the operator's string verbatim + /// therefore let a value that decodes to exactly the right key match no record at all: + /// record.Ed25519PublicKey is always Convert.ToBase64String of the raw key and + /// both verification tiers compare ordinally. Fail-CLOSED, so an outage rather than a hole — + /// but a host that refuses every honest record while reporting itself correctly pinned is a + /// nasty thing to debug. + /// + [Fact] + public void A_key_encoded_differently_but_decoding_the_same_still_matches_its_records() + { + var signer = CreateEd25519Key(); + var variant = SecondValidEncodingOf(signer.PublicKeyBase64); + + variant.Should().NotBeNull( + "this fact's premise is that a second valid encoding of the same 32 bytes exists; " + + "without one it asserts nothing, so the premise is checked rather than assumed"); + variant.Should().NotBe(signer.PublicKeyBase64, "the two STRINGS must differ"); + Convert.FromBase64String(variant!).Should().Equal( + Convert.FromBase64String(signer.PublicKeyBase64), "and the BYTES must not"); + + var policy = Policy(variant!); + + policy.TrustedEd25519PublicKeys.Should().BeEquivalentTo( + new[] { signer.PublicKeyBase64 }, + "the operator's encoding is normalized to the one a record carries"); + Store(policy).Get(SaveRecordSignedBy(signer.PrivateKeyBase64)).Should().NotBeNull( + "this is the key that minted the record; storing the operator's spelling verbatim " + + "would refuse it"); + } + + /// + /// Fact (a) on a third production consumer: CertifiedBrickRegistry's admission path. + /// Five of the seven wiring sites were unfalsifiable — with nothing configured + /// Ambient.Strict IS the Strict preset instance, so reverting a call site to the + /// bare preset is invisible to every test that does not configure a policy. This one and + /// PinnedHotSwapVerifyAtLoadTests take two of the five. + /// + [Fact] + public void A_configured_pinning_set_makes_the_certified_registry_refuse_a_foreign_signer() + { + var ours = CreateEd25519Key(); + var theirs = CreateEd25519Key(); + var brick = new MutationProbeBrick(); + var record = new CertificationRecordSigner( + hmacKey: StoreHmacKey, + ed25519PrivateKeyBase64: theirs.PrivateKeyBase64) + .SignRecord(UnsignedRecord(brick.Id)); + var verifier = new CertificationRecordSigner(hmacKey: StoreHmacKey); + + new CertifiedBrickRegistry( + new InMemoryCertificationRecordStore(), + verifier, + trustPolicy: Policy(ours.PublicKeyBase64)) + .TryAdmit(brick, record) + .Should().BeFalse( + "the record is signed by a key this operator did not pin, and without a pinning " + + "set the registry only asks whether the record is self-consistent"); + + new CertifiedBrickRegistry( + new InMemoryCertificationRecordStore(), + verifier, + trustPolicy: Policy(theirs.PublicKeyBase64)) + .TryAdmit(brick, record) + .Should().BeTrue( + "pinned to the key that minted it, the same record must still be admitted — " + + "otherwise the refusal above proves nothing about pinning"); + } + + /// + /// A second valid 44-character Base64 encoding of the same 32 bytes, or null when this + /// runtime's decoder rejects the spare bits — in which case the trap this guards cannot arise + /// and the caller should say so rather than pass quietly. Found by search over the alphabet + /// rather than by bit arithmetic, so the fact does not rest on my arithmetic. + /// + private static string? SecondValidEncodingOf(string canonical) + { + const string Alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + + if (canonical.Length != 44) + return null; + + var bytes = Convert.FromBase64String(canonical); + foreach (var replacement in Alphabet) + { + var candidate = canonical[..42] + replacement + "="; + if (string.Equals(candidate, canonical, StringComparison.Ordinal)) + continue; + + try + { + if (Convert.FromBase64String(candidate).AsSpan().SequenceEqual(bytes)) + return candidate; + } + catch (FormatException) + { + // Not a valid encoding at all; keep looking. + } + } + + return null; + } + private static Func Config(string? trustedKeys, string? pinningRequired = null) => name => name switch { From 65fdeab149b2839d17187f992a58594a0fa802ee Mon Sep 17 00:00:00 2001 From: t Date: Sun, 27 Sep 2026 22:33:54 -0400 Subject: [PATCH 3/3] fix(certification): pin the operator-facing variable names, and stop leaving wirings silently unverified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four things the tail review asked for. 1. A regression the SameKeys rewrite introduced. `new HashSet(left, StringComparer.Ordinal)` is not something a security type should be resting on when `left` — a caller-supplied basis — can contain a null entry; the loop it replaced tolerated one. `Apply` now refuses a null or whitespace basis entry with this type's own named refusal, before the set is built, for the same reason `Validate` refuses a blank configured entry. 2. The operator-facing variable NAMES were pinned by nothing. Every fact wrote and read through `TrustedKeysVariable` itself, so changing that constant's VALUE reddened zero of 26 cases while every operator following docs/Configuration.md would have set a variable nothing reads — pinning silently off. The names are now pinned to their literals AND to the document that tells an operator to set them. 3. Three of seven wiring sites were unfalsifiable, because with nothing configured `Ambient.Strict` is reference-identical to the `Strict` preset. `CompositionConstituentChecker` has the same policy seam the registry and the hot-swap host have, so it takes a behavioural fact. `StateLogVerifier`'s five-argument overload and the adaptation DI factory have no seam, and creating one is a public API change on a packable library — so each carries a comment at the line saying it is unverified and why, rather than letting a green gate imply coverage. `PinnedWiringSiteConventionTests` asserts on shipped IL that all seven still call `CertificationTrustPolicy.Ambient`, so deleting any of the wiring reddens even where enforcement cannot be measured. 4. certification-evidence.md records which of the seven are behaviourally covered and which are carried by the IL guard alone. Co-Authored-By: Claude Opus 5 --- docs/certification-evidence.md | 16 ++ .../CertificationTrustPolicy.cs | 44 +++++- .../StateLogVerifier.cs | 16 ++ .../AdaptationServiceCollectionExtensions.cs | 13 ++ .../CompositionConstituentChecker.cs | 4 + .../PinnedWiringSiteConventionTests.cs | 127 +++++++++++++++ .../TrustedKeyPinningConfigurationTests.cs | 148 ++++++++++++++++++ 7 files changed, 363 insertions(+), 5 deletions(-) create mode 100644 src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedWiringSiteConventionTests.cs diff --git a/docs/certification-evidence.md b/docs/certification-evidence.md index 487899257..fc467487b 100644 --- a/docs/certification-evidence.md +++ b/docs/certification-evidence.md @@ -760,6 +760,22 @@ redundant guard is exactly what a careful proposer writes. every deployment that has not configured one still verifies unpinned: this row is downgraded from an open hole to a *configurable* one, not closed. + *How much of that wiring is itself verified.* Seven production verifiers apply the + policy, and they are not equally covered. Five are falsifiable behaviourally — the file + record store, the certified brick registry, the hot-swap verify-at-load path, the + composition constituent check and the self-extend admission edge each take a + `CertificationTrustPolicy` a test can hand them, and each has a fact that reddens when + its call site reverts to the bare preset. Two are not: `StateLogVerifier`'s five-argument + overload and the adaptation DI factory resolve the process-wide `Ambient` directly, and + with nothing configured `Ambient.Strict` is *reference-identical* to the `Strict` preset, + so reverting either one is invisible to every test that configures no policy. That is not + hypothetical — five of these seven sites were in that state when pinning was first wired + in. `PinnedWiringSiteConventionTests` now asserts on shipped IL that all seven still call + `CertificationTrustPolicy.Ambient`, so the wiring being DELETED reddens; it does not prove + the pinned set is *enforced* at the two sites with no behavioural fact, and both of those + lines carry a comment saying so. Giving those two a policy parameter is the fix, and it is + a public API change on a packable library. + **Pin against `operator.pub` only.** Do *not* enumerate `~/.ashlar/keys/trusted/`: `OperatorKey.Generate(rotate: true)` writes the *previous* public key into it (`src/Ashlar.Manifest/Signing/OperatorKey.cs:45-52`) and there is no revocation (`:28`), diff --git a/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs b/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs index 9104ce418..aa73b3cfb 100644 --- a/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs +++ b/src/Ashlar.Certification.Contracts/CertificationTrustPolicy.cs @@ -205,6 +205,8 @@ public static CertificationTrustPolicy FromTrustedKeys(IEnumerable trust /// already pins a different set of keys. Silently replacing it would /// mean configuration could redirect trust that was chosen in code, and merging the two would /// widen the accepted signer set beyond either — so a conflict is reported, not resolved. + /// Also thrown when pins a blank or null entry: the same refusal + /// Validate gives a configured set, for the same reason. /// public CertificationVerifyOptions Apply(CertificationVerifyOptions basis) { @@ -215,12 +217,38 @@ public CertificationVerifyOptions Apply(CertificationVerifyOptions basis) if (keys is null) return basis; - if (basis.PinningEnabled && !SameKeys(basis.TrustedEd25519PublicKeys!, keys)) + if (basis.PinningEnabled) { - throw new CertificationTrustConfigurationException( - "The supplied verification options already pin a different set of Ed25519 signers. " - + $"Configured pinning ({TrustedKeysVariable}) will not override a pinning set chosen " - + "in code, and will not widen it by merging: resolve the two deliberately."); + var basisKeys = basis.TrustedEd25519PublicKeys!; + foreach (var basisKey in basisKeys) + { + if (!string.IsNullOrWhiteSpace(basisKey)) + continue; + + // The caller-supplied side gets the same refusal a configured set gets, and for + // the same reason: an entry no signature can match is not part of the set the + // caller chose, so neither skipping it (which decides "same set" on a set that + // was never stated) nor comparing it (which decides "conflict" on a hole) is an + // answer. Refusing here is also what keeps this a NAMED failure: SameKeys builds + // a HashSet under StringComparer.Ordinal, and a null entry reaching that would + // surface as a bare ArgumentNullException out of a security type. + throw new CertificationTrustConfigurationException( + "The supplied verification options pin a blank entry. Refusing rather than " + + "ignoring it: an entry that is null or whitespace is a key no signature can " + + "ever match, so it is not part of the pinning set the caller chose — and " + + "silently dropping it would decide the conflict check below against a set " + + "that was never stated. Remove the blank entry, or leave " + + $"{nameof(CertificationVerifyOptions.TrustedEd25519PublicKeys)} unset on the " + + $"basis and let configured pinning ({TrustedKeysVariable}) supply the set."); + } + + if (!SameKeys(basisKeys, keys)) + { + throw new CertificationTrustConfigurationException( + "The supplied verification options already pin a different set of Ed25519 signers. " + + $"Configured pinning ({TrustedKeysVariable}) will not override a pinning set chosen " + + "in code, and will not widen it by merging: resolve the two deliberately."); + } } return new CertificationVerifyOptions @@ -357,6 +385,12 @@ private static bool ParsePinningRequired(string? configured) /// summary promises never happens. de-duplicates the configured side, /// but comes from a caller-supplied options object and nothing /// de-duplicates that. + /// + /// arrives free of null and blank entries, because + /// — the only caller — refuses them by name first. That guard is not + /// decoration: the below is built under + /// , and a security type should not be resting on what + /// that comparer happens to do with a null entry. /// private static bool SameKeys(IReadOnlyCollection left, IReadOnlyCollection right) => new HashSet(left, StringComparer.Ordinal).SetEquals(right); diff --git a/src/Ashlar.Certification.State/StateLogVerifier.cs b/src/Ashlar.Certification.State/StateLogVerifier.cs index 9718b1577..e00f95eca 100644 --- a/src/Ashlar.Certification.State/StateLogVerifier.cs +++ b/src/Ashlar.Certification.State/StateLogVerifier.cs @@ -34,6 +34,22 @@ public static StateLogTrustResult Verify( ICertificateResolver resolver, string? hmacKey = null, ITransitionReplayer? replayer = null) => + // UNVERIFIED BEHAVIOURALLY, deliberately, and this comment is the disclosure. + // + // With nothing configured, `Ambient.Strict` is reference-identical to the `Strict` preset, + // so replacing this argument with `Contracts.CertificationVerifyOptions.Strict` changes + // nothing any test can observe: a green gate is NOT evidence that an operator's pinning set + // reaches this overload. Five of the seven pinning wiring sites drifted invisibly for + // exactly this reason before anyone looked. + // + // What covers it: `PinnedWiringSiteConventionTests` asserts on shipped IL that this method + // still calls `CertificationTrustPolicy.get_Ambient`, so DELETING the wiring reddens. What + // does not cover it: nothing proves the pinning set is then ENFORCED here, because this + // overload takes no policy a test could hand it, and `Ambient` is a process-wide `Lazy` + // resolved once — a test that set the environment variables would either lose the race to + // whichever class touched `Ambient` first or pin the whole process to its own key. + // Giving this overload a `CertificationTrustPolicy` parameter is the fix; it is a public API + // change on a packable library, so it is flagged for review rather than taken here. Verify(log, schema, resolver, hmacKey, replayer, Contracts.CertificationTrustPolicy.Ambient.Strict); /// diff --git a/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs b/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs index 268016902..2561f96d9 100644 --- a/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs +++ b/src/Ashlar.Infrastructure/Adaptation/Sdk/Extensions/AdaptationServiceCollectionExtensions.cs @@ -100,6 +100,19 @@ public static IServiceCollection AddAdaptationInfrastructure( var record = store.Get("observation.context"); // Strict plus whatever signer set the operator pinned; with nothing configured this // is the Strict preset unchanged. + // + // UNVERIFIED BEHAVIOURALLY, deliberately, and this comment is the disclosure. + // Because `Ambient.Strict` IS the `Strict` preset instance when nothing is + // configured, replacing this argument with `CertificationVerifyOptions.Strict` + // changes nothing any test can observe — a green gate is not evidence that an + // operator's pinning set reaches this factory. `PinnedWiringSiteConventionTests` + // asserts on shipped IL that this type still calls + // `CertificationTrustPolicy.get_Ambient`, so DELETING the wiring reddens; nothing + // proves the pinned set is then enforced here. The reason it cannot be reached is + // this factory's own shape: it resolves the process-wide `Ambient` directly, and + // `Ambient` is a `Lazy` resolved once per process, so a test cannot vary it without + // pinning the whole run. Resolving an optional `CertificationTrustPolicy` from `sp` + // instead would create the seam — a DI contract change, flagged rather than taken. if (record is { Admitted: true, Signed: true } && signer.Verify(record, CertificationTrustPolicy.Ambient.Strict)) { diff --git a/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs b/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs index fdda73a63..2338f1af1 100644 --- a/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs +++ b/src/Ashlar.Infrastructure/Certification/Composition/CompositionConstituentChecker.cs @@ -23,6 +23,10 @@ public static ConstituentCheckResult Check( CertificationTrustPolicy? trustPolicy = null) { var violations = new List(); + // Falsifiable through the trustPolicy parameter: + // TrustedKeyPinningConfigurationTests.A_configured_pinning_set_makes_the_composition_constituent_check_refuse_a_foreign_signer + // reddens if this reverts to the bare preset. Keep the parameter — without it, reverting + // this line is invisible, because with nothing configured Ambient.Strict IS that preset. var verifyOptions = (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict; foreach (var node in spec.Nodes) diff --git a/src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedWiringSiteConventionTests.cs b/src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedWiringSiteConventionTests.cs new file mode 100644 index 000000000..ca0f148dc --- /dev/null +++ b/src/Ashlar.Tests.Infrastructure/Tests/Certification/PinnedWiringSiteConventionTests.cs @@ -0,0 +1,127 @@ +using FluentAssertions; +using Mono.Cecil; +using Xunit; + +namespace Ashlar.Tests.Infrastructure.Tests.Certification; + +/// +/// Every production verifier must take its strictness from CertificationTrustPolicy, and this +/// is the guard that notices when one stops. +/// +/// Why a structural guard and not only behavioural ones. With nothing configured, +/// CertificationTrustPolicy.Ambient.Strict is reference-identical to the +/// CertificationVerifyOptions.Strict preset. So reverting any wiring site from +/// (trustPolicy ?? CertificationTrustPolicy.Ambient).Strict back to the bare preset changes +/// no observable behaviour in a test that configures no policy — and a green gate is therefore not +/// evidence that the wiring is still there. That is not hypothetical: when pinning was first wired +/// in, five of these seven sites could each be reverted with the whole suite green, and nobody would +/// have seen the operator's pinning set stop being applied. A behavioural fact closes a site only +/// where the site takes a policy a test can hand it; two of the seven do not, and this guard is what +/// covers them. +/// +/// What it asserts. For each site, that the compiled IL of the declaring type — its +/// nested closures and state machines included, because one site lives inside a DI factory +/// lambda — still calls CertificationTrustPolicy.get_Ambient. Reverting the site to the bare +/// preset deletes that call and reddens the row. This is a call-graph assertion on shipped IL, not +/// a source-text grep: a site cannot satisfy it by mentioning the type in a comment or a doc +/// cref. +/// +/// What it does NOT assert. That the options reaching each verifier are actually +/// enforced — that is what the behavioural facts in +/// and +/// are for, and five of the seven rows have one. It +/// also asserts presence, not count: if a type ever grows a second Ambient site, losing the +/// first would no longer redden. Each of the seven types has exactly one today. +/// +[Trait("Category", "Certification")] +public sealed class PinnedWiringSiteConventionTests +{ + private const string TrustPolicyTypeName = "Ashlar.Certification.Contracts.CertificationTrustPolicy"; + + private const string AmbientAccessor = "get_Ambient"; + + /// + /// The seven sites, each named by the assembly it ships in and the type that holds it. The + /// fifth column of the story — whether the site also has a behavioural fact — is in the + /// behaviourallyCovered argument, so a reader can see at a glance which rows are carried + /// by this guard alone. + /// + [Theory] + [InlineData( + "Ashlar.Infrastructure.dll", + "Ashlar.Infrastructure.Certification.FileCertificationRecordStore", + true)] + [InlineData( + "Ashlar.Infrastructure.dll", + "Ashlar.Infrastructure.Certification.CertifiedBrickRegistry", + true)] + [InlineData( + "Ashlar.Infrastructure.dll", + "Ashlar.Infrastructure.Certification.HotSwap.CertifiedBrickHotSwapHost", + true)] + [InlineData( + "Ashlar.Infrastructure.dll", + "Ashlar.Infrastructure.Certification.Composition.CompositionConstituentChecker", + true)] + [InlineData( + "Ashlar.BackgroundAgents.dll", + "Ashlar.BackgroundAgents.Security.SelfProducedBrickCertificationPolicy", + true)] + [InlineData( + "Ashlar.Certification.State.dll", + "Ashlar.Certification.State.StateLogVerifier", + false)] + [InlineData( + "Ashlar.Infrastructure.dll", + "Ashlar.Infrastructure.Adaptation.Sdk.Extensions.AdaptationServiceCollectionExtensions", + false)] + public void Every_production_verifier_still_takes_its_strictness_from_the_trust_policy( + string assemblyFileName, + string typeFullName, + bool behaviourallyCovered) + { + var path = Path.Combine(AppContext.BaseDirectory, assemblyFileName); + File.Exists(path).Should().BeTrue( + "{0} must sit beside the test assembly, or this row scans nothing and passes vacuously", + assemblyFileName); + + using var module = ModuleDefinition.ReadModule(path); + var scope = module.Types + .SelectMany(Flatten) + .Where(t => string.Equals(t.FullName, typeFullName, StringComparison.Ordinal) + || t.FullName.StartsWith(typeFullName + "/", StringComparison.Ordinal)) + .ToArray(); + + scope.Should().NotBeEmpty( + "{0} is a named pinning wiring site. If it moved or was renamed, move this row with it " + + "rather than deleting it — deleting the row is how a site goes back to being " + + "unverified.", + typeFullName); + + var sites = scope + .SelectMany(t => t.Methods.Where(m => m.HasBody)) + .Where(m => m.Body.Instructions.Any(IsAmbientAccess)) + .Select(m => m.FullName) + .ToArray(); + + sites.Should().NotBeEmpty( + "nothing in {0} reads CertificationTrustPolicy.Ambient any more, so an operator's " + + "pinning set is not applied there. With nothing configured Ambient.Strict IS the " + + "Strict preset, so this change is invisible to every behavioural test — which is " + + "precisely why this guard exists.{1}", + typeFullName, + behaviourallyCovered + ? " This site also has a behavioural fact, which should have reddened alongside " + + "this row; if it did not, check that the fact still hands the site a policy." + : " This site has NO behavioural fact: it takes no policy a test can supply, so " + + "this row is the only thing standing between it and silent drift."); + } + + private static bool IsAmbientAccess(Mono.Cecil.Cil.Instruction instruction) => + instruction.Operand is MethodReference callee + && string.Equals(callee.Name, AmbientAccessor, StringComparison.Ordinal) + && string.Equals(callee.DeclaringType.FullName, TrustPolicyTypeName, StringComparison.Ordinal); + + private static IEnumerable Flatten(TypeDefinition type) => + type.NestedTypes.SelectMany(Flatten).Prepend(type); +} diff --git a/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs b/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs index 5edfd1a16..fb787936e 100644 --- a/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs +++ b/src/Ashlar.Tests.Infrastructure/Tests/Certification/TrustedKeyPinningConfigurationTests.cs @@ -4,7 +4,9 @@ using Ashlar.BackgroundAgents.Security; using Ashlar.Certification.Contracts; using Ashlar.Core.Application.Certification.Models; +using Ashlar.Core.Application.Paths; using Ashlar.Infrastructure.Certification; +using Ashlar.Infrastructure.Certification.Composition; using Ashlar.Tests.Infrastructure.Certification.Fixtures; using FluentAssertions; using NSec.Cryptography; @@ -564,6 +566,152 @@ public void A_configured_pinning_set_makes_the_certified_registry_refuse_a_forei + "otherwise the refusal above proves nothing about pinning"); } + /// + /// The operator-facing NAMES, pinned to their literals and to the document that tells an + /// operator to set them. + /// + /// Measured, not assumed. Every other fact in this class — the environment fact + /// above included — writes and reads through + /// CertificationTrustPolicy.TrustedKeysVariable itself, so arrange and act agree on + /// whatever that constant happens to say. A reviewer changed its VALUE to + /// ASHLAR_CERT_TRUSTED_ED25519_KEYS_MUTANT and reddened nothing at all, while every + /// operator following docs/Configuration.md would have been setting a variable this + /// process never reads — pinning silently off, with no error anywhere. A variable whose NAME is + /// pinned by nothing is not an operator-facing variable; it is an internal detail with + /// documentation attached. + /// + /// The documentation half is the other direction of the same fact: Configuration.md + /// is the only place an operator learns the spelling, and line 444 of it had already drifted + /// from this code once. Asserting the code's literal alone would let the doc rot instead. + /// + [Fact] + public void The_variable_names_an_operator_is_told_to_set_are_the_names_this_type_reads() + { + CertificationTrustPolicy.TrustedKeysVariable.Should().Be( + "ASHLAR_CERT_TRUSTED_ED25519_KEYS", + "this exact string lives in docs/Configuration.md and in deployment templates nothing " + + "in this repository can grep, so renaming it turns pinning off for every host that " + + "had configured it — fail-open, and silent"); + CertificationTrustPolicy.PinningRequiredVariable.Should().Be( + "ASHLAR_CERT_PINNING_REQUIRED", + "the same, for the switch whose whole purpose is making pinning checkable at startup"); + + var configurationDoc = Path.Combine( + RepoPathResolver.FindRepoRoot(), "docs", "Configuration.md"); + File.Exists(configurationDoc).Should().BeTrue( + "the documentation half of this fact needs the document to exist, or it asserts nothing"); + + var documented = File.ReadAllText(configurationDoc); + documented.Should().Contain( + CertificationTrustPolicy.TrustedKeysVariable, + "the name an operator is told to set and the name this type reads are one fact, or they " + + "are two facts that drift"); + documented.Should().Contain( + CertificationTrustPolicy.PinningRequiredVariable, + "an undocumented switch is a switch nobody sets"); + } + + /// + /// A basis that pins a blank or null entry is refused by name, rather than reaching + /// SameKeys. + /// + /// This is the sharp edge the SameKeys rewrite introduced. The loop it replaced + /// tolerated a null entry; new HashSet<string>(left, StringComparer.Ordinal) does + /// not necessarily, and a security type should not be resting on which. No production caller + /// pins in code today, so nothing reached it — which is exactly why it had to be decided + /// deliberately rather than left to the framework. + /// + /// The configured side is {mine} and the basis is {mine} plus a hole, so + /// the two sets agree on every stated key: without the guard this is either a bare + /// ArgumentNullException out of a security type or a CONFLICT report about a key the + /// caller never stated. The message assertion is what separates the two refusals — the + /// conflict message does not say "blank entry". + /// + [Fact] + public void A_basis_that_pins_a_blank_entry_is_refused_by_name() + { + var mine = CreateEd25519Key().PublicKeyBase64; + var policy = Policy(mine); + + var nullEntry = () => policy.Apply(new CertificationVerifyOptions + { + TrustedEd25519PublicKeys = new[] { mine, null! }, + }); + nullEntry.Should().Throw( + "a null entry is a key no signature can match, and the answer to it is this type's " + + "own named refusal rather than whatever StringComparer.Ordinal does with null") + .WithMessage("*blank entry*"); + + var whitespaceEntry = () => policy.Apply(new CertificationVerifyOptions + { + TrustedEd25519PublicKeys = new[] { mine, " " }, + }); + whitespaceEntry.Should().Throw( + "whitespace is refused for the same reason and with the same message, so the two " + + "cannot diverge later") + .WithMessage("*blank entry*"); + + policy.Apply(new CertificationVerifyOptions { TrustedEd25519PublicKeys = new[] { mine } }) + .TrustedEd25519PublicKeys.Should().BeEquivalentTo( + new[] { mine }, + "the same basis without the hole is the same set as the configuration and must " + + "still pass, or the guard has turned an honest basis into a startup failure"); + } + + /// + /// Fact (a) on a fourth production consumer: the composition lane's constituent check. + /// + /// Of the seven wiring sites, three were unfalsifiable even after the first round — with + /// nothing configured Ambient.Strict IS the Strict preset instance, so reverting a + /// call site to the bare preset is invisible to every test that configures no policy. This one + /// has the same seam the registry and the hot-swap host have (an optional + /// CertificationTrustPolicy parameter), so it takes a behavioural fact rather than a + /// disclosure: a composition whose constituent record was signed by an unpinned key must fail + /// the check, and the identical composition under a policy pinned to the minting key must + /// pass. + /// + /// The positive control is load-bearing here for the usual reason: Check reports a + /// violation for a missing record, an unadmitted record and a bad signature alike, so a failure + /// alone would prove nothing about pinning. + /// + [Fact] + public void A_configured_pinning_set_makes_the_composition_constituent_check_refuse_a_foreign_signer() + { + var ours = CreateEd25519Key(); + var theirs = CreateEd25519Key(); + var brickId = "pinned-constituent-" + Guid.NewGuid().ToString("N")[..8]; + var store = new InMemoryCertificationRecordStore(); + store.Save(new CertificationRecordSigner( + hmacKey: StoreHmacKey, + ed25519PrivateKeyBase64: theirs.PrivateKeyBase64) + .SignRecord(UnsignedRecord(brickId))); + var verifier = new CertificationRecordSigner(hmacKey: StoreHmacKey); + var spec = new CompositionSpec( + "pinned-composition", + new[] { new CompositionNode("only-node", brickId) }, + Array.Empty(), + Array.Empty(), + Array.Empty()); + + var foreign = CompositionConstituentChecker.Check( + spec, store, verifier, trustPolicy: Policy(ours.PublicKeyBase64)); + + foreign.Passed.Should().BeFalse( + "the constituent's record is signed by a key this operator did not pin, and without a " + + "pinning set the check only asks whether the record is self-consistent"); + foreign.Violations.Should().ContainSingle() + .Which.Should().Contain( + "invalid certification signature", + "the violation must be the SIGNATURE one — a missing or unadmitted record produces " + + "its own violation, and either would satisfy a bare Passed==false"); + + CompositionConstituentChecker + .Check(spec, store, verifier, trustPolicy: Policy(theirs.PublicKeyBase64)) + .Passed.Should().BeTrue( + "pinned to the key that minted it, the identical composition must pass — otherwise " + + "the refusal above proves nothing about pinning"); + } + /// /// A second valid 44-character Base64 encoding of the same 32 bytes, or null when this /// runtime's decoder rejects the spare bits — in which case the trap this guards cannot arise