aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
...
* selftests: fixup headersJason A. Donenfeld2021-04-224-12/+14
| | | | | | Also remove the stale entry from the TODO list. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_noise: add selftestMatt Dunwoodie2021-04-224-0/+100
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_cookie: add selftestMatt Dunwoodie2021-04-224-0/+303
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: port allowedips selftest from Linux code and fix bugsJason A. Donenfeld2021-04-223-66/+674
| | | | | | | And then fix broken allowedips implementation for the static unit tests to pass. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_cookie: ensure gc is called regularlyMatt Dunwoodie2021-04-212-29/+44
| | | | | | | | | | | | | | | | | Previously we relied on gc being called when adding a new entry, which could leave us in a gc "blind spot". With this change, we schedule a callout to run gc whenever we have entries in the table. The callout will continue to run every ELEMENT_TIMEOUT seconds until the table is empty. Access to rl_gc is locked by rl_lock, so we will never have any threads racing to callout_{pending,stop,reset}. The alternative (which Linux does currently) is just to run the callout every ELEMENT_TIMEOUT (1) second even when no entries are in the table. However, the callout solution proposed here seems simple enough. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* global: update timer-type commentsJason A. Donenfeld2021-04-202-5/+5
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: cleanup openbsd lock definesJason A. Donenfeld2021-04-204-60/+26
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: use ck for loads/stores, rather than macro mazeJason A. Donenfeld2021-04-204-86/+70
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: move siphash helper out of supportJason A. Donenfeld2021-04-204-32/+24
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* TODO: add a few thingsJason A. Donenfeld2021-04-201-0/+4
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: use sbintime_t consistentlyJason A. Donenfeld2021-04-204-48/+45
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_noise: inline noise_timer_expired to make expensive multiplication go awayJason A. Donenfeld2021-04-201-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: minor code cleanup, improve readabilityMatt Dunwoodie2021-04-211-69/+75
| | | | | | | | | | | Nothing serious here, just use a goto in wg_deliver_{in,out} rather than another if/else indentation. The code should have no functional change, just improve readability. Additionally, use a local `sc` variable rather than `peer->p_sc` in spots. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: unify two state bools to an enumMatt Dunwoodie2021-04-211-14/+16
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* global: use proper boolean typesJason A. Donenfeld2021-04-204-46/+49
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_noise: ensure we check peer count on hashtable insertMatt Dunwoodie2021-04-213-11/+19
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: avoid handshake/keypair type confusionMatt Dunwoodie2021-04-203-7/+16
| | | | | | | | | | | | | | | | | | | | So the last change broke consuming responses, as it may return an invalid remote pointer. Thanks for the catch zx2c4. We just pass a flag "lookup_keypair" which will lookup the keypair when we want (for cookie) and will not when we don't (for consuming responses). It would be possible to merge both noise_remote_index_lookup and noise_keypair_lookup, but the result would probably need to return a void * (for both keypair and remote) or a noise_index * which would need to be cast to the relevant type somewhere. The trickiest thing here would be for if_wg to "put" the result of the function, as it may be a remote or a keypair (which store their refcount in different locations). Perhaps it would return a noise_index * which could contain the refcount for both keypair and remote. It all seems easier to leave them separate. The only argument for combining them would be to reduce duplication of (similar) functions. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: insert/remove peer independent of alloc/destroyMatt Dunwoodie2021-04-203-34/+49
| | | | | | | | | This is needed, to remove the peer from the public key hashtable before calling noise_remote_destroy. This will prevent any incoming handshakes from starting in that time. It also cleans up the insert path to make it more like it was before the wg_noise EPOCH changes. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: assign index without lock then checkMatt Dunwoodie2021-04-201-1/+11
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: remove duplicate peer checkMatt Dunwoodie2021-04-201-5/+1
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: remove unused loadMatt Dunwoodie2021-04-201-1/+0
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: check keypair recvwith after nonceMatt Dunwoodie2021-04-203-38/+31
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: use sbintime_t instead of timespecMatt Dunwoodie2021-04-201-23/+19
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: no need to enter epoch hereMatt Dunwoodie2021-04-201-6/+1
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: whitespace cleanupMatt Dunwoodie2021-04-201-5/+0
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_noise: lookup both keypair and handshake index at onceMatt Dunwoodie2021-04-202-9/+10
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: add missing return parens to follow style(9)Jason A. Donenfeld2021-04-191-25/+25
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: allow v4 xor v6 socket binding to fail with EADDRNOTAVAILJason A. Donenfeld2021-04-191-21/+40
| | | | | | | | | | | This happens if a jail does not have an interface with a configured v4 or v6 address. In that case, we just fall back to only having one socket for the address family that does exist. In the case that neither socket can be created, fail as before. Closes: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=254212 Reported-by: Mark Johnston <markj@FreeBSD.org> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* crypto: chacha and poly in same loopMatt Dunwoodie2021-04-191-92/+58
| | | | | | | | | | This is a fixup of f685f466, where previously we chacha'd in a different loop to poly'ing. Now we do in the same loop to keep the cache hot. In practice this didn't result in an (easily) observable change, which could be due to only having 1-2 mbufs in a chain. However this is still the preferred way to do it. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: fixup wg_mbuf_resetMatt Dunwoodie2021-04-191-2/+0
| | | | | | | Zeroing these values broke TCP recv, so not great, just remove them and hope they don't store anything secret. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: replace %lu with PRIu64Matt Dunwoodie2021-04-191-14/+15
| | | | | | | | While on __LP64__ uint64_t is unsigned long, that is not the case for !__LP64__, which is commonly unsigned long long. Here we use the PRIu64 macro as defined in machine/_inttypes.h. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: fix up bodged wg_mbuf_resetMatt Dunwoodie2021-04-191-3/+3
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: add wg_mbuf_reset to clear metadataMatt Dunwoodie2021-04-191-2/+14
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: replace timer lock with EPOCHMatt Dunwoodie2021-04-191-232/+194
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* crypto: encrypt mbuf in placeMatt Dunwoodie2021-04-195-33/+150
| | | | | | | | | | | This introduces a couple of routines to encrypt the mbufs in place. It is likely that these will be replaced by something in opencrypto, however for the time being this fixes a heap overflow and sets up wg_noise for the "correct" API. When the time comes, this should make it easier to drop in new crypto. It should be noted, this was written at 0500. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: actually use DEFAULT_MTU valueMatt Dunwoodie2021-04-191-2/+2
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: cleanup allowed-ips functionsMatt Dunwoodie2021-04-191-305/+168
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There was a lot of junk that didn't need to be here. This commit cleans it all up and replaces with these three functions: * add * lookup * remove_all We've removed wg_aip_table because the abstraction there is minimal, as well as the fact that t_count was never used. The interface of lookup has changed to provide the af and the address pointer, rather than an mbuf and direction. This simplifies the lookup code, as well as aligning better with the calling locations. We've also changed the list type of `p_aips` from CK_LIST to a regular LIST, as we only modify the list while holding `sc_lock`. Additionally, we keep a count of allowed ips in memory, rather than counting them in wgc_get. While on this though, I think we're safe to remove the panic checks in wgc_get (that the number of peers/allowed ips) have changed underneath us. But I'll leave that for another day. There is a new TODO, which is to check the response of rn_inithead. While at first glance this may appear to be a regression, in actual fact these were never really checked. wg_aip_init would check, and free if appropriate, returning an error - but wg_clone_create would never check the return value of wg_aip_init, so it is possible that t_ip4 and t_ip6 may be NULL in a created interface. The algorithm for removing all allowed ips for a peer should be a lot quicker, that is, we don't need to traverse the entire graph to remove our entries. Instead, we just iterate over the list of entries stored in the peer. This will speedup in the case of a lot of peers with a small number of allowed ips each. It might still be worth porting the self test for allowed ips (I still want to port over a couple of other tests too), but again, that is a task for another day. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: remove superfluous ull castsMatt Dunwoodie2021-04-191-15/+12
| | | | | | | | | | | | These are likely bad hangovers from OpenBSD because: OpenBSD: uint64_t == unsigned long long FreeBSD: uint64_t == unsigned long It makes sense to use the native platform types in the calls to printf, so convert ull to ul and remove the casts. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: add packet loop detectionMatt Dunwoodie2021-04-191-0/+8
| | | | | | | | | | | | | | | | | | | | | This is more sophisticated loop detection than OpenBSD, due to the loop detection relying on a "cookie". Each "cookie" is unique to the peer (in this case we use the peer id) and allows us to track which peers a packet has been sent to. Each time a packet hits wg_transmit, if_tunnel_check_nesting will count the number of correspinding tags (indexed by ifp, peer->p_id). If this is greater than or equal to 1 (i.e. it has been sent to this peer before), then raise an error. Currently the cookie is a uint32_t, which means the p_id gets truncated. This isn't ideal as it may cause conflicts (if a user adds 2**32 + 1 peers to an interface). However, I'm not too concerned about this for the time being because nested routing is uncommon and this is an improvement over the current situation which would likely DoS a host if a packet was sent in a nested configuration. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: replace wg_tag with wg_packetMatt Dunwoodie2021-04-192-688/+649
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `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>
* if_wg: import latest wg_noise.{c,h}Matt Dunwoodie2021-04-195-1068/+1137
| | | | | | | | | | | | | | | | | | | | | Note: this is a partial diff, introducing temporary bugs that will be resolved in following commits, detailed below. This commit brings wg_noise.{c,h} up to date with wireguard-openbsd. The primary motivator for this large patchset is to allow checking nonces serial, requiring a reference to the receiving keypair across noise_* calls. Due to requiring reference counting on the keypairs, we also take this opportunity to throw away the old locking and bring in EPOCH (roughly equivalent to SMR on OpenBSD and RCU on Linux). The changes to if_wg.c are purely to allow it to compile, there are most certainly refcount leaks present (to be addressed in the following commits). Readers should review wg_noise.{c,h} in their entirety rather than the diffs, as there are significant changes. if_wg.c can be reviewed, but must be contextualised with the following commits (repace wg_tag with wg_packet, encrypt mbuf in place). Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: warn when we can't bind to socketsJason A. Donenfeld2021-04-181-0/+1
| | | | | | | This currently happens when there's no configured address in that family inside of the jail. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: rewrite and clarify socket bindingJason A. Donenfeld2021-04-181-49/+41
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: when setting the tunnel fib allow to set to fib number 0Frank Behrens2021-04-171-5/+3
| | | | | Signed-off-by: Frank Behrens <frank@harz.behrens.de> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* version: bumpv0.0.20210415Jason A. Donenfeld2021-04-151-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: set multicast flagJason A. Donenfeld2021-04-151-1/+1
| | | | | | | In order to send to ff00::/8 addresses, even over unicast, the interface needs the multicast flag enabled. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: do not allow ioctl to race with clone_destroyJason A. Donenfeld2021-04-151-10/+16
| | | | | | | | | | | | | | | | | | | | | This fixes the crash from: bash -c 'while true; do ifconfig wg0 create; ifconfig wg0 destroy; done& while true; do wg show wg0 > /dev/null 2>&1; done& wait' Since we're setting ifp to NULL here, we also have to account for multicast v6 packets being transmitted during destroy, which can be triggered by: ifconfig wg0 create ifconfig wg0 inet6 fe80::1234/120 ifconfig wg0 up route add -inet6 ff02::1:0/120 -iface wg0 ifconfig wg0 destroy These are unfixed upstream bug that we're working around. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: don't check return value of WAITOKJason A. Donenfeld2021-04-151-4/+0
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: allow debugging with `ifconfig wg0 debug`Jason A. Donenfeld2021-04-131-8/+2
| | | | | | This is better than a custom sysctl. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: remove peer marshalling from get requestJason A. Donenfeld2021-04-132-312/+94
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Copyright © 1996 – 2023 Jason A. Donenfeld. All Rights Reverse Engineered.