[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#694349: marked as done (unblock: tor/0.2.3.25-1)



Your message dated Sun, 25 Nov 2012 19:29:32 +0000
with message-id <1353871772.18815.79.camel@jacala.jungle.funky-badger.org>
and subject line Re: Bug#694349: unblock: tor/0.2.3.25-1
has caused the Debian Bug report #694349,
regarding unblock: tor/0.2.3.25-1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
694349: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=694349
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: freeze-exception

Please unblock package tor:

unblock tor/0.2.3.25-1

Tor does a lot of crypto operations, and these, as you very well know,
involve using secrets.  Tor tries to wipe these secrets and other
sensitive data when they are no longer needed.

It turns out that gcc sometimes is too smart for our good: when it
realizes that we write pretty zeroes over a buffer but then no longer
use that buffer, it will optimise away that memset().  And for a
correctly functioning program that's fine, but our use of memset() is
for defense in depth - to limit damage when we are not working
correctly.

In version 0.2.3.25-1, Tor tries harder to wipe memory, replacing
several calls to memset() with calls to a tor memwipe().  This memwipe()
implementation calls openssl's OPENSSL_cleanse() which does some pretty
weird stuff to ensure gcc doesn't catch on.

As a safeguard for cases where we now call memwipe() when we really
wanted memset() after all, but didn't realize it, the memwipe() then
goes on and writes the bytes that we wanted over the buffer - gcc might
optimize that away, but only in cases where it's convinced we don't need
it.  That should be fine.


This isn't a super criticial fix, but if possible it would still be
prudent to include it in Debian.


Tor 0.2.3.25 also fixes another harmless bug (this is line 587 in the
attached diff):
| -    if (advertised_server_mode() & !options->DisableNetwork)
| +    if (advertised_server_mode() && !options->DisableNetwork)


Attached is the git diff between the version in testing and the version
in unstable, debian-tor-0.2.3.24-rc-1 and debian-tor-0.2.3.25-1
respectively.

The uploaded source also contains differences in auto* generated files.

Please unblock tor.
unblock tor/0.2.3.25-1

Cheers,
weasel
diff --git a/ChangeLog b/ChangeLog
index 8f3689b..62099c5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,42 @@
+Changes in version 0.2.3.25 - 2012-11-19
+  The Tor 0.2.3 release series is dedicated to the memory of Len "rabbi"
+  Sassaman (1980-2011), a long-time cypherpunk, anonymity researcher,
+  Mixmaster maintainer, Pynchon Gate co-designer, CodeCon organizer,
+  programmer, and friend. Unstinting in his dedication to the cause of
+  freedom, he inspired and helped many of us as we began our work on
+  anonymity, and inspires us still. Please honor his memory by writing
+  software to protect people's freedoms, and by helping others to do so.
+
+  Tor 0.2.3.25, the first stable release in the 0.2.3 branch, features
+  significantly reduced directory overhead (via microdescriptors),
+  enormous crypto performance improvements for fast relays on new
+  enough hardware, a new v3 TLS handshake protocol that can better
+  resist fingerprinting, support for protocol obfuscation plugins (aka
+  pluggable transports), better scalability for hidden services, IPv6
+  support for bridges, performance improvements like allowing clients
+  to skip the first round-trip on the circuit ("optimistic data") and
+  refilling token buckets more often, a new "stream isolation" design
+  to isolate different applications on different circuits, and many
+  stability, security, and privacy fixes.
+
+  o Major bugfixes:
+    - Tor tries to wipe potentially sensitive data after using it, so
+      that if some subsequent security failure exposes Tor's memory,
+      the damage will be limited. But we had a bug where the compiler
+      was eliminating these wipe operations when it decided that the
+      memory was no longer visible to a (correctly running) program,
+      hence defeating our attempt at defense in depth. We fix that
+      by using OpenSSL's OPENSSL_cleanse() operation, which a compiler
+      is unlikely to optimize away. Future versions of Tor may use
+      a less ridiculously heavy approach for this. Fixes bug 7352.
+      Reported in an article by Andrey Karpov.
+
+  o Minor bugfixes:
+    - Fix a harmless bug when opting against publishing a relay descriptor
+      because DisableNetwork is set. Fixes bug 7464; bugfix on
+      0.2.3.9-alpha.
+
+
 Changes in version 0.2.3.24-rc - 2012-10-25
   Tor 0.2.3.24-rc fixes two important security vulnerabilities that
   could lead to remotely triggerable relay crashes, and fixes
diff --git a/ReleaseNotes b/ReleaseNotes
index 5717330..db23c52 100644
--- a/ReleaseNotes
+++ b/ReleaseNotes
@@ -3,8 +3,7 @@ This document summarizes new features and bugfixes in each stable release
 of Tor. If you want to see more detailed descriptions of the changes in
 each development snapshot, see the ChangeLog file.
 
-Changes in version 0.2.3.x - 2012-10-??
-
+Changes in version 0.2.3.25 - 2012-11-19
   The Tor 0.2.3 release series is dedicated to the memory of Len "rabbi"
   Sassaman (1980-2011), a long-time cypherpunk, anonymity researcher,
   Mixmaster maintainer, Pynchon Gate co-designer, CodeCon organizer,
@@ -13,7 +12,17 @@ Changes in version 0.2.3.x - 2012-10-??
   anonymity, and inspires us still. Please honor his memory by writing
   software to protect people's freedoms, and by helping others to do so.
 
-  Tor 0.2.3.x, the first stable release in the 0.2.3 branch, features ...
+  Tor 0.2.3.25, the first stable release in the 0.2.3 branch, features
+  significantly reduced directory overhead (via microdescriptors),
+  enormous crypto performance improvements for fast relays on new
+  enough hardware, a new v3 TLS handshake protocol that can better
+  resist fingerprinting, support for protocol obfuscation plugins (aka
+  pluggable transports), better scalability for hidden services, IPv6
+  support for bridges, performance improvements like allowing clients
+  to skip the first round-trip on the circuit ("optimistic data") and
+  refilling token buckets more often, a new "stream isolation" design
+  to isolate different applications on different circuits, and many
+  stability, security, and privacy fixes.
 
   Major features (v3 directory protocol):
     - Clients now use microdescriptors instead of regular descriptors
@@ -259,6 +268,16 @@ Changes in version 0.2.3.x - 2012-10-??
       could decrypt a link connection as soon as the link connection
       was closed. Fixes bug 7139; bugfix on all versions of Tor linked
       against OpenSSL 1.0.0 or later. Found by Florent DaigniÚre.
+    - Tor tries to wipe potentially sensitive data after using it, so
+      that if some subsequent security failure exposes Tor's memory,
+      the damage will be limited. But we had a bug where the compiler
+      was eliminating these wipe operations when it decided that the
+      memory was no longer visible to a (correctly running) program,
+      hence defeating our attempt at defense in depth. We fix that
+      by using OpenSSL's OPENSSL_cleanse() operation, which a compiler
+      is unlikely to optimize away. Future versions of Tor may use
+      a less ridiculously heavy approach for this. Fixes bug 7352.
+      Reported in an article by Andrey Karpov.
 
   o Major bugfixes (crashes and asserts):
     - Avoid a pair of double-free and use-after-mark bugs that can
diff --git a/configure.in b/configure.in
index 124bf9f..b29f318 100644
--- a/configure.in
+++ b/configure.in
@@ -4,7 +4,7 @@ dnl Copyright (c) 2007-2012, The Tor Project, Inc.
 dnl See LICENSE for licensing information
 
 AC_INIT
-AM_INIT_AUTOMAKE(tor, 0.2.3.24-rc)
+AM_INIT_AUTOMAKE(tor, 0.2.3.25)
 AM_CONFIG_HEADER(orconfig.h)
 
 AC_CANONICAL_HOST
diff --git a/contrib/tor-mingw.nsi.in b/contrib/tor-mingw.nsi.in
index 2f02a3f..69c4c2e 100644
--- a/contrib/tor-mingw.nsi.in
+++ b/contrib/tor-mingw.nsi.in
@@ -8,7 +8,7 @@
 !include "LogicLib.nsh"
 !include "FileFunc.nsh"
 !insertmacro GetParameters
-!define VERSION "0.2.3.24-rc"
+!define VERSION "0.2.3.25"
 !define INSTALLER "tor-${VERSION}-win32.exe"
 !define WEBSITE "https://www.torproject.org/";
 !define LICENSE "LICENSE"
diff --git a/debian/changelog b/debian/changelog
index d8f0a49..f2fd0e6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+tor (0.2.3.25-1) unstable; urgency=low
+
+  * New upstream release.  The 0.2.3.x tree goes stable.
+  * Dedicated to the memory of Len "rabbi" Sassaman (1980-2011).  We miss
+    you, Len.
+
+ -- Peter Palfrader <weasel@debian.org>  Tue, 20 Nov 2012 22:00:34 +0100
+
 tor (0.2.3.24-rc-1) unstable; urgency=high
 
   * New upstream version:
diff --git a/src/common/aes.c b/src/common/aes.c
index 59d864a..295a907 100644
--- a/src/common/aes.c
+++ b/src/common/aes.c
@@ -106,7 +106,7 @@ aes_cipher_free(aes_cnt_cipher_t *cipher)
   if (!cipher)
     return;
   EVP_CIPHER_CTX_cleanup(&cipher->evp);
-  memset(cipher, 0, sizeof(aes_cnt_cipher_t));
+  memwipe(cipher, 0, sizeof(aes_cnt_cipher_t));
   tor_free(cipher);
 }
 void
