diff --git a/pkg/frost/signing/native_adapter_registration_frost_native.go b/pkg/frost/signing/native_adapter_registration_frost_native.go index 86c94bd3f4..313971394e 100644 --- a/pkg/frost/signing/native_adapter_registration_frost_native.go +++ b/pkg/frost/signing/native_adapter_registration_frost_native.go @@ -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 } } diff --git a/pkg/frost/signing/native_ffi_primitive_registration.go b/pkg/frost/signing/native_ffi_primitive_registration.go index 18fc204600..516330a56f 100644 --- a/pkg/frost/signing/native_ffi_primitive_registration.go +++ b/pkg/frost/signing/native_ffi_primitive_registration.go @@ -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. @@ -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) } diff --git a/pkg/frost/signing/native_ffi_primitive_registration_frost_native_test.go b/pkg/frost/signing/native_ffi_primitive_registration_frost_native_test.go index 4259c01697..16d9468b2e 100644 --- a/pkg/frost/signing/native_ffi_primitive_registration_frost_native_test.go +++ b/pkg/frost/signing/native_ffi_primitive_registration_frost_native_test.go @@ -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") @@ -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", + ) + } } diff --git a/pkg/frost/signing/native_frost_engine_tbtc_signer_registration_frost_native.go b/pkg/frost/signing/native_frost_engine_tbtc_signer_registration_frost_native.go index b97b693ad1..23aff727c5 100644 --- a/pkg/frost/signing/native_frost_engine_tbtc_signer_registration_frost_native.go +++ b/pkg/frost/signing/native_frost_engine_tbtc_signer_registration_frost_native.go @@ -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) diff --git a/pkg/tbtc/signer_material_encoding.go b/pkg/tbtc/signer_material_encoding.go index f4275896a7..dba6e6e7f4 100644 --- a/pkg/tbtc/signer_material_encoding.go +++ b/pkg/tbtc/signer_material_encoding.go @@ -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 @@ -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) { @@ -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) { diff --git a/pkg/tbtc/signer_material_encoding_test.go b/pkg/tbtc/signer_material_encoding_test.go index 2f83fe87e4..fdef4ccb34 100644 --- a/pkg/tbtc/signer_material_encoding_test.go +++ b/pkg/tbtc/signer_material_encoding_test.go @@ -2,6 +2,7 @@ package tbtc import ( "bytes" + "encoding/binary" "reflect" "strings" "testing" @@ -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) @@ -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 {