Refactoring in Pqc.Crypto.Lms

4 files changed, 60 insertions, 115 deletions

diff --git a/crypto/src/pqc/crypto/lms/HSS.cs b/crypto/src/pqc/crypto/lms/HSS.cs
index 75d8a0558..ca2ee7b6d 100644
--- a/crypto/src/pqc/crypto/lms/HSS.cs
+++ b/crypto/src/pqc/crypto/lms/HSS.cs
@@ -104,7 +104,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
                 int L = keyPair.Level;
                 int d = L;
                 var prv = keyPair.GetKeys();
-                while (prv[d - 1].GetIndex() == 1 << prv[d - 1].GetSigParameters().H)
+                while (prv[d - 1].GetIndex() == 1 << prv[d - 1].SigParameters.H)
                 {
                     if (--d == 0)
                         // TODO ExhaustedPrivateKeyException
diff --git a/crypto/src/pqc/crypto/lms/HSSPrivateKeyParameters.cs b/crypto/src/pqc/crypto/lms/HSSPrivateKeyParameters.cs
index cda020b82..da81b08e4 100644
--- a/crypto/src/pqc/crypto/lms/HSSPrivateKeyParameters.cs
+++ b/crypto/src/pqc/crypto/lms/HSSPrivateKeyParameters.cs
@@ -125,7 +125,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
                 {
                     LmsPrivateKeyParameters lmsPrivateKey = m_keys[i];
 
-                    parameters[i] = new LmsParameters(lmsPrivateKey.GetSigParameters(), lmsPrivateKey.GetOtsParameters());
+                    parameters[i] = new LmsParameters(lmsPrivateKey.SigParameters, lmsPrivateKey.OtsParameters);
                 }
 
                 return parameters;
@@ -221,7 +221,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
 
             for (int t = originalKeys.Count - 1; t >= 0; t--)
             {
-                LMSigParameters sigParameters = originalKeys[t].GetSigParameters();
+                LMSigParameters sigParameters = originalKeys[t].SigParameters;
                 int mask = (1 << sigParameters.H) - 1;
                 qTreePath[t] = q & mask;
                 q >>= sigParameters.H;
@@ -241,8 +241,8 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             if (m_keys[0].GetIndex() - 1 != qTreePath[0])
             {
                 m_keys[0] = Lms.GenerateKeys(
-                    originalRootKey.GetSigParameters(),
-                    originalRootKey.GetOtsParameters(),
+                    originalRootKey.SigParameters,
+                    originalRootKey.OtsParameters,
                     (int)qTreePath[0], originalRootKey.GetI(), originalRootKey.GetMasterSecret());
                 changed = true;
             }
@@ -250,14 +250,14 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             for (int i = 1; i < qTreePath.Length; i++)
             {
                 LmsPrivateKeyParameters intermediateKey = m_keys[i - 1];
-                int n = intermediateKey.GetOtsParameters().N;
+                int n = intermediateKey.OtsParameters.N;
 
                 byte[] childI = new byte[16];
                 byte[] childSeed = new byte[n];
                 SeedDerive derive = new SeedDerive(
                     intermediateKey.GetI(),
                     intermediateKey.GetMasterSecret(),
-                    LmsUtilities.GetDigest(intermediateKey.GetOtsParameters()))
+                    LmsUtilities.GetDigest(intermediateKey.OtsParameters))
                 {
                     Q = (int)qTreePath[i - 1],
                     J = ~1,
@@ -290,8 +290,8 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
                     // This means the parent has changed.
                     //
                     m_keys[i] = Lms.GenerateKeys(
-                        originalKeys[i].GetSigParameters(),
-                        originalKeys[i].GetOtsParameters(),
+                        originalKeys[i].SigParameters,
+                        originalKeys[i].OtsParameters,
                         (int)qTreePath[i], childI, childSeed);
 
                     //
@@ -307,8 +307,8 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
                     // key so we do not need to sign it again.
                     //
                     m_keys[i] = Lms.GenerateKeys(
-                        originalKeys[i].GetSigParameters(),
-                        originalKeys[i].GetOtsParameters(),
+                        originalKeys[i].SigParameters,
+                        originalKeys[i].OtsParameters,
                         (int)qTreePath[i], childI, childSeed);
                     changed = true;
                 }
@@ -348,7 +348,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             //
             LmsPrivateKeyParameters oldPk = m_keys[d];
 
-            newKeys[d] = Lms.GenerateKeys(oldPk.GetSigParameters(), oldPk.GetOtsParameters(), 0, childI, childRootSeed);
+            newKeys[d] = Lms.GenerateKeys(oldPk.SigParameters, oldPk.OtsParameters, 0, childI, childRootSeed);
 
             var newSig = new List<LmsSignature>(m_sig);
 
diff --git a/crypto/src/pqc/crypto/lms/LMSPrivateKeyParameters.cs b/crypto/src/pqc/crypto/lms/LMSPrivateKeyParameters.cs
index 96f1765c2..5b34f9713 100644
--- a/crypto/src/pqc/crypto/lms/LMSPrivateKeyParameters.cs
+++ b/crypto/src/pqc/crypto/lms/LMSPrivateKeyParameters.cs
@@ -1,9 +1,8 @@
 using System;
-using System.Collections.Generic;
+using System.Collections.Concurrent;
 using System.IO;
 
 using Org.BouncyCastle.Crypto;
-using Org.BouncyCastle.Security;
 using Org.BouncyCastle.Utilities;
 using Org.BouncyCastle.Utilities.IO;
 
@@ -12,30 +11,19 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
     public sealed class LmsPrivateKeyParameters
         : LmsKeyParameters, ILmsContextBasedSigner
     {
-        private static readonly CacheKey T1 = new CacheKey(1);
-        private static readonly CacheKey[] internedKeys = new CacheKey[129];
-
         private static LmsPublicKeyParameters DerivePublicKey(LmsPrivateKeyParameters privateKey)
         {
-            return new LmsPublicKeyParameters(privateKey.parameters, privateKey.otsParameters, privateKey.FindT(T1),
+            return new LmsPublicKeyParameters(privateKey.sigParameters, privateKey.otsParameters, privateKey.FindT(1),
                 privateKey.I);
         }
 
-        static LmsPrivateKeyParameters()
-        {
-            internedKeys[1] = T1;
-            for (int i = 2; i < internedKeys.Length; i++)
-            {
-                internedKeys[i] = new CacheKey(i);
-            }
-        }
-
         private byte[] I;
-        private LMSigParameters parameters;
+        private LMSigParameters sigParameters;
         private LMOtsParameters otsParameters;
         private int maxQ;
         private byte[] masterSecret;
-        private Dictionary<CacheKey, byte[]> tCache;
+        // TODO Java uses a WeakHashMap
+        private ConcurrentDictionary<int, byte[]> tCache;
         private int maxCacheR;
         private IDigest tDigest;
 
@@ -58,14 +46,14 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             int maxQ, byte[] masterSecret, bool isPlaceholder)
             : base(true)
         {
-            this.parameters = lmsParameter;
+            this.sigParameters = lmsParameter;
             this.otsParameters = otsParameters;
             this.q = q;
             this.I = Arrays.Clone(I);
             this.maxQ = maxQ;
             this.masterSecret = Arrays.Clone(masterSecret);
-            this.maxCacheR = 1 << (parameters.H + 1);
-            this.tCache = new Dictionary<CacheKey, byte[]>();
+            this.maxCacheR = 1 << (sigParameters.H + 1);
+            this.tCache = new ConcurrentDictionary<int, byte[]>();
             this.tDigest = LmsUtilities.GetDigest(lmsParameter);
             this.m_isPlaceholder = isPlaceholder;
         }
@@ -73,15 +61,15 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
         private LmsPrivateKeyParameters(LmsPrivateKeyParameters parent, int q, int maxQ)
             : base(true)
         {
-            this.parameters = parent.parameters;
+            this.sigParameters = parent.sigParameters;
             this.otsParameters = parent.otsParameters;
             this.q = q;
             this.I = parent.I;
             this.maxQ = maxQ;
             this.masterSecret = parent.masterSecret;
-            this.maxCacheR = 1 << parameters.H;
+            this.maxCacheR = 1 << sigParameters.H;
             this.tCache = parent.tCache;
-            this.tDigest = LmsUtilities.GetDigest(parameters);
+            this.tDigest = LmsUtilities.GetDigest(sigParameters);
             this.m_publicKey = parent.m_publicKey;
         }
 
@@ -168,7 +156,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
         public LmsContext GenerateLmsContext()
         {
             // Step 1.
-            LMSigParameters lmsParameter = this.GetSigParameters();
+            LMSigParameters lmsParameter = SigParameters;
 
             // Step 2
             int h = lmsParameter.H;
@@ -183,11 +171,10 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             {
                 int tmp = (r / (1 << i)) ^ 1;
 
-                path[i] = this.FindT(tmp);
-                i++;
+                path[i++] = FindT(tmp);
             }
 
-            return otsPk.GetSignatureContext(this.GetSigParameters(), path);
+            return otsPk.GetSignatureContext(sigParameters, path);
         }
 
         public byte[] GenerateSignature(LmsContext context)
@@ -241,30 +228,21 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             }
         }
 
-        public LMSigParameters GetSigParameters()
-        {
-            return parameters;
-        }
+        [Obsolete("Use 'SigParameters' instead")]
+        public LMSigParameters GetSigParameters() => sigParameters;
 
-        public LMOtsParameters GetOtsParameters()
-        {
-            return otsParameters;
-        }
+        public LMSigParameters SigParameters => sigParameters;
 
-        public byte[] GetI()
-        {
-            return Arrays.Clone(I);
-        }
+        [Obsolete("Use 'OtsParameters' instead")]
+        public LMOtsParameters GetOtsParameters() => otsParameters;
 
-        public byte[] GetMasterSecret()
-        {
-            return Arrays.Clone(masterSecret);
-        }
+        public LMOtsParameters OtsParameters => otsParameters;
 
-        public long GetUsagesRemaining()
-        {
-            return maxQ - q;
-        }
+        public byte[] GetI() => Arrays.Clone(I);
+
+        public byte[] GetMasterSecret() => Arrays.Clone(masterSecret);
+
+        public long GetUsagesRemaining() => maxQ - GetIndex();
 
         public LmsPublicKeyParameters GetPublicKey()
         {
@@ -276,64 +254,49 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
 
         internal byte[] FindT(int r)
         {
-            if (r < maxCacheR)
-            {
-                return FindT(r < internedKeys.Length ? internedKeys[r] : new CacheKey(r));
-            }
-
-            return CalcT(r);
-        }
-
-        private byte[] FindT(CacheKey key)
-        {
-            lock (tCache)
-            {
-                if (tCache.TryGetValue(key, out byte[] t))
-                    return t;
+            // TODO Should be > instead of >= ?
+            if (r >= maxCacheR)
+                return CalcT(r);
 
-                return tCache[key] = CalcT(key.m_index);
-            }
+            return tCache.GetOrAdd(r, CalcT);
         }
 
         private byte[] CalcT(int r)
         {
-            int h = this.GetSigParameters().H;
+            int h = sigParameters.H;
 
             int twoToh = 1 << h;
 
-            byte[] T;
+            byte[] T = new byte[tDigest.GetDigestSize()];
 
             // r is a base 1 index.
 
             if (r >= twoToh)
             {
-                LmsUtilities.ByteArray(this.GetI(), tDigest);
+                LmsUtilities.ByteArray(I, tDigest);
                 LmsUtilities.U32Str(r, tDigest);
                 LmsUtilities.U16Str((short)Lms.D_LEAF, tDigest);
                 //
                 // These can be pre generated at the time of key generation and held within the private key.
                 // However it will cost memory to have them stick around.
                 //
-                byte[] K = LMOts.LmsOtsGeneratePublicKey(this.GetOtsParameters(), this.GetI(), (r - twoToh),
-                    this.GetMasterSecret());
+                byte[] K = LMOts.LmsOtsGeneratePublicKey(otsParameters, I, r - twoToh, masterSecret);
 
                 LmsUtilities.ByteArray(K, tDigest);
-                T = new byte[tDigest.GetDigestSize()];
-                tDigest.DoFinal(T, 0);
-                return T;
             }
+            else
+            {
+                byte[] t2r = FindT(2 * r);
+                byte[] t2rPlus1 = FindT(2 * r + 1);
 
-            byte[] t2r = FindT(2 * r);
-            byte[] t2rPlus1 = FindT((2 * r + 1));
+                LmsUtilities.ByteArray(I, tDigest);
+                LmsUtilities.U32Str(r, tDigest);
+                LmsUtilities.U16Str((short)Lms.D_INTR, tDigest);
+                LmsUtilities.ByteArray(t2r, tDigest);
+                LmsUtilities.ByteArray(t2rPlus1, tDigest);
+            }
 
-            LmsUtilities.ByteArray(this.GetI(), tDigest);
-            LmsUtilities.U32Str(r, tDigest);
-            LmsUtilities.U16Str((short)Lms.D_INTR, tDigest);
-            LmsUtilities.ByteArray(t2r, tDigest);
-            LmsUtilities.ByteArray(t2rPlus1, tDigest);
-            T = new byte[tDigest.GetDigestSize()];
             tDigest.DoFinal(T, 0);
-
             return T;
         }
 
@@ -347,7 +310,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
                 && this.q == that.q
                 && this.maxQ == that.maxQ
                 && Arrays.AreEqual(this.I, that.I)
-                && Objects.Equals(this.parameters, that.parameters)
+                && Objects.Equals(this.sigParameters, that.sigParameters)
                 && Objects.Equals(this.otsParameters, that.otsParameters)
                 && Arrays.AreEqual(this.masterSecret, that.masterSecret);
         }
@@ -357,7 +320,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
             int result = q;
             result = 31 * result + maxQ;
             result = 31 * result + Arrays.GetHashCode(I);
-            result = 31 * result + Objects.GetHashCode(parameters);
+            result = 31 * result + Objects.GetHashCode(sigParameters);
             result = 31 * result + Objects.GetHashCode(otsParameters);
             result = 31 * result + Arrays.GetHashCode(masterSecret);
             return result;
@@ -382,7 +345,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
 
             return Composer.Compose()
                 .U32Str(0) // version
-                .U32Str(parameters.ID) // type
+                .U32Str(sigParameters.ID) // type
                 .U32Str(otsParameters.ID) // ots type
                 .Bytes(I) // I at 16 bytes
                 .U32Str(q) // q
@@ -391,23 +354,5 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms
                 .Bytes(masterSecret) // the master secret
                 .Build();
         }
