Re: [PATCH] Server certificate error to user

On 02/12/10 11:59, Tsantilas Christos wrote:
> On 12/01/2010 01:16 PM, Amos Jeffries wrote:
>> On 30/11/10 23:16, Tsantilas Christos wrote:
>>>
>>> Hi all,
>>> This patch adds an interface to allow Squid error responses to contain
>>> detailed information about SSL certificate verification failure. For
>>> example, the error message may contain the following text:
>>>
>>> "Server Certificate Verification Failed: Certificate Common Name
>>> (www.lufthansa.com) does not match the host name you are connecting to
>>> (www.lufthansa.de)."
>>>
>>> Supplying SSL error details is useful to end-users if the user can
>>> bypass errors or communicate with proxy operators to update Squid's
>>> whitelists.
>>>
>>> For more informations please look inside patch documentation.
>>>
>>> This is a Measurement Factory project.
>>>
>>> Regards,
>>> Christos
>>
>> (I'm still not here for a day or so, will check the patch then).
>>
>> Right now I just want to raw your attention to %Z which is for use as an
>> internal error messages like this.
>
> Yes, I saw this formating code but it is documented as "preformated
> error message". The new code is not exactly like this.
>

Ah I see the confusion.
That "preformatted" is related to the HTML syntax. %Z is embeded inside
<pre></pre> tags. %z is not guaranteed to be. In the more recent
templates it is only due to a bug in html2po.

My reasons for picking it were that it had no sub-case logics already,
is currently only generated by internal processes (ESI) instead of
unknown input from external sources and generally looked the easiest to use.

>>
>> Doing the Ssl recursion logic in the case where err_msg is empty for %Z
>> seems to be possible without adding a new code to existing page.
>> Alternatively, ESI appears to be the only code using err_msg. You could
>> followup with a patch which alters ESI to use the same *::ErrorDetails
>> recursion API and drop err_msg entirely. :)
>
> I agree it will be good to merge two codes to one.
> In this case we should define a base ErrorDetail class as follows:
> class ErrorDetail{
> public:
> virtual const String &toString() const;
> }
>
> And define a new class ESI::ErrorDetail for the ESI.
>
> But the same we should do with %z (dns server error message), ftp
> related formating codes, %m (Error message returned by auth helper), %o
> (Message returned external acl helper) and maybe other formating codes.

True. They are a separate merger project.

(NP: from here on is discussion of outside the scope of this patch or
alteration requests).

  Their logics could be converted and combined to this API.

  Their template file codes can't be changed without forking the
langpack. I'd rather do that only at the point when we upgrade the
template macros to something more flexible, such as the logformat codes.

Meanwhile we can try and avoid adding new codes which have to be handled
at that time.

>
> This project was just for SSL related error details, but I hope it is
> written in such way that it will be easy for us to extend it and support
> error details from other modules to.
>
> Also I think with a little work can be extended to use language
> templates in the future.

Possibly. The normal API gettext() does not support remote-end language
requirements. Nor does it support HTTP language codes. So translation of
internal strings right now is slightly complex.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.9
   Beta testers wanted for 3.2.0.3