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

[tor/master] Merge branch 'maint-0.4.4'
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 2e98e881178470201ed23ef381b0e9ede4f4f613 Merge: 20d1a1cdb 18fa53fdf Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 10:15:04 2020 -0400 Merge branch 'maint-0.4.4' changes/ticket32622 | 5 +++++ src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 18 +++++++++++++----- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 9 files changed, 62 insertions(+), 6 deletions(-)

1 0

[tor/master] Add a TLS_ERROR bootstrap failure reason.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit e429ceb2667787750af070929d0e64cbc7a6dda4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 10:59:59 2020 -0400 Add a TLS_ERROR bootstrap failure reason. If our TLS connection fails for a "misc" reason, we don't need to say that the reason is "misc" -- we can at least localize it to the TLS module. Part of a fix for #32622. --- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/or/or.h b/src/core/or/or.h index 5b35cbe7f..8758a2ec6 100644 --- a/src/core/or/or.h +++ b/src/core/or/or.h @@ -220,7 +220,8 @@ struct curve25519_public_key_t; #define END_OR_CONN_REASON_IO_ERROR 7 /* read/write error */ #define END_OR_CONN_REASON_RESOURCE_LIMIT 8 /* sockets, buffers, etc */ #define END_OR_CONN_REASON_PT_MISSING 9 /* PT failed or not available */ -#define END_OR_CONN_REASON_MISC 10 +#define END_OR_CONN_REASON_TLS_ERROR 10 /* Problem in TLS protocol */ +#define END_OR_CONN_REASON_MISC 11 /* Reasons why we (or a remote OR) might close a stream. See tor-spec.txt for * documentation of these. The values must match. */ diff --git a/src/core/or/reasons.c b/src/core/or/reasons.c index 7da7843ca..708f43a68 100644 --- a/src/core/or/reasons.c +++ b/src/core/or/reasons.c @@ -244,6 +244,8 @@ orconn_end_reason_to_control_string(int r) return "IOERROR"; case END_OR_CONN_REASON_RESOURCE_LIMIT: return "RESOURCELIMIT"; + case END_OR_CONN_REASON_TLS_ERROR: + return "TLS_ERROR"; case END_OR_CONN_REASON_MISC: return "MISC"; case END_OR_CONN_REASON_PT_MISSING: @@ -276,6 +278,8 @@ tls_error_to_orconn_end_reason(int e) case TOR_TLS_CLOSE: case TOR_TLS_DONE: return END_OR_CONN_REASON_DONE; + case TOR_TLS_ERROR_MISC: + return END_OR_CONN_REASON_TLS_ERROR; default: return END_OR_CONN_REASON_MISC; }

1 0

[tor/master] Store errors from TLS write and handshake in the tls_error field.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit d486c6cf74db6bce80689c17c7427a6e3a055f93 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 10:45:42 2020 -0400 Store errors from TLS write and handshake in the tls_error field. Previously, we would only update this field when the error happened during a read. This will improves our reporting for our bootstrap status, and help to address #32622. The problem is not completely solved by this patch, however: too many errors are still lumped into "MISC". --- src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index a8417e46d..792cdbf68 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -4205,6 +4205,7 @@ connection_handle_write_impl(connection_t *conn, int force) switch (result) { CASE_TOR_TLS_ERROR_ANY: case TOR_TLS_CLOSE: + or_conn->tls_error = result; log_info(LD_NET, result != TOR_TLS_CLOSE ? "tls error. breaking.":"TLS connection closed on flush"); /* Don't flush; connection is dead. */ diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 5d71b363f..4cb83f45a 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -1692,7 +1692,8 @@ connection_tls_continue_handshake(or_connection_t *conn) switch (result) { CASE_TOR_TLS_ERROR_ANY: - log_info(LD_OR,"tls error [%s]. breaking connection.", + conn->tls_error = result; + log_info(LD_OR,"tls error [%s]. breaking connection.", tor_tls_err_to_string(result)); return -1; case TOR_TLS_DONE: @@ -1724,6 +1725,7 @@ connection_tls_continue_handshake(or_connection_t *conn) log_debug(LD_OR,"wanted read"); return 0; case TOR_TLS_CLOSE: + conn->tls_error = result; log_info(LD_OR,"tls closed. breaking connection."); return -1; }

1 0

