Backport 39 security and correctness fixes to the v0.2 line

.github/workflows/test.yml

@@ -64,12 +64,16 @@ jobs:
       - name: Test
         run: dotnet test --no-build --collect:"XPlat Code Coverage"
 
-      # Coverage is identical across platforms; upload once from Linux to avoid duplicate reports
+      # Upload from every platform and let Codecov merge them. This used to run on Linux alone,
+      # on the assumption that coverage is identical everywhere — which stopped being true once
+      # PluginLocator gained genuinely platform-specific code (PATHEXT expansion and the Windows
+      # branch of the executable check simply cannot execute on Linux). A Linux-only report shows
+      # those lines as untested when they are in fact covered by the Windows job.
       - name: Upload coverage to Codecov
-        if: matrix.os == 'ubuntu-latest'
         uses: codecov/codecov-action@v5
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
+          flags: ${{ matrix.os }}
 
   # Runs the whole suite + CCTV vectors through the managed BouncyCastle AEAD backend —
   # the same path Blazor/WASM takes. Managed code is OS-independent, so one OS is enough.

.gitignore

@@ -26,3 +26,7 @@ BenchmarkDotNet.Artifacts/
 .DS_Store
 Thumbs.db
 dist/
+
+## Reference implementations, cloned locally to read while working.
+## Nested git repos — tracking them would create broken gitlinks.
+/references/

Age.Benchmarks/KeyGenBenchmarks.cs

@@ -3,6 +3,17 @@
 
 namespace Age.Benchmarks;
 
+/// <summary>
+/// Key generation, measured two ways because the two types put the work in different places.
+/// </summary>
+/// <remarks>
+/// X25519 does its keygen in <c>Generate</c> and derives the recipient from an already-computed
+/// public key. ML-KEM-768-X25519 is the reverse: <c>Generate</c> only fills a 32-byte seed, and the
+/// ML-KEM keygen runs on first access to <c>Recipient</c> (cached thereafter). Comparing the two
+/// <c>Generate</c> calls alone therefore reports post-quantum keygen as the faster of the two,
+/// which is backwards — it has merely not happened yet. The …ToRecipient pair is the comparable
+/// number: from nothing to a usable public key.
+/// </remarks>
 [MemoryDiagnoser]
 public class KeyGenBenchmarks
 {
@@ -11,4 +22,10 @@ public class KeyGenBenchmarks
 
     [Benchmark]
     public MlKem768X25519Identity MlKem768X25519Generate() => MlKem768X25519Identity.Generate();
+
+    [Benchmark]
+    public X25519Recipient X25519ToRecipient() => X25519Identity.Generate().Recipient;
+
+    [Benchmark]
+    public MlKem768X25519Recipient MlKem768X25519ToRecipient() => MlKem768X25519Identity.Generate().Recipient;
 }

Age.Tests/AgreementGuardTests.cs

