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 <[email protected]> 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 <[email protected]> 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 later. >> >> This element would then be allocated from a pool, attached there, and >> released+nulled once the session is completed, or when freeing the >> SSL context. >> >> In any case, please be extremely cautious about what you place there. >> We're counting *bytes* in every struct, not kilobytes, especially for >> the front-facing ones. For example the struct connection is already >> 3 cache lines (192 bytes), adding a pointer there means rounding up >> to the next cache line and it goes 256 bytes. At 10 million concurrent >> connections during an attack, that's 640 MB of extra RAM. Said >> differently, when your CPU has only a 10 MB L3 cache, while 54k >> connections were enough to ruin the cache before, now only 40k become >> sufficient to do the same. >> >> There's nothing critical in any case, it's just an example to give you >> some figures that we use all day long in the design to maintain a good >> level of performance for everyone, so that you know what to care about >> during your progress on this. >> >> Cheers, >> Willy >> >
