jedisct1 / dsvpn

A Dead Simple VPN.
MIT License
5.16k stars 393 forks source link

charm selftests #68

Closed nnathan closed 4 years ago

nnathan commented 4 years ago

Hi,

Wireguard used to have a selftest that would run on loading the kernel module. It would basically run the crypto primitive a few times to make sure it satisfies a KAT (Known-Answer-Test).

I would like to see the same with dsvpn+charm.

Is this something you would be amenable to? I feel like there's not much enthusiasm for self tests. For example cryptographer software engineer Colin Percival made a diasastrous bug where he removed a line incrementing nonce during a refactor and it was caught by code review. A noisy failure would've been a better choice and this is exactly the type of protection a crypto self test would afford.

I recommend a crypto selftest running for every invocation of dsvpn, usage screen, etc. It is essentially an initialisation assertion.

Is this something you would be amenable to? If so, would you want the feature first implemented in charm and then brought into dsvpn through upstream merge? I'm happy to write the code and figure out the niggly details.

I just want dsvpn to have high assurance and a bit of sensible paranoia :).

jedisct1 commented 4 years ago

Hi!

Test vectors would definitely be a good thing to have in charm, as well as running SAW in CI. Looks like vectorized implementations for x86 and ARM can also now be verified by SAW, so that is something we may want to add.

Considering the size of dsvpn, I'd be a bit reluctant to adding self-tests, that would be larger than the rest of the actual code. Also, unlikely Tarsnap that encrypts data at rest, dsvpn maintains a rolling state for the whole session. If the client and the server don't update the state the exact same way, the session is immediately closed, making the software unusable. That would be quickly detected.

Bugs and regressions are more likely to happen at protocol level, so maybe we should add integration tests instead.