Re: [PATCH] Tying validation errors to certificates

On 06/04/2013 05:36 AM, Amos Jeffries wrote:
> On 4/06/2013 12:40 a.m., Tsantilas Christos wrote:
>> On 06/02/2013 02:35 PM, Amos Jeffries wrote:
>>> On 29/05/2013 8:59 p.m., Tsantilas Christos wrote:
>>>> When Squid sends errors to the certificate validation daemon, the
>>>> daemon
>>>> cannot tell which certificate caused which error. This is especially
>>>> bad
>>>> because the validator has to return that same information in the
>>>> response (the response format requires the validator to match the error
>>>> to the certificate).
>>>> This patch adjust the validation request format to provide that
>>>> information using a set of the following key=value pairs:
>>>>
>>>> error_name_N=the name of the certificate error number N
>>>> error_cert_N=the ID of the certificate which caused error_name_N
>>>>
>>>> where N is non-negative integer. N values start from zero and increase
>>>> sequentially.
>>>>
>>>> This is a Measurement Factory project
>>> I think this problem is a side-effect of not following my suggestion
>>> earlier to split the certificates across concurrency channels. Yes?
>> I think no....
>> The server may sent more than one certificates, eg the site certificate
>> plus an intermediate certificate.
>> The error maybe exist in the first certificate or in the second
>> certificate. Currently we just sent to helper the error name.
>> This patch ties the error to the certificate.
>>
>> For example if the site certificate is expired and we can not find the
>> issuer of intermediate certificate the message will send to the
>> validator helper will looks like the following:
>> cert_0=.....-Site Certificate-.....
>> cert_1=.....-Intermediate Certificate-....
>> error_name_0=X509_V_ERR_CERT_HAS_EXPIRED
>> error_cert_0=cert_0
>> error_name_1=X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY
>> error_cert_1=cert_1
>>
>>> If that were done each channel would be dealing with only one
>>> certificate and its errors. No need to explicitly tie them together like
>>> this.
>>
>> If the server uses a certificate which signed using an intermediate
>> certificate, then the validator helper will need also the intermediate
>> certificate to verify server certificate.
>> In this case we are verifying a chain of certificates.
>> So they must included in the same request.
>
> Okay then. +0.5 from me (since my perl skills are fairly low still).

If there is not any objection I will commit this patch to trunk.

>
> There is one other change which seems unrelated to the cert errors:
>
> - $request =~ s/^host=.*\n//;
> + $request =~ s/^host=.*$//m;
>
> That could also do with a mention about why its there in the commit
> message (or removal).

This change just allow the "host=" parameter to exist anywhere in the
request body data. Before this change it should be placed at the begin
of body data...
I can remove it , or post it as separated patch, if required...

>
> Amos
>
Received on Wed Jun 05 2013 - 08:54:01 MDT