alloy-rs / alloy-rs/alloy

[Bug] YubiHSM signer constructors panic and async signing blocks executor threads

Abierto
#4,127 2 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Rust
Estrellas
1.3k
Forks
668
Merge medio
2 d 1 h
PR fusionados (30 d)
29

Descripción

### Component

signers

### What version of Alloy are you on?

`alloy-signer-local` 2.3.0 and current `main` at `85f4d9127965e1edc50851abfa04115186b63c29`.

### Operating System

macOS (Apple Silicon), but the source-level behavior is not OS-specific.

### Describe the bug

`alloy_signer_local::YubiSigner` treats a synchronous hardware signer like an
infallible local software key. This causes two related problems.

#### Fallible operations panic

The YubiHSM `connect`, `new`, and `from_key` constructors return `Self` and call
`unwrap()` on connector, session, key-generation, key-import, and public-key
operations.

For example, this panics when the object does not exist:

```rust
use alloy_signer_local::YubiSigner;
use yubihsm::{Connector, Credentials};

let _signer = YubiSigner::connect(
Connector::mockhsm(),
Credentials::default(),
0x1234,
);
```

Connector failures, authentication failures, missing objects, policy errors,
and device errors are expected runtime conditions. Applications need a
`Result`, not a process panic.

#### Async signing performs blocking I/O on the executor thread

`YubiSigner::connect` accepts a `yubihsm::Connector`. It supports both direct
USB and HTTP through the Yubico `yubihsm-connector` process.

In `yubihsm` 0.42, both connector implementations are synchronous:

- The HTTP connector uses blocking `std::net::TcpStream` reads and writes.
- The USB connector uses blocking `rusb` bulk transfers.

Alloy's blanket asynchronous `Signer` implementation calls `sign_hash_sync`
directly. The asynchronous `TxSigner` implementation also calls
`sign_hash_sync` directly. Both current YubiHSM transports can therefore block
an async executor worker until the connector or device responds.

#### Expected behavior

- Provide fallible YubiHSM constructors. This can use `Result`-returning
constructors or additive `try_connect`, `try_new`, and `try_from_key`
constructors if changing the current signatures is too disruptive.
- Keep the synchronous `SignerSync` and `TxSignerSync` paths.
- Give the current synchronous YubiHSM transports a blocking boundary, such as
`tokio::task::spawn_blocking`, when an application uses the asynchronous
signer traits.
- Do not require blocking-task dispatch for a future connector that provides
genuine asynchronous I/O.
- Return connector, authentication, object, and signing errors through
`alloy_signer::Error` or a YubiHSM-specific error type.

The blocking fix should apply to the hardware-backed YubiHSM path. Ordinary
in-memory local signers do not need blocking-task dispatch.

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Start at alloy_signer_local::YubiSigner and its connect, new, from_key, sign_hash_sync, Signer, and TxSigner implementations. Trace where unwrap() is used for connector/session/key operations and where async signing calls the sync path. Done means YubiHSM failures return errors instead of panicking, async YubiHSM signing has a blocking boundary, and in-memory signers are not changed.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
rust
Área
blockchain, cryptography, security
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Tranquilo
Claridad
Bien especificado
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.