Re: [PATCH] APT https method improvements

[Michael Vogt <mvogt@acm.org>]

On Mon, Jun 02, 2008 at 06:35:32PM +0200, Arnaud Ebalard wrote:
> Hi,
Hi,
 
> Attached is a set of simple patches for apt https method developed by
> Axel and I. They add client authentication, CRL handling, and ability to
> check issuer, and also some other things that are described in detail
> below.

Thanks a lot for your patch and sorry for the late reply.
 
> Note that this post is more a request for comments and preliminary code
> review (for instance w.r.t documentation patch) for the follwing reason:
> Because support for CRL handling and issuer check were missing 
> in the underlying library (libcurl-gnutls), we submitted [1] a set of
> patches to curl developers providing those functionalitie. They were
> accepted but came during a freeze window and will only be in 7.18.3 (see
> [2]). For that reason, we thought we would request your comments in the
> meantime for apt patches.
[..]

For when is 7.18.3 scheduled? 
 

> Index: apt-0.7.13/methods/https.cc
> ===================================================================
> --- apt-0.7.13.orig/methods/https.cc	2008-05-30 12:37:20.866185027 +0200
> +++ apt-0.7.13/methods/https.cc	2008-06-02 16:32:03.270155899 +0200
> @@ -108,6 +108,8 @@
>     struct curl_slist *headers=NULL;  
>     char curl_errorstr[CURL_ERROR_SIZE];
>     long curl_responsecode;
> +   URI Uri = Itm->Uri;
> +   string remotehost = Uri.Host;
>  
>     // TODO:
>     //       - http::Pipeline-Depth
> @@ -127,23 +129,56 @@
>     curl_easy_setopt(curl, CURLOPT_FAILONERROR, true);
>     curl_easy_setopt(curl, CURLOPT_FILETIME, true);
>  
> -   // FIXME: https: offer various options of verification
> -   bool peer_verify = _config->FindB("Acquire::https::Verify-Peer", false);
> +   // SSL parameters are set by default to the common (non mirror-specific) value
> +   // if available (or a default one) and gets overload by mirror-specific ones.
> +
> +   // File containing the list of trusted CA.
> +   string cainfo = _config->Find("Acquire::https::CaInfo","");
[..]

If this is a file, then we should probably use the "Dir::Etc"
hirarchie together with "FindFile()". So "Dir::Etc::CaInfo". Maybe a
new level under "Dir" instead of "Dir::Etc" if there are a lot of
option. One of Dir::Auth, Dir::TLS, Dir::SSL? (Same for the other bits
below were files are used). Otherwise it looks good.



> Index: apt-0.7.13/doc/examples/apt-https-method-example.conf
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ apt-0.7.13/doc/examples/apt-https-method-example.conf	2008-05-30 12:35:14.050157887 +0200
> @@ -0,0 +1,186 @@
> +/* This file is a sample configuration for apt https method. Configuration
> +   parameters found in this example file are expected to be used in main
> +   apt.conf file, just like other configuration parameters for different
> +   methods (ftp, file, ...).
> +
> +   This example file starts with a common setup that voluntarily exhibits
> +   all available configurations knobs with simple comments. Extended
> +   comments on the behavior of the option is provided at the end for
> +   better readibility. As a matter of fact, a common configuration file
> +   will certainly contain far less elements and benefit of default values
> +   for many parameters.
> +
> +   Because some configuration parameters for apt https method in following
> +   examples apply to specific (fictional) repositories, the associated
> +   sources.list file is provided here:
[..]

Thanks for this configuration example with the documentation, that is
very welcome! 

Thanks,
 Michael