Added documentation and some review comments for selected classes

focused mostly on the OT implementation and dependencies.

Author: lumip

CompactMPC/BitArray.cs

@@ -29,6 +29,8 @@ public static int RequiredBytes(int numberOfBits)
             return RequiredBytes(numberOfBits, ElementsPerByte);
         }
 
+        // note(lumip): it seems to me like all these would be nicer if they would return the result
+        //  instead of inplace manipulation. Is there a specific design reason for the current way?
         public void Or(BitArray other)
         {
             if (other.Length != Length)

CompactMPC/Buffers/BufferBuilder.cs

@@ -6,6 +6,9 @@
 
 namespace CompactMPC.Buffers
 {
+    /// <summary>
+    /// Allows to build a byte buffer by composing existing buffers and integer values.
+    /// </summary>
     public class BufferBuilder
     {
         private MessageComposer _composer;

CompactMPC/ObliviousTransfer/IGeneralizedObliviousTransfer.cs

@@ -10,9 +10,36 @@
 
 namespace CompactMPC.ObliviousTransfer
 {
+    /// <summary>
+    /// Common interface for 1-out-of-4 bit oblivious transfer implementations.
+    /// </summary>
+    /// 
+    /// For single bit for messages/options, see <see cref="IObliviousTransfer"/>.
     public interface IGeneralizedObliviousTransfer
     {
+        /// <summary>
+        /// Supplies the four options from the sender to the oblivious transfer.
+        /// </summary>
+        /// 
+        /// To increase efficiency, several invocations of OT can be batched into one transmission.
+        /// 
+        /// <param name="channel">The network message channel to the receiver.</param>
+        /// <param name="options">Array containing the four options supplied to 1-out-of-4 OT by the sender for each invocation.</param>
+        /// <param name="numberOfInvocations">The number of OT invocations in the transmission.</param>
+        /// <returns>An asynchronous task which performs the server side of the oblivious transfer.</returns>
         Task SendAsync(IMessageChannel channel, Quadruple<byte[]>[] options, int numberOfInvocations, int numberOfMessageBytes);
+
+        /// <summary>
+        /// Supplies the selection index from the client side to the oblivious transfer and returns the corresponding option from the server.
+        /// </summary>
+        /// 
+        /// To increase efficiency, several invocations of OT can be batched into one transmission
+        /// 
+        /// <param name="channel">The network message channel to the sender.</param>
+        /// <param name="selectionIndices">Array containing the selection index supplied to 1-out-of-4 OT by the client for each invocation.</param>
+        /// <param name="numberOfInvocations">The number of OT invocations in the transmission.</param>
+        /// <returns>An asynchronous task performing the client side of the oblivious transfer and returning the array containing
+        /// the options as selected by the client retrieved from the server.</returns>
         Task<byte[][]> ReceiveAsync(IMessageChannel channel, QuadrupleIndexArray selectionIndices, int numberOfInvocations, int numberOfMessageBytes);
     }
 }

CompactMPC/ObliviousTransfer/IObliviousTransfer.cs

@@ -10,9 +10,36 @@
 
 namespace CompactMPC.ObliviousTransfer
 {
+    /// <summary>
+    /// Common interface for 1-out-of-4 single bit oblivious transfer implementations.
+    /// </summary>
+    /// 
+    /// For a variant with arbitrary bit length for messages/options, see <see cref="IGeneralizedObliviousTransfer"/>.
     public interface IObliviousTransfer
     {
+        /// <summary>
+        /// Supplies the four options from the sender to the oblivious transfer.
+        /// </summary>
+        /// 
+        /// To increase efficiency, several invocations of OT can be batched into one transmission.
+        /// 
+        /// <param name="channel">The network message channel to the receiver.</param>
+        /// <param name="options">Array containing the four options supplied to 1-out-of-4 OT by the sender for each invocation.</param>
+        /// <param name="numberOfInvocations">The number of OT invocations in the transmission.</param>
+        /// <returns>An asynchronous task which performs the server side of the oblivious transfer.</returns>
         Task SendAsync(IMessageChannel channel, BitQuadrupleArray options, int numberOfInvocations);
+
+        /// <summary>
+        /// Supplies the selection index from the client side to the oblivious transfer and returns the corresponding option from the server.
+        /// </summary>
+        /// 
+        /// To increase efficiency, several invocations of OT can be batched into one transmission
+        /// 
+        /// <param name="channel">The network message channel to the sender.</param>
+        /// <param name="selectionIndices">Array containing the selection index supplied to 1-out-of-4 OT by the client for each invocation.</param>
+        /// <param name="numberOfInvocations">The number of OT invocations in the transmission.</param>
+        /// <returns>An asynchronous task performing the client side of the oblivious transfer and returning the array containing
+        /// the options as selected by the client retrieved from the server.</returns>
         Task<BitArray> ReceiveAsync(IMessageChannel channel, QuadrupleIndexArray selectionIndices, int numberOfInvocations);
     }
 }

CompactMPC/ObliviousTransfer/InsecureObliviousTransfer.cs

@@ -9,14 +9,21 @@
 
 namespace CompactMPC.ObliviousTransfer
 {
+    /// <summary>
+    /// Insecure (non-oblivious) implementation of the oblivious transfer interface.
+    /// </summary>
+    /// 
+    /// Caution! This class is intended for testing and debugging purposes and does not provide any security.
+    /// Hence, it should not be used with any sensitive or in production deployments. All options are SENT IN THE PLAIN.
     public class InsecureObliviousTransfer : GeneralizedObliviousTransfer
     {
         public override Task SendAsync(IMessageChannel channel, Quadruple<byte[]>[] options, int numberOfInvocations, int numberOfMessageBytes)
         {
+            // note(lumip): common argument verification code.. wrap into a common method in a base class?
             if (options.Length != numberOfInvocations)
                 throw new ArgumentException("Provided options must match the specified number of invocations.", nameof(options));
 
-            for (int i = 0; i < options.Length; ++i)
+            for (int i = 0; i < numberOfInvocations; ++i)
             {
                 foreach (byte[] message in options[i])
                 {

CompactMPC/ObliviousTransfer/NaorPinkasObliviousTransfer.cs

@@ -14,6 +14,11 @@
 
 namespace CompactMPC.ObliviousTransfer
 {
+    // note(lumip): the implementation does not seem to actually follow any construction given in
+    // [Moni Naor and Benny Pinkas: Computationally Secure Oblivious Transfer. 2005.]
+    // so what is the exact reference here?
+    // looks like
+    // [Moni Naor and Benny Pinkas: Efficient oblivious transfer protocols 2001.]
     public class NaorPinkasObliviousTransfer : GeneralizedObliviousTransfer
     {
         private SecurityParameters _parameters;
@@ -37,6 +42,7 @@ public NaorPinkasObliviousTransfer(SecurityParameters parameters, CryptoContext
 
         public override async Task SendAsync(IMessageChannel channel, Quadruple<byte[]>[] options, int numberOfInvocations, int numberOfMessageBytes)
         {
+            // note(lumip): common argument verification code.. wrap into a common method in a base class?
             if (options.Length != numberOfInvocations)
                 throw new ArgumentException("Provided options must match the specified number of invocations.", nameof(options));
 
@@ -64,6 +70,8 @@ public override async Task SendAsync(IMessageChannel channel, Quadruple<byte[]>[
             });
 
             BigInteger alpha = listOfExponents[0];
+            // note(lumip): sender should probably verify that the all generated elements are different!
+            //  otherwise the receiver could recover two or more of the sent values
 
 #if DEBUG
             stopwatch.Stop();
@@ -175,6 +183,11 @@ public override async Task<byte[][]> ReceiveAsync(IMessageChannel channel, Quadr
             return selectedOptions;
         }
 
+        /// <summary>
+        /// Returns a random element from the group as well as the corresponding exponent for the group generator.
+        /// </summary>
+        /// <param name="exponent">The exponent with which the returned group element can be obtained from the group generator.</param>
+        /// <returns>A random group element.</returns>
         private BigInteger GenerateGroupElement(out BigInteger exponent)
         {
             do
@@ -186,11 +199,22 @@ private BigInteger GenerateGroupElement(out BigInteger exponent)
             return BigInteger.ModPow(_parameters.G, exponent, _parameters.P);
         }
 
+        /// <summary>
+        /// Multiplicatively inverts a group element.
+        /// </summary>
+        /// <param name="groupElement">The group element to be inverted.</param>
+        /// <returns>The multiplicative inverse of the argument in the group.</returns>
         private BigInteger Invert(BigInteger groupElement)
         {
             return BigInteger.ModPow(groupElement, _parameters.Q - 1, _parameters.P);
         }
 
+        /// <summary>
+        /// Asynchronously writes a list of group elements (BigInteger) to a message channel.
+        /// </summary>
+        /// <param name="channel">The network message channel.</param>
+        /// <param name="groupElements">The list of group elements to write/send.</param>
+        /// <returns></returns>
         private Task WriteGroupElements(IMessageChannel channel, IReadOnlyList<BigInteger> groupElements)
         {
             MessageComposer message = new MessageComposer(2 * groupElements.Count);
@@ -204,6 +228,12 @@ private Task WriteGroupElements(IMessageChannel channel, IReadOnlyList<BigIntege
             return channel.WriteMessageAsync(message.Compose());
         }
 
+        /// <summary>
+        /// Asynchronously reads a specified number of group elements from a message channel.
+        /// </summary>
+        /// <param name="channel">The network message channel.</param>
+        /// <param name="numberOfGroupElements">Number of group elements to read/receive.</param>
+        /// <returns></returns>
         private async Task<BigInteger[]> ReadGroupElements(IMessageChannel channel, int numberOfGroupElements)
         {
             MessageDecomposer message = new MessageDecomposer(await channel.ReadMessageAsync());
@@ -246,10 +276,22 @@ private async Task<Quadruple<byte[]>[]> ReadOptions(IMessageChannel channel, int
             return options;
         }
 
-        private byte[] MaskOption(byte[] message, BigInteger groupElement,  int invocationIndex, int optionIndex)
+        /// <summary>
+        /// Masks an option (i.e., a sender input message).
+        /// </summary>
+        /// 
+        /// The option is XOR-masked with the output of a random oracle queried with the
+        /// concatentation of the binary representations of the given groupElement, invocationIndex and optionIndex.
+        /// 
+        /// <param name="option">The sender input/option to be masked.</param>
+        /// <param name="groupElement">The group element that contributes receiver choice to the query.</param>
+        /// <param name="invocationIndex">The index of the OT invocation this options belongs to.</param>
+        /// <param name="optionIndex">The index of the option.</param>
+        /// <returns></returns>
+        private byte[] MaskOption(byte[] option, BigInteger groupElement,  int invocationIndex, int optionIndex)
         {
             byte[] query = BufferBuilder.From(groupElement.ToByteArray()).With(invocationIndex).With(optionIndex).Create();
-            return _randomOracle.Mask(message, query);
+            return _randomOracle.Mask(option, query);
         }
     }
 }

CompactMPC/ObliviousTransfer/RandomOracle.cs

@@ -6,10 +6,42 @@
 
 namespace CompactMPC.ObliviousTransfer
 {
+    /// <summary>
+    /// A random oracle as canonically defined.
+    /// </summary>
+    /// 
+    /// As per the canonical definition of the random oracle, it produces a random but deterministic output
+    /// for each unique query it is invoked on, i.e., the output is unpredictably random over different queries
+    /// but providing the same query will always yield the same output sequence.
+    /// 
+    /// The RandomOracle class also provides a Mask message that masks a given message with the output of the
+    /// random oracle for a given query. In that case the query can be seen as a shared key that allows for
+    /// masking and unmasking (encryption/decryption) of a message.
+    /// 
+    /// Naturally, a true random oracle cannot be implemented so the outputs of all implementations of
+    /// RandomOracle will not be perfectly random but computationally indistinguishable from true randomness
+    /// by the usual cryptographic standards.
     public abstract class RandomOracle
     {
+        /// <summary>
+        /// Supplies the response of the random oracle to the given query.
+        /// </summary>
+        /// 
+        /// As per the canonical definition of the random oracle, it produces a random but deterministic output
+        /// for each unique query, i.e., the output is unpredictably random over different queries but
+        /// providing the same query will always yield the same output sequence.
+        /// 
+        /// <param name="query">The query for the random oracle.</param>
+        /// <returns></returns>
         public abstract IEnumerable<byte> Invoke(byte[] query);
 
+        /// <summary>
+        /// Masks a given message by applying bitwise XOR with the random oracle output stream.
+        /// </summary>
+        /// <param name="message">The message to be masked with the random oracle output.</param>
+        /// <param name="query">The query for the random oracle.</param>
+        /// <returns>Byte array containing the masked message.</returns>
+        /// <exception cref="ArgumentException">Thrown when the random oracle does not provide enough data to mask the given message.</exception>
         public byte[] Mask(byte[] message, byte[] query)
         {
             byte[] result = new byte[message.Length];

SampleCircuits/SecureSetIntersection.cs

@@ -9,6 +9,12 @@
 
 namespace CompactMPC.SampleCircuits
 {
+    /// <summary>
+    /// Circuit which computes the intersection of sets given in a binary word representation.
+    /// </summary>
+    /// 
+    /// In addition to the intersection result, a counter giving the cardinality of the intersection
+    /// is also calculated.
     public class SecureSetIntersection
     {
         private SecureWord _intersection;
@@ -25,6 +31,9 @@ public SecureSetIntersection(SecureWord[] inputs, int numberOfCounterBits)
             _counter = counter.OfFixedLength(numberOfCounterBits);
         }
 
+        /// <summary>
+        /// The binary encoding of the intersection set.
+        /// </summary>
         public SecureWord Intersection
         {
             get
@@ -33,6 +42,9 @@ public SecureWord Intersection
             }
         }
 
+        /// <summary>
+        /// The number of elements in the intersection set.
+        /// </summary>
         public SecureInteger Counter
         {
             get