Changeset View
Standalone View
sys/net/if_ovpn.c
- This file was added.
/*- | ||||||||||||
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD | ||||||||||||
* | ||||||||||||
* Copyright (c) 2021 Rubicon Communications, LLC (Netgate) | ||||||||||||
* | ||||||||||||
* Redistribution and use in source and binary forms, with or without | ||||||||||||
* modification, are permitted provided that the following conditions | ||||||||||||
* are met: | ||||||||||||
* 1. Redistributions of source code must retain the above copyright | ||||||||||||
* notice, this list of conditions and the following disclaimer. | ||||||||||||
* 2. Redistributions in binary form must reproduce the above copyright | ||||||||||||
* notice, this list of conditions and the following disclaimer in the | ||||||||||||
* documentation and/or other materials provided with the distribution. | ||||||||||||
* | ||||||||||||
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND | ||||||||||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | ||||||||||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | ||||||||||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE | ||||||||||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | ||||||||||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | ||||||||||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | ||||||||||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | ||||||||||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | ||||||||||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | ||||||||||||
* SUCH DAMAGE. | ||||||||||||
* | ||||||||||||
*/ | ||||||||||||
#include "opt_inet.h" | ||||||||||||
#include "opt_inet6.h" | ||||||||||||
#include <sys/param.h> | ||||||||||||
#include <sys/systm.h> | ||||||||||||
#include <sys/buf_ring.h> | ||||||||||||
#include <sys/epoch.h> | ||||||||||||
#include <sys/file.h> | ||||||||||||
#include <sys/filedesc.h> | ||||||||||||
#include <sys/kernel.h> | ||||||||||||
#include <sys/malloc.h> | ||||||||||||
#include <sys/mbuf.h> | ||||||||||||
#include <sys/module.h> | ||||||||||||
#include <sys/nv.h> | ||||||||||||
#include <sys/socket.h> | ||||||||||||
#include <sys/socketvar.h> | ||||||||||||
#include <sys/sockio.h> | ||||||||||||
#include <sys/sysctl.h> | ||||||||||||
#include <sys/protosw.h> | ||||||||||||
#include <sys/rmlock.h> | ||||||||||||
#include <machine/atomic.h> | ||||||||||||
#include <net/bpf.h> | ||||||||||||
#include <net/if.h> | ||||||||||||
#include <net/if_clone.h> | ||||||||||||
#include <net/if_types.h> | ||||||||||||
#include <net/if_var.h> | ||||||||||||
#include <net/netisr.h> | ||||||||||||
#include <netinet/in.h> | ||||||||||||
#include <netinet/ip.h> | ||||||||||||
#include <netinet/ip6.h> | ||||||||||||
#include <netinet/ip_var.h> | ||||||||||||
#include <netinet/udp.h> | ||||||||||||
#include <netinet/udp_var.h> | ||||||||||||
#include <netinet6/ip6_var.h> | ||||||||||||
#include <machine/in_cksum.h> | ||||||||||||
#include <opencrypto/cryptodev.h> | ||||||||||||
#include "if_ovpn.h" | ||||||||||||
struct ovpn_kpeer { | ||||||||||||
struct sockaddr_storage local; | ||||||||||||
struct sockaddr_storage remote; | ||||||||||||
}; | ||||||||||||
struct ovpn_kkey_dir { | ||||||||||||
uint8_t key[32]; | ||||||||||||
uint8_t keylen; | ||||||||||||
uint8_t nonce[8]; | ||||||||||||
uint8_t noncelen; | ||||||||||||
enum ovpn_key_cipher cipher; | ||||||||||||
crypto_session_t cryptoid; | ||||||||||||
struct mtx replay_mtx; | ||||||||||||
/* | ||||||||||||
* Last seen gapless sequence number. New rx seq numbers must be | ||||||||||||
* strictly higher than this. | ||||||||||||
*/ | ||||||||||||
uint32_t rx_seq; | ||||||||||||
/* Seen packets, relative to rx_seq. bit(0) will always be 0. */ | ||||||||||||
uint64_t rx_window; | ||||||||||||
}; | ||||||||||||
struct ovpn_kkey { | ||||||||||||
struct ovpn_kkey_dir *encrypt; | ||||||||||||
struct ovpn_kkey_dir *decrypt; | ||||||||||||
uint8_t keyid; | ||||||||||||
uint32_t peerid; | ||||||||||||
}; | ||||||||||||
struct ovpn_keepalive { | ||||||||||||
uint32_t interval; | ||||||||||||
uint32_t timeout; | ||||||||||||
}; | ||||||||||||
struct ovpn_wire_header { | ||||||||||||
uint32_t opcode; /* opcode, key id, peer id */ | ||||||||||||
uint32_t seq; | ||||||||||||
uint8_t auth_tag[16]; | ||||||||||||
}; | ||||||||||||
struct ovpn_notification { | ||||||||||||
enum ovpn_notif_type type; | ||||||||||||
uint32_t peerid; | ||||||||||||
}; | ||||||||||||
struct ovpn_softc { | ||||||||||||
struct rmlock lock; | ||||||||||||
struct ifnet *ifp; | ||||||||||||
struct ovpn_kpeer peer; | ||||||||||||
struct ovpn_kkey keys[2]; | ||||||||||||
uint32_t tx_seq; | ||||||||||||
struct socket *so; | ||||||||||||
struct ovpn_keepalive keepalive; | ||||||||||||
struct callout ping_send; | ||||||||||||
struct callout ping_rcv; | ||||||||||||
/* Pending packets */ | ||||||||||||
struct buf_ring *rxring; | ||||||||||||
struct buf_ring *notifring; | ||||||||||||
/* Counters */ | ||||||||||||
counter_u64_t lost_ctrl_pkts_in; | ||||||||||||
counter_u64_t lost_ctrl_pkts_out; | ||||||||||||
counter_u64_t lost_data_pkts_in; | ||||||||||||
counter_u64_t lost_data_pkts_out; | ||||||||||||
counter_u64_t received_ctrl_pkts; | ||||||||||||
counter_u64_t received_data_pkts; | ||||||||||||
counter_u64_t sent_ctrl_pkts; | ||||||||||||
counter_u64_t sent_data_pkts; | ||||||||||||
counter_u64_t transport_bytes_sent; | ||||||||||||
counter_u64_t transport_bytes_received; | ||||||||||||
counter_u64_t tunnel_bytes_sent; | ||||||||||||
counter_u64_t tunnel_bytes_received; | ||||||||||||
struct epoch_context epoch_ctx; | ||||||||||||
}; | ||||||||||||
static void ovpn_udp_input(struct mbuf *, int, struct inpcb *, | ||||||||||||
const struct sockaddr *, void *); | ||||||||||||
static int ovpn_encap(struct ovpn_softc *, struct mbuf *); | ||||||||||||
static int ovpn_get_af(struct mbuf *); | ||||||||||||
#ifdef ENABLE_RX_REPLAY_PROTECTION | ||||||||||||
static bool ovpn_check_replay(struct ovpn_kkey_dir *, uint32_t); | ||||||||||||
#endif | ||||||||||||
#define OVPN_MTU_MIN 576 | ||||||||||||
#define OVPN_MTU_MAX (IP_MAXPACKET - sizeof(struct ip) - \ | ||||||||||||
sizeof(struct udphdr) - sizeof(struct ovpn_wire_header)) | ||||||||||||
#define OVPN_OP_DATA_V2 0x09 | ||||||||||||
#define OVPN_OP_SHIFT 3 | ||||||||||||
VNET_DEFINE_STATIC(struct if_clone *, ovpn_cloner); | ||||||||||||
#define V_ovpn_cloner VNET(ovpn_cloner) | ||||||||||||
#define OVPN_RLOCK_TRACKER struct rm_priotracker _ovpn_lock_tracker; \ | ||||||||||||
struct rm_priotracker *_ovpn_lock_trackerp = &_ovpn_lock_tracker | ||||||||||||
#define OVPN_RLOCK(sc) rm_rlock(&(sc)->lock, _ovpn_lock_trackerp) | ||||||||||||
#define OVPN_RUNLOCK(sc) rm_runlock(&(sc)->lock, _ovpn_lock_trackerp) | ||||||||||||
#define OVPN_WLOCK(sc) rm_wlock(&(sc)->lock) | ||||||||||||
#define OVPN_WUNLOCK(sc) rm_wunlock(&(sc)->lock) | ||||||||||||
#define OVPN_ASSERT(sc) rm_assert(&(sc)->lock, RA_LOCKED) | ||||||||||||
#define OVPN_RASSERT(sc) rm_assert(&(sc)->lock, RA_RLOCKED) | ||||||||||||
#define OVPN_WASSERT(sc) rm_assert(&(sc)->lock, RA_WLOCKED) | ||||||||||||
#define OVPN_UNLOCK_ASSERT(sc) rm_assert(&(sc)->lock, RA_UNLOCKED) | ||||||||||||
#define TO_IN(x) ((struct sockaddr_in *)(x)) | ||||||||||||
#define TO_IN6(x) ((struct sockaddr_in6 *)(x)) | ||||||||||||
static const char ovpnname[] = "ovpn"; | ||||||||||||
static const char ovpngroupname[] = "openvpn"; | ||||||||||||
static MALLOC_DEFINE(M_OVPN, ovpnname, "OpenVPN DCO Interface"); | ||||||||||||
static uint16_t | ||||||||||||
ovpn_get_port(struct sockaddr_storage *s) | ||||||||||||
{ | ||||||||||||
switch (s->ss_family) { | ||||||||||||
case AF_INET: { | ||||||||||||
struct sockaddr_in *in = (struct sockaddr_in *)s; | ||||||||||||
return (in->sin_port); | ||||||||||||
} | ||||||||||||
case AF_INET6: { | ||||||||||||
struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)s; | ||||||||||||
return (in6->sin6_port); | ||||||||||||
} | ||||||||||||
default: | ||||||||||||
panic("Unsupported address family %d", s->ss_family); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_nvlist_to_sockaddr(const nvlist_t *nvl, struct sockaddr_storage *sa) | ||||||||||||
{ | ||||||||||||
int af; | ||||||||||||
if (! nvlist_exists_number(nvl, "af")) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_binary(nvl, "address")) | ||||||||||||
return (EINVAL); | ||||||||||||
ae: I think it would be a good idea initialize sin_len and sin6_len too. But probably this is not… | ||||||||||||
if (! nvlist_exists_number(nvl, "port")) | ||||||||||||
return (EINVAL); | ||||||||||||
af = nvlist_get_number(nvl, "af"); | ||||||||||||
switch (af) { | ||||||||||||
#ifdef INET | ||||||||||||
case AF_INET: { | ||||||||||||
struct sockaddr_in *in = (struct sockaddr_in *)sa; | ||||||||||||
size_t len; | ||||||||||||
const void *addr = nvlist_get_binary(nvl, "address", &len); | ||||||||||||
in->sin_family = af; | ||||||||||||
if (len != sizeof(in->sin_addr)) | ||||||||||||
return (EINVAL); | ||||||||||||
memcpy(&in->sin_addr, addr, sizeof(in->sin_addr)); | ||||||||||||
in->sin_port = nvlist_get_number(nvl, "port"); | ||||||||||||
break; | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
#ifdef INET6 | ||||||||||||
case AF_INET6: { | ||||||||||||
struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)sa; | ||||||||||||
size_t len; | ||||||||||||
const void *addr = nvlist_get_binary(nvl, "address", &len); | ||||||||||||
in6->sin6_family = af; | ||||||||||||
if (len != sizeof(in6->sin6_addr)) | ||||||||||||
return (EINVAL); | ||||||||||||
memcpy(&in6->sin6_addr, addr, sizeof(in6->sin6_addr)); | ||||||||||||
in6->sin6_port = nvlist_get_number(nvl, "port"); | ||||||||||||
break; | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
default: | ||||||||||||
return (EINVAL); | ||||||||||||
} | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_rele_so(struct ovpn_softc *sc) | ||||||||||||
{ | ||||||||||||
OVPN_WASSERT(sc); | ||||||||||||
if (sc->so == NULL) | ||||||||||||
return; | ||||||||||||
(void)udp_set_kernel_tunneling(sc->so, NULL, NULL, NULL); | ||||||||||||
sorele(sc->so); | ||||||||||||
sc->so = NULL; | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_new_peer(struct ifnet *ifp, const nvlist_t *nvl) | ||||||||||||
{ | ||||||||||||
#ifdef INET6 | ||||||||||||
struct epoch_tracker et; | ||||||||||||
#endif | ||||||||||||
struct sockaddr_storage remote; | ||||||||||||
struct file *fp = NULL; | ||||||||||||
struct socket *so; | ||||||||||||
struct sockaddr *nam; | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
struct thread *td = curthread; | ||||||||||||
u_int fflag; | ||||||||||||
int fd; | ||||||||||||
int ret = 0; | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_number(nvl, "fd")) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_nvlist(nvl, "remote")) | ||||||||||||
return (EINVAL); | ||||||||||||
ret = ovpn_nvlist_to_sockaddr(nvlist_get_nvlist(nvl, "remote"), | ||||||||||||
&remote); | ||||||||||||
if (ret != 0) | ||||||||||||
return (ret); | ||||||||||||
fd = nvlist_get_number(nvl, "fd"); | ||||||||||||
Done Inline ActionsQ: what about link-local addresses? melifaro: Q: what about link-local addresses? | ||||||||||||
Done Inline ActionsInteresting question, I didn't think about them. I've done a little digging in the OpenVPN code, and it appears that OpenVPN also doesn't support configuration of link-local addresses, so right now I'd be inclined to not support them here. The ioctl interface uses nvlists, so if that changes in the future it'll be relatively easy to add this support. Although to be honest, I think the use case for a VPN over link-local addresses is fairly weak. kp: Interesting question, I didn't think about them.
I've done a little digging in the OpenVPN… | ||||||||||||
/* Look up the userspace process and use the fd to find the socket. */ | ||||||||||||
ret = getsock_cap(td, fd, &cap_connect_rights, &fp, | ||||||||||||
&fflag, NULL); | ||||||||||||
if (ret != 0) | ||||||||||||
return (ret); | ||||||||||||
so = fp->f_data; | ||||||||||||
ret = (*so->so_proto->pr_usrreqs->pru_sockaddr)(so, &nam); | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
if (ret) | ||||||||||||
goto error; | ||||||||||||
if (ovpn_get_port((struct sockaddr_storage *)nam) == 0) { | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
ret = EINVAL; | ||||||||||||
goto error; | ||||||||||||
} | ||||||||||||
if (nam->sa_family != remote.ss_family) { | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
ret = EINVAL; | ||||||||||||
goto error; | ||||||||||||
} | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
ovpn_rele_so(sc); | ||||||||||||
memcpy(&sc->peer.local, nam, nam->sa_len); | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
memcpy(&sc->peer.remote, &remote, | ||||||||||||
sizeof(remote)); | ||||||||||||
free(nam, M_SONAME); | ||||||||||||
Lint: Possible Spelling Mistake Possible spelling error. You wrote 'nam', but did you mean 'name'? Lint: Possible Spelling Mistake: Possible spelling error. You wrote 'nam', but did you mean 'name'? | ||||||||||||
#ifdef INET6 | ||||||||||||
if (sc->peer.local.ss_family == AF_INET6 && | ||||||||||||
IN6_IS_ADDR_UNSPECIFIED(&TO_IN6(&sc->peer.local)->sin6_addr)) { | ||||||||||||
NET_EPOCH_ENTER(et); | ||||||||||||
ret = in6_selectsrc_addr(0, | ||||||||||||
&TO_IN6(&sc->peer.remote)->sin6_addr, | ||||||||||||
0, NULL, &TO_IN6(&sc->peer.local)->sin6_addr, NULL); | ||||||||||||
NET_EPOCH_EXIT(et); | ||||||||||||
if (ret != 0) { | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
goto error; | ||||||||||||
} | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
sc->so = so; | ||||||||||||
soref(sc->so); | ||||||||||||
ret = udp_set_kernel_tunneling(so, ovpn_udp_input, NULL, sc); | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
error: | ||||||||||||
if (fp != NULL) | ||||||||||||
fdrop(fp, td); | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_del_peer(struct ifnet *ifp) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
struct ovpn_notification *n; | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
n = malloc(sizeof(*n), M_OVPN, M_NOWAIT); | ||||||||||||
if (n != NULL) { | ||||||||||||
n->peerid = sc->keys[OVPN_KEY_SLOT_PRIMARY].peerid; | ||||||||||||
n->type = OVPN_NOTIF_DEL_PEER; | ||||||||||||
if (buf_ring_enqueue(sc->notifring, n) != 0) { | ||||||||||||
free(n, M_OVPN); | ||||||||||||
} else if (sc->so != NULL) { | ||||||||||||
/* Wake up userspace */ | ||||||||||||
sorwakeup(sc->so); | ||||||||||||
sowwakeup(sc->so); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
bzero(&sc->peer, sizeof(sc->peer)); | ||||||||||||
ovpn_rele_so(sc); | ||||||||||||
/* Link down. */ | ||||||||||||
ifp->if_flags &= ~IFF_UP; | ||||||||||||
ifp->if_drv_flags &= ~IFF_DRV_RUNNING; | ||||||||||||
if_link_state_change(ifp, LINK_STATE_DOWN); | ||||||||||||
Not Done Inline ActionsNeed to drop the ref on fp before returning. markj: Need to drop the ref on `fp` before returning. | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (0); | ||||||||||||
Not Done Inline ActionsShould we hold the softc write lock when setting sc->so? markj: Should we hold the softc write lock when setting `sc->so`? | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_check_peer(struct ovpn_softc *sc) | ||||||||||||
{ | ||||||||||||
OVPN_ASSERT(sc); | ||||||||||||
/* For now just check that port is nonzero. */ | ||||||||||||
if (ovpn_get_port(&sc->peer.remote) == 0) | ||||||||||||
return (EINVAL); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_create_kkey_dir(struct ovpn_kkey_dir **kdirp, | ||||||||||||
const nvlist_t *nvl) | ||||||||||||
{ | ||||||||||||
struct crypto_session_params csp; | ||||||||||||
struct ovpn_kkey_dir *kdir; | ||||||||||||
const char *ciphername; | ||||||||||||
enum ovpn_key_cipher cipher; | ||||||||||||
const void *key, *iv; | ||||||||||||
size_t keylen = 0, ivlen = 0; | ||||||||||||
int error; | ||||||||||||
if (! nvlist_exists_string(nvl, "cipher")) | ||||||||||||
return (EINVAL); | ||||||||||||
ciphername = nvlist_get_string(nvl, "cipher"); | ||||||||||||
if (strcmp(ciphername, "none") == 0) | ||||||||||||
cipher = OVPN_CIPHER_ALG_NONE; | ||||||||||||
else if (strcmp(ciphername, "AES-256-GCM") == 0) | ||||||||||||
cipher = OVPN_CIPHER_ALG_AES_GCM; | ||||||||||||
else if (strcmp(ciphername, "CHACHA20-POLY1305") == 0) | ||||||||||||
cipher = OVPN_CIPHER_ALG_CHACHA20_POLY1305; | ||||||||||||
else | ||||||||||||
return (EINVAL); | ||||||||||||
if (cipher != OVPN_CIPHER_ALG_NONE) { | ||||||||||||
if (! nvlist_exists_binary(nvl, "key")) | ||||||||||||
return (EINVAL); | ||||||||||||
key = nvlist_get_binary(nvl, "key", &keylen); | ||||||||||||
if (keylen > sizeof(kdir->key)) | ||||||||||||
return (E2BIG); | ||||||||||||
if (! nvlist_exists_binary(nvl, "iv")) | ||||||||||||
Not Done Inline ActionsThis can be one line. markj: This can be one line. | ||||||||||||
return (EINVAL); | ||||||||||||
iv = nvlist_get_binary(nvl, "iv", &ivlen); | ||||||||||||
if (ivlen != 8) | ||||||||||||
return (E2BIG); | ||||||||||||
} | ||||||||||||
kdir = malloc(sizeof(struct ovpn_kkey_dir), M_OVPN, | ||||||||||||
M_WAITOK | M_ZERO); | ||||||||||||
kdir->cipher = cipher; | ||||||||||||
kdir->keylen = keylen; | ||||||||||||
memcpy(kdir->key, key, keylen); | ||||||||||||
kdir->noncelen = ivlen; | ||||||||||||
memcpy(kdir->nonce, iv, ivlen); | ||||||||||||
if (kdir->cipher != OVPN_CIPHER_ALG_NONE) { | ||||||||||||
/* Crypto init */ | ||||||||||||
bzero(&csp, sizeof(csp)); | ||||||||||||
csp.csp_mode = CSP_MODE_AEAD; | ||||||||||||
if (kdir->cipher == OVPN_CIPHER_ALG_CHACHA20_POLY1305) | ||||||||||||
csp.csp_cipher_alg = CRYPTO_CHACHA20_POLY1305; | ||||||||||||
else | ||||||||||||
csp.csp_cipher_alg = CRYPTO_AES_NIST_GCM_16; | ||||||||||||
csp.csp_flags |= CSP_F_SEPARATE_AAD; | ||||||||||||
csp.csp_cipher_klen = kdir->keylen; | ||||||||||||
csp.csp_cipher_key = kdir->key; | ||||||||||||
csp.csp_ivlen = 96 / 8; | ||||||||||||
error = crypto_newsession(&kdir->cryptoid, &csp, | ||||||||||||
CRYPTOCAP_F_HARDWARE | CRYPTOCAP_F_SOFTWARE); | ||||||||||||
if (error) { | ||||||||||||
free(kdir, M_OVPN); | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
mtx_init(&kdir->replay_mtx, "if_ovpn rx replay", NULL, MTX_DEF); | ||||||||||||
*kdirp = kdir; | ||||||||||||
Not Done Inline ActionsI suspect it'd be a bit better to set the tunneling function after finding a slot in the peer table. If we fail to find a slot, we leave the tunneling function set. markj: I suspect it'd be a bit better to set the tunneling function after finding a slot in the peer… | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_free_kkey_dir(struct ovpn_kkey_dir *kdir) | ||||||||||||
{ | ||||||||||||
if (kdir == NULL) | ||||||||||||
return; | ||||||||||||
mtx_destroy(&kdir->replay_mtx); | ||||||||||||
crypto_freesession(kdir->cryptoid); | ||||||||||||
free(kdir, M_OVPN); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_set_key(struct ifnet *ifp, const nvlist_t *nvl) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
struct ovpn_kkey_dir *enc, *dec; | ||||||||||||
int slot, keyid, peerid; | ||||||||||||
int error; | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_number(nvl, "slot")) | ||||||||||||
return (EINVAL); | ||||||||||||
slot = nvlist_get_number(nvl, "slot"); | ||||||||||||
if (! nvlist_exists_number(nvl, "keyid")) | ||||||||||||
return (EINVAL); | ||||||||||||
keyid = nvlist_get_number(nvl, "keyid"); | ||||||||||||
if (! nvlist_exists_number(nvl, "peerid")) | ||||||||||||
return (EINVAL); | ||||||||||||
peerid = nvlist_get_number(nvl, "peerid"); | ||||||||||||
if (slot != OVPN_KEY_SLOT_PRIMARY && | ||||||||||||
slot != OVPN_KEY_SLOT_SECONDARY) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_nvlist(nvl, "encrypt") || | ||||||||||||
Done Inline ActionsIt should be peer ( or process) fib, not 0. melifaro: It should be peer ( or process) fib, not 0. | ||||||||||||
! nvlist_exists_nvlist(nvl, "decrypt")) | ||||||||||||
return (EINVAL); | ||||||||||||
error = ovpn_create_kkey_dir(&enc, nvlist_get_nvlist(nvl, "encrypt")); | ||||||||||||
if (error) | ||||||||||||
return (error); | ||||||||||||
error = ovpn_create_kkey_dir(&dec, nvlist_get_nvlist(nvl, "decrypt")); | ||||||||||||
if (error) { | ||||||||||||
ovpn_free_kkey_dir(enc); | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
ovpn_free_kkey_dir(sc->keys[slot].encrypt); | ||||||||||||
ovpn_free_kkey_dir(sc->keys[slot].decrypt); | ||||||||||||
sc->keys[slot].encrypt = enc; | ||||||||||||
sc->keys[slot].decrypt = dec; | ||||||||||||
sc->keys[slot].keyid = keyid; | ||||||||||||
sc->keys[slot].peerid = peerid; | ||||||||||||
Not Done Inline ActionsI think this needs to be a callout_drain() when the caller is not the callout itself. Otherwise the callout could be running concurrently, and the subsequent free() call isn't safe. markj: I think this needs to be a callout_drain() when the caller is not the callout itself. Otherwise… | ||||||||||||
Done Inline ActionsThe callout requires the sc->lock, so I wouldn't expect that to be an issue. I'm also seeing panics over sleepq_add with sleeping prohibited if we sleep here. Thinking about it some more I'm wondering if the drain for ping_send shouldn't also be a stop. It takes the same lock, so it should be similarly safe. kp: The callout requires the sc->lock, so I wouldn't expect that to be an issue.
I'm also seeing… | ||||||||||||
Not Done Inline ActionsI think you're right that the ping_rcv callout is ok, my mistake. But the ping_snd callout handler drops the softc lock while executing, and we really do need to defer the free of peer until that's done. markj: I think you're right that the `ping_rcv` callout is ok, my mistake. But the `ping_snd` callout… | ||||||||||||
Done Inline ActionsI don't think it does. Not any more at least. It keeps the lock, because it's a read lock and the callout support for rmlocks is a little limited, in that it doesn't export the rm_priotracker anywhere, so there's no way for callout users to release an rm-read lock (unlike mutex, or rwlock-based callouts). kp: I don't think it does. Not any more at least.
It keeps the lock, because it's a read lock and… | ||||||||||||
Not Done Inline ActionsAh, got it. I missed that the tracker pointer is NULL in that case. markj: Ah, got it. I missed that the tracker pointer is NULL in that case. | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_check_key(struct ovpn_softc *sc, enum ovpn_key_slot slot) | ||||||||||||
{ | ||||||||||||
OVPN_ASSERT(sc); | ||||||||||||
if (sc->keys[slot].encrypt == NULL) | ||||||||||||
return (ENOLINK); | ||||||||||||
if (sc->keys[slot].decrypt == NULL) | ||||||||||||
return (ENOLINK); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_start(struct ifnet *ifp) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
int error; | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
/* | ||||||||||||
* Ensure we have all relevant information (i.e. peer, keys, ..) set | ||||||||||||
* before we up the link. | ||||||||||||
*/ | ||||||||||||
if ((error = ovpn_check_peer(sc))) { | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
if ((error = ovpn_check_key(sc, OVPN_KEY_SLOT_PRIMARY))) { | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
ifp->if_flags |= IFF_UP; | ||||||||||||
ifp->if_drv_flags |= IFF_DRV_RUNNING; | ||||||||||||
if_link_state_change(ifp, LINK_STATE_UP); | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_swap_keys(struct ifnet *ifp) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
struct ovpn_kkey tmpkey; | ||||||||||||
int error; | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
/* Check that we have a second key to swap to. */ | ||||||||||||
error = ovpn_check_key(sc, OVPN_KEY_SLOT_SECONDARY); | ||||||||||||
if (error) { | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
tmpkey = sc->keys[0]; | ||||||||||||
sc->keys[0] = sc->keys[1]; | ||||||||||||
sc->keys[1] = tmpkey; | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_del_key(struct ifnet *ifp, const nvlist_t *nvl) | ||||||||||||
{ | ||||||||||||
enum ovpn_key_slot slot; | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_number(nvl, "slot")) | ||||||||||||
return (EINVAL); | ||||||||||||
slot = nvlist_get_number(nvl, "slot"); | ||||||||||||
if (slot != OVPN_KEY_SLOT_PRIMARY && | ||||||||||||
slot != OVPN_KEY_SLOT_SECONDARY) | ||||||||||||
return (EINVAL); | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
ovpn_free_kkey_dir(sc->keys[slot].encrypt); | ||||||||||||
ovpn_free_kkey_dir(sc->keys[slot].decrypt); | ||||||||||||
sc->keys[slot].encrypt = NULL; | ||||||||||||
sc->keys[slot].decrypt = NULL; | ||||||||||||
sc->keys[slot].keyid = 0; | ||||||||||||
sc->keys[slot].peerid = 0; | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_send_pkt(struct ifnet *ifp, const nvlist_t *nvl) | ||||||||||||
{ | ||||||||||||
struct epoch_tracker et; | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
struct mbuf *m; | ||||||||||||
const uint8_t *pkt; | ||||||||||||
size_t pktlen; | ||||||||||||
int ret; | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_binary(nvl, "packet")) | ||||||||||||
return (EINVAL); | ||||||||||||
pkt = nvlist_get_binary(nvl, "packet", &pktlen); | ||||||||||||
/* | ||||||||||||
* Check that userspace isn't giving us a data packet. That might lead | ||||||||||||
* to IV re-use, which would be bad. | ||||||||||||
*/ | ||||||||||||
if ((pkt[0] >> OVPN_OP_SHIFT) == OVPN_OP_DATA_V2) | ||||||||||||
return (EINVAL); | ||||||||||||
m = m_get2(pktlen, M_WAITOK, MT_DATA, M_PKTHDR); | ||||||||||||
if (m == NULL) | ||||||||||||
return (ENOMEM); | ||||||||||||
m->m_len = m->m_pkthdr.len = pktlen; | ||||||||||||
m_copyback(m, 0, m->m_len, pkt); | ||||||||||||
/* Now prepend IP/UDP headers and transmit the mbuf. */ | ||||||||||||
NET_EPOCH_ENTER(et); | ||||||||||||
ret = ovpn_encap(sc, m); | ||||||||||||
NET_EPOCH_EXIT(et); | ||||||||||||
if (ret == 0) | ||||||||||||
counter_u64_add(sc->sent_ctrl_pkts, 1); | ||||||||||||
else | ||||||||||||
counter_u64_add(sc->lost_ctrl_pkts_out, 1); | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_send_ping(void *arg) | ||||||||||||
{ | ||||||||||||
static uint8_t ping_str[] = { | ||||||||||||
0x2a, 0x18, 0x7b, 0xf3, 0x64, 0x1e, 0xb4, 0xcb, | ||||||||||||
0x07, 0xed, 0x2d, 0x0a, 0x98, 0x1f, 0xc7, 0x48 | ||||||||||||
}; | ||||||||||||
struct epoch_tracker et; | ||||||||||||
struct ovpn_softc *sc = arg; | ||||||||||||
struct mbuf *m; | ||||||||||||
/* Ensure we repeat! */ | ||||||||||||
callout_reset(&sc->ping_send, sc->keepalive.interval * hz, | ||||||||||||
ovpn_send_ping, sc); | ||||||||||||
m = m_get2(sizeof(ping_str), M_NOWAIT, MT_DATA, M_PKTHDR); | ||||||||||||
if (m == NULL) | ||||||||||||
return; | ||||||||||||
m_copyback(m, 0, sizeof(ping_str), ping_str); | ||||||||||||
m->m_len = m->m_pkthdr.len = sizeof(ping_str); | ||||||||||||
CURVNET_SET(sc->ifp->if_vnet); | ||||||||||||
NET_EPOCH_ENTER(et); | ||||||||||||
sc->ifp->if_transmit(sc->ifp, m); | ||||||||||||
NET_EPOCH_EXIT(et); | ||||||||||||
CURVNET_RESTORE(); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_timeout(void *arg) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = arg; | ||||||||||||
CURVNET_SET(sc->ifp->if_vnet); | ||||||||||||
ovpn_del_peer(sc->ifp); | ||||||||||||
CURVNET_RESTORE(); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_set_peer(struct ifnet *ifp, const nvlist_t *nvl) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (EINVAL); | ||||||||||||
if (! nvlist_exists_number(nvl, "interval") | ||||||||||||
|| ! nvlist_exists_number(nvl, "timeout")) | ||||||||||||
return (EINVAL); | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
sc->keepalive.interval = nvlist_get_number(nvl, "interval"); | ||||||||||||
sc->keepalive.timeout = nvlist_get_number(nvl, "timeout"); | ||||||||||||
callout_reset(&sc->ping_send, sc->keepalive.interval * hz, | ||||||||||||
ovpn_send_ping, sc); | ||||||||||||
callout_reset(&sc->ping_rcv, sc->keepalive.timeout * hz, | ||||||||||||
ovpn_timeout, sc); | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_ioctl_set(struct ifnet *ifp, struct ifdrv *ifd) | ||||||||||||
{ | ||||||||||||
uint8_t *buf = NULL; | ||||||||||||
nvlist_t *nvl = NULL; | ||||||||||||
int ret; | ||||||||||||
if (ifd->ifd_len != 0) { | ||||||||||||
if (ifd->ifd_len > OVPN_MAX_REQUEST_SIZE) | ||||||||||||
return (E2BIG); | ||||||||||||
buf = malloc(ifd->ifd_len, M_OVPN, M_WAITOK); | ||||||||||||
ret = copyin(ifd->ifd_data, buf, ifd->ifd_len); | ||||||||||||
if (ret != 0) { | ||||||||||||
free(buf, M_OVPN); | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
nvl = nvlist_unpack(buf, ifd->ifd_len, 0); | ||||||||||||
free(buf, M_OVPN); | ||||||||||||
if (nvl == NULL) { | ||||||||||||
return (EINVAL); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
switch (ifd->ifd_cmd) { | ||||||||||||
case OVPN_NEW_PEER: | ||||||||||||
ret = ovpn_new_peer(ifp, nvl); | ||||||||||||
break; | ||||||||||||
case OVPN_DEL_PEER: | ||||||||||||
ret = ovpn_del_peer(ifp); | ||||||||||||
break; | ||||||||||||
case OVPN_NEW_KEY: | ||||||||||||
ret = ovpn_set_key(ifp, nvl); | ||||||||||||
break; | ||||||||||||
case OVPN_START_VPN: | ||||||||||||
ret = ovpn_start(ifp); | ||||||||||||
break; | ||||||||||||
case OVPN_SWAP_KEYS: | ||||||||||||
ret = ovpn_swap_keys(ifp); | ||||||||||||
break; | ||||||||||||
case OVPN_DEL_KEY: | ||||||||||||
ret = ovpn_del_key(ifp, nvl); | ||||||||||||
break; | ||||||||||||
case OVPN_SEND_PKT: | ||||||||||||
ret = ovpn_send_pkt(ifp, nvl); | ||||||||||||
break; | ||||||||||||
case OVPN_SET_PEER: | ||||||||||||
ret = ovpn_set_peer(ifp, nvl); | ||||||||||||
break; | ||||||||||||
default: | ||||||||||||
ret = ENOTSUP; | ||||||||||||
} | ||||||||||||
nvlist_destroy(nvl); | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_add_counters(nvlist_t *parent, const char *name, counter_u64_t in, | ||||||||||||
counter_u64_t out) | ||||||||||||
{ | ||||||||||||
nvlist_t *nvl; | ||||||||||||
nvl = nvlist_create(0); | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (ENOMEM); | ||||||||||||
nvlist_add_number(nvl, "in", counter_u64_fetch(in)); | ||||||||||||
nvlist_add_number(nvl, "out", counter_u64_fetch(out)); | ||||||||||||
nvlist_add_nvlist(parent, name, nvl); | ||||||||||||
nvlist_destroy(nvl); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_get_stats(struct ovpn_softc *sc, nvlist_t **onvl) | ||||||||||||
{ | ||||||||||||
nvlist_t *nvl; | ||||||||||||
int ret; | ||||||||||||
nvl = nvlist_create(0); | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (ENOMEM); | ||||||||||||
ret = ovpn_add_counters(nvl, "lost_ctrl", sc->lost_ctrl_pkts_in, | ||||||||||||
sc->lost_ctrl_pkts_out); | ||||||||||||
if (ret != 0) | ||||||||||||
goto error; | ||||||||||||
ret = ovpn_add_counters(nvl, "lost_data", sc->lost_data_pkts_in, | ||||||||||||
sc->lost_data_pkts_out); | ||||||||||||
if (ret != 0) | ||||||||||||
goto error; | ||||||||||||
ret = ovpn_add_counters(nvl, "data", sc->received_data_pkts, | ||||||||||||
sc->sent_data_pkts); | ||||||||||||
if (ret != 0) | ||||||||||||
goto error; | ||||||||||||
ret = ovpn_add_counters(nvl, "ctrl", sc->received_ctrl_pkts, | ||||||||||||
sc->sent_ctrl_pkts); | ||||||||||||
if (ret != 0) | ||||||||||||
goto error; | ||||||||||||
Not Done Inline ActionsIs there any guarantee that the mbuf is fully contiguous? m_defrag() can still split a sufficiently long packet among multiple mbufs. markj: Is there any guarantee that the mbuf is fully contiguous? m_defrag() can still split a… | ||||||||||||
Done Inline ActionsI don't think this is guaranteed to be contiguous, but I also don't think it matters. m_copyback() deals with mbuf chains and ovpn_encap() basically just prepends data. kp: I don't think this is guaranteed to be contiguous, but I also don't think it matters. | ||||||||||||
Not Done Inline ActionsI think this comment dates back to the initial revision of the patch, which had m_defrag(m); nvlist_add_binary(... mtod(m, uint8_t *), m->m_pkthdr.len); Now it's ok because we allocate a contiguous buffer, copy the packet, and move it into the nvlist. markj: I think this comment dates back to the initial revision of the patch, which had
```
m_defrag… | ||||||||||||
ret = ovpn_add_counters(nvl, "tunnel", sc->tunnel_bytes_received, | ||||||||||||
sc->tunnel_bytes_received); | ||||||||||||
if (ret != 0) | ||||||||||||
goto error; | ||||||||||||
ret = ovpn_add_counters(nvl, "transport", sc->transport_bytes_received, | ||||||||||||
sc->transport_bytes_received); | ||||||||||||
if (ret != 0) | ||||||||||||
goto error; | ||||||||||||
*onvl = nvl; | ||||||||||||
return (0); | ||||||||||||
error: | ||||||||||||
nvlist_destroy(nvl); | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
Done Inline Actions
markj: | ||||||||||||
static int | ||||||||||||
ovpn_poll_pkt(struct ovpn_softc *sc, nvlist_t **onvl) | ||||||||||||
{ | ||||||||||||
nvlist_t *nvl; | ||||||||||||
nvl = nvlist_create(0); | ||||||||||||
if (nvl == NULL) | ||||||||||||
return (ENOMEM); | ||||||||||||
nvlist_add_number(nvl, "pending", | ||||||||||||
buf_ring_count(sc->rxring) + buf_ring_count(sc->notifring)); | ||||||||||||
*onvl = nvl; | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
opvn_get_pkt(struct ovpn_softc *sc, nvlist_t **onvl) | ||||||||||||
{ | ||||||||||||
struct ovpn_notification *n; | ||||||||||||
struct mbuf *m; | ||||||||||||
uint8_t *buf; | ||||||||||||
nvlist_t *nvl; | ||||||||||||
/* Check if we have notifications pending. */ | ||||||||||||
n = buf_ring_dequeue_mc(sc->notifring); | ||||||||||||
if (n != NULL) { | ||||||||||||
nvl = nvlist_create(0); | ||||||||||||
if (nvl == NULL) { | ||||||||||||
free(n, M_OVPN); | ||||||||||||
return (ENOMEM); | ||||||||||||
} | ||||||||||||
nvlist_add_number(nvl, "peerid", n->peerid); | ||||||||||||
nvlist_add_number(nvl, "notification", n->type); | ||||||||||||
free(n, M_OVPN); | ||||||||||||
*onvl = nvl; | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
/* Queued packet. */ | ||||||||||||
m = buf_ring_dequeue_mc(sc->rxring); | ||||||||||||
if (m == NULL) | ||||||||||||
return (ENOENT); | ||||||||||||
buf = malloc(m_length(m, NULL), M_OVPN, M_WAITOK); | ||||||||||||
m_copydata(m, 0, m_length(m, NULL), buf); | ||||||||||||
nvl = nvlist_create(0); | ||||||||||||
if (nvl == NULL) { | ||||||||||||
counter_u64_add(sc->lost_ctrl_pkts_in, 1); | ||||||||||||
m_freem(m); | ||||||||||||
free(buf, M_OVPN); | ||||||||||||
Done Inline ActionsStyle, || should appear at the end of the previous line. markj: Style, `||` should appear at the end of the previous line. | ||||||||||||
return (ENOMEM); | ||||||||||||
} | ||||||||||||
nvlist_add_binary(nvl, "packet", buf, m_length(m, NULL)); | ||||||||||||
nvlist_add_number(nvl, "peerid", sc->keys[OVPN_KEY_SLOT_PRIMARY].peerid); | ||||||||||||
*onvl = nvl; | ||||||||||||
m_freem(m); | ||||||||||||
free(buf, M_OVPN); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_ioctl_get(struct ifnet *ifp, struct ifdrv *ifd) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
nvlist_t *nvl = NULL; | ||||||||||||
int error; | ||||||||||||
switch (ifd->ifd_cmd) { | ||||||||||||
case OVPN_GET_STATS: | ||||||||||||
error = ovpn_get_stats(sc, &nvl); | ||||||||||||
break; | ||||||||||||
case OVPN_POLL_PKT: | ||||||||||||
error = ovpn_poll_pkt(sc, &nvl); | ||||||||||||
break; | ||||||||||||
case OVPN_GET_PKT: | ||||||||||||
error = opvn_get_pkt(sc, &nvl); | ||||||||||||
break; | ||||||||||||
default: | ||||||||||||
error = ENOTSUP; | ||||||||||||
break; | ||||||||||||
} | ||||||||||||
if (error == 0) { | ||||||||||||
void *packed = NULL; | ||||||||||||
size_t len; | ||||||||||||
MPASS(nvl != NULL); | ||||||||||||
packed = nvlist_pack(nvl, &len); | ||||||||||||
if (! packed) { | ||||||||||||
nvlist_destroy(nvl); | ||||||||||||
return (ENOMEM); | ||||||||||||
} | ||||||||||||
if (len > ifd->ifd_len) { | ||||||||||||
free(packed, M_NVLIST); | ||||||||||||
nvlist_destroy(nvl); | ||||||||||||
return (ENOSPC); | ||||||||||||
} | ||||||||||||
error = copyout(packed, ifd->ifd_data, len); | ||||||||||||
ifd->ifd_len = len; | ||||||||||||
free(packed, M_NVLIST); | ||||||||||||
nvlist_destroy(nvl); | ||||||||||||
} | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) | ||||||||||||
{ | ||||||||||||
struct ifdrv *ifd; | ||||||||||||
int error = EINVAL; | ||||||||||||
switch (cmd) { | ||||||||||||
case SIOCSDRVSPEC: | ||||||||||||
ifd = (struct ifdrv *)data; | ||||||||||||
error = ovpn_ioctl_set(ifp, ifd); | ||||||||||||
break; | ||||||||||||
case SIOCGDRVSPEC: | ||||||||||||
ifd = (struct ifdrv *)data; | ||||||||||||
error = ovpn_ioctl_get(ifp, ifd); | ||||||||||||
break; | ||||||||||||
case SIOCSIFMTU: { | ||||||||||||
struct ifreq *ifr = (struct ifreq *)data; | ||||||||||||
if (ifr->ifr_mtu < OVPN_MTU_MIN || ifr->ifr_mtu > OVPN_MTU_MAX) | ||||||||||||
return (EINVAL); | ||||||||||||
ifp->if_mtu = ifr->ifr_mtu; | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
case SIOCSIFADDR: | ||||||||||||
case SIOCADDMULTI: | ||||||||||||
case SIOCDELMULTI: | ||||||||||||
case SIOCGIFMTU: | ||||||||||||
case SIOCSIFFLAGS: | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
return (error); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_encrypt_tx_cb(struct cryptop *crp) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = crp->crp_opaque; | ||||||||||||
struct mbuf *m = crp->crp_buf.cb_mbuf; | ||||||||||||
int ret; | ||||||||||||
CURVNET_SET(sc->ifp->if_vnet); | ||||||||||||
MPASS(crp->crp_buf.cb_type == CRYPTO_BUF_MBUF); | ||||||||||||
ret = ovpn_encap(sc, m); | ||||||||||||
if (ret == 0) { | ||||||||||||
counter_u64_add(sc->sent_data_pkts, 1); | ||||||||||||
counter_u64_add(sc->tunnel_bytes_sent, m->m_pkthdr.len - | ||||||||||||
sizeof(struct ovpn_wire_header)); | ||||||||||||
} | ||||||||||||
CURVNET_RESTORE(); | ||||||||||||
crypto_freereq(crp); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_finish_rx(struct ovpn_softc *sc, struct mbuf *m, | ||||||||||||
struct ovpn_kkey *key, uint32_t seq, | ||||||||||||
struct rm_priotracker *_ovpn_lock_trackerp) | ||||||||||||
{ | ||||||||||||
uint32_t af; | ||||||||||||
int ret __diagused; | ||||||||||||
OVPN_RASSERT(sc); | ||||||||||||
#ifdef ENABLE_RX_REPLAY_PROTECTION | ||||||||||||
/* Replay protection. */ | ||||||||||||
if (! ovpn_check_replay(key->decrypt, seq)) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
/* Valid packet, kick the timeout watchdog. */ | ||||||||||||
Done Inline Actionsmay be add here OVPN_UNLOCK_ASSERT() to improve readability? ae: may be add here OVPN_UNLOCK_ASSERT() to improve readability? | ||||||||||||
if (sc->keepalive.timeout) | ||||||||||||
callout_reset(&sc->ping_rcv, sc->keepalive.timeout * hz, | ||||||||||||
ovpn_timeout, sc); | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->received_data_pkts, 1); | ||||||||||||
counter_u64_add(sc->tunnel_bytes_received, m->m_pkthdr.len); | ||||||||||||
/* Receive the packet on our interface. */ | ||||||||||||
m->m_pkthdr.rcvif = sc->ifp; | ||||||||||||
/* | ||||||||||||
* Check for address family, and disregard any control packets (e.g. | ||||||||||||
* keepalive). | ||||||||||||
*/ | ||||||||||||
af = ovpn_get_af(m); | ||||||||||||
if (af != 0) { | ||||||||||||
Done Inline ActionsMissing parentheses. And probably #ifdef INET/INET6. But there already several places where them are missed. ae: Missing parentheses. And probably #ifdef INET/INET6. But there already several places where… | ||||||||||||
BPF_MTAP2(sc->ifp, &af, sizeof(af), m); | ||||||||||||
ret = netisr_dispatch(af == AF_INET ? NETISR_IP : NETISR_IPV6, | ||||||||||||
m); | ||||||||||||
MPASS(ret == 0); | ||||||||||||
} else { | ||||||||||||
m_freem(m); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
static struct ovpn_kkey * | ||||||||||||
ovpn_find_key(struct ovpn_softc *sc, const struct ovpn_wire_header *ohdr) | ||||||||||||
{ | ||||||||||||
struct ovpn_kkey *key = NULL; | ||||||||||||
uint32_t peerid; | ||||||||||||
uint8_t keyid; | ||||||||||||
Not Done Inline ActionsJust musing, but it seems kind of lame that you can't specify M_WAITOK here... markj: Just musing, but it seems kind of lame that you can't specify M_WAITOK here... | ||||||||||||
OVPN_RASSERT(sc); | ||||||||||||
keyid = (ntohl(ohdr->opcode) >> 24) & 0x07; | ||||||||||||
peerid = ntohl(ohdr->opcode) & 0x00ffffff; | ||||||||||||
if (sc->keys[0].keyid == keyid && sc->keys[0].peerid == peerid) | ||||||||||||
key = &sc->keys[0]; | ||||||||||||
else if (sc->keys[1].keyid == keyid && sc->keys[1].peerid == peerid) | ||||||||||||
key = &sc->keys[1]; | ||||||||||||
return (key); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_encrypt_rx_cb(struct cryptop *crp) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = crp->crp_opaque; | ||||||||||||
struct mbuf *m = crp->crp_buf.cb_mbuf; | ||||||||||||
struct ovpn_kkey *key; | ||||||||||||
Done Inline ActionsCaching the return value of m_length() might be worthwhile since it has to walk the chain each time. markj: Caching the return value of m_length() might be worthwhile since it has to walk the chain each… | ||||||||||||
struct ovpn_wire_header *ohdr; | ||||||||||||
OVPN_RLOCK_TRACKER; | ||||||||||||
MPASS(crp->crp_buf.cb_type == CRYPTO_BUF_MBUF); | ||||||||||||
OVPN_RLOCK(sc); | ||||||||||||
CURVNET_SET(sc->ifp->if_vnet); | ||||||||||||
Done Inline ActionsIt seems you didn't do any checks for if_mtu in SIOCGIFMTU handling. Did you test it with different values? ae: It seems you didn't do any checks for if_mtu in SIOCGIFMTU handling. Did you test it with… | ||||||||||||
ohdr = mtodo(m, sizeof(struct udphdr)); | ||||||||||||
key = ovpn_find_key(sc, ohdr); | ||||||||||||
if (key == NULL) { | ||||||||||||
Done Inline ActionsIf you use nvlist_move_binary(), you can avoid having to make another copy of the packet (the memory allocation for which might fail). markj: If you use nvlist_move_binary(), you can avoid having to make another copy of the packet (the… | ||||||||||||
/* | ||||||||||||
* Has this key been removed between us starting the decrypt | ||||||||||||
* and finishing it? | ||||||||||||
*/ | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
m_freem(m); | ||||||||||||
CURVNET_RESTORE(); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
/* Now remove the outer headers */ | ||||||||||||
m_adj_decap(m, sizeof(struct udphdr) + | ||||||||||||
sizeof(struct ovpn_wire_header)); | ||||||||||||
ovpn_finish_rx(sc, m, key, ntohl(ohdr->seq), _ovpn_lock_trackerp); | ||||||||||||
OVPN_UNLOCK_ASSERT(sc); | ||||||||||||
CURVNET_RESTORE(); | ||||||||||||
crypto_freereq(crp); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static uint8_t EMPTY_BUFFER[AES_BLOCK_LEN]; | ||||||||||||
static int | ||||||||||||
ovpn_get_af(struct mbuf *m) | ||||||||||||
{ | ||||||||||||
struct ip *ip; | ||||||||||||
Done Inline ActionsIt seems you miss OVPN_RUNLOCK() here. ae: It seems you miss OVPN_RUNLOCK() here.
And probably m_freem() too. | ||||||||||||
struct ip6_hdr *ip6; | ||||||||||||
/* | ||||||||||||
* We should pullup, but we're only interested in the first byte, so | ||||||||||||
* that'll always be contiguous. | ||||||||||||
*/ | ||||||||||||
ip = mtod(m, struct ip *); | ||||||||||||
if (ip->ip_v == IPVERSION) | ||||||||||||
return (AF_INET); | ||||||||||||
ip6 = mtod(m, struct ip6_hdr *); | ||||||||||||
if (ip6->ip6_vfc == IPV6_VERSION) | ||||||||||||
return (AF_INET6); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_transmit(struct ifnet *ifp, struct mbuf *m) | ||||||||||||
{ | ||||||||||||
struct ovpn_wire_header *ohdr; | ||||||||||||
struct ovpn_kkey *key; | ||||||||||||
struct ovpn_softc *sc; | ||||||||||||
struct cryptop *crp; | ||||||||||||
uint32_t af, seq; | ||||||||||||
size_t len, real_len, ovpn_hdr_len; | ||||||||||||
int tunnel_len; | ||||||||||||
int ret; | ||||||||||||
OVPN_RLOCK_TRACKER; | ||||||||||||
sc = ifp->if_softc; | ||||||||||||
if (ifp->if_link_state != LINK_STATE_UP) { | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
return (ENETDOWN); | ||||||||||||
} | ||||||||||||
tunnel_len = m->m_pkthdr.len; | ||||||||||||
OVPN_RLOCK(sc); | ||||||||||||
key = &sc->keys[OVPN_KEY_SLOT_PRIMARY]; | ||||||||||||
MPASS(key->encrypt != NULL); | ||||||||||||
Done Inline ActionsWe need some priv_check() here, or for the individual ioctls depending on what they do (I guess things like fetching stats are ok). Right now I think there's nothing preventing an unprivileged user from issuing get/set ioctls to any ovpn interface. The kernel does this check for us for SIOCSIFMTU. markj: We need some `priv_check()` here, or for the individual ioctls depending on what they do (I… | ||||||||||||
af = ovpn_get_af(m); | ||||||||||||
Done Inline ActionsThis clobbers the first assignment to error, so unhandled ioctls will silently succeed. markj: This clobbers the first assignment to `error`, so unhandled ioctls will silently succeed. | ||||||||||||
/* Don't capture control packets. */ | ||||||||||||
if (af != 0) | ||||||||||||
BPF_MTAP2(ifp, &af, sizeof(af), m); | ||||||||||||
real_len = len = m->m_pkthdr.len; | ||||||||||||
MPASS(real_len <= ifp->if_mtu); | ||||||||||||
ovpn_hdr_len = sizeof(struct ovpn_wire_header); | ||||||||||||
if (key->encrypt->cipher == OVPN_CIPHER_ALG_NONE) | ||||||||||||
ovpn_hdr_len -= 16; /* No auth tag. */ | ||||||||||||
else { | ||||||||||||
/* Round up the len to a multiple of our block size. */ | ||||||||||||
len = roundup2(real_len, AES_BLOCK_LEN); | ||||||||||||
/* Now extend the mbuf. */ | ||||||||||||
m_append(m, len - real_len, EMPTY_BUFFER); | ||||||||||||
} | ||||||||||||
M_PREPEND(m, ovpn_hdr_len, M_NOWAIT); | ||||||||||||
if (m == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
return (ENOBUFS); | ||||||||||||
} | ||||||||||||
ohdr = mtod(m, struct ovpn_wire_header *); | ||||||||||||
ohdr->opcode = (OVPN_OP_DATA_V2 << OVPN_OP_SHIFT) | key->keyid; | ||||||||||||
ohdr->opcode <<= 24; | ||||||||||||
ohdr->opcode |= key->peerid; | ||||||||||||
ohdr->opcode = htonl(ohdr->opcode); | ||||||||||||
seq = atomic_fetchadd_32(&sc->tx_seq, 1); | ||||||||||||
seq = htonl(seq); | ||||||||||||
ohdr->seq = seq; | ||||||||||||
if (key->encrypt->cipher == OVPN_CIPHER_ALG_NONE) { | ||||||||||||
ret = ovpn_encap(sc, m); | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
if (ret == 0) { | ||||||||||||
counter_u64_add(sc->sent_data_pkts, 1); | ||||||||||||
counter_u64_add(sc->tunnel_bytes_sent, tunnel_len); | ||||||||||||
} | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
crp = crypto_getreq(key->encrypt->cryptoid, M_NOWAIT); | ||||||||||||
if (crp == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return (ENOBUFS); | ||||||||||||
} | ||||||||||||
/* Encryption covers only the payload, not the header. */ | ||||||||||||
crp->crp_payload_start = sizeof(*ohdr); | ||||||||||||
Done Inline ActionsWhy is this an ifdef instead of a runtime option? markj: Why is this an ifdef instead of a runtime option? | ||||||||||||
crp->crp_payload_length = len; | ||||||||||||
crp->crp_op = CRYPTO_OP_ENCRYPT; | ||||||||||||
/* | ||||||||||||
* AAD data covers the ovpn_wire_header minus the auth | ||||||||||||
* tag. | ||||||||||||
*/ | ||||||||||||
crp->crp_aad_length = sizeof(*ohdr) - | ||||||||||||
sizeof(ohdr->auth_tag); | ||||||||||||
crp->crp_aad = ohdr; | ||||||||||||
crp->crp_aad_start = 0; | ||||||||||||
crp->crp_op |= CRYPTO_OP_COMPUTE_DIGEST; | ||||||||||||
crp->crp_digest_start = | ||||||||||||
offsetof(struct ovpn_wire_header, auth_tag); | ||||||||||||
crp->crp_flags |= CRYPTO_F_IV_SEPARATE; | ||||||||||||
memcpy(crp->crp_iv, &seq, sizeof(seq)); | ||||||||||||
memcpy(crp->crp_iv + sizeof(seq), key->encrypt->nonce, | ||||||||||||
key->encrypt->noncelen); | ||||||||||||
crypto_use_mbuf(crp, m); | ||||||||||||
crp->crp_flags |= CRYPTO_F_CBIFSYNC; | ||||||||||||
crp->crp_callback = ovpn_encrypt_tx_cb; | ||||||||||||
crp->crp_opaque = sc; | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
ret = crypto_dispatch(crp); | ||||||||||||
if (ret) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
Not Done Inline ActionsIt seems pretty expensive to do this for every packet. markj: It seems pretty expensive to do this for every packet. | ||||||||||||
Done Inline ActionsGood catch. The first potential fix that comes to mind is to not reset the callout, but to track the timestamp of the last packet (with second level accuracy). We can then perform the check in the ovpn_timeout_peer() callback and either restart the callout or handle a timeout. It may be worth replicating the trick Reid used in pf (https://cgit.freebsd.org/src/commit/sys?id=0abcc1d2d33aef2333dab28e1ec1858cf45b314a), where we have a per-cpu variable to write to. If we go that route it might be worth abstracting that a little. kp: Good catch.
The first potential fix that comes to mind is to not reset the callout, but to… | ||||||||||||
m_freem(m); | ||||||||||||
} | ||||||||||||
return (ret); | ||||||||||||
} | ||||||||||||
/* | ||||||||||||
* Note: Expects to hold the read lock on entry, and will release it itself. | ||||||||||||
*/ | ||||||||||||
static int | ||||||||||||
ovpn_encap(struct ovpn_softc *sc, struct mbuf *m) | ||||||||||||
{ | ||||||||||||
struct udphdr *udp; | ||||||||||||
int len; | ||||||||||||
OVPN_RLOCK_TRACKER; | ||||||||||||
OVPN_RLOCK(sc); | ||||||||||||
NET_EPOCH_ASSERT(); | ||||||||||||
if (sc->ifp->if_link_state != LINK_STATE_UP) { | ||||||||||||
Done Inline ActionsCount this drop perhaps? markj: Count this drop perhaps? | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return (ENETDOWN); | ||||||||||||
} | ||||||||||||
len = m->m_pkthdr.len; | ||||||||||||
Done Inline ActionsThis function needs to check crp->crp_etype. Currently I think it doesn't detect digest verification failures? In particular, such failures are not returned directly from crypto_dispatch() since cryptop processing may happen asynchronously. markj: This function needs to check `crp->crp_etype`. Currently I think it doesn't detect digest… | ||||||||||||
M_PREPEND(m, sizeof(struct udphdr), M_NOWAIT); | ||||||||||||
if (m == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return (ENOBUFS); | ||||||||||||
} | ||||||||||||
udp = mtod(m, struct udphdr *); | ||||||||||||
MPASS(sc->peer.local.ss_family == sc->peer.remote.ss_family); | ||||||||||||
udp->uh_sport = ovpn_get_port(&sc->peer.local); | ||||||||||||
udp->uh_dport = ovpn_get_port(&sc->peer.remote); | ||||||||||||
udp->uh_ulen = htons(sizeof(struct udphdr) + len); | ||||||||||||
switch (sc->peer.remote.ss_family) { | ||||||||||||
#ifdef INET | ||||||||||||
case AF_INET: { | ||||||||||||
struct sockaddr_in *in_local = TO_IN(&sc->peer.local); | ||||||||||||
struct sockaddr_in *in_remote = TO_IN(&sc->peer.remote); | ||||||||||||
struct ip *ip; | ||||||||||||
/* | ||||||||||||
* This requires knowing the source IP, which we don't. Happily | ||||||||||||
* we're allowed to keep this at 0, and the checksum won't do | ||||||||||||
* anything the crypto won't already do. | ||||||||||||
*/ | ||||||||||||
udp->uh_sum = 0; | ||||||||||||
/* Set the checksum flags so we recalculate checksums. */ | ||||||||||||
m->m_pkthdr.csum_flags |= CSUM_IP; | ||||||||||||
m->m_pkthdr.csum_data = offsetof(struct udphdr, uh_sum); | ||||||||||||
M_PREPEND(m, sizeof(struct ip), M_NOWAIT); | ||||||||||||
if (m == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
Done Inline ActionsSince the request is handled synchronously, you could perhaps save a trip to the memory allocator and put the cryptop on the stack. markj: Since the request is handled synchronously, you could perhaps save a trip to the memory… | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
return (ENOBUFS); | ||||||||||||
} | ||||||||||||
ip = mtod(m, struct ip *); | ||||||||||||
ip->ip_tos = 0; | ||||||||||||
ip->ip_len = htons(sizeof(struct ip) + sizeof(struct udphdr) + | ||||||||||||
len); | ||||||||||||
ip->ip_off = 0; | ||||||||||||
ip->ip_ttl = V_ip_defttl; | ||||||||||||
ip->ip_p = IPPROTO_UDP; | ||||||||||||
ip->ip_sum = 0; | ||||||||||||
if (in_local->sin_port != 0) | ||||||||||||
ip->ip_src = in_local->sin_addr; | ||||||||||||
else | ||||||||||||
ip->ip_src.s_addr = INADDR_ANY; | ||||||||||||
ip->ip_dst = in_remote->sin_addr; | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->transport_bytes_sent, m->m_pkthdr.len); | ||||||||||||
return (ip_output(m, NULL, NULL, 0, NULL, NULL)); | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
#ifdef INET6 | ||||||||||||
case AF_INET6: { | ||||||||||||
struct sockaddr_in6 *in6_local = TO_IN6(&sc->peer.local); | ||||||||||||
struct sockaddr_in6 *in6_remote = TO_IN6(&sc->peer.remote); | ||||||||||||
struct ip6_hdr *ip6; | ||||||||||||
M_PREPEND(m, sizeof(struct ip6_hdr), M_NOWAIT); | ||||||||||||
if (m == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_out, 1); | ||||||||||||
return (ENOBUFS); | ||||||||||||
} | ||||||||||||
Done Inline Actionscrp might have been freed if we slept waiting for a wakeup. markj: `crp` might have been freed if we slept waiting for a wakeup. | ||||||||||||
Done Inline ActionsFor what it's worth, I suspect you'll get poor throughput using hw offload since the dispatching thread is blocked until the request completes. Actually, I'm not sure if sleeping here is permitted at all? markj: For what it's worth, I suspect you'll get poor throughput using hw offload since the… | ||||||||||||
ip6 = mtod(m, struct ip6_hdr *); | ||||||||||||
ip6->ip6_vfc = IPV6_VERSION; | ||||||||||||
ip6->ip6_flow &= ~IPV6_FLOWINFO_MASK; | ||||||||||||
Done Inline Actions
markj: | ||||||||||||
ip6->ip6_plen = htons(sizeof(*ip6) + sizeof(struct udphdr) + | ||||||||||||
len); | ||||||||||||
ip6->ip6_nxt = IPPROTO_UDP; | ||||||||||||
ip6->ip6_hlim = V_ip6_defhlim; | ||||||||||||
memcpy(&ip6->ip6_src, &in6_local->sin6_addr, | ||||||||||||
sizeof(ip6->ip6_src)); | ||||||||||||
memcpy(&ip6->ip6_dst, &in6_remote->sin6_addr, | ||||||||||||
sizeof(ip6->ip6_dst)); | ||||||||||||
udp->uh_sum = in6_cksum_pseudo(ip6, sizeof(struct udphdr), | ||||||||||||
IPPROTO_UDP, 0); | ||||||||||||
m->m_pkthdr.csum_flags |= CSUM_UDP_IPV6; | ||||||||||||
m->m_pkthdr.csum_data = offsetof(struct udphdr, uh_sum); | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->transport_bytes_sent, m->m_pkthdr.len); | ||||||||||||
return (ip6_output(m, NULL, NULL, IPV6_UNSPECSRC, NULL, NULL, | ||||||||||||
NULL)); | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
default: | ||||||||||||
panic("Unsupported address family %d", | ||||||||||||
sc->peer.remote.ss_family); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_output(struct ifnet *ifp, struct mbuf *m, const struct sockaddr *dst, | ||||||||||||
struct route *ro) | ||||||||||||
{ | ||||||||||||
return (ifp->if_transmit(ifp, m)); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_rcv_ctrl(struct ovpn_softc *sc, struct mbuf *m, int off) | ||||||||||||
{ | ||||||||||||
/* Lop off the IP and UDP headers */ | ||||||||||||
m_adj_decap(m, off); | ||||||||||||
/* Keep in the local ring until userspace fetches it. */ | ||||||||||||
if (buf_ring_enqueue(sc->rxring, m) != 0) { | ||||||||||||
counter_u64_add(sc->lost_ctrl_pkts_in, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
counter_u64_add(sc->received_ctrl_pkts, 1); | ||||||||||||
} | ||||||||||||
#ifdef ENABLE_RX_REPLAY_PROTECTION | ||||||||||||
static bool | ||||||||||||
ovpn_check_replay(struct ovpn_kkey_dir *key, uint32_t seq) | ||||||||||||
{ | ||||||||||||
uint32_t d; | ||||||||||||
Done Inline ActionsI think this needs to be OPVN_WASSERT() since you're using buf_ring_dequeue_sc()? The write lock is held in practice. markj: I think this needs to be OPVN_WASSERT() since you're using buf_ring_dequeue_sc()? The write… | ||||||||||||
mtx_lock(&key->replay_mtx); | ||||||||||||
/* Sequence number must be strictly greater than rx_seq */ | ||||||||||||
if (seq <= key->rx_seq) { | ||||||||||||
mtx_unlock(&key->replay_mtx); | ||||||||||||
return (false); | ||||||||||||
} | ||||||||||||
/* Large jump. The packet authenticated okay, so just accept that. */ | ||||||||||||
if (seq > (key->rx_seq + (sizeof(key->rx_window) * 8))) { | ||||||||||||
key->rx_seq = seq; | ||||||||||||
key->rx_window = 0; | ||||||||||||
mtx_unlock(&key->replay_mtx); | ||||||||||||
Done Inline ActionsAlso, to improve readability, can you add OVPN_UNLOCK_ASSERT() here? ae: Also, to improve readability, can you add OVPN_UNLOCK_ASSERT() here? | ||||||||||||
return (true); | ||||||||||||
} | ||||||||||||
/* Happy case. */ | ||||||||||||
if ((seq == key->rx_seq + 1) && key->rx_window == 0) { | ||||||||||||
key->rx_seq++; | ||||||||||||
mtx_unlock(&key->replay_mtx); | ||||||||||||
return (true); | ||||||||||||
} | ||||||||||||
d = seq - key->rx_seq - 1; | ||||||||||||
if (key->rx_window & (1 << d)) { | ||||||||||||
/* Dupe! */ | ||||||||||||
mtx_unlock(&key->replay_mtx); | ||||||||||||
return (false); | ||||||||||||
} | ||||||||||||
key->rx_window |= 1 << d; | ||||||||||||
while (key->rx_window & 1) { | ||||||||||||
key->rx_seq++; | ||||||||||||
key->rx_window >>= 1; | ||||||||||||
} | ||||||||||||
mtx_unlock(&key->replay_mtx); | ||||||||||||
return (true); | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
static void | ||||||||||||
ovpn_udp_input(struct mbuf *m, int off, struct inpcb *inp, | ||||||||||||
const struct sockaddr *sa, void *ctx) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ctx; | ||||||||||||
struct ovpn_wire_header *ohdr; | ||||||||||||
struct udphdr *uhdr; | ||||||||||||
struct ovpn_kkey *key; | ||||||||||||
struct cryptop *crp; | ||||||||||||
size_t ohdrlen; | ||||||||||||
int ret; | ||||||||||||
uint8_t op; | ||||||||||||
M_ASSERTPKTHDR(m); | ||||||||||||
OVPN_RLOCK_TRACKER; | ||||||||||||
/* | ||||||||||||
* Simplify things by getting rid of the preceding headers, we don't | ||||||||||||
* care about them. | ||||||||||||
*/ | ||||||||||||
m_adj_decap(m, off); | ||||||||||||
counter_u64_add(sc->transport_bytes_received, m->m_pkthdr.len); | ||||||||||||
ohdrlen = sizeof(*ohdr) - sizeof(ohdr->auth_tag); | ||||||||||||
/* | ||||||||||||
* Get the UDP and OpenVPN header into a contiguous buffer. | ||||||||||||
* Note that we do not guarantee that there's an auth tag here. Control | ||||||||||||
* packets may not contain one, and be short enough for this to fail. | ||||||||||||
*/ | ||||||||||||
m = m_pullup(m, sizeof(*uhdr) + ohdrlen); | ||||||||||||
if (m == NULL) { | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
ohdr = mtodo(m, sizeof(struct udphdr)); | ||||||||||||
op = ntohl(ohdr->opcode) >> 24 >> OVPN_OP_SHIFT; | ||||||||||||
if (op != OVPN_OP_DATA_V2) { | ||||||||||||
ovpn_rcv_ctrl(sc, m, sizeof(struct udphdr)); | ||||||||||||
INP_WLOCK(inp); | ||||||||||||
udp_notify(inp, EAGAIN); | ||||||||||||
INP_WUNLOCK(inp); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
OVPN_RLOCK(sc); | ||||||||||||
key = ovpn_find_key(sc, ohdr); | ||||||||||||
if (key == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
MPASS(key->decrypt != NULL); | ||||||||||||
if (key->decrypt->cipher == OVPN_CIPHER_ALG_NONE) { | ||||||||||||
Done Inline ActionsMissing m_freem(m)? markj: Missing `m_freem(m)`? | ||||||||||||
/* Now remove the outer headers */ | ||||||||||||
m_adj_decap(m, sizeof(struct udphdr) + ohdrlen); | ||||||||||||
ovpn_finish_rx(sc, m, key, ntohl(ohdr->seq), | ||||||||||||
_ovpn_lock_trackerp); | ||||||||||||
OVPN_UNLOCK_ASSERT(sc); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
ohdrlen += sizeof(ohdr->auth_tag); | ||||||||||||
/* Now make sure the auth tag is in contiguous memory. */ | ||||||||||||
m = m_pullup(m, sizeof(struct udphdr) + ohdrlen); | ||||||||||||
if (m == NULL) { | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
uhdr = mtodo(m, 0); | ||||||||||||
Done Inline Actions
melifaro: | ||||||||||||
ohdr = mtodo(m, sizeof(struct udphdr)); | ||||||||||||
/* Decrypt */ | ||||||||||||
crp = crypto_getreq(key->decrypt->cryptoid, M_NOWAIT); | ||||||||||||
if (crp == NULL) { | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
m_freem(m); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
Done Inline ActionsThere's no need to use both _stop and _drain if you're in a context where sleeping is permitted. callout_drain() on its own is sufficient. markj: There's no need to use both _stop and _drain if you're in a context where sleeping is permitted. | ||||||||||||
crp->crp_payload_start = sizeof(struct udphdr) + sizeof(*ohdr); | ||||||||||||
crp->crp_payload_length = ntohs(uhdr->uh_ulen) - | ||||||||||||
sizeof(*uhdr) - sizeof(*ohdr); | ||||||||||||
crp->crp_op = CRYPTO_OP_DECRYPT; | ||||||||||||
/* AAD validation. */ | ||||||||||||
crp->crp_aad_length = sizeof(*ohdr) - | ||||||||||||
sizeof(ohdr->auth_tag); | ||||||||||||
Done Inline Actions
melifaro: | ||||||||||||
crp->crp_aad = ohdr; | ||||||||||||
crp->crp_aad_start = 0; | ||||||||||||
crp->crp_op |= CRYPTO_OP_VERIFY_DIGEST; | ||||||||||||
crp->crp_digest_start = sizeof(struct udphdr) + | ||||||||||||
offsetof(struct ovpn_wire_header, auth_tag); | ||||||||||||
crp->crp_flags |= CRYPTO_F_IV_SEPARATE; | ||||||||||||
memcpy(crp->crp_iv, &ohdr->seq, sizeof(ohdr->seq)); | ||||||||||||
memcpy(crp->crp_iv + sizeof(ohdr->seq), key->decrypt->nonce, | ||||||||||||
key->decrypt->noncelen); | ||||||||||||
crypto_use_mbuf(crp, m); | ||||||||||||
crp->crp_flags |= CRYPTO_F_CBIFSYNC; | ||||||||||||
crp->crp_callback = ovpn_encrypt_rx_cb; | ||||||||||||
crp->crp_opaque = sc; | ||||||||||||
OVPN_RUNLOCK(sc); | ||||||||||||
ret = crypto_dispatch(crp); | ||||||||||||
if (ret != 0) { | ||||||||||||
counter_u64_add(sc->lost_data_pkts_in, 1); | ||||||||||||
m_freem(m); | ||||||||||||
return; | ||||||||||||
} | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_qflush(struct ifnet *ifp __unused) | ||||||||||||
{ | ||||||||||||
} | ||||||||||||
Done Inline ActionsThis can be one line. markj: This can be one line. | ||||||||||||
static void | ||||||||||||
ovpn_flush_rxring(struct ovpn_softc *sc) | ||||||||||||
{ | ||||||||||||
struct mbuf *m; | ||||||||||||
struct ovpn_notification *n; | ||||||||||||
Done Inline ActionsThis can be one line. markj: This can be one line. | ||||||||||||
OVPN_WASSERT(sc); | ||||||||||||
while (! buf_ring_empty(sc->rxring)) { | ||||||||||||
m = buf_ring_dequeue_sc(sc->rxring); | ||||||||||||
m_freem(m); | ||||||||||||
} | ||||||||||||
while (! buf_ring_empty(sc->notifring)) { | ||||||||||||
n = buf_ring_dequeue_sc(sc->notifring); | ||||||||||||
Not Done Inline ActionsThe read lock is dropped before crypto processing, so it looks like peer can be freed in the window before ovpn_encrypt_tx_cb() is called. For instance if the peer times out. Holding the lock across the crypto_dispatch() call isn't sufficient since with hardware offload drivers the callback is executed asynchronously. Similarly I believe the softc reference is not protected in the asynchronous case; net_epoch isn't enough there. markj: The read lock is dropped before crypto processing, so it looks like `peer` can be freed in the… | ||||||||||||
Not Done Inline ActionsHmm. Do you have any ideas on how to cope with that? We're kind of stuck passing at least the softc, both for counters and in some cases because we need to look up the peer (in ovpn_encap(), from ovpn_encrypt_tx_cb()). kp: Hmm. Do you have any ideas on how to cope with that?
We're kind of stuck passing at least the… | ||||||||||||
Not Done Inline ActionsAn atomic reference count is the standard way to handle it. That's how IPSec solves this problem. If you ignore the possibility of hardware offloads, then I believe it would be safe to simply hold the softc lock across the crypto_dispatch() call. The ping_send callout does this. I don't believe any of our hw offload drivers implement CHACHA20, for what it's worth, though that of course could change. If you want to be able to make use of hardware offloads and need to avoid the overhead of a global refcount, then per-CPU reference counting for each peer would probably be the way to go. I don't believe we have a generic implementation of that, though. I would probably try adding per-peer atomic refcounting and see how affects throughput, since that's easy to implement and think about, and can be replaced with something more sophisticated later. markj: An atomic reference count is the standard way to handle it. That's how IPSec solves this… | ||||||||||||
free(n, M_OVPN); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
#ifdef VIMAGE | ||||||||||||
static void | ||||||||||||
ovpn_reassign(struct ifnet *ifp, struct vnet *new_vnet __unused, | ||||||||||||
char *unused __unused) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc = ifp->if_softc; | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
ovpn_flush_rxring(sc); | ||||||||||||
/* Flush keys & configuration. */ | ||||||||||||
memset(&sc->peer, 0, sizeof(sc->peer)); | ||||||||||||
for (int i = 0; i < 2; i++) { | ||||||||||||
ovpn_free_kkey_dir(sc->keys[i].encrypt); | ||||||||||||
ovpn_free_kkey_dir(sc->keys[i].decrypt); | ||||||||||||
} | ||||||||||||
memset(&sc->keys, 0, sizeof(sc->keys)); | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
} | ||||||||||||
#endif | ||||||||||||
static int | ||||||||||||
ovpn_clone_match(struct if_clone *ifc, const char *name) | ||||||||||||
{ | ||||||||||||
/* | ||||||||||||
* Allow all names that start with 'ovpn', specifically because pfSense | ||||||||||||
* uses ovpnc1 / ovpns2 | ||||||||||||
*/ | ||||||||||||
return (strncmp(ovpnname, name, strlen(ovpnname)) == 0); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc; | ||||||||||||
Done Inline ActionsI'm not sure. But. Isn't it possible, that at some point some mbuf is waiting for OVPN_RLOCK() in the ovpn_udp_input(), and you trying change/free softc there. ae: I'm not sure. But. Isn't it possible, that at some point some mbuf is waiting for OVPN_RLOCK()… | ||||||||||||
struct ifnet *ifp; | ||||||||||||
char *dp; | ||||||||||||
int error, unit, wildcard; | ||||||||||||
/* Try to see if a special unit was requested. */ | ||||||||||||
error = ifc_name2unit(name, &unit); | ||||||||||||
if (error != 0) | ||||||||||||
return (error); | ||||||||||||
wildcard = (unit < 0); | ||||||||||||
error = ifc_alloc_unit(ifc, &unit); | ||||||||||||
if (error != 0) | ||||||||||||
return (error); | ||||||||||||
/* | ||||||||||||
* If no unit had been given, we need to adjust the ifName. | ||||||||||||
*/ | ||||||||||||
for (dp = name; *dp != '\0'; dp++); | ||||||||||||
if (wildcard) { | ||||||||||||
error = snprintf(dp, len - (dp - name), "%d", unit); | ||||||||||||
if (error > len - (dp - name)) { | ||||||||||||
/* ifName too long. */ | ||||||||||||
ifc_free_unit(ifc, unit); | ||||||||||||
return (ENOSPC); | ||||||||||||
} | ||||||||||||
dp += error; | ||||||||||||
} | ||||||||||||
/* Make sure it doesn't already exist. */ | ||||||||||||
if (ifunit(name) != NULL) | ||||||||||||
return (EEXIST); | ||||||||||||
sc = malloc(sizeof(struct ovpn_softc), M_OVPN, M_WAITOK | M_ZERO); | ||||||||||||
sc->ifp = if_alloc(IFT_ENC); | ||||||||||||
sc->tx_seq = 1; | ||||||||||||
rm_init(&sc->lock, "if_ovpn_lock"); | ||||||||||||
sc->rxring = buf_ring_alloc(32, M_OVPN, M_WAITOK, NULL); | ||||||||||||
sc->notifring = buf_ring_alloc(32, M_OVPN, M_WAITOK, NULL); | ||||||||||||
callout_init(&sc->ping_send, 1); | ||||||||||||
callout_init(&sc->ping_rcv, 1); | ||||||||||||
sc->lost_ctrl_pkts_in = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->lost_ctrl_pkts_out = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->lost_data_pkts_in = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->lost_data_pkts_out = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->received_ctrl_pkts = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->received_data_pkts = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->sent_ctrl_pkts = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->sent_data_pkts = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->transport_bytes_sent = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->transport_bytes_received = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->tunnel_bytes_sent = counter_u64_alloc(M_WAITOK); | ||||||||||||
sc->tunnel_bytes_received = counter_u64_alloc(M_WAITOK); | ||||||||||||
ifp = sc->ifp; | ||||||||||||
ifp->if_softc = sc; | ||||||||||||
strlcpy(ifp->if_xname, name, IFNAMSIZ); | ||||||||||||
ifp->if_dname = ovpngroupname; | ||||||||||||
ifp->if_dunit = unit; | ||||||||||||
ifp->if_addrlen = 0; | ||||||||||||
ifp->if_mtu = 1428; | ||||||||||||
ifp->if_flags = IFF_POINTOPOINT | IFF_MULTICAST; | ||||||||||||
ifp->if_ioctl = ovpn_ioctl; | ||||||||||||
ifp->if_transmit = ovpn_transmit; | ||||||||||||
ifp->if_output = ovpn_output; | ||||||||||||
ifp->if_qflush = ovpn_qflush; | ||||||||||||
#ifdef VIMAGE | ||||||||||||
ifp->if_reassign = ovpn_reassign; | ||||||||||||
#endif | ||||||||||||
ifp->if_capabilities |= IFCAP_LINKSTATE; | ||||||||||||
ifp->if_capenable |= IFCAP_LINKSTATE; | ||||||||||||
if_attach(ifp); | ||||||||||||
bpfattach(ifp, DLT_NULL, sizeof(u_int32_t)); | ||||||||||||
Done Inline ActionsI'm wondering if we can build this prepend once for a peer and then use it via simply memcpy() (maybe with a bit of tiny per-af tweaks). melifaro: I'm wondering if we can build this prepend once for a peer and then use it via simply `memcpy… | ||||||||||||
Done Inline ActionsInteresting idea, but there are some cache invalidation issues. We'd have to be careful about rebuilding it if the peer configuration changes. That's manageable, but we also use V_ip_defttl, so if we cached it we'd end up not changing TTL when the default is changed. Pretty minor of course, but I'm also skeptical that there's a measurable performance improvement in doing this. We'd also still have to tweak things a bit for length and checksum fields, for every packet. kp: Interesting idea, but there are some cache invalidation issues.
We'd have to be careful about… | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
ovpn_clone_destroy_cb(struct epoch_context *ctx) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc; | ||||||||||||
Done Inline Actionsudp might have been invalidated by the earlier M_PREPEND(), I think. markj: `udp` might have been invalidated by the earlier M_PREPEND(), I think. | ||||||||||||
sc = __containerof(ctx, struct ovpn_softc, epoch_ctx); | ||||||||||||
Done Inline ActionsIs it guaranteed that there are no IPv6 extension options following the fixed-length header? markj: Is it guaranteed that there are no IPv6 extension options following the fixed-length header? | ||||||||||||
Done Inline ActionsYes, because we're creating the IPv6 header ourselves here. kp: Yes, because we're creating the IPv6 header ourselves here. | ||||||||||||
counter_u64_free(sc->lost_ctrl_pkts_in); | ||||||||||||
counter_u64_free(sc->lost_ctrl_pkts_out); | ||||||||||||
counter_u64_free(sc->lost_data_pkts_in); | ||||||||||||
counter_u64_free(sc->lost_data_pkts_out); | ||||||||||||
counter_u64_free(sc->received_ctrl_pkts); | ||||||||||||
counter_u64_free(sc->received_data_pkts); | ||||||||||||
counter_u64_free(sc->sent_ctrl_pkts); | ||||||||||||
counter_u64_free(sc->sent_data_pkts); | ||||||||||||
counter_u64_free(sc->transport_bytes_sent); | ||||||||||||
counter_u64_free(sc->transport_bytes_received); | ||||||||||||
counter_u64_free(sc->tunnel_bytes_sent); | ||||||||||||
counter_u64_free(sc->tunnel_bytes_received); | ||||||||||||
if_free(sc->ifp); | ||||||||||||
free(sc, M_OVPN); | ||||||||||||
} | ||||||||||||
static int | ||||||||||||
ovpn_clone_destroy(struct if_clone *ifc, struct ifnet *ifp) | ||||||||||||
{ | ||||||||||||
struct ovpn_softc *sc; | ||||||||||||
int unit; | ||||||||||||
sc = ifp->if_softc; | ||||||||||||
unit = ifp->if_dunit; | ||||||||||||
callout_drain(&sc->ping_send); | ||||||||||||
callout_drain(&sc->ping_rcv); | ||||||||||||
OVPN_WLOCK(sc); | ||||||||||||
ovpn_rele_so(sc); | ||||||||||||
ovpn_flush_rxring(sc); | ||||||||||||
buf_ring_free(sc->rxring, M_OVPN); | ||||||||||||
buf_ring_free(sc->notifring, M_OVPN); | ||||||||||||
for (int i = 0; i < 2; i++) { | ||||||||||||
ovpn_free_kkey_dir(sc->keys[i].encrypt); | ||||||||||||
ovpn_free_kkey_dir(sc->keys[i].decrypt); | ||||||||||||
} | ||||||||||||
OVPN_WUNLOCK(sc); | ||||||||||||
bpfdetach(ifp); | ||||||||||||
if_detach(ifp); | ||||||||||||
ifp->if_softc = NULL; | ||||||||||||
Not Done Inline ActionsThe sequence number can roll over, after which point I think this check will always fail? markj: The sequence number can roll over, after which point I think this check will always fail? | ||||||||||||
Done Inline ActionsYes. The protocol doesn't really have a mechanism to cope with that, but in practice it's not much of a concern because it will negotiate a new key and switch to it (which resets the sequence counter) long before we've passed enough packets for that. kp: Yes. The protocol doesn't really have a mechanism to cope with that, but in practice it's not… | ||||||||||||
NET_EPOCH_CALL(ovpn_clone_destroy_cb, &sc->epoch_ctx); | ||||||||||||
if (unit != IF_DUNIT_NONE) | ||||||||||||
ifc_free_unit(ifc, unit); | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static void | ||||||||||||
vnet_ovpn_init(const void *unused __unused) | ||||||||||||
{ | ||||||||||||
V_ovpn_cloner = if_clone_advanced(ovpngroupname, 0, ovpn_clone_match, | ||||||||||||
ovpn_clone_create, ovpn_clone_destroy); | ||||||||||||
} | ||||||||||||
VNET_SYSINIT(vnet_ovpn_init, SI_SUB_PSEUDO, SI_ORDER_ANY, | ||||||||||||
vnet_ovpn_init, NULL); | ||||||||||||
static void | ||||||||||||
vnet_ovpn_uninit(const void *unused __unused) | ||||||||||||
Not Done Inline ActionsProbably worth having struct route in the peer data & reference it here, so we leverage L3+L2 cache lookups. melifaro: Probably worth having `struct route` in the peer data & reference it here, so we leverage L3+L2… | ||||||||||||
Done Inline ActionsOoh, interesting. Although I worry about locking of the 'struct route'. if_ovpn uses an rmlock and multiple cores may be running ip_output() at the same time. It's also unpredictable if ip_output() will modify the 'struct route', because the routing table could have been updated. (so if_ovpn can't just take the write lock when it has an invalid struct route). I wonder if we should keep a per-CPU 'struct route'. kp: Ooh, interesting.
Although I worry about locking of the 'struct route'. if_ovpn uses an rmlock… | ||||||||||||
{ | ||||||||||||
if_clone_detach(V_ovpn_cloner); | ||||||||||||
Done Inline ActionsProbably needs to be ((uint64_t)1 << d). markj: Probably needs to be `((uint64_t)1 << d)`. | ||||||||||||
} | ||||||||||||
VNET_SYSUNINIT(vnet_ovpn_uninit, SI_SUB_PSEUDO, SI_ORDER_ANY, | ||||||||||||
vnet_ovpn_uninit, NULL); | ||||||||||||
static int | ||||||||||||
ovpnmodevent(module_t mod, int type, void *data) | ||||||||||||
Done Inline ActionsSame here. markj: Same here. | ||||||||||||
{ | ||||||||||||
switch (type) { | ||||||||||||
case MOD_LOAD: | ||||||||||||
/* Done in vnet_ovpn_init() */ | ||||||||||||
break; | ||||||||||||
case MOD_UNLOAD: | ||||||||||||
/* Done in vnet_ovpn_uninit() */ | ||||||||||||
break; | ||||||||||||
default: | ||||||||||||
return (EOPNOTSUPP); | ||||||||||||
} | ||||||||||||
return (0); | ||||||||||||
} | ||||||||||||
static moduledata_t ovpn_mod = { | ||||||||||||
Done Inline ActionsHow do we know that the packet is at least off + sizeof(uhdr) + ohdrlen bytes long? markj: How do we know that the packet is at least `off + sizeof(uhdr) + ohdrlen` bytes long? | ||||||||||||
"if_ovpn", | ||||||||||||
ovpnmodevent, | ||||||||||||
0 | ||||||||||||
}; | ||||||||||||
DECLARE_MODULE(if_ovpn, ovpn_mod, SI_SUB_PSEUDO, SI_ORDER_ANY); | ||||||||||||
MODULE_VERSION(if_ovpn, 1); | ||||||||||||
Done Inline Actions
markj: | ||||||||||||
Done Inline ActionsIs it a bug that udp6_append() ignores the return value of the tunnel function? It looks like we'd leak an mbuf here. markj: Is it a bug that udp6_append() ignores the return value of the tunnel function? It looks like… | ||||||||||||
Done Inline ActionsAt this point we need to check the packet length again, since the previous check didn't include the tag bytes. markj: At this point we need to check the packet length again, since the previous check didn't include… | ||||||||||||
Done Inline ActionsThis is a pointer to a stack variable, it's not safe if hardware crypto offload is in use. I think you could perhaps use a pointer into the mbuf instead, if the portion of the mbuf containing the 8-byte header is known to be contiguous. markj: This is a pointer to a stack variable, it's not safe if hardware crypto offload is in use. I… | ||||||||||||
Done Inline ActionsSame comment as the tx path about liveness of the softc when asynchronous crypto offload is in use. markj: Same comment as the tx path about liveness of the softc when asynchronous crypto offload is in… | ||||||||||||
Done Inline ActionsIf counters were split out into a structure, could be initialized with one liner macros like ipstat, tcpstat. Exporting them to userland as a structure also seems easier. glebius: If counters were split out into a structure, could be initialized with one liner macros like… | ||||||||||||
Done Inline ActionsThis is a stack declaration. Please move it above M_ASSERTPKTHDR() which is a statement. glebius: This is a stack declaration. Please move it above M_ASSERTPKTHDR() which is a statement. | ||||||||||||
Done Inline ActionsCan we please count differently packets lost due to crypto failures and due to memory allocation failures? Just add one or two more counters. glebius: Can we please count differently packets lost due to crypto failures and due to memory… | ||||||||||||
Done Inline ActionsWhy not using refcount(9)? glebius: Why not using refcount(9)? | ||||||||||||
Done Inline ActionsWe adjust the refcount with only the read lock held, so multiple cores can be modifying it at the same time. refcount(9) requires additional protection we don't provide. kp: We adjust the refcount with only the read lock held, so multiple cores can be modifying it at… |
I think it would be a good idea initialize sin_len and sin6_len too. But probably this is not strictly required.