Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions pkg/frost/signing/native_adapter_registration_frost_native.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,40 @@ type buildTaggedNativeExecutionAdapter struct {
}

func registerNativeExecutionAdapterForBuild() {
// Registration errors are surfaced via `LastNativeRegistrationError()`
// rather than panicking, so a transient registration failure at init time
// does not crash the binary. `currentNativeExecutionBackend()` already
// reports `ErrNativeCryptographyUnavailable` when no native adapter is
// registered, which keeps the legacy execution backend as the safe-by-
// default fallback.
err := RegisterNativeExecutionBridge(newBuildTaggedNativeExecutionBridge())
if err != nil {
panic(fmt.Sprintf("failed to register build-tagged native bridge: [%v]", err))
registrationLogger.Warnf(
"failed to register build-tagged native bridge: [%v]; "+
"native execution will report unavailable and the legacy "+
"execution backend remains the safe-by-default path",
err,
)
setLastRegistrationError(fmt.Errorf(
"failed to register build-tagged native bridge: [%w]",
err,
))
return
}

err = RegisterNativeExecutionAdapter(newBuildTaggedNativeExecutionAdapter())
if err != nil {
panic(fmt.Sprintf("failed to register build-tagged native adapter: [%v]", err))
registrationLogger.Warnf(
"failed to register build-tagged native adapter: [%v]; "+
"native execution will report unavailable and the legacy "+
"execution backend remains the safe-by-default path",
err,
)
setLastRegistrationError(fmt.Errorf(
"failed to register build-tagged native adapter: [%w]",
err,
))
return
}
}

Expand Down
53 changes: 50 additions & 3 deletions pkg/frost/signing/native_ffi_primitive_registration.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,36 @@
package signing

import "fmt"
import (
"fmt"
"sync"

"github.com/ipfs/go-log/v2"
)

var (
registrationLogger = log.Logger("keep-frost-signing-registration")
registrationErrorMu sync.RWMutex
lastRegistrationError error
)

func setLastRegistrationError(err error) {
registrationErrorMu.Lock()
defer registrationErrorMu.Unlock()
lastRegistrationError = err
}

// LastNativeRegistrationError returns the most recent error observed while
// registering build-tagged native FROST execution adapters or FFI signing
// primitives. It is nil when the most recent registration attempt succeeded
// or when no registration has been attempted yet. Callers that want to fail
// startup on a registration error should check this after invoking
// `RegisterNativeExecutionAdapterForBuild` rather than relying on the
// previously panicking registration helpers themselves.
func LastNativeRegistrationError() error {
registrationErrorMu.RLock()
defer registrationErrorMu.RUnlock()
return lastRegistrationError
}

