fix(config): don't panic on a corrupt config, and don't leave keys wo… - #63
Open
Aporis3674 wants to merge 1 commit into
Open
Conversation
…rld-readable
Loading an identity went through `impl From<PersistedIdentity> for Identity`,
which decoded the two WireGuard keys with `.expect()` and then
`copy_from_slice()` into `[u8; 32]`. Both panic on input the user can
easily end up with:
$ cargo test bug_repro -- --nocapture
panicked at src/config.rs:55:24:
copy_from_slice: source slice length (20) does not match destination
slice length (32)
panicked at src/config.rs:46:14: # non-base64 value
`config::save` writes with `std::fs::write`, which truncates in place, so a
process killed mid-save leaves exactly that truncated file — and every later
start panics instead of re-registering. `config::load` already returns
`Result`, so the caller is prepared for an error; it cannot catch a panic.
The `client_id` field a few lines below is length-checked, which is the
pattern the two key fields were missing.
- `TryFrom` replaces `From`, and `decode_key` names the offending field:
"config: wg_private_key decodes to 20 bytes, expected 32"
- the config holds a WireGuard private key, a MASQUE client private key and
an access token, and was created with the process umask — 0644 on a
typical Linux box. It is now written to a temp file, chmod 0600, then
renamed, which fixes the world-readable mode and makes the write atomic
so the truncation cannot happen in the first place
- five tests cover both malformed shapes, an oversized key, the happy path
and the resulting file mode
Aporis3674
force-pushed
the
fix/config-corrupt-panic-and-key-permissions
branch
from
July 26, 2026 21:15
ce7e655 to
8e6593b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two problems in
aether/src/config.rs, both reached through the same code path.1. A corrupt config panics instead of erroring
config::loadreturnsResult, but the conversion it calls does not:Both lines panic on input a user can end up with. Reproduced against
ee5a5f5:How you get there.
config::saveusesstd::fs::write, which truncates inplace — so a process killed mid-save leaves exactly that truncated file, and
every later start panics instead of re-registering. The user has to work out on
their own that deleting
aether.tomlfixes it. The caller is already preparedfor an error; it cannot catch a panic.
client_id, a few lines below, is length-checked. These two fields justmissed the same guard.
2. The key file is world-readable
aether.tomlholds the WireGuard private key, the MASQUE client private key andthe access token.
std::fs::writecreates it with the process umask —0644ona typical Linux box — and there is no permission handling anywhere in the tree:
The change
TryFromreplacesFrom, anddecode_keynames the field that is wrong:config: wg_private_key decodes to 20 bytes, expected 32chmod 0600, then renamed. That fixesthe mode and makes the write atomic, so the truncation behind the panic
can no longer happen in the first place
resulting file mode
config.rsis left rustfmt-clean. No behaviour change for a valid config.Generated by Claude Code