aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatt Dunwoodie <ncon@mail.noconroy.net>2019-10-07 01:00:10 +0100
committerMatt Dunwoodie <ncon@mail.noconroy.net>2019-10-07 10:30:55 +0100
commitc31822c2fff78e6c9c84a018435ca4fa77383f2f (patch)
tree37cf79b7bcc1077ca2a4584ef6a2976902086fa8
parentAdd beginnings of mutex filled get* functions (diff)
downloadwireguard-openbsd-c31822c2fff78e6c9c84a018435ca4fa77383f2f.tar.xz
wireguard-openbsd-c31822c2fff78e6c9c84a018435ca4fa77383f2f.zip
Fix a number of bugs
-rw-r--r--src/Makefile2
-rw-r--r--src/if_wg.c2
-rw-r--r--src/kern_wg.c7
-rw-r--r--src/mpq.h2
-rw-r--r--src/wireguard.c86
5 files changed, 59 insertions, 40 deletions
diff --git a/src/Makefile b/src/Makefile
index 40db60f..0634a77 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -125,7 +125,7 @@ kernel: patch_kernel
make -C /usr/src/sys/arch/${ARCH}/compile/$(CONFIG)/
.PHONY:
installgdb:
- doas cp /usr/obj/sys/arch/${ARCH}/compile/$(CONFIG)/bsd.gdb /
+ doas cp /usr/obj/sys/arch/${ARCH}/compile/$(CONFIG)/bsd.gdb /bsd
.PHONY:
clean:
diff --git a/src/if_wg.c b/src/if_wg.c
index a98fa4e..8a978e6 100644
--- a/src/if_wg.c
+++ b/src/if_wg.c
@@ -78,7 +78,6 @@ struct wg_route_entry {
};
struct wg_softc {
- SLIST_ENTRY(wg_softc) sc_next;
struct ifnet sc_if;
in_port_t sc_port;
struct wg_device sc_dev;
@@ -127,7 +126,6 @@ struct wg_route *wg_softc_route_lookup(struct wg_softc *, struct mbuf *, bool);
int wg_mbuf_ratelimit(struct wg_softc *, struct mbuf *);
struct wg_tag *wg_mbuf_get_tag(struct mbuf *);
-void wg_route_init(struct wg_route *);
void wg_route_broken(struct wg_route *);
void wg_route_queue(struct wg_route *, enum wg_pkt_type, uint32_t);
void wg_peer_queue(struct wg_peer *, enum wg_pkt_type, uint32_t);
diff --git a/src/kern_wg.c b/src/kern_wg.c
index 67de06b..f6d8def 100644
--- a/src/kern_wg.c
+++ b/src/kern_wg.c
@@ -254,9 +254,11 @@ fm_lookup(struct fixed_map *fm, uint32_t k)
if (item->key == k) {
refcnt_take(&item->refcnt);
v = item->value;
+ FMPRINTF("lookup %x in %p - %d\n", k, fm, item->refcnt.refs);
+ } else {
+ FMPRINTF("lookup %x in %p - failed\n", k, fm);
}
mtx_leave(&fm->mtx);
- FMPRINTF("lookup %x in %p - %d\n", k, fm, item->refcnt.refs);
return v;
}
@@ -283,6 +285,7 @@ fm_drop(struct fixed_map *fm, uint32_t k)
item = fm->map + (k & (fm->size - 1));
if (item->key != k)
panic("element should be in map");
+ FMPRINTF("drop %x in %p - %d\n", k, fm, item->refcnt.refs);
mtx_leave(&fm->mtx);
refcnt_finalize(&item->refcnt, "fm_drop");
@@ -292,8 +295,6 @@ fm_drop(struct fixed_map *fm, uint32_t k)
item->state = FM_ITEM_EMPTY;
item->value = NULL;
mtx_leave(&fm->mtx);
-
- FMPRINTF("drop %x in %p - %d\n", k, fm, item->refcnt.refs);
}
/*
diff --git a/src/mpq.h b/src/mpq.h
index c5c27c4..70f0dba 100644
--- a/src/mpq.h
+++ b/src/mpq.h
@@ -43,7 +43,6 @@ void mpq_done(struct mbuf *m);
void fn_name(void *_mpq) { \
struct mbuf *m; \
struct mpq *mpq = _mpq; \
- int s = splraise(mpq->mpq_mtx.mtx_wantipl); \
while ((m = mpq_dethread(mpq)) != NULL) { \
parallel_fn(m); \
mpq_threaddone(mpq, m); \
@@ -54,7 +53,6 @@ void fn_name(void *_mpq) { \
} \
mpq_serialize_leave(mpq); \
} \
- splx(s); \
} \
#endif /* _MPQ_H_ */
diff --git a/src/wireguard.c b/src/wireguard.c
index 340a918..a6e94c5 100644
--- a/src/wireguard.c
+++ b/src/wireguard.c
@@ -56,7 +56,6 @@ CTASSERT(WG_KEY_SIZE == WG_HASH_SIZE);
CTASSERT(WG_MSG_PADDING_SIZE == CHACHA20POLY1305_AUTHTAG_SIZE);
CTASSERT(WG_XNONCE_SIZE == XCHACHA20POLY1305_NONCE_SIZE);
-#define ret_error_peer(err) do { ret = err; goto leave_peer; } while (0)
#define ret_error(err) do { ret = err; goto leave; } while (0)
#define offsetof(type, member) ((size_t)(&((type *)0)->member))
@@ -357,7 +356,7 @@ wg_peer_last_session(struct wg_peer *peer)
return NULL;
if (!wg_timespec_timedout(&session->s_created, WG_REJECT_AFTER_TIME) &&
- session->s_keyset.k_txcounter > WG_REJECT_AFTER_MESSAGES)
+ session->s_keyset.k_txcounter < WG_REJECT_AFTER_MESSAGES)
return session;
wg_session_put(session);
@@ -374,10 +373,12 @@ wg_device_rx_initiation(struct wg_device *dev, struct wg_msg_initiation *init,
struct wg_handshake hs;
struct wg_timestamp ts;
struct wg_pubkey remote;
- struct wg_session *session = NULL;
+ struct wg_session *session;
enum wg_error ret = WG_OK;
+ *s = NULL;
+
/* We want to ensure that the keypair is not modified during the
* handshake, so we take a local copy here and bzero it before
* returning */
@@ -443,6 +444,8 @@ wg_device_rx_initiation(struct wg_device *dev, struct wg_msg_initiation *init,
wg_peer_put(peer);
*s = session;
leave:
+ if (ret != WG_OK)
+ wg_session_put(session);
explicit_bzero(&kp, sizeof(kp));
explicit_bzero(&hs, sizeof(hs));
return ret;
@@ -460,7 +463,7 @@ wg_device_rx_response(struct wg_device *dev, struct wg_msg_response *resp,
enum wg_error ret = WG_OK;
if ((session = fm_lookup(&dev->d_sessions, resp->receiver)) == NULL)
- ret_error(WG_ID);
+ return WG_ID;
/* Load requried values */
wg_device_getkey(dev, &kp, 1);
@@ -513,6 +516,8 @@ wg_device_rx_response(struct wg_device *dev, struct wg_msg_response *resp,
*s = session;
leave:
+ if (ret != WG_OK)
+ wg_session_put(session);
explicit_bzero(&shared, sizeof(shared));
explicit_bzero(&kp, sizeof(kp));
explicit_bzero(&hs, sizeof(hs));
@@ -531,7 +536,7 @@ wg_device_rx_cookie(struct wg_device *dev, struct wg_msg_cookie *cookie,
enum wg_error ret = WG_OK;
if ((session = fm_lookup(&dev->d_sessions, cookie->receiver)) == NULL)
- ret_error(WG_ID);
+ return WG_ID;
wg_hash2(key, WG_COOKIE, strlen(WG_COOKIE),
session->s_peer->p_remote.k, WG_KEY_SIZE);
@@ -551,6 +556,8 @@ wg_device_rx_cookie(struct wg_device *dev, struct wg_msg_cookie *cookie,
*s = session;
leave:
+ if (ret != WG_OK)
+ wg_session_put(session);
return ret;
}
@@ -564,7 +571,7 @@ wg_device_rx_transport(struct wg_device *dev, struct wg_msg_transport *msg,
uint64_t counter = letoh64(msg->counter);
if ((session = fm_lookup(&dev->d_sessions, msg->receiver)) == NULL)
- ret_error(WG_ID);
+ return WG_ID;
wg_session_promote(session);
@@ -588,11 +595,16 @@ wg_device_rx_transport(struct wg_device *dev, struct wg_msg_transport *msg,
session->s_peer->p_rx_bytes += data_len - WG_MAC_SIZE;
if (session->s_state == WG_STATE_RESPONDER &&
- wg_timespec_timedout(&session->s_created, WG_REKEY_AFTER_TIME_RECV))
+ wg_timespec_timedout(&session->s_created,
+ WG_REKEY_AFTER_TIME_RECV) &&
+ wg_timespec_timedout(&session->s_peer->p_last_initiation,
+ WG_REKEY_TIMEOUT))
dev->d_outq(session->s_peer, WG_PKT_INITIATION, session->s_peer->p_id);
*s = session;
leave:
+ if (ret != WG_OK)
+ wg_session_put(session);
return ret;
}
@@ -609,14 +621,21 @@ wg_device_tx_initiation(struct wg_device *dev, struct wg_msg_initiation *init,
enum wg_error ret = WG_OK;
if ((peer = fm_lookup(&dev->d_peers, id)) == NULL)
- ret_error(WG_ID);
+ return WG_ID;
- /* TODO do we care about locking these? */
+ mtx_enter(&peer->p_mtx);
if (!wg_timespec_timedout(&peer->p_last_initiation, WG_REKEY_TIMEOUT))
- ret_error_peer(WG_HS_RATE);
-
+ ret = WG_HS_RATE;
if (peer->p_attempts >= WG_REKEY_ATTEMPT_COUNT)
- ret_error_peer(WG_HS_ATTEMPTS);
+ ret = WG_HS_ATTEMPTS;
+ if (ret == WG_OK) {
+ getnanotime(&peer->p_last_initiation);
+ peer->p_attempts++;
+ }
+ mtx_leave(&peer->p_mtx);
+
+ if (ret != WG_OK)
+ goto leave;
/* We need to generate the session here first, so we can use s_local_id
* below. We also want to operate on a local handshake, so we don't
@@ -664,19 +683,14 @@ wg_device_tx_initiation(struct wg_device *dev, struct wg_msg_initiation *init,
sizeof(init->mac2), offsetof(struct wg_msg_initiation, mac2),
sizeof(peer->p_cookie.cookie));
- /* Update peer */
- mtx_enter(&peer->p_mtx);
- getnanotime(&peer->p_last_initiation);
- peer->p_attempts++;
- mtx_leave(&peer->p_mtx);
-
- /* Attach session to peer */
- wg_peer_attach_session(peer, session, &hs, WG_STATE_MADE_INITIATION);
+ wg_peer_attach_session(peer, session, &hs,
+ WG_STATE_MADE_INITIATION);
*s = session;
-leave_peer:
- wg_peer_put(peer);
leave:
+ /* if (ret != WG_OK)
+ wg_session_put(session); */
+ wg_peer_put(peer);
explicit_bzero(&kp, sizeof(kp));
explicit_bzero(&hs, sizeof(hs));
return ret;
@@ -686,13 +700,13 @@ enum wg_error
wg_device_tx_response(struct wg_device *dev, struct wg_msg_response *resp,
uint32_t id, struct wg_session **s)
{
- enum wg_error ret = WG_OK;
-
struct wg_handshake hs;
struct wg_session *session;
+ enum wg_error ret = WG_OK;
+
if ((session = fm_lookup(&dev->d_sessions, id)) == NULL)
- ret_error(WG_ID);
+ return WG_ID;
resp->type = WG_MSG_RESPONSE;
resp->sender = session->s_local_id;
@@ -730,16 +744,19 @@ wg_device_tx_response(struct wg_device *dev, struct wg_msg_response *resp,
sizeof(resp->mac2), offsetof(struct wg_msg_response, mac2),
sizeof(session->s_peer->p_cookie.cookie));
- /* Update session */
mtx_enter(&session->s_mtx);
if (session->s_state == WG_STATE_RECV_INITIATION)
session->s_state = WG_STATE_MADE_RESPONSE;
- else
- ret = WG_STATE;
mtx_leave(&session->s_mtx);
+ if (session->s_state != WG_STATE_MADE_RESPONSE)
+ session->s_state = WG_STATE_MADE_RESPONSE;
+ ret_error(WG_STATE);
+
*s = session;
leave:
+ if (ret != WG_OK)
+ wg_session_put(session);
return ret;
}
@@ -754,11 +771,12 @@ enum wg_error
wg_device_tx_transport(struct wg_device *dev, struct wg_msg_transport *msg,
size_t len, uint32_t id, struct wg_session **s)
{
- enum wg_error ret = WG_OK;
struct wg_session *session;
+ enum wg_error ret = WG_OK;
+
if ((session = fm_lookup(&dev->d_sessions, id)) == NULL)
- ret_error(WG_ID);
+ return WG_ID;
/* TODO we should do some locking */
if (session->s_state != WG_STATE_INITIATOR &&
@@ -777,14 +795,18 @@ wg_device_tx_transport(struct wg_device *dev, struct wg_msg_transport *msg,
session->s_keyset.k_txkey.k);
/* Packet OK, but we do want a rekey */
- if ((session->s_state == WG_STATE_INITIATOR &&
+ if (((session->s_state == WG_STATE_INITIATOR &&
wg_timespec_timedout(&session->s_created, WG_REKEY_AFTER_TIME)) ||
- session->s_keyset.k_txcounter > WG_REKEY_AFTER_MESSAGES)
+ session->s_keyset.k_txcounter > WG_REKEY_AFTER_MESSAGES) &&
+ wg_timespec_timedout(&session->s_peer->p_last_initiation,
+ WG_REKEY_TIMEOUT))
dev->d_outq(session->s_peer, WG_PKT_INITIATION, session->s_peer->p_id);
session->s_peer->p_tx_bytes += len;
*s = session;
leave:
+ if (ret != WG_OK)
+ wg_session_put(session);
return ret;
}