Browse Source

Merge branch '25162_zstd_static'

Nick Mathewson 6 years ago
parent
commit
a1dd8afc16
8 changed files with 167 additions and 14 deletions
  1. 6 0
      changes/ticket25162
  2. 6 0
      configure.ac
  3. 10 0
      src/common/compress.c
  4. 1 0
      src/common/compress.h
  5. 96 10
      src/common/compress_zstd.c
  6. 7 0
      src/common/compress_zstd.h
  7. 2 0
      src/or/main.c
  8. 39 4
      src/test/test_util.c

+ 6 - 0
changes/ticket25162

@@ -0,0 +1,6 @@
+  o Minor features (compression, zstd):
+    - When running with zstd, Tor now considers using advanced functions that
+      the zstd maintainers have labeled as potentially unstable. To
+      prevent breakage, Tor will only use this functionality when
+      the runtime version of the zstd library matches the version
+      with which it were compiled.  Closes ticket 25162.

+ 6 - 0
configure.ac

@@ -61,6 +61,8 @@ AC_ARG_ENABLE(cargo-online-mode,
    AS_HELP_STRING(--enable-cargo-online-mode, [Allow cargo to make network requests to fetch crates. For builds with rust only.]))
 AC_ARG_ENABLE(restart-debugging,
    AS_HELP_STRING(--enable-restart-debugging, [Build Tor with support for debugging in-process restart. Developers only.]))
