fix: improve ffi safety (#247)

- *mut ptr to indicate ownership
- Boxed slices instead of vecs for safe transfers
- expanded checks for null ptrs

Closes filecoin-project/rust-fil-ffi-toolkit#9

.circleci/config.yml

@@ -67,7 +67,6 @@ jobs:
       - run: cd rust && cargo install cargo-lipo
       - build_project
       - compile_tests
-      - ensure_generated_cgo_up_to_date
   publish_linux_staticlib:
     executor: golang
     steps:
@@ -305,13 +304,6 @@ commands:
           name: Build project without CGO
           command: env CGO_ENABLED=0 go build .
 
-  ensure_generated_cgo_up_to_date:
-    steps:
-      - run:
-          name: Generate CGO bindings (using forked c-for-go) and compare with what's tracked by Git
-          command: |
-            make cgo-gen
-            git diff --exit-code ./generated/
   run_tests:
     parameters:
       run_leak_detector:

Makefile

@@ -34,10 +34,6 @@ cgo-leakdetect: runner
 	valgrind --leak-check=full --show-leak-kinds=definite ./runner
 .PHONY: cgo-leakdetect
 
-cgo-gen: $(DEPS)
-	go run github.com/xlab/c-for-go --nostamp filcrypto.yml
-.PHONY: cgo-gen
-
 runner: $(DEPS)
 	rm -f ./runner
 	go build -o ./runner ./cgoleakdetect/

README.md

@@ -82,13 +82,6 @@ $ make -C extern/filecoin-ffi
 $ go mod edit -replace=github.com/filecoin-project/filecoin-ffi=./extern/filecoin-ffi
 ```
 
-## Updating CGO Bindings
-
-The CGO bindings are generated using [c-for-go](https://github.com/xlab/c-for-go)
-and committed to Git. To generate bindings yourself, install the c-for-go
-binary, ensure that it's on your path, and then run `make cgo-gen`. CI builds
-will fail if generated CGO diverges from what's checked into Git.
-
 ## Updating the Changelog
 
 The `mkreleaselog` script (in the project root) can be used to generate a good

bls.go

@@ -9,20 +9,16 @@ package ffi
 // #include "./filcrypto.h"
 import "C"
 import (
-	"github.com/filecoin-project/filecoin-ffi/generated"
+	"github.com/filecoin-project/filecoin-ffi/cgo"
 )
 
 // Hash computes the digest of a message
 func Hash(message Message) Digest {
-	resp := generated.FilHash(message, uint(len(message)))
-	resp.Deref()
-	resp.Digest.Deref()
-
-	defer generated.FilDestroyHashResponse(resp)
-
-	var out Digest
-	copy(out[:], resp.Digest.Inner[:])
-	return out
+	digest := cgo.Hash(cgo.AsSliceRefUint8(message))
+	if digest == nil {
+		return Digest{}
+	}
+	return *digest
 }
 
 // Verify verifies that a signature is the aggregated signature of digests - pubkeys
@@ -38,9 +34,11 @@ func Verify(signature *Signature, digests []Digest, publicKeys []PublicKey) bool
 		copy(flattenedPublicKeys[(PublicKeyBytes*idx):(PublicKeyBytes*(1+idx))], publicKey[:])
 	}
 
-	isValid := generated.FilVerify(signature[:], flattenedDigests, uint(len(flattenedDigests)), flattenedPublicKeys, uint(len(flattenedPublicKeys)))
-
-	return isValid > 0
+	return cgo.Verify(
+		cgo.AsSliceRefUint8(signature[:]),
+		cgo.AsSliceRefUint8(flattenedDigests),
+		cgo.AsSliceRefUint8(flattenedPublicKeys),
+	)
 }
 
 // HashVerify verifies that a signature is the aggregated signature of hashed messages.
@@ -57,9 +55,12 @@ func HashVerify(signature *Signature, messages []Message, publicKeys []PublicKey
 		copy(flattenedPublicKeys[(PublicKeyBytes*idx):(PublicKeyBytes*(1+idx))], publicKey[:])
 	}
 
-	isValid := generated.FilHashVerify(signature[:], flattenedMessages, uint(len(flattenedMessages)), messagesSizes, uint(len(messagesSizes)), flattenedPublicKeys, uint(len(flattenedPublicKeys)))
-
-	return isValid > 0
+	return cgo.HashVerify(
+		cgo.AsSliceRefUint8(signature[:]),
+		cgo.AsSliceRefUint8(flattenedMessages),
+		cgo.AsSliceRefUint(messagesSizes),
+		cgo.AsSliceRefUint8(flattenedPublicKeys),
+	)
 }
 
 // Aggregate aggregates signatures together into a new signature. If the
@@ -72,84 +73,43 @@ func Aggregate(signatures []Signature) *Signature {
 		copy(flattenedSignatures[(SignatureBytes*idx):(SignatureBytes*(1+idx))], sig[:])
 	}
 
-	resp := generated.FilAggregate(flattenedSignatures, uint(len(flattenedSignatures)))
-	if resp == nil {
-		return nil
-	}
-
-	defer generated.FilDestroyAggregateResponse(resp)
-
-	resp.Deref()
-	resp.Signature.Deref()
-
-	var out Signature
-	copy(out[:], resp.Signature.Inner[:])
-	return &out
+	return cgo.Aggregate(cgo.AsSliceRefUint8(flattenedSignatures))
 }
 
 // PrivateKeyGenerate generates a private key
 func PrivateKeyGenerate() PrivateKey {
-	resp := generated.FilPrivateKeyGenerate()
-	resp.Deref()
-	resp.PrivateKey.Deref()
-	defer generated.FilDestroyPrivateKeyGenerateResponse(resp)
-
-	var out PrivateKey
-	copy(out[:], resp.PrivateKey.Inner[:])
-	return out
+	key := cgo.PrivateKeyGenerate()
+	if key == nil {
+		return PrivateKey{}
+	}
+	return *key
 }
 
-// PrivateKeyGenerate generates a private key in a predictable manner
+// PrivateKeyGenerate generates a private key in a predictable manner.
 func PrivateKeyGenerateWithSeed(seed PrivateKeyGenSeed) PrivateKey {
-	var ary generated.Fil32ByteArray
-	copy(ary.Inner[:], seed[:])
-
-	resp := generated.FilPrivateKeyGenerateWithSeed(ary)
-	resp.Deref()
-	resp.PrivateKey.Deref()
-	defer generated.FilDestroyPrivateKeyGenerateResponse(resp)
-
-	var out PrivateKey
-	copy(out[:], resp.PrivateKey.Inner[:])
-	return out
+	ary := cgo.AsByteArray32(seed[:])
+	key := cgo.PrivateKeyGenerateWithSeed(&ary)
+	if key == nil {
+		return PrivateKey{}
+	}
+	return *key
 }
 
 // PrivateKeySign signs a message
 func PrivateKeySign(privateKey PrivateKey, message Message) *Signature {
-	resp := generated.FilPrivateKeySign(privateKey[:], message, uint(len(message)))
-	resp.Deref()
-	resp.Signature.Deref()
-
-	defer generated.FilDestroyPrivateKeySignResponse(resp)
-
-	var signature Signature
-	copy(signature[:], resp.Signature.Inner[:])
-	return &signature
+	return cgo.PrivateKeySign(cgo.AsSliceRefUint8(privateKey[:]), cgo.AsSliceRefUint8(message))
 }
 
 // PrivateKeyPublicKey gets the public key for a private key
-func PrivateKeyPublicKey(privateKey PrivateKey) PublicKey {
-	resp := generated.FilPrivateKeyPublicKey(privateKey[:])
-	resp.Deref()
-	resp.PublicKey.Deref()
-
-	defer generated.FilDestroyPrivateKeyPublicKeyResponse(resp)
-
-	var publicKey PublicKey
-	copy(publicKey[:], resp.PublicKey.Inner[:])
-	return publicKey
+func PrivateKeyPublicKey(privateKey PrivateKey) *PublicKey {
+	return cgo.PrivateKeyPublicKey(cgo.AsSliceRefUint8(privateKey[:]))
 }
 
 // CreateZeroSignature creates a zero signature, used as placeholder in filecoin.
 func CreateZeroSignature() Signature {
-	resp := generated.FilCreateZeroSignature()
-	resp.Deref()
-	resp.Signature.Deref()
-
-	defer generated.FilDestroyZeroSignatureResponse(resp)
-
-	var sig Signature
-	copy(sig[:], resp.Signature.Inner[:])
-
-	return sig
+	signature := cgo.CreateZeroSignature()
+	if signature == nil {
+		return Signature{}
+	}
+	return *signature
 }

bls_test.go

@@ -51,30 +51,30 @@ func TestBLSSigningAndVerification(t *testing.T) {
 	aggregateSign := Aggregate([]Signature{*fooSignature, *barSignature})
 
 	// assert the foo message was signed with the foo key
-	assert.True(t, Verify(fooSignature, []Digest{fooDigest}, []PublicKey{fooPublicKey}))
+	assert.True(t, Verify(fooSignature, []Digest{fooDigest}, []PublicKey{*fooPublicKey}))
 
 	// assert the bar message was signed with the bar key
-	assert.True(t, Verify(barSignature, []Digest{barDigest}, []PublicKey{barPublicKey}))
+	assert.True(t, Verify(barSignature, []Digest{barDigest}, []PublicKey{*barPublicKey}))
 
 	// assert the foo message was signed with the foo key
-	assert.True(t, HashVerify(fooSignature, []Message{fooMessage}, []PublicKey{fooPublicKey}))
+	assert.True(t, HashVerify(fooSignature, []Message{fooMessage}, []PublicKey{*fooPublicKey}))
 
 	// assert the bar message was signed with the bar key
-	assert.True(t, HashVerify(barSignature, []Message{barMessage}, []PublicKey{barPublicKey}))
+	assert.True(t, HashVerify(barSignature, []Message{barMessage}, []PublicKey{*barPublicKey}))
 
 	// assert the foo message was not signed by the bar key
-	assert.False(t, Verify(fooSignature, []Digest{fooDigest}, []PublicKey{barPublicKey}))
+	assert.False(t, Verify(fooSignature, []Digest{fooDigest}, []PublicKey{*barPublicKey}))
 
 	// assert the bar/foo message was not signed by the foo/bar key
-	assert.False(t, Verify(barSignature, []Digest{barDigest}, []PublicKey{fooPublicKey}))
-	assert.False(t, Verify(barSignature, []Digest{fooDigest}, []PublicKey{barPublicKey}))
-	assert.False(t, Verify(fooSignature, []Digest{barDigest}, []PublicKey{fooPublicKey}))
+	assert.False(t, Verify(barSignature, []Digest{barDigest}, []PublicKey{*fooPublicKey}))
+	assert.False(t, Verify(barSignature, []Digest{fooDigest}, []PublicKey{*barPublicKey}))
+	assert.False(t, Verify(fooSignature, []Digest{barDigest}, []PublicKey{*fooPublicKey}))
 
 	//assert the foo and bar message was signed with the foo and bar key
-	assert.True(t, HashVerify(aggregateSign, []Message{fooMessage, barMessage}, []PublicKey{fooPublicKey, barPublicKey}))
+	assert.True(t, HashVerify(aggregateSign, []Message{fooMessage, barMessage}, []PublicKey{*fooPublicKey, *barPublicKey}))
 
 	//assert the bar and foo message was not signed by the foo and bar key
-	assert.False(t, HashVerify(aggregateSign, []Message{fooMessage, barMessage}, []PublicKey{fooPublicKey}))
+	assert.False(t, HashVerify(aggregateSign, []Message{fooMessage, barMessage}, []PublicKey{*fooPublicKey}))
 }
 
 func BenchmarkBLSVerify(b *testing.B) {
@@ -90,7 +90,7 @@ func BenchmarkBLSVerify(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		if !Verify(sig, []Digest{digest}, []PublicKey{pubk}) {
+		if !Verify(sig, []Digest{digest}, []PublicKey{*pubk}) {
 			b.Fatal("failed to verify")
 		}
 	}
@@ -132,7 +132,7 @@ func benchmarkBLSVerifyBatchSize(size int) func(b *testing.B) {
 			sig := PrivateKeySign(priv, msg)
 			sigs = append(sigs, *sig)
 			pubk := PrivateKeyPublicKey(priv)
-			pubks = append(pubks, pubk)
+			pubks = append(pubks, *pubk)
 		}
 
 		t := time.Now()
@@ -161,7 +161,7 @@ func BenchmarkBLSHashAndVerify(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		digest := Hash(msg)
-		if !Verify(sig, []Digest{digest}, []PublicKey{pubk}) {
+		if !Verify(sig, []Digest{digest}, []PublicKey{*pubk}) {
 			b.Fatal("failed to verify")
 		}
 	}
@@ -179,7 +179,7 @@ func BenchmarkBLSHashVerify(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		if !HashVerify(sig, []Message{msg}, []PublicKey{pubk}) {
+		if !HashVerify(sig, []Message{msg}, []PublicKey{*pubk}) {
 			b.Fatal("failed to verify")
 		}
 	}

build.sh

@@ -9,9 +9,6 @@ rustup target add x86_64-apple-darwin --toolchain $(cat rust-toolchain)
 rustup target add aarch64-apple-darwin --toolchain $(cat rust-toolchain)
 cargo update -p "filecoin-proofs-api"
 cargo install cargo-lipo
-cargo install cbindgen
-cbindgen --clean --config cbindgen.toml --crate filcrypto --output ../include/filcrypto.h
 cd ..
 FFI_BUILD_FROM_SOURCE=1 make
-make cgo-gen
 go mod tidy

cgo/bls.go

@@ -0,0 +1,61 @@
+package cgo
+
+/*
+#cgo LDFLAGS: -L${SRCDIR}/..
+#cgo pkg-config: ${SRCDIR}/../filcrypto.pc
+#include "../filcrypto.h"
+#include <stdlib.h>
+*/
+import "C"
+
+func Hash(message SliceRefUint8) *[96]byte {
+	resp := C.hash(message)
+	defer resp.destroy()
+	return resp.copyAsArray()
+}
+
+func Aggregate(flattenedSignatures SliceRefUint8) *[96]byte {
+	resp := C.aggregate(flattenedSignatures)
+	defer resp.destroy()
+	return resp.copyAsArray()
+}
+
+func Verify(signature SliceRefUint8, flattenedDigests SliceRefUint8, flattenedPublicKeys SliceRefUint8) bool {
+	resp := C.verify(signature, flattenedDigests, flattenedPublicKeys)
+	return bool(resp)
+}
+
+func HashVerify(signature SliceRefUint8, flattenedMessages SliceRefUint8, messageSizes SliceRefUint, flattenedPublicKeys SliceRefUint8) bool {
+	resp := C.hash_verify(signature, flattenedMessages, messageSizes, flattenedPublicKeys)
+	return bool(resp)
+}
+
+func PrivateKeyGenerate() *[32]byte {
+	resp := C.private_key_generate()
+	defer resp.destroy()
+	return resp.copyAsArray()
+}
+
+func PrivateKeyGenerateWithSeed(rawSeed *ByteArray32) *[32]byte {
+	resp := C.private_key_generate_with_seed(rawSeed)
+	defer resp.destroy()
+	return resp.copyAsArray()
+}
+
+func PrivateKeySign(rawPrivateKey SliceRefUint8, message SliceRefUint8) *[96]byte {
+	resp := C.private_key_sign(rawPrivateKey, message)
+	defer resp.destroy()
+	return resp.copyAsArray()
+}
+
+func PrivateKeyPublicKey(rawPrivateKey SliceRefUint8) *[48]byte {
+	resp := C.private_key_public_key(rawPrivateKey)
+	defer resp.destroy()
+	return resp.copyAsArray()
+}
+
+func CreateZeroSignature() *[96]byte {
+	resp := C.create_zero_signature()
+	defer resp.destroy()
+	return resp.copyAsArray()
+}