[tor/release-0.4.4] Store errors from TLS write and handshake in the tls_error field.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit d486c6cf74db6bce80689c17c7427a6e3a055f93 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 10:45:42 2020 -0400 Store errors from TLS write and handshake in the tls_error field. Previously, we would only update this field when the error happened during a read. This will improves our reporting for our bootstrap status, and help to address #32622. The problem is not completely solved by this patch, however: too many errors are still lumped into "MISC". --- src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index a8417e46d..792cdbf68 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -4205,6 +4205,7 @@ connection_handle_write_impl(connection_t *conn, int force) switch (result) { CASE_TOR_TLS_ERROR_ANY: case TOR_TLS_CLOSE: + or_conn->tls_error = result; log_info(LD_NET, result != TOR_TLS_CLOSE ? "tls error. breaking.":"TLS connection closed on flush"); /* Don't flush; connection is dead. */ diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 5d71b363f..4cb83f45a 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -1692,7 +1692,8 @@ connection_tls_continue_handshake(or_connection_t *conn) switch (result) { CASE_TOR_TLS_ERROR_ANY: - log_info(LD_OR,"tls error [%s]. breaking connection.", + conn->tls_error = result; + log_info(LD_OR,"tls error [%s]. breaking connection.", tor_tls_err_to_string(result)); return -1; case TOR_TLS_DONE: @@ -1724,6 +1725,7 @@ connection_tls_continue_handshake(or_connection_t *conn) log_debug(LD_OR,"wanted read"); return 0; case TOR_TLS_CLOSE: + conn->tls_error = result; log_info(LD_OR,"tls closed. breaking connection."); return -1; }

1 0

[tor/master] Merge branch 'ticket32622_044_squashed' into maint-0.4.4
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 18fa53fdf4e2b35ceee54c317e989bdbc31183d5 Merge: 8da10e823 65b05b12a Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 10:14:55 2020 -0400 Merge branch 'ticket32622_044_squashed' into maint-0.4.4 changes/ticket32622 | 5 +++++ src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 18 +++++++++++++----- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 9 files changed, 62 insertions(+), 6 deletions(-)

1 0

[tor/release-0.4.4] Changes file for #32622.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 65b05b12ab009aad6f7a2702e5217337d2f67f50 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 11:21:53 2020 -0400 Changes file for #32622. --- changes/ticket32622 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/ticket32622 b/changes/ticket32622 new file mode 100644 index 000000000..1c663567f --- /dev/null +++ b/changes/ticket32622 @@ -0,0 +1,5 @@ + o Minor features (bootstrap reporting): + - Report more detailed reasons for bootstrap failure when the failure + happens due to a TLS error. Previously we would just call these errors + "MISC" when they happened during read, and "DONE" when they + happened during any other TLS operation. Closes ticket 32622.

1 0

[tor/release-0.4.4] Add a TLS_ERROR bootstrap failure reason.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit e429ceb2667787750af070929d0e64cbc7a6dda4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 10:59:59 2020 -0400 Add a TLS_ERROR bootstrap failure reason. If our TLS connection fails for a "misc" reason, we don't need to say that the reason is "misc" -- we can at least localize it to the TLS module. Part of a fix for #32622. --- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/or/or.h b/src/core/or/or.h index 5b35cbe7f..8758a2ec6 100644 --- a/src/core/or/or.h +++ b/src/core/or/or.h @@ -220,7 +220,8 @@ struct curve25519_public_key_t; #define END_OR_CONN_REASON_IO_ERROR 7 /* read/write error */ #define END_OR_CONN_REASON_RESOURCE_LIMIT 8 /* sockets, buffers, etc */ #define END_OR_CONN_REASON_PT_MISSING 9 /* PT failed or not available */ -#define END_OR_CONN_REASON_MISC 10 +#define END_OR_CONN_REASON_TLS_ERROR 10 /* Problem in TLS protocol */ +#define END_OR_CONN_REASON_MISC 11 /* Reasons why we (or a remote OR) might close a stream. See tor-spec.txt for * documentation of these. The values must match. */ diff --git a/src/core/or/reasons.c b/src/core/or/reasons.c index 7da7843ca..708f43a68 100644 --- a/src/core/or/reasons.c +++ b/src/core/or/reasons.c @@ -244,6 +244,8 @@ orconn_end_reason_to_control_string(int r) return "IOERROR"; case END_OR_CONN_REASON_RESOURCE_LIMIT: return "RESOURCELIMIT"; + case END_OR_CONN_REASON_TLS_ERROR: + return "TLS_ERROR"; case END_OR_CONN_REASON_MISC: return "MISC"; case END_OR_CONN_REASON_PT_MISSING: @@ -276,6 +278,8 @@ tls_error_to_orconn_end_reason(int e) case TOR_TLS_CLOSE: case TOR_TLS_DONE: return END_OR_CONN_REASON_DONE; + case TOR_TLS_ERROR_MISC: + return END_OR_CONN_REASON_TLS_ERROR; default: return END_OR_CONN_REASON_MISC; }

1 0

