Use fascist firewall and ClientUseIPv4 for bridge clients

Bridge clients ignore ClientUseIPv6, acting as if it is always 1.
This preserves existing behaviour.

Make ClientPreferIPv6OR/DirPort auto by default:
 * Bridge clients prefer IPv6 by default.
 * Other clients prefer IPv4 by default.
This preserves existing behaviour.

2 files changed, 161 insertions, 38 deletions

diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index e4947e0959..a0208b9cfc 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -107,6 +107,11 @@ test_choose_random_entry_no_guards(void *arg)
   chosen_entry = choose_random_entry(NULL);
   tt_assert(chosen_entry);
 
+  /* And with the preference on auto */
+  mocked_options.ClientPreferIPv6ORPort = -1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
   /* Check that we don't get a guard if it doesn't pass mandatory address
    * settings */
   memset(&mocked_options, 0, sizeof(mocked_options));
@@ -128,6 +133,21 @@ test_choose_random_entry_no_guards(void *arg)
   chosen_entry = choose_random_entry(NULL);
   tt_assert(chosen_entry);
 
+  /* Check that we get a guard if it passes preferred address settings when
+   * they're auto */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = -1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
+  /* And with IPv6 active */
+  mocked_options.ClientUseIPv6 = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_assert(chosen_entry);
+
  done:
   memset(&mocked_options, 0, sizeof(mocked_options));
   UNMOCK(get_options);
@@ -166,6 +186,11 @@ test_choose_random_entry_one_possible_guard(void *arg)
   chosen_entry = choose_random_entry(NULL);
   tt_ptr_op(chosen_entry, OP_EQ, the_guard);
 
+  /* And with the preference on auto */
+  mocked_options.ClientPreferIPv6ORPort = -1;
+  chosen_entry = choose_random_entry(NULL);
+  tt_ptr_op(chosen_entry, OP_EQ, the_guard);
+
   /* Check that we don't get a guard if it doesn't pass mandatory address
    * settings */
   memset(&mocked_options, 0, sizeof(mocked_options));
@@ -190,6 +215,21 @@ test_choose_random_entry_one_possible_guard(void *arg)
    * time, so we can't be sure we get the guard */
   tt_assert(chosen_entry);
 
+  /* Check that we get the guard if it passes preferred address settings when
+   * they're auto */
+  memset(&mocked_options, 0, sizeof(mocked_options));
+  mocked_options.ClientUseIPv4 = 1;
+  mocked_options.ClientPreferIPv6ORPort = -1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_ptr_op(chosen_entry, OP_EQ, the_guard);
+
+  /* and with IPv6 active */
+  mocked_options.ClientUseIPv6 = 1;
+
+  chosen_entry = choose_random_entry(NULL);
+  tt_ptr_op(chosen_entry, OP_EQ, the_guard);
+
  done:
   memset(&mocked_options, 0, sizeof(mocked_options));
   UNMOCK(get_options);
@@ -722,8 +762,9 @@ test_node_preferred_orport(void *arg)
 
   /* Setup options */
   memset(&mocked_options, 0, sizeof(mocked_options));
-  /* We don't test ClientPreferIPv6ORPort here, because it's only used in
-   * nodelist_set_consensus to setup each node_t. */
+  /* We don't test ClientPreferIPv6ORPort here, because it's used in
+   * nodelist_set_consensus to setup node.ipv6_preferred, which we set
+   * directly. */
   MOCK(get_options, mock_get_options);
 
   /* Setup IP addresses */
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index 077d1b2af5..1daa38ecf2 100644
--- a/src/test/test_policy.c
+++ b/src/test/test_policy.c
@@ -1235,8 +1235,8 @@ test_policies_fascist_firewall_allows_address(void *arg)
 
   /* Test the function's address matching with UseBridges on */
   memset(&mock_options, 0, sizeof(or_options_t));
-  mock_options.ClientUseIPv4 = 0;
-  mock_options.ClientUseIPv6 = 0;
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
   mock_options.UseBridges = 1;
 
   tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
@@ -1248,6 +1248,38 @@ test_policies_fascist_firewall_allows_address(void *arg)
   tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
             == 0);
 