@@ -373,7 +373,7 @@ aes_cipher_free(aes_cnt_cipher_t *cipher)
   if (cipher->using_evp) {
     EVP_CIPHER_CTX_cleanup(&cipher->key.evp);
   }
-  memset(cipher, 0, sizeof(aes_cnt_cipher_t));
+  memwipe(cipher, 0, sizeof(aes_cnt_cipher_t));
   tor_free(cipher);
 }
 
diff --git a/src/common/compat.c b/src/common/compat.c
index ca850a3..59e3898 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -332,7 +332,7 @@ tor_munmap_file(tor_mmap_t *handle)
 {
   char *d = (char*)handle->data;
   tor_free(d);
-  memset(handle, 0, sizeof(tor_mmap_t));
+  memwipe(handle, 0, sizeof(tor_mmap_t));
   tor_free(handle);
 }
 #endif
diff --git a/src/common/crypto.c b/src/common/crypto.c
index a69e6c5..30990ec 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -425,7 +425,7 @@ crypto_cipher_free(crypto_cipher_t *env)
 
   tor_assert(env->cipher);
   aes_cipher_free(env->cipher);
-  memset(env, 0, sizeof(crypto_cipher_t));
+  memwipe(env, 0, sizeof(crypto_cipher_t));
   tor_free(env);
 }
 
