Merge branch 'maint-0.2.2_secfix' into master_secfix

Conflicts:
	src/common/tortls.c
	src/or/connection_or.c
	src/or/dirserv.c
	src/or/or.h

changes/issue-2011-10-19L

@@ -0,0 +1,28 @@
+  o Security fixes:
+
+    - Don't send TLS certificate chains on outgoing OR connections
+      from clients and bridges.  Previously, each client or bridge
+      would use a single cert chain for all outgoing OR connections
+      for up to 24 hours, which allowed any relay connected to by a
+      client or bridge to determine which entry guards it is using.
+      This is a potential user-tracing bug for *all* users; everyone
+      who uses Tor's client or hidden service functionality should
+      upgrade.  Fixes CVE-2011-2768.  Bugfix on FIXME; found by
+      frosty_un.
+
+    - Don't use any OR connection on which we have received a
+      CREATE_FAST cell to satisfy an EXTEND request.  Previously, we
+      would not consider whether a connection appears to be from a
+      client or bridge when deciding whether to use that connection to
+      satisfy an EXTEND request.  Mitigates CVE-2011-2768, by
+      preventing an attacker from determining whether an unpatched
+      client is connected to a patched relay.  Bugfix on FIXME; found
+      by frosty_un.
+
+    - Don't assign the Guard flag to relays running a version of Tor
+      which would use an OR connection on which it has received a
+      CREATE_FAST cell to satisfy an EXTEND request.  Mitigates
+      CVE-2011-2768, by ensuring that clients will not connect
+      directly to any relay which an attacker could probe for an
+      unpatched client's connections.
+

changes/issue-2011-10-23G

@@ -0,0 +1,9 @@
+  o Security fixes:
+
+    - Reject CREATE and CREATE_FAST cells on outgoing OR connections
+      from a bridge to a relay.  Previously, we would accept them and
+      handle them normally, thereby allowing a malicious relay to
+      easily distinguish bridges which connect to it from clients.
+      Fixes CVE-2011-2769.  Bugfix on 0.2.0.3-alpha, when bridges were
+      implemented; found by frosty_un.
+

src/common/tortls.c

@@ -207,9 +207,11 @@ static X509* tor_tls_create_certificate(crypto_pk_env_t *rsa,
 
 static int tor_tls_context_init_one(tor_tls_context_t **ppcontext,
                                     crypto_pk_env_t *identity,
-                                    unsigned int key_lifetime);
+                                    unsigned int key_lifetime,
+                                    int is_client);
 static tor_tls_context_t *tor_tls_context_new(crypto_pk_env_t *identity,
-                                              unsigned int key_lifetime);
+                                              unsigned int key_lifetime,
+                                              int is_client);
 static int check_cert_lifetime_internal(const X509 *cert, int tolerance);
 
 /** Global TLS contexts. We keep them here because nobody else needs
@@ -1017,7 +1019,7 @@ tor_tls_context_init(int is_public_server,
 
     rv1 = tor_tls_context_init_one(&server_tls_context,
                                    server_identity,
-                                   key_lifetime);
+                                   key_lifetime, 0);
 
     if (rv1 >= 0) {
       new_ctx = server_tls_context;
@@ -1033,7 +1035,8 @@ tor_tls_context_init(int is_public_server,
     if (server_identity != NULL) {
       rv1 = tor_tls_context_init_one(&server_tls_context,
                                      server_identity,
-                                     key_lifetime);
+                                     key_lifetime,
+                                     0);
     } else {
       tor_tls_context_t *old_ctx = server_tls_context;
       server_tls_context = NULL;
@@ -1045,7 +1048,8 @@ tor_tls_context_init(int is_public_server,
 
     rv2 = tor_tls_context_init_one(&client_tls_context,
                                    client_identity,
-                                   key_lifetime);
+                                   key_lifetime,
+                                   1);
   }
 
   return MIN(rv1, rv2);
@@ -1060,10 +1064,12 @@ tor_tls_context_init(int is_public_server,
 static int
 tor_tls_context_init_one(tor_tls_context_t **ppcontext,
                          crypto_pk_env_t *identity,
-                         unsigned int key_lifetime)
+                         unsigned int key_lifetime,
+                         int is_client)
 {
   tor_tls_context_t *new_ctx = tor_tls_context_new(identity,
-                                                   key_lifetime);
+                                                   key_lifetime,
+                                                   is_client);
   tor_tls_context_t *old_ctx = *ppcontext;
 
   if (new_ctx != NULL) {
@@ -1085,7 +1091,8 @@ tor_tls_context_init_one(tor_tls_context_t **ppcontext,
  * certificate.
  */
 static tor_tls_context_t *
-tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime)
+tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime,
+                    int is_client)
 {
   crypto_pk_env_t *rsa = NULL, *rsa_auth = NULL;
   EVP_PKEY *pkey = NULL;
@@ -1106,35 +1113,39 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime)
     goto error;
   if (crypto_pk_generate_key(rsa)<0)
     goto error;
-  /* Generate short-term RSA key for use in the in-protocol ("v3")
-   * authentication handshake. */
-  if (!(rsa_auth = crypto_new_pk_env()))
-    goto error;
-  if (crypto_pk_generate_key(rsa_auth)<0)
-    goto error;
-  /* Create a link certificate signed by identity key. */
-  cert = tor_tls_create_certificate(rsa, identity, nickname, nn2,
-                                    key_lifetime);
-  /* Create self-signed certificate for identity key. */
-  idcert = tor_tls_create_certificate(identity, identity, nn2, nn2,
-                                      IDENTITY_CERT_LIFETIME);
-  /* Create an authentication certificate signed by identity key. */
-  authcert = tor_tls_create_certificate(rsa_auth, identity, nickname, nn2,
-                                        key_lifetime);
-  if (!cert || !idcert || !authcert) {
-    log(LOG_WARN, LD_CRYPTO, "Error creating certificate");
-    goto error;
+  if (!is_client) {
+    /* Generate short-term RSA key for use in the in-protocol ("v3")
+     * authentication handshake. */
+    if (!(rsa_auth = crypto_new_pk_env()))
+      goto error;
+    if (crypto_pk_generate_key(rsa_auth)<0)
+      goto error;
+    /* Create a link certificate signed by identity key. */
+    cert = tor_tls_create_certificate(rsa, identity, nickname, nn2,
+                                      key_lifetime);
+    /* Create self-signed certificate for identity key. */
+    idcert = tor_tls_create_certificate(identity, identity, nn2, nn2,
+                                        IDENTITY_CERT_LIFETIME);
+    /* Create an authentication certificate signed by identity key. */
+    authcert = tor_tls_create_certificate(rsa_auth, identity, nickname, nn2,
+                                          key_lifetime);
+    if (!cert || !idcert || !authcert) {
+      log(LOG_WARN, LD_CRYPTO, "Error creating certificate");
+      goto error;
+    }
   }
 
   result = tor_malloc_zero(sizeof(tor_tls_context_t));
   result->refcnt = 1;
-  result->my_link_cert = tor_cert_new(X509_dup(cert));
-  result->my_id_cert = tor_cert_new(X509_dup(idcert));
-  result->my_auth_cert = tor_cert_new(X509_dup(authcert));
-  if (!result->my_link_cert || !result->my_id_cert || !result->my_auth_cert)
-    goto error;
-  result->link_key = crypto_pk_dup_key(rsa);
-  result->auth_key = crypto_pk_dup_key(rsa_auth);
+  if (!is_client) {
+    result->my_link_cert = tor_cert_new(X509_dup(cert));
+    result->my_id_cert = tor_cert_new(X509_dup(idcert));
+    result->my_auth_cert = tor_cert_new(X509_dup(authcert));
+    if (!result->my_link_cert || !result->my_id_cert || !result->my_auth_cert)
+      goto error;
+    result->link_key = crypto_pk_dup_key(rsa);
+    result->auth_key = crypto_pk_dup_key(rsa_auth);
+  }
 
 #ifdef EVERYONE_HAS_AES
   /* Tell OpenSSL to only use TLS1 */
@@ -1166,27 +1177,31 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime)
 #ifdef SSL_MODE_RELEASE_BUFFERS
   SSL_CTX_set_mode(result->ctx, SSL_MODE_RELEASE_BUFFERS);
 #endif
