From: Christos Tsantilas Date: Tue, 18 Sep 2012 09:56:08 +0000 (+0300) Subject: - Remove the ValidateCertificateResponse::ErrorItem::certId member. The X-Git-Tag: SQUID_3_4_0_1~460^2~13^2~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=77dce8a588d34f67287443ebb0f8f7a413221938;p=thirdparty%2Fsquid.git - Remove the ValidateCertificateResponse::ErrorItem::certId member. The user should always link with a certificate, not with an index to certificate. - The CertValidateMessage::parseResponse takes as argument the list of peer Certificates. It uses this list to fill the ValidateCertificateResponse object. - Return ERR_GATEWAY_FAILURE/HTTP_INTERNAL_SERVER_ERROR error if: * Failed to compose the Ssl::CertValidateMessage message to sent to cert validator * The response from cert validator is wrong * The cert validator returns an error. --- diff --git a/src/forward.cc b/src/forward.cc index 038bc27b7e..ee48acaba2 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -790,6 +790,14 @@ FwdState::negotiateSSL(int fd) " certificate: " << e.what() << "; will now block to " << "validate that certificate."); // fall through to do blocking in-process generation. + ErrorState *anErr = new ErrorState(ERR_GATEWAY_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request); + fail(anErr); + if (serverConnection()->getPeer()) { + peerConnectFailed(serverConnection()->getPeer()); + } + serverConn->close(); + self = NULL; + return; } } #endif // USE_SSL_CERT_VALIDATOR @@ -810,6 +818,7 @@ FwdState::sslCrtvdHandleReply(const char *reply) { Ssl::Errors *errs = NULL; Ssl::ErrorDetail *errDetails = NULL; + bool validatorFailed = false; if (!Comm::IsConnOpen(serverConnection())) { return; } @@ -817,24 +826,27 @@ FwdState::sslCrtvdHandleReply(const char *reply) if (!reply) { debugs(83, 1, HERE << "\"ssl_crtd\" helper return reply"); + validatorFailed = true; } else { Ssl::CertValidateMessage reply_message; Ssl::ValidateCertificateResponse resp; 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, error) ) { + !reply_message.parseResponse(resp, 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()); + validatorFailed = true; } else { debugs(83, 5, HERE << "Certificate for " << request->GetHost() << " was successfully validated from ssl_crtvd"); - // Copy the list of errors etc.... ACLFilledChecklist *check = NULL; if (acl_access *acl = Config.ssl_client.cert_error) { check = new ACLFilledChecklist(acl, request, dash_str); for(std::vector::const_iterator i = resp.errors.begin(); i != resp.errors.end(); ++i) { - debugs(83, 7, "Error item: " << i->error_no << " " << i->error_reason << " " << i->certId); + debugs(83, 7, "Error item: " << i->error_no << " " << i->error_reason); if (i->error_no == SSL_ERROR_NONE) continue; //ignore???? @@ -845,13 +857,7 @@ FwdState::sslCrtvdHandleReply(const char *reply) debugs(83, 3, "bypassing SSL error " << i->error_no << " in " << "buffer"); } else { debugs(83, 5, "confirming SSL error " << i->error_no); - STACK_OF(X509) *peerCerts = SSL_get_peer_cert_chain(ssl); - //if i->certID is not correct sk_X509_value returns NULL - X509 *brokenCert = NULL; - if (i->cert != NULL) - brokenCert = i->cert; - else - brokenCert = sk_X509_value(peerCerts, i->certId); + X509 *brokenCert = (i->cert ? i->cert : NULL); X509 *peerCert = SSL_get_peer_certificate(ssl); const char *aReason = i->error_reason.empty() ? NULL : i->error_reason.c_str(); errDetails = new Ssl::ErrorDetail(i->error_no, peerCert, brokenCert, aReason); @@ -877,22 +883,30 @@ FwdState::sslCrtvdHandleReply(const char *reply) } } } - // Check the list error with - if (errDetails && request->clientConnectionManager.valid()) { - // remember the server certificate from the ErrorDetail object - if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) { - // remember validation errors, if any - if (errs) { - if (serverBump->sslErrors) - cbdataReference(serverBump->sslErrors); - serverBump->sslErrors = cbdataReference(errs); + + ErrorState *anErr = NULL; + if (validatorFailed) { + anErr = new ErrorState(ERR_GATEWAY_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request); + } else { + + // Check the list error with + if (errDetails && request->clientConnectionManager.valid()) { + // remember the server certificate from the ErrorDetail object + if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) { + // remember validation errors, if any + if (errs) { + if (serverBump->sslErrors) + cbdataReference(serverBump->sslErrors); + serverBump->sslErrors = cbdataReference(errs); + } } } + + anErr = makeConnectingError(ERR_SECURE_CONNECT_FAIL); + anErr->detail = errDetails; + /*anErr->xerrno= Should preserved*/ } - ErrorState *const anErr = makeConnectingError(ERR_SECURE_CONNECT_FAIL); - anErr->detail = errDetails; - /*anErr->xerrno= Should preserved*/ fail(anErr); if (serverConnection()->getPeer()) { peerConnectFailed(serverConnection()->getPeer()); diff --git a/src/ssl/cert_validate_message.cc b/src/ssl/cert_validate_message.cc index b33645ce6d..4a5adc00ed 100644 --- a/src/ssl/cert_validate_message.cc +++ b/src/ssl/cert_validate_message.cc @@ -1,4 +1,5 @@ #include "squid.h" +#include "acl/FilledChecklist.h" #include "ssl/support.h" #include "ssl/cert_validate_message.h" #include "ssl/ErrorDetail.h" @@ -47,7 +48,7 @@ static int get_error_id(const char *label, size_t len) return strtol(e, 0 , 10); } -bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, std::string &error) +bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error) { int current_errorId = -1; std::vector certs; @@ -118,8 +119,10 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, } } if (!currentItem.cert) { - currentItem.certId = get_error_id(v.c_str(), v.length()); - debugs(83, 6, HERE << "Cert ID read:" << currentItem.certId); + int certId = get_error_id(v.c_str(), v.length()); + //if certId is not correct sk_X509_value returns NULL + currentItem.setCert(sk_X509_value(peerCerts, certId)); + debugs(83, 6, HERE << "Cert ID read:" << certId); } } @@ -135,7 +138,6 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, Ssl::ValidateCertificateResponse::ErrorItem::ErrorItem(const ErrorItem &old) { error_no = old.error_no; error_reason = old.error_reason; - certId = old.certId; cert = NULL; setCert(old.cert); } @@ -148,7 +150,6 @@ Ssl::ValidateCertificateResponse::ErrorItem::~ErrorItem() { Ssl::ValidateCertificateResponse::ErrorItem & Ssl::ValidateCertificateResponse::ErrorItem::operator = (const ErrorItem &old) { error_no = old.error_no; error_reason = old.error_reason; - certId = old.certId; setCert(old.cert); return *this; } @@ -168,7 +169,6 @@ void Ssl::ValidateCertificateResponse::ErrorItem::clear() { error_no = SSL_ERROR_NONE; error_reason = ""; - certId = 0; if (cert) X509_free(cert); cert = NULL; diff --git a/src/ssl/cert_validate_message.h b/src/ssl/cert_validate_message.h index 1b0af4e1dd..8b4a408251 100644 --- a/src/ssl/cert_validate_message.h +++ b/src/ssl/cert_validate_message.h @@ -25,7 +25,7 @@ class ValidateCertificateResponse { public: class ErrorItem{ public: - ErrorItem(): error_no(SSL_ERROR_NONE), certId(0), cert(NULL) {} + ErrorItem(): error_no(SSL_ERROR_NONE), cert(NULL) {} ErrorItem(const ErrorItem &); ~ErrorItem(); ErrorItem & operator = (const ErrorItem &); @@ -33,7 +33,6 @@ public: void clear(); ssl_error_t error_no; std::string error_reason; - int certId; X509 *cert; }; @@ -57,7 +56,7 @@ class CertValidateMessage: public CrtdMessage { public: CertValidateMessage(): CrtdMessage() {} void composeRequest(ValidateCertificate const &vcert); - bool parseResponse(ValidateCertificateResponse &resp, std::string &error); + bool parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error); /// String code for "cert_validate" messages static const std::string code_cert_validate;