Opened 10 years ago

Closed 3 years ago

#2297 closed defect (fixed)

fetching certs for legacy keys?

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Ian complaints at http://archives.seul.org/or/dev/Dec-2010/msg00001.html that Tor 0.2.3.x sees:

Dec 02 08:13:06.000 [notice] We're missing a certificate from authority with signing key F7C7B9191C74C0BA07363C84D37BBAD3A8A6C6D8: launching request.
Dec 02 08:13:06.000 [notice] We're missing a certificate from authority with signing key 604834622B54F2D9BA39B34AC53924546733AA60: launching request.

Our friend boboper posts a suggested fix at http://pastebin.com/raw.php?i=QFXB1Phb which I reproduce here:

--- dirserv.h.orig	Wed Dec 15 04:32:04 2010
+++ dirserv.h	Thu Dec 16 08:03:54 2010
@@ -64,6 +64,7 @@
 int directory_fetches_dir_info_later(or_options_t *options);
 int directory_caches_v2_dir_info(or_options_t *options);
 #define directory_caches_v1_dir_info(o) directory_caches_v2_dir_info(o)
+int directory_caches_distribute_dir_info(or_options_t *options);
 int directory_caches_dir_info(or_options_t *options);
 int directory_permits_begindir_requests(or_options_t *options);
 int directory_permits_controller_requests(or_options_t *options);

--- dirserv.c.orig	Wed Dec 15 04:32:04 2010
+++ dirserv.c	Thu Dec 16 08:06:28 2010
@@ -1237,9 +1237,18 @@
  * and we're willing to serve them to others. Else return 0.
  */
 int
+directory_caches_distribute_dir_info(or_options_t *options)
+{
+  return options->BridgeRelay != 0 || options->DirPort != 0;
+}
+
+/** Return 1 if we want to keep descriptors, networkstatuses, etc around
+ * for themself or we're willing to serve them to others. Else return 0.
+ */
+int
 directory_caches_dir_info(or_options_t *options)
 {
-  if (options->BridgeRelay || options->DirPort)
+  if (directory_caches_distribute_dir_info(options)) /* distribute stuff */
     return 1;
   if (!server_mode(options) || !advertised_server_mode())
     return 0;

--- routerlist.c.orig	Wed Dec 15 04:32:04 2010
+++ routerlist.c	Thu Dec 16 08:03:30 2010
@@ -227,7 +227,7 @@
                "signing key %s", from_store ? "cached" : "downloaded",
                ds->nickname, hex_str(cert->signing_key_digest,DIGEST_LEN));
     } else {
-      int adding = directory_caches_dir_info(get_options());
+      int adding = directory_caches_distribute_dir_info(get_options());
       log_info(LD_DIR, "%s %s certificate for unrecognized directory "
                "authority with signing key %s",
                adding ? "Adding" : "Not adding",
@@ -478,7 +478,7 @@
   smartlist_t *missing_digests;
   char *resource = NULL;
   cert_list_t *cl;
-  const int cache = directory_caches_dir_info(get_options());
+  const int cache = directory_caches_distribute_dir_info(get_options());
 
   if (should_delay_dir_fetches(get_options()))
     return;

Child Tickets

Change History (16)

comment:1 Changed 10 years ago by nickm

Status: newneeds_review

comment:2 Changed 10 years ago by nickm

Hm. So the behavior of the patch seems to be splitting directory_caches_dir_info() into two functions: one of which is just plain "Do you serve directory information?" and the other of which is "Do you serve directory information, or do you need to fetch directory information in a very up-to-date fashion because you want to block one-hop circuits?"

I think probably what we should do is kill "directory_caches_dir_info()" entirely and split into a "do you serve directory info" function and a "should you cache directory info aggressively" function, since none of the current function names is descriptive.

comment:3 in reply to:  2 Changed 10 years ago by cypherpunks

Replying to nickm:

I think probably what we should do is kill "directory_caches_dir_info()" entirely and split into a "do you serve directory info" function and a "should you cache directory info aggressively" function, since none of the current function names is descriptive.

The 0.2.2.x have the same problem with certs, need another way unless it's backportable.

comment:4 Changed 8 years ago by nickm

Status: needs_reviewneeds_revision

comment:5 Changed 8 years ago by nickm

Status: needs_revisionneeds_review

Examining again, this fix seems less than wholly related to the issue. The behavior of the fix is to *narrow* the circumstances under which we download and/or store certificates for authorities which we don't recognize. Currently, we do this if we serve directory information, or if we are an exit.

It's correct that if we're just an exit node, not a directory or a bridge, we don't need to fetch or store these certificates. So in that respect the patch is correct.

But it's not a necessarily patch for the original issue, I think. Ian's issue was that his server was fetching these certs on _every_ startup, and he wondered, "Why are we continually missing them?" And I don't see how this patch actually addresses that, unless there's some code someplace else that discards these certs as unwanted after getting them.

Nonetheless I've ported this patch to master, and written what I think is an accurate changes message; it makes stuff better, whether it solves the issue Ian was seeing or not. I'm not keen to merge it back to 0.2.2.x unless somebody sees some reason why the current behavior is actually causing trouble.

comment:6 Changed 8 years ago by nickm

Status: needs_reviewnew

Still looks good to me; merging that patch into 0.2.3.x and throwing the rest of this issue back into "new".

comment:7 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final

comment:8 Changed 8 years ago by nickm

Keywords: tor-client added

comment:9 Changed 8 years ago by nickm

Component: Tor ClientTor

comment:10 Changed 8 years ago by sysrqb

I'm hesitant to say this is the same bug as #2991 and #5595 and without more information about those signing keys and the info contained in their certs this will be very tricky to track down.

comment:11 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.5.x-final

Reproducing this is going to require fiddling about with authority certificates (maybe legacy certificates too?) on a private network.

comment:12 Changed 6 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.???

comment:13 Changed 4 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:14 Changed 4 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:15 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:16 Changed 3 years ago by nickm

Resolution: fixed
Severity: Normal
Status: newclosed
Note: See TracTickets for help on using tickets.