[tor-commits] [tor/master] Rewrite the logic for deciding when to drop old/superseded certificates
nickm at torproject.org
nickm at torproject.org
Fri Feb 20 06:11:01 UTC 2015
commit 69df16e3765383bd59fd2fbedbb5301f3b58ab14
Author: Nick Mathewson <nickm at torproject.org>
Date: Thu Jan 8 11:20:26 2015 -0500
Rewrite the logic for deciding when to drop old/superseded certificates
Fixes bug 11454, where we would keep around a superseded descriptor
if the descriptor replacing it wasn't at least a week later. Bugfix
on 0.2.1.8-alpha.
Fixes bug 11457, where a certificate with a publication time in the
future could make us discard existing (and subsequent!) certificates
with correct publication times. Bugfix on 0.2.0.3-alpha.
---
changes/bug11454 | 6 ++++
changes/bug11457 | 5 ++++
src/common/container.h | 11 ++++++++
src/or/routerlist.c | 73 ++++++++++++++++++++++++++++++------------------
4 files changed, 68 insertions(+), 27 deletions(-)
diff --git a/changes/bug11454 b/changes/bug11454
new file mode 100644
index 0000000..da793de
--- /dev/null
+++ b/changes/bug11454
@@ -0,0 +1,6 @@
+ o Minor bugfixes:
+ - Remove any old authority certificates that have been superseded
+ for at least two days. Previously, we would keep superseded
+ certificates until they expired, if they were published close
+ in time to the certificate that superseded them.
+ Fixes bug 11454; bugfix on 0.2.1.8-alpha.
diff --git a/changes/bug11457 b/changes/bug11457
new file mode 100644
index 0000000..a8b76f6
--- /dev/null
+++ b/changes/bug11457
@@ -0,0 +1,5 @@
+ o Minor bugfixes:
+ - If an authority operator accidentally makes a signing certificate with
+ a future publication time, do not discard its real signing
+ certificates. Fixes bug 11457; bugfix on 0.2.0.3-alpha.
+
diff --git a/src/common/container.h b/src/common/container.h
index 377cdf5..c704833 100644
--- a/src/common/container.h
+++ b/src/common/container.h
@@ -243,6 +243,17 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join,
STMT_END
/** Helper: While in a SMARTLIST_FOREACH loop over the list <b>sl</b> indexed
+ * with the variable <b>var</b>, remove the current element in a way that
+ * won't confuse the loop. */
+#define SMARTLIST_DEL_CURRENT_KEEPORDER(sl, var) \
+ STMT_BEGIN \
+ smartlist_del_keeporder(sl, var ## _sl_idx); \
+ --var ## _sl_idx; \
+ --var ## _sl_len; \
+ STMT_END
+
+
+/** Helper: While in a SMARTLIST_FOREACH loop over the list <b>sl</b> indexed
* with the variable <b>var</b>, replace the current element with <b>val</b>.
* Does not deallocate the current value of <b>var</b>.
*/
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 6cb052c..7112282 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -449,6 +449,19 @@ trusted_dirs_flush_certs_to_disk(void)
trusted_dir_servers_certs_changed = 0;
}
+static int
+compare_certs_by_pubdates(const void **_a, const void **_b)
+{
+ const authority_cert_t *cert1 = *_a, *cert2=*_b;
+
+ if (cert1->cache_info.published_on < cert2->cache_info.published_on)
+ return -1;
+ else if (cert1->cache_info.published_on > cert2->cache_info.published_on)
+ return 1;
+ else
+ return 0;
+}
+
/** Remove all expired v3 authority certificates that have been superseded for
* more than 48 hours or, if not expired, that were published more than 7 days
* before being superseded. (If the most recent cert was published more than 48
@@ -459,38 +472,44 @@ trusted_dirs_remove_old_certs(void)
{
time_t now = time(NULL);
#define DEAD_CERT_LIFETIME (2*24*60*60)
-#define OLD_CERT_LIFETIME (7*24*60*60)
+#define SUPERSEDED_CERT_LIFETIME (2*24*60*60)
if (!trusted_dir_certs)
return;
DIGESTMAP_FOREACH(trusted_dir_certs, key, cert_list_t *, cl) {
- authority_cert_t *newest = NULL;
- SMARTLIST_FOREACH(cl->certs, authority_cert_t *, cert,
- if (!newest || (cert->cache_info.published_on >
- newest->cache_info.published_on))
- newest = cert);
- if (newest) {
- const time_t newest_published = newest->cache_info.published_on;
- SMARTLIST_FOREACH_BEGIN(cl->certs, authority_cert_t *, cert) {
- int expired;
- time_t cert_published;
- if (newest == cert)
- continue;
- /* resolve spurious clang shallow analysis null pointer errors */
- tor_assert(cert);
- expired = now > cert->expires;
- cert_published = cert->cache_info.published_on;
- /* Store expired certs for 48 hours after a newer arrives;
+ /* Sort the list from first-published to last-published */
+ smartlist_sort(cl->certs, compare_certs_by_pubdates);
+
+ SMARTLIST_FOREACH_BEGIN(cl->certs, authority_cert_t *, cert) {
+ if (cert_sl_idx == smartlist_len(cl->certs) - 1) {
+ /* This is the most recently published cert. Keep it. */
+ continue;
+ }
+ authority_cert_t *next_cert = smartlist_get(cl->certs, cert_sl_idx+1);
+ const time_t next_cert_published = next_cert->cache_info.published_on;
+ if (next_cert_published > now) {
+ /* All later certs are published in the future. Keep everything
+ * we didn't discard. */
+ break;
+ }
+ int should_remove = 0;
+ if (cert->expires + DEAD_CERT_LIFETIME < now) {
+ /* Certificate has been expired for at least DEAD_CERT_LIFETIME.
+ * Remove it. */
+ should_remove = 1;
+ } else if (next_cert_published + SUPERSEDED_CERT_LIFETIME < now) {
+ /* Certificate has been superseded for OLD_CERT_LIFETIME.
+ * Remove it.
*/
- if (expired ?
- (newest_published + DEAD_CERT_LIFETIME < now) :
- (cert_published + OLD_CERT_LIFETIME < newest_published)) {
- SMARTLIST_DEL_CURRENT(cl->certs, cert);
- authority_cert_free(cert);
- trusted_dir_servers_certs_changed = 1;
- }
- } SMARTLIST_FOREACH_END(cert);
- }
+ should_remove = 1;
+ }
+ if (should_remove) {
+ SMARTLIST_DEL_CURRENT_KEEPORDER(cl->certs, cert);
+ authority_cert_free(cert);
+ trusted_dir_servers_certs_changed = 1;
+ }
+ } SMARTLIST_FOREACH_END(cert);
+
} DIGESTMAP_FOREACH_END;
#undef DEAD_CERT_LIFETIME
#undef OLD_CERT_LIFETIME
More information about the tor-commits
mailing list