cgo: Properly manage memory passing from cgo to go on Batch Verifiers

crypto/batchverifier.c

@@ -0,0 +1,28 @@
+#include "sodium.h"
+int ed25519_batch_wrapper(const unsigned char *messages1D,
+                          const unsigned long long *mlen,
+                          const unsigned char *publicKeys1D,
+                          const unsigned char *signatures1D,
+                          size_t num,
+                          int *valid) {
+    int ret;
+    const unsigned char **messages2D, **publicKeys2D, **signatures2D;
+    messages2D = malloc(num * sizeof(unsigned char *));
+    publicKeys2D = malloc(num * sizeof(unsigned char *));
+    signatures2D = malloc(num * sizeof(unsigned char *));
+
+    // fill 2-D arrays for messages, pks, sigs from provided 1-D arrays
+    unsigned long long mpos = 0;
+    for (size_t i = 0; i < num; i++) {
+        messages2D[i] = &messages1D[mpos];
+        mpos += mlen[i];
+        publicKeys2D[i] = &publicKeys1D[i*crypto_sign_ed25519_PUBLICKEYBYTES];
+        signatures2D[i] = &signatures1D[i*crypto_sign_ed25519_BYTES];
+    }
+    ret = crypto_sign_ed25519_open_batch(messages2D, mlen, publicKeys2D, signatures2D, num, valid);
+
+    free(messages2D);
+    free(publicKeys2D);
+    free(signatures2D);
+    return ret;
+}

crypto/batchverifier.go

@@ -30,11 +30,16 @@ package crypto
 // #cgo windows,amd64 CFLAGS: -I${SRCDIR}/libs/windows/amd64/include
 // #cgo windows,amd64 LDFLAGS: ${SRCDIR}/libs/windows/amd64/lib/libsodium.a
 // #include <stdint.h>
-// #include "sodium.h"
 // enum {
 //	sizeofPtr = sizeof(void*),
 //	sizeofULongLong = sizeof(unsigned long long),
 // };