@@ -0,0 +1,128 @@
+using System.Text;
+using Age.Crypto;
+using Age.Recipients;
+using Org.BouncyCastle.Crypto.Parameters;
+using Org.BouncyCastle.Security;
+using Xunit;
+
+namespace Age.Tests;
+
+/// <summary>
+/// C8 / C9 / C10 — all eight X25519 agreement sites now go through one guarded helper. This is
+/// defence in depth plus a consistent exception type, not the closing of an exploitable hole:
+/// BouncyCastle already refuses low-order points, so no zero shared secret was ever used. What
+/// was broken is the exception contract — five sites let a raw <c>InvalidOperationException</c>
+/// escape, so a caller catching <see cref="AgeException"/> to handle hostile files crashed, and
+/// the CLI reported a merely malformed input file as "This is a bug".
+/// </summary>
+public class AgreementGuardTests
+{
+    // Canonical low-order and identity points for Curve25519.
+    public static TheoryData<string, string> LowOrderPoints() => new()
+    {
+        { "all zeroes (identity)", new string('0', 64) },
+        { "u = 1", "0100000000000000000000000000000000000000000000000000000000000000" },
+        { "order 8", "e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800" },
+        { "order 4", "5f9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f1157" },
+        { "p - 1", "ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f" },
+    };
+
+    [Theory]
+    [MemberData(nameof(LowOrderPoints))]
+    public void X25519Agree_RejectsLowOrderPoints(string name, string pointHex)
+    {
+        Assert.NotEmpty(name);
+
+        var privateKey = new X25519PrivateKeyParameters(new SecureRandom());
+        var point = new X25519PublicKeyParameters(Convert.FromHexString(pointHex));
+        var sharedSecret = new byte[CryptoHelper.X25519SharedSecretSize];
+
+        var ex = Assert.Throws<AgeHeaderException>(
+            () => CryptoHelper.X25519Agree(privateKey, point, sharedSecret));
+
+        Assert.Contains("all-zero", ex.Message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public void X25519Agree_AcceptsAnHonestPeer()
+    {
+        var a = new X25519PrivateKeyParameters(new SecureRandom());
+        var b = new X25519PrivateKeyParameters(new SecureRandom());
+
+        var ab = new byte[CryptoHelper.X25519SharedSecretSize];
+        var ba = new byte[CryptoHelper.X25519SharedSecretSize];
+
+        CryptoHelper.X25519Agree(a, b.GeneratePublicKey(), ab);
+        CryptoHelper.X25519Agree(b, a.GeneratePublicKey(), ba);
+
+        Assert.Equal(ab, ba);
+        Assert.Contains(ab, x => x != 0);
+    }
+
+    // End to end through the public API: a tampered ephemeral share must surface as an
+    // AgeException, which is what a caller is documented to catch.
+    [Theory]
+    [MemberData(nameof(LowOrderPoints))]
+    public void TamperedEphemeralShare_IsCatchableAsAgeException(string name, string pointHex)
+    {
+        Assert.NotEmpty(name);
+
+        using var identity = X25519Identity.Generate();
+
+        using var input = new MemoryStream("guarded"u8.ToArray());
+        using var encrypted = new MemoryStream();
+        AgeEncrypt.Encrypt(input, encrypted, identity.Recipient);
+
+        var tampered = ReplaceFirstStanzaArg(encrypted.ToArray(), "X25519",
+            Base64Unpadded.Encode(Convert.FromHexString(pointHex)));
+
+        var ex = Record.Exception(() =>
+        {
+            using var source = new MemoryStream(tampered);
+            using var output = new MemoryStream();
+            AgeEncrypt.Decrypt(source, output, identity);
+        });
+
+        Assert.NotNull(ex);
+        Assert.IsAssignableFrom<AgeException>(ex);
+    }
+
+    // A recipient can carry a low-order point too, so the encrypt side is guarded as well.
+    [Theory]
+    [MemberData(nameof(LowOrderPoints))]
+    public void RecipientCarryingALowOrderPoint_IsCatchableAsAgeException(string name, string pointHex)
+    {
+        Assert.NotEmpty(name);
+
+        var recipient = X25519Recipient.Parse(Bech32.Encode("age", Convert.FromHexString(pointHex)));
+
+        var ex = Record.Exception(() =>
+        {
+            using var input = new MemoryStream("guarded"u8.ToArray());
+            using var output = new MemoryStream();
+            AgeEncrypt.Encrypt(input, output, recipient);
+        });
+
+        Assert.NotNull(ex);
+        Assert.IsAssignableFrom<AgeException>(ex);
+    }
+
+    private static byte[] ReplaceFirstStanzaArg(byte[] file, string stanzaType, string replacement)
+    {
+        var lines = Encoding.ASCII.GetString(file).Split('\n');
+
+        for (var i = 0; i < lines.Length; i++)
+        {
+            if (!lines[i].StartsWith($"-> {stanzaType} ", StringComparison.Ordinal))
+                continue;
+
+            var parts = lines[i].Split(' ');
+            parts[2] = replacement;
+            lines[i] = string.Join(' ', parts);
+
+            return Encoding.ASCII.GetBytes(string.Join('\n', lines));
+        }
+
+        throw new InvalidOperationException($"no {stanzaType} stanza found");
+    }
+}

Age.Tests/ArmorHardeningTests.cs

@@ -0,0 +1,89 @@
+using System.Text;
+using Age.Recipients;
+using Xunit;
+
+namespace Age.Tests;
+
+/// <summary>
+/// H9 / H11 — armor input hardening, bringing main in line with both reference implementations.
+/// </summary>
+public class ArmorHardeningTests
+{
+    private static byte[] Armored(X25519Recipient recipient, byte[] plaintext)
+    {
+        using var input = new MemoryStream(plaintext);
+        using var output = new MemoryStream();
+        AgeEncrypt.Encrypt(input, output, armor: true, recipient);
+        return output.ToArray();
+    }
+
+    private static byte[] Decrypt(byte[] file, IIdentity identity)
+    {
+        using var input = new MemoryStream(file);
+        using var output = new MemoryStream();
+        AgeEncrypt.Decrypt(input, output, identity);
+        return output.ToArray();
+    }
+
+    // H9. There is no explicit CR guard and deliberately so — StreamReader.ReadLine splits on a
+    // lone CR, so a returned line can never contain one and any check would be dead code. The CR
+    // is still rejected, because the fragments it creates fail the line-width rules. This test
+    // pins that outcome rather than proving a fix.
+    [Fact]
+    public void BareCarriageReturnInArmorBody_IsRejected()
+    {
+        using var identity = X25519Identity.Generate();
+        var armored = Encoding.ASCII.GetString(Armored(identity.Recipient, "hello armor"u8.ToArray()));
+
+        // Split one body line with a bare CR rather than a newline.
+        var lines = armored.Split('\n');
+        var bodyIndex = Array.FindIndex(lines, l => l.Length > 8 && !l.StartsWith("-----", StringComparison.Ordinal));
+        lines[bodyIndex] = lines[bodyIndex][..4] + "\r" + lines[bodyIndex][4..];
+
+        var tampered = Encoding.ASCII.GetBytes(string.Join('\n', lines));
+
+        Assert.Throws<AgeArmorException>(() => Decrypt(tampered, identity));
+    }
+
+    // CRLF is legitimate and must keep working — StreamReader consumes it as one terminator, so
+    // no CR survives into the line.
+    [Fact]
+    public void CrlfLineEndings_StillDecrypt()
+    {
+        using var identity = X25519Identity.Generate();
+        var plaintext = "hello armor"u8.ToArray();
+
+        var armored = Encoding.ASCII.GetString(Armored(identity.Recipient, plaintext));
+        var crlf = Encoding.ASCII.GetBytes(armored.Replace("\n", "\r\n"));
+
+        Assert.Equal(plaintext, Decrypt(crlf, identity));
+    }
+
+    [Fact]
+    public void ModestLeadingWhitespace_IsStillAccepted()
+    {
+        using var identity = X25519Identity.Generate();
+        var plaintext = "hello armor"u8.ToArray();
+
+        var padded = (byte[]) [.. Encoding.ASCII.GetBytes(new string('\n', 8)), .. Armored(identity.Recipient, plaintext)];
+
+        Assert.Equal(plaintext, Decrypt(padded, identity));
+    }
+
+    // Without a bound, a file that is nothing but newlines is read to its end before the header
+    // is even looked for.
+    [Fact]
+    public void UnboundedLeadingWhitespace_IsRejected()
+    {
+        using var identity = X25519Identity.Generate();
+
+        var flood = (byte[])
+        [
+            .. Encoding.ASCII.GetBytes(new string('\n', 64 * 1024)),
+            .. Armored(identity.Recipient, "hello armor"u8.ToArray()),
+        ];
+
+        var ex = Assert.Throws<AgeArmorException>(() => Decrypt(flood, identity));
+        Assert.Contains("whitespace", ex.Message, StringComparison.Ordinal);
+    }
+}

Age.Tests/ArmoredOnAPipeTests.cs

@@ -0,0 +1,87 @@
+using Age.Recipients;
+using Xunit;
+
+namespace Age.Tests;
+
+/// <summary>
+/// I1 (message half) — armor is auto-detected only on a seekable stream, so armored input from a
+/// pipe reaches the binary header parser intact and its BEGIN marker was reported as an
+/// "unsupported version". Supporting non-seekable armor outright widens what a patch release
+/// accepts, against the documented behaviour, so only the diagnosis is fixed here.
+/// </summary>
+public class ArmoredOnAPipeTests
+{
+    // A stream that refuses to seek, like a pipe or a network socket.
+    private sealed class NonSeekableStream(byte[] data) : Stream
+    {
+        private readonly MemoryStream _inner = new(data);
+
+        public override bool CanRead => true;
+        public override bool CanSeek => false;
+        public override bool CanWrite => false;
+        public override long Length => throw new NotSupportedException();
+
+        public override long Position
+        {
+            get => throw new NotSupportedException();
+            set => throw new NotSupportedException();
+        }
+
+        public override int Read(byte[] buffer, int offset, int count) => _inner.Read(buffer, offset, count);
+        public override void Flush() { }
+        public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
+        public override void SetLength(long value) => throw new NotSupportedException();
+        public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
+    }
+
+    [Fact]
+    public void ArmoredInputOnANonSeekableStream_SaysSoInsteadOfBlamingTheVersion()
+    {
+        using var identity = X25519Identity.Generate();
+
+        using var input = new MemoryStream("hello"u8.ToArray());
+        using var armored = new MemoryStream();
+        AgeEncrypt.Encrypt(input, armored, armor: true, identity.Recipient);
+
+        using var pipe = new NonSeekableStream(armored.ToArray());
+        using var output = new MemoryStream();
+
+        var ex = Assert.Throws<AgeHeaderException>(() => AgeEncrypt.Decrypt(pipe, output, identity));
+
+        Assert.Contains("ASCII-armored", ex.Message, StringComparison.Ordinal);
+        Assert.DoesNotContain("unsupported version", ex.Message, StringComparison.Ordinal);
+    }
+
+    // A genuinely wrong version line must still report a version problem.
+    [Fact]
+    public void AnActuallyUnsupportedVersion_StillReportsTheVersion()
+    {
+        using var identity = X25519Identity.Generate();
+        var file = System.Text.Encoding.ASCII.GetBytes("age-encryption.org/v2\n--- AAAA\n");
+
+        using var source = new MemoryStream(file);
+        using var output = new MemoryStream();
+
+        var ex = Assert.Throws<AgeHeaderException>(() => AgeEncrypt.Decrypt(source, output, identity));
+
+        Assert.Contains("unsupported version", ex.Message, StringComparison.Ordinal);
+    }
+
+    // Armored input on a seekable stream is detected and decrypts normally — unchanged.
+    [Fact]
+    public void ArmoredInputOnASeekableStream_StillDecrypts()
+    {
+        using var identity = X25519Identity.Generate();
+        var plaintext = "hello"u8.ToArray();
+
+        using var input = new MemoryStream(plaintext);
+        using var armored = new MemoryStream();
+        AgeEncrypt.Encrypt(input, armored, armor: true, identity.Recipient);
+
+        using var source = new MemoryStream(armored.ToArray());
+        using var output = new MemoryStream();
+        AgeEncrypt.Decrypt(source, output, identity);
+
+        Assert.Equal(plaintext, output.ToArray());
+    }
+}