]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Polished names, comments, and code formatting.
authorAlex Rousskov <rousskov@measurement-factory.com>
Tue, 19 Jun 2012 21:51:49 +0000 (15:51 -0600)
committerAlex Rousskov <rousskov@measurement-factory.com>
Tue, 19 Jun 2012 21:51:49 +0000 (15:51 -0600)
29 files changed:
include/CbDataList.h
src/ClientRequestContext.h
src/acl/FilledChecklist.cc
src/acl/FilledChecklist.h
src/acl/SslError.cc
src/acl/SslErrorData.cc
src/adaptation/icap/ModXact.h
src/adaptation/icap/Xaction.h
src/anyp/PortCfg.h
src/anyp/ProtocolType.h
src/cf.data.depend
src/cf.data.pre
src/client_side.cc
src/client_side.h
src/client_side_reply.h
src/client_side_request.cc
src/errorpage.cc
src/errorpage.h
src/forward.cc
src/globals.h
src/ssl/ErrorDetail.cc
src/ssl/ErrorDetail.h
src/ssl/ServerBump.cc
src/ssl/ServerBump.h
src/ssl/certificate_db.cc
src/ssl/crtd_message.h
src/ssl/ssl_crtd.cc
src/ssl/support.cc
src/ssl/support.h

index 8e221e20e20e17390400005001355e526712383c..2ffa0b9822fb93ed2d4551a45c7686161883db57 100644 (file)
@@ -44,10 +44,13 @@ public:
     CbDataList (C const &);
     ~CbDataList();
 
-    bool push_back_unique(C const &);
+    /// If element is already in the list return false.
+    /// Otherwise, adds the element to the end of the list and returns true.
+    /// Exists to avoid double iteration of find() and push() combo.
+    bool push_back_unique(C const &element);
     bool find(C const &)const;
     bool findAndTune(C const &);
-    /// iterates entire list to return the last element holder
+    /// Iterates the entire list to return the last element holder.
     CbDataList *tail();
     CbDataList *next;
     C element;
index a52d88fbdf15895acdd75d4e325f1fbc7a6e655b..74233cebc0abf689a7092d479cd957207d8d7aaa 100644 (file)
@@ -68,8 +68,8 @@ public:
 #if USE_SSL
     bool sslBumpCheckDone;
 #endif
-    ErrorState *error;
-    bool readNextRequest;
+    ErrorState *error; ///< saved error page for centralized/delayed processing
+    bool readNextRequest; ///< whether Squid should read after error handling
 
 private:
     CBDATA_CLASS(ClientRequestContext);
index 6db77825b628ed6bca6efb1b9ac542698e68a004..6e625f6acfca918fdd10018d17ffc3f4c0353461 100644 (file)
@@ -67,7 +67,7 @@ ACLFilledChecklist::ACLFilledChecklist() :
         snmp_community(NULL),
 #endif
 #if USE_SSL
-        sslErrorList(NULL),
+        sslErrors(NULL),
 #endif
         extacl_entry (NULL),
         conn_(NULL),
@@ -98,7 +98,7 @@ ACLFilledChecklist::~ACLFilledChecklist()
     cbdataReferenceDone(conn_);
 
 #if USE_SSL
-    cbdataReferenceDone(sslErrorList);
+    cbdataReferenceDone(sslErrors);
 #endif
 
     debugs(28, 4, HERE << "ACLFilledChecklist destroyed " << this);
@@ -183,7 +183,7 @@ ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_re
         snmp_community(NULL),
 #endif
 #if USE_SSL
-        sslErrorList(NULL),
+        sslErrors(NULL),
 #endif
         extacl_entry (NULL),
         conn_(NULL),
index 2ef86b6c856db61b95a61dc2367a16ad4125acf5..5c4b7668814ce839e07628dbff2ca017c5e1452f 100644 (file)
@@ -66,7 +66,8 @@ public:
 #endif
 
 #if USE_SSL
-    Ssl::Errors *sslErrorList;
+    /// SSL [certificate validation] errors in undefined order
+    Ssl::Errors *sslErrors;
 #endif
 
     ExternalACLEntry *extacl_entry;
index c81dc72741de918b93b8304c2cc1d2207a104e43..a062c127228871a5995c2fd96fa6f7bda5042647 100644 (file)
@@ -11,7 +11,7 @@
 int
 ACLSslErrorStrategy::match (ACLData<MatchType> * &data, ACLFilledChecklist *checklist)
 {
-    return data->match (checklist->sslErrorList);
+    return data->match (checklist->sslErrors);
 }
 
 ACLSslErrorStrategy *
