[squid-dev] [PREVIEW] Fetch missing certificates

[Amos Jeffries]

Sorry for not geting to this a long time back. It seems to have got lost
behind the xmas/holiday email backlog.

On 23/12/2015 6:35 a.m., Christos Tsantilas wrote:
> 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.

Please no more ConnStateData derived classes. ::Server is the head of
that hierarchy and the servers/ classes as the per-protocol children.

And Um...

ConnStateData is a ::Server class about managing the squid end of a
client->Squid connection.

"Download" activity is a ::Client class type of activity, which is about
managing the Squid end of a Squid->server connection.


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.


> 
>   - 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

Okay in theory. But please do this (and below?) as a separate feature to
the Downloader. They will each have a fair amount of discussion + audit
and there is no need to have this block this if the other get stuck on
some detail.

> 
>   - 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
> 

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.

Amos