[PATCH V2 ocserv] Hash the peer's DTLS IP separately from its CSTP IP

Kevin Cernekee cernekee at gmail.com
Mon Feb 19 15:25:17 PST 2018


From: Nikos Mavrogiannopoulos <nmav at redhat.com>

This allows keeping track of clients which have their DTLS
stream come from a different IP location than their CSTP
stream.

Relates #61
---
 src/main.c        |  3 ++
 src/main.h        |  7 ++++-
 src/proc-search.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 src/proc-search.h |  1 +
 4 files changed, 90 insertions(+), 8 deletions(-)


V1->V2:

 - Use a dedicated hash table for the DTLS IPs.  Storing (dtls_hash, proc)
   pairs in the db_ip table stops working once the htable code has to
   call double_table() and then invoke the rehash() callback to recompute
   the keys.

 - Write a proc_table_update_dtls_ip() function that keeps the DTLS
   connection info up-to-date, in case the IP changes.

 - Clean up length comparisons in local_ip_cmp().

 - Removed "total++" (not sure where this is used?)

This IP -> proc_st mapping still relies heavily on guesswork, and it will
break if there are multiple clients behind the same IP.  Is there a
standard way to notify a DTLS client that it is out of sync and needs to
redo the handshake?  Merely dropping the unrecognized packets leads to
a poor user experience as it causes the VPN session to enter a "living
dead" state.


diff --git a/src/main.c b/src/main.c
index cbfdd52..6376af2 100644
--- a/src/main.c
+++ b/src/main.c
@@ -880,6 +880,9 @@ int sfd = -1;
 
 		if (match_ip_only != 0) {
 			msg.hello = 0; /* by default this is one */
+		} else {
+			/* a new DTLS session, store the DTLS IPs into proc and add it into hash table */
+			proc_table_update_dtls_ip(s, proc_to_send, &cli_addr, cli_addr_size);
 		}
 
 		msg.data.data = s->msg_buffer;
