Merge pull request #6 from dexcompiler/copilot/post-phase-security-hardening-v2

Security hardening + commit hook (replacement for #5)

Author: dexcompiler

.githooks/commit-msg

@@ -0,0 +1,2 @@
+#!/bin/sh
+powershell -NoProfile -ExecutionPolicy Bypass -File ".githooks/commit-msg.ps1" "$1"

.githooks/commit-msg.ps1

@@ -0,0 +1,20 @@
+param(
+    [Parameter(Mandatory = $true)]
+    [string]$MessageFile
+)
+
+$lines = Get-Content -LiteralPath $MessageFile
+$filtered = foreach ($line in $lines) {
+    if ($line -notmatch '^(?i)co-authored-by:\s') { $line }
+}
+
+while ($filtered.Count -gt 0 -and $filtered[-1] -eq '') {
+    $filtered = $filtered[0..($filtered.Count - 2)]
+}
+
+if ($filtered.Count -eq 0) {
+    Set-Content -LiteralPath $MessageFile -NoNewline -Value ''
+}
+else {
+    Set-Content -LiteralPath $MessageFile -Value $filtered
+}

README.md

@@ -49,6 +49,12 @@ Ed25519.CreateKeypair(publicKey, privateKey, seed);
 - **Constant-time vs variable-time**:
   - Signing uses constant-time building blocks (`CMov`, etc.).
   - Verification uses variable-time operations (inputs are public).
+- **Strict verification**:
+  - Verification rejects signatures with non-canonical `S` (must satisfy `S < L`).
+  - Verification rejects low-order public keys (small torsion subgroup).
+- **Exact-size contracts**:
+  - `CreateKeypair`: public key = 32 bytes, private key = 64 bytes, seed = 32 bytes.
+  - `Sign`/`Verify`: fixed-size key/signature inputs must be exact size (not oversized).
 
 ### Sign a Message
 
@@ -115,6 +121,8 @@ If your goal is *certificate issuance*, this repo includes **CSR (PKCS#10) helpe
 
 It also supports exporting **encrypted** PKCS#8 PEM (`"ENCRYPTED PRIVATE KEY"`) via `Pkcs.ExportEncryptedPrivateKeyPem(seed, password, iterations)`.
 
+The old encrypted export overload `Pkcs.ExportEncryptedPrivateKeyPem(seed, publicKey, password, iterations)` is kept for compatibility but marked obsolete; the `publicKey` argument is ignored.
+
 ## Implementation Notes
 
 This implementation follows the ref10 naming conventions from the original C code. The short names preserve traceability to the reference implementation for auditing purposes.

src/Ed25519.cs

@@ -15,6 +15,15 @@ namespace Ed25519;
 /// </summary>
 public static class Ed25519
 {
+    // Group order L in little-endian form.
+    private static readonly byte[] ScalarOrderL =
+    [
+        0xED, 0xD3, 0xF5, 0x5C, 0x1A, 0x63, 0x12, 0x58,
+        0xD6, 0x9C, 0xF7, 0xA2, 0xDE, 0xF9, 0xDE, 0x14,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10
+    ];
+
     /// <summary>Size of a public key in bytes.</summary>
     public const int PublicKeySize = 32;
 
@@ -35,18 +44,18 @@ public static class Ed25519
     /// <param name="seed">Input: 32-byte random seed.</param>
     public static void CreateKeypair(Span<byte> publicKey, Span<byte> privateKey, ReadOnlySpan<byte> seed)
     {
-        if (publicKey.Length < PublicKeySize)
-            throw new ArgumentException($"Public key buffer must be at least {PublicKeySize} bytes", nameof(publicKey));
-        if (privateKey.Length < PrivateKeySize)
-            throw new ArgumentException($"Private key buffer must be at least {PrivateKeySize} bytes", nameof(privateKey));
-        if (seed.Length < SeedSize)
-            throw new ArgumentException($"Seed must be at least {SeedSize} bytes", nameof(seed));
+        if (publicKey.Length != PublicKeySize)
+            throw new ArgumentException($"Public key buffer must be exactly {PublicKeySize} bytes", nameof(publicKey));
+        if (privateKey.Length != PrivateKeySize)
+            throw new ArgumentException($"Private key buffer must be exactly {PrivateKeySize} bytes", nameof(privateKey));
+        if (seed.Length != SeedSize)
+            throw new ArgumentException($"Seed must be exactly {SeedSize} bytes", nameof(seed));
 
         // Hash the seed to produce the private scalar and prefix
         Span<byte> hash = stackalloc byte[64];
         try
         {
-            SHA512.HashData(seed[..SeedSize], hash);
+            SHA512.HashData(seed, hash);
 
             // Clamp the scalar (first 32 bytes)
             hash[0] &= 248;
@@ -91,12 +100,12 @@ public static (byte[] PublicKey, byte[] PrivateKey, byte[] Seed) GenerateKeypair
     /// <param name="privateKey">The 64-byte private key.</param>
     public static void Sign(Span<byte> signature, ReadOnlySpan<byte> message, ReadOnlySpan<byte> publicKey, ReadOnlySpan<byte> privateKey)
     {
-        if (signature.Length < SignatureSize)
-            throw new ArgumentException($"Signature buffer must be at least {SignatureSize} bytes", nameof(signature));
-        if (publicKey.Length < PublicKeySize)
-            throw new ArgumentException($"Public key must be at least {PublicKeySize} bytes", nameof(publicKey));
-        if (privateKey.Length < PrivateKeySize)
-            throw new ArgumentException($"Private key must be at least {PrivateKeySize} bytes", nameof(privateKey));
+        if (signature.Length != SignatureSize)
+            throw new ArgumentException($"Signature buffer must be exactly {SignatureSize} bytes", nameof(signature));
+        if (publicKey.Length != PublicKeySize)
+            throw new ArgumentException($"Public key must be exactly {PublicKeySize} bytes", nameof(publicKey));
+        if (privateKey.Length != PrivateKeySize)
+            throw new ArgumentException($"Private key must be exactly {PrivateKeySize} bytes", nameof(privateKey));
 
         // The private key contains: [0..31] = clamped hash (scalar), [32..63] = prefix
         ReadOnlySpan<byte> prefix = privateKey[32..64];
@@ -123,7 +132,7 @@ public static void Sign(Span<byte> signature, ReadOnlySpan<byte> message, ReadOn
             using (var sha = IncrementalHash.CreateHash(HashAlgorithmName.SHA512))
             {
                 sha.AppendData(signature[..32]);
-                sha.AppendData(publicKey[..32]);
+                sha.AppendData(publicKey);
                 sha.AppendData(message);
                 sha.GetHashAndReset(h);
             }
@@ -148,8 +157,8 @@ public static void Sign(Span<byte> signature, ReadOnlySpan<byte> message, ReadOn
     /// <param name="privateKey">The 64-byte private key (expanded key: scalar||prefix).</param>
     public static void Sign(Span<byte> signature, ReadOnlySpan<byte> message, ReadOnlySpan<byte> privateKey)
     {
-        if (privateKey.Length < PrivateKeySize)
-            throw new ArgumentException($"Private key buffer must be at least {PrivateKeySize} bytes", nameof(privateKey));
+        if (privateKey.Length != PrivateKeySize)
+            throw new ArgumentException($"Private key buffer must be exactly {PrivateKeySize} bytes", nameof(privateKey));
 
         // Derive public key: A = [scalar] * B
         Span<byte> publicKey = stackalloc byte[PublicKeySize];
@@ -196,14 +205,16 @@ public static byte[] Sign(ReadOnlySpan<byte> message, ReadOnlySpan<byte> private
     /// <returns>True if the signature is valid, false otherwise.</returns>
     public static bool Verify(ReadOnlySpan<byte> signature, ReadOnlySpan<byte> message, ReadOnlySpan<byte> publicKey)
     {
-        if (signature.Length < SignatureSize)
+        if (signature.Length != SignatureSize)
             return false;
-        if (publicKey.Length < PublicKeySize)
+        if (publicKey.Length != PublicKeySize)
             return false;
 
         // Check that s is in range (top 3 bits of s must be 0)
         if ((signature[63] & 224) != 0)
             return false;
+        if (!IsCanonicalScalar(signature[32..64]))
+            return false;
 
         // Decode public key as point A (negated for subtraction in verification)
         if (Ge.FromBytesNegateVartime(out GeP3 A, publicKey) != 0)
@@ -245,4 +256,24 @@ private static bool ConstantTimeEquals(ReadOnlySpan<byte> x, ReadOnlySpan<byte>
         }
         return r == 0;
     }
+
+    // Returns true iff s is a canonical scalar in [0, L).
+    private static bool IsCanonicalScalar(ReadOnlySpan<byte> s)
+    {
+        if (s.Length != 32)
+            return false;
+
+        // Inputs in Verify are public, so this lexicographic comparison does not
+        // introduce a secret-dependent side-channel.
+        for (int i = 31; i >= 0; i--)
+        {
+            if (s[i] < ScalarOrderL[i])
+                return true;
+            if (s[i] > ScalarOrderL[i])
+                return false;
+        }
+
+        // Equal to L is non-canonical and must be rejected.
+        return false;
+    }
 }

tests/Ed25519Tests.cs

@@ -14,6 +14,14 @@ namespace Ed25519.Tests;
 /// </summary>
 public class Ed25519Tests
 {
+    private static readonly byte[] ScalarOrderL =
+    [
+        0xED, 0xD3, 0xF5, 0x5C, 0x1A, 0x63, 0x12, 0x58,
+        0xD6, 0x9C, 0xF7, 0xA2, 0xDE, 0xF9, 0xDE, 0x14,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10
+    ];
+
     // RFC 8032 Section 7.1 - TEST 1
     // SECRET KEY: 9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60
     // PUBLIC KEY: d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a
@@ -214,6 +222,49 @@ public void SignatureWithNonCanonicalS_ShouldNotVerify()
         Assert.False(Ed25519.Verify(signature, message, publicKey));
     }
 
+    [Fact]
+    public void Verify_WithSExactlyL_ShouldReturnFalse()
+    {
+        byte[] lowOrderPublicKey = new byte[32];
+        lowOrderPublicKey[0] = 0x01; // Edwards identity
+
+        byte[] signature = new byte[64];
+        signature[0] = 0x01; // Encoded identity for R
+        ScalarOrderL.CopyTo(signature.AsSpan(32, 32));
+
+        Assert.False(Ed25519.Verify(signature, "m"u8, lowOrderPublicKey));
+    }
+
+    [Fact]
+    public void Verify_WithSGreaterThanL_ShouldReturnFalse()
+    {
+        byte[] lowOrderPublicKey = new byte[32];
+        lowOrderPublicKey[0] = 0x01; // Edwards identity
+
+        byte[] signature = new byte[64];
+        signature[0] = 0x01; // Encoded identity for R
+        ScalarOrderL.CopyTo(signature.AsSpan(32, 32));
+        signature[32]++; // S = L + 1
+
+        Assert.False(Ed25519.Verify(signature, "m"u8, lowOrderPublicKey));
+    }
+
+    [Fact]
+    public void Verify_WithValidSignatureMutatedByAddingLToS_ShouldReturnFalse()
+    {
+        var (publicKey, privateKey, _) = Ed25519.GenerateKeypair();
+        byte[] message = "malleability-check"u8.ToArray();
+        byte[] signature = Ed25519.Sign(message, publicKey, privateKey);
+
+        byte[] mutated = signature.ToArray();
+        AddLittleEndian(mutated.AsSpan(32, 32), ScalarOrderL);
+
+        if ((mutated[63] & 0b1110_0000) == 0)
+        {
+            Assert.False(Ed25519.Verify(mutated, message, publicKey));
+        }
+    }
+
     [Fact]
     public void Verify_WithInvalidPublicKeyEncoding_ShouldReturnFalse()
     {
@@ -238,6 +289,23 @@ public void Verify_WithLowOrderPublicKey_ShouldReturnFalse()
         Assert.False(Ed25519.Verify(signature, message, lowOrderPublicKey));
     }
 
+    [Fact]
+    public void Verify_WithOversizedInputs_ShouldReturnFalse()
+    {
+        var (publicKey, privateKey, _) = Ed25519.GenerateKeypair();
+        byte[] message = "oversize"u8.ToArray();
+        byte[] signature = Ed25519.Sign(message, publicKey, privateKey);
+
+        byte[] oversizedSignature = new byte[65];
+        signature.CopyTo(oversizedSignature, 0);
+
+        byte[] oversizedPublicKey = new byte[33];
+        publicKey.CopyTo(oversizedPublicKey, 0);
+
+        Assert.False(Ed25519.Verify(oversizedSignature, message, publicKey));
+        Assert.False(Ed25519.Verify(signature, message, oversizedPublicKey));
+    }
+
     [Fact]
     public void SignOverload_WithAndWithoutPublicKey_ShouldMatch()
     {
@@ -255,6 +323,38 @@ public void SignOverload_WithAndWithoutPublicKey_ShouldMatch()
         Assert.True(Ed25519.Verify(sig1, message, publicKey));
     }
 
+    [Fact]
+    public void CreateKeypair_WithUndersizedOrOversizedInputs_ShouldThrow()
+    {
+        byte[] seed = new byte[Ed25519.SeedSize];
+        byte[] publicKey = new byte[Ed25519.PublicKeySize];
+        byte[] privateKey = new byte[Ed25519.PrivateKeySize];
+
+        Assert.Throws<ArgumentException>(() => Ed25519.CreateKeypair(publicKey.AsSpan(0, 31), privateKey, seed));
+        Assert.Throws<ArgumentException>(() => Ed25519.CreateKeypair(new byte[33], privateKey, seed));
+        Assert.Throws<ArgumentException>(() => Ed25519.CreateKeypair(publicKey, privateKey.AsSpan(0, 63), seed));
+        Assert.Throws<ArgumentException>(() => Ed25519.CreateKeypair(publicKey, new byte[65], seed));
+        Assert.Throws<ArgumentException>(() => Ed25519.CreateKeypair(publicKey, privateKey, seed.AsSpan(0, 31)));
+        Assert.Throws<ArgumentException>(() => Ed25519.CreateKeypair(publicKey, privateKey, new byte[33]));
+    }
+
+    [Fact]
+    public void Sign_WithUndersizedOrOversizedInputs_ShouldThrow()
+    {
+        var (publicKey, privateKey, _) = Ed25519.GenerateKeypair();
+        byte[] message = "size-check-sign"u8.ToArray();
+
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[63], message, publicKey, privateKey));
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[65], message, publicKey, privateKey));
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[64], message, publicKey.AsSpan(0, 31), privateKey));
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[64], message, new byte[33], privateKey));
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[64], message, publicKey, privateKey.AsSpan(0, 63)));
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[64], message, publicKey, new byte[65]));
+
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[64], message, privateKey.AsSpan(0, 63)));
+        Assert.Throws<ArgumentException>(() => Ed25519.Sign(new byte[64], message, new byte[65]));
+    }
+
     [Fact]
     public void DeterministicSignatures_SameInputsSameSignature()
     {
@@ -484,6 +584,54 @@ public void EncryptedPkcs8Pem_RoundTrip_DecryptAndRecoverSeed()
         Assert.Equal(seed, recoveredSeed);
     }
 
