alloy-rs / alloy-rs/alloy

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

未关闭
#4,127 2 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
Rust
星标
1.3k
派生
668
平均合并
2 天 2 小时
30 天内合并 PR
29

描述

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

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。