-  if (cert && !SSL_CTX_use_certificate(result->ctx,cert))
-    goto error;
-  X509_free(cert); /* We just added a reference to cert. */
-  cert=NULL;
-  if (idcert) {
-    X509_STORE *s = SSL_CTX_get_cert_store(result->ctx);
-    tor_assert(s);
-    X509_STORE_add_cert(s, idcert);
-    X509_free(idcert); /* The context now owns the reference to idcert */
-    idcert = NULL;
+  if (! is_client) {
+    if (cert && !SSL_CTX_use_certificate(result->ctx,cert))
+      goto error;
+    X509_free(cert); /* We just added a reference to cert. */
+    cert=NULL;
+    if (idcert) {
+      X509_STORE *s = SSL_CTX_get_cert_store(result->ctx);
+      tor_assert(s);
+      X509_STORE_add_cert(s, idcert);
+      X509_free(idcert); /* The context now owns the reference to idcert */
+      idcert = NULL;
+    }
   }
   SSL_CTX_set_session_cache_mode(result->ctx, SSL_SESS_CACHE_OFF);
-  tor_assert(rsa);
-  if (!(pkey = _crypto_pk_env_get_evp_pkey(rsa,1)))
-    goto error;
-  if (!SSL_CTX_use_PrivateKey(result->ctx, pkey))
-    goto error;
-  EVP_PKEY_free(pkey);
-  pkey = NULL;
-  if (!SSL_CTX_check_private_key(result->ctx))
-    goto error;
+  if (!is_client) {
+    tor_assert(rsa);
+    if (!(pkey = _crypto_pk_env_get_evp_pkey(rsa,1)))
+      goto error;
+    if (!SSL_CTX_use_PrivateKey(result->ctx, pkey))
+      goto error;
+    EVP_PKEY_free(pkey);
+    pkey = NULL;
+    if (!SSL_CTX_check_private_key(result->ctx))
+      goto error;
+  }
   {
     crypto_dh_env_t *dh = crypto_dh_new(DH_TYPE_TLS);
     tor_assert(dh);

src/or/command.c

@@ -316,6 +316,7 @@ static void
 command_process_create_cell(cell_t *cell, or_connection_t *conn)
 {
   or_circuit_t *circ;
+  const or_options_t *options = get_options();
   int id_is_high;
 
   if (we_are_hibernating()) {
@@ -327,9 +328,11 @@ command_process_create_cell(cell_t *cell, or_connection_t *conn)
     return;
   }
 
-  if (!server_mode(get_options())) {
+  if (!server_mode(options) ||
+      (!public_server_mode(options) && conn->is_outgoing)) {
     log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
-           "Received create cell (type %d) from %s:%d, but we're a client. "
+           "Received create cell (type %d) from %s:%d, but we're connected "
+           "to it as a client. "
            "Sending back a destroy.",
            (int)cell->command, conn->_base.address, conn->_base.port);
     connection_or_send_destroy(cell->circ_id, conn,
@@ -392,7 +395,13 @@ command_process_create_cell(cell_t *cell, or_connection_t *conn)
      * a CPU worker. */
     char keys[CPATH_KEY_MATERIAL_LEN];
     char reply[DIGEST_LEN*2];
+
     tor_assert(cell->command == CELL_CREATE_FAST);
+
+    /* Make sure we never try to use the OR connection on which we
+     * received this cell to satisfy an EXTEND request,  */
+    conn->is_connection_with_client = 1;
+
     if (fast_server_handshake(cell->payload, (uint8_t*)reply,
                               (uint8_t*)keys, sizeof(keys))<0) {
       log_warn(LD_OR,"Failed to generate key material. Closing.");

src/or/config.c

@@ -282,6 +282,8 @@ static config_var_t _option_vars[] = {
   V(GeoIPFile,                   FILENAME,
     SHARE_DATADIR PATH_SEPARATOR "tor" PATH_SEPARATOR "geoip"),
 #endif
+  V(GiveGuardFlagTo_CVE_2011_2768_VulnerableRelays,
+                                 BOOL,     "0"),
   OBSOLETE("Group"),
   V(HardwareAccel,               BOOL,     "0"),
   V(HeartbeatPeriod,             INTERVAL, "6 hours"),

src/or/connection_or.c

@@ -767,6 +767,11 @@ connection_or_get_for_extend(const char *digest,
     tor_assert(tor_memeq(conn->identity_digest, digest, DIGEST_LEN));
     if (conn->_base.marked_for_close)
       continue;
+    /* Never return a connection on which the other end appears to be
+     * a client. */
+    if (conn->is_connection_with_client) {
+      continue;
+    }
     /* Never return a non-open connection. */
     if (conn->_base.state != OR_CONN_STATE_OPEN) {
       /* If the address matches, don't launch a new connection for this
@@ -1031,6 +1036,8 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
   conn->_base.state = OR_CONN_STATE_CONNECTING;
   control_event_or_conn_status(conn, OR_CONN_EVENT_LAUNCHED, 0);
 
+  conn->is_outgoing = 1;
+
   /* If we are using a proxy server, find it and use it. */
   r = get_proxy_addrport(&proxy_addr, &proxy_port, &proxy_type, TO_CONN(conn));
   if (r == 0) {

src/or/dirserv.c

@@ -2288,6 +2288,74 @@ get_possible_sybil_list(const smartlist_t *routers)
   return omit_as_sybil;
 }
 
+/** Return non-zero iff a relay running the Tor version specified in
+ * <b>platform</b> is suitable for use as a potential entry guard. */
+static int
+is_router_version_good_for_possible_guard(const char *platform)
+{
+  static int parsed_versions_initialized = 0;
+  static tor_version_t first_good_0_2_1_guard_version;
+  static tor_version_t first_good_0_2_2_guard_version;
+  static tor_version_t first_good_later_guard_version;
+
+  tor_version_t router_version;
+
+  /* XXX023 This block should be extracted into its own function. */
+  /* XXXX Begin code copied from tor_version_as_new_as (in routerparse.c) */
+  {
+    char *s, *s2, *start;
+    char tmp[128];
+
+    tor_assert(platform);
+
+    if (strcmpstart(platform,"Tor ")) /* nonstandard Tor; be safe and say yes */
+      return 1;
+
+    start = (char *)eat_whitespace(platform+3);
+    if (!*start) return 0;
+    s = (char *)find_whitespace(start); /* also finds '\0', which is fine */
+    s2 = (char*)eat_whitespace(s);
+    if (!strcmpstart(s2, "(r") || !strcmpstart(s2, "(git-"))
+      s = (char*)find_whitespace(s2);
+
+    if ((size_t)(s-start+1) >= sizeof(tmp)) /* too big, no */
+      return 0;
+    strlcpy(tmp, start, s-start+1);
+
+    if (tor_version_parse(tmp, &router_version)<0) {
+      log_info(LD_DIR,"Router version '%s' unparseable.",tmp);
+      return 1; /* be safe and say yes */
+    }
+  }
+  /* XXXX End code copied from tor_version_as_new_as (in routerparse.c) */
+
+  if (!parsed_versions_initialized) {
+    /* CVE-2011-2769 was fixed on the relay side in Tor versions
+     * 0.2.1.31, 0.2.2.34, and 0.2.3.6-alpha. */
+    tor_assert(tor_version_parse("0.2.1.31",
+                                 &first_good_0_2_1_guard_version)>=0);
+    tor_assert(tor_version_parse("0.2.2.34",
+                                 &first_good_0_2_2_guard_version)>=0);
+    tor_assert(tor_version_parse("0.2.3.6-alpha",
+                                 &first_good_later_guard_version)>=0);
+
+    /* Don't parse these constant version strings once for every relay
+     * for every vote. */
+    parsed_versions_initialized = 1;
+  }
+
+  return ((tor_version_same_series(&first_good_0_2_1_guard_version,
+                                   &router_version) &&
+           tor_version_compare(&first_good_0_2_1_guard_version,
+                               &router_version) <= 0) ||
+          (tor_version_same_series(&first_good_0_2_2_guard_version,
+                                   &router_version) &&
+           tor_version_compare(&first_good_0_2_2_guard_version,
+                               &router_version) <= 0) ||
+          (tor_version_compare(&first_good_later_guard_version,
+                               &router_version) <= 0));
+}
+
 /** Extract status information from <b>ri</b> and from other authority
  * functions and store it in <b>rs</b>>.  If <b>naming</b>, consider setting
  * the named flag in <b>rs</b>.
@@ -2303,6 +2371,7 @@ set_routerstatus_from_routerinfo(routerstatus_t *rs,
                                  int naming, int listbadexits,
                                  int listbaddirs, int vote_on_hsdirs)
 {
+  const or_options_t *options = get_options();
   int unstable_version =
     !tor_version_as_new_as(ri->platform,"0.1.1.16-rc-cvs");
   memset(rs, 0, sizeof(routerstatus_t));
@@ -2333,9 +2402,13 @@ set_routerstatus_from_routerinfo(routerstatus_t *rs,
       (router_get_advertised_bandwidth(ri) >= BANDWIDTH_TO_GUARANTEE_GUARD ||
        router_get_advertised_bandwidth(ri) >=
                               MIN(guard_bandwidth_including_exits,
-                                  guard_bandwidth_excluding_exits))) {
-    long tk = rep_hist_get_weighted_time_known(node->identity, now);
-    double wfu = rep_hist_get_weighted_fractional_uptime(node->identity, now);
+                                  guard_bandwidth_excluding_exits)) &&
+      (options->GiveGuardFlagTo_CVE_2011_2768_VulnerableRelays ||
+       is_router_version_good_for_possible_guard(ri->platform))) {
+    long tk = rep_hist_get_weighted_time_known(
+                                      node->identity, now);
+    double wfu = rep_hist_get_weighted_fractional_uptime(
+                                      node->identity, now);
     rs->is_possible_guard = (wfu >= guard_wfu && tk >= guard_tk) ? 1 : 0;
   } else {
     rs->is_possible_guard = 0;

src/or/or.h

@@ -1220,6 +1220,12 @@ typedef struct or_connection_t {
    * router itself has a problem.
    */
   unsigned int is_bad_for_new_circs:1;
+  /** True iff we have decided that the other end of this connection
+   * is a client.  Connections with this flag set should never be used
+   * to satisfy an EXTEND request.  */
+  unsigned int is_connection_with_client:1;
+  /** True iff this is an outgoing connection. */
+  unsigned int is_outgoing:1;
   unsigned int proxy_type:2; /**< One of PROXY_NONE...PROXY_SOCKS5 */
   uint8_t link_proto; /**< What protocol version are we using? 0 for
                        * "none negotiated yet." */
@@ -3152,6 +3158,10 @@ typedef struct {
                                      * number of servers per IP address shared
                                      * with an authority. */
 
+  /** Should we assign the Guard flag to relays which would allow
+   * exploitation of CVE-2011-2768 against their clients? */
+  int GiveGuardFlagTo_CVE_2011_2768_VulnerableRelays;
+
   char *AccountingStart; /**< How long is the accounting interval, and when
                           * does it start? */
   uint64_t AccountingMax; /**< How many bytes do we allow per accounting

src/or/routerparse.c

@@ -570,7 +570,6 @@ static int check_signature_token(const char *digest,
                                  int flags,
                                  const char *doctype);
 static crypto_pk_env_t *find_dir_signing_key(const char *str, const char *eos);
-static int tor_version_same_series(tor_version_t *a, tor_version_t *b);
 
 #undef DEBUG_AREA_ALLOC
 
@@ -4568,7 +4567,7 @@ tor_version_compare(tor_version_t *a, tor_version_t *b)
 
 /** Return true iff versions <b>a</b> and <b>b</b> belong to the same series.
  */
-static int
+int
 tor_version_same_series(tor_version_t *a, tor_version_t *b)
 {
   tor_assert(a);

src/or/routerparse.h

@@ -47,6 +47,7 @@ version_status_t tor_version_is_obsolete(const char *myversion,
 int tor_version_parse(const char *s, tor_version_t *out);
 int tor_version_as_new_as(const char *platform, const char *cutoff);
 int tor_version_compare(tor_version_t *a, tor_version_t *b);
+int tor_version_same_series(tor_version_t *a, tor_version_t *b);
 void sort_version_list(smartlist_t *lst, int remove_duplicates);
 void assert_addr_policy_ok(smartlist_t *t);
 void dump_distinct_digest_count(int severity);