@@ -529,7 +529,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_t *env,
 
   /* Try to parse it. */
   r = crypto_pk_read_private_key_from_string(env, contents, -1);
-  memset(contents, 0, strlen(contents));
+  memwipe(contents, 0, strlen(contents));
   tor_free(contents);
   if (r)
     return -1; /* read_private_key_from_string already warned, so we don't.*/
@@ -671,7 +671,7 @@ crypto_pk_write_private_key_to_filename(crypto_pk_t *env,
   s[len]='\0';
   r = write_str_to_file(fname, s, 0);
   BIO_free(bio);
-  memset(s, 0, strlen(s));
+  memwipe(s, 0, strlen(s));
   tor_free(s);
   return r;
 }
@@ -981,7 +981,7 @@ crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen,
   if (crypto_digest(digest,from,fromlen)<0)
     return -1;
   r = crypto_pk_private_sign(env,to,tolen,digest,DIGEST_LEN);
-  memset(digest, 0, sizeof(digest));
+  memwipe(digest, 0, sizeof(digest));
   return r;
 }
 
@@ -1045,14 +1045,14 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env,
                             from+pkeylen-overhead-CIPHER_KEY_LEN, symlen);
 
   if (r<0) goto err;
-  memset(buf, 0, pkeylen);
+  memwipe(buf, 0, pkeylen);
   tor_free(buf);
   crypto_cipher_free(cipher);
   tor_assert(outlen+symlen < INT_MAX);
   return (int)(outlen + symlen);
  err:
 
-  memset(buf, 0, pkeylen);
+  memwipe(buf, 0, pkeylen);
   tor_free(buf);
   crypto_cipher_free(cipher);
   return -1;
