Check more thoroughly for unlogged OpenSSL errors

This commit is contained in:
Nick Mathewson 2014-11-02 13:04:44 -05:00
parent a142fc29af
commit 5bcf952261
2 changed files with 47 additions and 12 deletions

4
changes/bug13319 Normal file
View File

@ -0,0 +1,4 @@
o Minor bugfixes:
- Check more thoroughly throughout the TLS code for possible unlogged
TLS errors. Possible diagnostic or fix for bug 13319.

View File

@ -452,6 +452,8 @@ tor_tls_get_error(tor_tls_t *tls, int r, int extra,
static void static void
tor_tls_init(void) tor_tls_init(void)
{ {
check_no_tls_errors();
if (!tls_library_is_initialized) { if (!tls_library_is_initialized) {
long version; long version;
SSL_library_init(); SSL_library_init();
@ -550,6 +552,8 @@ tor_tls_init(void)
void void
tor_tls_free_all(void) tor_tls_free_all(void)
{ {
check_no_tls_errors();
if (server_tls_context) { if (server_tls_context) {
tor_tls_context_t *ctx = server_tls_context; tor_tls_context_t *ctx = server_tls_context;
server_tls_context = NULL; server_tls_context = NULL;
@ -861,29 +865,33 @@ tor_cert_decode(const uint8_t *certificate, size_t certificate_len)
const unsigned char *cp = (const unsigned char *)certificate; const unsigned char *cp = (const unsigned char *)certificate;
tor_cert_t *newcert; tor_cert_t *newcert;
tor_assert(certificate); tor_assert(certificate);
check_no_tls_errors();
if (certificate_len > INT_MAX) if (certificate_len > INT_MAX)
return NULL; goto err;
x509 = d2i_X509(NULL, &cp, (int)certificate_len); x509 = d2i_X509(NULL, &cp, (int)certificate_len);
if (!x509) if (!x509)
return NULL; /* Couldn't decode */ goto err; /* Couldn't decode */
if (cp - certificate != (int)certificate_len) { if (cp - certificate != (int)certificate_len) {
X509_free(x509); X509_free(x509);
return NULL; /* Didn't use all the bytes */ goto err; /* Didn't use all the bytes */
} }
newcert = tor_cert_new(x509); newcert = tor_cert_new(x509);
if (!newcert) { if (!newcert) {
return NULL; goto err;
} }
if (newcert->encoded_len != certificate_len || if (newcert->encoded_len != certificate_len ||
fast_memneq(newcert->encoded, certificate, certificate_len)) { fast_memneq(newcert->encoded, certificate, certificate_len)) {
/* Cert wasn't in DER */ /* Cert wasn't in DER */
tor_cert_free(newcert); tor_cert_free(newcert);
return NULL; goto err;
} }
return newcert; return newcert;
err:
tls_log_errors(NULL, LOG_INFO, LD_CRYPTO, "decoding a certificate");
return NULL;
} }
/** Set *<b>encoded_out</b> and *<b>size_out</b> to <b>cert</b>'s encoded DER /** Set *<b>encoded_out</b> and *<b>size_out</b> to <b>cert</b>'s encoded DER
@ -1025,21 +1033,24 @@ tor_tls_cert_is_valid(int severity,
const tor_cert_t *signing_cert, const tor_cert_t *signing_cert,
int check_rsa_1024) int check_rsa_1024)
{ {
check_no_tls_errors();
EVP_PKEY *cert_key; EVP_PKEY *cert_key;
EVP_PKEY *signing_key = X509_get_pubkey(signing_cert->cert); EVP_PKEY *signing_key = X509_get_pubkey(signing_cert->cert);
int r, key_ok = 0; int r, key_ok = 0;
if (!signing_key) if (!signing_key)
return 0; goto bad;
r = X509_verify(cert->cert, signing_key); r = X509_verify(cert->cert, signing_key);
EVP_PKEY_free(signing_key); EVP_PKEY_free(signing_key);
if (r <= 0) if (r <= 0)
return 0; goto bad;
/* okay, the signature checked out right. Now let's check the check the /* okay, the signature checked out right. Now let's check the check the
* lifetime. */ * lifetime. */
if (check_cert_lifetime_internal(severity, cert->cert, if (check_cert_lifetime_internal(severity, cert->cert,
48*60*60, 30*24*60*60) < 0) 48*60*60, 30*24*60*60) < 0)
return 0; goto bad;
cert_key = X509_get_pubkey(cert->cert); cert_key = X509_get_pubkey(cert->cert);
if (check_rsa_1024 && cert_key) { if (check_rsa_1024 && cert_key) {
@ -1059,11 +1070,14 @@ tor_tls_cert_is_valid(int severity,
} }
EVP_PKEY_free(cert_key); EVP_PKEY_free(cert_key);
if (!key_ok) if (!key_ok)
return 0; goto bad;
/* XXXX compare DNs or anything? */ /* XXXX compare DNs or anything? */
return 1; return 1;
bad:
tls_log_errors(NULL, LOG_INFO, LD_CRYPTO, "checking a certificate");
return 0;
} }
/** Increase the reference count of <b>ctx</b>. */ /** Increase the reference count of <b>ctx</b>. */
@ -1090,6 +1104,7 @@ tor_tls_context_init(unsigned flags,
int rv1 = 0; int rv1 = 0;
int rv2 = 0; int rv2 = 0;
const int is_public_server = flags & TOR_TLS_CTX_IS_PUBLIC_SERVER; const int is_public_server = flags & TOR_TLS_CTX_IS_PUBLIC_SERVER;
check_no_tls_errors();
if (is_public_server) { if (is_public_server) {
tor_tls_context_t *new_ctx; tor_tls_context_t *new_ctx;
@ -1134,6 +1149,7 @@ tor_tls_context_init(unsigned flags,
1); 1);
} }
tls_log_errors(NULL, LOG_WARN, LD_CRYPTO, "constructing a TLS context");
return MIN(rv1, rv2); return MIN(rv1, rv2);
} }
@ -1857,11 +1873,12 @@ tor_tls_new(int sock, int isServer)
client_tls_context; client_tls_context;
result->magic = TOR_TLS_MAGIC; result->magic = TOR_TLS_MAGIC;
check_no_tls_errors();
tor_assert(context); /* make sure somebody made it first */ tor_assert(context); /* make sure somebody made it first */
if (!(result->ssl = SSL_new(context->ctx))) { if (!(result->ssl = SSL_new(context->ctx))) {
tls_log_errors(NULL, LOG_WARN, LD_NET, "creating SSL object"); tls_log_errors(NULL, LOG_WARN, LD_NET, "creating SSL object");
tor_free(result); tor_free(result);
return NULL; goto err;
} }
#ifdef SSL_set_tlsext_host_name #ifdef SSL_set_tlsext_host_name
@ -1881,7 +1898,7 @@ tor_tls_new(int sock, int isServer)
#endif #endif
SSL_free(result->ssl); SSL_free(result->ssl);
tor_free(result); tor_free(result);
return NULL; goto err;
} }
if (!isServer) if (!isServer)
rectify_client_ciphers(&result->ssl->cipher_list); rectify_client_ciphers(&result->ssl->cipher_list);
@ -1894,7 +1911,7 @@ tor_tls_new(int sock, int isServer)
#endif #endif
SSL_free(result->ssl); SSL_free(result->ssl);
tor_free(result); tor_free(result);
return NULL; goto err;
} }
{ {
int set_worked = int set_worked =
@ -1928,6 +1945,10 @@ tor_tls_new(int sock, int isServer)
if (isServer) if (isServer)
tor_tls_setup_session_secret_cb(result); tor_tls_setup_session_secret_cb(result);
goto done;
err:
result = NULL;
done:
/* Not expected to get called. */ /* Not expected to get called. */
tls_log_errors(NULL, LOG_WARN, LD_NET, "creating tor_tls_t object"); tls_log_errors(NULL, LOG_WARN, LD_NET, "creating tor_tls_t object");
return result; return result;
@ -2175,6 +2196,7 @@ int
tor_tls_finish_handshake(tor_tls_t *tls) tor_tls_finish_handshake(tor_tls_t *tls)
{ {
int r = TOR_TLS_DONE; int r = TOR_TLS_DONE;
check_no_tls_errors();
if (tls->isServer) { if (tls->isServer) {
SSL_set_info_callback(tls->ssl, NULL); SSL_set_info_callback(tls->ssl, NULL);
SSL_set_verify(tls->ssl, SSL_VERIFY_PEER, always_accept_verify_cb); SSL_set_verify(tls->ssl, SSL_VERIFY_PEER, always_accept_verify_cb);
@ -2220,6 +2242,7 @@ tor_tls_finish_handshake(tor_tls_t *tls)
r = TOR_TLS_ERROR_MISC; r = TOR_TLS_ERROR_MISC;
} }
} }
tls_log_errors(NULL, LOG_WARN, LD_NET, "finishing the handshake");
return r; return r;
} }
@ -2250,6 +2273,8 @@ tor_tls_renegotiate(tor_tls_t *tls)
/* We could do server-initiated renegotiation too, but that would be tricky. /* We could do server-initiated renegotiation too, but that would be tricky.
* Instead of "SSL_renegotiate, then SSL_do_handshake until done" */ * Instead of "SSL_renegotiate, then SSL_do_handshake until done" */
tor_assert(!tls->isServer); tor_assert(!tls->isServer);
check_no_tls_errors();
if (tls->state != TOR_TLS_ST_RENEGOTIATE) { if (tls->state != TOR_TLS_ST_RENEGOTIATE) {
int r = SSL_renegotiate(tls->ssl); int r = SSL_renegotiate(tls->ssl);
if (r <= 0) { if (r <= 0) {
@ -2278,6 +2303,7 @@ tor_tls_shutdown(tor_tls_t *tls)
char buf[128]; char buf[128];
tor_assert(tls); tor_assert(tls);
tor_assert(tls->ssl); tor_assert(tls->ssl);
check_no_tls_errors();
while (1) { while (1) {
if (tls->state == TOR_TLS_ST_SENTCLOSE) { if (tls->state == TOR_TLS_ST_SENTCLOSE) {
@ -2465,6 +2491,7 @@ tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity_key)
RSA *rsa; RSA *rsa;
int r = -1; int r = -1;
check_no_tls_errors();
*identity_key = NULL; *identity_key = NULL;
try_to_extract_certs_from_tls(severity, tls, &cert, &id_cert); try_to_extract_certs_from_tls(severity, tls, &cert, &id_cert);
@ -2705,6 +2732,8 @@ dn_indicates_v3_cert(X509_NAME *name)
int int
tor_tls_received_v3_certificate(tor_tls_t *tls) tor_tls_received_v3_certificate(tor_tls_t *tls)
{ {
check_no_tls_errors();
X509 *cert = SSL_get_peer_certificate(tls->ssl); X509 *cert = SSL_get_peer_certificate(tls->ssl);
EVP_PKEY *key = NULL; EVP_PKEY *key = NULL;
X509_NAME *issuer_name, *subject_name; X509_NAME *issuer_name, *subject_name;
@ -2737,6 +2766,8 @@ tor_tls_received_v3_certificate(tor_tls_t *tls)
} }
done: done:
tls_log_errors(tls, LOG_WARN, LD_NET, "checking for a v3 cert");
if (key) if (key)
EVP_PKEY_free(key); EVP_PKEY_free(key);
if (cert) if (cert)