alloy-rs / alloy-rs/alloy

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

Offen
#4,127 2 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Rust
Sterne
1.3k
Forks
668
Ø Merge
2 T. 1 Std.
Gemergte PRs (30 T.)
29

Beschreibung

### 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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

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.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
rust
Bereich
blockchain, cryptography, security
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
48/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.