@@ -1103,13 +1103,13 @@ crypto_pk_private_hybrid_decrypt(crypto_pk_t *env,
   r = crypto_cipher_decrypt(cipher, to+outlen, from+pkeylen, fromlen-pkeylen);
   if (r<0)
     goto err;
-  memset(buf,0,pkeylen);
+  memwipe(buf,0,pkeylen);
   tor_free(buf);
   crypto_cipher_free(cipher);
   tor_assert(outlen + fromlen < INT_MAX);
   return (int)(outlen + (fromlen-pkeylen));
  err:
-  memset(buf,0,pkeylen);
+  memwipe(buf,0,pkeylen);
   tor_free(buf);
   crypto_cipher_free(cipher);
   return -1;
@@ -1509,7 +1509,7 @@ crypto_digest_free(crypto_digest_t *digest)
 {
   if (!digest)
     return;
-  memset(digest, 0, sizeof(crypto_digest_t));
+  memwipe(digest, 0, sizeof(crypto_digest_t));
   tor_free(digest);
 }
 
@@ -1571,7 +1571,7 @@ crypto_digest_get_digest(crypto_digest_t *digest,
       break;
   }
   memcpy(out, r, out_len);
-  memset(r, 0, sizeof(r));
+  memwipe(r, 0, sizeof(r));
 }
 
 /** Allocate and return a new digest object with the same state as
@@ -1673,10 +1673,10 @@ crypto_hmac_sha256(char *hmac_out,
   SHA256_Final((uint8_t*)hmac_out, &st);
 
   /* Now clear everything. */
-  memset(k, 0, sizeof(k));
-  memset(pad, 0, sizeof(pad));
-  memset(d, 0, sizeof(d));
-  memset(&st, 0, sizeof(st));
+  memwipe(k, 0, sizeof(k));
+  memwipe(pad, 0, sizeof(pad));
+  memwipe(d, 0, sizeof(d));
+  memwipe(&st, 0, sizeof(st));
 #undef BLOCKSIZE
 #undef DIGESTSIZE
 #endif
@@ -2208,7 +2208,7 @@ crypto_dh_compute_secret(int severity, crypto_dh_t *dh,
   if (pubkey_bn)
     BN_free(pubkey_bn);
   if (secret_tmp) {
-    memset(secret_tmp, 0, secret_tmp_len);
+    memwipe(secret_tmp, 0, secret_tmp_len);
     tor_free(secret_tmp);
   }
   if (result < 0)
@@ -2243,15 +2243,15 @@ crypto_expand_key_material(const char *key_in, size_t key_in_len,
       goto err;
     memcpy(cp, digest, MIN(DIGEST_LEN, key_out_len-(cp-key_out)));
   }
-  memset(tmp, 0, key_in_len+1);
+  memwipe(tmp, 0, key_in_len+1);
   tor_free(tmp);
-  memset(digest, 0, sizeof(digest));
+  memwipe(digest, 0, sizeof(digest));
   return 0;
 
  err:
-  memset(tmp, 0, key_in_len+1);
+  memwipe(tmp, 0, key_in_len+1);
   tor_free(tmp);
-  memset(digest, 0, sizeof(digest));
+  memwipe(digest, 0, sizeof(digest));
   return -1;
 }
 
@@ -2343,7 +2343,7 @@ crypto_seed_rng(int startup)
     return rand_poll_status ? 0 : -1;
   }
   RAND_seed(buf, sizeof(buf));
-  memset(buf, 0, sizeof(buf));
+  memwipe(buf, 0, sizeof(buf));
   seed_weak_rng();
   return 0;
 #else
@@ -2360,7 +2360,7 @@ crypto_seed_rng(int startup)
       return -1;
     }
     RAND_seed(buf, (int)sizeof(buf));
-    memset(buf, 0, sizeof(buf));
+    memwipe(buf, 0, sizeof(buf));
     seed_weak_rng();
     return 0;
   }
@@ -2843,7 +2843,7 @@ base32_decode(char *dest, size_t destlen, const char *src, size_t srclen)
     }
   }
 
-  memset(tmp, 0, srclen);
+  memwipe(tmp, 0, srclen);
   tor_free(tmp);
   tmp = NULL;
   return 0;
@@ -2888,11 +2888,54 @@ secret_to_key(char *key_out, size_t key_out_len, const char *secret,
     }
   }
   crypto_digest_get_digest(d, key_out, key_out_len);
-  memset(tmp, 0, tmplen);
+  memwipe(tmp, 0, tmplen);
   tor_free(tmp);
   crypto_digest_free(d);
 }
 