// NativeExecutionFFISigningPrimitiveProviderForBuild produces a native FFI
// signing primitive for the current build/runtime flavor.
Expand Down Expand Up @@ -48,12 +78,29 @@ func currentNativeExecutionFFISigningPrimitiveProviderForBuild() NativeExecution
//
// On default builds, this is a no-op.
// On `frost_native` builds, this can be wired to a concrete primitive.
//
// Registration errors are surfaced via `LastNativeRegistrationError()` rather
// than panicking, so a transient FFI lookup failure at init time does not
// crash the binary. Downstream code in `pkg/frost/signing/backend.go` already
// handles the absence of a registered native adapter through
// `ErrNativeCryptographyUnavailable`, so the legacy execution backend remains
// the safe-by-default path even when this registration fails.
func RegisterNativeExecutionFFISigningPrimitiveForBuild() {
err := registerNativeExecutionFFISigningPrimitiveForBuild()
if err != nil {
panic(fmt.Sprintf(
"failed to register build-tagged native FFI signing primitive: [%v]",
registrationLogger.Warnf(
"failed to register build-tagged native FFI signing primitive: [%v]; "+
"the native execution backend will report unavailable and callers "+
"that selected the legacy or native-with-fallback backend will "+
"continue using the legacy bridge",
err,
)
setLastRegistrationError(fmt.Errorf(
"failed to register build-tagged native FFI signing primitive: [%w]",
err,
))
return
}

setLastRegistrationError(nil)
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,14 @@ func TestRegisterNativeExecutionFFISigningPrimitiveForBuild_UsesDefaultProvider(
}
}

func TestRegisterNativeExecutionFFISigningPrimitiveForBuild_ProviderErrorPanics(
func TestRegisterNativeExecutionFFISigningPrimitiveForBuild_ProviderErrorIsRecordedNotPanicked(
t *testing.T,
) {
UnregisterNativeExecutionFFISigningPrimitiveProviderForBuild()
UnregisterNativeExecutionFFIExecutor()
t.Cleanup(UnregisterNativeExecutionFFISigningPrimitiveProviderForBuild)
t.Cleanup(UnregisterNativeExecutionFFIExecutor)
t.Cleanup(func() { setLastRegistrationError(nil) })

expectedErr := errors.New("provider error")

Expand All @@ -71,24 +72,35 @@ func TestRegisterNativeExecutionFFISigningPrimitiveForBuild_ProviderErrorPanics(
}

defer func() {
recovered := recover()
if recovered == nil {
t.Fatal("expected panic")
}

recoveredError, ok := recovered.(string)
if !ok {
t.Fatalf("unexpected panic type: [%T]", recovered)
}

if !strings.Contains(recoveredError, expectedErr.Error()) {
if recovered := recover(); recovered != nil {
t.Fatalf(
"unexpected panic value\nexpected substring: [%s]\nactual: [%v]",
expectedErr.Error(),
"registration must not panic; recovered: [%v]",
recovered,
)
}
}()

// Pre-condition: the registration error slot is clear before invoking the
// helper, so any non-nil error after the call is from this attempt.
setLastRegistrationError(nil)

RegisterNativeExecutionFFISigningPrimitiveForBuild()

registered := LastNativeRegistrationError()
if registered == nil {
t.Fatal("expected LastNativeRegistrationError to surface the provider error")
}
if !strings.Contains(registered.Error(), expectedErr.Error()) {
t.Fatalf(
"LastNativeRegistrationError missing expected substring\nexpected: [%s]\nactual: [%v]",
expectedErr.Error(),
registered,
)
}

if currentNativeExecutionFFIExecutor() != nil {
t.Fatal(
"FFI executor must not be registered when the provider returned an error",
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,14 @@ func parseBuildTaggedTBTCSignerResult(
operation string,
result C.TbtcSignerResult,
) ([]byte, error) {
defer C.tbtc_signer_free_buffer(result.buffer.ptr, result.buffer.len)
// The C wrapper guards against a missing `frost_tbtc_free_buffer` symbol
// but not against a NULL buffer pointer. Status code -1 paths (FFI lib
// unavailable) and any future path that returns an empty buffer can leave
// `result.buffer.ptr == nil`, so skip the deferred free in that case to
// avoid handing a NULL pointer to Rust's `frost_tbtc_free_buffer`.
if result.buffer.ptr != nil {
defer C.tbtc_signer_free_buffer(result.buffer.ptr, result.buffer.len)
}

statusCode := int32(result.status_code)

Expand Down
29 changes: 29 additions & 0 deletions pkg/tbtc/signer_material_encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,21 @@ import (

var signerMaterialEnvelopePrefix = []byte("tbtc-signer-material-v1:")

// signerMaterialMaxFormatLength bounds the length of the format identifier in
// a serialized signer-material envelope. Real format identifiers are short
// labels like "frost-tbtc-signer-v1", so 256 bytes is generous; the cap exists
// to refuse a uvarint-claimed length that would allocate a huge string from a
// hostile or corrupted payload before the existing `offset+int(formatLength) >
// len(data)` bounds check runs.
const signerMaterialMaxFormatLength uint64 = 256

// signerMaterialMaxPayloadLength bounds the length of the payload body. JSON
// envelopes for FROST and the tBTC-signer key material carry tens of KiB of
// hex; 256 KiB is comfortably above that and refuses a uvarint-claimed length
// that would allocate hundreds of MiB from a corrupted state file or a
// hostile peer.
const signerMaterialMaxPayloadLength uint64 = 256 * 1024

type unmarshaledSignerMaterial struct {
signerMaterial any
privateKeyShare *tecdsa.PrivateKeyShare
Expand Down Expand Up @@ -154,6 +169,13 @@ func decodeNativeSignerMaterialFromPersistence(
if err != nil {
return nil, true, fmt.Errorf("invalid signer material format length: [%w]", err)
}
if formatLength > signerMaterialMaxFormatLength {
return nil, true, fmt.Errorf(
"signer material format length %d exceeds maximum %d",
formatLength,
signerMaterialMaxFormatLength,
)
}
offset += lengthBytes

if offset+int(formatLength) > len(data) {
Expand All @@ -167,6 +189,13 @@ func decodeNativeSignerMaterialFromPersistence(
if err != nil {
return nil, true, fmt.Errorf("invalid signer material payload length: [%w]", err)
}
if payloadLength > signerMaterialMaxPayloadLength {
return nil, true, fmt.Errorf(
"signer material payload length %d exceeds maximum %d",
payloadLength,
signerMaterialMaxPayloadLength,
)
}
offset += lengthBytes

if offset+int(payloadLength) > len(data) {
Expand Down
59 changes: 59 additions & 0 deletions pkg/tbtc/signer_material_encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tbtc

import (
"bytes"
"encoding/binary"
"reflect"
"strings"
"testing"
Expand All @@ -14,6 +15,16 @@ import (
"google.golang.org/protobuf/proto"
)

// appendUvarintForTest emits a uvarint matching the format
// `unmarshalSignerMaterialFromPersistence` expects. It is duplicated in the
// test package rather than exported so test-only construction of corrupted
// envelopes cannot accidentally be reused by production code.
func appendUvarintForTest(buf []byte, value uint64) []byte {
var scratch [binary.MaxVarintLen64]byte
n := binary.PutUvarint(scratch[:], value)
return append(buf, scratch[:n]...)
}

func TestMarshalSignerMaterialForPersistence_LegacyPrivateKeyShare(t *testing.T) {
signer := createMockSigner(t)

Expand Down Expand Up @@ -171,6 +182,54 @@ func TestUnmarshalSignerMaterialFromPersistence_CorruptedNativeEnvelope(t *testi
}
}

func TestUnmarshalSignerMaterialFromPersistence_RejectsOversizedFormatLength(
t *testing.T,
) {
// Build an envelope that claims a format length one byte above the cap.
// The body itself is short, so without the length cap the bounds check
// would still catch this, but the cap rejects the claim earlier and with
// a clear error before any allocation.
encoded := append([]byte{}, signerMaterialEnvelopePrefix...)
encoded = appendUvarintForTest(encoded, signerMaterialMaxFormatLength+1)
encoded = append(encoded, []byte("ignored")...)

_, err := unmarshalSignerMaterialFromPersistence(encoded)
if err == nil {
t.Fatal("expected unmarshal error")
}

if !strings.Contains(err.Error(), "format length") ||
!strings.Contains(err.Error(), "exceeds maximum") {
t.Fatalf(
"unexpected unmarshal error\nexpected substrings: [format length], [exceeds maximum]\nactual: [%v]",
err,
)
}
}

func TestUnmarshalSignerMaterialFromPersistence_RejectsOversizedPayloadLength(
t *testing.T,
) {
encoded := append([]byte{}, signerMaterialEnvelopePrefix...)
format := []byte(frostsigning.NativeSignerMaterialFormatFrostUniFFIV1)
encoded = appendUvarintForTest(encoded, uint64(len(format)))
encoded = append(encoded, format...)
encoded = appendUvarintForTest(encoded, signerMaterialMaxPayloadLength+1)

_, err := unmarshalSignerMaterialFromPersistence(encoded)
if err == nil {
t.Fatal("expected unmarshal error")
}

if !strings.Contains(err.Error(), "payload length") ||
!strings.Contains(err.Error(), "exceeds maximum") {
t.Fatalf(
"unexpected unmarshal error\nexpected substrings: [payload length], [exceeds maximum]\nactual: [%v]",
err,
)
}
}

func TestMarshalSignerMaterialForPersistence_UnsupportedType(t *testing.T) {
_, err := marshalSignerMaterialForPersistence(struct{}{}, nil)
if err == nil {
Expand Down
Loading