Re: [PATCH] Dynamic SSL Certificate Generation

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.
The major problem is that we are not passing RefCount or TidyPointers to
the openSSL library functions. We are passing raw pointers.

 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.

Most possible is that we should write a new object classes which
represents SSL objects and their dependencies on SSL_CTX objects...

>
>
> Amos
Received on Thu Nov 11 2010 - 14:05:29 MST