Re: [PATCH] Tying validation errors to certificates

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

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

Amos
Received on Tue Jun 04 2013 - 02:37:04 MDT