Re: [squid-dev] [PREVIEW] Fetch missing certificates
On 02/09/2016 09:59 AM, Amos Jeffries wrote: > "Download" activity is a ::Client class type of activity, which is about > managing the Squid end of a Squid->server connection. I am afraid you got it backwards: Downloader is a request source (and a response sink). Downloader does not actually care about the Squid-to-server connection! It is up to Squid to manage Downloader request and deliver the response back to Downloader (with or without the connection to the origin server or peer). > I am aware there is a need for the store logics to be involved. But > surely StoreEntry and/or clientStreams does not require the whole > ::Server 'side' of Squid to be involved. Besides StoreEntry and/or clientStreams, download requests should go through the regular doCallout() processing sequence including StoreID, ICAP, and other complex things. Thus, I am sure Downloader needs to be in the same group of classes as ConnStateData. > Please no more ConnStateData derived classes. ::Server is the head of > that hierarchy and the servers/ classes as the per-protocol children. As for the "Server" parent specifically, I cannot yet say whether your request is valid. As I said earlier in another thread, I need more time to study what that unaudited class is and what it should be. Based on the current "used to manage connections from clients" description alone, it is obviously the wrong parent for Downloader, but I do not claim that the description itself is correct. FWIW, the introduction of that Server class is the primary reason we have not posted the long-polished "Fetch missing certificates" patch for review... > Looking at the patch I also see you have added a thing called > BinaryTokenizer, but the Tokenizer we already have operates on binary > octets. All that seems needed is some new accessor(s) for fetching the > fixed-width binary fields alongside the current int64() ASCII->integer > decoder. * Tokenizer API is about sequences of characters -- it is built around CharacterSet. When it retrieves a number, it may retrieve "012345" characters. * BinaryTokenizer is about sequences of bits. When it retrieves a number, it may retrieve 64 bits. While it all boils down to "binary octets" at some level, it would be a design mistake to mix the two concepts in one class IMO; just like it would be a mistake to add "some accessors" to HTTP parser to parse FTP messages, even though both HTTP and FTP deal with the same kind of input token streams. It is possible to create a common Tokenizer parent for TextTokenizer and BinaryTokenizer, but I think we should wait for BinaryTokenizer to mature first. It is possible that it (or another *Tokenizer class using it) will start dealing with not-byte-aligned bits. If that happens, building a common parent for all tokenizers would be awkward and expensive performance-wise. Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
[squid-dev] [PREVIEW] Fetch missing certificates
This is a preview patch. The internal review procedure is not finished yet, there are some TODOs, which probably we need to address them. However I am posting it here to start a discussion on this patch, which is a little big and important. Patch description: Many web servers do not have complete certificate chains. Many browsers use certificate extensions of the server certificate and download the missing intermediate certificates automatically from the Internet. This patch add this feature to Squid. The information for missing issuer certificates provided by the Authority Information Access X509 extension. This describes the format and location of additional information provided by the issuer of the certificate in which in which this extension appears. If the caIssuers access method provided then the issuer certificate information provided and the access location field exist in thois extension provides the location of the issuer certificate. This patch: - Implements an Downloader class as ConnStateData kid class. This new class can be used by internal squid subsystems to download objects from net. - Modify Ssl::PeerConnector class to use new Downloader class to retrieve missing certificaes from the net. It retrieved the URIs of missing certificates from the Authority Information Access X509 extension. - Implements a new SSL records and SSL handshake messages parser (Ssl::HandshakeParser class) to improve current SSL messages parsing. The new parser now used to check if a Change Cipher Spec message included in server hello. The related code removed from Ssl::Bio::sslFeatures class - Modify the Ssl::ServerBio class to: * Buffer the Server Hello message and process it before pass it to the openSSL library. * Extract server certificates from server hello message. This is required to check if there are missing certificates, and if yes give the chance to squid to download missing certificates and complete certificate chains before pass them for processing to openSSL - Fixes and improves the Ssl::Bio related code. This is a Measurement Factory project Fetch missing certificates Many web servers do not have complete certificate chains. Many browsers use certificate extensions of the server certificate and download the missing intermediate certificates automatically from the Internet. This patch add this feature to Squid. The information for missing issuer certificates provided by the Authority Information Access X509 extension. This describes the format and location of additional information provided by the issuer of the certificate in which in which this extension appears. If the caIssuers access method provided then the issuer certificate information provided and the access location field exist in thois extension provides the location of the issuer certificate. This patch: - Implements an Downloader class as ConnStateData kid class. This new class can be used by internal squid subsystems to download objects from net. - Modify Ssl::PeerConnector class to use new Downloader class to retrieve missing certificaes from the net. It retrieved the URIs of missing certificates from the Authority Information Access X509 extension. - Implements a new SSL records and SSL handshake messages parser (Ssl::HandshakeParser class) to improve current SSL messages parsing. The new parser now used to check if a Change Cipher Spec message included in server hello. The related code removed from Ssl::Bio::sslFeatures class - Modify the Ssl::ServerBio class to: * Buffer the Server Hello message and process it before pass it to the openSSL library. * Extract server certificates from server hello message. This is required to check if there are missing certificates, and if yes give the chance to squid to download missing certificates and complete certificate chains before pass them for processing to openSSL - Fixes and improves the Ssl::Bio related code. This is a Measurement Factory project === modified file 'src/Debug.h' --- src/Debug.h 2015-08-24 17:49:50 + +++ src/Debug.h 2015-10-29 16:15:23 + @@ -142,52 +142,58 @@ } /* Legacy debug style. Still used in some places. needs to die... */ #define do_debug(SECTION, LEVEL) ((Debug::level = (LEVEL)) <= Debug::Levels[SECTION]) #define old_debug(SECTION, LEVEL) if do_debug((SECTION), (LEVEL)) _db_print /* Legacy debug function definitions */ void _db_init(const char *logfile, const char *options); void _db_print(const char *,...) PRINTF_FORMAT_ARG1; void _db_set_syslog(const char *facility); void _db_rotate_log(void); /// Prints raw and/or non-terminated data safely, efficiently, and beautifully. /// Allows raw data debugging in debugs() statements with low debugging levels /// by printing only if higher section debugging levels are configured: /// debugs(11, DBG_IMPORTANT, "always printed" << Raw(may be
