Browse Source

Address nickm's comments at https://trac.torproject.org/projects/tor/ticket/933#comment:8

    1. Only allow '*.' in MapAddress expressions. Ignore '*ample.com' and '.example.com'.
       This has resulted in a slight refactoring of config_register_addressmaps.
    2. Add some more detail to the man page entry for AddressMap.
    3. Fix initialization of a pointer to NULL rather than 0.
    4. Update the unit tests to cater for the changes in 1 and test more explicitly for
       recursive mapping.
Robert Hogan 15 years ago
parent
commit
53ce6bb52d
4 changed files with 96 additions and 40 deletions
  1. 34 7
      doc/tor.1.txt
  2. 38 24
      src/or/config.c
  3. 1 1
      src/or/connection_edge.c
  4. 23 8
      src/test/test_config.c

+ 34 - 7
doc/tor.1.txt

@@ -658,15 +658,42 @@ The following options are useful only for clients (that is, if
     When a request for address arrives to Tor, it will rewrite it to newaddress
     When a request for address arrives to Tor, it will rewrite it to newaddress
     before processing it. For example, if you always want connections to
     before processing it. For example, if you always want connections to
     www.indymedia.org to exit via __torserver__ (where __torserver__ is the
     www.indymedia.org to exit via __torserver__ (where __torserver__ is the
-    nickname of the server), use "MapAddress www.indymedia.org
-    www.indymedia.org.torserver.exit". If the value is prepended with a \'*.\',
-    it is treated as matching an entire domain. For example, if you always
-    want connections to  any sub-domain of indymedia.org to exit via
+    nickname of the server), use MapAddress www.indymedia.org
+    www.indymedia.org.torserver.exit. If the value is prepended with a
+    '*.', it is treated as matching an entire domain. For example, if you
+    always want connections to  any sub-domain of indymedia.org to exit via
     __torserver__ (where __torserver__ is the nickname of the server), use
     __torserver__ (where __torserver__ is the nickname of the server), use
-    "MapAddress *.indymedia.org *.indymedia.org.torserver.exit". (Note the
+    MapAddress *.indymedia.org *.indymedia.org.torserver.exit. (Note the
     leading '*.' in each part of the directive.) You can also redirect all
     leading '*.' in each part of the directive.) You can also redirect all
-    subdomains of a domain to a single address. For example, "MapAddress
-    *.indymedia.org www.indymedia.org".
+    subdomains of a domain to a single address. For example, MapAddress
+    *.indymedia.org www.indymedia.org. +
+ +
+    NOTES:
+
+    1. When evaluating MapAddress expressions Tor stops when it hits the most
+    recently added expression that matches the requested address. So if you
+    have the following in your torrc, www.torproject.org will map to 1.1.1.1:
+
+     MapAddress www.torproject.org 2.2.2.2
+     MapAddress www.torproject.org 1.1.1.1
+
+    2. Tor evaluates the MapAddress configuration until it finds no matches. So
+    if you have the following in your torrc, www.torproject.org will map to
+    2.2.2.2:
+
+      MapAddress www.torproject.org 3.3.3.3
+      MapAddress 1.1.1.1 4.4.4.4
+      MapAddress 1.1.1.1 2.2.2.2
+      MapAddress www.torproject.org 1.1.1.1
+
+    3. The following MapAddress expression is invalid (and will be
+    ignored) because you cannot map from a specific address to a wildcarded
+    address:
+
+      MapAddress www.torproject.org *.torproject.org.torserver.exit
+
+    4. Using a wildcard as a regular expression (e.g. *ample.com) is
+    also invalid.
 
 
 **NewCircuitPeriod** __NUM__::
 **NewCircuitPeriod** __NUM__::
     Every NUM seconds consider whether to build a new circuit. (Default: 30
     Every NUM seconds consider whether to build a new circuit. (Default: 30

+ 38 - 24
src/or/config.c

@@ -4461,33 +4461,47 @@ config_register_addressmaps(const or_options_t *options)
   for (opt = options->AddressMap; opt; opt = opt->next) {
   for (opt = options->AddressMap; opt; opt = opt->next) {
     smartlist_split_string(elts, opt->value, NULL,
     smartlist_split_string(elts, opt->value, NULL,
                            SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 2);
                            SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 2);
-    if (smartlist_len(elts) >= 2) {
-      from = smartlist_get(elts,0);
-      to = smartlist_get(elts,1);
-
-      /* Remove leading asterisk in expressions of type: '*.example.com' */
-      if (from[0] == '*' && strlen(from) > 1)
-        from++;
-      if (to[0] == '*' && strlen(to) > 1)
-        to++;
-      if (to[0] == '.' && from[0] != '.') {
-        log_warn(LD_CONFIG,
-                 "Skipping invalid argument '%s' to MapAddress: "
-                 "can only use wildcard (i.e. '.' or '*.') if 'from' address "
-                 "uses wildcard also", to);
-      } else if (address_is_invalid_destination(to, 1)) {
-        log_warn(LD_CONFIG,
-                 "Skipping invalid argument '%s' to MapAddress", to);
-      } else {
-        addressmap_register(from, tor_strdup(to), 0, ADDRMAPSRC_TORRC);
-        if (smartlist_len(elts)>2) {
-          log_warn(LD_CONFIG,"Ignoring extra arguments to MapAddress.");
-        }
-      }
-    } else {
+    if (smartlist_len(elts) < 2) {
       log_warn(LD_CONFIG,"MapAddress '%s' has too few arguments. Ignoring.",
       log_warn(LD_CONFIG,"MapAddress '%s' has too few arguments. Ignoring.",
                opt->value);
                opt->value);
+      goto cleanup;
+    }
+
+    from = smartlist_get(elts,0);
+    to = smartlist_get(elts,1);
+
+    if (to[0] == '.' || from[0] == '.') {
+      log_warn(LD_CONFIG,"MapAddress '%s' is ambiguous - address starts with a"
+              "'.'. Ignoring.",opt->value);
+      goto cleanup;
+    }
+
+    /* Remove leading asterisk in expressions of type: '*.example.com' */
+    if (!strncmp(from,"*.",2))
+      from++;
+    if (!strncmp(to,"*.",2))
+      to++;
+
+    if (to[0] == '.' && from[0] != '.') {
+      log_warn(LD_CONFIG,
+                "Skipping invalid argument '%s' to MapAddress: "
+                "can only use wildcard (i.e. '*.') if 'from' address "
+                "uses wildcard also", to);
+      goto cleanup;
+    }
+
+    if (address_is_invalid_destination(to, 1)) {
+      log_warn(LD_CONFIG,
+                "Skipping invalid argument '%s' to MapAddress", to);
+      goto cleanup;
     }
     }
+
+    addressmap_register(from, tor_strdup(to), 0, ADDRMAPSRC_TORRC);
+
+    if (smartlist_len(elts) > 2)
+      log_warn(LD_CONFIG,"Ignoring extra arguments to MapAddress.");
+
+  cleanup:
     SMARTLIST_FOREACH(elts, char*, cp, tor_free(cp));
     SMARTLIST_FOREACH(elts, char*, cp, tor_free(cp));
     smartlist_clear(elts);
     smartlist_clear(elts);
   }
   }

+ 1 - 1
src/or/connection_edge.c

@@ -1052,7 +1052,7 @@ addressmap_match_superdomains(char *address)
   const char *key;
   const char *key;
   void *_val;
   void *_val;
   addressmap_entry_t *val;
   addressmap_entry_t *val;
-  char *matched_domains = 0;
+  char *matched_domains = NULL;
 
 
   for (iter = strmap_iter_init(addressmap); !strmap_iter_done(iter); ) {
   for (iter = strmap_iter_init(addressmap); !strmap_iter_done(iter); ) {
     strmap_iter_get(iter, &key, &_val);
     strmap_iter_get(iter, &key, &_val);

+ 23 - 8
src/test/test_config.c

@@ -15,9 +15,11 @@ test_config_addressmap(void)
   char buf[1024];
   char buf[1024];
   char address[256];
   char address[256];
   time_t expires = TIME_MAX;
   time_t expires = TIME_MAX;
-  strlcpy(buf, "MapAddress .google.com .torserver.exit\n"
+  strlcpy(buf, "MapAddress .invalidwildcard.com *.torserver.exit\n" // invalid
+          "MapAddress *invalidasterisk.com *.torserver.exit\n" // invalid
+          "MapAddress *.google.com *.torserver.exit\n"
           "MapAddress *.yahoo.com *.google.com.torserver.exit\n"
           "MapAddress *.yahoo.com *.google.com.torserver.exit\n"
-          "MapAddress .cn.com www.cnn.com\n"
+          "MapAddress *.cn.com www.cnn.com\n"
           "MapAddress *.cnn.com www.cnn.com\n"
           "MapAddress *.cnn.com www.cnn.com\n"
           "MapAddress ex.com www.cnn.com\n"
           "MapAddress ex.com www.cnn.com\n"
           "MapAddress ey.com *.cnn.com\n"
           "MapAddress ey.com *.cnn.com\n"
@@ -27,6 +29,7 @@ test_config_addressmap(void)
           "MapAddress test.torproject.org 2.2.2.2\n"
           "MapAddress test.torproject.org 2.2.2.2\n"
           "MapAddress www.google.com 3.3.3.3\n"
           "MapAddress www.google.com 3.3.3.3\n"
           "MapAddress www.example.org 4.4.4.4\n"
           "MapAddress www.example.org 4.4.4.4\n"
+          "MapAddress 4.4.4.4 7.7.7.7\n"
           "MapAddress 4.4.4.4 5.5.5.5\n"
           "MapAddress 4.4.4.4 5.5.5.5\n"
           "MapAddress www.infiniteloop.org 6.6.6.6\n"
           "MapAddress www.infiniteloop.org 6.6.6.6\n"
           "MapAddress 6.6.6.6 www.infiniteloop.org\n"
           "MapAddress 6.6.6.6 www.infiniteloop.org\n"
@@ -35,6 +38,14 @@ test_config_addressmap(void)
   config_get_lines(buf, &(get_options()->AddressMap));
   config_get_lines(buf, &(get_options()->AddressMap));
   config_register_addressmaps(get_options());
   config_register_addressmaps(get_options());
 
 
+  /* MapAddress .invalidwildcard.com .torserver.exit  - no match */
+  strlcpy(address, "www.invalidwildcard.com", sizeof(address));
+  test_assert(!addressmap_rewrite(address, sizeof(address), &expires));
+
+  /* MapAddress *invalidasterisk.com .torserver.exit  - no match */
+  strlcpy(address, "www.invalidasterisk.com", sizeof(address));
+  test_assert(!addressmap_rewrite(address, sizeof(address), &expires));
+
   /* Where no mapping for FQDN match on top-level domain */
   /* Where no mapping for FQDN match on top-level domain */
   /* MapAddress .google.com .torserver.exit */
   /* MapAddress .google.com .torserver.exit */
   strlcpy(address, "reader.google.com", sizeof(address));
   strlcpy(address, "reader.google.com", sizeof(address));
@@ -81,7 +92,11 @@ test_config_addressmap(void)
   test_assert(addressmap_rewrite(address, sizeof(address), &expires));
   test_assert(addressmap_rewrite(address, sizeof(address), &expires));
   test_streq(address, "2.2.2.2");
   test_streq(address, "2.2.2.2");
 
 
-  /* Test a chain of address mappings */
+  /* Test a chain of address mappings and the order in which they were added:
+          "MapAddress www.example.org 4.4.4.4"
+          "MapAddress 4.4.4.4 7.7.7.7"
+          "MapAddress 4.4.4.4 5.5.5.5"
+  */
   strlcpy(address, "www.example.org", sizeof(address));
   strlcpy(address, "www.example.org", sizeof(address));
   test_assert(addressmap_rewrite(address, sizeof(address), &expires));
   test_assert(addressmap_rewrite(address, sizeof(address), &expires));
   test_streq(address, "5.5.5.5");
   test_streq(address, "5.5.5.5");
@@ -97,9 +112,9 @@ test_config_addressmap(void)
 
 
   /* Test top-level-domain matching a bit harder */
   /* Test top-level-domain matching a bit harder */
   addressmap_clear_configured();
   addressmap_clear_configured();
-  strlcpy(buf, "MapAddress .com .torserver.exit\n"
-          "MapAddress .torproject.org 1.1.1.1\n"
-          "MapAddress .net 2.2.2.2\n"
+  strlcpy(buf, "MapAddress *.com *.torserver.exit\n"
+          "MapAddress *.torproject.org 1.1.1.1\n"
+          "MapAddress *.net 2.2.2.2\n"
           , sizeof(buf));
           , sizeof(buf));
   config_get_lines(buf, &(get_options()->AddressMap));
   config_get_lines(buf, &(get_options()->AddressMap));
   config_register_addressmaps(get_options());
   config_register_addressmaps(get_options());
@@ -124,9 +139,9 @@ test_config_addressmap(void)
   test_assert(addressmap_rewrite(address, sizeof(address), &expires));
   test_assert(addressmap_rewrite(address, sizeof(address), &expires));
   test_streq(address, "2.2.2.2");
   test_streq(address, "2.2.2.2");
 
 
-  /* We don't support '.' as a mapping directive */
+  /* We don't support '*' as a mapping directive */
   addressmap_clear_configured();
   addressmap_clear_configured();
-  strlcpy(buf, "MapAddress . .torserver.exit\n", sizeof(buf));
+  strlcpy(buf, "MapAddress * *.torserver.exit\n", sizeof(buf));
   config_get_lines(buf, &(get_options()->AddressMap));
   config_get_lines(buf, &(get_options()->AddressMap));
   config_register_addressmaps(get_options());
   config_register_addressmaps(get_options());