+// int ed25519_batch_wrapper(const unsigned char *messages1D,
+//                           const unsigned long long *mlen,
+//                           const unsigned char *publicKeys1D,
+//                           const unsigned char *signatures1D,
+//                           size_t num,
+//                           int *valid_p);
 import "C"
 import (
 	"errors"
@@ -123,11 +128,18 @@ func (b *BatchVerifier) VerifyWithFeedback() (failed []bool, err error) {
 	if b.GetNumberOfEnqueuedSignatures() == 0 {
 		return nil, nil
 	}
-	var messages = make([][]byte, b.GetNumberOfEnqueuedSignatures())
+
+	const estimatedMessageSize = 64
+	msgLengths := make([]uint64, 0, len(b.messages))
+	var messages = make([]byte, 0, b.GetNumberOfEnqueuedSignatures()*estimatedMessageSize)
+
+	lenWas := 0
 	for i := range b.messages {
-		messages[i] = HashRep(b.messages[i])
+		messages = HashRepToBuff(b.messages[i], messages)
+		msgLengths = append(msgLengths, uint64(len(messages)-lenWas))
+		lenWas = len(messages)
 	}
-	allValid, failed := batchVerificationImpl(messages, b.publicKeys, b.signatures)
+	allValid, failed := batchVerificationImpl(messages, msgLengths, b.publicKeys, b.signatures)
 	if allValid {
 		return failed, nil
 	}
@@ -137,50 +149,29 @@ func (b *BatchVerifier) VerifyWithFeedback() (failed []bool, err error) {
 // batchVerificationImpl invokes the ed25519 batch verification algorithm.
 // it returns true if all the signatures were authentically signed by the owners
 // otherwise, returns false, and sets the indexes of the failed sigs in failed
-func batchVerificationImpl(messages [][]byte, publicKeys []SignatureVerifier, signatures []Signature) (allSigsValid bool, failed []bool) {
-
-	numberOfSignatures := len(messages)
-
-	messagesAllocation := C.malloc(C.size_t(C.sizeofPtr * numberOfSignatures))
-	messagesLenAllocation := C.malloc(C.size_t(C.sizeofULongLong * numberOfSignatures))
-	publicKeysAllocation := C.malloc(C.size_t(C.sizeofPtr * numberOfSignatures))
-	signaturesAllocation := C.malloc(C.size_t(C.sizeofPtr * numberOfSignatures))
-	valid := C.malloc(C.size_t(C.sizeof_int * numberOfSignatures))
-
-	defer func() {
-		// release staging memory
-		C.free(messagesAllocation)
-		C.free(messagesLenAllocation)
-		C.free(publicKeysAllocation)
-		C.free(signaturesAllocation)
-		C.free(valid)
-	}()
-
-	// load all the data pointers into the array pointers.
-	for i := 0; i < numberOfSignatures; i++ {
-		*(*uintptr)(unsafe.Pointer(uintptr(messagesAllocation) + uintptr(i*C.sizeofPtr))) = uintptr(unsafe.Pointer(&messages[i][0]))
-		*(*C.ulonglong)(unsafe.Pointer(uintptr(messagesLenAllocation) + uintptr(i*C.sizeofULongLong))) = C.ulonglong(len(messages[i]))
-		*(*uintptr)(unsafe.Pointer(uintptr(publicKeysAllocation) + uintptr(i*C.sizeofPtr))) = uintptr(unsafe.Pointer(&publicKeys[i][0]))
-		*(*uintptr)(unsafe.Pointer(uintptr(signaturesAllocation) + uintptr(i*C.sizeofPtr))) = uintptr(unsafe.Pointer(&signatures[i][0]))
-	}
+func batchVerificationImpl(messages []byte, msgLengths []uint64, publicKeys []SignatureVerifier, signatures []Signature) (allSigsValid bool, failed []bool) {
 
-	// call the batch verifier
-	allValid := C.crypto_sign_ed25519_open_batch(
-		(**C.uchar)(unsafe.Pointer(messagesAllocation)),
-		(*C.ulonglong)(unsafe.Pointer(messagesLenAllocation)),
-		(**C.uchar)(unsafe.Pointer(publicKeysAllocation)),
-		(**C.uchar)(unsafe.Pointer(signaturesAllocation)),
-		C.size_t(len(messages)),
-		(*C.int)(unsafe.Pointer(valid)))
+	numberOfSignatures := len(msgLengths)
+	valid := make([]C.int, numberOfSignatures)
 
+	// call the batch verifier
+	allValid := C.ed25519_batch_wrapper(
+		(*C.uchar)(&messages[0]),
+		(*C.ulonglong)(&msgLengths[0]),
+		(*C.uchar)(&(publicKeys[0][0])),
+		(*C.uchar)(&(signatures[0][0])),
+		C.size_t(numberOfSignatures),
+		(*C.int)(&valid[0]))
+
+	// These calls will be replaced with Pin in the next Go version upgrade
 	runtime.KeepAlive(messages)
+	runtime.KeepAlive(msgLengths)
 	runtime.KeepAlive(publicKeys)
 	runtime.KeepAlive(signatures)
 
 	failed = make([]bool, numberOfSignatures)
 	for i := 0; i < numberOfSignatures; i++ {
-		cint := *(*C.int)(unsafe.Pointer(uintptr(valid) + uintptr(i*C.sizeof_int)))
-		failed[i] = (cint == 0)
+		failed[i] = (valid[i] == 0)
 	}
 	return allValid == 0, failed
 }

crypto/onetimesig.go

@@ -319,8 +319,21 @@ func (v OneTimeSignatureVerifier) Verify(id OneTimeSignatureIdentifier, message
 		Batch:    id.Batch,
 	}
 
+	// serialize encoded batchID, offsetID, message into a continuous memory buffer with the layout
+	// hashRep(batchID)... hashRep(offsetID)... hashRep(message)...
+	const estimatedSize = 256
+	messageBuffer := make([]byte, 0, estimatedSize)
+
+	messageBuffer = HashRepToBuff(batchID, messageBuffer)
+	batchIDLen := uint64(len(messageBuffer))
+	messageBuffer = HashRepToBuff(offsetID, messageBuffer)
+	offsetIDLen := uint64(len(messageBuffer)) - batchIDLen
+	messageBuffer = HashRepToBuff(message, messageBuffer)
+	messageLen := uint64(len(messageBuffer)) - offsetIDLen - batchIDLen
+	msgLengths := []uint64{batchIDLen, offsetIDLen, messageLen}
 	allValid, _ := batchVerificationImpl(
-		[][]byte{HashRep(batchID), HashRep(offsetID), HashRep(message)},
+		messageBuffer,
+		msgLengths,
 		[]PublicKey{PublicKey(v), PublicKey(batchID.SubKeyPK), PublicKey(offsetID.SubKeyPK)},
 		[]Signature{Signature(sig.PK2Sig), Signature(sig.PK1Sig), Signature(sig.Sig)},
 	)

crypto/util.go

@@ -40,6 +40,14 @@ func HashRep(h Hashable) []byte {
 	return append([]byte(hashid), data...)
 }
 
+// HashRepToBuff appends the correct hashid before the message to be hashed into the provided buffer
+func HashRepToBuff(h Hashable, buffer []byte) []byte {
+	hashid, data := h.ToBeHashed()
+	buffer = append(buffer, hashid...)
+	buffer = append(buffer, data...)
+	return buffer
+}
+
 // DigestSize is the number of bytes in the preferred hash Digest used here.
 const DigestSize = sha512.Size256
 

crypto/util_test.go

@@ -17,8 +17,10 @@
 package crypto
 
 import (
+	"fmt"
 	"testing"
 
+	"github.com/algorand/go-algorand/protocol"
 	"github.com/algorand/go-algorand/test/partitiontest"
 	"github.com/stretchr/testify/require"
 )
@@ -46,3 +48,32 @@ func TestDigest_IsZero(t *testing.T) {
 	require.NotZero(t, d2)
 
 }
+
+type testToBeHashed struct {
+	i int
+}
+
+func (tbh *testToBeHashed) ToBeHashed() (protocol.HashID, []byte) {
+	data := make([]byte, tbh.i)
+	for x := 0; x < tbh.i; x++ {
+		data[x] = byte(tbh.i)
+	}
+	return protocol.HashID(fmt.Sprintf("ID%d", tbh.i)), data
+}
+
+func TestHashRepToBuff(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	values := []int{32, 64, 512, 1024}
+	buffer := make([]byte, 0, 128)
+	for _, val := range values {
+		tbh := &testToBeHashed{i: val}
+		buffer = HashRepToBuff(tbh, buffer)
+	}
+	pos := 0
+	for _, val := range values {
+		tbh := &testToBeHashed{i: val}
+		data := HashRep(tbh)
+		require.Equal(t, data, buffer[pos:pos+len(data)])
+		pos = pos + len(data)
+	}
+}