July 2021 - tor-commits - lists.torproject.org

[tor/main] Merge remote-tracking branch 'tor-gitlab/mr/409' into maint-0.4.5
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 4865eabd18d0b5a70bf971814213327c0ca8b106 Merge: 98b9df61f6 2bc02b2199 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jul 6 13:51:58 2021 +0300 Merge remote-tracking branch 'tor-gitlab/mr/409' into maint-0.4.5 src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

1 0

[tor/main] Merge branch 'maint-0.4.6'
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 8949317eccfc1334bfc03344fc3ae32d380aa791 Merge: 6aff048978 a522aabd3b Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jul 6 13:52:16 2021 +0300 Merge branch 'maint-0.4.6' src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

1 0

[tor/main] Suppress a clang 12 warning about "suspicious concatenation".
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 2bc02b21992825e1e348f83c31ebbe0ddcee0b9b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 1 13:03:19 2021 -0400 Suppress a clang 12 warning about "suspicious concatenation". My clang doesn't like it when we write code like this: char *list[] = { "abc", "def", "ghi" "jkl" } It wonders whether we meant to put a comma between "ghi" and "jkl" or not, and gives a warning. To suppress this warning (since in this case, we did mean to omit the comma), we just wrap the two strings in parentheses. Closes #40426; bugfix on 0.4.0.4-rc. --- src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index d62dd3fb9e..467b740d71 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2135,8 +2135,8 @@ test_dir_measured_bw_kb(void *arg) /* Test that a line with vote=0 will fail too, so that it is ignored. */ "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0\n", /* Test that a line with vote=0 will fail even if unmeasured=0. */ - "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0 " - "unmeasured=0\n", + ("node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0 " + "unmeasured=0\n"), "end" };

1 0

[tor/main] Merge branch 'maint-0.4.5' into maint-0.4.6
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit a522aabd3b436b56d5575ea808451e1577185bcd Merge: 167f3bc4ec 4865eabd18 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jul 6 13:52:15 2021 +0300 Merge branch 'maint-0.4.5' into maint-0.4.6 src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

1 0

[tor/maint-0.4.5] Suppress a clang 12 warning about "suspicious concatenation".
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 2bc02b21992825e1e348f83c31ebbe0ddcee0b9b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 1 13:03:19 2021 -0400 Suppress a clang 12 warning about "suspicious concatenation". My clang doesn't like it when we write code like this: char *list[] = { "abc", "def", "ghi" "jkl" } It wonders whether we meant to put a comma between "ghi" and "jkl" or not, and gives a warning. To suppress this warning (since in this case, we did mean to omit the comma), we just wrap the two strings in parentheses. Closes #40426; bugfix on 0.4.0.4-rc. --- src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index d62dd3fb9e..467b740d71 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2135,8 +2135,8 @@ test_dir_measured_bw_kb(void *arg) /* Test that a line with vote=0 will fail too, so that it is ignored. */ "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0\n", /* Test that a line with vote=0 will fail even if unmeasured=0. */ - "node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0 " - "unmeasured=0\n", + ("node_id=$557365204145532d32353620696e73746561642e bw=1024 vote=0 " + "unmeasured=0\n"), "end" };

1 0

[tor/maint-0.4.5] Merge remote-tracking branch 'tor-gitlab/mr/409' into maint-0.4.5
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 4865eabd18d0b5a70bf971814213327c0ca8b106 Merge: 98b9df61f6 2bc02b2199 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jul 6 13:51:58 2021 +0300 Merge remote-tracking branch 'tor-gitlab/mr/409' into maint-0.4.5 src/test/test_dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

1 0

[tor/main] Merge branch 'maint-0.4.5' into maint-0.4.6
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 167f3bc4ecbe4f0265413f3a1b91b6faab1c0f81 Merge: 2758ae30b3 98b9df61f6 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jul 6 13:42:53 2021 +0300 Merge branch 'maint-0.4.5' into maint-0.4.6 changes/bug40383 | 7 +++++++ configure.ac | 1 + src/lib/encoding/time_fmt.c | 35 +++++++++++++++++++++++++++++++++-- src/lib/encoding/time_fmt.h | 6 ++++++ src/test/test_util.c | 26 +++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 3 deletions(-)

1 0

[tor/main] Merge branch 'mr/395' into maint-0.4.5
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit 98b9df61f6f2d33dab857b1e7d2f8e848678f5ed Merge: e60d14bb6b c1d96358d4 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Jul 6 13:42:29 2021 +0300 Merge branch 'mr/395' into maint-0.4.5 changes/bug40383 | 7 +++++++ configure.ac | 1 + src/lib/encoding/time_fmt.c | 35 +++++++++++++++++++++++++++++++++-- src/lib/encoding/time_fmt.h | 6 ++++++ src/test/test_util.c | 26 +++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 3 deletions(-)