index 7933f6d1a8f8daf4213124fdc79c6f49b262ed6f..12f3b88d234ab5c4a76419c21a184e29ce37e61c 100644 (file)
@@ -24,9 +24,8 @@ ACLSslErrorData::~ACLSslErrorData()
 bool
 ACLSslErrorData::match(const Ssl::Errors *toFind)
 {
-    const Ssl::Errors * err;
-    for (err = toFind ; err != NULL; err = err->next ) {
-        if (values->findAndTune (err->element))
+    for (const Ssl::Errors *err = toFind; err; err = err->next ) {
+        if (values->findAndTune(err->element))
             return true;
     }
     return false;
index 4bbe0b85735a6d78f7997d53d27cb39dede0e962..c5cc0f628cc7b64f37f54d9f393713e2eb4466db 100644 (file)
@@ -168,6 +168,7 @@ public:
 
     /// record error detail in the virgin request if possible
     virtual void detailError(int errDetail);
+    // Icap::Xaction API
     virtual void clearError();
 
 private:
index 012d98db863b3f47962bc64fb15cf1c948d86fdd..8fb26a4ebd3fd5699b9b20fcded781f291bd41ce 100644 (file)
@@ -134,7 +134,7 @@ public:
     // custom exception handling and end-of-call checks
     virtual void callException(const std::exception  &e);
     virtual void callEnd();
-    // clear the error details on retries/repeats
+    /// clear stored error details, if any; used for retries/repeats
     virtual void clearError() {}
     void dnsLookupDone(const ipcache_addrs *ia);
 
index 1e22043f0e248d46cecf52e624c60af15188815a..032e1dc54c70e7726110c3d32905d51399e900bf 100644 (file)
@@ -16,6 +16,7 @@ struct PortCfg {
     ~PortCfg();
     AnyP::PortCfg *clone() const;
 #if USE_SSL
+    /// creates, configures, and validates SSL context and related port options
     void configureSslServerContext();
 #endif
 
index bdafd7e8be92da10b944d7b2018fbd40a0dfc700..4f5ab3b753f1f8c5b5a05628c16fde7819c480a8 100644 (file)
@@ -29,7 +29,7 @@ typedef enum {
 #endif
     PROTO_URN,
     PROTO_WHOIS,
-    PROTO_INTERNAL, // miss on an internal object such as an icon
+    PROTO_INTERNAL,
     PROTO_ICY,
     PROTO_UNKNOWN,
     PROTO_MAX
index 2c93520541410b98667e7bec3ef8621d5e2ad8b1..81b9aca438377d6b9ae36e4ac09a5d3e2e350ea5 100644 (file)
@@ -69,5 +69,5 @@ wccp2_service
 wccp2_service_info
 wordlist
 sslproxy_ssl_bump      acl
-sslproxy_cert_sign    acl
+sslproxy_cert_sign     acl
 sslproxy_cert_adapt    acl
index 66a56fbf60dcbf71cb6537e30de6a3773db35223..116ea2ee8d16972c385a18e72b05cc49900ed6d8 100644 (file)
@@ -881,25 +881,24 @@ DOC_START
 IFDEF USE_SSL
        acl aclname ssl_error errorname
          # match against SSL certificate validation error [fast]
-         # For valid error names see in @DEFAULT_ERROR_DIR@/templates/error-details.txt template file
-         # The user aditionaly can use as error name the following error name
-         # shortcuts:
-         #  [ssl::]certHasExpired: certificate "not after" field is in the past
-         #  [ssl::]certNotYetValid: certificate "not before" field is in the 
-         #       future
-         #  [ssl::]certDomainMismatch: The certificate CN domain does not match
-         #       connecting host name
-         #  [ssl::]certUntrusted: The certificate is untrusted because of an
-         #       error says that the certificate issuer is not trusted.
-         #  [ssl::]certSelfSigned: The certificate is self signed
          #
-         # The ssl::certHasExpired, ssl::certNotYetValid ssl::certDomainMismatch,
-         # ssl::certUntrusted and ssl::certSelfSigned also exists as predefined
-         # acl lists.
+         # For valid error names see in @DEFAULT_ERROR_DIR@/templates/error-details.txt
+         # template file.
          #
-         # NOTE: The ssl_error acl has effect only when used with
-         # sslproxy_cert_error, sslproxy_cert_sign and sslproxy_cert_adapt
-         # access lists.
+         # The following can be used as shortcuts for certificate properties:
+         #  [ssl::]certHasExpired: the "not after" field is in the past
+         #  [ssl::]certNotYetValid: the "not before" field is in the future
+         #  [ssl::]certUntrusted: The certificate issuer is not to be trusted.
+         #  [ssl::]certSelfSigned: The certificate is self signed.
+         #  [ssl::]certDomainMismatch: The certificate CN domain does not
+         #         match the name the name of the host we are connecting to.
+         #
+         # The ssl::certHasExpired, ssl::certNotYetValid, ssl::certDomainMismatch,
+         # ssl::certUntrusted, and ssl::certSelfSigned can also be used as
+         # predefined ACLs, just like the 'all' ACL.
+         #
+         # NOTE: The ssl_error ACL is only supported with sslproxy_cert_error,
+         # sslproxy_cert_sign, and sslproxy_cert_adapt options.
 ENDIF
 
        Examples:
@@ -1353,7 +1352,7 @@ DOC_START
                        Squid, and treat them as unencrypted HTTP messages,
                        becoming the man-in-the-middle.
 
-                       The "ssl_bump" option is required to fully enable
+                       The ssl_bump option is required to fully enable
                        bumping of CONNECT requests.
 
        Omitting the mode flag causes default forward proxy mode to be used.
@@ -1569,7 +1568,7 @@ DOC_START
                        NP: disables authentication and maybe IPv6 on the port.
 
           ssl-bump     For each intercepted connection allowed by ssl_bump
-                   ACLs, establish a secure connection with the client and with
+                       ACLs, establish a secure connection with the client and with
                        the server, decrypt HTTPS messages as they pass through
                        Squid, and treat them as unencrypted HTTP messages,
                        becoming the man-in-the-middle.
index 438ce2e8c38cda04c4e2044e0bcaade842a91934..66913cf3c41680ee2289b19752a5b8786d409221 100644 (file)
@@ -2471,40 +2471,45 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context)
         return false;
 
     assert(sslServerBump->entry);
+    // Did we create an error entry while processing CONNECT?
     if (!sslServerBump->entry->isEmpty()) {
         quitAfterError(http->request);
 
-        //Failed? Here we should get the error from conn and send it to client
-        // The error stored in ConnStateData::bumpFirstEntry, replace the
-        // ClientHttpRequest store entry with this.
+        // Get the saved error entry and send it to the client by replacing the
+        // ClientHttpRequest store entry with it.
         clientStreamNode *node = context->getClientReplyContext();
         clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
-        assert (repContext);
-        debugs(33, 5, "Connection first has failed for " << http->uri << ". Respond with an error");
+        assert(repContext);
+        debugs(33, 5, "Responding with delated error for " << http->uri);
         repContext->setReplyToStoreEntry(sslServerBump->entry);
-        /*Save the original request for logging purposes*/
+
+        // save the original request for logging purposes
         if (!context->http->al.request)
             context->http->al.request = HTTPMSGLOCK(http->request);
-        /*Get the error details from the fake request used to retrieve SSL server certificate*/
+
+        // Get error details from the fake certificate-peeking request.
         http->request->detailError(sslServerBump->request->errType, sslServerBump->request->errDetail);
         context->pullData();
         return true;
     }
 
-    // We are in ssl-bump first mode. We have not the server connect name when
-    // we connected to server so we have to check certificates subject with our server name
-    if (sslServerBump && sslServerBump->serverCert.get()) {
+    // In bump-server-first mode, we have not necessarily seen the intended
+    // server name at certificate-peeking time. Check for domain mismatch now,
+    // when we can extract the intended name from the bumped HTTP request.
+    if (sslServerBump->serverCert.get()) {
         HttpRequest *request = http->request;
         if (!Ssl::checkX509ServerValidity(sslServerBump->serverCert.get(), request->GetHost())) {
-            debugs(33, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate does not match domainname " << request->GetHost());
+            debugs(33, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate " <<
+                   "does not match domainname " << request->GetHost());
 
             ACLFilledChecklist check(Config.ssl_client.cert_error, request, dash_str);
-            check.sslErrorList = new Ssl::Errors(SQUID_X509_V_ERR_DOMAIN_MISMATCH);
+            check.sslErrors = new Ssl::Errors(SQUID_X509_V_ERR_DOMAIN_MISMATCH);
             if (Comm::IsConnOpen(pinning.serverConnection))
                 check.fd(pinning.serverConnection->fd);
-            bool allowDomainMismatch = (check.fastCheck() == ACCESS_ALLOWED);
-            delete check.sslErrorList;
-            check.sslErrorList = NULL;
+            const bool allowDomainMismatch =
+                check.fastCheck() == ACCESS_ALLOWED;
+            delete check.sslErrors;
+            check.sslErrors = NULL;
 
             if (!allowDomainMismatch) {
                 quitAfterError(request);
@@ -2513,16 +2518,18 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context)
                 clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
                 assert (repContext);
 
-                // Fill the server ip address and server hostname for use with ErrorState
+                // Fill the server IP and hostname for error page generation.
                 HttpRequest::Pointer const & peekerRequest = sslServerBump->request;
                 request->hier.note(peekerRequest->hier.tcpServer, request->GetHost());
+
                 // Create an error object and fill it
                 ErrorState *err = new ErrorState(ERR_SECURE_CONNECT_FAIL, HTTP_SERVICE_UNAVAILABLE, request);
-
                 err->src_addr = clientConnection->remote;
-                Ssl::ErrorDetail *errDetail = new Ssl::ErrorDetail( SQUID_X509_V_ERR_DOMAIN_MISMATCH, sslServerBump->serverCert.get(), NULL);
+                Ssl::ErrorDetail *errDetail = new Ssl::ErrorDetail(
+                    SQUID_X509_V_ERR_DOMAIN_MISMATCH,
+                    sslServerBump->serverCert.get(), NULL);
                 err->detail = errDetail;
-                /*Save the original request for logging purposes*/
+                // Save the original request for logging purposes.
                 if (!context->http->al.request)
                     context->http->al.request = HTTPMSGLOCK(request);
                 repContext->setReplyToError(request->method, err);
@@ -2535,7 +2542,7 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context)
 
     return false;
 }
-#endif //USE_SSL
+#endif // USE_SSL
 
 static void
 clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *context, const HttpRequestMethod& method, HttpVersion http_ver)
@@ -3542,9 +3549,9 @@ httpsEstablish(ConnStateData *connState,  SSL_CTX *sslContext, Ssl::BumpMode bum
     if (sslContext && !(ssl = httpsCreate(details, sslContext)))
         return;
 
-     typedef CommCbMemFunT<ConnStateData, CommTimeoutCbParams> TimeoutDialer;
-    AsyncCall::Pointer timeoutCall =  JobCallback(33, 5,
-                                      TimeoutDialer, connState, ConnStateData::requestTimeout);
+    typedef CommCbMemFunT<ConnStateData, CommTimeoutCbParams> TimeoutDialer;
+    AsyncCall::Pointer timeoutCall = JobCallback(33, 5, TimeoutDialer,
+        connState, ConnStateData::requestTimeout);
     commSetConnTimeout(details, Config.Timeout.request, timeoutCall);
 
     if (ssl) 
@@ -3570,22 +3577,22 @@ httpsEstablish(ConnStateData *connState,  SSL_CTX *sslContext, Ssl::BumpMode bum
 
 /**
  * A callback function to use with the ACLFilledChecklist callback. 
- * In the case of ACCES_ALLOWED answer initializes an ssl bumped connection,
- * else revert the connection to tunnel mode.
+ * In the case of ACCES_ALLOWED answer initializes a bumped SSL connection,
+ * else reverts the connection to tunnel mode.
  */
 static void
 httpsSslBumpAccessCheckDone(allow_t answer, void *data)
 {
     ConnStateData *connState = (ConnStateData *) data;
 
-    //if connection closed/closing just return.
+    // if the connection is closed or closing, just return.
     if (!connState->isOpen())
         return;
 
-    // Require both a match and a positive mode to work around exceptional
+    // Require both a match and a positive bump mode to work around exceptional
     // cases where ACL code may return ACCESS_ALLOWED with zero answer.kind.
     if (answer == ACCESS_ALLOWED && answer.kind != Ssl::bumpNone) {
-        debugs(33, 2, HERE << " sslBump done data: " << connState->clientConnection);
+        debugs(33, 2, HERE << "sslBump done data: " << connState->clientConnection);
         httpsEstablish(connState, NULL, (Ssl::BumpMode)answer.kind);
     } else {
         // fake a CONNECT request to force connState to tunnel
@@ -3702,8 +3709,10 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
         return;
     }
 
-    // In the case of error while connecting to secure server, use a fake trusted certificate,
-    // with no mimicked fields and no adaptation algorithms
+    // In case of an error while connecting to the secure server, use a fake
+    // trusted certificate, with no mimicked fields and no adaptation
+    // algorithms. There is nothing we can mimic so we want to minimize the
+    // number of warnings the user will have to see to get to the error page.
     assert(sslServerBump->entry);
     if (sslServerBump->entry->isEmpty()) {
         if (X509 *mimicCert = sslServerBump->serverCert.get())
@@ -3712,10 +3721,10 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
         ACLFilledChecklist checklist(NULL, sslServerBump->request, 
                                      clientConnection != NULL ? clientConnection->rfc931 : dash_str);
         checklist.conn(this);
-        checklist.sslErrorList = cbdataReference(sslServerBump->bumpSslErrorNoList);
+        checklist.sslErrors = cbdataReference(sslServerBump->sslErrors);
 
         for (sslproxy_cert_adapt *ca = Config.ssl_client.cert_adapt; ca != NULL; ca = ca->next) {
-            // If the algorithm already set ignore.
+            // If the algorithm already set, then ignore it.
             if ((ca->alg == Ssl::algSetCommonName && certProperties.setCommonName) ||
                 (ca->alg == Ssl::algSetValidAfter && certProperties.setValidAfter) ||
                 (ca->alg == Ssl::algSetValidBefore && certProperties.setValidBefore) )
@@ -3725,8 +3734,8 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
                 const char *alg = Ssl::CertAdaptAlgorithmStr[ca->alg];
                 const char *param = ca->param;
   
-                // if not param defined for Common Name adaptation use hostname from 
-                // the CONNECT request
+                // For parameterless CN adaptation, use hostname from the
+                // CONNECT request.
                 if (ca->alg == Ssl::algSetCommonName) {
                     if (!param)
                         param = sslConnectHostOrIp.termedBuf();
@@ -4398,8 +4407,8 @@ ConnStateData::sendControlMsg(HttpControlMsg msg)
 void
 ConnStateData::clientPinnedConnectionClosed(const CommCloseCbParams &io)
 {
-    // it might be possible for FwdState to repin a failed connection sooner
-    // than this close callback is called for the failed connection
+    // FwdState might repin a failed connection sooner than this close
+    // callback is called for the failed connection.
     if (pinning.serverConnection == io.conn) {
         pinning.closeHandler = NULL; // Comm unregisters handlers before calling
         unpinConnection();
@@ -4416,7 +4425,7 @@ ConnStateData::pinConnection(const Comm::ConnectionPointer &pinServer, HttpReque
             return;
     }
 
-    unpinConnection(); // closes pinned connection, if any, and resets fields.
+    unpinConnection(); // closes pinned connection, if any, and resets fields
 
     pinning.serverConnection = pinServer;
 
index d4c34765e68a03a207c1e02e010725e51f312731..845bc412786829b43fc3905e7462aa0d1fa78438 100644 (file)
@@ -347,6 +347,10 @@ public:
     /// Fill the certAdaptParams with the required data for certificate adaptation
     /// and create the key for storing/retrieve the certificate to/from the cache
     void buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties);
+    /// Called when the client sends the first request on a bumped connection.
+    /// Returns false if no [delayed] error should be written to the client.
+    /// Otherwise, writes the error to the client and returns true. Also checks
+    /// for SQUID_X509_V_ERR_DOMAIN_MISMATCH on bumped requests.
     bool serveDelayedError(ClientSocketContext *context);
 #else
     bool switchedToHttps() const { return false; }
index 7eac9ea302f8a6e81dbb3080af1ae7648930d2cd..26d9d11d057fe4cf1ff761a782c004d0a791f1a3 100644 (file)
@@ -71,15 +71,17 @@ public:
     void identifyFoundObject(StoreEntry *entry);
     int storeOKTransferDone() const;
     int storeNotOKTransferDone() const;
-    /// Replaces the store entry with the given and awaiting the client side to read it
-    void setReplyToStoreEntry(StoreEntry *entry);
+    /// replaces current response store entry with the given one
+    void setReplyToStoreEntry(StoreEntry *e);
+    /// builds error using clientBuildError() and calls setReplyToError() below
     void setReplyToError(err_type, http_status, const HttpRequestMethod&, char const *, Ip::Address &, HttpRequest *, const char *,
 #if USE_AUTH
                          Auth::UserRequest::Pointer);
 #else
                          void * unused);
 #endif
-    void setReplyToError(const HttpRequestMethod& method, ErrorState *errstate);
+    /// creates a store entry for the reply and appends err to it
+    void setReplyToError(const HttpRequestMethod& method, ErrorState *err);
     void createStoreEntry(const HttpRequestMethod& m, request_flags flags);
     void removeStoreReference(store_client ** scp, StoreEntry ** ep);
     void removeClientStoreReference(store_client **scp, ClientHttpRequest *http);
index 26a74e266862425cbce5b3c3d0d74d44cd475e00..8fb7dacd66a63da7d04f122f6e5b2f151e67c5d4 100644 (file)
@@ -1278,7 +1278,7 @@ bool
 ClientRequestContext::sslBumpAccessCheck()
 {
     if (http->request->method == METHOD_CONNECT &&
-            !http->request->flags.spoof_client_ip && // is not a fake ssl-bumped request from a https port
+            !http->request->flags.spoof_client_ip && // not a fake bumped request from an https port
             Config.accessList.ssl_bump && http->getConn()->port->sslBump) {
         debugs(85, 5, HERE << "SslBump possible, checking ACL");
 
@@ -1591,8 +1591,8 @@ ClientHttpRequest::doCallouts()
     }
 
 #if USE_SSL
-    // We need to check for SSL bump even if the calloutContext->error is set
-    // because we are handling with different way the error inside SSL-bump
+    // We need to check for SslBump even if the calloutContext->error is set
+    // because bumping may require delaying the error until after CONNECT.
     if (!calloutContext->sslBumpCheckDone) {
         calloutContext->sslBumpCheckDone = true;
         if (calloutContext->sslBumpAccessCheck())
@@ -1616,7 +1616,7 @@ ClientHttpRequest::doCallouts()
         } else 
 #endif
         {
-            // send the error to the client
+            // send the error to the client now
             clientStreamNode *node = (clientStreamNode *)client_stream.tail->prev->data;
             clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
             assert (repContext);
@@ -1628,7 +1628,6 @@ ClientHttpRequest::doCallouts()
             node = (clientStreamNode *)client_stream.tail->data;
             clientStreamRead(node, this, node->readBuffer);
             e->unlock();
-            // Stop here.
             return;
         }
     }
index ef6e715482e1062a3c717ba21c85d7230fddbeda..0f90ad3509d7a1b51703fb7df0c6241861f1a75d 100644 (file)
@@ -1219,7 +1219,8 @@ ErrorState::BuildHttpReply()
         /* do not memBufClean() or delete the content, it was absorbed by httpBody */
     }
 
-    /* Make sure error codes get back to the client side for logging and error tracking */
+    // Make sure error codes get back to the client side for logging and
+    // error tracking.
     if (request) {
         int edc = ERR_DETAIL_NONE; // error detail code
 #if USE_SSL
index 2861a20da2bb754a954e2a4560ea58de7c957ad9..525310686011a9b0b48a0d980a4058d1af62aa9c 100644 (file)
@@ -105,9 +105,7 @@ public:
      */
     HttpReply *BuildHttpReply(void);
 
-    /**
-     * Sets the error details
-     */
+    /// set error type-specific detail code
     void detailError(int detailCode);
 
 private:
index cac6531a7f63214a037bc9a99e667c9c1addf47a..ff2b36414db03f57c8b08817506b7fa0cdc7b5e6 100644 (file)
@@ -649,10 +649,10 @@ FwdState::negotiateSSL(int fd)
             if (errFromFailure != NULL) {
                 // The errFromFailure is attached to the ssl object
                 // and will be released when ssl object destroyed.
-                // Copy errFromFailure to a new Ssl::ErrorDetail object
+                // Copy errFromFailure to a new Ssl::ErrorDetail object.
                 errDetails = new Ssl::ErrorDetail(*errFromFailure);
             } else {
-                // server_cert can be be NULL
+                // server_cert can be NULL here
                 X509 *server_cert = SSL_get_peer_certificate(ssl);
                 errDetails = new Ssl::ErrorDetail(SQUID_ERR_SSL_HANDSHAKE, server_cert, NULL);
                 X509_free(server_cert);
@@ -662,22 +662,21 @@ FwdState::negotiateSSL(int fd)
                 errDetails->setLibError(ssl_lib_error);
 
             if (request->clientConnectionManager.valid()) {
-                // Get the server certificate from ErrorDetail object and store it 
-                // to connection manager
+                // remember the server certificate from the ErrorDetail object
                 if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {
                     serverBump->serverCert.resetAndLock(errDetails->peerCert());
 
-                // if there is a list of ssl errors, pass it to connection manager
-                    if (Ssl::Errors *errNoList = static_cast<Ssl::Errors *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_sslerrno)))
-                        serverBump->bumpSslErrorNoList = cbdataReference(errNoList);
-                }
+                    // remember validation errors, if any
+                    if (Ssl::Errors *errs = static_cast<Ssl::Errors*>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_errors)))
+                        serverBump->sslErrors = cbdataReference(errs);
+                               }
             }
 
+            // For intercepted connections, set the host name to the server
+            // certificate CN. Otherwise, we just hope that CONNECT is using
+            // a user-entered address (a host name or a user-entered IP).
             const bool isConnectRequest = !request->clientConnectionManager->port->spoof_client_ip && 
                 !request->clientConnectionManager->port->intercepted;
-            // For intercepted connections, set host name to server
-            // certificate CN. Otherwise, we hope that CONNECT is using
-            // a user-entered address (a host name or a user-entered IP).
             if (request->flags.sslPeek && !isConnectRequest) {
                 if (X509 *srvX509 = errDetails->peerCert()) {
                     if (const char *name = Ssl::CommonHostName(srvX509)) {
@@ -686,6 +685,7 @@ FwdState::negotiateSSL(int fd)
                     }
                 }
             }
+
             ErrorState *const anErr = makeConnectingError(ERR_SECURE_CONNECT_FAIL);
             anErr->xerrno = sysErrNo;
             anErr->detail = errDetails;
@@ -701,11 +701,13 @@ FwdState::negotiateSSL(int fd)
     }
     
     if (request->clientConnectionManager.valid()) {
+        // remember the server certificate from the ErrorDetail object
         if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {
             serverBump->serverCert.reset(SSL_get_peer_certificate(ssl));
 
-            if (Ssl::Errors *errNoList = static_cast<Ssl::Errors *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_sslerrno)))
-                serverBump->bumpSslErrorNoList = cbdataReference(errNoList);
+            // remember validation errors, if any
+            if (Ssl::Errors *errs = static_cast<Ssl::Errors *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_errors)))
+                serverBump->sslErrors = cbdataReference(errs);
         }
     }
 
@@ -776,7 +778,7 @@ FwdState::initiateSSL()
             SSL_set_ex_data(ssl, ssl_ex_index_server, (void*)hostname);
 
         // Use SNI TLS extension only when we connect directly
-        // to the origin server and we know the server host name
+        // to the origin server and we know the server host name.
         if (!hostnameIsIp)
             Ssl::setClientSNI(ssl, hostname);
     }
