aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* version: bumpv0.0.20210424Jason A. Donenfeld2021-04-241-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* crypto: optimize out `if (encrypt)`Jason A. Donenfeld2021-04-241-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: count on peers always having a remoteJason A. Donenfeld2021-04-243-11/+5
| | | | | | | We do a pretty nasty hack in the allowedips selftest to avoid having to allocate more memory. Seems to work. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: ensure peer lifetimeMatt Dunwoodie2021-04-253-46/+86
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The peer (and keypair and local) lifecycle are managed through EPOCH and refcounts. Primarily this is used in wg_noise to keep track of active keypairs, however we can also use it to be sure no more peer references exist. The structures are linked as such, so noise_remote cannot be freed until all noise_keypairs are freed, and noise_local cannot be freed until all noise_remotes are freed. noise_keypair -> noise_remote -> noise_local Therefore, if you hold a keypair reference you can be sure that remote and local will still be around. There are three main ways peers are referenced: 1) Incoming packets 1.a) Incoming handshake packets are passed to noise_consume_*, which will (on success) return a refcounted remote which is dropped at the end of wg_handshake. 1.b) Incoming cookie packets will have their index looked which will (on success) return a refcounted remote, which is also dropped at the end of wg_handshake. 1.c) Incoming data packets will have their index looked up which will (on success) return a refcounted keypair. This keypair will be dropped after the packet has been passed up the network stack, or otherwise freed. 2) Outgoing data packets 2.a) Outgoing data packets are first looked up by wg_aip_lookup, which returns a peer pointer, with an incremented remote refcount. This is then dropped in wg_transmit after adding the packet to the staged queue and sending the staged queue. 2.b) Packets in the staged queue do not hold any refcount for the remote or keypair, because they do not reference the peer in any way, they are just in the queue. 2.c) Packets finally get a refcoutned keypair in wg_peer_send_staged, which is dropped after the packet is sent out the UDP socket, or otherwise freed. 3) wg_timers system 3.a) The wg_timers system holds a reference to the peer whenever a callout is scheduled. Instead of holding a refcount, we instead disable the peer's timers, such that no callouts can be scheduled. Some rationale for changes here: We move the p_{send,recv} taskqgroup_detach into peer_free_deferred as they will NULL fields in p_{send,recv}. If there are packets being processed in wg_{en,de}crypt, then a call tou GROUPTASK_ENQUEUE will dereference a NULL pointer. In general, we remove all references to the peer in wg_peer_destroy, and free/deinit all the peer members once no more references to the remote exist, in wg_peer_free_deferred. Currently we take a refcount in wg_aip_lookup, which is to be sure that the peer reference is valid for the entirety of wg_transmit. We do not care about the refcount in wg_decrypt. It might be worth considering storing the remote pointer in the allowedip entry, but it could be argued both ways. For the time being, this is still correct. We don't have a refcount for the peer stored in the allowedip table, as it is protected by the table lock. One note here is the NULL p_remote check is necessary to support selftest/allowedips.c, which does not specify a p_remote. If we update the tests, then we may remove this check. There are two added p_enabled checks, in run_retry_handshake and run_send_keepalive. This is to align them with the other callout_reset calls. In the case of p_zero_key_material, if we have set p_enabled = false, then we subsequently clear keypairs and handshakes (on wg_down), or we free the peer which will clear the keypairs for us. We want to hold a refcount of remote in wg_{en,de}crypt to ensure that the peer is still valid in the call to GROUPTASK_ENQUEUE. If we don't then peer may become invalid after setting p_state. Another thread may take the packet, put the keypair refcount and free the peer prior to the call to GROUPTASK_ENQUEUE. We no longer need to hold (haven't for a while) the EPOCH in wg_send_initiation and wg_send_response, as we hold valid references for the duration. This could be either a refcount of a remote or through the wg_timers system as described above. We also fix some refcount leaks in wgc_set. Notes: We may want to pull NET_EPOCH_WAIT out of wg_timers_disable, to improve performance. However, we can destroy 20000 peers in less than 20ms so the performance is not critical for this snapshot and can be addressed later. Finally, there is the special case of noise_remote_arg, which stores the corresponding peer pointer. The peer is not refcounted however it will have the same scope as the remote. In otherwords it is valid until we call noise_remote_put on the remote. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* selftests: capitalise fail messages for readabilityMatt Dunwoodie2021-04-232-4/+4
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: zero out remaining mallocsJason A. Donenfeld2021-04-221-4/+4
| | | | | | | We might add locks and things later. Mainly it doesn't cost much and makes things easier/safer to reason about. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_noise: zero out new structuresJason A. Donenfeld2021-04-221-16/+2
| | | | | | | Good for hygiene, but also, lock hardening traps on double initialization if we don't do this. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* compat: backport m_snd_tag_rele to 12Jason A. Donenfeld2021-04-221-0/+11
| | | | | | | This doesn't add any reference counting, opting instead to go right to the free. This could cause problems, but hopefully not. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: remove M_WAITOK, check return codes on initMatt Dunwoodie2021-04-232-68/+90
| | | | | | | | | | | | | | | Here we remove all M_WAITOK checks, because we don't want to hang while trying to allocate memory. It is better to return an error so the user can try again later. We also make sure to check all the return codes in peer and interface allocation. The structure of those functions is: 1) Allocate all memory 2) Initialise fields in order of the struct 3) Cleanup gotos Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: check wg_module_init succeededMatt Dunwoodie2021-04-231-9/+15
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: set snd_tag to NULL after releasingJason A. Donenfeld2021-04-221-1/+3
| | | | | | The rest of the stack does this. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: destroy interfaces on module unloadJason A. Donenfeld2021-04-221-10/+4
| | | | | | This is already done anyway by if_clone_detach, so let that happen. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_cookie: import optional inet6 headersJason A. Donenfeld2021-04-221-0/+2
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_cookie: hash vnet into ratelimiter entryJason A. Donenfeld2021-04-225-62/+46
| | | | | | IPs mean different things per-vnet. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: properly use rn_inithead and rn_detachheadJason A. Donenfeld2021-04-224-35/+59
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_cookie: allocate ratelimit table staticallyMatt Dunwoodie2021-04-232-34/+26
| | | | | | | | | | | | | | | We can simplify the ratelimit init/deinit calls by allocating the table statically, that is by not using hashinit_flags. That function ended up doing some unnecessary calculation and meant that the mask couldn't be constant. By increasing the size of struct ratelimit, this also caught a nasty (but benign) bug, where ratelimit_pool was initialised to allocate sizeof(struct ratelimit) and not sizeof(struct ratelimit_entry). It has been this way since FreeBSD tree and I didn't pick up on it while moving the uma_zcreate call to wg_cookie. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_cookie: cleanup internal codeMatt Dunwoodie2021-04-232-70/+68
| | | | | | | | | | | | | | The two main changes here are: * Remove cookie_ prefix from static functions. This is a leftover from OpenBSD where they don't want static functions. * Rename cm to macs, and cp to cm. Not sure where this came from but it didn't really make much sense to leave it as is. The reset are whitespace changes. Overall there is no modification to functionality here, just appearances. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_cookie: add cookie_valid boolMatt Dunwoodie2021-04-233-20/+22
| | | | | | | | | | | | | | | | | | | | | | | | Primarily this commit adds a cookie_valid state, to prevent a recently booted machine from sending a mac2. We also do a little bit of reworking on locking and a fixup for int to bool. There is one slight difference to cookie_valid (latest_cookie.is_valid) on Linux and that is to set cookie_valid to false when the cookie_birthdate has expired. The purpose of this is to prevent the expensive timer check after it has expired. For the locking, we want to hold a write lock in cookie_maker_mac because we write to mac1_last, mac1_valid and cookie_valid. This wouldn't cause too much contention as this is a per peer lock and we only do so when sending handshake packets. This is different from Linux as Linux writes all it's variables at the start, then downgrades to a read lock. We also match cookie_maker_consume_payload locking to Linux, that is to read lock while checking mac1_valid and decrypting the cookie then take a write lock to set the cookie. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* wg_cookie: make ratelimiter globalMatt Dunwoodie2021-04-235-115/+113
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* TODO: more nitsJason A. Donenfeld2021-04-221-2/+12
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: add more usual string concat spacingJason A. Donenfeld2021-04-221-14/+14
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: correct logic in tag clearingJason A. Donenfeld2021-04-221-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: add missing bracketsJason A. Donenfeld2021-04-222-2/+4
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: more thorough wg_mbuf_resetMatt Dunwoodie2021-04-221-6/+36
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* if_wg: better loop detectionMatt Dunwoodie2021-04-221-3/+4
| | | | | | | | | | | | | While it was nice to have per peer loop detection, it was not meant to be. The loop tag has a tag type == 0, which conflicts with other tags. Therefore we want to at least be a little bit more sure that the tag cookie is unique to the loop tag. I guess the peer address was also quite hacky so on the other side, I'm glad to be rid of that. Now we have a loop of 8 (to any peer) which should be good enough for an edge case operation. Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* 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>