diff options
author | 2021-04-19 00:13:44 +1000 | |
---|---|---|
committer | 2021-04-19 10:46:06 +1000 | |
commit | 84ef9b6dd4303f8be4d27d34faa4da6b09f70941 (patch) | |
tree | b28923f88cf45638c193a37a07d7fa26919fed1a /TODO.md | |
parent | if_wg: import latest wg_noise.{c,h} (diff) | |
download | wireguard-freebsd-84ef9b6dd4303f8be4d27d34faa4da6b09f70941.tar.xz wireguard-freebsd-84ef9b6dd4303f8be4d27d34faa4da6b09f70941.zip |
if_wg: replace wg_tag with wg_packet
`wg_tag` is a source of trouble when it comes to handling mbufs. This is
due to the fact that calls to things like m_prepend may free the mbuf
underneath us, which would be bad if the tag is still queued in the
peer's queue.
`wg_tag` has also been made redundant on other platforms due to size
restrictions (80 bytes on OpenBSD) which means we cannot grow it to the
required size to hold new fields. With wg_packet, this is no longer a
concern.
This patch includes an import of the send/recv paths (from OpenBSD) to
ensure we don't leak an refcounts. This additionally solves two of the
TODOs as well (chop rx padding, don't copy mbuf). The second TODO is
helpful, because we no longer need to allocate mbufs of a specific size
when encrypting, meaning we no longer have an upper bound on the MTU.
(rebase) On second thoughts, that m_defrag is deadly, as it does not
behave the same as m_defrag on OpenBSD. If the packet is large enough,
there will still be multiple clusters, so treating the first mbuf as the
whole buffer may lead to a heap overflow. This is addressed by the
"encrypt mbuf in place" commit, so while is an issue here, it is already
resolved. To say it in caps:
THIS COMMIT INTRODUCES A VULN, FIXED BY: encrypt mbuf in place
There could be some discussion around using p_parallel for the staged
and handshake queues. It isn't as idiomatic as I would like, however the
right structure is there so that is something we could address later.
One other thing to consider is that `wg_peer_send_staged` is likely
being called one packet at a time. Is it worthwhile trying to batch
calls together?
Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
Diffstat (limited to 'TODO.md')
-rw-r--r-- | TODO.md | 5 |
1 files changed, 0 insertions, 5 deletions
@@ -3,15 +3,10 @@ - Finish porting [this script](https://git.zx2c4.com/wireguard-linux/tree/tools/testing/selftests/wireguard/netns.sh) to `./tests/netns.sh` using vnets and epairs. - Rework locking and epoch lifetimes; come up with consistent set of rules. -- Chop off padding on rx after verifying lengths, so that tcpdump doesn't see - zeros. - Shore up vnet support and races/locking around moving between vnets. - Work out `priv_check` from vnet perspective. (There's no `ns_capable()` on FreeBSD, just `capable()`, which makes it a bit weird for one jail to have permissions in another.) -- Resize mbufs once at the beginning, and then encrypt/decrypt in place, rather - than making a new mbuf and copying. (Remember to clear the tags and other - pieces of metadata before passing it off to udp sending or netisr receiving.) - Audit allowedips / radix tree checks, and make sure it's actually behaving as expected. (It might be useful to port [this selftest](https://git.zx2c4.com/wireguard-linux/tree/drivers/net/wireguard/selftest/allowedips.c).) - Make code style consistent with one FreeBSD way, rather than a mix of styles. |