+/**
+ * Destroy the <b>sz</b> bytes of data stored at <b>mem</b>, setting them to
+ * the value <b>byte</b>.
+ *
+ * This function is preferable to memset, since many compilers will happily
+ * optimize out memset() when they can convince themselves that the data being
+ * cleared will never be read.
+ *
+ * Right now, our convention is to use this function when we are wiping data
+ * that's about to become inaccessible, such as stack buffers that are about
+ * to go out of scope or structures that are about to get freed.  (In
+ * practice, it appears that the compilers we're currently using will optimize
+ * out the memset()s for stack-allocated buffers, but not those for
+ * about-to-be-freed structures. That could change, though, so we're being
+ * wary.)  If there are live reads for the data, then you can just use
+ * memset().
+ */
+void
+memwipe(void *mem, uint8_t byte, size_t sz)
+{
+  /* Because whole-program-optimization exists, we may not be able to just
+   * have this function call "memset".  A smart compiler could inline it, then
+   * eliminate dead memsets, and declare itself to be clever. */
+
+  /* This is a slow and ugly function from OpenSSL that fills 'mem' with junk
+   * based on the pointer value, then uses that junk to update a global
+   * variable.  It's an elaborate ruse to trick the compiler into not
+   * optimizing out the "wipe this memory" code.  Read it if you like zany
+   * programming tricks! In later versions of Tor, we should look for better
+   * not-optimized-out memory wiping stuff. */
+  OPENSSL_cleanse(mem, sz);
+  /* Just in case some caller of memwipe() is relying on getting a buffer
+   * filled with a particular value, fill the buffer.
+   *
+   * If this function gets inlined, this memset might get eliminated, but
+   * that's okay: We only care about this particular memset in the case where
+   * the caller should have been using memset(), and the memset() wouldn't get
+   * eliminated.  In other words, this is here so that we won't break anything
+   * if somebody accidentally calls memwipe() instead of memset().
+   **/
+  memset(mem, byte, sz);
+}
+
 #ifdef TOR_IS_MULTITHREADED
 /** Helper: OpenSSL uses this callback to manipulate mutexes. */
 static void
diff --git a/src/common/crypto.h b/src/common/crypto.h
index 76bcbf7..7d56271 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -271,6 +271,9 @@ int digest256_from_base64(char *digest, const char *d64);
 void secret_to_key(char *key_out, size_t key_out_len, const char *secret,
                    size_t secret_len, const char *s2k_specifier);
 
+/** OpenSSL-based utility functions. */
+void memwipe(void *mem, uint8_t byte, size_t sz);
+
 #ifdef CRYPTO_PRIVATE
 /* Prototypes for private functions only used by tortls.c, crypto.c, and the
  * unit tests. */
diff --git a/src/common/mempool.c b/src/common/mempool.c
index 2416bce..637f081 100644
--- a/src/common/mempool.c
+++ b/src/common/mempool.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include "torint.h"
+#include "crypto.h"
 #define MEMPOOL_PRIVATE
 #include "mempool.h"
 
@@ -519,7 +520,7 @@ mp_pool_destroy(mp_pool_t *pool)
   destroy_chunks(pool->empty_chunks);
   destroy_chunks(pool->used_chunks);
   destroy_chunks(pool->full_chunks);
-  memset(pool, 0xe0, sizeof(mp_pool_t));
+  memwipe(pool, 0xe0, sizeof(mp_pool_t));
   FREE(pool);
 }
 
diff --git a/src/common/tortls.c b/src/common/tortls.c
index bec2c71..60aac64 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -713,7 +713,7 @@ tor_cert_free(tor_cert_t *cert)
   if (cert->cert)
     X509_free(cert->cert);
   tor_free(cert->encoded);
-  memset(cert, 0x03, sizeof(*cert));
+  memwipe(cert, 0x03, sizeof(*cert));
   tor_free(cert);
 }
 
@@ -2448,7 +2448,7 @@ tor_tls_get_tlssecrets(tor_tls_t *tls, uint8_t *secrets_out)
                      (char*)tls->ssl->session->master_key,
                      tls->ssl->session->master_key_length,
                      buf, len);
