From 5827e2e216ad759f240a0c332848cadf65794742 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 23 Apr 2005 14:26:02 +0000 Subject: [PATCH] Fix "JAP-client" hideous ASN1 bug, twice. (Fix1: check more thoroughly for TLS errors when handling certs. Fix2: stop assert(0)ing on uncaught TLS errors.) svn:r4085 --- src/common/tortls.c | 32 ++++++++++++++++++++++---------- src/common/tortls.h | 4 ++-- src/or/buffers.c | 3 ++- src/or/connection_or.c | 5 +++++ 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 4eb4488193..f0e57e5455 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -251,12 +251,12 @@ tor_tls_create_certificate(crypto_pk_env_t *rsa, goto done; error: - tls_log_errors(LOG_WARN, "generating certificate"); if (x509) { X509_free(x509); x509 = NULL; } done: + tls_log_errors(LOG_WARN, "generating certificate"); if (sign_pkey) EVP_PKEY_free(sign_pkey); if (pkey) @@ -421,13 +421,18 @@ tor_tls_new(int sock, int isServer, int use_no_cert) tor_assert(global_tls_context); /* make sure somebody made it first */ ctx = use_no_cert ? global_tls_context->client_only_ctx : global_tls_context->ctx; - if (!(result->ssl = SSL_new(ctx))) + if (!(result->ssl = SSL_new(ctx))) { + tls_log_errors(LOG_WARN, "generating TLS context"); + tor_free(result); return NULL; + } result->socket = sock; SSL_set_fd(result->ssl, sock); result->state = TOR_TLS_ST_HANDSHAKE; result->isServer = isServer; result->wantwrite_n = 0; + /* Not expected to get called. */ + tls_log_errors(LOG_WARN, "generating TLS context"); return result; } @@ -603,7 +608,9 @@ int tor_tls_peer_has_cert(tor_tls *tls) { X509 *cert; - if (!(cert = SSL_get_peer_certificate(tls->ssl))) + cert = SSL_get_peer_certificate(tls->ssl); + tls_log_errors(LOG_WARN, "getting peer certificate"); + if (!cert) return 0; X509_free(cert); return 1; @@ -621,6 +628,7 @@ tor_tls_get_peer_cert_nickname(tor_tls *tls, char *buf, size_t buflen) X509_NAME *name = NULL; int nid; int lenout; + int r = -1; if (!(cert = SSL_get_peer_certificate(tls->ssl))) { log_fn(LOG_WARN, "Peer has no certificate"); @@ -643,13 +651,15 @@ tor_tls_get_peer_cert_nickname(tor_tls *tls, char *buf, size_t buflen) log_fn(LOG_WARN, " (Maybe it is not really running Tor at its advertised OR port.)"); goto error; } - X509_free(cert); - return 0; + r = 0; + error: if (cert) X509_free(cert); - return -1; + + tls_log_errors(LOG_WARN, "getting peer certificate nickname"); + return r; } static void log_cert_lifetime(X509 *cert, const char *problem) @@ -688,6 +698,8 @@ static void log_cert_lifetime(X509 *cert, const char *problem) log_fn(LOG_WARN, "(certificate lifetime runs from %s through %s. Your time is %s.)",s1,s2,mytime); end: + /* Not expected to get invoked */ + tls_log_errors(LOG_WARN, "getting certificate lifetime"); if (bio) BIO_free(bio); if (s1) @@ -797,6 +809,8 @@ tor_tls_check_lifetime(tor_tls *tls, int tolerance) done: if (cert) X509_free(cert); + /* Not expected to get invoked */ + tls_log_errors(LOG_WARN, "checking certificate lifetime"); return r; } @@ -830,16 +844,14 @@ unsigned long tor_tls_get_n_bytes_written(tor_tls *tls) return BIO_number_written(SSL_get_wbio(tls->ssl)); } -/** Implement assert_no_tls_errors: If there are any pending OpenSSL +/** Implement check_no_tls_errors: If there are any pending OpenSSL * errors, log an error message and assert(0). */ -void _assert_no_tls_errors(const char *fname, int line) +void _check_no_tls_errors(const char *fname, int line) { if (ERR_peek_error() == 0) return; log_fn(LOG_ERR, "Unhandled OpenSSL errors found at %s:%d: ", fname, line); tls_log_errors(LOG_ERR, NULL); - - tor_assert(0); } diff --git a/src/common/tortls.h b/src/common/tortls.h index 2d568a9807..4d8aba264b 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -46,9 +46,9 @@ unsigned long tor_tls_get_n_bytes_written(tor_tls *tls); /* Log and abort if there are unhandled TLS errors in OpenSSL's error stack. */ -#define assert_no_tls_errors() _assert_no_tls_errors(_SHORT_FILE_,__LINE__) +#define check_no_tls_errors() _check_no_tls_errors(_SHORT_FILE_,__LINE__) -void _assert_no_tls_errors(const char *fname, int line); +void _check_no_tls_errors(const char *fname, int line); #endif diff --git a/src/or/buffers.c b/src/or/buffers.c index fe6b8d739d..7edc2cdf34 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -224,7 +224,7 @@ int read_to_buf_tls(tor_tls *tls, size_t at_most, buf_t *buf) { (int)buf_datalen(buf), (int)tor_tls_get_pending_bytes(tls), (int)at_most); - assert_no_tls_errors(); + check_no_tls_errors(); r = tor_tls_read(tls, buf->mem+buf->datalen, at_most); if (r<0) return r; @@ -281,6 +281,7 @@ int flush_buf_tls(tor_tls *tls, buf_t *buf, size_t *buf_flushlen) /* we want to let tls write even if flushlen is zero, because it might * have a partial record pending */ + check_no_tls_errors(); r = tor_tls_write(tls, buf->mem, *buf_flushlen); if (r < 0) { return r; diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 4f8943e60a..4c9163e696 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -378,6 +378,7 @@ int connection_tls_start_handshake(connection_t *conn, int receiving) { * Return -1 if conn is broken, else return 0. */ int connection_tls_continue_handshake(connection_t *conn) { + check_no_tls_errors(); switch (tor_tls_handshake(conn->tls)) { case TOR_TLS_ERROR: case TOR_TLS_CLOSE: @@ -442,16 +443,19 @@ connection_tls_finish_handshake(connection_t *conn) { conn->state = OR_CONN_STATE_OPEN; connection_watch_events(conn, EV_READ); log_fn(LOG_DEBUG,"tls handshake done. verifying."); + check_no_tls_errors(); if (! tor_tls_peer_has_cert(conn->tls)) { log_fn(LOG_INFO,"Peer didn't send a cert! Closing."); /* XXX we should handle this case rather than just closing. */ return -1; } + check_no_tls_errors(); if (tor_tls_get_peer_cert_nickname(conn->tls, nickname, sizeof(nickname))) { log_fn(LOG_WARN,"Other side (%s:%d) has a cert without a valid nickname. Closing.", conn->address, conn->port); return -1; } + check_no_tls_errors(); log_fn(LOG_DEBUG, "Other side (%s:%d) claims to be router '%s'", conn->address, conn->port, nickname); @@ -460,6 +464,7 @@ connection_tls_finish_handshake(connection_t *conn) { nickname, conn->address, conn->port); return -1; } + check_no_tls_errors(); #if 0 if (tor_tls_check_lifetime(conn->tls, LOOSE_CERT_ALLOW_SKEW)<0) { log_fn(LOG_WARN,"Other side '%s' (%s:%d) has a very highly skewed clock, or an expired certificate. Closing.",