1 0

[tor/main] Use native timegm when available.
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit c1d96358d49a4583b8aa9bdb1e8d73c70f9d8d06 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jun 1 16:18:23 2021 -0400 Use native timegm when available. Continue having a tor_gmtime_impl() unit test so that we can detect any problems in our replacement function; add a new test function to make sure that gmtime<->timegm are a round-trip on now-ish times. This is a fix for bug #40383, wherein we ran into trouble because tor_timegm() does not believe that time_t should include a count of leap seconds, but FreeBSD's gmtime believes that it should. This disagreement meant that for a certain amount of time each day, instead of calculating the most recent midnight, our voting-schedule functions would calculate the second-most-recent midnight, and lead to an assertion failure. I am calling this a bugfix on 0.2.0.3-alpha when we first started calculating our voting schedule in this way. --- changes/bug40383 | 7 +++++++ configure.ac | 1 + src/lib/encoding/time_fmt.c | 35 +++++++++++++++++++++++++++++++++-- src/lib/encoding/time_fmt.h | 6 ++++++ src/test/test_util.c | 26 +++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/changes/bug40383 b/changes/bug40383 new file mode 100644 index 0000000000..c4ca46fac7 --- /dev/null +++ b/changes/bug40383 @@ -0,0 +1,7 @@ + o Minor bugfixes (timekeeping): + - Calculate the time of day correctly on systems where the time_t + type includes leap seconds. (This is not the case on most + operating systems, but on those where it occurs, our tor_timegm + function did not correctly invert the system's gmtime function, + which could result in assertion failures when calculating + voting schedules.) Fixes bug 40383; bugfix on 0.2.0.3-alpha. diff --git a/configure.ac b/configure.ac index 6d7cd1dd6a..5a01c28bde 100644 --- a/configure.ac +++ b/configure.ac @@ -793,6 +793,7 @@ AC_CHECK_FUNCS( strtoull \ sysconf \ sysctl \ + timegm \ truncate \ uname \ usleep \ diff --git a/src/lib/encoding/time_fmt.c b/src/lib/encoding/time_fmt.c index 573dfaad82..5e58d36698 100644 --- a/src/lib/encoding/time_fmt.c +++ b/src/lib/encoding/time_fmt.c @@ -13,6 +13,7 @@ * and handles a larger variety of types. It converts between different time * formats, and encodes and decodes them from strings. **/ +#define TIME_FMT_PRIVATE #include "lib/encoding/time_fmt.h" #include "lib/log/log.h" @@ -25,6 +26,7 @@ #include <string.h> #include <time.h> +#include <errno.h> #ifdef HAVE_SYS_TIME_H #include <sys/time.h> @@ -92,8 +94,8 @@ static const int days_per_month[] = /** Compute a time_t given a struct tm. The result is given in UTC, and * does not account for leap seconds. Return 0 on success, -1 on failure. */ -int -tor_timegm(const struct tm *tm, time_t *time_out) +ATTR_UNUSED STATIC int +tor_timegm_impl(const struct tm *tm, time_t *time_out) { /* This is a pretty ironclad timegm implementation, snarfed from Python2.2. * It's way more brute-force than fiddling with tzset(). @@ -162,6 +164,35 @@ tor_timegm(const struct tm *tm, time_t *time_out) return 0; } +/** Compute a time_t given a struct tm. The result here should be an inverse + * of the system's gmtime() function. Return 0 on success, -1 on failure. + */ +int +tor_timegm(const struct tm *tm, time_t *time_out) +{ +#ifdef HAVE_TIMEGM + /* If the system gives us a timegm(), use it: if the system's time_t + * includes leap seconds, then we can hope that its timegm() knows too. + * + * https://k5wiki.kerberos.org/wiki/Leap_second_handling says the in + * general we can rely on any system with leap seconds also having a + * timegm implementation. Let's hope it's right! + * */ + time_t result = timegm((struct tm *) tm); + if (result == -1) { + log_warn(LD_BUG, "timegm() could not convert time: %s", strerror(errno)); + *time_out = 0; + return -1; + } else { + *time_out = result; + return 0; + } +#else + /* The system doesn't have timegm; we'll have to use our own. */ + return tor_timegm_impl(tm, time_out); +#endif +} + /* strftime is locale-specific, so we need to replace those parts */ /** A c-locale array of 3-letter names of weekdays, starting with Sun. */ diff --git a/src/lib/encoding/time_fmt.h b/src/lib/encoding/time_fmt.h index 80e47c5332..4adccb5990 100644 --- a/src/lib/encoding/time_fmt.h +++ b/src/lib/encoding/time_fmt.h @@ -18,6 +18,8 @@ #include <sys/types.h> #endif +#include "lib/testsupport/testsupport.h" + struct tm; struct timeval; @@ -41,4 +43,8 @@ int parse_iso_time_nospace(const char *cp, time_t *t); int parse_http_time(const char *buf, struct tm *tm); int format_time_interval(char *out, size_t out_len, long interval); +#ifdef TIME_FMT_PRIVATE +STATIC int tor_timegm_impl(const struct tm *tm, time_t *time_out); +#endif + #endif /* !defined(TOR_TIME_FMT_H) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index d43bf781f2..ab63344806 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -7,6 +7,7 @@ #define COMPAT_TIME_PRIVATE #define UTIL_MALLOC_PRIVATE #define PROCESS_WIN32_PRIVATE +#define TIME_FMT_PRIVATE #include "lib/testsupport/testsupport.h" #include "core/or/or.h" #include "lib/buf/buffers.h" @@ -111,7 +112,7 @@ static time_t tor_timegm_wrapper(const struct tm *tm) { time_t t; - if (tor_timegm(tm, &t) < 0) + if (tor_timegm_impl(tm, &t) < 0) return -1; return t; } @@ -1501,6 +1502,28 @@ test_util_parse_http_time(void *arg) teardown_capture_of_logs(); } +static void +test_util_timegm_real(void *arg) +{ + (void)arg; + /* Get the real timegm again! We're not testing our impl; we want the + * one that will actually get called. */ +#undef tor_timegm + + /* Now check: is timegm the real inverse of gmtime? */ + time_t now = time(NULL), time2=0; + struct tm tm, *p; + p = tor_gmtime_r(&now, &tm); + tt_ptr_op(p, OP_NE, NULL); + + int r = tor_timegm(&tm, &time2); + tt_int_op(r, OP_EQ, 0); + tt_i64_op((int64_t) now, OP_EQ, (int64_t) time2); + + done: + ; +} + static void test_util_config_line(void *arg) { @@ -7043,6 +7066,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(monotonic_time_ratchet, TT_FORK), UTIL_TEST(monotonic_time_zero, 0), UTIL_TEST(monotonic_time_add_msec, 0), + UTIL_TEST(timegm_real, 0), UTIL_TEST(htonll, 0), UTIL_TEST(get_unquoted_path, 0), UTIL_TEST(map_anon, 0),