-  memset(buf, 0, sizeof(buf));
+  memwipe(buf, 0, sizeof(buf));
   return 0;
 }
 
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 9acc229..ad5ab83 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1546,14 +1546,14 @@ socks_request_free(socks_request_t *req)
   if (!req)
     return;
   if (req->username) {
-    memset(req->username, 0x10, req->usernamelen);
+    memwipe(req->username, 0x10, req->usernamelen);
     tor_free(req->username);
   }
   if (req->password) {
-    memset(req->password, 0x04, req->passwordlen);
+    memwipe(req->password, 0x04, req->passwordlen);
     tor_free(req->password);
   }
-  memset(req, 0xCC, sizeof(socks_request_t));
+  memwipe(req, 0xCC, sizeof(socks_request_t));
   tor_free(req);
 }
 
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 7ed942c..93ba69d 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -612,11 +612,11 @@ circuit_free(circuit_t *circ)
 
     tor_free(ocirc->dest_address);
     if (ocirc->socks_username) {
-      memset(ocirc->socks_username, 0x12, ocirc->socks_username_len);
+      memwipe(ocirc->socks_username, 0x12, ocirc->socks_username_len);
       tor_free(ocirc->socks_username);
     }
     if (ocirc->socks_password) {
-      memset(ocirc->socks_password, 0x06, ocirc->socks_password_len);
+      memwipe(ocirc->socks_password, 0x06, ocirc->socks_password_len);
       tor_free(ocirc->socks_password);
     }
   } else {
@@ -657,7 +657,7 @@ circuit_free(circuit_t *circ)
    * "active" checks will be violated. */
   cell_queue_clear(&circ->n_conn_cells);
 
-  memset(mem, 0xAA, memlen); /* poison memory */
+  memwipe(mem, 0xAA, memlen); /* poison memory */
   tor_free(mem);
 }
 
@@ -721,7 +721,7 @@ circuit_free_cpath_node(crypt_path_t *victim)
   crypto_dh_free(victim->dh_handshake_state);
   extend_info_free(victim->extend_info);
 
-  memset(victim, 0xBB, sizeof(crypt_path_t)); /* poison memory */
+  memwipe(victim, 0xBB, sizeof(crypt_path_t)); /* poison memory */
   tor_free(victim);
 }
 
diff --git a/src/or/connection.c b/src/or/connection.c
index 364e491..eac9c4f 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -561,7 +561,7 @@ _connection_free(connection_t *conn)
   }
 #endif
 
-  memset(mem, 0xCC, memlen); /* poison memory */
+  memwipe(mem, 0xCC, memlen); /* poison memory */
   tor_free(mem);
 }
 
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 1592033..9563ca6 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -3709,11 +3709,11 @@ circuit_clear_isolation(origin_circuit_t *circ)
   circ->session_group = -1;
   circ->nym_epoch = 0;
   if (circ->socks_username) {
-    memset(circ->socks_username, 0x11, circ->socks_username_len);
+    memwipe(circ->socks_username, 0x11, circ->socks_username_len);
     tor_free(circ->socks_username);
   }
   if (circ->socks_password) {
-    memset(circ->socks_password, 0x05, circ->socks_password_len);
+    memwipe(circ->socks_password, 0x05, circ->socks_password_len);
     tor_free(circ->socks_password);
   }
   circ->socks_username_len = circ->socks_password_len = 0;
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 6293fe8..5eecee0 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -1657,7 +1657,7 @@ or_handshake_state_free(or_handshake_state_t *state)
   crypto_digest_free(state->digest_received);
   tor_cert_free(state->auth_cert);
   tor_cert_free(state->id_cert);
-  memset(state, 0xBE, sizeof(or_handshake_state_t));
+  memwipe(state, 0xBE, sizeof(or_handshake_state_t));
   tor_free(state);
 }
 