+AC_ARG_ENABLE(zstd-advanced-apis,
+   AS_HELP_STRING(--disable-zstd-advanced-apis, [Build without support for zstd's "static-only" APIs.]))
 
 if test "x$enable_coverage" != "xyes" -a "x$enable_asserts_in_tests" = "xno" ; then
     AC_MSG_ERROR([Can't disable assertions outside of coverage build])
@@ -114,6 +116,10 @@ if test "$enable_restart_debugging" = "yes"; then
             [Defined if we're building with support for in-process restart debugging.])
 fi
 
+if test "$enable_zstd_advanced_apis" != "no"; then
+   AC_DEFINE(ENABLE_ZSTD_ADVANCED_APIS, 1,
+             [Defined if we're going to try to use zstd's "static-only" APIs.])
+fi
 
 # systemd support
 if test "x$enable_systemd" = "xno"; then

+ 10 - 0
src/common/compress.c

@@ -663,3 +663,13 @@ tor_compress_init(void)
   tor_zstd_init();
 }
 
+/** Warn if we had any problems while setting up our compression libraries.
+ *
+ * (This isn't part of tor_compress_init, since the logs aren't set up yet.)
+ */
+void
+tor_compress_log_init_warnings(void)
+{
+  tor_zstd_warn_if_version_mismatched();
+}
+

+ 1 - 0
src/common/compress.h

@@ -87,6 +87,7 @@ void tor_compress_free_(tor_compress_state_t *state);
 size_t tor_compress_state_size(const tor_compress_state_t *state);
 
 void tor_compress_init(void);
+void tor_compress_log_init_warnings(void);
 
 #endif /* !defined(TOR_COMPRESS_H) */
 

+ 96 - 10
src/common/compress_zstd.c

@@ -18,6 +18,13 @@
 #include "compress.h"
 #include "compress_zstd.h"
 
+#ifdef ENABLE_ZSTD_ADVANCED_APIS
+/* This is a lie, but we make sure it doesn't get us in trouble by wrapping
+ * all invocations of zstd's static-only functions in a check to make sure
+ * that the compile-time version matches the run-time version. */
+#define ZSTD_STATIC_LINKING_ONLY
+#endif
+
 #ifdef HAVE_ZSTD
 #include <zstd.h>
 #endif
@@ -51,21 +58,29 @@ tor_zstd_method_supported(void)
 #endif
 }
 
+/** Format a zstd version number as a string in <b>buf</b>. */
+static void
+tor_zstd_format_version(char *buf, size_t buflen, unsigned version_number)
+{
+  tor_snprintf(buf, buflen,
+               "%u.%u.%u",
+               version_number / 10000 % 100,
+               version_number / 100 % 100,
+               version_number % 100);
+}
+
+#define VERSION_STR_MAX_LEN 16 /* more than enough space for 99.99.99 */
+
 /** Return a string representation of the version of the currently running
  * version of libzstd. Returns NULL if Zstandard is unsupported. */
 const char *
 tor_zstd_get_version_str(void)
 {
 #ifdef HAVE_ZSTD
-  static char version_str[16];
-  size_t version_number;
+  static char version_str[VERSION_STR_MAX_LEN];
 
-  version_number = ZSTD_versionNumber();
-  tor_snprintf(version_str, sizeof(version_str),
-               "%d.%d.%d",
-               (int) version_number / 10000 % 100,
-               (int) version_number / 100 % 100,
-               (int) version_number % 100);
+  tor_zstd_format_version(version_str, sizeof(version_str),
+                          ZSTD_versionNumber());
 
   return version_str;
 #else /* !(defined(HAVE_ZSTD)) */
@@ -85,6 +100,26 @@ tor_zstd_get_header_version_str(void)
 #endif
 }
 
+#ifdef TOR_UNIT_TESTS
+static int static_apis_disable_for_testing = 0;
+#endif
+
+/** Return true iff we can use the "static-only" APIs. */
+int
+tor_zstd_can_use_static_apis(void)
+{
+#if defined(ZSTD_STATIC_LINKING_ONLY) && defined(HAVE_ZSTD)
+#ifdef TOR_UNIT_TESTS
+  if (static_apis_disable_for_testing) {
+    return 0;
+  }
+#endif
+  return (ZSTD_VERSION_NUMBER == ZSTD_versionNumber());
+#else
+  return 0;
+#endif
+}
+
 /** Internal Zstandard state for incremental compression/decompression.
  * The body of this struct is not exposed. */
 struct tor_zstd_compress_state_t {
@@ -112,9 +147,11 @@ struct tor_zstd_compress_state_t {
 
 #ifdef HAVE_ZSTD
 /** Return an approximate number of bytes stored in memory to hold the
- * Zstandard compression/decompression state. */
+ * Zstandard compression/decompression state. This is a fake estimate
+ * based on inspecting the zstd source: tor_zstd_state_size_precalc() is
+ * more accurate when it's allowed to use "static-only" functions */
 static size_t
-tor_zstd_state_size_precalc(int compress, int preset)
+tor_zstd_state_size_precalc_fake(int compress, int preset)
 {
   tor_assert(preset > 0);
 
@@ -171,6 +208,24 @@ tor_zstd_state_size_precalc(int compress, int preset)
 
   return memory_usage;
 }
+
+/** Return an approximate number of bytes stored in memory to hold the
+ * Zstandard compression/decompression state. */
+static size_t
+tor_zstd_state_size_precalc(int compress, int preset)
+{
+#ifdef ZSTD_STATIC_LINKING_ONLY
+  if (tor_zstd_can_use_static_apis()) {
+    if (compress) {
+      return ZSTD_estimateCStreamSize(preset);
+    } else {
+      /* Could use DStream, but that takes a windowSize. */
+      return ZSTD_estimateDCtxSize();
+    }
+  }
+#endif
+  return tor_zstd_state_size_precalc_fake(compress, preset);
+}
 #endif /* defined(HAVE_ZSTD) */
 
 /** Construct and return a tor_zstd_compress_state_t object using
@@ -440,3 +495,34 @@ tor_zstd_init(void)
   atomic_counter_init(&total_zstd_allocation);
 }
 
+/** Warn if the header and library versions don't match. */
+void
+tor_zstd_warn_if_version_mismatched(void)
+{
+#if defined(HAVE_ZSTD) && defined(ENABLE_ZSTD_ADVANCED_APIS)
+  if (! tor_zstd_can_use_static_apis()) {
+    char header_version[VERSION_STR_MAX_LEN];
+    char runtime_version[VERSION_STR_MAX_LEN];
+    tor_zstd_format_version(header_version, sizeof(header_version),
+                            ZSTD_VERSION_NUMBER);
+    tor_zstd_format_version(runtime_version, sizeof(runtime_version),
+                            ZSTD_versionNumber());
+
+    log_warn(LD_GENERAL,
+             "Tor was compiled with zstd %s, but is running with zstd %s. "
+             "For safety, we'll avoid using advanced zstd functionality.",
+             header_version, runtime_version);
+  }
+#endif
+}
+
+#ifdef TOR_UNIT_TESTS
+/** Testing only: disable usage of static-only APIs, so we can make sure that
+ * we still work without them. */
+void
+tor_zstd_set_static_apis_disabled_for_testing(int disabled)
+{
+  static_apis_disable_for_testing = disabled;
+}
+#endif
+

+ 7 - 0
src/common/compress_zstd.h

@@ -17,6 +17,8 @@ const char *tor_zstd_get_version_str(void);
 
 const char *tor_zstd_get_header_version_str(void);
 
+int tor_zstd_can_use_static_apis(void);
+
 /** Internal state for an incremental Zstandard compression/decompression. */
 typedef struct tor_zstd_compress_state_t tor_zstd_compress_state_t;
 
@@ -41,6 +43,11 @@ size_t tor_zstd_compress_state_size(const tor_zstd_compress_state_t *state);
 size_t tor_zstd_get_total_allocation(void);
 
 void tor_zstd_init(void);
+void tor_zstd_warn_if_version_mismatched(void);
+
+#ifdef TOR_UNIT_TESTS
+void tor_zstd_set_static_apis_disabled_for_testing(int disabled);
+#endif
 
 #endif /* !defined(TOR_COMPRESS_ZSTD_H) */
 

+ 2 - 0
src/or/main.c

@@ -3332,6 +3332,8 @@ tor_init(int argc, char *argv[])
     if (strstr(version, "alpha") || strstr(version, "beta"))
       log_notice(LD_GENERAL, "This version is not a stable Tor release. "
                  "Expect more bugs than usual.");
+
+    tor_compress_log_init_warnings();
   }
 
 #ifdef HAVE_RUST

+ 39 - 4
src/test/test_util.c

@@ -16,6 +16,7 @@
 #include "memarea.h"
 #include "util_process.h"
 #include "log_test_helpers.h"
+#include "compress_zstd.h"
 
 #ifdef HAVE_PWD_H
 #include <pwd.h>
@@ -2396,6 +2397,37 @@ test_util_compress_stream_impl(compress_method_t method,
   tor_free(buf3);
 }
 
+/** Setup function for compression tests: handles x-zstd:nostatic
+ */
+static void *
+compression_test_setup(const struct testcase_t *testcase)
+{
+  tor_assert(testcase->setup_data);
+  tor_assert(testcase->setup_data != (void*)TT_SKIP);
+  const char *methodname = testcase->setup_data;
+
+  if (!strcmp(methodname, "x-zstd:nostatic")) {
+    methodname = "x-zstd";
+    tor_zstd_set_static_apis_disabled_for_testing(1);
+  }
+
+  return (void *)methodname;
+}
+
+/** Cleanup for compression tests: disables nostatic */
+static int
+compression_test_cleanup(const struct testcase_t *testcase, void *ptr)
+{
+  (void)testcase;
+  (void)ptr;
+  tor_zstd_set_static_apis_disabled_for_testing(0);
+  return 1;
+}
+
+static const struct testcase_setup_t compress_setup = {
+  compression_test_setup, compression_test_cleanup
+};
+
 /** Run unit tests for compression functions */
 static void
 test_util_compress(void *arg)
@@ -6095,22 +6127,22 @@ test_util_get_unquoted_path(void *arg)
   { #name, test_util_ ## name, flags, NULL, NULL }
 
 #define COMPRESS(name, identifier)              \
-  { "compress/" #name, test_util_compress, 0, &passthrough_setup,       \
+  { "compress/" #name, test_util_compress, 0, &compress_setup,          \
     (char*)(identifier) }
 
 #define COMPRESS_CONCAT(name, identifier)                               \
   { "compress_concat/" #name, test_util_decompress_concatenated, 0,     \
-    &passthrough_setup,                                                 \
+    &compress_setup,                                                    \
     (char*)(identifier) }
 
 #define COMPRESS_JUNK(name, identifier)                                 \
   { "compress_junk/" #name, test_util_decompress_junk, 0,               \
-    &passthrough_setup,                                                 \
+    &compress_setup,                                                    \
     (char*)(identifier) }
 
 #define COMPRESS_DOS(name, identifier)                                  \
   { "compress_dos/" #name, test_util_decompress_dos, 0,                 \
-    &passthrough_setup,                                                 \
+    &compress_setup,                                                    \
     (char*)(identifier) }
 
 #ifdef _WIN32
@@ -6141,11 +6173,13 @@ struct testcase_t util_tests[] = {
   COMPRESS(gzip, "gzip"),
   COMPRESS(lzma, "x-tor-lzma"),
   COMPRESS(zstd, "x-zstd"),
+  COMPRESS(zstd_nostatic, "x-zstd:nostatic"),
   COMPRESS(none, "identity"),
   COMPRESS_CONCAT(zlib, "deflate"),
   COMPRESS_CONCAT(gzip, "gzip"),
   COMPRESS_CONCAT(lzma, "x-tor-lzma"),
   COMPRESS_CONCAT(zstd, "x-zstd"),
+  COMPRESS_CONCAT(zstd_nostatic, "x-zstd:nostatic"),
   COMPRESS_CONCAT(none, "identity"),
   COMPRESS_JUNK(zlib, "deflate"),
   COMPRESS_JUNK(gzip, "gzip"),
@@ -6154,6 +6188,7 @@ struct testcase_t util_tests[] = {
   COMPRESS_DOS(gzip, "gzip"),
   COMPRESS_DOS(lzma, "x-tor-lzma"),
   COMPRESS_DOS(zstd, "x-zstd"),
+  COMPRESS_DOS(zstd_nostatic, "x-zstd:nostatic"),
   UTIL_TEST(gzip_compression_bomb, TT_FORK),
   UTIL_LEGACY(datadir),
   UTIL_LEGACY(memarea),