index 8d8222429ea4a63651aa006807feda472f1269d0..4463ee90792d515e2f75fd7455703d073a8657b8 100644 (file)
@@ -153,7 +153,7 @@ extern "C" {
     extern int ssl_ex_index_cert_error_check;  /* -1 */
     extern int ssl_ex_index_ssl_error_detail;      /* -1 */
     extern int ssl_ex_index_ssl_peeked_cert;      /* -1 */
-    extern int ssl_ex_index_ssl_error_sslerrno;   /* -1 */
+    extern int ssl_ex_index_ssl_errors;   /* -1 */
 
     extern const char *external_acl_message;      /* NULL */
     extern int opt_send_signal;        /* -1 */
index 2c8c4090c534c33ead89f13d2252d5db9844aa91..6c36fb511a9967873d18b51a8e8d8733dbeff2e2 100644 (file)
@@ -109,7 +109,7 @@ static const Ssl::ssl_error_t certSelfSigned[] = {X509_V_ERR_DEPTH_ZERO_SELF_SIG
 // The list of error name shortcuts  for use with ssl_error acls.
 // The keys without the "ssl::" scope prefix allow shorter error
 // names within the SSL options scope. This is easier than
-// carefully stripping the scope prefix in Ssl::ParseErrorString()
+// carefully stripping the scope prefix in Ssl::ParseErrorString().
 static SslErrorAlias TheSslErrorShortcutsArray[] = {
     {"ssl::certHasExpired", hasExpired},
     {"certHasExpired", hasExpired},
@@ -124,7 +124,7 @@ static SslErrorAlias TheSslErrorShortcutsArray[] = {
     {NULL, NULL}
 };
 
-//Use std::map to optimize search
+// Use std::map to optimize search.
 typedef std::map<std::string, const Ssl::ssl_error_t *> SslErrorShortcuts;
 SslErrorShortcuts TheSslErrorShortcuts;
 
@@ -340,7 +340,7 @@ const char *Ssl::ErrorDetail::err_lib_error() const
 }
 
 /**
- * It converts the code to a string value. Supported formating codes are:
+ * Converts the code to a string value. Supported formating codes are:
  *
  * Error meta information:
  * %err_name: The name of a high-level SSL error (e.g., X509_V_ERR_*)
index b26a87a66e0121cb560b003de823ac45ccb8e882..9a4cc0ae5392ee44785e927c2923478d7fbdd859 100644 (file)
@@ -15,10 +15,10 @@ namespace Ssl
 {
 /**
   \ingroup ServerProtocolSSLAPI
- * The Ssl::Errors representation of the error described by "name".
- * The result may be a single element of a list of errors, and needs to be
+ * Converts user-friendly error "name" into an Ssl::Errors list.
+ * The resulting list may have one or more elements, and needs to be
  * released by the caller.
- * This function also parses numeric arguments.
+ * This function can handle numeric error numbers as well as names.
  */
 Ssl::Errors *ParseErrorString(const char *name);
 
@@ -59,7 +59,7 @@ public:
     ssl_error_t errorNo() const {return error_no;}
     ///Sets the low-level error returned by OpenSSL ERR_get_error()
     void setLibError(unsigned long lib_err_no) {lib_error_no = lib_err_no;}
-    ///The peer certificate
+    /// the peer certificate
     X509 *peerCert() { return peer_cert.get(); }
     /// peer or intermediate certificate that failed validation
     X509 *brokenCert() {return broken_cert.get(); }
index bac940f24a29d1732d3f9b576cfba7728fbc4617..95ba24d1d6a4bccd290f3432c2152b26a3d040bd 100644 (file)
@@ -18,7 +18,7 @@ CBDATA_NAMESPACED_CLASS_INIT(Ssl, ServerBump);
 
 Ssl::ServerBump::ServerBump(HttpRequest *fakeRequest, StoreEntry *e):
     request(fakeRequest),
-    bumpSslErrorNoList(NULL)
+    sslErrors(NULL)
 {
     debugs(33, 4, HERE << "will peek at " << request->GetHost() << ':' << request->port);
     const char *uri = urlCanonical(request);
@@ -40,6 +40,6 @@ Ssl::ServerBump::~ServerBump()
         storeUnregister(sc, entry, this);
         entry->unlock();
     }
-    cbdataReferenceDone(bumpSslErrorNoList);
+    cbdataReferenceDone(sslErrors);
 }
 
index df62acae61e90628f4fcba868acfea37fbda05f1..acaf91b0b103740596e6b0c613cbaae3eb6c82d7 100644 (file)
@@ -14,18 +14,19 @@ namespace Ssl
 
 /**
   \ingroup ServerProtocolSSLAPI
-  * Used to store bump-server-first related informations
+ * Maintains bump-server-first related information.
  */
 class ServerBump
 {
 public:
     explicit ServerBump(HttpRequest *fakeRequest, StoreEntry *e = NULL);
     ~ServerBump();
+
     /// faked, minimal request; required by server-side API
     HttpRequest::Pointer request;
     StoreEntry *entry; ///< for receiving Squid-generated error messages
     Ssl::X509_Pointer serverCert; ///< HTTPS server certificate
-    Ssl::Errors *bumpSslErrorNoList; ///< The list of SSL certificate errors which ignored
+    Ssl::Errors *sslErrors; ///< SSL [certificate validation] errors
 
 private:
     store_client *sc; ///< dummy client to prevent entry trimming
index 708d2f07f01da519735411fceafe91dfc74dede7..618f976208c004f259a6447b92b9c0394a427d59 100644 (file)
@@ -229,9 +229,8 @@ bool Ssl::CertificateDb::addCertAndPrivateKey(Ssl::X509_Pointer & cert, Ssl::EVP
     }
     row.setValue(cnlSerial, serial_string.c_str());
     char ** rrow = TXT_DB_get_by_index(db.get(), cnlSerial, row.getRow());
-    // We are creating certificates with unique serial number. If the serial
-    // number found in the database, means that the certificate already exist
-    // in the database
+    // We are creating certificates with unique serial numbers. If the serial
+    // number is found in the database, the same certificate is already stored.
     if (rrow != NULL)
         return true;
 
index 7891ace37659cf04392e599dec427e1116017207..cd787a2796fc8480f5ad53e05ccda956836f594f 100644 (file)
@@ -64,6 +64,7 @@ public:
     */
     void composeBody(BodyParams const & map, std::string const & other_part);
 
+    /// orchestrates entire request parsing
     bool parseRequest(Ssl::CertificateProperties &, std::string &error);
     void composeRequest(Ssl::CertificateProperties const &); // throws
 
index 550d7276ebbbcc8df9bdd39eac4988421b240bbc..9a8abe58300e3969a10e678000813c6282190a5d 100644 (file)
@@ -207,6 +207,7 @@ static bool proccessNewRequest(Ssl::CrtdMessage & request_message, std::string c
         throw std::runtime_error("Error while parsing the crtd request: " + error);
 
     Ssl::CertificateDb db(db_path, max_db_size, fs_block_size);
+
     Ssl::X509_Pointer cert;
     Ssl::EVP_PKEY_Pointer pkey;
     std::string &cert_subject = certProperties.dbKey();
index 011db2ae46fa7fccaf33b6b0d26f928ffe4c38e3..0b7d8c960d2b31e38a08e7bcdbc117eac1dd799c 100644 (file)
@@ -244,7 +244,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
     }
 
     if (ok && peeked_cert) {
-        /*Check if the already peeked certificate match the new one*/
+        // Check whether the already peeked certificate matches the new one.
         if (X509_cmp(peer_cert, peeked_cert) != 0) {
             debugs(83, 2, "SQUID_X509_V_ERR_CERT_CHANGE: Certificate " << buffer << " does not match peeked certificate");
             ok = 0;
@@ -253,17 +253,17 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
     }
 
     if (!ok) {
-        Ssl::Errors *errNoList = static_cast<Ssl::Errors *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_sslerrno));
-        if (!errNoList) {
-            errNoList = new Ssl::Errors(error_no);
-            if (!SSL_set_ex_data(ssl, ssl_ex_index_ssl_error_sslerrno,  (void *)errNoList)) {
+        Ssl::Errors *errs = static_cast<Ssl::Errors *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_errors));
+        if (!errs) {
+            errs = new Ssl::Errors(error_no);
+            if (!SSL_set_ex_data(ssl, ssl_ex_index_ssl_errors,  (void *)errs)) {
                 debugs(83, 2, "Failed to set ssl error_no in ssl_verify_cb: Certificate " << buffer);
-                delete errNoList;
-                errNoList = NULL;
+                delete errs;
+                errs = NULL;
             }
         }
-        else // Append the err no to the SSL errors lists.
-            errNoList->push_back_unique(error_no);
+        else // remember another error number
+            errs->push_back_unique(error_no);
 
         if (const char *err_descr = Ssl::GetErrorDescr(error_no))
             debugs(83, 5, err_descr << ": " << buffer);
@@ -272,17 +272,16 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
 
         if (check) {
             ACLFilledChecklist *filledCheck = Filled(check);
-            assert(filledCheck->sslErrorList == NULL);
-            filledCheck->sslErrorList = new Ssl::Errors(error_no);
+            assert(!filledCheck->sslErrors);
+            filledCheck->sslErrors = new Ssl::Errors(error_no);
             if (check->fastCheck() == ACCESS_ALLOWED) {
                 debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
                 ok = 1;
             } else {
                 debugs(83, 5, "confirming SSL error " << error_no);
             }
-            // Delete the ssl error list
-            delete filledCheck->sslErrorList;
-            filledCheck->sslErrorList = NULL;
+            delete filledCheck->sslErrors;
+            filledCheck->sslErrors = NULL;
         }
     }
 
@@ -615,11 +614,11 @@ ssl_free_ErrorDetail(void *, void *ptr, CRYPTO_EX_DATA *,
 }
 
 static void
-ssl_free_SslErrNoList(void *, void *ptr, CRYPTO_EX_DATA *,
+ssl_free_SslErrors(void *, void *ptr, CRYPTO_EX_DATA *,
                      int, long, void *)
 {
-    Ssl::Errors *errNo = static_cast <Ssl::Errors *>(ptr);
-    delete errNo;
+    Ssl::Errors *errs = static_cast <Ssl::Errors*>(ptr);
+    delete errs;
 }
 
 // "free" function for X509 certificates
@@ -671,7 +670,7 @@ ssl_initialize(void)
     ssl_ex_index_cert_error_check = SSL_get_ex_new_index(0, (void *) "cert_error_check", NULL, &ssl_dupAclChecklist, &ssl_freeAclChecklist);
     ssl_ex_index_ssl_error_detail = SSL_get_ex_new_index(0, (void *) "ssl_error_detail", NULL, NULL, &ssl_free_ErrorDetail);
     ssl_ex_index_ssl_peeked_cert  = SSL_get_ex_new_index(0, (void *) "ssl_peeked_cert", NULL, NULL, &ssl_free_X509);
-    ssl_ex_index_ssl_error_sslerrno =  SSL_get_ex_new_index(0, (void *) "ssl_error_sslerrno", NULL, NULL, &ssl_free_SslErrNoList);
+    ssl_ex_index_ssl_errors =  SSL_get_ex_new_index(0, (void *) "ssl_errors", NULL, NULL, &ssl_free_SslErrors);
 }
 
 /// \ingroup ServerProtocolSSLInternal
index 4144042077dbecaa4d1353f1828984b5222719b7..76a437d19a01f7d67832fc99663eb8511b114fc3 100644 (file)
@@ -210,7 +210,6 @@ int asn1timeToString(ASN1_TIME *tm, char *buf, int len);
    \return true if SNI set false otherwise
 */
 bool setClientSNI(SSL *ssl, const char *fqdn);
-
 } //namespace Ssl
 
 #if _SQUID_MSWIN_