@@ -1698,7 +1698,7 @@ or_handshake_state_record_cell(or_handshake_state_t *state,
      this very often at all. */
   cell_pack(&packed, cell);
   crypto_digest_add_bytes(d, packed.body, sizeof(packed.body));
-  memset(&packed, 0, sizeof(packed));
+  memwipe(&packed, 0, sizeof(packed));
 }
 
 /** Remember that a variable-length <b>cell</b> has been transmitted (if
@@ -1733,7 +1733,7 @@ or_handshake_state_record_var_cell(or_handshake_state_t *state,
   crypto_digest_add_bytes(d, buf, sizeof(buf));
   crypto_digest_add_bytes(d, (const char *)cell->payload, cell->payload_len);
 
-  memset(buf, 0, sizeof(buf));
+  memwipe(buf, 0, sizeof(buf));
 }
 
 /** Set <b>conn</b>'s state to OR_CONN_STATE_OPEN, and tell other subsystems
@@ -2090,7 +2090,7 @@ connection_or_send_auth_challenge_cell(or_connection_t *conn)
 
   connection_or_write_var_cell_to_buf(cell, conn);
   var_cell_free(cell);
-  memset(challenge, 0, sizeof(challenge));
+  memwipe(challenge, 0, sizeof(challenge));
 
   return 0;
 }
diff --git a/src/or/main.c b/src/or/main.c
index 20a1e08..34bf3e5 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1199,7 +1199,7 @@ run_scheduled_events(time_t now)
     if (router_rebuild_descriptor(1)<0) {
       log_info(LD_CONFIG, "Couldn't rebuild router descriptor");
     }
-    if (advertised_server_mode() & !options->DisableNetwork)
+    if (advertised_server_mode() && !options->DisableNetwork)
       router_upload_dir_desc_to_dirservers(0);
   }
 
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index fadaf90..2553a74 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -413,7 +413,7 @@ networkstatus_vote_free(networkstatus_t *ns)
 
   digestmap_free(ns->desc_digest_map, NULL);
 
-  memset(ns, 11, sizeof(*ns));
+  memwipe(ns, 11, sizeof(*ns));
   tor_free(ns);
 }
 
diff --git a/src/or/onion.c b/src/or/onion.c
index ff1556c..f8c4d72 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -206,12 +206,12 @@ onion_skin_create(crypto_pk_t *dest_router_key,
                                       PK_PKCS1_OAEP_PADDING, 1)<0)
     goto err;
 
-  memset(challenge, 0, sizeof(challenge));
+  memwipe(challenge, 0, sizeof(challenge));
   *handshake_state_out = dh;
 
   return 0;
  err:
-  memset(challenge, 0, sizeof(challenge));
+  memwipe(challenge, 0, sizeof(challenge));
   if (dh) crypto_dh_free(dh);
   return -1;
 }
@@ -286,15 +286,15 @@ onion_skin_server_handshake(const char *onion_skin, /*ONIONSKIN_CHALLENGE_LEN*/
   /* use the rest of the key material for our shared keys, digests, etc */
   memcpy(key_out, key_material+DIGEST_LEN, key_out_len);
 
-  memset(challenge, 0, sizeof(challenge));
-  memset(key_material, 0, key_material_len);
+  memwipe(challenge, 0, sizeof(challenge));
+  memwipe(key_material, 0, key_material_len);
   tor_free(key_material);
   crypto_dh_free(dh);
   return 0;
  err:
-  memset(challenge, 0, sizeof(challenge));
+  memwipe(challenge, 0, sizeof(challenge));
   if (key_material) {
-    memset(key_material, 0, key_material_len);
+    memwipe(key_material, 0, key_material_len);
     tor_free(key_material);
   }
   if (dh) crypto_dh_free(dh);
@@ -340,11 +340,11 @@ onion_skin_client_handshake(crypto_dh_t *handshake_state,
   /* use the rest of the key material for our shared keys, digests, etc */
   memcpy(key_out, key_material+DIGEST_LEN, key_out_len);
 
-  memset(key_material, 0, key_material_len);
+  memwipe(key_material, 0, key_material_len);
   tor_free(key_material);
   return 0;
  err:
-  memset(key_material, 0, key_material_len);
+  memwipe(key_material, 0, key_material_len);
   tor_free(key_material);
   return -1;
 }
@@ -381,8 +381,8 @@ fast_server_handshake(const uint8_t *key_in, /* DIGEST_LEN bytes */
   memcpy(key_out, out+DIGEST_LEN, key_out_len);
   r = 0;
  done:
-  memset(tmp, 0, sizeof(tmp));
-  memset(out, 0, out_len);
+  memwipe(tmp, 0, sizeof(tmp));
+  memwipe(out, 0, out_len);
   tor_free(out);
   return r;
 }