diff --git a/src/main.h b/src/main.h
index 1761e4f..97eb869 100644
--- a/src/main.h
+++ b/src/main.h
@@ -98,8 +98,12 @@ typedef struct proc_st {
 	struct ip_lease_st *ipv6;
 	unsigned leases_in_use; /* someone else got our IP leases */
 
-	struct sockaddr_storage remote_addr; /* peer address */
+	struct sockaddr_storage remote_addr; /* peer address (CSTP) */
 	socklen_t remote_addr_len;
+	/* It can happen that the peer's DTLS stream comes through a different
+	 * address. Most likely that's due to interception of the initial TLS/CSTP session */
+	struct sockaddr_storage dtls_remote_addr; /* peer address (DTLS) */
+	socklen_t dtls_remote_addr_len;
 	struct sockaddr_storage our_addr; /* our address */
 	socklen_t our_addr_len;
 
@@ -166,6 +170,7 @@ struct script_list_st {
 
 struct proc_hash_db_st {
 	struct htable *db_ip;
+	struct htable *db_dtls_ip;
 	struct htable *db_dtls_id;
 	struct htable *db_sid;
 	unsigned total;
diff --git a/src/proc-search.c b/src/proc-search.c
index 1678165..ec6e7d0 100644
--- a/src/proc-search.c
+++ b/src/proc-search.c
@@ -48,6 +48,15 @@ const struct proc_st * proc = _p;
 		SA_IN_SIZE(proc->remote_addr_len), 0);
 }
 
+static size_t rehash_dtls_ip(const void* _p, void* unused)
+{
+const struct proc_st * proc = _p;
+
+	return hash_any(
+		SA_IN_P_GENERIC(&proc->dtls_remote_addr, proc->dtls_remote_addr_len),
+		SA_IN_SIZE(proc->dtls_remote_addr_len), 0);
+}
+
 static size_t rehash_dtls_id(const void* _p, void* unused)
 {
 const struct proc_st * proc = _p;
@@ -65,9 +74,11 @@ const struct proc_st * proc = _p;
 void proc_table_init(main_server_st *s)
 {
 	s->proc_table.db_ip = talloc(s, struct htable);
+	s->proc_table.db_dtls_ip = talloc(s, struct htable);
 	s->proc_table.db_dtls_id = talloc(s, struct htable);
 	s->proc_table.db_sid = talloc(s, struct htable);
 	htable_init(s->proc_table.db_ip, rehash_ip, NULL);
+	htable_init(s->proc_table.db_dtls_ip, rehash_dtls_ip, NULL);
 	htable_init(s->proc_table.db_dtls_id, rehash_dtls_id, NULL);
 	htable_init(s->proc_table.db_sid, rehash_sid, NULL);
 	s->proc_table.total = 0;
@@ -76,13 +87,18 @@ void proc_table_init(main_server_st *s)
 void proc_table_deinit(main_server_st *s)
 {
 	htable_clear(s->proc_table.db_ip);
+	htable_clear(s->proc_table.db_dtls_ip);
 	htable_clear(s->proc_table.db_dtls_id);
 	htable_clear(s->proc_table.db_sid);
-	talloc_free(s->proc_table.db_dtls_id);
 	talloc_free(s->proc_table.db_ip);
+	talloc_free(s->proc_table.db_dtls_ip);
+	talloc_free(s->proc_table.db_dtls_id);
 	talloc_free(s->proc_table.db_sid);
 }
 
+/* Adds the IP of the CSTP channel into the IPs hash table and
+ * the session ID into the IDs hash table.
+ */
 int proc_table_add(main_server_st *s, struct proc_st *proc)
 {
 	size_t ip_hash = rehash_ip(proc, NULL);
@@ -125,8 +141,46 @@ int proc_table_update_ip(main_server_st *s, struct proc_st *proc, struct sockadd
 	return 0;
 }
 
+/* Adds the IP of the DTLS channel into the DTLS IP hash table. It
+ * only adds the IP if it is different than the CSTP channel IP.
+ */
+int proc_table_update_dtls_ip(main_server_st *s, struct proc_st *proc, struct sockaddr_storage *addr, unsigned addr_size)
+{
+	if (proc->dtls_remote_addr_len) {
+		if (proc->dtls_remote_addr_len == addr_size &&
+		    memcmp(SA_IN_P_GENERIC(&proc->dtls_remote_addr, addr_size),
+			   SA_IN_P_GENERIC(addr, addr_size),
+			   SA_IN_SIZE(addr_size)) == 0) {
+			return -1; /* DTLS address is already up to date */
+		}
+		htable_del(s->proc_table.db_dtls_ip, rehash_dtls_ip(proc, NULL), proc);
+	}
+
+	proc->dtls_remote_addr_len = 0;
+	if (addr_size == 0)
+		return 0;
+
+	if (proc->remote_addr_len == addr_size &&
+	    memcmp(SA_IN_P_GENERIC(&proc->remote_addr, addr_size),
+		   SA_IN_P_GENERIC(addr, addr_size),
+		   SA_IN_SIZE(addr_size)) == 0) {
+		return -1; /* CSTP and DTLS peer addresses match; do nothing */
+	}
+
+	proc->dtls_remote_addr_len = addr_size;
+	memcpy(&proc->dtls_remote_addr, addr, addr_size);
+
+	if (htable_add(s->proc_table.db_dtls_ip, rehash_dtls_ip(proc, NULL), proc) == 0)
+		return -1;
+
+	return 0;
+}
+
 void proc_table_del(main_server_st *s, struct proc_st *proc)
 {
+	if (proc->dtls_remote_addr_len > 0)
+		htable_del(s->proc_table.db_dtls_ip, rehash_dtls_ip(proc, NULL), proc);
+
 	htable_del(s->proc_table.db_ip, rehash_ip(proc, NULL), proc);
 	htable_del(s->proc_table.db_dtls_id, rehash_dtls_id(proc, NULL), proc);
 	htable_del(s->proc_table.db_sid, rehash_sid(proc, NULL), proc);
@@ -137,10 +191,21 @@ static bool local_ip_cmp(const void* _c1, void* _c2)
 const struct proc_st* c1 = _c1;
 struct find_ip_st* c2 = _c2;
 
-	if (c1->remote_addr_len != c2->sockaddr_size)
+	if (c2->sockaddr_size == 0)
 		return 0;
 
-	if (memcmp(SA_IN_P_GENERIC(&c1->remote_addr, c1->remote_addr_len),
+	/* Test if peer IP matches DTLS IP */
+	if (c1->dtls_remote_addr_len == c2->sockaddr_size &&
+	    memcmp(SA_IN_P_GENERIC(&c1->dtls_remote_addr, c1->dtls_remote_addr_len),
+		   SA_IN_P_GENERIC(c2->sockaddr, c2->sockaddr_size),
+		   SA_IN_SIZE(c1->dtls_remote_addr_len)) == 0) {
+		c2->found_ips++;
+		return 1;
+	}
+
+	/* Test if peer IP matches CSTP IP */
+	if (c1->remote_addr_len == c2->sockaddr_size &&
+	    memcmp(SA_IN_P_GENERIC(&c1->remote_addr, c1->remote_addr_len),
 		   SA_IN_P_GENERIC(c2->sockaddr, c2->sockaddr_size),
 		   SA_IN_SIZE(c1->remote_addr_len)) == 0) {
 		c2->found_ips++;
@@ -162,15 +227,21 @@ struct proc_st *proc_search_single_ip(struct main_server_st *s,
 
 	fip.sockaddr = sockaddr;
 	fip.sockaddr_size = sockaddr_size;
-	fip.found_ips = 0;
 
 	h = hash_any(SA_IN_P_GENERIC(sockaddr, sockaddr_size),
 			SA_IN_SIZE(sockaddr_size), 0);
+
+	fip.found_ips = 0;
+	proc = htable_get(s->proc_table.db_dtls_ip, h, local_ip_cmp, &fip);
+	if (proc && fip.found_ips == 1)
+		return proc;
+
+	fip.found_ips = 0;
 	proc = htable_get(s->proc_table.db_ip, h, local_ip_cmp, &fip);
+	if (proc && fip.found_ips == 1)
+		return proc;
 
-	if (fip.found_ips > 1)
-		return NULL;
-	return proc;
+	return NULL;
 }
 
 static bool dtls_id_cmp(const void* _c1, void* _c2)
@@ -189,6 +260,7 @@ struct find_dtls_id_st* c2 = _c2;
 
 	return 0;
 }
+
 struct proc_st *proc_search_dtls_id(struct main_server_st *s,
 			        const uint8_t *id, unsigned id_size)
 {
@@ -213,6 +285,7 @@ struct find_sid_st* c2 = _c2;
 
 	return 0;
 }
+
 struct proc_st *proc_search_sid(struct main_server_st *s,
 			        const uint8_t sid[SID_SIZE])
 {
diff --git a/src/proc-search.h b/src/proc-search.h
index 9003698..e4fd2c3 100644
--- a/src/proc-search.h
+++ b/src/proc-search.h
@@ -39,5 +39,6 @@ void proc_table_deinit(main_server_st *s);
 int proc_table_add(main_server_st *s, struct proc_st *proc);
 void proc_table_del(main_server_st *s, struct proc_st *proc);
 int proc_table_update_ip(main_server_st *s, struct proc_st *proc, struct sockaddr_storage *addr, unsigned addr_size);
+int proc_table_update_dtls_ip(main_server_st *s, struct proc_st *proc, struct sockaddr_storage *addr, unsigned addr_size);
 
 #endif
-- 
2.7.4




More information about the openconnect-devel mailing list