From b56756cbe3a06ca6d86b07b2d9db92ee36cf232e Mon Sep 17 00:00:00 2001 From: Christos Tsantilas Date: Thu, 20 Sep 2012 19:26:47 +0300 Subject: [PATCH] - Rename CertValidateMessage-> CertValidationMsg, ValidateCertificateResponse-> CertValidationResponse and ValidateCertificate -> CertValidationRequest - fixes so that the "make check" and "make distcheck" works - Document new classes and members --- helpers/Makefile.am | 3 +- src/forward.cc | 45 ++++++++--------- src/forward.h | 11 +++-- src/ssl/cert_validate_message.cc | 61 ++++++++++------------- src/ssl/cert_validate_message.h | 85 +++++++++++++++++++++----------- src/tests/stub_libsslsquid.cc | 2 +- 6 files changed, 116 insertions(+), 91 deletions(-) diff --git a/helpers/Makefile.am b/helpers/Makefile.am index 7f581f9731..37b765fb0b 100644 --- a/helpers/Makefile.am +++ b/helpers/Makefile.am @@ -7,7 +7,8 @@ DIST_SUBDIRS = \ log_daemon \ negotiate_auth \ ntlm_auth \ - url_rewrite + url_rewrite \ + ssl SUBDIRS = \ basic_auth \ diff --git a/src/forward.cc b/src/forward.cc index f4f41b099e..af364c56ec 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -761,32 +761,33 @@ FwdState::negotiateSSL(int fd) #if 1 // USE_SSL_CERT_VALIDATOR if (Ssl::TheConfig.ssl_crt_validator) { - Ssl::ValidateCertificate certValidate; + Ssl::CertValidationRequest validationRequest; // WARNING: The STACK_OF(*) OpenSSL objects does not support locking. // If we need to support locking we need to sk_X509_dup the STACK_OF(X509) // list and lock all of the X509 members of the list. // Currently we do not use any locking for any of the members of the - // Ssl::ValidateCertificate class. If the ssl object gone, the value returned + // Ssl::CertValidationRequest class. If the ssl object gone, the value returned // from SSL_get_peer_cert_chain may not exist any more. In this code the - // Ssl::ValidateCertificate object used only to pass data to + // Ssl::CertValidationRequest object used only to pass data to // Ssl::CertValidationHelper::submit method. - certValidate.peerCerts = SSL_get_peer_cert_chain(ssl); - certValidate.domainName = request->GetHost(); + validationRequest.peerCerts = SSL_get_peer_cert_chain(ssl); + validationRequest.domainName = request->GetHost(); if (Ssl::Errors *errs = static_cast(SSL_get_ex_data(ssl, ssl_ex_index_ssl_errors))) - certValidate.errors = errs; + // validationRequest disappears on return so no need to cbdataReference + validationRequest.errors = errs; else - certValidate.errors = NULL; + validationRequest.errors = NULL; try { debugs(83, 5, HERE << "Sending SSL certificate for validation to ssl_crtvd."); - Ssl::CertValidateMessage request_message; - request_message.setCode(Ssl::CertValidateMessage::code_cert_validate); - request_message.composeRequest(certValidate); - debugs(83, 5, HERE << "SSL crtvd request: " << request_message.compose().c_str()); - Ssl::CertValidationHelper::GetInstance()->sslSubmit(request_message, sslCrtvdHandleReplyWrapper, this); + Ssl::CertValidationMsg requestMsg; + requestMsg.setCode(Ssl::CertValidationMsg::code_cert_validate); + requestMsg.composeRequest(validationRequest); + debugs(83, 5, HERE << "SSL crtvd request: " << requestMsg.compose().c_str()); + Ssl::CertValidationHelper::GetInstance()->sslSubmit(requestMsg, sslCrtvdHandleReplyWrapper, this); return; } catch (const std::exception &e) { debugs(33, DBG_IMPORTANT, "ERROR: Failed to compose ssl_crtd " << - "request for " << certValidate.domainName << + "request for " << validationRequest.domainName << " certificate: " << e.what() << "; will now block to " << "validate that certificate."); // fall through to do blocking in-process generation. @@ -828,21 +829,21 @@ FwdState::sslCrtvdHandleReply(const char *reply) debugs(83, 1, HERE << "\"ssl_crtd\" helper return reply"); validatorFailed = true; } else { - Ssl::CertValidateMessage reply_message; - Ssl::ValidateCertificateResponse resp; + Ssl::CertValidationMsg replyMsg; + Ssl::CertValidationResponse validationResponse; std::string error; STACK_OF(X509) *peerCerts = SSL_get_peer_cert_chain(ssl); - if (reply_message.parse(reply, strlen(reply)) != Ssl::CrtdMessage::OK || - !reply_message.parseResponse(resp, peerCerts, error) ) { + if (replyMsg.parse(reply, strlen(reply)) != Ssl::CrtdMessage::OK || + !replyMsg.parseResponse(validationResponse, peerCerts, error) ) { debugs(83, 5, HERE << "Reply from ssl_crtvd for " << request->GetHost() << " is incorrect"); validatorFailed = true; } else { - if (reply_message.getCode() != "OK") { - debugs(83, 5, HERE << "Certificate for " << request->GetHost() << " cannot be validated. ssl_crtvd response: " << reply_message.getBody()); + if (replyMsg.getCode() != "OK") { + debugs(83, 5, HERE << "Certificate for " << request->GetHost() << " cannot be validated. ssl_crtvd response: " << replyMsg.getBody()); validatorFailed = true; } else { debugs(83, 5, HERE << "Certificate for " << request->GetHost() << " was successfully validated from ssl_crtvd"); - errs = sslCrtvdCheckForErrors(resp, errDetails); + errs = sslCrtvdCheckForErrors(validationResponse, errDetails); if (!errDetails) { dispatch(); return; @@ -887,7 +888,7 @@ FwdState::sslCrtvdHandleReply(const char *reply) /// The first honored error, if any, is returned via errDetails parameter. /// The method returns all seen errors except SSL_ERROR_NONE as Ssl::Errors. Ssl::Errors * -FwdState::sslCrtvdCheckForErrors(Ssl::ValidateCertificateResponse &resp, Ssl::ErrorDetail *& errDetails) +FwdState::sslCrtvdCheckForErrors(Ssl::CertValidationResponse &resp, Ssl::ErrorDetail *& errDetails) { Ssl::Errors *errs = NULL; @@ -896,7 +897,7 @@ FwdState::sslCrtvdCheckForErrors(Ssl::ValidateCertificateResponse &resp, Ssl::Er check = new ACLFilledChecklist(acl, request, dash_str); SSL *ssl = fd_table[serverConnection()->fd].ssl; - typedef Ssl::ValidateCertificateResponse::Errors::const_iterator SVCRECI; + typedef Ssl::CertValidationResponse::RecvdErrors::const_iterator SVCRECI; for (SVCRECI i = resp.errors.begin(); i != resp.errors.end(); ++i) { debugs(83, 7, "Error item: " << i->error_no << " " << i->error_reason); diff --git a/src/forward.h b/src/forward.h index 2a4b311352..30200ad4a3 100644 --- a/src/forward.h +++ b/src/forward.h @@ -18,11 +18,13 @@ typedef RefCount AccessLogEntryPointer; class ErrorState; class HttpRequest; +#if USE_SSL namespace Ssl { class ErrorDetail; - class ValidateCertificateResponse; + class CertValidationResponse; }; +#endif /** * Returns the TOS value that we should be setting on the connection @@ -78,10 +80,13 @@ public: /** return a ConnectionPointer to the current server connection (may or may not be open) */ Comm::ConnectionPointer const & serverConnection() const { return serverConn; }; -#if 1 // USE_SSL_CERT_VALIDATOR +#if USE_SSL //&& USE_SSL_CERT_VALIDATOR + /// Callback function called when squid receive message from cert validator helper static void sslCrtvdHandleReplyWrapper(void *data, char *reply); + /// Process response from cert validator helper void sslCrtvdHandleReply(const char *reply); - Ssl::Errors *sslCrtvdCheckForErrors(Ssl::ValidateCertificateResponse &, Ssl::ErrorDetail *&); + /// Check SSL errors returned from cert validator against sslproxy_cert_error access list + Ssl::Errors *sslCrtvdCheckForErrors(Ssl::CertValidationResponse &, Ssl::ErrorDetail *&); #endif private: // hidden for safer management of self; use static fwdStart diff --git a/src/ssl/cert_validate_message.cc b/src/ssl/cert_validate_message.cc index 91637e265b..82ad35db5f 100644 --- a/src/ssl/cert_validate_message.cc +++ b/src/ssl/cert_validate_message.cc @@ -6,12 +6,12 @@ void -Ssl::CertValidateMessage::composeRequest(ValidateCertificate const &vcert) +Ssl::CertValidationMsg::composeRequest(CertValidationRequest const &vcert) { body.clear(); - body += Ssl::CertValidateMessage::param_host + "=" + vcert.domainName; + body += Ssl::CertValidationMsg::param_host + "=" + vcert.domainName; if (vcert.errors) { - body += "\n" + Ssl::CertValidateMessage::param_error + "="; + body += "\n" + Ssl::CertValidationMsg::param_error + "="; bool comma = false; for (const Ssl::Errors *err = vcert.errors; err; err = err->next ) { if (comma) @@ -51,7 +51,7 @@ get_error_id(const char *label, size_t len) } bool -Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error) +Ssl::CertValidationMsg::parseResponse(CertValidationResponse &resp, STACK_OF(X509) *peerCerts, std::string &error) { std::vector certs; @@ -94,7 +94,7 @@ Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK v.c_str()); int errorId = get_error_id(param, param_len); - Ssl::ValidateCertificateResponse::ErrorItem ¤tItem = resp.getError(errorId); + Ssl::CertValidationResponse::RecvdError ¤tItem = resp.getError(errorId); if (param_len > param_error_name.length() && strncmp(param, param_error_name.c_str(), param_error_name.length()) == 0){ @@ -117,7 +117,7 @@ Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK // to cert validator. The certificates sent to cert validator have names in // form "cert_xx" where the "xx" is an integer represents the position of // the certificate inside peer certificates list. - int certId = get_error_id(v.c_str(), v.length()); + const int certId = get_error_id(v.c_str(), v.length()); debugs(83, 6, HERE << "Cert index in peer certificates list:" << certId); //if certId is not correct sk_X509_value returns NULL currentItem.setCert(sk_X509_value(peerCerts, certId)); @@ -137,7 +137,7 @@ Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK } X509 * -Ssl::CertValidateMessage::getCertByName(std::vector const &certs, std::string const & name) +Ssl::CertValidationMsg::getCertByName(std::vector const &certs, std::string const & name) { for (std::vector::const_iterator ci = certs.begin(); ci != certs.end(); ++ci) { if (ci->name.compare(name) == 0) @@ -146,32 +146,32 @@ Ssl::CertValidateMessage::getCertByName(std::vector const &certs, std: return NULL; } -Ssl::ValidateCertificateResponse::ErrorItem & -Ssl::ValidateCertificateResponse::getError(int errorId) +Ssl::CertValidationResponse::RecvdError & +Ssl::CertValidationResponse::getError(int errorId) { - for(Ssl::ValidateCertificateResponse::Errors::iterator i = errors.begin(); i != errors.end(); ++i){ + for(Ssl::CertValidationResponse::RecvdErrors::iterator i = errors.begin(); i != errors.end(); ++i){ if (i->id == errorId) return *i; } - Ssl::ValidateCertificateResponse::ErrorItem errItem; + Ssl::CertValidationResponse::RecvdError errItem; errItem.id = errorId; errors.push_back(errItem); return errors.back(); } -Ssl::ValidateCertificateResponse::ErrorItem::ErrorItem(const ErrorItem &old) { +Ssl::CertValidationResponse::RecvdError::RecvdError(const RecvdError &old) { error_no = old.error_no; error_reason = old.error_reason; cert = NULL; setCert(old.cert); } -Ssl::ValidateCertificateResponse::ErrorItem::~ErrorItem() { +Ssl::CertValidationResponse::RecvdError::~RecvdError() { if (cert) X509_free(cert); } -Ssl::ValidateCertificateResponse::ErrorItem & Ssl::ValidateCertificateResponse::ErrorItem::operator = (const ErrorItem &old) { +Ssl::CertValidationResponse::RecvdError & Ssl::CertValidationResponse::RecvdError::operator = (const RecvdError &old) { error_no = old.error_no; error_reason = old.error_reason; setCert(old.cert); @@ -179,7 +179,7 @@ Ssl::ValidateCertificateResponse::ErrorItem & Ssl::ValidateCertificateResponse:: } void -Ssl::ValidateCertificateResponse::ErrorItem::setCert(X509 *aCert) { +Ssl::CertValidationResponse::RecvdError::setCert(X509 *aCert) { if (cert) X509_free(cert); if (aCert) { @@ -189,37 +189,28 @@ Ssl::ValidateCertificateResponse::ErrorItem::setCert(X509 *aCert) { cert = NULL; } -void -Ssl::ValidateCertificateResponse::ErrorItem::clear() { - error_no = SSL_ERROR_NONE; - error_reason = ""; - if (cert) - X509_free(cert); - cert = NULL; -} - -Ssl::CertValidateMessage::CertItem::CertItem(const CertItem &old) +Ssl::CertValidationMsg::CertItem::CertItem(const CertItem &old) { name = old.name; cert = NULL; setCert(old.cert); } -Ssl::CertValidateMessage::CertItem & Ssl::CertValidateMessage::CertItem::operator = (const CertItem &old) +Ssl::CertValidationMsg::CertItem & Ssl::CertValidationMsg::CertItem::operator = (const CertItem &old) { name = old.name; setCert(old.cert); return *this; } -Ssl::CertValidateMessage::CertItem::~CertItem() +Ssl::CertValidationMsg::CertItem::~CertItem() { if (cert) X509_free(cert); } void -Ssl::CertValidateMessage::CertItem::setCert(X509 *aCert) +Ssl::CertValidationMsg::CertItem::setCert(X509 *aCert) { if (cert) X509_free(cert); @@ -230,11 +221,11 @@ Ssl::CertValidateMessage::CertItem::setCert(X509 *aCert) cert = NULL; } -const std::string Ssl::CertValidateMessage::code_cert_validate("cert_validate"); -const std::string Ssl::CertValidateMessage::param_domain("domain"); -const std::string Ssl::CertValidateMessage::param_error("errors"); -const std::string Ssl::CertValidateMessage::param_cert("cert_"); -const std::string Ssl::CertValidateMessage::param_error_name("error_name_"); -const std::string Ssl::CertValidateMessage::param_error_reason("error_reason_"); -const std::string Ssl::CertValidateMessage::param_error_cert("error_cert_"); +const std::string Ssl::CertValidationMsg::code_cert_validate("cert_validate"); +const std::string Ssl::CertValidationMsg::param_domain("domain"); +const std::string Ssl::CertValidationMsg::param_error("errors"); +const std::string Ssl::CertValidationMsg::param_cert("cert_"); +const std::string Ssl::CertValidationMsg::param_error_name("error_name_"); +const std::string Ssl::CertValidationMsg::param_error_reason("error_reason_"); +const std::string Ssl::CertValidationMsg::param_error_cert("error_cert_"); diff --git a/src/ssl/cert_validate_message.h b/src/ssl/cert_validate_message.h index a2e4b80199..99e1982a6a 100644 --- a/src/ssl/cert_validate_message.h +++ b/src/ssl/cert_validate_message.h @@ -12,57 +12,84 @@ namespace Ssl { - -class ValidateCertificate { +/** + * This class is used to hold the required informations to build + * a request message for the certificate validator helper + */ +class CertValidationRequest { public: - STACK_OF(X509) *peerCerts; - Errors *errors; - std::string domainName; - ValidateCertificate() : peerCerts(NULL), errors(NULL) {} + STACK_OF(X509) *peerCerts; ///< The list of sent by SSL server + Errors *errors; ///< The list of errors detected + std::string domainName; ///< The server name + CertValidationRequest() : peerCerts(NULL), errors(NULL) {} }; -class ValidateCertificateResponse { +/** + * This class is used to store informations found in certificate validation + * response messages read from certificate validator helper + */ +class CertValidationResponse { public: - class ErrorItem{ + /** + * This class used to hold error informations returned from + * cert validator helper. + */ + class RecvdError{ public: - ErrorItem(): id(0), error_no(SSL_ERROR_NONE), cert(NULL) {} - ErrorItem(const ErrorItem &); - ~ErrorItem(); - ErrorItem & operator = (const ErrorItem &); - void setCert(X509 *); - void clear(); + RecvdError(): id(0), error_no(SSL_ERROR_NONE), cert(NULL) {} + RecvdError(const RecvdError &); + ~RecvdError(); + RecvdError & operator = (const RecvdError &); + void setCert(X509 *); ///< Sets cert to the given certificate int id; ///< The id of the error - ssl_error_t error_no; ///< The SSL error code + ssl_error_t error_no; ///< The OpenSSL error code std::string error_reason; ///< A string describing the error X509 *cert; ///< The broken certificate }; - typedef std::vector Errors; + typedef std::vector RecvdErrors; - ValidateCertificateResponse() {} - /// Search in errors list for an error with id=errorId - /// If know found a new ErrorItem added with the given id; - ErrorItem &getError(int errorId); - Errors errors; ///< The list of parsed errors + /// Search in errors list for the error item with id=errorId. + /// If none found a new RecvdError item added with the given id; + RecvdError &getError(int errorId); + RecvdErrors errors; ///< The list of parsed errors }; -class CertValidateMessage: public CrtdMessage { +/** + * This class is responsible for composing or parsing messages destined to + * or comming from a cert validator helper. + * The messages format is: + * ...\1 + */ +class CertValidationMsg: public CrtdMessage { private: + /** + * This class used to hold the certId/cert pairs found + * in cert validation messages. + */ class CertItem { public: - std::string name; - X509 *cert; + std::string name; ///< The certificate Id to use + X509 *cert; ///< A pointer to certificate CertItem(): cert(NULL) {} CertItem(const CertItem &); CertItem & operator = (const CertItem &); ~CertItem(); - void setCert(X509 *); + void setCert(X509 *); ///< Sets cert to the given certificate }; + public: - CertValidateMessage(): CrtdMessage() {} - void composeRequest(ValidateCertificate const &vcert); - bool parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error); - X509 *getCertByName(std::vector const &, std::string const & name); ///< search in a list of CertItems for a certificate + CertValidationMsg(): CrtdMessage() {} + + /// Build a request message for the cert validation helper + /// using informations provided by vcert object + void composeRequest(CertValidationRequest const &vcert); + + /// Parse a response message and fill the resp object with parsed informations + bool parseResponse(CertValidationResponse &resp, STACK_OF(X509) *peerCerts, std::string &error); + + /// Search a CertItems list for the certificate with ID "name" + X509 *getCertByName(std::vector const &, std::string const & name); /// String code for "cert_validate" messages static const std::string code_cert_validate; diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index be72326115..36ed401456 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -37,7 +37,7 @@ void Ssl::GlobalContextStorage::reconfigureStart() STUB #include "ssl/ErrorDetail.h" Ssl::ssl_error_t parseErrorString(const char *name) STUB_RETVAL(0) //const char *Ssl::getErrorName(ssl_error_t value) STUB_RETVAL(NULL) -Ssl::ErrorDetail::ErrorDetail(ssl_error_t err_no, X509 *, X509 *) STUB +Ssl::ErrorDetail::ErrorDetail(ssl_error_t err_no, X509 *, X509 *, const char *) STUB Ssl::ErrorDetail::ErrorDetail(ErrorDetail const &) STUB const String & Ssl::ErrorDetail::toString() const STUB_RETSTATREF(String) -- 2.47.2