Re: Memory allocation for haproxy

2021-05-03 Thread Robert Ionescu
Hi all,

any possible feedback on my last Email, maybe it was missed ?

I have already tried a couple of ways to implement the new structures, but
somehow I am still running into segmentation faults, most probably caused
by the way the memory allocation works. Can somebody support me there ?

BR,
___
Robert Ionescu

*The information contained in this message is confidential and may be
legally privileged. The message is intended solely for the addressee(s). If
you are not the intended recipient, you are hereby notified that any use,
dissemination, or reproduction is strictly prohibited and may be unlawful.
If you are not the intended recipient, please contact the sender by return
e-mail and destroy all copies of the original message.*


On Tue, Apr 27, 2021 at 10:41 AM Robert Ionescu 
wrote:

> Hi Willy,
>
> Thank you again for your extremely detailed answer!
>
> I will go with the ssl_sock_ctx struct way you have mentioned in your last
> email.
>
> Pseudocode style for overview:
>
> Inside the ssl_sock_ctx struct I will add a new pointer to a new struct
> (let's call it tls_error_log for now)
>
> struct ssl_sock_ctx {
> ...
> struct tls_error_log *tel;
> }
>
> The new struct will look something like this:
>
> struct tls_error_log {
>char cert_fingerprint;
>char cert_serial;
>char cert_dn;
>char cert_issuer;
> }
>
> So far so good. To implement a new struct in the same way as haproxy is
> designed, where do I need to declare this struct? Is it enough to implement
> it into the corresponding .h and .c files and use the DECLARE_POOL and
> pool_free functions to manage the memory for it or do I need to take care
> of something else for this struct ?
>
> Thank you and best regards,
>
> ___
> Robert Ionescu
>
> *The information contained in this message is confidential and may be
> legally privileged. The message is intended solely for the addressee(s). If
> you are not the intended recipient, you are hereby notified that any use,
> dissemination, or reproduction is strictly prohibited and may be unlawful.
> If you are not the intended recipient, please contact the sender by return
> e-mail and destroy all copies of the original message.*
>
>
> On Tue, Apr 27, 2021 at 7:22 AM Willy Tarreau  wrote:
>
>> Hi Robert,
>>
>> I seem to have missed this message from you the other day.
>>
>> On Thu, Apr 22, 2021 at 10:07:03AM +0200, Robert Ionescu wrote:
>> > Thank you for your reply.
>> >
>> > I am working on the TLS error log issue which is already discussed on
>> > github (https://github.com/haproxy/haproxy/issues/693)
>>
>> OK, but please do not hesitate to at least mention it when you're working
>> on an issue, as it can sometimes happen that several persons will do the
>> same at the same time, which can cause some trouble when they finally
>> start to submit patches. It at least ends up with some frustration, and
>> usually for the two :-/
>>
>> I know that often, people do not want to engage too much and prefer to
>> stay discrete about their explorations of the code, but there's nothing
>> wrong with just writing "Let's see if I manage to get something done".
>> And as a bonus you could have some supporters willing to test your code!
>>
>> > Currently I am calculating all the client certificate details in
>> > the ssl_sock_bind_verifycbk() function and now I need the best way to
>> pass
>> > this information over to the log.
>> > My idea was to extend the connection struct with client details like
>> > certificate fingerprint (SHA1), DN, Serial and maybe the issuer. This
>> means
>> > that I need some char arrays there, where the size of these arrays is
>> not
>> > really random but due to the fact that the issuer is not known, I cannot
>> > tell the exact array size.
>>
>> Normally you don't need this because the logs support calling sample
>> fetch functions which will retrieve such information from the TLS stack.
>> Maybe there are *some* info that are not exposed and which would need to.
>> Maybe there are also some info that are known only during the callback
>> and that cannot be retrieved later, and which would then possibly deserve
>> being stored into the SSL context.
>>
>> > Afterwards, if the connection struct is working for this approach, I
>> > want to pass this information to the logger from
>> > the session_kill_embryonic() function in case of an error.
>>
>> It's better not to store it into this struct because it tries hard not
>> to keep transport-specific info which can become huge, and over time
>> we managed to shrink it in order to remove lots of non-systematic stuff.
>> For TLS, you should instead use the SSL context. There's this ssl_sock_ctx
>> struct that contains some info and into which you could possibly add a
>> pointer to a dynamic struct that would be used only during the initial
>> handshake and released 

Re: Memory allocation for haproxy

2021-04-15 Thread Willy Tarreau
Hi Robert,

On Wed, Apr 14, 2021 at 12:18:42PM +0200, Robert Ionescu wrote:
> Hi all,
> 
> I am working on a new patch for haproxy and would need the malloc method
> used to allocate memory for structs like the connection struct in
> connection-t.c
> In case I want to expand the struct with a new variable, do I need to take
> care of the memory for this variable or is there already a dynamic memory
> allocation for the given struct?

I'd respond: "it depends". Since the vast majority of our structs are of
fixed sizes, we use fixed-size pools to allocate them, so this normally
is the way to go. If the fields you need to add are of relatively small
size (a few bytes to a few tens of bytes), let's just add them to the
structure, and possibly try to figure if they're structurally exclusive
with other fields, in which case you could make a union between them.
If you're going to deal with several tens of bytes to a few kB, but of
mostly fixed size (or I'd say bounded size), then I guess it's not always
used so you'd rather create a new pool for this and store a pointer in
the first struct to this entry.

If the size is completely random, then you'd rather store a pointer in
the struct to something allocated using malloc(), but then you'll really
have to be extremely careful about operating systems dealing with a slow
memory allocator (especially in threaded environments) and be prepared
to see your design seriously questioned if you want to submit it, so
it'd better be discussed first to be sure you're on the right track!

Cheers,
Willy



Memory allocation for haproxy

2021-04-14 Thread Robert Ionescu
Hi all,

I am working on a new patch for haproxy and would need the malloc method
used to allocate memory for structs like the connection struct in
connection-t.c
In case I want to expand the struct with a new variable, do I need to take
care of the memory for this variable or is there already a dynamic memory
allocation for the given struct?

Best regards
___
Robert Ionescu

*The information contained in this message is confidential and may be
legally privileged. The message is intended solely for the addressee(s). If
you are not the intended recipient, you are hereby notified that any use,
dissemination, or reproduction is strictly prohibited and may be unlawful.
If you are not the intended recipient, please contact the sender by return
e-mail and destroy all copies of the original message.*