diff options
45 files changed, 900 insertions, 148 deletions
@@ -1,3 +1,187 @@ +Changes in version 0.3.1.4-alpha - 2017-06-29 + Tor 0.3.1.4-alpha fixes a path selection bug that would allow a client + to use a guard that was in the same network family as a chosen exit + relay. This is a security regression; all clients running earlier + versions of 0.3.0.x or 0.3.1.x should upgrade to 0.3.0.9 + or 0.3.1.4-alpha. + + This release also fixes several other bugs introduced in 0.3.0.x + and 0.3.1.x, including others that can affect bandwidth usage + and correctness. + + o New dependencies: + - To build with zstd and lzma support, Tor now requires the + pkg-config tool at build time. (This requirement was new in + 0.3.1.1-alpha, but was not noted at the time. Noting it here to + close ticket 22623.) + + o Major bugfixes (path selection, security): + - When choosing which guard to use for a circuit, avoid the exit's + family along with the exit itself. Previously, the new guard + selection logic avoided the exit, but did not consider its family. + Fixes bug 22753; bugfix on 0.3.0.1-alpha. Tracked as TROVE-2016- + 006 and CVE-2017-0377. + + o Major bugfixes (compression, zstd): + - Correctly detect a full buffer when decompressing a large zstd- + compressed input. Previously, we would sometimes treat a full + buffer as an error. Fixes bug 22628; bugfix on 0.3.1.1-alpha. + + o Major bugfixes (directory protocol): + - Ensure that we send "304 Not modified" as HTTP status code when a + client is attempting to fetch a consensus or consensus diff, and + the best one we can send them is one they already have. Fixes bug + 22702; bugfix on 0.3.1.1-alpha. + + o Major bugfixes (entry guards): + - When starting with an old consensus, do not add new entry guards + unless the consensus is "reasonably live" (under 1 day old). Fixes + one root cause of bug 22400; bugfix on 0.3.0.1-alpha. + + o Minor features (bug mitigation, diagnostics, logging): + - Avoid an assertion failure, and log a better error message, when + unable to remove a file from the consensus cache on Windows. + Attempts to mitigate and diagnose bug 22752. + + o Minor features (geoip): + - Update geoip and geoip6 to the June 8 2017 Maxmind GeoLite2 + Country database. + + o Minor bugfixes (compression): + - When compressing or decompressing a buffer, check for a failure to + create a compression object. Fixes bug 22626; bugfix + on 0.3.1.1-alpha. + - When decompressing a buffer, check for extra data after the end of + the compressed data. Fixes bug 22629; bugfix on 0.3.1.1-alpha. + - When decompressing an object received over an anonymous directory + connection, if we have already decompressed it using an acceptable + compression method, do not reject it for looking like an + unacceptable compression method. Fixes part of bug 22670; bugfix + on 0.3.1.1-alpha. + - When serving directory votes compressed with zlib, do not claim to + have compressed them with zstd. Fixes bug 22669; bugfix + on 0.3.1.1-alpha. + - When spooling compressed data to an output buffer, don't try to + spool more data when there is no more data to spool and we are not + trying to flush the input. Previously, we would sometimes launch + compression requests with nothing to do, which interferes with our + 22672 checks. Fixes bug 22719; bugfix on 0.2.0.16-alpha. + + o Minor bugfixes (defensive programming): + - Detect and break out of infinite loops in our compression code. We + don't think that any such loops exist now, but it's best to be + safe. Closes ticket 22672. + - Fix a memset() off the end of an array when packing cells. This + bug should be harmless in practice, since the corrupted bytes are + still in the same structure, and are always padding bytes, + ignored, or immediately overwritten, depending on compiler + behavior. Nevertheless, because the memset()'s purpose is to make + sure that any other cell-handling bugs can't expose bytes to the + network, we need to fix it. Fixes bug 22737; bugfix on + 0.2.4.11-alpha. Fixes CID 1401591. + + o Minor bugfixes (linux seccomp2 sandbox): + - Permit the fchmod system call, to avoid crashing on startup when + starting with the seccomp2 sandbox and an unexpected set of + permissions on the data directory or its contents. Fixes bug + 22516; bugfix on 0.2.5.4-alpha. + - Fix a crash in the LZMA module, when the sandbox was enabled, and + liblzma would allocate more than 16 MB of memory. We solve this by + bumping the mprotect() limit in the sandbox module from 16 MB to + 20 MB. Fixes bug 22751; bugfix on 0.3.1.1-alpha. + + o Minor bugfixes (logging): + - When decompressing, do not warn if we fail to decompress using a + compression method that we merely guessed. Fixes part of bug + 22670; bugfix on 0.1.1.14-alpha. + - When decompressing, treat mismatch between content-encoding and + actual compression type as a protocol warning. Fixes part of bug + 22670; bugfix on 0.1.1.9-alpha. + - Downgrade "assigned_to_cpuworker failed" message to info-level + severity. In every case that can reach it, either a better warning + has already been logged, or no warning is warranted. Fixes bug + 22356; bugfix on 0.2.6.3-alpha. + - Demote a warn that was caused by libevent delays to info if + netflow padding is less than 4.5 seconds late, or to notice + if it is more (4.5 seconds is the amount of time that a netflow + record might be emitted after, if we chose the maximum timeout). + Fixes bug 22212; bugfix on 0.3.1.1-alpha. + + o Minor bugfixes (process behavior): + - When exiting because of an error, always exit with a nonzero exit + status. Previously, we would fail to report an error in our exit + status in cases related to __OwningControllerProcess failure, + lockfile contention, and Ed25519 key initialization. Fixes bug + 22720; bugfix on versions 0.2.1.6-alpha, 0.2.2.28-beta, and + 0.2.7.2-alpha respectively. Reported by "f55jwk4f"; patch + from "huyvq". + + o Documentation: + - Add a manpage description for the key-pinning-journal file. Closes + ticket 22347. + - Correctly note that bandwidth accounting values are stored in the + state file, and the bw_accounting file is now obsolete. Closes + ticket 16082. + - Document more of the files in the Tor data directory, including + cached-extrainfo, secret_onion_key{,_ntor}.old, hidserv-stats, + approved-routers, sr-random, and diff-cache. Found while fixing + ticket 22347. + + +Changes in version 0.3.0.9 - 2017-06-29 + Tor 0.3.0.9 fixes a path selection bug that would allow a client + to use a guard that was in the same network family as a chosen exit + relay. This is a security regression; all clients running earlier + versions of 0.3.0.x or 0.3.1.x should upgrade to 0.3.0.9 or + 0.3.1.4-alpha. + + This release also backports several other bugfixes from the 0.3.1.x + series. + + o Major bugfixes (path selection, security, backport from 0.3.1.4-alpha): + - When choosing which guard to use for a circuit, avoid the exit's + family along with the exit itself. Previously, the new guard + selection logic avoided the exit, but did not consider its family. + Fixes bug 22753; bugfix on 0.3.0.1-alpha. Tracked as TROVE-2016- + 006 and CVE-2017-0377. + + o Major bugfixes (entry guards, backport from 0.3.1.1-alpha): + - Don't block bootstrapping when a primary bridge is offline and we + can't get its descriptor. Fixes bug 22325; fixes one case of bug + 21969; bugfix on 0.3.0.3-alpha. + + o Major bugfixes (entry guards, backport from 0.3.1.4-alpha): + - When starting with an old consensus, do not add new entry guards + unless the consensus is "reasonably live" (under 1 day old). Fixes + one root cause of bug 22400; bugfix on 0.3.0.1-alpha. + + o Minor features (geoip): + - Update geoip and geoip6 to the June 8 2017 Maxmind GeoLite2 + Country database. + + o Minor bugfixes (voting consistency, backport from 0.3.1.1-alpha): + - Reject version numbers with non-numeric prefixes (such as +, -, or + whitespace). Disallowing whitespace prevents differential version + parsing between POSIX-based and Windows platforms. Fixes bug 21507 + and part of 21508; bugfix on 0.0.8pre1. + + o Minor bugfixes (linux seccomp2 sandbox, backport from 0.3.1.4-alpha): + - Permit the fchmod system call, to avoid crashing on startup when + starting with the seccomp2 sandbox and an unexpected set of + permissions on the data directory or its contents. Fixes bug + 22516; bugfix on 0.2.5.4-alpha. + + o Minor bugfixes (defensive programming, backport from 0.3.1.4-alpha): + - Fix a memset() off the end of an array when packing cells. This + bug should be harmless in practice, since the corrupted bytes are + still in the same structure, and are always padding bytes, + ignored, or immediately overwritten, depending on compiler + behavior. Nevertheless, because the memset()'s purpose is to make + sure that any other cell-handling bugs can't expose bytes to the + network, we need to fix it. Fixes bug 22737; bugfix on + 0.2.4.11-alpha. Fixes CID 1401591. + + Changes in version 0.3.1.3-alpha - 2017-06-08 Tor 0.3.1.3-alpha fixes a pair of bugs that would allow an attacker to remotely crash a hidden service with an assertion failure. Anyone @@ -1996,7 +2180,7 @@ Changes in version 0.3.0.1-alpha - 2016-12-19 subprotocol-versions mechanism, so that clients can tell which relays can identity themselves by Ed25519 ID. Closes ticket 20552. - o Minor features (fingerprinting resistence, authentication): + o Minor features (fingerprinting resistance, authentication): - Extend the length of RSA keys used for TLS link authentication to 2048 bits. (These weren't used for forward secrecy; for forward secrecy, we used P256.) Closes ticket 13752. @@ -5062,7 +5246,7 @@ Changes in version 0.2.6.8 - 2015-05-21 o Major bugfixes (hidden services, backport from 0.2.7.1-alpha): - Revert commit that made directory authorities assign the HSDir - flag to relay without a DirPort; this was bad because such relays + flag to relays without a DirPort; this was bad because such relays can't handle BEGIN_DIR cells. Fixes bug 15850; bugfix on 0.2.6.3-alpha. @@ -5103,7 +5287,7 @@ Changes in version 0.2.7.1-alpha - 2015-05-12 o Major bugfixes (hidden services): - Revert commit that made directory authorities assign the HSDir - flag to relay without a DirPort; this was bad because such relays + flag to relays without a DirPort; this was bad because such relays can't handle BEGIN_DIR cells. Fixes bug 15850; bugfix on 0.2.6.3-alpha. diff --git a/ReleaseNotes b/ReleaseNotes index 1e56ffaf89..80c3e4c72d 100644 --- a/ReleaseNotes +++ b/ReleaseNotes @@ -2,6 +2,60 @@ 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.3.0.9 - 2017-06-29 + Tor 0.3.0.9 fixes a path selection bug that would allow a client + to use a guard that was in the same network family as a chosen exit + relay. This is a security regression; all clients running earlier + versions of 0.3.0.x or 0.3.1.x should upgrade to 0.3.0.9 or + 0.3.1.4-alpha. + + This release also backports several other bugfixes from the 0.3.1.x + series. + + o Major bugfixes (path selection, security, backport from 0.3.1.4-alpha): + - When choosing which guard to use for a circuit, avoid the exit's + family along with the exit itself. Previously, the new guard + selection logic avoided the exit, but did not consider its family. + Fixes bug 22753; bugfix on 0.3.0.1-alpha. Tracked as TROVE-2016- + 006 and CVE-2017-0377. + + o Major bugfixes (entry guards, backport from 0.3.1.1-alpha): + - Don't block bootstrapping when a primary bridge is offline and we + can't get its descriptor. Fixes bug 22325; fixes one case of bug + 21969; bugfix on 0.3.0.3-alpha. + + o Major bugfixes (entry guards, backport from 0.3.1.4-alpha): + - When starting with an old consensus, do not add new entry guards + unless the consensus is "reasonably live" (under 1 day old). Fixes + one root cause of bug 22400; bugfix on 0.3.0.1-alpha. + + o Minor features (geoip): + - Update geoip and geoip6 to the June 8 2017 Maxmind GeoLite2 + Country database. + + o Minor bugfixes (voting consistency, backport from 0.3.1.1-alpha): + - Reject version numbers with non-numeric prefixes (such as +, -, or + whitespace). Disallowing whitespace prevents differential version + parsing between POSIX-based and Windows platforms. Fixes bug 21507 + and part of 21508; bugfix on 0.0.8pre1. + + o Minor bugfixes (linux seccomp2 sandbox, backport from 0.3.1.4-alpha): + - Permit the fchmod system call, to avoid crashing on startup when + starting with the seccomp2 sandbox and an unexpected set of + permissions on the data directory or its contents. Fixes bug + 22516; bugfix on 0.2.5.4-alpha. + + o Minor bugfixes (defensive programming, backport from 0.3.1.4-alpha): + - Fix a memset() off the end of an array when packing cells. This + bug should be harmless in practice, since the corrupted bytes are + still in the same structure, and are always padding bytes, + ignored, or immediately overwritten, depending on compiler + behavior. Nevertheless, because the memset()'s purpose is to make + sure that any other cell-handling bugs can't expose bytes to the + network, we need to fix it. Fixes bug 22737; bugfix on + 0.2.4.11-alpha. Fixes CID 1401591. + + Changes in version 0.3.0.8 - 2017-06-08 Tor 0.3.0.8 fixes a pair of bugs that would allow an attacker to remotely crash a hidden service with an assertion failure. Anyone @@ -538,7 +592,7 @@ Changes in version 0.3.0.6 - 2017-04-26 - Select 200 fallback directories for each release. Closes ticket 20881. - o Minor features (fingerprinting resistence, authentication): + o Minor features (fingerprinting resistance, authentication): - Extend the length of RSA keys used for TLS link authentication to 2048 bits. (These weren't used for forward secrecy; for forward secrecy, we used P256.) Closes ticket 13752. @@ -3185,7 +3239,7 @@ Changes in version 0.2.7.5 - 2015-11-20 o Major bugfixes (hidden services): - Revert commit that made directory authorities assign the HSDir - flag to relay without a DirPort; this was bad because such relays + flag to relays without a DirPort; this was bad because such relays can't handle BEGIN_DIR cells. Fixes bug 15850; bugfix on 0.2.6.3-alpha. - When cannibalizing a circuit for an introduction point, always @@ -3800,7 +3854,7 @@ Changes in version 0.2.6.8 - 2015-05-21 o Major bugfixes (hidden services, backport from 0.2.7.1-alpha): - Revert commit that made directory authorities assign the HSDir - flag to relay without a DirPort; this was bad because such relays + flag to relays without a DirPort; this was bad because such relays can't handle BEGIN_DIR cells. Fixes bug 15850; bugfix on 0.2.6.3-alpha. diff --git a/changes/bug22006 b/changes/bug22006 new file mode 100644 index 0000000000..912bdd87bd --- /dev/null +++ b/changes/bug22006 @@ -0,0 +1,4 @@ + o Minor features (ed25519): + - Add validation function that checks for torsion components in ed25119 + public keys. Currently unused but will be used by prop224 client-side + code. Addresses ticket #22006. Math help by Ian Goldberg. diff --git a/changes/bug22670 b/changes/bug22670 new file mode 100644 index 0000000000..47403277d2 --- /dev/null +++ b/changes/bug22670 @@ -0,0 +1,4 @@ + o Minor bugfixes (logging, compression): + - When decompressing, do not warn if we fail to decompress using a + compression method that we merely guessed. Fixes part of + bug 22670; bugfix on 0.1.1.14-alpha. diff --git a/changes/bug22670_02 b/changes/bug22670_02 new file mode 100644 index 0000000000..3e7a428faf --- /dev/null +++ b/changes/bug22670_02 @@ -0,0 +1,4 @@ + o Minor bugfixes (logging, compression): + - When decompressing, treat mismatch between content-encoding and + actual compression type as a protocol warning. Fixes part of bug + 22670; bugfix on 0.1.1.9-alpha. diff --git a/changes/bug22670_03 b/changes/bug22670_03 new file mode 100644 index 0000000000..8a7aa49bcd --- /dev/null +++ b/changes/bug22670_03 @@ -0,0 +1,6 @@ + o Minor bugfixes (compression): + - When decompressing an object received over an anonymous directory + connection, if we have already successfully decompressed it using an + acceptable compression method, do not reject it for looking like an + unacceptable compression method. Fixes part of bug 22670; bugfix on + 0.3.1.1-alpha. diff --git a/changes/bug22702 b/changes/bug22702 new file mode 100644 index 0000000000..a2044c70bf --- /dev/null +++ b/changes/bug22702 @@ -0,0 +1,5 @@ + o Major bugfixes (directory protocol): + - Ensure that we sent "304 Not modified" as HTTP status code when a + client is attempting to fetch a consensus or consensus diff that + matches the latest consensus we have available. Fixes bug 22702; + bugfix on 0.3.1.1-alpha. diff --git a/changes/bug22719 b/changes/bug22719 new file mode 100644 index 0000000000..bfcda0a4e1 --- /dev/null +++ b/changes/bug22719 @@ -0,0 +1,7 @@ + o Minor bugfixes (compression): + - When spooling compressed data to an output buffer, don't try to + spool more data when there is no more data to spool and we are + not trying to flush the input. Previously, we would sometimes + launch compression requests with nothing to do, which interferes + with our 22672 checks. Fixes bug 22719; bugfix on 0.2.0.16-alpha. + diff --git a/changes/bug22737 b/changes/bug22737 new file mode 100644 index 0000000000..f0de8e6c41 --- /dev/null +++ b/changes/bug22737 @@ -0,0 +1,12 @@ + o Minor bugfixes (defensive programming, undefined behavior): + + - Fix a memset() off the end of an array when packing cells. This + bug should be harmless in practice, since the corrupted bytes + are still in the same structure, and are always padding bytes, + ignored, or immediately overwritten, depending on compiler + behavior. Nevertheless, because the memset()'s purpose is to + make sure that any other cell-handling bugs can't expose bytes + to the network, we need to fix it. Fixes bug 22737; bugfix on + 0.2.4.11-alpha. Fixes CID 1401591. + + diff --git a/changes/bug22746 b/changes/bug22746 new file mode 100644 index 0000000000..b036460c30 --- /dev/null +++ b/changes/bug22746 @@ -0,0 +1,4 @@ + o Minor bugfixes (crypto): + - Properly detect and refuse to blind bad ed25519 keys. The key blinding + code is currently unused, so this bug does not affect tor clients or + services on the network. Fixes bug 22746; bugfix on 0.2.6.1-alpha. diff --git a/changes/bug22751 b/changes/bug22751 new file mode 100644 index 0000000000..714525c8af --- /dev/null +++ b/changes/bug22751 @@ -0,0 +1,5 @@ + o Major bugfixes (compression): + - Fix crash in LZMA module, when the Sandbox is enabled, where + liblzma would allocate more than 16 MB of memory. We solve this + by bumping the mprotect() limit in the Sandbox module from 16 MB + to 20 MB. Fixes bug 22751; bugfix on 0.3.1.1-alpha. diff --git a/changes/bug22753 b/changes/bug22753 new file mode 100644 index 0000000000..32a6dfa56c --- /dev/null +++ b/changes/bug22753 @@ -0,0 +1,7 @@ + o Major bugfixes (path selection, security): + - When choosing which guard to use for a circuit, avoid the + exit's family along with the exit itself. Previously, the new + guard selection logic avoided the exit, but did not consider + its family. Fixes bug 22753; bugfix on 0.3.0.1-alpha. Tracked + as TROVE-2016-006 and CVE-2017-0377. + diff --git a/changes/diagnose_22752 b/changes/diagnose_22752 new file mode 100644 index 0000000000..b5bda05ec0 --- /dev/null +++ b/changes/diagnose_22752 @@ -0,0 +1,4 @@ + o Minor features (bug mitigation, diagnostics, logging): + - Avoid an assertion failure, and log a better error message, + when unable to remove a file from the consensus cache on + Windows. Attempts to mitigate and diagnose bug 22752. diff --git a/changes/ticket22684 b/changes/ticket22684 new file mode 100644 index 0000000000..f1d9d21abb --- /dev/null +++ b/changes/ticket22684 @@ -0,0 +1,5 @@ + o Minor features (control): + - Add GETINFO desc/download-enabled and md/download-enabled, to + inform the controller whether try to download router descriptors + and microdescriptors respectively. Closes ticket 22684. + diff --git a/scripts/codegen/fuzzing_include_am.py b/scripts/codegen/fuzzing_include_am.py index 91e9e5f3f6..6e45c21926 100755 --- a/scripts/codegen/fuzzing_include_am.py +++ b/scripts/codegen/fuzzing_include_am.py @@ -1,4 +1,4 @@ - +#!/usr/bin/python FUZZERS = """ consensus @@ -33,7 +33,10 @@ FUZZING_LIBS = \ @TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ \ @TOR_LIBEVENT_LIBS@ \ @TOR_OPENSSL_LIBS@ @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@ \ - @TOR_SYSTEMD_LIBS@ + @TOR_SYSTEMD_LIBS@ \ + @TOR_LZMA_LIBS@ \ + @TOR_ZSTD_LIBS@ \ + $(rust_ldadd) oss-fuzz-prereqs: \ src/or/libtor-testing.a \ @@ -48,7 +51,7 @@ oss-fuzz-prereqs: \ noinst_HEADERS += \ src/test/fuzz/fuzzing.h -LIBFUZZER = /home/nickm/build/libfuzz/libFuzzer.a +LIBFUZZER = -lFuzzer LIBFUZZER_CPPFLAGS = $(FUZZING_CPPFLAGS) -DLLVM_FUZZ LIBFUZZER_CFLAGS = $(FUZZING_CFLAGS) LIBFUZZER_LDFLAG = $(FUZZING_LDFLAG) diff --git a/src/common/crypto_ed25519.c b/src/common/crypto_ed25519.c index 188e18c710..d61549b797 100644 --- a/src/common/crypto_ed25519.c +++ b/src/common/crypto_ed25519.c @@ -28,6 +28,7 @@ #include "crypto_format.h" #include "torlog.h" #include "util.h" +#include "util_format.h" #include "ed25519/ref10/ed25519_ref10.h" #include "ed25519/donna/ed25519_donna_tor.h" @@ -57,6 +58,9 @@ typedef struct { int (*pubkey_from_curve25519_pubkey)(unsigned char *, const unsigned char *, int); + + int (*ed25519_scalarmult_with_group_order)(unsigned char *, + const unsigned char *); } ed25519_impl_t; /** The Ref10 Ed25519 implementation. This one is pure C and lightly @@ -77,6 +81,7 @@ static const ed25519_impl_t impl_ref10 = { ed25519_ref10_blind_public_key, ed25519_ref10_pubkey_from_curve25519_pubkey, + ed25519_ref10_scalarmult_with_group_order, }; /** The Ref10 Ed25519 implementation. This one is heavily optimized, but still @@ -97,6 +102,7 @@ static const ed25519_impl_t impl_donna = { ed25519_donna_blind_public_key, ed25519_donna_pubkey_from_curve25519_pubkey, + ed25519_donna_scalarmult_with_group_order, }; /** Which Ed25519 implementation are we using? NULL if we haven't decided @@ -491,7 +497,8 @@ ed25519_public_key_from_curve25519_public_key(ed25519_public_key_t *pubkey, * service descriptors are encrypted with a key derived from the service's * long-term public key, and then signed with (and stored at a position * indexed by) a short-term key derived by blinding the long-term keys. - */ + * + * Return 0 if blinding was successful, else return -1. */ int ed25519_keypair_blind(ed25519_keypair_t *out, const ed25519_keypair_t *inp, @@ -502,7 +509,9 @@ ed25519_keypair_blind(ed25519_keypair_t *out, get_ed_impl()->blind_secret_key(out->seckey.seckey, inp->seckey.seckey, param); - ed25519_public_blind(&pubkey_check, &inp->pubkey, param); + if (ed25519_public_blind(&pubkey_check, &inp->pubkey, param) < 0) { + return -1; + } ed25519_public_key_generate(&out->pubkey, &out->seckey); tor_assert(fast_memeq(pubkey_check.pubkey, out->pubkey.pubkey, 32)); @@ -522,8 +531,7 @@ ed25519_public_blind(ed25519_public_key_t *out, const ed25519_public_key_t *inp, const uint8_t *param) { - get_ed_impl()->blind_public_key(out->pubkey, inp->pubkey, param); - return 0; + return get_ed_impl()->blind_public_key(out->pubkey, inp->pubkey, param); } /** @@ -754,3 +762,47 @@ ed25519_init(void) pick_ed25519_impl(); } +/* Return true if <b>point</b> is the identity element of the ed25519 group. */ +static int +ed25519_point_is_identity_element(const uint8_t *point) +{ + /* The identity element in ed25159 is the point with coordinates (0,1). */ + static const uint8_t ed25519_identity[32] = { + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; + tor_assert(sizeof(ed25519_identity) == ED25519_PUBKEY_LEN); + return tor_memeq(point, ed25519_identity, sizeof(ed25519_identity)); +} + +/** Validate <b>pubkey</b> to ensure that it has no torsion component. + * Return 0 if <b>pubkey</b> is valid, else return -1. */ +int +ed25519_validate_pubkey(const ed25519_public_key_t *pubkey) +{ + uint8_t result[32] = {9}; + + /* First check that we were not given the identity element */ + if (ed25519_point_is_identity_element(pubkey->pubkey)) { + log_warn(LD_CRYPTO, "ed25519 pubkey is the identity"); + return -1; + } + + /* For any point on the curve, doing l*point should give the identity element + * (where l is the group order). Do the computation and check that the + * identity element is returned. */ + if (get_ed_impl()->ed25519_scalarmult_with_group_order(result, + pubkey->pubkey) < 0) { + log_warn(LD_CRYPTO, "ed25519 group order scalarmult failed"); + return -1; + } + + if (!ed25519_point_is_identity_element(result)) { + log_warn(LD_CRYPTO, "ed25519 validation failed"); + return -1; + } + + return 0; +} + diff --git a/src/common/crypto_ed25519.h b/src/common/crypto_ed25519.h index 77a3313adc..3a439207b3 100644 --- a/src/common/crypto_ed25519.h +++ b/src/common/crypto_ed25519.h @@ -127,6 +127,8 @@ void ed25519_pubkey_copy(ed25519_public_key_t *dest, void ed25519_set_impl_params(int use_donna); void ed25519_init(void); +int ed25519_validate_pubkey(const ed25519_public_key_t *pubkey); + #ifdef TOR_UNIT_TESTS void crypto_ed25519_testing_force_impl(const char *name); void crypto_ed25519_testing_restore_impl(void); diff --git a/src/common/sandbox.c b/src/common/sandbox.c index aae0705af4..52caa4fcc6 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -19,8 +19,14 @@ #define _LARGEFILE64_SOURCE #endif -/** Malloc mprotect limit in bytes. */ -#define MALLOC_MP_LIM (16*1024*1024) +/** Malloc mprotect limit in bytes. + * + * 28/06/2017: This value was increased from 16 MB to 20 MB after we introduced + * LZMA support in Tor (0.3.1.1-alpha). We limit our LZMA coder to 16 MB, but + * liblzma have a small overhead that we need to compensate for to avoid being + * killed by the sandbox. + */ +#define MALLOC_MP_LIM (20*1024*1024) #include <stdio.h> #include <string.h> diff --git a/src/common/storagedir.c b/src/common/storagedir.c index 4405731884..31933f64c2 100644 --- a/src/common/storagedir.c +++ b/src/common/storagedir.c @@ -119,7 +119,8 @@ storage_dir_clean_tmpfiles(storage_dir_t *d) char *path = NULL; tor_asprintf(&path, "%s/%s", d->directory, fname); if (unlink(sandbox_intern_string(path))) { - log_warn(LD_FS, "Unable to unlink %s", escaped(path)); + log_warn(LD_FS, "Unable to unlink %s while cleaning " + "temporary files: %s", escaped(path), strerror(errno)); tor_free(path); continue; } @@ -455,7 +456,8 @@ storage_dir_remove_file(storage_dir_t *d, if (unlink(ipath) == 0) { storage_dir_reduce_usage(d, size); } else { - log_warn(LD_FS, "Unable to unlink %s", escaped(path)); + log_warn(LD_FS, "Unable to unlink %s while removing file: %s", + escaped(path), strerror(errno)); tor_free(path); return; } diff --git a/src/ext/ed25519/donna/ed25519_donna_tor.h b/src/ext/ed25519/donna/ed25519_donna_tor.h index d225407b1c..7d7b8c0625 100644 --- a/src/ext/ed25519/donna/ed25519_donna_tor.h +++ b/src/ext/ed25519/donna/ed25519_donna_tor.h @@ -30,4 +30,9 @@ int ed25519_donna_blind_public_key(unsigned char *out, const unsigned char *inp, int ed25519_donna_pubkey_from_curve25519_pubkey(unsigned char *out, const unsigned char *inp, int signbit); + +int +ed25519_donna_scalarmult_with_group_order(unsigned char *out, + const unsigned char *pubkey); + #endif diff --git a/src/ext/ed25519/donna/ed25519_tor.c b/src/ext/ed25519/donna/ed25519_tor.c index 9537ae66a1..6bc22675ae 100644 --- a/src/ext/ed25519/donna/ed25519_tor.c +++ b/src/ext/ed25519/donna/ed25519_tor.c @@ -304,7 +304,9 @@ ed25519_donna_blind_public_key(unsigned char *out, const unsigned char *inp, /* No "ge25519_unpack", negate the public key. */ memcpy(pkcopy, inp, 32); pkcopy[31] ^= (1<<7); - ge25519_unpack_negative_vartime(&A, pkcopy); + if (!ge25519_unpack_negative_vartime(&A, pkcopy)) { + return -1; + } /* A' = [tweak] * A + [0] * basepoint. */ ge25519_double_scalarmult_vartime(&Aprime, &A, t, zero); @@ -340,5 +342,32 @@ ed25519_donna_pubkey_from_curve25519_pubkey(unsigned char *out, return 0; } +/* Do the scalar multiplication of <b>pubkey</b> with the group order + * <b>modm_m</b>. Place the result in <b>out</b> which must be at least 32 + * bytes long. */ +int +ed25519_donna_scalarmult_with_group_order(unsigned char *out, + const unsigned char *pubkey) +{ + static const bignum256modm ALIGN(16) zero = { 0 }; + unsigned char pkcopy[32]; + ge25519 ALIGN(16) Point, Result; + + /* No "ge25519_unpack", negate the public key and unpack it back. + * See ed25519_donna_blind_public_key() */ + memcpy(pkcopy, pubkey, 32); + pkcopy[31] ^= (1<<7); + if (!ge25519_unpack_negative_vartime(&Point, pkcopy)) { + return -1; /* error: bail out */ + } + + /* There is no regular scalarmult function so we have to do: + * Result = l*P + 0*B */ + ge25519_double_scalarmult_vartime(&Result, &Point, modm_m, zero); + ge25519_pack(out, &Result); + + return 0; +} + #include "test-internals.c" diff --git a/src/ext/ed25519/ref10/blinding.c b/src/ext/ed25519/ref10/blinding.c index ee3e8666fa..31332a2719 100644 --- a/src/ext/ed25519/ref10/blinding.c +++ b/src/ext/ed25519/ref10/blinding.c @@ -49,6 +49,7 @@ int ed25519_ref10_blind_public_key(unsigned char *out, unsigned char pkcopy[32]; ge_p3 A; ge_p2 Aprime; + int retval = -1; ed25519_ref10_gettweak(tweak, param); @@ -62,15 +63,57 @@ int ed25519_ref10_blind_public_key(unsigned char *out, * "ge_frombytes", we'd use that, but there isn't. */ memcpy(pkcopy, inp, 32); pkcopy[31] ^= (1<<7); - ge_frombytes_negate_vartime(&A, pkcopy); + if (ge_frombytes_negate_vartime(&A, pkcopy) != 0) { + goto done; + } /* There isn't a regular ge_scalarmult -- we have to do tweak*A + zero*B. */ ge_double_scalarmult_vartime(&Aprime, tweak, &A, zero); ge_tobytes(out, &Aprime); + retval = 0; + + done: memwipe(tweak, 0, sizeof(tweak)); memwipe(&A, 0, sizeof(A)); memwipe(&Aprime, 0, sizeof(Aprime)); memwipe(pkcopy, 0, sizeof(pkcopy)); + return retval; +} + +/* This is the group order encoded in a format that + * ge_double_scalarmult_vartime() understands. The group order m is: + * m = 2^252 + 27742317777372353535851937790883648493 = + * 0x1000000000000000000000000000000014def9dea2f79cd65812631a5cf5d3ed + */ +static const uint8_t modm_m[32] = {0xed,0xd3,0xf5,0x5c,0x1a,0x63,0x12,0x58, + 0xd6,0x9c,0xf7,0xa2,0xde,0xf9,0xde,0x14, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x10}; + +/* Do the scalar multiplication of <b>pubkey</b> with the group order + * <b>modm_m</b>. Place the result in <b>out</b> which must be at least 32 + * bytes long. */ +int +ed25519_ref10_scalarmult_with_group_order(unsigned char *out, + const unsigned char *pubkey) +{ + unsigned char pkcopy[32]; + unsigned char zero[32] = {0}; + ge_p3 Point; + ge_p2 Result; + + /* All this is done to fit 'pubkey' in 'Point' so that it can be used by + * ed25519 ref code. Same thing as in blinding function */ + memcpy(pkcopy, pubkey, 32); + pkcopy[31] ^= (1<<7); + if (ge_frombytes_negate_vartime(&Point, pkcopy) != 0) { + return -1; /* error: bail out */ + } + + /* There isn't a regular scalarmult -- we have to do r = l*P + 0*B */ + ge_double_scalarmult_vartime(&Result, modm_m, &Point, zero); + ge_tobytes(out, &Result); + return 0; } diff --git a/src/ext/ed25519/ref10/ed25519_ref10.h b/src/ext/ed25519/ref10/ed25519_ref10.h index af7e21a2ad..5965694977 100644 --- a/src/ext/ed25519/ref10/ed25519_ref10.h +++ b/src/ext/ed25519/ref10/ed25519_ref10.h @@ -27,4 +27,8 @@ int ed25519_ref10_blind_public_key(unsigned char *out, const unsigned char *inp, const unsigned char *param); +int +ed25519_ref10_scalarmult_with_group_order(unsigned char *out, + const unsigned char *pubkey); + #endif diff --git a/src/or/buffers.c b/src/or/buffers.c index 1907d69cfb..d0639b81eb 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -2084,7 +2084,7 @@ fetch_from_buf_line(buf_t *buf, char *data_out, size_t *data_len) int write_to_buf_compress(buf_t *buf, tor_compress_state_t *state, const char *data, size_t data_len, - int done) + const int done) { char *next; size_t old_avail, avail; @@ -2106,8 +2106,10 @@ write_to_buf_compress(buf_t *buf, tor_compress_state_t *state, case TOR_COMPRESS_ERROR: return -1; case TOR_COMPRESS_OK: - if (data_len == 0) + if (data_len == 0) { + tor_assert_nonfatal(!done); over = 1; + } break; case TOR_COMPRESS_BUFFER_FULL: if (avail) { @@ -2116,6 +2118,11 @@ write_to_buf_compress(buf_t *buf, tor_compress_state_t *state, * whether were going to or not. */ need_new_chunk = 1; } + if (data_len == 0 && !done) { + /* We've consumed all the input data, though, so there's no + * point in forging ahead right now. */ + over = 1; + } break; } buf->datalen += old_avail - avail; diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 707dd5ba8e..6547451181 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1915,7 +1915,6 @@ certs_cell_typenum_to_cert_type(int typenum) * of the connection, we then authenticate the server or mark the connection. * If it's the server side, wait for an AUTHENTICATE cell. */ - STATIC void channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) { diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 240c64b6d1..833c0b98b7 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2581,7 +2581,7 @@ extend_info_from_node(const node_t *node, int for_direct_connect) ed_pubkey = node_get_ed25519_id(node); } else if (node_get_ed25519_id(node)) { log_info(LD_CIRC, "Not including the ed25519 ID for %s, since it won't " - " be able to authenticate it.", + "be able to authenticate it.", node_describe(node)); } diff --git a/src/or/connection_or.c b/src/or/connection_or.c index ab0f411cc5..753148291c 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -420,9 +420,11 @@ cell_pack(packed_cell_t *dst, const cell_t *src, int wide_circ_ids) set_uint32(dest, htonl(src->circ_id)); dest += 4; } else { + /* Clear the last two bytes of dest, in case we can accidentally + * send them to the network somehow. */ + memset(dest+CELL_MAX_NETWORK_SIZE-2, 0, 2); set_uint16(dest, htons(src->circ_id)); dest += 2; - memset(dest+CELL_MAX_NETWORK_SIZE-2, 0, 2); /*make sure it's clear */ } set_uint8(dest, src->command); memcpy(dest+1, src->payload, CELL_PAYLOAD_SIZE); diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 2af104733b..638fcd6794 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -325,7 +325,8 @@ cdm_diff_ht_purge(consensus_flavor_t flav, if ((*diff)->cdm_diff_status == CDM_DIFF_PRESENT && flav == (*diff)->flavor) { - if (consensus_cache_entry_handle_get((*diff)->entry) == NULL) { + if (BUG((*diff)->entry == NULL) || + consensus_cache_entry_handle_get((*diff)->entry) == NULL) { /* the underlying entry has gone away; drop this. */ next = HT_NEXT_RMV(cdm_diff_ht, &cdm_diff_ht, diff); cdm_diff_free(this); @@ -622,6 +623,9 @@ consdiffmgr_find_diff_from(consensus_cache_entry_t **entry_out, return CONSDIFF_IN_PROGRESS; } + if (BUG(ent->entry == NULL)) { + return CONSDIFF_NOT_FOUND; + } *entry_out = consensus_cache_entry_handle_get(ent->entry); return (*entry_out) ? CONSDIFF_AVAILABLE : CONSDIFF_NOT_FOUND; @@ -1840,3 +1844,21 @@ consensus_cache_entry_get_valid_until(const consensus_cache_entry_t *ent, return 0; } +/** Read the valid after timestamp from the cached object <b>ent</b> into + * *<b>out</b> and return 0, or return -1 if no such time was recorded. */ +int +consensus_cache_entry_get_valid_after(const consensus_cache_entry_t *ent, + time_t *out) +{ + tor_assert(ent); + tor_assert(out); + + const char *s; + s = consensus_cache_entry_get_value(ent, LABEL_VALID_AFTER); + + if (s == NULL || parse_iso_time_nospace(s, out) < 0) + return -1; + else + return 0; +} + diff --git a/src/or/consdiffmgr.h b/src/or/consdiffmgr.h index fe4f9ee239..079f9fe2d2 100644 --- a/src/or/consdiffmgr.h +++ b/src/or/consdiffmgr.h @@ -44,6 +44,9 @@ int consensus_cache_entry_get_fresh_until( int consensus_cache_entry_get_valid_until( const struct consensus_cache_entry_t *ent, time_t *out); +int consensus_cache_entry_get_valid_after( + const struct consensus_cache_entry_t *ent, + time_t *out); void consdiffmgr_rescan(void); int consdiffmgr_cleanup(void); diff --git a/src/or/control.c b/src/or/control.c index 232f7b9c2c..4300dae9a3 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -803,7 +803,7 @@ queued_events_flush_all(int force) } /** Libevent callback: Flushes pending events to controllers that are - * interested in them */ + * interested in them. */ static void flush_queued_events_cb(evutil_socket_t fd, short what, void *arg) { @@ -1919,6 +1919,9 @@ getinfo_helper_dir(control_connection_t *control_conn, "instead of desc/name/*."; return 0; } + } else if (!strcmp(question, "desc/download-enabled")) { + int r = we_fetch_router_descriptors(get_options()); + tor_asprintf(answer, "%d", !!r); } else if (!strcmp(question, "desc/all-recent")) { routerlist_t *routerlist = router_get_routerlist(); smartlist_t *sl = smartlist_new(); @@ -2004,6 +2007,9 @@ getinfo_helper_dir(control_connection_t *control_conn, if (md && md->body) { *answer = tor_strndup(md->body, md->bodylen); } + } else if (!strcmp(question, "md/download-enabled")) { + int r = we_fetch_microdescriptors(get_options()); + tor_asprintf(answer, "%d", !!r); } else if (!strcmpstart(question, "desc-annotations/id/")) { const routerinfo_t *ri = NULL; const node_t *node = @@ -3026,9 +3032,13 @@ static const getinfo_item_t getinfo_items[] = { PREFIX("desc/name/", dir, "Router descriptors by nickname."), ITEM("desc/all-recent", dir, "All non-expired, non-superseded router descriptors."), + ITEM("desc/download-enabled", dir, + "Do we try to download router descriptors?"), ITEM("desc/all-recent-extrainfo-hack", dir, NULL), /* Hack. */ PREFIX("md/id/", dir, "Microdescriptors by ID"), PREFIX("md/name/", dir, "Microdescriptors by name"), + ITEM("md/download-enabled", dir, + "Do we try to download microdescriptors?"), PREFIX("extra-info/digest/", dir, "Extra-info documents by digest."), PREFIX("hs/client/desc/id", dir, "Hidden Service descriptor in client's cache by onion."), diff --git a/src/or/directory.c b/src/or/directory.c index 5e8e6638b6..69479ad745 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2191,6 +2191,123 @@ static int handle_response_fetch_renddesc_v2(dir_connection_t *, static int handle_response_upload_renddesc_v2(dir_connection_t *, const response_handler_args_t *); +static int +dir_client_decompress_response_body(char **bodyp, size_t *bodylenp, + dir_connection_t *conn, + compress_method_t compression, + int anonymized_connection) +{ + int rv = 0; + const char *body = *bodyp; + size_t body_len = *bodylenp; + int allow_partial = (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || + conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO || + conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC); + + int plausible = body_is_plausible(body, body_len, conn->base_.purpose); + + if (plausible && compression == NO_METHOD) { + return 0; + } + + int severity = LOG_DEBUG; + char *new_body = NULL; + size_t new_len = 0; + const char *description1, *description2; + int want_to_try_both = 0; + int tried_both = 0; + compress_method_t guessed = detect_compression_method(body, body_len); + + description1 = compression_method_get_human_name(compression); + + if (BUG(description1 == NULL)) + description1 = compression_method_get_human_name(UNKNOWN_METHOD); + + if (guessed == UNKNOWN_METHOD && !plausible) + description2 = "confusing binary junk"; + else + description2 = compression_method_get_human_name(guessed); + + /* Tell the user if we don't believe what we're told about compression.*/ + want_to_try_both = (compression == UNKNOWN_METHOD || + guessed != compression); + if (want_to_try_both) { + severity = LOG_PROTOCOL_WARN; + } + + tor_log(severity, LD_HTTP, + "HTTP body from server '%s:%d' was labeled as %s, " + "%s it seems to be %s.%s", + conn->base_.address, conn->base_.port, description1, + guessed != compression?"but":"and", + description2, + (compression>0 && guessed>0 && want_to_try_both)? + " Trying both.":""); + + /* Try declared compression first if we can. + * tor_compress_supports_method() also returns true for NO_METHOD. + * Ensure that the server is not sending us data compressed using a + * compression method that is not allowed for anonymous connections. */ + if (anonymized_connection && + ! allowed_anonymous_connection_compression_method(compression)) { + warn_disallowed_anonymous_compression_method(compression); + rv = -1; + goto done; + } + + if (tor_compress_supports_method(compression)) { + tor_uncompress(&new_body, &new_len, body, body_len, compression, + !allow_partial, LOG_PROTOCOL_WARN); + if (new_body) { + /* We succeeded with the declared compression method. Great! */ + rv = 0; + goto done; + } + } + + /* Okay, if that didn't work, and we think that it was compressed + * differently, try that. */ + if (anonymized_connection && + ! allowed_anonymous_connection_compression_method(guessed)) { + warn_disallowed_anonymous_compression_method(guessed); + rv = -1; + goto done; + } + + if (tor_compress_supports_method(guessed) && + compression != guessed) { + tor_uncompress(&new_body, &new_len, body, body_len, guessed, + !allow_partial, LOG_INFO); + tried_both = 1; + } + /* If we're pretty sure that we have a compressed directory, and + * we didn't manage to uncompress it, then warn and bail. */ + if (!plausible && !new_body) { + log_fn(LOG_PROTOCOL_WARN, LD_HTTP, + "Unable to decompress HTTP body (tried %s%s%s, server '%s:%d').", + description1, + tried_both?" and ":"", + tried_both?description2:"", + conn->base_.address, conn->base_.port); + rv = -1; + goto done; + } + + done: + if (new_body) { + if (rv == 0) { + /* success! */ + tor_free(*bodyp); + *bodyp = new_body; + *bodylenp = new_len; + } else { + tor_free(new_body); + } + } + + return rv; +} + /** We are a client, and we've finished reading the server's * response. Parse it and act appropriately. * @@ -2211,7 +2328,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) time_t date_header = 0; long apparent_skew; compress_method_t compression; - int plausible; int skewed = 0; int rv; int allow_partial = (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || @@ -2325,89 +2441,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn) goto done; } - plausible = body_is_plausible(body, body_len, conn->base_.purpose); - if (compression != NO_METHOD || !plausible) { - int severity = LOG_DEBUG; - char *new_body = NULL; - size_t new_len = 0; - const char *description1, *description2; - int want_to_try_both = 0; - int tried_both = 0; - compress_method_t guessed = detect_compression_method(body, body_len); - - description1 = compression_method_get_human_name(compression); - - if (BUG(description1 == NULL)) - description1 = compression_method_get_human_name(UNKNOWN_METHOD); - - if (guessed == UNKNOWN_METHOD && !plausible) - description2 = "confusing binary junk"; - else - description2 = compression_method_get_human_name(guessed); - - /* Tell the user if we don't believe what we're told about compression.*/ - want_to_try_both = (compression == UNKNOWN_METHOD || - guessed != compression); - if (want_to_try_both) { - severity = LOG_INFO; - } - - tor_log(severity, LD_HTTP, - "HTTP body from server '%s:%d' was labeled as %s, " - "%s it seems to be %s.%s", - conn->base_.address, conn->base_.port, description1, - guessed != compression?"but":"and", - description2, - (compression>0 && guessed>0 && want_to_try_both)? - " Trying both.":""); - - /* Try declared compression first if we can. - * tor_compress_supports_method() also returns true for NO_METHOD. - * Ensure that the server is not sending us data compressed using a - * compression method that is not allowed for anonymous connections. */ - if (anonymized_connection && - ! allowed_anonymous_connection_compression_method(compression)) { - warn_disallowed_anonymous_compression_method(compression); - rv = -1; - goto done; - } - - if (tor_compress_supports_method(compression)) - tor_uncompress(&new_body, &new_len, body, body_len, compression, - !allow_partial, LOG_PROTOCOL_WARN); - - /* Okay, if that didn't work, and we think that it was compressed - * differently, try that. */ - if (anonymized_connection && - ! allowed_anonymous_connection_compression_method(guessed)) { - warn_disallowed_anonymous_compression_method(guessed); - rv = -1; - goto done; - } - - if (!new_body && tor_compress_supports_method(guessed) && - compression != guessed) { - tor_uncompress(&new_body, &new_len, body, body_len, guessed, - !allow_partial, LOG_PROTOCOL_WARN); - tried_both = 1; - } - /* If we're pretty sure that we have a compressed directory, and - * we didn't manage to uncompress it, then warn and bail. */ - if (!plausible && !new_body) { - log_fn(LOG_PROTOCOL_WARN, LD_HTTP, - "Unable to decompress HTTP body (tried %s%s%s, server '%s:%d').", - description1, - tried_both?" and ":"", - tried_both?description2:"", - conn->base_.address, conn->base_.port); - rv = -1; - goto done; - } - if (new_body) { - tor_free(body); - body = new_body; - body_len = new_len; - } + if (dir_client_decompress_response_body(&body, &body_len, + conn, compression, anonymized_connection) < 0) { + rv = -1; + goto done; } response_handler_args_t args; @@ -3883,6 +3920,30 @@ find_best_compression_method(unsigned compression_methods, int stream) return NO_METHOD; } +/** Check if any of the digests in <b>digests</b> matches the latest consensus + * flavor (given in <b>flavor</b>) that we have available. */ +static int +digest_list_contains_best_consensus(consensus_flavor_t flavor, + const smartlist_t *digests) +{ + const networkstatus_t *ns = NULL; + + if (digests == NULL) + return 0; + + ns = networkstatus_get_latest_consensus_by_flavor(flavor); + + if (ns == NULL) + return 0; + + SMARTLIST_FOREACH_BEGIN(digests, const uint8_t *, digest) { + if (tor_memeq(ns->digest_sha3_as_signed, digest, DIGEST256_LEN)) + return 1; + } SMARTLIST_FOREACH_END(digest); + + return 0; +} + /** Check if the given compression method is allowed for a connection that is * supposed to be anonymous. Returns 1 if the compression method is allowed, * otherwise 0. */ @@ -4052,6 +4113,13 @@ handle_get_current_consensus(dir_connection_t *conn, goto done; } + if (digest_list_contains_best_consensus(req.flav, + req.diff_from_digests)) { + write_http_status_line(conn, 304, "Not modified"); + geoip_note_ns_response(GEOIP_REJECT_NOT_MODIFIED); + goto done; + } + struct consensus_cache_entry_t *cached_consensus = NULL; compress_method_t compression_used = NO_METHOD; diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 408f58b22b..75a245e07a 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -14,6 +14,7 @@ #include "connection.h" #include "connection_or.h" #include "conscache.h" +#include "consdiffmgr.h" #include "control.h" #include "directory.h" #include "dirserv.h" @@ -703,10 +704,22 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source) /* Do keypinning again ... this time, to add the pin if appropriate */ int keypin_status; if (ri->cache_info.signing_key_cert) { + ed25519_public_key_t *pkey = &ri->cache_info.signing_key_cert->signing_key; + /* First let's validate this pubkey before pinning it */ + if (ed25519_validate_pubkey(pkey) < 0) { + log_warn(LD_DIRSERV, "Received bad key from %s (source %s)", + router_describe(ri), source); + control_event_or_authdir_new_descriptor("REJECTED", + ri->cache_info.signed_descriptor_body, + desclen, *msg); + routerinfo_free(ri); + return ROUTER_AUTHDIR_REJECTS; + } + + /* Now pin it! */ keypin_status = keypin_check_and_add( (const uint8_t*)ri->cache_info.identity_digest, - ri->cache_info.signing_key_cert->signing_key.pubkey, - ! key_pinning); + pkey->pubkey, ! key_pinning); } else { keypin_status = keypin_check_lone_rsa( (const uint8_t*)ri->cache_info.identity_digest); @@ -3518,6 +3531,11 @@ spooled_resource_estimate_size(const spooled_resource_t *spooled, } else { cached_dir_t *cached; if (spooled->consensus_cache_entry) { + if (published_out) { + consensus_cache_entry_get_valid_after( + spooled->consensus_cache_entry, published_out); + } + return spooled->cce_len; } if (spooled->cached_dir_ref) { diff --git a/src/or/dns.c b/src/or/dns.c index 722c5925d8..cc062e30ef 100644 --- a/src/or/dns.c +++ b/src/or/dns.c @@ -1940,7 +1940,7 @@ dns_launch_wildcard_checks(void) launch_wildcard_check(8, 16, ipv6, ".com"); launch_wildcard_check(8, 16, ipv6, ".org"); launch_wildcard_check(8, 16, ipv6, ".net"); - } + } } } diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index be9f85a89f..739ec82484 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -813,7 +813,7 @@ STATIC entry_guard_t * entry_guard_add_to_sample(guard_selection_t *gs, const node_t *node) { - log_info(LD_GUARD, "Adding %s as to the entry guard sample set.", + log_info(LD_GUARD, "Adding %s to the entry guard sample set.", node_describe(node)); /* make sure that the guard is not already sampled. */ @@ -1428,6 +1428,38 @@ entry_guard_passes_filter(const or_options_t *options, guard_selection_t *gs, } } +/** Return true iff <b>guard</b> is in the same family as <b>node</b>. + */ +static int +guard_in_node_family(const entry_guard_t *guard, const node_t *node) +{ + const node_t *guard_node = node_get_by_id(guard->identity); + if (guard_node) { + return nodes_in_same_family(guard_node, node); + } else { + /* If we don't have a node_t for the guard node, we might have + * a bridge_info_t for it. So let's check to see whether the bridge + * address matches has any family issues. + * + * (Strictly speaking, I believe this check is unnecessary, since we only + * use it to avoid the exit's family when building circuits, and we don't + * build multihop circuits until we have a routerinfo_t for the + * bridge... at which point, we'll also have a node_t for the + * bridge. Nonetheless, it seems wise to include it, in case our + * assumptions change down the road. -nickm.) + */ + if (get_options()->EnforceDistinctSubnets && guard->bridge_addr) { + tor_addr_t node_addr; + node_get_addr(node, &node_addr); + if (addrs_in_same_network_family(&node_addr, + &guard->bridge_addr->addr)) { + return 1; + } + } + return 0; + } +} + /** * Return true iff <b>guard</b> obeys the restrictions defined in <b>rst</b>. * (If <b>rst</b> is NULL, there are no restrictions.) @@ -1440,7 +1472,12 @@ entry_guard_obeys_restriction(const entry_guard_t *guard, if (! rst) return 1; // No restriction? No problem. - // Only one kind of restriction exists right now + // Only one kind of restriction exists right now: excluding an exit + // ID and all of its family. + const node_t *node = node_get_by_id((const char*)rst->exclude_id); + if (node && guard_in_node_family(guard, node)) + return 0; + return tor_memneq(guard->identity, rst->exclude_id, DIGEST_LEN); } diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 11b618bb50..735c7738ba 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -276,16 +276,17 @@ struct entry_guard_handle_t; * A restriction to remember which entry guards are off-limits for a given * circuit. * - * Right now, we only use restrictions to block a single guard from being - * selected; this mechanism is designed to be more extensible in the future, - * however. + * Right now, we only use restrictions to block a single guard and its family + * from being selected; this mechanism is designed to be more extensible in + * the future, however. * * Note: This mechanism is NOT for recording which guards are never to be * used: only which guards cannot be used on <em>one particular circuit</em>. */ struct entry_guard_restriction_t { /** - * The guard's RSA identity digest must not equal this. + * The guard's RSA identity digest must not equal this; and it must not + * be in the same family as any node with this digest. */ uint8_t exclude_id[DIGEST_LEN]; }; diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 2a000f5002..b55f9663bc 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -1747,18 +1747,13 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start) /* Given a descriptor string at <b>data</b>, decode all possible introduction * points that we can find. Add the introduction point object to desc_enc as we - * find them. Return 0 on success. - * - * On error, a negative value is returned. It is possible that some intro - * point object have been added to the desc_enc, they should be considered - * invalid. One single bad encoded introduction point will make this function - * return an error. */ -STATIC int + * find them. This function can't fail and it is possible that zero + * introduction points can be decoded. */ +static void decode_intro_points(const hs_descriptor_t *desc, hs_desc_encrypted_data_t *desc_enc, const char *data) { - int retval = -1; smartlist_t *chunked_desc = smartlist_new(); smartlist_t *intro_points = smartlist_new(); @@ -1799,22 +1794,19 @@ decode_intro_points(const hs_descriptor_t *desc, SMARTLIST_FOREACH_BEGIN(intro_points, const char *, intro_point) { hs_desc_intro_point_t *ip = decode_introduction_point(desc, intro_point); if (!ip) { - /* Malformed introduction point section. Stop right away, this - * descriptor shouldn't be used. */ - goto err; + /* Malformed introduction point section. We'll ignore this introduction + * point and continue parsing. New or unknown fields are possible for + * forward compatibility. */ + continue; } smartlist_add(desc_enc->intro_points, ip); } SMARTLIST_FOREACH_END(intro_point); done: - retval = 0; - - err: SMARTLIST_FOREACH(chunked_desc, char *, a, tor_free(a)); smartlist_free(chunked_desc); SMARTLIST_FOREACH(intro_points, char *, a, tor_free(a)); smartlist_free(intro_points); - return retval; } /* Return 1 iff the given base64 encoded signature in b64_sig from the encoded * descriptor in encoded_desc validates the descriptor content. */ @@ -2040,9 +2032,8 @@ desc_decode_encrypted_v3(const hs_descriptor_t *desc, /* Initialize the descriptor's introduction point list before we start * decoding. Having 0 intro point is valid. Then decode them all. */ desc_encrypted_out->intro_points = smartlist_new(); - if (decode_intro_points(desc, desc_encrypted_out, message) < 0) { - goto err; - } + decode_intro_points(desc, desc_encrypted_out, message); + /* Validation of maximum introduction points allowed. */ if (smartlist_len(desc_encrypted_out->intro_points) > MAX_INTRO_POINTS) { log_warn(LD_REND, "Service descriptor contains too many introduction " diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h index b8b94792de..58c4089795 100644 --- a/src/or/hs_descriptor.h +++ b/src/or/hs_descriptor.h @@ -223,9 +223,6 @@ STATIC smartlist_t *decode_link_specifiers(const char *encoded); STATIC hs_desc_intro_point_t *decode_introduction_point( const hs_descriptor_t *desc, const char *text); -STATIC int decode_intro_points(const hs_descriptor_t *desc, - hs_desc_encrypted_data_t *desc_enc, - const char *data); STATIC int encrypted_data_length_is_valid(size_t len); STATIC int cert_is_valid(tor_cert_t *cert, uint8_t type, const char *log_obj_type); diff --git a/src/or/microdesc.c b/src/or/microdesc.c index a4e6b409c4..18a6fbded7 100644 --- a/src/or/microdesc.c +++ b/src/or/microdesc.c @@ -876,7 +876,7 @@ update_microdesc_downloads(time_t now) smartlist_free(missing); } -/** For every microdescriptor listed in the current microdecriptor consensus, +/** For every microdescriptor listed in the current microdescriptor consensus, * update its last_listed field to be at least as recent as the publication * time of the current microdescriptor consensus. */ diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index fffd1078be..a9a6227cd6 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -76,7 +76,7 @@ static strmap_t *unnamed_server_map = NULL; * status. */ STATIC networkstatus_t *current_ns_consensus = NULL; -/** Most recently received and validated v3 "microdec"-flavored consensus +/** Most recently received and validated v3 "microdesc"-flavored consensus * network status. */ STATIC networkstatus_t *current_md_consensus = NULL; @@ -1783,7 +1783,7 @@ networkstatus_set_current_consensus(const char *consensus, if (from_cache && !was_waiting_for_certs) { /* We previously stored this; check _now_ to make sure that version-kills - * really work. This happens even before we check signatures: we did so + * really work. This happens even before we check signatures: we did so * before when we stored this to disk. This does mean an attacker who can * write to the datadir can make us not start: such an attacker could * already harm us by replacing our guards, which would be worse. */ diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 4513e97188..6ec7da798f 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -1374,7 +1374,7 @@ nodelist_refresh_countries(void) /** Return true iff router1 and router2 have similar enough network addresses * that we should treat them as being in the same family */ -static inline int +int addrs_in_same_network_family(const tor_addr_t *a1, const tor_addr_t *a2) { diff --git a/src/or/nodelist.h b/src/or/nodelist.h index f59e767d1e..405b79d820 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -95,6 +95,8 @@ int node_is_unreliable(const node_t *router, int need_uptime, int router_exit_policy_all_nodes_reject(const tor_addr_t *addr, uint16_t port, int need_uptime); void router_set_status(const char *digest, int up); +int addrs_in_same_network_family(const tor_addr_t *a1, + const tor_addr_t *a2); /** router_have_minimum_dir_info tests to see if we have enough * descriptor information to create circuits. diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 0f6113ccfc..6a03194472 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3360,8 +3360,8 @@ extract_shared_random_srvs(networkstatus_t *ns, smartlist_t *tokens) voter_identity = "consensus"; } - /* We extract both and on error, everything is stopped because it means - * the votes is malformed for the shared random value(s). */ + /* We extract both, and on error everything is stopped because it means + * the vote is malformed for the shared random value(s). */ if (extract_one_srv(tokens, K_PREVIOUS_SRV, &ns->sr_info.previous_srv) < 0) { log_warn(LD_DIR, "SR: Unable to parse previous SRV from %s", voter_identity); diff --git a/src/test/ed25519_exts_ref.py b/src/test/ed25519_exts_ref.py index af5010415e..1898256540 100644 --- a/src/test/ed25519_exts_ref.py +++ b/src/test/ed25519_exts_ref.py @@ -69,6 +69,11 @@ def signatureWithESK(m,h,pk): def newSK(): return os.urandom(32) +def random_scalar(entropy_f): # 0..L-1 inclusive + # reduce the bias to a safe level by generating 256 extra bits + oversized = int(binascii.hexlify(entropy_f(32+32)), 16) + return oversized % ell + # ------------------------------------------------------------ MSG = "This is extremely silly. But it is also incredibly serious business!" @@ -126,6 +131,31 @@ class SelfTest(unittest.TestCase): self._testSignatures(besk, bpk) + def testIdentity(self): + # Base point: + # B is the unique point (x, 4/5) \in E for which x is positive + By = 4 * inv(5) + Bx = xrecover(By) + B = [Bx % q,By % q] + + # Get identity E by doing: E = l*B, where l is the group order + identity = scalarmult(B, ell) + + # Get identity E by doing: E = l*A, where A is a random point + sk = newSK() + pk = decodepoint(publickey(sk)) + identity2 = scalarmult(pk, ell) + + # Check that identities match + assert(identity == identity2) + # Check that identity is the point (0,1) + assert(identity == [0L,1L]) + + # Check identity element: a*E = E, where a is a random scalar + scalar = random_scalar(os.urandom) + result = scalarmult(identity, scalar) + assert(result == identity == identity2) + # ------------------------------------------------------------ # From pprint.pprint([ binascii.b2a_hex(os.urandom(32)) for _ in xrange(8) ]) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index ec9d4e2709..4d9651db9c 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -2170,6 +2170,9 @@ test_crypto_ed25519_simple(void *arg) tt_int_op(0, OP_EQ, ed25519_public_key_generate(&pub1, &sec1)); tt_int_op(0, OP_EQ, ed25519_public_key_generate(&pub2, &sec1)); + tt_int_op(ed25519_validate_pubkey(&pub1), OP_EQ, 0); + tt_int_op(ed25519_validate_pubkey(&pub2), OP_EQ, 0); + tt_mem_op(pub1.pubkey, OP_EQ, pub2.pubkey, sizeof(pub1.pubkey)); tt_assert(ed25519_pubkey_eq(&pub1, &pub2)); tt_assert(ed25519_pubkey_eq(&pub1, &pub1)); @@ -2541,6 +2544,39 @@ test_crypto_ed25519_blinding(void *arg) ; } +/** Test that our blinding functions will fail if we pass them bad pubkeys */ +static void +test_crypto_ed25519_blinding_fail(void *arg) +{ + int retval; + uint8_t param[32] = {2}; + ed25519_public_key_t pub; + ed25519_public_key_t pub_blinded; + + (void)arg; + + /* This point is not on the curve: the blind routines should fail */ + const char badkey[] = + "e19c65de75c68cf3b7643ea732ba9eb1a3d20d6d57ba223c2ece1df66feb5af0"; + retval = base16_decode((char*)pub.pubkey, sizeof(pub.pubkey), + badkey, strlen(badkey)); + tt_int_op(retval, OP_EQ, sizeof(pub.pubkey)); + retval = ed25519_public_blind(&pub_blinded, &pub, param); + tt_int_op(retval, OP_EQ, -1); + + /* This point is legit: blind routines should be happy */ + const char goodkey[] = + "4ba2e44760dff4c559ef3c38768c1c14a8a54740c782c8d70803e9d6e3ad8794"; + retval = base16_decode((char*)pub.pubkey, sizeof(pub.pubkey), + goodkey, strlen(goodkey)); + tt_int_op(retval, OP_EQ, sizeof(pub.pubkey)); + retval = ed25519_public_blind(&pub_blinded, &pub, param); + tt_int_op(retval, OP_EQ, 0); + + done: + ; +} + static void test_crypto_ed25519_testvectors(void *arg) { @@ -2832,6 +2868,67 @@ crypto_rand_check_failure_mode_predict(void) #undef FAILURE_MODE_BUFFER_SIZE +/** Test that our ed25519 validation function rejects evil public keys and + * accepts good ones. */ +static void +test_crypto_ed25519_validation(void *arg) +{ + (void) arg; + + int retval; + ed25519_public_key_t pub1; + + /* See https://lists.torproject.org/pipermail/tor-dev/2017-April/012230.html + for a list of points with torsion components in ed25519. */ + + { /* Point with torsion component (order 8l) */ + const char badkey[] = + "300ef2e64e588e1df55b48e4da0416ffb64cc85d5b00af6463d5cc6c2b1c185e"; + retval = base16_decode((char*)pub1.pubkey, sizeof(pub1.pubkey), + badkey, strlen(badkey)); + tt_int_op(retval, OP_EQ, sizeof(pub1.pubkey)); + tt_int_op(ed25519_validate_pubkey(&pub1), OP_EQ, -1); + } + + { /* Point with torsion component (order 4l) */ + const char badkey[] = + "f43e3a046db8749164c6e69b193f1e942c7452e7d888736f40b98093d814d5e7"; + retval = base16_decode((char*)pub1.pubkey, sizeof(pub1.pubkey), + badkey, strlen(badkey)); + tt_int_op(retval, OP_EQ, sizeof(pub1.pubkey)); + tt_int_op(ed25519_validate_pubkey(&pub1), OP_EQ, -1); + } + + { /* Point with torsion component (order 2l) */ + const char badkey[] = + "c9fff3af0471c28e33e98c2043e44f779d0427b1e37c521a6bddc011ed1869af"; + retval = base16_decode((char*)pub1.pubkey, sizeof(pub1.pubkey), + badkey, strlen(badkey)); + tt_int_op(retval, OP_EQ, sizeof(pub1.pubkey)); + tt_int_op(ed25519_validate_pubkey(&pub1), OP_EQ, -1); + } + + { /* This point is not even on the curve */ + const char badkey[] = + "e19c65de75c68cf3b7643ea732ba9eb1a3d20d6d57ba223c2ece1df66feb5af0"; + retval = base16_decode((char*)pub1.pubkey, sizeof(pub1.pubkey), + badkey, strlen(badkey)); + tt_int_op(retval, OP_EQ, sizeof(pub1.pubkey)); + tt_int_op(ed25519_validate_pubkey(&pub1), OP_EQ, -1); + } + + { /* This one is a good key */ + const char goodkey[] = + "4ba2e44760dff4c559ef3c38768c1c14a8a54740c782c8d70803e9d6e3ad8794"; + retval = base16_decode((char*)pub1.pubkey, sizeof(pub1.pubkey), + goodkey, strlen(goodkey)); + tt_int_op(retval, OP_EQ, sizeof(pub1.pubkey)); + tt_int_op(ed25519_validate_pubkey(&pub1), OP_EQ, 0); + } + + done: ; +} + static void test_crypto_failure_modes(void *arg) { @@ -2917,7 +3014,9 @@ struct testcase_t crypto_tests[] = { ED25519_TEST(encode, 0), ED25519_TEST(convert, 0), ED25519_TEST(blinding, 0), + ED25519_TEST(blinding_fail, 0), ED25519_TEST(testvectors, 0), + ED25519_TEST(validation, 0), { "ed25519_storage", test_crypto_ed25519_storage, 0, NULL, NULL }, { "siphash", test_crypto_siphash, 0, NULL, NULL }, { "failure_modes", test_crypto_failure_modes, TT_FORK, NULL, NULL }, diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 3db7e63ee3..1f008d93b3 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -121,6 +121,8 @@ big_fake_network_setup(const struct testcase_t *testcase) n->is_running = n->is_valid = n->is_fast = n->is_stable = 1; + /* Note: all these guards have the same address, so you'll need to + * disable EnforceDistinctSubnets when a restriction is applied. */ n->rs->addr = 0x04020202; n->rs->or_port = 1234; n->rs->is_v2_dir = 1; @@ -1846,14 +1848,17 @@ test_entry_guard_select_for_circuit_confirmed(void *arg) tt_uint_op(state, OP_EQ, GUARD_CIRC_STATE_USABLE_IF_NO_BETTER_GUARD); tt_i64_op(g2->last_tried_to_connect, OP_EQ, approx_time()); - // If we say that the next confirmed guard in order is excluded, we get - // The one AFTER that. + // If we say that the next confirmed guard in order is excluded, and + // we disable EnforceDistinctSubnets, we get the guard AFTER the + // one we excluded. + get_options_mutable()->EnforceDistinctSubnets = 0; g = smartlist_get(gs->confirmed_entry_guards, smartlist_len(gs->primary_entry_guards)+2); entry_guard_restriction_t rst; memset(&rst, 0, sizeof(rst)); memcpy(rst.exclude_id, g->identity, DIGEST_LEN); g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, &rst, &state); + tt_ptr_op(g2, OP_NE, NULL); tt_ptr_op(g2, OP_NE, g); tt_int_op(g2->confirmed_idx, OP_EQ, smartlist_len(gs->primary_entry_guards)+3); @@ -1873,6 +1878,16 @@ test_entry_guard_select_for_circuit_confirmed(void *arg) tt_assert(g->is_pending); tt_int_op(g->confirmed_idx, OP_EQ, -1); + // If we EnforceDistinctSubnets and apply a restriction, we get + // nothing, since we put all of the nodes in the same /16. + // Regression test for bug 22753/TROVE-2017-006. + get_options_mutable()->EnforceDistinctSubnets = 1; + g = smartlist_get(gs->confirmed_entry_guards, 0); + memset(&rst, 0, sizeof(rst)); + memcpy(rst.exclude_id, g->identity, DIGEST_LEN); + g2 = select_entry_guard_for_circuit(gs, GUARD_USAGE_TRAFFIC, &rst, &state); + tt_ptr_op(g2, OP_EQ, NULL); + done: guard_selection_free(gs); } |