+    [Fact]
+    public void ExportEncryptedPrivateKeyPem_ObsoleteOverload_ShouldMatchNewOverload()
+    {
+        var (publicKey, _, seed) = Ed25519.GenerateKeypair();
+        const string password = "migration-check-password";
+
+#pragma warning disable CS0618
+        string oldPem = Pkcs.ExportEncryptedPrivateKeyPem(seed, publicKey, password, iterations: 1_000);
+#pragma warning restore CS0618
+        string newPem = Pkcs.ExportEncryptedPrivateKeyPem(seed, password, iterations: 1_000);
+
+        byte[] oldDer = Pkcs.DecodePem(oldPem, "ENCRYPTED PRIVATE KEY");
+        byte[] newDer = Pkcs.DecodePem(newPem, "ENCRYPTED PRIVATE KEY");
+
+        var oldInfo = Pkcs8PrivateKeyInfo.DecryptAndDecode(password, oldDer, out _);
+        var newInfo = Pkcs8PrivateKeyInfo.DecryptAndDecode(password, newDer, out _);
+        Assert.Equal(Pkcs.DecodePkcs8PrivateKey(oldInfo.Encode()), Pkcs.DecodePkcs8PrivateKey(newInfo.Encode()));
+    }
+
+    [Fact]
+    public void VerifyPkcs10CertificationRequest_WithTamperedSignature_ShouldReturnFalse()
+    {
+        var (publicKey, privateKey, _) = Ed25519.GenerateKeypair();
+        byte[] subjectNameDer = new X500DistinguishedName("CN=tamper-check").RawData;
+        byte[] csrDer = Pkcs.EncodePkcs10CertificationRequest(subjectNameDer, publicKey, privateKey);
+
+        byte[] tampered = csrDer.ToArray();
+        tampered[^1] ^= 0x01;
+
+        Assert.False(Pkcs.VerifyPkcs10CertificationRequest(tampered, out _, out _));
+    }
+
+    [Fact]
+    public void VerifyPkcs10CertificationRequest_WithWrongAlgorithmOid_ShouldReturnFalse()
+    {
+        var (publicKey, privateKey, _) = Ed25519.GenerateKeypair();
+        byte[] subjectNameDer = new X500DistinguishedName("CN=oid-check").RawData;
+        byte[] csrDer = Pkcs.EncodePkcs10CertificationRequest(subjectNameDer, publicKey, privateKey);
+
+        byte[] tampered = csrDer.ToArray();
+        // OID encoding in CSR: 06 03 2B 65 70. Corrupt payload byte.
+        int oidIndex = Array.IndexOf(tampered, (byte)0x06);
+        Assert.True(oidIndex >= 0);
+        tampered[oidIndex + 2] ^= 0x01;
+
+        Assert.False(Pkcs.VerifyPkcs10CertificationRequest(tampered, out _, out _));
+    }
+
     [Fact]
     public void Pkcs10Csr_RoundTrip_VerifyAndExtract()
     {
@@ -551,4 +699,17 @@ private static byte[] EncodeWithNonMinimalOuterLength(ReadOnlySpan<byte> der)
         der[2..].CopyTo(nonMinimal.AsSpan(3));
         return nonMinimal;
     }
+
+    private static void AddLittleEndian(Span<byte> destination, ReadOnlySpan<byte> addend)
+    {
+        Assert.Equal(destination.Length, addend.Length);
+
+        int carry = 0;
+        for (int i = 0; i < destination.Length; i++)
+        {
+            int sum = destination[i] + addend[i] + carry;
+            destination[i] = (byte)sum;
+            carry = sum >> 8;
+        }
+    }
 }