Browse code

Disallow lameduck's float to an address taken by another client

Existing check didn't take into account the case when floated client is
lame duck (CN for lame duck is NULL), which allowed lame duck to float
to an address taken by another client.

As a fix we use cert_hash_compare function which, besides fixing
mentioned case, also allows lame duck to float to an address already
taken by the same client.
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1420658798-29943-1-git-send-email-lstipakov@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/9386

Signed-off-by: Gert Doering <gert@greenie.muc.de>

Lev Stipakov authored on 2015/01/08 04:26:38
Showing 3 changed files
... ...
@@ -2127,17 +2127,20 @@ void multi_process_float (struct multi_context* m, struct multi_instance* mi)
2127 2127
   const uint32_t hv = hash_value (hash, &real);
2128 2128
   struct hash_bucket *bucket = hash_bucket (hash, hv);
2129 2129
 
2130
+  /* make sure that we don't float to an address taken by another client */
2130 2131
   struct hash_element *he = hash_lookup_fast (hash, bucket, &real, hv);
2131 2132
   if (he)
2132 2133
     {
2133 2134
       struct multi_instance *ex_mi = (struct multi_instance *) he->value;
2134 2135
 
2135
-      const char *cn = tls_common_name (mi->context.c2.tls_multi, true);
2136
-      const char *ex_cn = tls_common_name (ex_mi->context.c2.tls_multi, true);
2137
-      if (cn && ex_cn && strcmp (cn, ex_cn))
2136
+      struct tls_multi *m1 = mi->context.c2.tls_multi;
2137
+      struct tls_multi *m2 = ex_mi->context.c2.tls_multi;
2138
+
2139
+      /* do not float if target address is taken by client with another cert */
2140
+      if (!cert_hash_compare(m1->locked_cert_hash_set, m2->locked_cert_hash_set))
2138 2141
 	{
2139
-	  msg (D_MULTI_MEDIUM, "prevent float to %s",
2140
-		multi_instance_string (ex_mi, false, &gc));
2142
+	  msg (D_MULTI_MEDIUM, "Disallow float to an address taken by another client %s",
2143
+	       multi_instance_string (ex_mi, false, &gc));
2141 2144
 
2142 2145
 	  mi->context.c2.buf.len = 0;
2143 2146
 
... ...
@@ -238,7 +238,7 @@ cert_hash_free (struct cert_hash_set *chs)
238 238
     }
239 239
 }
240 240
 
241
-static bool
241
+bool
242 242
 cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set *chs2)
243 243
 {
244 244
   if (chs1 && chs2)
... ...
@@ -137,6 +137,14 @@ const char *tls_common_name (const struct tls_multi* multi, const bool null);
137 137
  */
138 138
 const char *tls_username (const struct tls_multi *multi, const bool null);
139 139
 
140
+/**
141
+ * Compares certificates hashes, returns true if hashes are equal.
142
+ *
143
+ * @param chs1 cert 1 hash set
144
+ * @param chs2 cert 2 hash set
145
+ */
146
+bool cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set *chs2);
147
+
140 148
 #ifdef ENABLE_PF
141 149
 
142 150
 /**