[tor/release-0.4.4] Carry TLS error strings forward to controller when reporting them.
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 7b5f58a1c93f99ef3eaa840b4443ca0a4120a512 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 26 11:18:38 2020 -0400 Carry TLS error strings forward to controller when reporting them. Now instead of saying "DONE, DONE" or "MISC, MISC" or "TLS_ERROR, TLS_ERROR", we can finally give a nice sensible "TLS_ERROR, wrong version number" which should help debug a great deal. Closes ticket 32622. --- src/core/or/connection_or.c | 14 ++++++++++---- src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 4cb83f45a..b88d1b6af 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -745,10 +745,16 @@ connection_or_about_to_close(or_connection_t *or_conn) int reason = tls_error_to_orconn_end_reason(or_conn->tls_error); connection_or_event_status(or_conn, OR_CONN_EVENT_FAILED, reason); - if (!authdir_mode_tests_reachability(options)) - control_event_bootstrap_prob_or( - orconn_end_reason_to_control_string(reason), - reason, or_conn); + if (!authdir_mode_tests_reachability(options)) { + const char *warning = NULL; + if (reason == END_OR_CONN_REASON_TLS_ERROR && or_conn->tls) { + warning = tor_tls_get_last_error_msg(or_conn->tls); + } + if (warning == NULL) { + warning = orconn_end_reason_to_control_string(reason); + } + control_event_bootstrap_prob_or(warning, reason, or_conn); + } } } } else if (conn->hold_open_until_flushed) { diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h index e8dbbf527..517cdc17d 100644 --- a/src/lib/tls/tortls.h +++ b/src/lib/tls/tortls.h @@ -81,6 +81,7 @@ void tor_tls_free_all(void); void tor_tls_init(void); void tls_log_errors(tor_tls_t *tls, int severity, int domain, const char *doing); +const char *tor_tls_get_last_error_msg(const tor_tls_t *tls); int tor_tls_context_init(unsigned flags, crypto_pk_t *client_identity, crypto_pk_t *server_identity, diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index 62e826211..a9ec731c0 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -369,6 +369,8 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, (void)tls; PRErrorCode code = PORT_GetError(); + if (tls) + tls->last_error = code; const char *addr = tls ? tls->address : NULL; const char *string = PORT_ErrorToString(code); @@ -391,6 +393,17 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, with, addr); } } +const char * +tor_tls_get_last_error_msg(const tor_tls_t *tls) +{ + IF_BUG_ONCE(!tls) { + return NULL; + } + if (tls->last_error == 0) { + return NULL; + } + return PORT_ErrorToString((PRErrorCode)tls->last_error); +} tor_tls_t * tor_tls_new(tor_socket_t sock, int is_server) diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index 68d6e2aa5..226971414 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -245,10 +245,30 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, const char *doing) unsigned long err; while ((err = ERR_get_error()) != 0) { + if (tls) + tls->last_error = err; tor_tls_log_one_error(tls, err, severity, domain, doing); } } +/** + * Return a string representing more detail about the last error received + * on TLS. + * + * May return null if no error was found. + **/ +const char * +tor_tls_get_last_error_msg(const tor_tls_t *tls) +{ + IF_BUG_ONCE(!tls) { + return NULL; + } + if (tls->last_error == 0) { + return NULL; + } + return (const char*)ERR_reason_error_string(tls->last_error); +} + #define CATCH_SYSCALL 1 #define CATCH_ZERO 2 diff --git a/src/lib/tls/tortls_st.h b/src/lib/tls/tortls_st.h index 925896d49..34abe52ee 100644 --- a/src/lib/tls/tortls_st.h +++ b/src/lib/tls/tortls_st.h @@ -67,6 +67,8 @@ struct tor_tls_t { */ unsigned long last_write_count; unsigned long last_read_count; + /** Most recent error value from ERR_get_error(). */ + unsigned long last_error; /** If set, a callback to invoke whenever the client tries to renegotiate * the handshake. */ void (*negotiated_callback)(tor_tls_t *tls, void *arg); @@ -77,6 +79,7 @@ struct tor_tls_t { /** Last values retried from tor_get_prfiledesc_byte_counts(). */ uint64_t last_write_count; uint64_t last_read_count; + long last_error; #endif };

1 0

[tor/release-0.4.4] Merge branch 'maint-0.4.4' into release-0.4.4
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 30e2652216eabd7c5c35748cf6e71de9f69686f1 Merge: df246794f 18fa53fdf Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 10:15:04 2020 -0400 Merge branch 'maint-0.4.4' into release-0.4.4 changes/ticket32622 | 5 +++++ src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 18 +++++++++++++----- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 9 files changed, 62 insertions(+), 6 deletions(-)

1 0

[tor/release-0.4.4] Merge branch 'ticket32622_044_squashed' into maint-0.4.4
by nickm＠torproject.org 02 Jul '20

02 Jul '20

commit 18fa53fdf4e2b35ceee54c317e989bdbc31183d5 Merge: 8da10e823 65b05b12a Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 2 10:14:55 2020 -0400 Merge branch 'ticket32622_044_squashed' into maint-0.4.4 changes/ticket32622 | 5 +++++ src/core/mainloop/connection.c | 1 + src/core/or/connection_or.c | 18 +++++++++++++----- src/core/or/or.h | 3 ++- src/core/or/reasons.c | 4 ++++ src/lib/tls/tortls.h | 1 + src/lib/tls/tortls_nss.c | 13 +++++++++++++ src/lib/tls/tortls_openssl.c | 20 ++++++++++++++++++++ src/lib/tls/tortls_st.h | 3 +++ 9 files changed, 62 insertions(+), 6 deletions(-)

1 0