@@ -426,8 +426,8 @@ fast_client_handshake(const uint8_t *handshake_state,/*DIGEST_LEN bytes*/
   memcpy(key_out, out+DIGEST_LEN, key_out_len);
   r = 0;
  done:
-  memset(tmp, 0, sizeof(tmp));
-  memset(out, 0, out_len);
+  memwipe(tmp, 0, sizeof(tmp));
+  memwipe(out, 0, out_len);
   tor_free(out);
   return r;
 }
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 6c751be..3a0cd1a 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -908,10 +908,10 @@ rend_client_receive_rendezvous(origin_circuit_t *circ, const uint8_t *request,
 
   circuit_try_attaching_streams(circ);
 
-  memset(keys, 0, sizeof(keys));
+  memwipe(keys, 0, sizeof(keys));
   return 0;
  err:
-  memset(keys, 0, sizeof(keys));
+  memwipe(keys, 0, sizeof(keys));
   circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_TORPROTOCOL);
   return -1;
 }
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 6af4778..d235f08 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1404,10 +1404,10 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request,
   memcpy(cpath->handshake_digest, keys, DIGEST_LEN);
   if (extend_info) extend_info_free(extend_info);
 
-  memset(keys, 0, sizeof(keys));
+  memwipe(keys, 0, sizeof(keys));
   return 0;
  err:
-  memset(keys, 0, sizeof(keys));
+  memwipe(keys, 0, sizeof(keys));
   if (dh) crypto_dh_free(dh);
   if (launched)
     circuit_mark_for_close(TO_CIRCUIT(launched), reason);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 2bf072b..299d07d 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -4596,7 +4596,7 @@ tor_version_parse(const char *s, tor_version_t *out)
     if (close_paren-cp > HEX_DIGEST_LEN)
       return -1;
     hexlen = (int)(close_paren-cp);
-    memset(digest, 0, sizeof(digest));
+    memwipe(digest, 0, sizeof(digest));
     if ( hexlen == 0 || (hexlen % 2) == 1)
       return -1;
     if (base16_decode(digest, hexlen/2, cp, hexlen))
diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c
index 38b2101..c7ab8dc 100644
--- a/src/tools/tor-gencert.c
+++ b/src/tools/tor-gencert.c
@@ -105,7 +105,7 @@ load_passphrase(void)
   cp = memchr(buf, '\n', n);
   passphrase_len = cp-buf;
   passphrase = tor_strndup(buf, passphrase_len);
-  memset(buf, 0, sizeof(buf));
+  memwipe(buf, 0, sizeof(buf));
   return 0;
 }
 
@@ -113,7 +113,7 @@ static void
 clear_passphrase(void)
 {
   if (passphrase) {
-    memset(passphrase, 0, passphrase_len);
+    memwipe(passphrase, 0, passphrase_len);
     tor_free(passphrase);
   }
 }
@@ -191,7 +191,7 @@ parse_commandline(int argc, char **argv)
     }
   }
 
-  memset(&s, 0, sizeof(s));
+  memwipe(&s, 0, sizeof(s));
   if (verbose)
     set_log_severity_config(LOG_DEBUG, LOG_ERR, &s);
   else
diff --git a/src/win32/orconfig.h b/src/win32/orconfig.h
index 5a67369..90c6b52 100644
--- a/src/win32/orconfig.h
+++ b/src/win32/orconfig.h
@@ -232,7 +232,7 @@
 #define USING_TWOS_COMPLEMENT
 
 /* Version number of package */
-#define VERSION "0.2.3.24-rc"
+#define VERSION "0.2.3.25"
 
 
 

--- End Message ---
--- Begin Message ---
On Sun, 2012-11-25 at 19:48 +0100, Peter Palfrader wrote:
> It turns out that gcc sometimes is too smart for our good: when it
> realizes that we write pretty zeroes over a buffer but then no longer
> use that buffer, it will optimise away that memset().  And for a
> correctly functioning program that's fine, but our use of memset() is
> for defense in depth - to limit damage when we are not working
> correctly.
> 
> In version 0.2.3.25-1, Tor tries harder to wipe memory, replacing
> several calls to memset() with calls to a tor memwipe().  This memwipe()
> implementation calls openssl's OPENSSL_cleanse() which does some pretty
> weird stuff to ensure gcc doesn't catch on.

Unblocked; thanks.

Regards,

Adam

--- End Message ---

Reply to: