fix: isolate native binding errors by OS thread

go-bindings/blschia.cpp

@@ -18,7 +18,7 @@
 #include "error.h"
 #include "blschia.h"
 
-std::string gErrMsg;
+thread_local std::string gErrMsg;
 
 void SecFree(void *p) {
     bls::Util::SecFree(p);

go-bindings/blschia.h

@@ -38,6 +38,8 @@ uint8_t* SecAllocBytes(size_t len);
 
 void* GetAddressAtIndex(uint8_t *ptr, int index);
 
+// Returns the last error on the calling OS thread. The borrowed pointer remains
+// valid until the next failure on that thread or until the thread exits.
 const char* GetLastErrorMsg();
 
 #ifdef __cplusplus

go-bindings/elements.go

@@ -34,6 +34,8 @@ type G1Element struct {
 // G1ElementFromBytes returns a new G1Element (public-key) from bytes
 // this method allocates the new bls::G2Element object and keeps its pointer
 func G1ElementFromBytes(data []byte) (*G1Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cBytesPtr := C.CBytes(data)
 	defer C.free(cBytesPtr)
 	var cDidErr C.bool
@@ -125,6 +127,8 @@ type G2Element struct {
 // G2ElementFromBytes returns a new G2Element (signature) from passed byte slice
 // this method allocates the new bls::G2Element object and keeps its pointer
 func G2ElementFromBytes(data []byte) (*G2Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cBytesPtr := C.CBytes(data)
 	defer C.free(cBytesPtr)
 	var cDidErr C.bool

go-bindings/error.go

@@ -18,6 +18,8 @@ package blschia
 import "C"
 import "errors"
 
+// The caller must remain locked to its OS thread from the native failure until
+// this function has copied the thread-local error message.
 func errFromC() error {
 	return errors.New(C.GoString(C.GetLastErrorMsg()))
 }

go-bindings/error.h

@@ -16,6 +16,6 @@
 #define GO_BINDINGS_ERROR_H_
 #include <string>
 
-extern std::string gErrMsg;
+extern thread_local std::string gErrMsg;
 
 #endif  // GO_BINDINGS_ERROR_H_

go-bindings/error_test.go

@@ -0,0 +1,39 @@
+package blschia
+
+import (
+	"runtime"
+	"testing"
+)
+
+func TestErrorMessageThreadOwnership(t *testing.T) {
+	firstWritten := make(chan struct{})
+	secondWritten := make(chan struct{})
+	firstResult := make(chan [2]error, 1)
+	secondResult := make(chan error, 1)
+
+	go func() {
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+		_, before := G1ElementFromBytes([]byte{0})
+		close(firstWritten)
+		<-secondWritten
+		firstResult <- [2]error{before, errFromC()}
+	}()
+	go func() {
+		<-firstWritten
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+		_, err := G2ElementFromBytes([]byte{0})
+		secondResult <- err
+		close(secondWritten)
+	}()
+
+	for i, err := range <-firstResult {
+		if err == nil || err.Error() != "G1Element::FromBytes: Invalid size" {
+			t.Errorf("first thread error %d = %v", i, err)
+		}
+	}
+	if err := <-secondResult; err == nil || err.Error() != "G2Element::FromBytes: Invalid size" {
+		t.Errorf("second thread error = %v", err)
+	}
+}

go-bindings/privatekey.go

@@ -32,6 +32,8 @@ type PrivateKey struct {
 // PrivateKeyFromBytes returns a new PrivateKey from bytes
 // this method allocates the new bls::PrivateKey object and keeps its pointer
 func PrivateKeyFromBytes(data []byte, modOrder bool) (*PrivateKey, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cBytesPtr := cAllocBytes(data)
 	defer C.SecFree(cBytesPtr)
 	var cDidErr C.bool
@@ -48,6 +50,8 @@ func PrivateKeyFromBytes(data []byte, modOrder bool) (*PrivateKey, error) {
 // G1Element returns a G1Element (public key) using a state of the current private key
 // this method is a binding of the bls::PrivateKey::G1Element
 func (sk *PrivateKey) G1Element() (*G1Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	var cDidErr C.bool
 	el := G1Element{
 		val: C.CPrivateKeyGetG1Element(sk.val, &cDidErr),
@@ -63,6 +67,8 @@ func (sk *PrivateKey) G1Element() (*G1Element, error) {
 // G2Element returns a G2Element (signature) using a state of the current private key
 // this method is a binding of the bls::PrivateKey::G2Element
 func (sk *PrivateKey) G2Element() (*G2Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	var cDidErr C.bool
 	el := G2Element{
 		val: C.CPrivateKeyGetG2Element(sk.val, &cDidErr),

go-bindings/schemes.go

@@ -73,6 +73,8 @@ type coreMPL struct {
 // KeyGen returns a new generated PrivateKey using passed a genSeed data
 // this method is a binding of bls::CoreMPL::KeyGen
 func (s *coreMPL) KeyGen(seed []byte) (*PrivateKey, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cSeedPtr := C.CBytes(seed)
 	defer C.free(cSeedPtr)
 	var cDidErr C.bool

go-bindings/threshold.go

@@ -64,6 +64,8 @@ func BuildSignHash(llmqType uint8, quorumHash Hash, signID Hash, msgHash Hash) H
 // from a set of PrivateKey(s) and a hash
 // this function is a binding of bls::Threshold::PrivateKeyShare
 func ThresholdPrivateKeyShare(sks []*PrivateKey, hash Hash) (*PrivateKey, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cHashPtr := C.CBytes(hash[:])
 	defer C.free(cHashPtr)
 	cArrPtr := cAllocPrivKeys(sks...)
@@ -84,6 +86,8 @@ func ThresholdPrivateKeyShare(sks []*PrivateKey, hash Hash) (*PrivateKey, error)
 // from a set of G1Element(s) and a hash
 // this function is a binding of bls::Threshold::PublicKeyShare
 func ThresholdPublicKeyShare(pks []*G1Element, hash Hash) (*G1Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cHashPtr := C.CBytes(hash[:])
 	defer C.free(cHashPtr)
 	cArrPtr := cAllocPubKeys(pks...)
@@ -104,6 +108,8 @@ func ThresholdPublicKeyShare(pks []*G1Element, hash Hash) (*G1Element, error) {
 // from a set of G2Element(s) and a hash
 // this function is a binding of bls::Threshold::SignatureShare
 func ThresholdSignatureShare(sigs []*G2Element, hash Hash) (*G2Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cHashPtr := C.CBytes(hash[:])
 	defer C.free(cHashPtr)
 	cArrPtr := cAllocSigs(sigs...)
@@ -124,6 +130,8 @@ func ThresholdSignatureShare(sigs []*G2Element, hash Hash) (*G2Element, error) {
 // from the set of shared PrivateKey(s) with a list of the hashes
 // this function is a binding of bls::Threshold::PrivateKeyRecover
 func ThresholdPrivateKeyRecover(sks []*PrivateKey, hashes []Hash) (*PrivateKey, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cArrPtr := cAllocPrivKeys(sks...)
 	defer C.FreePtrArray(cArrPtr)
 	cHashArrPtr := cAllocHashes(hashes)
@@ -150,6 +158,8 @@ func ThresholdPrivateKeyRecover(sks []*PrivateKey, hashes []Hash) (*PrivateKey,
 // from the set of shared G1Element(s) with a list of the hashes
 // this function is a binding of bls::Threshold::PublicKeyRecover
 func ThresholdPublicKeyRecover(pks []*G1Element, hashes []Hash) (*G1Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cArrPtr := cAllocPubKeys(pks...)
 	defer C.FreePtrArray(cArrPtr)
 	cHashArrPtr := cAllocHashes(hashes)
@@ -176,6 +186,8 @@ func ThresholdPublicKeyRecover(pks []*G1Element, hashes []Hash) (*G1Element, err
 // from the set of shared G2Element(s) with a list of the hashes
 // this function is a binding of bls::Threshold::SignatureRecover
 func ThresholdSignatureRecover(sigs []*G2Element, hashes []Hash) (*G2Element, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	cArrPtr := cAllocSigs(sigs...)
 	defer C.FreePtrArray(cArrPtr)
 	cHashArrPtr := cAllocHashes(hashes)

rust-bindings/bls-dash-sys/bindings.rs

@@ -116,6 +116,8 @@ extern "C" {
         index: ::std::os::raw::c_int,
     ) -> *mut ::std::os::raw::c_void;
 
+    /// Returns the last error on the calling OS thread. The borrowed pointer remains
+    /// valid until the next failure on that thread or until the thread exits.
     pub fn GetLastErrorMsg() -> *const ::std::os::raw::c_char;
 
     pub fn CoreMPLKeyGen(

rust-bindings/bls-dash-sys/c-bindings/blschia.cpp

@@ -20,7 +20,7 @@
 
 // TODO: Revisit
 
-std::string gErrMsg;
+thread_local std::string gErrMsg;
 
 void SecFree(void *p) {
     bls::Util::SecFree(p);

rust-bindings/bls-dash-sys/c-bindings/blschia.h

@@ -38,6 +38,8 @@ uint8_t* SecAllocBytes(size_t len);
 
 void* GetAddressAtIndex(uint8_t *ptr, int index);
 
+// Returns the last error on the calling OS thread. The borrowed pointer remains
+// valid until the next failure on that thread or until the thread exits.
 const char* GetLastErrorMsg();
 
 #ifdef __cplusplus

rust-bindings/bls-dash-sys/c-bindings/error.h

@@ -16,6 +16,6 @@
 #define ERROR_H_
 #include <string>
 
-extern std::string gErrMsg;
+extern thread_local std::string gErrMsg;
 
 #endif  // ERROR_H_

rust-bindings/bls-dash-sys/tests/error_messages.rs

@@ -0,0 +1,87 @@
+use bls_dash_sys as sys;
+use std::{ffi::CStr, sync::mpsc, thread};
+
+fn last_error_message() -> String {
+    unsafe { CStr::from_ptr(sys::GetLastErrorMsg()) }
+        .to_string_lossy()
+        .into_owned()
+}
+
+#[test]
+fn error_message_thread_ownership() {
+    let (first_written_tx, first_written_rx) = mpsc::channel();
+    let (second_written_tx, second_written_rx) = mpsc::channel();
+
+    let first = thread::spawn(move || {
+        let invalid = [0u8];
+        let mut did_err = false;
+        let element = unsafe {
+            sys::G1ElementFromBytes(
+                invalid.as_ptr() as *const _,
+                invalid.len(),
+                false,
+                &mut did_err,
+            )
+        };
+        assert!(did_err);
+        assert!(element.is_null());
+        let before = last_error_message();
+        assert_eq!(before, "G1Element::FromBytes: Invalid size");
+        first_written_tx.send(()).unwrap();
+        second_written_rx.recv().unwrap();
+        assert_eq!(last_error_message(), before);
+
+        let mut valid = [0u8; 48];
+        valid[0] = 0xc0;
+        let element = unsafe {
+            sys::G1ElementFromBytes(valid.as_ptr() as *const _, valid.len(), false, &mut did_err)
+        };
+        assert!(!did_err);
+        assert!(!element.is_null());
+        assert!(unsafe { sys::G1ElementIsValid(element) });
+        unsafe { sys::G1ElementFree(element) };
+
+        let key = unsafe {
+            sys::PrivateKeyFromBytes(
+                invalid.as_ptr() as *const _,
+                invalid.len(),
+                false,
+                &mut did_err,
+            )
+        };
+        assert!(did_err);
+        assert!(key.is_null());
+        assert_eq!(last_error_message(), "PrivateKey::FromBytes: Invalid size");
+        assert_eq!(before, "G1Element::FromBytes: Invalid size");
+    });
+    let second = thread::spawn(move || {
+        first_written_rx.recv().unwrap();
+        let invalid = [0u8];
+        let mut did_err = false;
+        let element = unsafe {
+            sys::G2ElementFromBytes(
+                invalid.as_ptr() as *const _,
+                invalid.len(),
+                false,
+                &mut did_err,
+            )
+        };
+        assert!(did_err);
+        assert!(element.is_null());
+        assert_eq!(last_error_message(), "G2Element::FromBytes: Invalid size");
+        second_written_tx.send(()).unwrap();
+
+        let mut valid = [0u8; 96];
+        valid[0] = 0xc0;
+        let element = unsafe {
+            sys::G2ElementFromBytes(valid.as_ptr() as *const _, valid.len(), false, &mut did_err)
+        };
+        assert!(!did_err);
+        assert!(!element.is_null());
+        assert!(unsafe { sys::G2ElementIsValid(element) });
+        unsafe { sys::G2ElementFree(element) };
+    });
+
+    first.join().unwrap();
+    second.join().unwrap();
+}