February 2017 - tor-commits - lists.torproject.org

[tor/master] Try to check for (and prevent) buffer size INT_MAX overflow better.
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit ee5471f9aab55269c8c480f1f90dfeb08803ac15 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 13 15:51:55 2017 -0500 Try to check for (and prevent) buffer size INT_MAX overflow better. Possible fix or diagnostic for 21369. --- changes/bug21369_check | 3 +++ src/or/buffers.c | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/changes/bug21369_check b/changes/bug21369_check new file mode 100644 index 0000000..2cd808c --- /dev/null +++ b/changes/bug21369_check @@ -0,0 +1,3 @@ + o Minor features (reliability, crash): + - Try better to detect problems in buffers where they might grow (or + think they have grown) over 2 GB in size. Diagnostic for bug 21369. diff --git a/src/or/buffers.c b/src/or/buffers.c index 8981fd2..fc9e7e4 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -562,6 +562,11 @@ read_to_buf(tor_socket_t s, size_t at_most, buf_t *buf, int *reached_eof, tor_assert(reached_eof); tor_assert(SOCKET_OK(s)); + if (BUG(buf->datalen >= INT_MAX)) + return -1; + if (BUG(buf->datalen >= INT_MAX - at_most)) + return -1; + while (at_most > total_read) { size_t readlen = at_most - total_read; chunk_t *chunk; @@ -619,6 +624,11 @@ read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf) check(); + if (BUG(buf->datalen >= INT_MAX)) + return -1; + if (BUG(buf->datalen >= INT_MAX - at_most)) + return -1; + while (at_most > total_read) { size_t readlen = at_most - total_read; chunk_t *chunk; @@ -813,6 +823,11 @@ write_to_buf(const char *string, size_t string_len, buf_t *buf) return (int)buf->datalen; check(); + if (BUG(buf->datalen >= INT_MAX)) + return -1; + if (BUG(buf->datalen >= INT_MAX - string_len)) + return -1; + while (string_len) { size_t copy; if (!buf->tail || !CHUNK_REMAINING_CAPACITY(buf->tail)) @@ -962,6 +977,12 @@ move_buf_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen) /* We can do way better here, but this doesn't turn up in any profiles. */ char b[4096]; size_t cp, len; + + if (BUG(buf_out->datalen >= INT_MAX)) + return -1; + if (BUG(buf_out->datalen >= INT_MAX - *buf_flushlen)) + return -1; + len = *buf_flushlen; if (len > buf_in->datalen) len = buf_in->datalen;

1 0

[tor/master] Merge branch 'bug21369_check_029_squashed' into maint-0.3.0
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit c51919b0da2afe73780508e5593ebf9052e177cc Merge: 6747c62 074f248 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 27 11:25:34 2017 -0500 Merge branch 'bug21369_check_029_squashed' into maint-0.3.0 changes/bug21369_check | 3 +++ src/or/buffers.c | 21 +++++++++++++++++++++ src/or/dirserv.c | 10 ++++++++-- 3 files changed, 32 insertions(+), 2 deletions(-)

1 0

[tor/master] Merge branch 'maint-0.3.0'
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit b6a9be0415587b161e59702a56cbfa42d1ac31c8 Merge: 90283f0 c51919b Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 27 11:25:46 2017 -0500 Merge branch 'maint-0.3.0' changes/bug21369_check | 3 +++ src/or/buffers.c | 21 +++++++++++++++++++++ src/or/dirserv.c | 10 ++++++++-- 3 files changed, 32 insertions(+), 2 deletions(-)

1 0

[tor/master] Add one other BUG check to try to fix/solve 21369.
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit 074f24846321b8d08a8f67c37d72018842274c4e Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 27 09:12:51 2017 -0500 Add one other BUG check to try to fix/solve 21369. Teor thinks that this connection_dirserv_add_dir_bytes_to_outbuf() might be the problem, if the "remaining" calculation underflows. So I'm adding a couple of checks there, and improving the casts. --- src/or/dirserv.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index fa3938b..fd9d0c7 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3629,8 +3629,14 @@ connection_dirserv_add_dir_bytes_to_outbuf(dir_connection_t *conn) if (bytes < 8192) bytes = 8192; remaining = conn->cached_dir->dir_z_len - conn->cached_dir_offset; - if (bytes > remaining) + if (BUG(remaining < 0)) { + remaining = 0; + } + if (bytes > remaining) { bytes = (ssize_t) remaining; + if (BUG(bytes < 0)) + return -1; + } if (conn->zlib_state) { connection_write_to_buf_zlib( @@ -3641,7 +3647,7 @@ connection_dirserv_add_dir_bytes_to_outbuf(dir_connection_t *conn) bytes, TO_CONN(conn)); } conn->cached_dir_offset += bytes; - if (conn->cached_dir_offset == (int)conn->cached_dir->dir_z_len) { + if (conn->cached_dir_offset >= (off_t)conn->cached_dir->dir_z_len) { /* We just wrote the last one; finish up. */ connection_dirserv_finish_spooling(conn); cached_dir_decref(conn->cached_dir);

1 0

[tor/master] Merge branch 'bug21420_029_squashed' into maint-0.3.0
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit 6747c62386964d961c3e98768e4dc9c51f8529de Merge: 2b3518b f6e5a65 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 27 11:20:39 2017 -0500 Merge branch 'bug21420_029_squashed' into maint-0.3.0 changes/bug21420 | 3 +++ src/common/tortls.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)

1 0

[tor/master] Revise the logic for picking the start time for link certs
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit f6e5a658df84cf9dd01ab7d61cfb25f0fb9040c1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 13 15:37:41 2017 -0500 Revise the logic for picking the start time for link certs Since 0.2.4.11-alpha (in 0196647970a91d) we've tried to randomize the start time to up to some time in the past. But unfortunately we allowed the start time to be in the future as well, which isn't really legit. The new behavior lets the start time be be up to MAX(cert_lifetime-2days, 0) in the past, but never in the future. Fixes bug 21420; bugfix on 0.2.4.11-alpha. --- changes/bug21420 | 3 +++ src/common/tortls.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/changes/bug21420 b/changes/bug21420 new file mode 100644 index 0000000..0144044 --- /dev/null +++ b/changes/bug21420 @@ -0,0 +1,3 @@ + o Minor bugfixes (certificate expiration time): + - Avoid using link certificates that don't become valid till + some time in the future. Fixes bug 21420; bugfix on 0.2.4.11-alpha diff --git a/src/common/tortls.c b/src/common/tortls.c index 62ed5be..5c5aa72 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -482,8 +482,22 @@ MOCK_IMPL(STATIC X509 *, * then we might pick a time where we're about to expire. Lastly, be * sure to start on a day boundary. */ time_t now = time(NULL); - start_time = crypto_rand_time_range(now - cert_lifetime, now) + 2*24*3600; - start_time -= start_time % (24*3600); + /* Our certificate lifetime will be cert_lifetime no matter what, but if we + * start cert_lifetime in the past, we'll have 0 real lifetime. instead we + * start up to (cert_lifetime - min_real_lifetime - start_granularity) in + * the past. */ + const time_t min_real_lifetime = 24*3600; + const time_t start_granularity = 24*3600; + time_t earliest_start_time = now - cert_lifetime + min_real_lifetime + + start_granularity; + /* Don't actually start in the future! */ + if (earliest_start_time >= now) + earliest_start_time = now - 1; + start_time = crypto_rand_time_range(earliest_start_time, now); + /* Round the start time back to the start of a day. */ + start_time -= start_time % start_granularity; + + end_time = start_time + cert_lifetime; tor_assert(rsa); tor_assert(cname); @@ -517,7 +531,6 @@ MOCK_IMPL(STATIC X509 *, if (!X509_time_adj(X509_get_notBefore(x509),0,&start_time)) goto error; - end_time = start_time + cert_lifetime; if (!X509_time_adj(X509_get_notAfter(x509),0,&end_time)) goto error; if (!X509_set_pubkey(x509, pkey))

1 0

[tor/release-0.3.0] Revise the logic for picking the start time for link certs
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit f6e5a658df84cf9dd01ab7d61cfb25f0fb9040c1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 13 15:37:41 2017 -0500 Revise the logic for picking the start time for link certs Since 0.2.4.11-alpha (in 0196647970a91d) we've tried to randomize the start time to up to some time in the past. But unfortunately we allowed the start time to be in the future as well, which isn't really legit. The new behavior lets the start time be be up to MAX(cert_lifetime-2days, 0) in the past, but never in the future. Fixes bug 21420; bugfix on 0.2.4.11-alpha. --- changes/bug21420 | 3 +++ src/common/tortls.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/changes/bug21420 b/changes/bug21420 new file mode 100644 index 0000000..0144044 --- /dev/null +++ b/changes/bug21420 @@ -0,0 +1,3 @@ + o Minor bugfixes (certificate expiration time): + - Avoid using link certificates that don't become valid till + some time in the future. Fixes bug 21420; bugfix on 0.2.4.11-alpha diff --git a/src/common/tortls.c b/src/common/tortls.c index 62ed5be..5c5aa72 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -482,8 +482,22 @@ MOCK_IMPL(STATIC X509 *, * then we might pick a time where we're about to expire. Lastly, be * sure to start on a day boundary. */ time_t now = time(NULL); - start_time = crypto_rand_time_range(now - cert_lifetime, now) + 2*24*3600; - start_time -= start_time % (24*3600); + /* Our certificate lifetime will be cert_lifetime no matter what, but if we + * start cert_lifetime in the past, we'll have 0 real lifetime. instead we + * start up to (cert_lifetime - min_real_lifetime - start_granularity) in + * the past. */ + const time_t min_real_lifetime = 24*3600; + const time_t start_granularity = 24*3600; + time_t earliest_start_time = now - cert_lifetime + min_real_lifetime + + start_granularity; + /* Don't actually start in the future! */ + if (earliest_start_time >= now) + earliest_start_time = now - 1; + start_time = crypto_rand_time_range(earliest_start_time, now); + /* Round the start time back to the start of a day. */ + start_time -= start_time % start_granularity; + + end_time = start_time + cert_lifetime; tor_assert(rsa); tor_assert(cname); @@ -517,7 +531,6 @@ MOCK_IMPL(STATIC X509 *, if (!X509_time_adj(X509_get_notBefore(x509),0,&start_time)) goto error; - end_time = start_time + cert_lifetime; if (!X509_time_adj(X509_get_notAfter(x509),0,&end_time)) goto error; if (!X509_set_pubkey(x509, pkey))

1 0

[tor/master] Merge branch 'maint-0.3.0'
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit 90283f0db39349807614b6a7ed510084e3bb8527 Merge: 1421f75 6747c62 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 27 11:23:02 2017 -0500 Merge branch 'maint-0.3.0' changes/bug21420 | 3 +++ src/common/tortls.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)

1 0

[tor/release-0.3.0] Merge branch 'bug21420_029_squashed' into maint-0.3.0
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit 6747c62386964d961c3e98768e4dc9c51f8529de Merge: 2b3518b f6e5a65 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 27 11:20:39 2017 -0500 Merge branch 'bug21420_029_squashed' into maint-0.3.0 changes/bug21420 | 3 +++ src/common/tortls.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)

1 0

[tor/maint-0.3.0] Revise the logic for picking the start time for link certs
by nickm＠torproject.org 27 Feb '17

27 Feb '17

commit f6e5a658df84cf9dd01ab7d61cfb25f0fb9040c1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Feb 13 15:37:41 2017 -0500 Revise the logic for picking the start time for link certs Since 0.2.4.11-alpha (in 0196647970a91d) we've tried to randomize the start time to up to some time in the past. But unfortunately we allowed the start time to be in the future as well, which isn't really legit. The new behavior lets the start time be be up to MAX(cert_lifetime-2days, 0) in the past, but never in the future. Fixes bug 21420; bugfix on 0.2.4.11-alpha. --- changes/bug21420 | 3 +++ src/common/tortls.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/changes/bug21420 b/changes/bug21420 new file mode 100644 index 0000000..0144044 --- /dev/null +++ b/changes/bug21420 @@ -0,0 +1,3 @@ + o Minor bugfixes (certificate expiration time): + - Avoid using link certificates that don't become valid till + some time in the future. Fixes bug 21420; bugfix on 0.2.4.11-alpha diff --git a/src/common/tortls.c b/src/common/tortls.c index 62ed5be..5c5aa72 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -482,8 +482,22 @@ MOCK_IMPL(STATIC X509 *, * then we might pick a time where we're about to expire. Lastly, be * sure to start on a day boundary. */ time_t now = time(NULL); - start_time = crypto_rand_time_range(now - cert_lifetime, now) + 2*24*3600; - start_time -= start_time % (24*3600); + /* Our certificate lifetime will be cert_lifetime no matter what, but if we + * start cert_lifetime in the past, we'll have 0 real lifetime. instead we + * start up to (cert_lifetime - min_real_lifetime - start_granularity) in + * the past. */ + const time_t min_real_lifetime = 24*3600; + const time_t start_granularity = 24*3600; + time_t earliest_start_time = now - cert_lifetime + min_real_lifetime + + start_granularity; + /* Don't actually start in the future! */ + if (earliest_start_time >= now) + earliest_start_time = now - 1; + start_time = crypto_rand_time_range(earliest_start_time, now); + /* Round the start time back to the start of a day. */ + start_time -= start_time % start_granularity; + + end_time = start_time + cert_lifetime; tor_assert(rsa); tor_assert(cname); @@ -517,7 +531,6 @@ MOCK_IMPL(STATIC X509 *, if (!X509_time_adj(X509_get_notBefore(x509),0,&start_time)) goto error; - end_time = start_time + cert_lifetime; if (!X509_time_adj(X509_get_notAfter(x509),0,&end_time)) goto error; if (!X509_set_pubkey(x509, pkey))

1 0