From 5e9d2f57c82a57307a48afea09aa539b9fa80abf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 11:36:24 -0400 Subject: MAINT-5011: Use LLTHROW() instead of plain BOOST_THROW_EXCEPTION(). A level of preprocessor indirection lets us later change the implementation if desired. --- indra/newview/llsechandler_basic.cpp | 54 ++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 39ce64ad0e..183a625382 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -35,7 +35,7 @@ #include "llfile.h" #include "lldir.h" #include "llviewercontrol.h" -#include +#include "llexception.h" #include #include #include @@ -73,14 +73,14 @@ LLBasicCertificate::LLBasicCertificate(const std::string& pem_cert) if(pem_bio == NULL) { LL_WARNS("SECAPI") << "Could not allocate an openssl memory BIO." << LL_ENDL; - BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); + LLTHROW(LLInvalidCertificate(this)); } mCert = NULL; PEM_read_bio_X509(pem_bio, &mCert, 0, NULL); BIO_free(pem_bio); if (!mCert) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); + LLTHROW(LLInvalidCertificate(this)); } } @@ -89,7 +89,7 @@ LLBasicCertificate::LLBasicCertificate(X509* pCert) { if (!pCert || !pCert->cert_info) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); + LLTHROW(LLInvalidCertificate(this)); } mCert = X509_dup(pCert); } @@ -874,22 +874,22 @@ void _validateCert(int validation_policy, // check basic properties exist in the cert if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info.has(CERT_SUBJECT_NAME_STRING)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have a Subject Name")); + LLTHROW(LLCertException(cert, "Cert doesn't have a Subject Name")); } if(!current_cert_info.has(CERT_ISSUER_NAME_STRING)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have an Issuer Name")); + LLTHROW(LLCertException(cert, "Cert doesn't have an Issuer Name")); } // check basic properties exist in the cert if(!current_cert_info.has(CERT_VALID_FROM) || !current_cert_info.has(CERT_VALID_TO)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have an expiration period")); + LLTHROW(LLCertException(cert, "Cert doesn't have an expiration period")); } if (!current_cert_info.has(CERT_SHA1_DIGEST)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "No SHA1 digest")); + LLTHROW(LLCertException(cert, "No SHA1 digest")); } if (validation_policy & VALIDATION_POLICY_TIME) @@ -904,7 +904,7 @@ void _validateCert(int validation_policy, if((validation_date < current_cert_info[CERT_VALID_FROM].asDate()) || (validation_date > current_cert_info[CERT_VALID_TO].asDate())) { - BOOST_THROW_EXCEPTION(LLCertValidationExpirationException(cert, validation_date)); + LLTHROW(LLCertValidationExpirationException(cert, validation_date)); } } if (validation_policy & VALIDATION_POLICY_SSL_KU) @@ -915,14 +915,14 @@ void _validateCert(int validation_policy, !(_LLSDArrayIncludesValue(current_cert_info[CERT_KEY_USAGE], LLSD((std::string)CERT_KU_KEY_ENCIPHERMENT))))) { - BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); + LLTHROW(LLCertKeyUsageValidationException(cert)); } // only validate EKU if the cert has it if(current_cert_info.has(CERT_EXTENDED_KEY_USAGE) && current_cert_info[CERT_EXTENDED_KEY_USAGE].isArray() && (!_LLSDArrayIncludesValue(current_cert_info[CERT_EXTENDED_KEY_USAGE], LLSD((std::string)CERT_EKU_SERVER_AUTH)))) { - BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); + LLTHROW(LLCertKeyUsageValidationException(cert)); } } if (validation_policy & VALIDATION_POLICY_CA_KU) @@ -931,7 +931,7 @@ void _validateCert(int validation_policy, (!_LLSDArrayIncludesValue(current_cert_info[CERT_KEY_USAGE], (std::string)CERT_KU_CERT_SIGN))) { - BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); + LLTHROW(LLCertKeyUsageValidationException(cert)); } } @@ -943,13 +943,13 @@ void _validateCert(int validation_policy, if(!current_cert_info[CERT_BASIC_CONSTRAINTS].has(CERT_BASIC_CONSTRAINTS_CA) || !current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_CA]) { - BOOST_THROW_EXCEPTION(LLCertBasicConstraintsValidationException(cert)); + LLTHROW(LLCertBasicConstraintsValidationException(cert)); } if (current_cert_info[CERT_BASIC_CONSTRAINTS].has(CERT_BASIC_CONSTRAINTS_PATHLEN) && ((current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_PATHLEN].asInteger() != 0) && (depth > current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_PATHLEN].asInteger()))) { - BOOST_THROW_EXCEPTION(LLCertBasicConstraintsValidationException(cert)); + LLTHROW(LLCertBasicConstraintsValidationException(cert)); } } } @@ -1019,7 +1019,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(cert_chain->size() < 1) { - BOOST_THROW_EXCEPTION(LLCertException(NULL, "No certs in chain")); + LLTHROW(LLCertException(NULL, "No certs in chain")); } iterator current_cert = cert_chain->begin(); LLSD current_cert_info; @@ -1034,11 +1034,11 @@ void LLBasicCertificateStore::validate(int validation_policy, (*current_cert)->getLLSD(current_cert_info); if(!validation_params.has(CERT_HOSTNAME)) { - BOOST_THROW_EXCEPTION(LLCertException((*current_cert), "No hostname passed in for validation")); + LLTHROW(LLCertException((*current_cert), "No hostname passed in for validation")); } if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info[CERT_SUBJECT_NAME].has(CERT_NAME_CN)) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate((*current_cert))); + LLTHROW(LLInvalidCertificate((*current_cert))); } LL_DEBUGS("SECAPI") << "Validating the hostname " << validation_params[CERT_HOSTNAME].asString() << @@ -1055,7 +1055,7 @@ void LLBasicCertificateStore::validate(int validation_policy, X509* cert_x509 = (*current_cert)->getOpenSSLX509(); if(!cert_x509) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate((*current_cert))); + LLTHROW(LLInvalidCertificate((*current_cert))); } std::string sha1_hash((const char *)cert_x509->sha1_hash, SHA_DIGEST_LENGTH); X509_free( cert_x509 ); @@ -1076,7 +1076,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if((validation_date < cache_entry->second.first) || (validation_date > cache_entry->second.second)) { - BOOST_THROW_EXCEPTION(LLCertValidationExpirationException((*current_cert), validation_date)); + LLTHROW(LLCertValidationExpirationException((*current_cert), validation_date)); } } // successfully found in cache @@ -1108,7 +1108,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(!_verify_signature((*current_cert), previous_cert)) { - BOOST_THROW_EXCEPTION(LLCertValidationInvalidSignatureException(previous_cert)); + LLTHROW(LLCertValidationInvalidSignatureException(previous_cert)); } } _validateCert(local_validation_policy, @@ -1157,7 +1157,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(!_verify_signature((*found_store_cert), (*current_cert))) { - BOOST_THROW_EXCEPTION(LLCertValidationInvalidSignatureException(*current_cert)); + LLTHROW(LLCertValidationInvalidSignatureException(*current_cert)); } // successfully validated. mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); @@ -1174,7 +1174,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if (validation_policy & VALIDATION_POLICY_TRUSTED) { // we reached the end without finding a trusted cert. - BOOST_THROW_EXCEPTION(LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1])); + LLTHROW(LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1])); } mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); @@ -1262,7 +1262,7 @@ void LLSecAPIBasicHandler::_readProtectedData() protected_data_stream.read((char *)salt, STORE_SALT_SIZE); if (protected_data_stream.gcount() < STORE_SALT_SIZE) { - BOOST_THROW_EXCEPTION(LLProtectedDataException("Config file too short.")); + LLTHROW(LLProtectedDataException("Config file too short.")); } cipher.decrypt(salt, STORE_SALT_SIZE); @@ -1302,7 +1302,7 @@ void LLSecAPIBasicHandler::_readProtectedData() if (parser->parse(parse_stream, mProtectedDataMap, LLSDSerialize::SIZE_UNLIMITED) == LLSDParser::PARSE_FAILURE) { - BOOST_THROW_EXCEPTION(LLProtectedDataException("Config file cannot be decrypted.")); + LLTHROW(LLProtectedDataException("Config file cannot be decrypted.")); } } } @@ -1373,7 +1373,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //BOOST_THROW_EXCEPTION(LLProtectedDataException("Error writing Protected Data Store")); + //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } try @@ -1388,7 +1388,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //BOOST_THROW_EXCEPTION(LLProtectedDataException("Could not overwrite protected data store")); + //LLTHROW(LLProtectedDataException("Could not overwrite protected data store")); } } catch (...) @@ -1402,7 +1402,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() //crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //BOOST_THROW_EXCEPTION(LLProtectedDataException("Error writing Protected Data Store")); + //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } } -- cgit v1.2.3 From 993f54f6e91d78a9c2e1389ad878d6bd46e9be5b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 15:40:03 -0400 Subject: MAINT-5011: Try to enrich catch (...) logging throughout viewer. Turns out we have a surprising number of catch (...) clauses in the viewer code base. If all we currently do is LL_ERRS() << "unknown exception" << LL_ENDL; then call CRASH_ON_UNHANDLED_EXCEPTION() instead. If what we do is LL_WARNS() << "unknown exception" << LL_ENDL; then call LOG_UNHANDLED_EXCEPTION() instead. Since many places need LOG_UNHANDLED_EXCEPTION() and nobody catches LLContinueError yet, eliminate LLContinueError& parameter from LOG_UNHANDLED_EXCEPTION(). This permits us to use the same log message as CRASH_ON_UNHANDLED_EXCEPTION(), just with a different severity level. Where a catch (...) clause actually provides contextual information, or makes an error string, add boost::current_exception_diagnostic_information() to try to figure out actual exception class and message. --- indra/newview/llsechandler_basic.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 183a625382..54396cb9a4 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -38,6 +38,7 @@ #include "llexception.h" #include #include +#include #include #include #include @@ -618,7 +619,8 @@ void LLBasicCertificateStore::load_from_file(const std::string& filename) } catch (...) { - LL_WARNS("SECAPI") << "Failure creating certificate from the certificate store file." << LL_ENDL; + LL_WARNS("SECAPI") << "Failure creating certificate from the certificate store file: " + << boost::current_exception_diagnostic_information() << LL_ENDL; } X509_free(cert_x509); cert_x509 = NULL; @@ -1365,7 +1367,8 @@ void LLSecAPIBasicHandler::_writeProtectedData() } catch (...) { - LL_WARNS() << "LLProtectedDataException(Error writing Protected Data Store)" << LL_ENDL; + LL_WARNS() << "LLProtectedDataException(Error writing Protected Data Store): " + << boost::current_exception_diagnostic_information() << LL_ENDL; // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future // it may be, however. @@ -1394,7 +1397,8 @@ void LLSecAPIBasicHandler::_writeProtectedData() catch (...) { LL_WARNS() << "LLProtectedDataException(Error renaming '" << tmp_filename - << "' to '" << mProtectedDataFilename << "')" << LL_ENDL; + << "' to '" << mProtectedDataFilename << "'): " + << boost::current_exception_diagnostic_information() << LL_ENDL; // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future // it may be, however. -- cgit v1.2.3 From 4d10172d8b2c72fa809e322a3b4ff326b19ff340 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 18 Aug 2016 17:33:44 -0400 Subject: MAINT-5011: Catch unhandled exceptions in LLCoros coroutines. Wrap coroutine call in try/catch in top-level coroutine wrapper function LLCoros::toplevel(). Distinguish exception classes derived from LLContinueError (log and continue) from all others (crash with LL_ERRS). Enhance CRASH_ON_UNHANDLED_EXCEPTIONS() and LOG_UNHANDLED_EXCEPTIONS() macros to accept a context string to supplement the log message. This lets us replace many places that called boost::current_exception_diagnostic_information() with LOG_UNHANDLED_EXCEPTIONS() instead, since the explicit calls were mostly to log supplemental information. Provide supplemental information (coroutine name, function parameters) for some of the previous LOG_UNHANDLED_EXCEPTIONS() calls. This information duplicates LL_DEBUGS() information at the top of these functions, but in a typical log file we wouldn't see the LL_DEBUGS() message. Eliminate a few catch (std::exception e) clauses: the information we get from boost::current_exception_diagnostic_information() in a catch (...) clause makes it unnecessary to distinguish. In a few cases, add a final 'throw;' to a catch (...) clause: having logged the local context info, propagate the exception to be caught by higher-level try/catch. In a couple places, couldn't resist reconciling indentation within a particular function: tabs where the rest of the function uses tabs, spaces where the rest of the function uses spaces. In LLLogin::Impl::loginCoro(), eliminate some confusing comments about an array of rewritten URIs that date back to a long-deleted implementation. --- indra/newview/llsechandler_basic.cpp | 43 +++++++++++++++++------------------- 1 file changed, 20 insertions(+), 23 deletions(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 54396cb9a4..d6fb801cc0 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -36,9 +36,9 @@ #include "lldir.h" #include "llviewercontrol.h" #include "llexception.h" +#include "stringize.h" #include #include -#include #include #include #include @@ -619,8 +619,7 @@ void LLBasicCertificateStore::load_from_file(const std::string& filename) } catch (...) { - LL_WARNS("SECAPI") << "Failure creating certificate from the certificate store file: " - << boost::current_exception_diagnostic_information() << LL_ENDL; + LOG_UNHANDLED_EXCEPTION("creating certificate from the certificate store file"); } X509_free(cert_x509); cert_x509 = NULL; @@ -1367,8 +1366,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() } catch (...) { - LL_WARNS() << "LLProtectedDataException(Error writing Protected Data Store): " - << boost::current_exception_diagnostic_information() << LL_ENDL; + LOG_UNHANDLED_EXCEPTION("LLProtectedDataException(Error writing Protected Data Store)"); // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future // it may be, however. @@ -1379,29 +1377,28 @@ void LLSecAPIBasicHandler::_writeProtectedData() //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } - try - { - // move the temporary file to the specified file location. - if((( (LLFile::isfile(mProtectedDataFilename) != 0) - && (LLFile::remove(mProtectedDataFilename) != 0))) - || (LLFile::rename(tmp_filename, mProtectedDataFilename))) - { - LL_WARNS() << "LLProtectedDataException(Could not overwrite protected data store)" << LL_ENDL; - LLFile::remove(tmp_filename); + try + { + // move the temporary file to the specified file location. + if((( (LLFile::isfile(mProtectedDataFilename) != 0) + && (LLFile::remove(mProtectedDataFilename) != 0))) + || (LLFile::rename(tmp_filename, mProtectedDataFilename))) + { + LL_WARNS() << "LLProtectedDataException(Could not overwrite protected data store)" << LL_ENDL; + LLFile::remove(tmp_filename); - // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() - // Decided throwing an exception here was overkill until we figure out why this happens - //LLTHROW(LLProtectedDataException("Could not overwrite protected data store")); - } + // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() + // Decided throwing an exception here was overkill until we figure out why this happens + //LLTHROW(LLProtectedDataException("Could not overwrite protected data store")); + } } catch (...) { - LL_WARNS() << "LLProtectedDataException(Error renaming '" << tmp_filename - << "' to '" << mProtectedDataFilename << "'): " - << boost::current_exception_diagnostic_information() << LL_ENDL; + LOG_UNHANDLED_EXCEPTION(STRINGIZE("renaming '" << tmp_filename << "' to '" + << mProtectedDataFilename << "'")); // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future - // it may be, however. + // it may be, however). LLFile::remove(tmp_filename); //crash in LLSecAPIBasicHandler::_writeProtectedData() @@ -1409,7 +1406,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } } - + // instantiate a certificate from a pem string LLPointer LLSecAPIBasicHandler::getCertificate(const std::string& pem_cert) { -- cgit v1.2.3