The main() function in unix.c is full of lies

[dx]

Since OFTC is being awful right now, this is quite long, and I need to ping pesco, opening a ticket here.

I'm trying to fix #785. It seemed like a simple fix initially, just moving code around, but the main() function is a mess.

In particular, there's a comment written by pesco in 2008 that says that SSL must be initialized before libotr

​r315.1.39, February 16 2008

      /* Ugly Note: libotr and gnutls both use libgcrypt. libgcrypt
         has a process-global config state whose initialization happpens
         twice if libotr and gnutls are used together. libotr installs custom
         memory management functions for libgcrypt while our gnutls module
         uses the defaults. Therefore we initialize OTR after SSL. *sigh* */

Followed by ssl_init() and otr_init().

That was 2008. Since then, a few things changed.

​r667.1.9, September 30 2010, support OTR as a plugin, this comment was added after the #ifdef OTR_BI:

     /* And in case OTR is loaded as a plugin, it'll also get loaded after
        this point. */

​r702, October 16 2010, call nogaim_init() only once except when using purple, this was added right before the libgcrypt comment.

	/* libpurple doesn't like fork()s after initializing itself, so if
	   we use it, do this init a little later (in case we're running in
	   ForkDaemon mode). */
#ifndef WITH_PURPLE
	nogaim_init();
#endif

And it turns out, nogaim_init() does plugin initialization. Which includes libotr.

So to summarize in a cute table:

Purple	OTR	Protocols	SSL	OTR	Comment
No	Builtin	1st	2nd	3rd	Right
No	Plugin	1st	3rd	2nd	Wrong
Yes	Builtin	3rd	1st	2nd	Right
Yes	Plugin	2nd	1st	3rd	Right

First two columns for the compile flags, next three for the initialization order of each of those. "Protocols" being whatever nogaim_init() does - and it happens to have plugin initialization at the end.

So the most common setup (having otr as a plugin and no libpurple) shows the first two comments above are probably a lie.

---

So, initializing libotr before gnutls certainly doesn't result in crashes.

1. Why was that first comment added, then?

2. Is the only side effect of this that free()d memory doesn't get completely wiped as otr wants to, but it uses the gnutls methods instead?

3. Or was this something that was broken back in 2008 but two years of gnutls updates later it turned out to be ok? mem.c in libotr doesn't seem to have changed at all between 3.2.1 and 4.0.0.

4. And most importantly: can I ignore this bullshit and move the ssl initialization after the fork?

[dx]

Vaguely relevant bug in the otr bugtracker:

​https://bugs.otr.im/issues/23

If i'm reading this right, this secure memory handling is not only pointless but also potentially insecure. Yay. When the hell will they release bugfixes for 4.0.0?

[dx]

I brought up the issue of otr's mem.c shittiness in #otr, they basically told me to write to the mailing list so ian can know about the issue. Currently writing that email, stay tuned!

But while doing research to avoid saying dumb stuff, I found that gnutls doesn't set libgcrypt allocation handlers since 2008:

​http://git.savannah.gnu.org/cgit/gnutls.git/commit/?h=c75014b0776eff5c907d028d2fcd945ff73fc63e

The thread that pointed out this issue is very similar to the libotr issue - gnutls used to override those functions for no good reason with malloc wrappers that are not secure at all:

​http://lists.gnupg.org/pipermail/gcrypt-devel/2008-December/001415.html

To answer my own questions:

1. Even if there were two different custom allocators, it wouldn't segfault. The first one wins, the other gets ignored (it might throw a warning).
   • Since gnutls doesn't use libgcrypt secure memory at all nowadays, there's no conflict at all
2. Yes, ignoring the side effects of having a custom allocator at all. Of course, if someone else were to set custom allocators before libotr, that would probably be more secure.
3. Bingo. The timestamps fit perfectly with this explanation. Thanks past-dx for including them, btw.
4. SSL initialization can be moved after the fork, for sure. I can't ignore the issue, though.

Time to keep writing that email.

[pesco]

Just to confirm, it does indeed look like that Dec. 2008 change to gnutls fixes exactly the issue that was the reason for my Feb. 2008 comment.

Specifically, one of libotr's gcry_free()s would segfault because it had been reset to the standard free() but the pointer would point into the allocated area (after the added length field).

So it appears you can now put the libotr/ssl initialization in any order and remove that comment.

Good detective work, dx!

[dx]

:D

Seems like one of my conclusions was incorrect - i'm surprised there was a segfault, but i guess nobody would have noticed otherwise.

Either way - added a patch that fixes the original bug to #785, and removes the misleading comments.

The patch isn't merged yet, but we can consider this issue fixed. Closing!