From 1ad46b5cd0fcac0d3224d37d555092258593eabd Mon Sep 17 00:00:00 2001 From: Roxie Linden Date: Mon, 24 May 2010 13:59:10 -0700 Subject: DEV-50173 - investigate certificate code performance DEV-50166 - LLBasicCertificateChain::validate calls in log Added caching of certificates that have been validated. The sha1 hash for the certificate is stored and is associated with the from and to times. When the certificate is validated, the code determines whether the certificate has successfully been validated before by looking for it in the cache, and then checks the date of the cert. If that is successful, the validation calls with success. Otherwise, it proceeds to do a full validation of the certificate. --- indra/newview/llsechandler_basic.cpp | 114 +++++++++++++++++++++++++---------- 1 file changed, 81 insertions(+), 33 deletions(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index edf5ce9b60..84ab9b9175 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -85,7 +85,6 @@ LLBasicCertificate::LLBasicCertificate(const std::string& pem_cert) { throw LLInvalidCertificate(this); } - _initLLSD(); } @@ -96,7 +95,6 @@ LLBasicCertificate::LLBasicCertificate(X509* pCert) throw LLInvalidCertificate(this); } mCert = X509_dup(pCert); - _initLLSD(); } LLBasicCertificate::~LLBasicCertificate() @@ -150,9 +148,13 @@ std::vector LLBasicCertificate::getBinary() const } -LLSD LLBasicCertificate::getLLSD() const +void LLBasicCertificate::getLLSD(LLSD &llsd) { - return mLLSDInfo; + if (mLLSDInfo.isUndefined()) + { + _initLLSD(); + } + llsd = mLLSDInfo; } // Initialize the LLSD info for the certificate @@ -516,8 +518,9 @@ LLBasicCertificateVector::iterator LLBasicCertificateVector::find(const LLSD& pa cert++) { - found= TRUE; - LLSD cert_info = (*cert)->getLLSD(); + found= TRUE; + LLSD cert_info; + (*cert)->getLLSD(cert_info); for (LLSD::map_const_iterator param = params.beginMap(); param != params.endMap(); param++) @@ -543,7 +546,8 @@ LLBasicCertificateVector::iterator LLBasicCertificateVector::find(const LLSD& pa void LLBasicCertificateVector::insert(iterator _iter, LLPointer cert) { - LLSD cert_info = cert->getLLSD(); + LLSD cert_info; + cert->getLLSD(cert_info); if (cert_info.isMap() && cert_info.has(CERT_SHA1_DIGEST)) { LLSD existing_cert_info = LLSD::emptyMap(); @@ -691,7 +695,8 @@ LLBasicCertificateChain::LLBasicCertificateChain(const X509_STORE_CTX* store) while(untrusted_certs.size() > 0) { LLSD find_data = LLSD::emptyMap(); - LLSD cert_data = current->getLLSD(); + LLSD cert_data; + current->getLLSD(cert_data); // we simply build the chain via subject/issuer name as the // client should not have passed in multiple CA's with the same // subject name. If they did, it'll come out in the wash during @@ -850,12 +855,13 @@ bool _LLSDArrayIncludesValue(const LLSD& llsd_set, LLSD llsd_value) } void _validateCert(int validation_policy, - const LLPointer cert, + LLPointer cert, const LLSD& validation_params, int depth) { - LLSD current_cert_info = cert->getLLSD(); + LLSD current_cert_info; + cert->getLLSD(current_cert_info); // check basic properties exist in the cert if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info.has(CERT_SUBJECT_NAME_STRING)) { @@ -943,8 +949,9 @@ bool _verify_signature(LLPointer parent, LLPointer child) { bool verify_result = FALSE; - LLSD cert1 = parent->getLLSD(); - LLSD cert2 = child->getLLSD(); + LLSD cert1, cert2; + parent->getLLSD(cert1); + child->getLLSD(cert2); X509 *signing_cert = parent->getOpenSSLX509(); X509 *child_cert = child->getOpenSSLX509(); if((signing_cert != NULL) && (child_cert != NULL)) @@ -979,6 +986,7 @@ bool _verify_signature(LLPointer parent, return verify_result; } + // validate the certificate chain against a store. // There are many aspects of cert validatioin policy involved in // trust validation. The policies in this validation algorithm include @@ -993,8 +1001,8 @@ bool _verify_signature(LLPointer parent, // and verify the last cert is in the certificate store, or points // to a cert in the store. It validates whether any cert in the chain // is trusted in the store, even if it's not the last one. -void LLBasicCertificateChain::validate(int validation_policy, - LLPointer ca_store, +void LLBasicCertificateStore::validate(int validation_policy, + LLPointer cert_chain, const LLSD& validation_params) { @@ -1002,8 +1010,8 @@ void LLBasicCertificateChain::validate(int validation_policy, { throw LLCertException(NULL, "No certs in chain"); } - iterator current_cert = begin(); - LLSD current_cert_info = (*current_cert)->getLLSD(); + iterator current_cert = cert_chain->begin(); + LLSD current_cert_info; LLSD validation_date; if (validation_params.has(CERT_VALIDATION_DATE)) { @@ -1012,6 +1020,7 @@ void LLBasicCertificateChain::validate(int validation_policy, if (validation_policy & VALIDATION_POLICY_HOSTNAME) { + (*current_cert)->getLLSD(current_cert_info); if(!validation_params.has(CERT_HOSTNAME)) { throw LLCertException((*current_cert), "No hostname passed in for validation"); @@ -1021,7 +1030,7 @@ void LLBasicCertificateChain::validate(int validation_policy, throw LLInvalidCertificate((*current_cert)); } - LL_INFOS("SECAPI") << "Validating the hostname " << validation_params[CERT_HOSTNAME].asString() << + LL_DEBUGS("SECAPI") << "Validating the hostname " << validation_params[CERT_HOSTNAME].asString() << "against the cert CN " << current_cert_info[CERT_SUBJECT_NAME][CERT_NAME_CN].asString() << LL_ENDL; if(!_cert_hostname_wildcard_match(validation_params[CERT_HOSTNAME].asString(), current_cert_info[CERT_SUBJECT_NAME][CERT_NAME_CN].asString())) @@ -1030,16 +1039,50 @@ void LLBasicCertificateChain::validate(int validation_policy, (*current_cert)); } } - + // check the cache of already validated certs + X509* cert_x509 = (*current_cert)->getOpenSSLX509(); + if(!cert_x509) + { + throw LLInvalidCertificate((*current_cert)); + } + std::string sha1_hash((const char *)cert_x509->sha1_hash, SHA_DIGEST_LENGTH); + t_cert_cache::iterator cache_entry = mTrustedCertCache.find(sha1_hash); + if(cache_entry != mTrustedCertCache.end()) + { + LL_DEBUGS("SECAPI") << "Found cert in cache" << LL_ENDL; + // this cert is in the cache, so validate the time. + if (validation_policy & VALIDATION_POLICY_TIME) + { + LLDate validation_date(time(NULL)); + if(validation_params.has(CERT_VALIDATION_DATE)) + { + validation_date = validation_params[CERT_VALIDATION_DATE]; + } + + if((validation_date < cache_entry->second.first) || + (validation_date > cache_entry->second.second)) + { + throw LLCertValidationExpirationException((*current_cert), validation_date); + } + } + // successfully found in cache + return; + } + if(current_cert_info.isUndefined()) + { + (*current_cert)->getLLSD(current_cert_info); + } + LLDate from_time = current_cert_info[CERT_VALID_FROM].asDate(); + LLDate to_time = current_cert_info[CERT_VALID_TO].asDate(); int depth = 0; LLPointer previous_cert; // loop through the cert chain, validating the current cert against the next one. - while(current_cert != end()) + while(current_cert != cert_chain->end()) { int local_validation_policy = validation_policy; - if(current_cert == begin()) + if(current_cert == cert_chain->begin()) { // for the child cert, we don't validate CA stuff local_validation_policy &= ~(VALIDATION_POLICY_CA_KU | @@ -1061,23 +1104,23 @@ void LLBasicCertificateChain::validate(int validation_policy, depth); // look for a CA in the CA store that may belong to this chain. - LLSD cert_llsd = (*current_cert)->getLLSD(); LLSD cert_search_params = LLSD::emptyMap(); // is the cert itself in the store? - cert_search_params[CERT_SHA1_DIGEST] = cert_llsd[CERT_SHA1_DIGEST]; - LLCertificateStore::iterator found_store_cert = ca_store->find(cert_search_params); - if(found_store_cert != ca_store->end()) + cert_search_params[CERT_SHA1_DIGEST] = current_cert_info[CERT_SHA1_DIGEST]; + LLCertificateStore::iterator found_store_cert = find(cert_search_params); + if(found_store_cert != end()) { + mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); return; } // is the parent in the cert store? cert_search_params = LLSD::emptyMap(); - cert_search_params[CERT_SUBJECT_NAME_STRING] = cert_llsd[CERT_ISSUER_NAME_STRING]; - if (cert_llsd.has(CERT_AUTHORITY_KEY_IDENTIFIER)) + cert_search_params[CERT_SUBJECT_NAME_STRING] = current_cert_info[CERT_ISSUER_NAME_STRING]; + if (current_cert_info.has(CERT_AUTHORITY_KEY_IDENTIFIER)) { - LLSD cert_aki = cert_llsd[CERT_AUTHORITY_KEY_IDENTIFIER]; + LLSD cert_aki = current_cert_info[CERT_AUTHORITY_KEY_IDENTIFIER]; if(cert_aki.has(CERT_AUTHORITY_KEY_IDENTIFIER_ID)) { cert_search_params[CERT_SUBJECT_KEY_IDENTFIER] = cert_aki[CERT_AUTHORITY_KEY_IDENTIFIER_ID]; @@ -1087,11 +1130,10 @@ void LLBasicCertificateChain::validate(int validation_policy, cert_search_params[CERT_SERIAL_NUMBER] = cert_aki[CERT_AUTHORITY_KEY_IDENTIFIER_SERIAL]; } } - found_store_cert = ca_store->find(cert_search_params); + found_store_cert = find(cert_search_params); - if(found_store_cert != ca_store->end()) + if(found_store_cert != end()) { - LLSD foo = (*found_store_cert)->getLLSD(); // validate the store cert against the depth _validateCert(validation_policy & VALIDATION_POLICY_CA_BASIC_CONSTRAINTS, (*found_store_cert), @@ -1105,11 +1147,16 @@ void LLBasicCertificateChain::validate(int validation_policy, throw LLCertValidationInvalidSignatureException(*current_cert); } // successfully validated. + mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); return; } previous_cert = (*current_cert); current_cert++; - depth++; + depth++; + if(current_cert != current_cert != cert_chain->end()) + { + (*current_cert)->getLLSD(current_cert_info); + } } if (validation_policy & VALIDATION_POLICY_TRUSTED) { @@ -1118,6 +1165,7 @@ void LLBasicCertificateChain::validate(int validation_policy, throw LLCertValidationTrustException((*this)[size()-1]); } + mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); } @@ -1155,7 +1203,7 @@ void LLSecAPIBasicHandler::init() "CA.pem"); - LL_INFOS("SECAPI") << "Loading certificate store from " << store_file << LL_ENDL; + LL_DEBUGS("SECAPI") << "Loading certificate store from " << store_file << LL_ENDL; mStore = new LLBasicCertificateStore(store_file); // grab the application CA.pem file that contains the well-known certs shipped @@ -1465,7 +1513,7 @@ void LLSecAPIBasicHandler::saveCredential(LLPointer cred, bool sav { credential["authenticator"] = cred->getAuthenticator(); } - LL_INFOS("SECAPI") << "Saving Credential " << cred->getGrid() << ":" << cred->userID() << " " << save_authenticator << LL_ENDL; + LL_DEBUGS("SECAPI") << "Saving Credential " << cred->getGrid() << ":" << cred->userID() << " " << save_authenticator << LL_ENDL; setProtectedData("credential", cred->getGrid(), credential); //*TODO: If we're saving Agni credentials, should we write the // credentials to the legacy password.dat/etc? -- cgit v1.2.3 From a7d1c68c787fcccf888632b9787c4cc9b30393f7 Mon Sep 17 00:00:00 2001 From: Roxie Linden Date: Mon, 24 May 2010 15:31:10 -0700 Subject: Fixup some certificate related unit tests --- indra/newview/llsechandler_basic.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 84ab9b9175..a81cde3126 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -1153,7 +1153,7 @@ void LLBasicCertificateStore::validate(int validation_policy, previous_cert = (*current_cert); current_cert++; depth++; - if(current_cert != current_cert != cert_chain->end()) + if(current_cert != cert_chain->end()) { (*current_cert)->getLLSD(current_cert_info); } -- cgit v1.2.3 From 32ad37b3f7ca48564bd15de2664f323ad4a2d367 Mon Sep 17 00:00:00 2001 From: Roxie Linden Date: Mon, 24 May 2010 16:21:29 -0700 Subject: Few more touchups for the cert performance code --- indra/newview/llsechandler_basic.cpp | 5 ++--- 1 file changed, 2 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 a81cde3126..5f24d4398a 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -1006,7 +1006,7 @@ void LLBasicCertificateStore::validate(int validation_policy, const LLSD& validation_params) { - if(size() < 1) + if(cert_chain->size() < 1) { throw LLCertException(NULL, "No certs in chain"); } @@ -1160,9 +1160,8 @@ void LLBasicCertificateStore::validate(int validation_policy, } if (validation_policy & VALIDATION_POLICY_TRUSTED) { - LLPointer untrusted_ca_cert = (*this)[size()-1]; // we reached the end without finding a trusted cert. - throw LLCertValidationTrustException((*this)[size()-1]); + throw LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1]); } mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); -- cgit v1.2.3 From cca5aa48ae07a2d0e4f4a26e647b360c098cd831 Mon Sep 17 00:00:00 2001 From: karina Date: Wed, 26 May 2010 23:46:42 -0700 Subject: Add new mechanism for getting a unique machine id. --- indra/newview/llsechandler_basic.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index edf5ce9b60..c006f4dd0d 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -52,6 +52,7 @@ LLS * By copying, modifying or distributing this software, you acknowledge #include #include #include +#include "llmachineid.h" @@ -1195,9 +1196,9 @@ void LLSecAPIBasicHandler::_readProtectedData() U8 buffer[BUFFER_READ_SIZE]; U8 decrypted_buffer[BUFFER_READ_SIZE]; int decrypted_length; - unsigned char MACAddress[MAC_ADDRESS_BYTES]; - LLUUID::getNodeID(MACAddress); - LLXORCipher cipher(MACAddress, MAC_ADDRESS_BYTES); + unsigned char unique_id[MAC_ADDRESS_BYTES]; + LLMachineID::getUniqueID(unique_id, sizeof(unique_id)); + LLXORCipher cipher(unique_id, sizeof(unique_id)); // read in the salt and key protected_data_stream.read((char *)salt, STORE_SALT_SIZE); @@ -1281,9 +1282,9 @@ void LLSecAPIBasicHandler::_writeProtectedData() EVP_CIPHER_CTX ctx; EVP_CIPHER_CTX_init(&ctx); EVP_EncryptInit(&ctx, EVP_rc4(), salt, NULL); - unsigned char MACAddress[MAC_ADDRESS_BYTES]; - LLUUID::getNodeID(MACAddress); - LLXORCipher cipher(MACAddress, MAC_ADDRESS_BYTES); + unsigned char unique_id[MAC_ADDRESS_BYTES]; + LLMachineID::getUniqueID(unique_id, sizeof(unique_id)); + LLXORCipher cipher(unique_id, sizeof(unique_id)); cipher.encrypt(salt, STORE_SALT_SIZE); protected_data_stream.write((const char *)salt, STORE_SALT_SIZE); @@ -1501,9 +1502,9 @@ std::string LLSecAPIBasicHandler::_legacyLoadPassword() } // Decipher with MAC address - unsigned char MACAddress[MAC_ADDRESS_BYTES]; - LLUUID::getNodeID(MACAddress); - LLXORCipher cipher(MACAddress, 6); + unsigned char unique_id[MAC_ADDRESS_BYTES]; + LLMachineID::getUniqueID(unique_id, sizeof(unique_id)); + LLXORCipher cipher(unique_id, sizeof(unique_id)); cipher.decrypt(&buffer[0], buffer.size()); return std::string((const char*)&buffer[0], buffer.size()); -- cgit v1.2.3 From 7e0b36d6102f5e285296cd8e0dc6961b5c73c7cb Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Fri, 28 May 2010 12:47:45 +0100 Subject: CID-442 Checker: FORWARD_NULL Function: LLBasicCertificateVector::insert(LLCertificateVector::iterator, LLPointer) File: /indra/newview/llsechandler_basic.cpp --- indra/newview/llsechandler_basic.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'indra/newview/llsechandler_basic.cpp') diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index edf5ce9b60..c4c13ccdca 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -541,7 +541,7 @@ LLBasicCertificateVector::iterator LLBasicCertificateVector::find(const LLSD& pa // Insert a certificate into the store. If the certificate already // exists in the store, nothing is done. void LLBasicCertificateVector::insert(iterator _iter, - LLPointer cert) + LLPointer cert) { LLSD cert_info = cert->getLLSD(); if (cert_info.isMap() && cert_info.has(CERT_SHA1_DIGEST)) @@ -551,7 +551,11 @@ void LLBasicCertificateVector::insert(iterator _iter, if(find(existing_cert_info) == end()) { BasicIteratorImpl *basic_iter = dynamic_cast(_iter.mImpl.get()); - mCerts.insert(basic_iter->mIter, cert); + llassert(basic_iter); + if (basic_iter) + { + mCerts.insert(basic_iter->mIter, cert); + } } } } -- cgit v1.2.3