1 0

[tor/maint-0.4.5] Use native timegm when available.
by asn＠torproject.org 06 Jul '21

06 Jul '21

commit c1d96358d49a4583b8aa9bdb1e8d73c70f9d8d06 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jun 1 16:18:23 2021 -0400 Use native timegm when available. Continue having a tor_gmtime_impl() unit test so that we can detect any problems in our replacement function; add a new test function to make sure that gmtime<->timegm are a round-trip on now-ish times. This is a fix for bug #40383, wherein we ran into trouble because tor_timegm() does not believe that time_t should include a count of leap seconds, but FreeBSD's gmtime believes that it should. This disagreement meant that for a certain amount of time each day, instead of calculating the most recent midnight, our voting-schedule functions would calculate the second-most-recent midnight, and lead to an assertion failure. I am calling this a bugfix on 0.2.0.3-alpha when we first started calculating our voting schedule in this way. --- changes/bug40383 | 7 +++++++ configure.ac | 1 + src/lib/encoding/time_fmt.c | 35 +++++++++++++++++++++++++++++++++-- src/lib/encoding/time_fmt.h | 6 ++++++ src/test/test_util.c | 26 +++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/changes/bug40383 b/changes/bug40383 new file mode 100644 index 0000000000..c4ca46fac7 --- /dev/null +++ b/changes/bug40383 @@ -0,0 +1,7 @@ + o Minor bugfixes (timekeeping): + - Calculate the time of day correctly on systems where the time_t + type includes leap seconds. (This is not the case on most + operating systems, but on those where it occurs, our tor_timegm + function did not correctly invert the system's gmtime function, + which could result in assertion failures when calculating + voting schedules.) Fixes bug 40383; bugfix on 0.2.0.3-alpha. diff --git a/configure.ac b/configure.ac index 6d7cd1dd6a..5a01c28bde 100644 --- a/configure.ac +++ b/configure.ac @@ -793,6 +793,7 @@ AC_CHECK_FUNCS( strtoull \ sysconf \ sysctl \ + timegm \ truncate \ uname \ usleep \ diff --git a/src/lib/encoding/time_fmt.c b/src/lib/encoding/time_fmt.c index 573dfaad82..5e58d36698 100644 --- a/src/lib/encoding/time_fmt.c +++ b/src/lib/encoding/time_fmt.c @@ -13,6 +13,7 @@ * and handles a larger variety of types. It converts between different time * formats, and encodes and decodes them from strings. **/ +#define TIME_FMT_PRIVATE #include "lib/encoding/time_fmt.h" #include "lib/log/log.h" @@ -25,6 +26,7 @@ #include <string.h> #include <time.h> +#include <errno.h> #ifdef HAVE_SYS_TIME_H #include <sys/time.h> @@ -92,8 +94,8 @@ static const int days_per_month[] = /** Compute a time_t given a struct tm. The result is given in UTC, and * does not account for leap seconds. Return 0 on success, -1 on failure. */ -int -tor_timegm(const struct tm *tm, time_t *time_out) +ATTR_UNUSED STATIC int +tor_timegm_impl(const struct tm *tm, time_t *time_out) { /* This is a pretty ironclad timegm implementation, snarfed from Python2.2. * It's way more brute-force than fiddling with tzset(). @@ -162,6 +164,35 @@ tor_timegm(const struct tm *tm, time_t *time_out) return 0; } +/** Compute a time_t given a struct tm. The result here should be an inverse + * of the system's gmtime() function. Return 0 on success, -1 on failure. + */ +int +tor_timegm(const struct tm *tm, time_t *time_out) +{ +#ifdef HAVE_TIMEGM + /* If the system gives us a timegm(), use it: if the system's time_t + * includes leap seconds, then we can hope that its timegm() knows too. + * + * https://k5wiki.kerberos.org/wiki/Leap_second_handling says the in + * general we can rely on any system with leap seconds also having a + * timegm implementation. Let's hope it's right! + * */ + time_t result = timegm((struct tm *) tm); + if (result == -1) { + log_warn(LD_BUG, "timegm() could not convert time: %s", strerror(errno)); + *time_out = 0; + return -1; + } else { + *time_out = result; + return 0; + } +#else + /* The system doesn't have timegm; we'll have to use our own. */ + return tor_timegm_impl(tm, time_out); +#endif +} + /* strftime is locale-specific, so we need to replace those parts */ /** A c-locale array of 3-letter names of weekdays, starting with Sun. */ diff --git a/src/lib/encoding/time_fmt.h b/src/lib/encoding/time_fmt.h index 80e47c5332..4adccb5990 100644 --- a/src/lib/encoding/time_fmt.h +++ b/src/lib/encoding/time_fmt.h @@ -18,6 +18,8 @@ #include <sys/types.h> #endif +#include "lib/testsupport/testsupport.h" + struct tm; struct timeval; @@ -41,4 +43,8 @@ int parse_iso_time_nospace(const char *cp, time_t *t); int parse_http_time(const char *buf, struct tm *tm); int format_time_interval(char *out, size_t out_len, long interval); +#ifdef TIME_FMT_PRIVATE +STATIC int tor_timegm_impl(const struct tm *tm, time_t *time_out); +#endif + #endif /* !defined(TOR_TIME_FMT_H) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index d43bf781f2..ab63344806 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -7,6 +7,7 @@ #define COMPAT_TIME_PRIVATE #define UTIL_MALLOC_PRIVATE #define PROCESS_WIN32_PRIVATE +#define TIME_FMT_PRIVATE #include "lib/testsupport/testsupport.h" #include "core/or/or.h" #include "lib/buf/buffers.h" @@ -111,7 +112,7 @@ static time_t tor_timegm_wrapper(const struct tm *tm) { time_t t; - if (tor_timegm(tm, &t) < 0) + if (tor_timegm_impl(tm, &t) < 0) return -1; return t; } @@ -1501,6 +1502,28 @@ test_util_parse_http_time(void *arg) teardown_capture_of_logs(); } +static void +test_util_timegm_real(void *arg) +{ + (void)arg; + /* Get the real timegm again! We're not testing our impl; we want the + * one that will actually get called. */ +#undef tor_timegm + + /* Now check: is timegm the real inverse of gmtime? */ + time_t now = time(NULL), time2=0; + struct tm tm, *p; + p = tor_gmtime_r(&now, &tm); + tt_ptr_op(p, OP_NE, NULL); + + int r = tor_timegm(&tm, &time2); + tt_int_op(r, OP_EQ, 0); + tt_i64_op((int64_t) now, OP_EQ, (int64_t) time2); + + done: + ; +} + static void test_util_config_line(void *arg) { @@ -7043,6 +7066,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(monotonic_time_ratchet, TT_FORK), UTIL_TEST(monotonic_time_zero, 0), UTIL_TEST(monotonic_time_add_msec, 0), + UTIL_TEST(timegm_real, 0), UTIL_TEST(htonll, 0), UTIL_TEST(get_unquoted_path, 0), UTIL_TEST(map_anon, 0),

1 0