Re: [PATCH] Dynamic SSL Certificate Generation

Give me some time to resolve the changes you are requested and see the
problem and what we can do about.
It is something I should see before post the patch .

On 11/11/2010 04:24 PM, Amos Jeffries wrote:
> On 12/11/10 03:05, Tsantilas Christos wrote:
>> On 11/11/2010 02:26 PM, Amos Jeffries wrote:
>>> On 11/11/10 23:38, Tsantilas Christos wrote:
>>>> On 11/11/2010 03:11 AM, Amos Jeffries wrote:
>>>>> On Wed, 10 Nov 2010 17:05:16 +0200, Tsantilas Christos
>>>>>>
>>>>>> * Introduce the TidyPointer class similar to std::auto_ptr, which
>>>>>> implements a pointer that deletes the object it points to when the
>>>>>> pointer's owner or context is gone. It is designed to avoid memory
>>>>>> leaks in the presence of exceptions and processing short cuts.
>>>>>
>>>>> So whats different about this new pointer type that the existing
>>>>> std:auto_ptr, RefCount::Pointer and CbcPointer are all insufficient?
>>>>
>>>> The TidyPointer is completely different than RefCount and CbcPointers.
>>>>
>>>> It is similar to the std::auto_ptr but allow developer to define the
>>>> deallocator method for the object it points to.
>>>> Also does not implement the "=" operator and prevents developers from
>>>> using the copy constructor.
>>>>
>>>> It can be usefull when you have code as the following:
>>>> -----------------------
>>>> A *a= newA();
>>>> if (!a)
>>>> return NULL;
>>>> B *b=newB();
>>>> if (!b) {
>>>> freeA(a);
>>>> return NULL;
>>>> }
>>>> C *c=newC();
>>>> if (!c) {
>>>> freeA(a);
>>>> freeB(b);
>>>> return NULL;
>>>> }
>>>>
>>>> If(problem){
>>>> freeA(a);
>>>> freeB(b);
>>>> freeC(c);
>>>> return NULL;
>>>> }
>>>> return C;
>>>>
>>>> Using TidyPointer the above can be written as:
>>>>
>>>> TidyPointer<A> a(newA(),freeA);
>>>> if (!a)
>>>> return NULL;
>>>> TidyPointer<B> b(newB(),freeB);
>>>> if (!b)
>>>> return NULL;
>>>> TidyPointer<C> c(newC(), freeC);
>>>> if (!c)
>>>> return NULL;
>>>>
>>>> if (problem)
>>>> return NULL;
>>>> return C.release();
>>>>
>>>>
>>>> Can help you to avoid memory leeks when you have the following case:
>>>>
>>>> A *a= newA();
>>>> DoSomeWorkWhichMayThrow(a);
>>>> freeA(a);
>>>>
>>>> In the above case if the DoSomeWorkWhichMayThrow() throws the a object
>>>> will never be released.
>>>>
>>>> The above can be written:
>>>>
>>>> TidyPointer<A> a(newA(),freeA);
>>>> DoSomeWorkWhichMayThrow(a.get());
>>>>
>>>>>
>>>>>> From what I've heard of the SSL problems they are all cause by it not
>>>>> using RefCount on the contexts themselves.
>>>>>
>>>>
>>>> Maybe RefCount is usefull for the SSL_CTX objects. But I do not
>>>> think it
>>>> is required.
>>>>
>>>> In most cases using the openSSL library you have code like the
>>>> following:
>>>>
>>>> a=NewA();
>>>> b=newB(a);
>>>> if (!b) {
>>>> freeA(a);
>>>> }
>>>> else {
>>>> //Just forget the a
>>>> }
>>>>
>>>> I these cases the TidyPointer is very useful.
>>>>
>>>> Regards,
>>>> Christos
>>>>
>>>
>>> Yes. Thank you for that.
>>>
>>> So it is a special case of RefCount() with enforced maximum count of 1.
>> Not exactly.
>>
>>> Using a specified C-function fro freeing instead of the objects own
>>> destructor. So a plus for C objects without destructors.
>> Yes.
>>
>>>
>>> Like all the smart pointers TidyPointer will delete it's data if it ever
>>> dies. Yet it provides a get() method which allows duplication of the raw
>>> pointer. This permits the following case:
>>>
>>> TidyPointer<a> A(a, free-a);
>>> void *B = A->get();
>>> A = NULL;
>>> if (!B) ...
>>
>> It does not allow "=" operator to be used as used in your example.
>>
>> But yes, someone can write:
>>
>> TidyPointer<a, free-a> A(a);
>> void *B = A.get();
>> A.reset(NULL);
>> if (!B) ...
>>
>> When the "A.reset(NULL)" statement called the B points to non valid
>> memory. But this is a bad use of TidyPointer.
>> The correct usage of the above is the following:
>>
>> TidyPointer<a, free-a> A(a);
>> void *B = A.release();
>> if (!B) ...
>>
>> The TidyPointer is designed to be used for local use (eg. inside
>> functions/methods ), or passed as const to other functions/methods, or
>> for objects allocated once and used as read-only objects until they are
>> going to be released.
>>
>> The TidyPointer does not have implemented "=" operator, does not permit
>> copy constructor, it force you to use "::reset(T)" and "::release()"
>> methods to modify the pointer. These restrictions are features.
>> It is a (new) tool which has completely different purpose of the
>> RefCount so I believe should be a different/separate class.
>>
>>
>>>
>>> Use of RefCount is preferrable here since B can be recognized.
>>>
>>>
>>> Over in the dynamic generation code this nasty usage case is implemented
>>> by the http_port contexts and again in the context "storage".
>>>
>>> Specifically on the http_port... if you recall the history of bug
>>> http://bugs.squid-cache.org/show_bug.cgi?id=2586 the destructor of
>>> http_port globals are perfectly capable of cleanly erasing the member
>>> contexts. We had to leave the SSL context leaking until they could be
>>> *RefCounted* (with a full 1->N count) because active connections held
>>> raw-pointers to it long after the "owner" port was closed.
>>> Using a TidyPointers there, with get() method in httpsAccept() to attach
>>> to SSL* children is exactly identical logic to our previous attempt at
>>> making the http_port destructor cleanup.
>>
>> hmm, I had forgot this bug.
>> About SSL_CTX objects (and SSL * ?) we may have to use RefCounts. But it
>> is not so simple. You can not just use a different type of pointer.
>
> Yes. So as I see it, until this problems is resolved the http_port will
> need to stay using a raw-pointer.
> The context "storage" containers will need a look to see if they suffer
> the same problem or resolve it somehow.
>
>
>> The major problem is that we are not passing RefCount or TidyPointers to
>> the openSSL library functions. We are passing raw pointers.
>
> If they do not throw that is not a problem. Or if they do throw we much
> catch in our caller where the pointer can be treated to special cleanup.
>
>>
>> From what I can understand the major problem is that on reconfigure
>> there are SSL_CTX objects which are used by some SSL objects so the
>> SSL_CTX objects can not be released.
>
> Yes exactly.
>
>>
>> Most possible is that we should write a new object classes which
>> represents SSL objects and their dependencies on SSL_CTX objects...
>
> Maybe.
>
> Or move to GnuTLS and NSS instead of OpenSSL which has been requested by
> several groups of people. Apparently GnuTLS has a nicer API as well,
> though I have not looked.
>
> Amos
Received on Thu Nov 11 2010 - 14:44:26 MST