+  /* Preferring IPv4 */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 1, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 1, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 1, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 1, 0)
+            == 0);
+
+  /* Preferring IPv6 */
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 1, 1)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 1, 1)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 1, 1)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 1, 1)
+            == 0);
+
+  /* bridge clients always use IPv6, regardless of ClientUseIPv6 */
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 0;
+  tt_assert(fascist_firewall_allows_address(&ipv4_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&ipv6_addr, port, policy, 0, 0)
+            == 1);
+  tt_assert(fascist_firewall_allows_address(&r_ipv4_addr, port, policy, 0, 0)
+            == 0);
+  tt_assert(fascist_firewall_allows_address(&r_ipv6_addr, port, policy, 0, 0)
+            == 0);
+
   /* Test the function's address matching with IPv4 on */
   memset(&mock_options, 0, sizeof(or_options_t));
   mock_options.ClientUseIPv4 = 1;
@@ -1389,6 +1421,22 @@ test_policies_fascist_firewall_choose_address(void *arg)
                                                  FIREWALL_DIR_CONNECTION, 1)
             == &ipv4_dir_ap);
 
+  /* Auto (Preferring IPv4) */
+  mock_options.ClientPreferIPv6ORPort = -1;
+  mock_options.ClientPreferIPv6DirPort = -1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
   /* Preferring IPv6 */
   mock_options.ClientPreferIPv6ORPort = 1;
   mock_options.ClientPreferIPv6DirPort = 1;
@@ -1440,41 +1488,75 @@ test_policies_fascist_firewall_choose_address(void *arg)
   /* Choose an address with UseBridges on */
   memset(&mock_options, 0, sizeof(or_options_t));
   mock_options.UseBridges = 1;
+  mock_options.ClientUseIPv4 = 1;
+  mock_options.ClientUseIPv6 = 1;
 
-  for (mock_options.ClientUseIPv4 = 0; mock_options.ClientUseIPv4 <= 1;
-       mock_options.ClientUseIPv4++) {
-    for (mock_options.ClientUseIPv6 = 0; mock_options.ClientUseIPv6 <= 1;
-         mock_options.ClientUseIPv6++) {
-      for (mock_options.ClientPreferIPv6ORPort = 0;
-           mock_options.ClientPreferIPv6ORPort <= 1;
-           mock_options.ClientPreferIPv6ORPort++) {
-        for (mock_options.ClientPreferIPv6DirPort = 0;
-             mock_options.ClientPreferIPv6DirPort <= 1;
-             mock_options.ClientPreferIPv6DirPort++) {
-          /* This (ab)uses the actual enum values */
-          tt_assert(FIREWALL_OR_CONNECTION < FIREWALL_DIR_CONNECTION);
-          for (firewall_connection_t fw_connection = FIREWALL_OR_CONNECTION;
-               fw_connection <= FIREWALL_DIR_CONNECTION; fw_connection++) {
-            for (int pref_only = 0; pref_only <= 1; pref_only++) {
-
-              /* Ignoring all other settings, want_a should choose the address
-               * for bridge clients */
-              tt_assert(fascist_firewall_choose_address(&ipv4_or_ap,
-                                                             &ipv6_or_ap, 1,
-                                                             fw_connection,
-                                                             pref_only)
-                        == &ipv4_or_ap);
-              tt_assert(fascist_firewall_choose_address(&ipv4_or_ap,
-                                                             &ipv6_or_ap, 0,
-                                                             fw_connection,
-                                                             pref_only)
-                        == &ipv6_or_ap);
-            }
-          }
-        }
-      }
-    }
-  }
+  /* Preferring IPv4 */
+  mock_options.ClientPreferIPv6ORPort = 0;
+  mock_options.ClientPreferIPv6DirPort = 0;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv4_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv4_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv4_dir_ap);
+
+  /* Auto (Preferring IPv6 for bridge clients) */
+  mock_options.ClientPreferIPv6ORPort = -1;
+  mock_options.ClientPreferIPv6DirPort = -1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+  /* Preferring IPv6 */
+  mock_options.ClientPreferIPv6ORPort = 1;
+  mock_options.ClientPreferIPv6DirPort = 1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
+
+
+  /* In the default configuration (Auto / IPv6 off), bridge clients should
+   * still use and prefer IPv6 regardless of ClientUseIPv6. */
+  mock_options.ClientUseIPv6 = 0;
+  mock_options.ClientPreferIPv6ORPort = -1;
+  mock_options.ClientPreferIPv6DirPort = -1;
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 0)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0,
+                                            FIREWALL_OR_CONNECTION, 1)
+            == &ipv6_or_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 0)
+            == &ipv6_dir_ap);
+  tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0,
+                                            FIREWALL_DIR_CONNECTION, 1)
+            == &ipv6_dir_ap);
 
   /* Choose an address with IPv4 on */
   memset(&mock_options, 0, sizeof(or_options_t));