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

crypto/batchverifier.c

@@ -0,0 +1,20 @@
+#include "sodium.h"
+int ed25519_batch_wrapper(const unsigned char **messages2D,
+                          const unsigned char **publicKeys2D,
+                          const unsigned char **signatures2D,
+                          const unsigned char *messages1D,
+                          const unsigned long long *mlen,
+                          const unsigned char *publicKeys1D,
+                          const unsigned char *signatures1D,
+                          size_t num,
+                          int *valid) {
+    // 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];
+    }
+    return crypto_sign_ed25519_open_batch(messages2D, mlen, publicKeys2D, signatures2D, num, valid);
+}

crypto/batchverifier.go

@@ -30,15 +30,22 @@ 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 **messages2D,
+//                           const unsigned char **publicKeys2D,
+//                           const unsigned char **signatures2D,
+//                           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"
-	"runtime"
 	"unsafe"
 )
 
@@ -120,14 +127,21 @@ func (b *BatchVerifier) Verify() error {
 // if some signatures are invalid, true will be set in failed at the corresponding indexes, and
 // ErrBatchVerificationFailed for err
 func (b *BatchVerifier) VerifyWithFeedback() (failed []bool, err error) {
-	if b.GetNumberOfEnqueuedSignatures() == 0 {
+	if len(b.messages) == 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, len(b.messages)*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 +151,27 @@ 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) {
+
+	numberOfSignatures := len(msgLengths)
+	valid := make([]C.int, numberOfSignatures)
+	messages2D := make([]*C.uchar, numberOfSignatures)
+	publicKeys2D := make([]*C.uchar, numberOfSignatures)
+	signatures2D := make([]*C.uchar, numberOfSignatures)
 
 	// 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)))
-
-	runtime.KeepAlive(messages)
-	runtime.KeepAlive(publicKeys)
-	runtime.KeepAlive(signatures)
+	allValid := C.ed25519_batch_wrapper(
+		&messages2D[0], &publicKeys2D[0], &signatures2D[0],
+		(*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]))
 
 	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/curve25519.go

@@ -35,6 +35,7 @@ import "C"
 
 import (
 	"fmt"
+	"unsafe"
 
 	"github.com/algorand/go-algorand/logging"
 	"github.com/algorand/go-algorand/util/metrics"
@@ -64,6 +65,30 @@ func init() {
 	_ = [C.crypto_sign_ed25519_PUBLICKEYBYTES]byte(ed25519PublicKey{})
 	_ = [C.crypto_sign_ed25519_SECRETKEYBYTES]byte(ed25519PrivateKey{})
 	_ = [C.crypto_sign_ed25519_SEEDBYTES]byte(ed25519Seed{})
+
+	// Check that this platform makes slices []Signature and []SignatureVerifier that use a backing
+	// array of contiguously allocated 64- and 32-byte segments, respectively, with no padding.
+	// These slice's backing arrays are passed to C.ed25519_batch_wrapper. In practice, this check
+	// should always succeed, but to be careful we can double-check, since the Go specification does
+	// not explicitly define platform-specific alignment sizes and slice allocation behavior.
+	length := 1024
+	sigs := make([]Signature, length)        // same as [][64]byte
+	pks := make([]SignatureVerifier, length) // same as [][32]byte
+
+	for i := 1; i < length; i++ {
+		if uintptr(unsafe.Pointer(&sigs[i]))-uintptr(unsafe.Pointer(&sigs[0])) != uintptr(i)*C.crypto_sign_ed25519_BYTES {
+			panic("Unexpected alignment for a slice of signatures")
+		}
+		if uintptr(unsafe.Pointer(&pks[i]))-uintptr(unsafe.Pointer(&pks[0])) != uintptr(i)*C.crypto_sign_ed25519_PUBLICKEYBYTES {
+			panic("Unexpected alignment for a slice of public keys")
+		}
+	}
+	if uintptr(unsafe.Pointer(&sigs[length-1]))-uintptr(unsafe.Pointer(&sigs[0])) != uintptr(length-1)*C.crypto_sign_ed25519_BYTES {
+		panic("Unexpected total size for a backing array of signatures")
+	}
+	if uintptr(unsafe.Pointer(&pks[length-1]))-uintptr(unsafe.Pointer(&pks[0])) != uintptr(length-1)*C.crypto_sign_ed25519_PUBLICKEYBYTES {
+		panic("Unexpected total size for a backing array of public keys")
+	}
 }
 
 // A Seed holds the entropy needed to generate cryptographic keys.

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)
+	}
+}