-
-        internal struct CacheKey
-        {
-            internal readonly int m_index;
-
-            public CacheKey(int index)
-            {
-                m_index = index;
-            }
-
-            public override bool Equals(object obj)
-            {
-                return obj is CacheKey that
-                    && this.m_index == that.m_index;
-            }
-
-            public override int GetHashCode() => m_index;
-        }
     }
-}
\ No newline at end of file
+}
diff --git a/crypto/test/src/pqc/crypto/lms/test/HssTests.cs b/crypto/test/src/pqc/crypto/lms/test/HssTests.cs
index 29da1f8c6..9d5048b27 100644
--- a/crypto/test/src/pqc/crypto/lms/test/HssTests.cs
+++ b/crypto/test/src/pqc/crypto/lms/test/HssTests.cs
@@ -770,7 +770,7 @@ namespace Org.BouncyCastle.Pqc.Crypto.Lms.Tests
 
                         for (int t = keyPair.GetKeys().Count - 1; t >= 0; t--)
                         {
-                            LMSigParameters sigParameters = keyPair.GetKeys()[t].GetSigParameters();
+                            LMSigParameters sigParameters = keyPair.GetKeys()[t].SigParameters;
                             int mask = (1 << sigParameters.H) - 1;
                             qValues[t] = q & mask;
                             q >>= sigParameters.H;