cloudflare / cloudflare/boringtun

Failing to set private key before peers makes socket unresponsive

Open
#163 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
7.2k
Forks
532
PR merge metrics
No merged PRs in 30d

Description

If `boringtun` is given peer settings without the private key being set, it will panic and the socket will end session without providing a response. Subsequent requests to the socket yield no response.

To reproduce:

1. Run a `boringtun`:
```
RUST_BACKTRACE=1 boringtun myvpn -v debug -l /dev/stdout -f
Feb 08 02:43:49.373 INFO BoringTun started successfully
```

2. Check socket works:
```
socat - UNIX-CONNECT:/var/run/wireguard/myvpn.sock
get=1
listen_port=51601
errno=0
```

3. Issue a `set` without the private key:
```
socat - UNIX-CONNECT:/var/run/wireguard/myvpn.sock
set=1
listen_port=12345
public_key=6040ad1d02595ad318d7b54673de74361d7cce0a1a617da5b851031243abe67a
endpoint=1.2.3.4:5678

```

4. Observe no response from `set`, and observe error:
```
thread '' panicked at 'Private key must be set first', src/device/mod.rs:337:14
stack backtrace:
0: rust_begin_unwind
at /rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca/library/std/src/panicking.rs:495:5
1: core::panicking::panic_fmt
at /rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca/library/core/src/panicking.rs:92:14
2: core::option::expect_failed
at /rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca/library/core/src/option.rs:1260:5
3: boringtun::device::Device::update_peer
4: boringtun::device::api::api_set_peer
5: boringtun::device::dev_lock::LockReadGuard::try_writeable
6: boringtun::device::api::>::register_api_handler::{{closure}}
7: boringtun::device::DeviceHandle::event_loop
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
```

5. Subsequent socket interaction yields nothing:
```
socat - UNIX-CONNECT:/var/run/wireguard/myvpn.sock
get=1

```

It seems that `private_key` could have been set in a previous `set`, so it's not necessary for it to be there. But, if it isn't previously set, an appropriate `errno` response should come back and avoid the panic.

Contributor guide

Open the contributing guide

Research direction

Start at src/device/mod.rs:337 and trace the update_peer, api_set_peer, and register_api_handler frames shown in the backtrace. Reproduce the set request without private_key, then verify that it returns an appropriate errno response, does not panic, and leaves subsequent socket requests responsive.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.