New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTR support, take 2 #854

Merged
merged 10 commits into from Mar 31, 2018

Conversation

Projects
None yet
5 participants
@ahf
Copy link
Member

ahf commented Feb 24, 2018

This is a second attempt at #589 - this time the statusbar have been brought back to life and the code compiles with Travis CI.

If this is merged it will also fix #196

Test & discuss!

int g_io_channel_read_block(GIOChannel *channel, void *data, int len);

/* returns 1 if IPADDRs are the same */
int net_ip_compare(IPADDR *ip1, IPADDR *ip2);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

Merge conflict messup here? It's already defined above, and deprecated

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Good catch. Fixed in fixup commit: 3eacf5e

extern OtrlMessageAppOps otr_ops;

/* Active debug or not */
extern int debug;

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

This is basically a global with an extremely generic name

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Potential fix in 51ac540

}

/* Cygwin need this, don't know others.. */
/*#define BLOCKING_SOCKETS 1*/

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

This was removed in 3b3939b

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Fixed in f3a9b7e


target = window_item_get_target(item);

ret = asprintf(&msg, OTR_IRC_MARKER_ME "%s", data);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

I'd replace every asprintf with g_strdup_printf(), more portable

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Fixed in c43af47

if (g_mkdir_with_parents(dir_path, 0700) != 0)
g_error("Unable to create OTR directory path.");
} else if (!S_ISDIR(statbuf.st_mode))
g_error("%s is not a directory.\nYou should remove it with command: rm %s", dir_path, dir_path);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_error() results in abort(), hell no

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Fixed in 7b6cc27

SERVER_REC *server = opdata;
struct otr_peer_context *opc;

g_assert(context != NULL);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

I'd replace every g_assert() (which aborts) with g_return_if_fail()

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Fixed in cffa5ee

/*
* Gone insecure.
*/
static void ops_insecure(void *opdata, ConnContext *context)

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

This isn't something you can address but i feel the need to complain that libotr doesn't emit this op ever, and my patch never got merged https://bugs.otr.im/lib/libotr/issues/48

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

😢

/* Active debug or not */
extern int debug;

void irssi_send_message(SERVER_REC *irssi, const char *recipient,

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

Questionable naming here too

int otr_debug_get(void);
void otr_debug_toggle(void);

void irssi_send_message(SERVER_REC *irssi, const char *recipient,

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

Heh the comment here got marked as outdated because of the debug changes above. Anyway. irssi_send_message is questionable naming too

This comment has been minimized.

@ahf

ahf Feb 24, 2018

Member

Fixed in 2c7da88

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Feb 24, 2018

I'm adding fixup commits when I'm fixing issues that is spotted during review. Let's make sure that we rebase this before it lands - also to clean up some of the intermediate Travis CI commits.

@dequis
Copy link
Member

dequis left a comment

g_free as in g_freedom

Because in theory glib could use an allocator different than the system wide one.

I think that's all the relevant calls to free, some others still use malloc, and, other than the odd strdup which i could nitpick into turning into g_strdup, it's a bit hard to tell at a glance which ones are owned by libotr.

Also noticed a couple of leaks.

g_warning("You should remove it with command: rm %s", dir_path);
}

free(dir_path);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free


/* Critical section. On error, message MUST NOT be sent */
otr_send(query->server, msg, target, &otrmsg);
free(msg);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

ret = access(filename, F_OK);
if (ret < 0) {
IRSSI_OTR_DEBUG("no instance tags found at %9%s%9", filename);
free(filename);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

else
IRSSI_OTR_DEBUG("Error loading instance tags: %d (%d)", gcry_strerror(err), gcry_strsource(err));

free(filename);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

{
/* Safety. */
if (key_gen_state.key_file_path != NULL) {
free(key_gen_state.key_file_path);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

gcry_strerror(err), gcry_strsource(err));
}

free(filename);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

ret = access(filename, F_OK);
if (ret < 0) {
IRSSI_OTR_DEBUG("No private keys found in %9%s%9", filename);
free(filename);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

IRSSI_OTR_DEBUG("Error loading private keys: %d (%d)",
gcry_strerror(err), gcry_strsource(err));
}
}

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

Is filename leaking here?

ret = access(filename, F_OK);
if (ret < 0) {
IRSSI_OTR_DEBUG("No fingerprints found in %9%s%9", filename);
free(filename);

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

g_free

IRSSI_OTR_DEBUG("Error loading fingerprints: %d (%d)",
gcry_strerror(err), gcry_strsource(err));
}
}

This comment has been minimized.

@dequis

dequis Feb 24, 2018

Member

filename leak?

@micah

This comment has been minimized.

Copy link

micah commented Feb 25, 2018

I just compiled this and have been running it, and it seems to be functioning well. I had to remember how to turn on the status bar (/statusbar window add otr), but so far so good!

@josephbisch

This comment has been minimized.

Copy link
Member

josephbisch commented Feb 26, 2018

  1. Start irssi
  2. /otr authabort
ASAN:DEADLYSIGNAL
=================================================================
==1224==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x5564927cec5e bp 0x7fff414afaa0 sp 0x7fff414afa70 T0)
==1224==The signal is caused by a READ memory access.
==1224==Hint: address points to the zero page.
    #0 0x5564927cec5d in cmd_params_free /home/joseph/irssi-otr/irssi/src/core/commands.c:786:19
    #1 0x55649286c78d in cmd_otr_authabort /home/joseph/irssi-otr/irssi/src/otr/otr-fe.c:204:3
    #2 0x556492821760 in signal_emit_real /home/joseph/irssi-otr/irssi/src/core/signals.c:242:3
    #3 0x556492820e30 in signal_emit /home/joseph/irssi-otr/irssi/src/core/signals.c:286:3
    #4 0x5564927c9dd8 in command_runsub /home/joseph/irssi-otr/irssi/src/core/commands.c:329:7
    #5 0x55649286b3f7 in cmd_otr /home/joseph/irssi-otr/irssi/src/otr/otr-fe.c:41:2
    #6 0x556492821760 in signal_emit_real /home/joseph/irssi-otr/irssi/src/core/signals.c:242:3
    #7 0x556492820e30 in signal_emit /home/joseph/irssi-otr/irssi/src/core/signals.c:286:3
    #8 0x5564927d0781 in parse_command /home/joseph/irssi-otr/irssi/src/core/commands.c:907:14
    #9 0x5564927cf514 in event_command /home/joseph/irssi-otr/irssi/src/core/commands.c:953:2
    #10 0x556492821760 in signal_emit_real /home/joseph/irssi-otr/irssi/src/core/signals.c:242:3
    #11 0x556492820e30 in signal_emit /home/joseph/irssi-otr/irssi/src/core/signals.c:286:3
    #12 0x55649262fd52 in key_send_line /home/joseph/irssi-otr/irssi/src/fe-text/gui-readline.c:503:3
    #13 0x556492799832 in event_input /home/joseph/irssi-otr/irssi/src/irc/core/channel-events.c:377:2
    #14 0x556492821760 in signal_emit_real /home/joseph/irssi-otr/irssi/src/core/signals.c:242:3
    #15 0x556492820e30 in signal_emit /home/joseph/irssi-otr/irssi/src/core/signals.c:286:3
    #16 0x5564927465fa in irc_server_event /home/joseph/irssi-otr/irssi/src/irc/core/irc.c:338:7
    #17 0x556492821760 in signal_emit_real /home/joseph/irssi-otr/irssi/src/core/signals.c:242:3
    #18 0x556492822007 in signal_emit_id /home/joseph/irssi-otr/irssi/src/core/signals.c:304:3
    #19 0x556492746a92 in irc_parse_incoming_line /home/joseph/irssi-otr/irssi/src/irc/core/irc.c:392:3
    #20 0x556492821760 in signal_emit_real /home/joseph/irssi-otr/irssi/src/core/signals.c:242:3
    #21 0x556492822007 in signal_emit_id /home/joseph/irssi-otr/irssi/src/core/signals.c:304:3
    #22 0x556492746e87 in irc_parse_incoming /home/joseph/irssi-otr/irssi/src/irc/core/irc.c:413:3
    #23 0x5564927e5e04 in irssi_io_invoke /home/joseph/irssi-otr/irssi/src/core/misc.c:51:3
    #24 0x7f549bbdd0bd in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6b0bd)
    #25 0x7f549bbdef68  (/usr/lib/libglib-2.0.so.0+0x6cf68)
    #26 0x7f549bbdefad in g_main_context_iteration (/usr/lib/libglib-2.0.so.0+0x6cfad)
    #27 0x556492689773 in main /home/joseph/irssi-otr/irssi/src/fe-text/irssi.c:346:3
    #28 0x7f549aaa8f69 in __libc_start_main (/usr/lib/libc.so.6+0x20f69)
    #29 0x556492520fe9 in _start (/usr/local/bin/irssi+0x113fe9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/joseph/irssi-otr/irssi/src/core/commands.c:786:19 in cmd_params_free
==1224==ABORTING
@dequis

This comment has been minimized.

Copy link
Member

dequis commented Feb 26, 2018

re: the /otr authabort error above - cmd_param_error is an interesting one, looks like it assumes there's a variable called free_arg written by cmd_get_params(data, &free_arg, ...), and in this case that isn't used, and it's set to NULL. Fix would be to just s/cmd_param_error/cmd_return_error/ in cmd_otr_authabort


OTR %|[OPTION]

Command to control the OTR module. Without an option, this help is printed.

This comment has been minimized.

@dequis

dequis Feb 26, 2018

Member

"Without an option, this help is printed" is a lie, it shows /otr info now


Command to control the OTR module. Without an option, this help is printed.

This help contains three sections which are %9options, quickstart and files.%n

This comment has been minimized.

@dequis

dequis Feb 26, 2018

Member

This help file is kinda massive, would be nice to split these into separate help topics.

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Feb 26, 2018

Hah, got you a fun one! A crash on /otr init but with a twist.

My preferred ircd for testing is bitlbee (of course) with a local prosody xmpp server with two accounts, test1 and test2. I also have the jabber xmlconsole open, which shows all incoming and outgoing messages in another irc query window. You can see where this is going.

  1. Remove ~/.irssi/otr
  2. Connect and identify to bitlbee
  3. /query test1
  4. /otr init
  5. Crash.
Thread 1 received signal SIGSEGV, Segmentation fault.
0x0000562936fbbd05 in read_key_gen_status (worker=0x5629385ed0d0, pipe=0x5629386017b0) at key.c:177
177                     err = otrl_privkey_read(key_gen_state.ustate->otr_state, key_gen_state.key_file_path);
(rr) bt
#0  0x0000562936fbbd05 in read_key_gen_status (worker=0x5629385ed0d0, pipe=0x5629386017b0) at key.c:177
#1  0x0000562936f99fad in irssi_io_invoke (source=<optimized out>, condition=<optimized out>, data=<optimized out>) at misc.c:51
#2  0x00007fe30c73bca6 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#3  0x00007fe30c73c081 in ?? () from /usr/lib/libglib-2.0.so.0
#4  0x00007fe30c73c10e in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#5  0x0000562936f21017 in main (argc=<optimized out>, argv=<optimized out>) at irssi.c:346
(rr) p key_gen_state
$5 = {ustate = 0x0, account_name = 0x0, key_file_path = 0x0, status = KEY_GEN_FINISHED, gcry_error = 0}

The keygen state is reset but it says finished.

Doing some time travel, writes to the keygen state in order. First:

Thread 1 hit Hardware watchpoint 1: -location *0x56293720dce0

Old value = 0
New value = 945640416
key_gen_run (ustate=0x5629385d53e0, account_name=<optimized out>) at key.c:219
219             key_gen_state.key_file_path = file_path_build(OTR_KEYFILE);
(rr) bt
#0  key_gen_run (ustate=0x5629385d53e0, account_name=<optimized out>) at key.c:219
#1  0x00007fe30d5567ba in otrl_message_receiving () from /usr/lib/libotr.so.5
#2  0x0000562936fbb528 in otr_receive (server=server@entry=0x5629385e3db0,
    msg=msg@entry=0x56293846d400 "TX: ?OTR:AAMK9Jyy1vScstYAAADAhPz7HGbBVjP/aPYcX4jAxTmFy86I6B+lf/2Fn4PuXLanm8F4sW0lmjegf04CU9yNQMccqLEGb4iCgunzSMg5ibPTPo9gwq+z5v35+QrMee3uCDvAiQK0K8bFHbuSlWDDgnLzLvdJ6WWl9cA3fQfL8/HS0B+WdjTOepJcbcAQDCVFljJ7zJtwfSbpF1MWPpcaBoH8mAiVBQcxGlVZwFrnJvAqA9TWHXoJuLgrltCKibFSfV6gUzq/kE43nUV9Tb9j.", from=from@entry=0x5629385f5a51 "_xmlconsole", new_msg=new_msg@entry=0x7ffc96fe8560) at otr.c:733
#3  0x0000562936fb8e06 in sig_message_private (server=0x5629385e3db0,
    msg=0x56293846d400 "TX: ?OTR:AAMK9Jyy1vScstYAAADAhPz7HGbBVjP/aPYcX4jAxTmFy86I6B+lf/2Fn4PuXLanm8F4sW0lmjegf04CU9yNQMccqLEGb4iCgunzSMg5ibPTPo9gwq+z5v35+QrMee3uCDvAiQK0K8bFHbuSlWDDgnLzLvdJ6WWl9cA3fQfL8/HS0B+WdjTOepJcbcAQDCVFljJ7zJtwfSbpF1MWPpcaBoH8mAiVBQcxGlVZwFrnJvAqA9TWHXoJuLgrltCKibFSfV6gUzq/kE43nUV9Tb9j.", nick=0x5629385f5a51 "_xmlconsole", address=0x5629385f5a5d "_xmlconsole@test1") at module.c:85
#4  0x0000562936fa89f6 in signal_emit_real (rec=rec@entry=<Signal 24>, params=params@entry=5, va=va@entry=0x7ffc96fe8620, first_hook=<optimized out>) at signals.c:242
#5  0x0000562936fa8f4d in signal_emit (signal=<optimized out>, params=params@entry=5) at signals.c:286
#6  0x0000562936f40568 in event_privmsg (server=0x5629385e3db0, data=<optimized out>, nick=0x5629385f5a51 "_xmlconsole", addr=0x5629385f5a5d "_xmlconsole@test1") at fe-events.c:68
#7  0x0000562936fa89f6 in signal_emit_real (rec=rec@entry=<Signal 142>, params=params@entry=4, va=va@entry=0x7ffc96fe8810, first_hook=<optimized out>) at signals.c:242
#8  0x0000562936fa8f4d in signal_emit (signal=<optimized out>, params=params@entry=4) at signals.c:286
#9  0x0000562936f6e8c8 in irc_server_event (server=0x5629385e3db0,
    line=0x5629385f5a6f "PRIVMSG dx|prosody :TX: ?OTR:AAMK9Jyy1vScstYAAADAhPz7HGbBVjP/aPYcX4jAxTmFy86I6B+lf/2Fn4PuXLanm8F4sW0lmjegf04CU9yNQMccqLEGb4iCgunzSMg5ibPTPo9gwq+z5v35+QrMee3uCDvAiQK0K8bFHbuSlWDDgnLzLvdJ6WWl9cA3fQfL8/HS0B+WdjTOepJcbcAQDCVFljJ7zJtwfSbpF1MWPpcaBoH8mAiVBQcxGlVZwFrnJvAqA9TWHXoJuLgrltCKibFSfV6gUzq/kE43nUV9Tb9j.", nick=0x5629385f5a51 "_xmlconsole", address=0x5629385f5a5d "_xmlconsole@test1") at irc.c:338
#10 0x0000562936fa89f6 in signal_emit_real (rec=rec@entry=<Signal 95>, params=params@entry=4, va=va@entry=0x7ffc96fe89d0, first_hook=<optimized out>) at signals.c:242
#11 0x0000562936fa9025 in signal_emit_id (signal_id=<optimized out>, params=4) at signals.c:304
#12 0x0000562936fa89f6 in signal_emit_real (rec=rec@entry=<Signal 203>, params=params@entry=2, va=va@entry=0x7ffc96fe8b40, first_hook=<optimized out>) at signals.c:242
#13 0x0000562936fa9025 in signal_emit_id (signal_id=<optimized out>, params=params@entry=2) at signals.c:304
#14 0x0000562936f6ea25 in irc_parse_incoming (server=0x5629385e3db0) at irc.c:413
#15 0x0000562936f99fad in irssi_io_invoke (source=<optimized out>, condition=<optimized out>, data=<optimized out>) at misc.c:51
#16 0x00007fe30c73bca6 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#17 0x00007fe30c73c081 in ?? () from /usr/lib/libglib-2.0.so.0
#18 0x00007fe30c73c10e in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#19 0x0000562936f21017 in main (argc=<optimized out>, argv=<optimized out>) at irssi.c:346

The xmlconsole is clearly messing things up.

Then it's reset:

(rr) c
Continuing.

Thread 1 hit Hardware watchpoint 1: -location *0x56293720dce0

Old value = 945640416
New value = 0
0x0000562936fbbbb1 in reset_key_gen_state () at key.c:128
128             memset(&key_gen_state, 0, sizeof(key_gen_state));
(rr) bt
#0  0x0000562936fbbbb1 in reset_key_gen_state () at key.c:128
#1  0x0000562936fbbd50 in read_key_gen_status (worker=0x5629385fd270, pipe=0x5629385f4400) at key.c:190
#2  0x0000562936f99fad in irssi_io_invoke (source=<optimized out>, condition=<optimized out>, data=<optimized out>) at misc.c:51
#3  0x00007fe30c73bca6 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#4  0x00007fe30c73c081 in ?? () from /usr/lib/libglib-2.0.so.0
#5  0x00007fe30c73c10e in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#6  0x0000562936f21017 in main (argc=<optimized out>, argv=<optimized out>) at irssi.c:346

Then it gets another irssi_io_invoke with another worker, read_key_gen_status sets the status to finished but the most of the state is wiped already and it crashes.

(rr) c
Continuing.

Thread 1 received signal SIGSEGV, Segmentation fault.
0x0000562936fbbd05 in read_key_gen_status (worker=0x5629385ed0d0, pipe=0x5629386017b0) at key.c:177
177                     err = otrl_privkey_read(key_gen_state.ustate->otr_state, key_gen_state.key_file_path);
(rr) bt
#0  0x0000562936fbbd05 in read_key_gen_status (worker=0x5629385ed0d0, pipe=0x5629386017b0) at key.c:177
#1  0x0000562936f99fad in irssi_io_invoke (source=<optimized out>, condition=<optimized out>, data=<optimized out>) at misc.c:51
#2  0x00007fe30c73bca6 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#3  0x00007fe30c73c081 in ?? () from /usr/lib/libglib-2.0.so.0
#4  0x00007fe30c73c10e in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#5  0x0000562936f21017 in main (argc=<optimized out>, argv=<optimized out>) at irssi.c:346

The status window shows:

01:53 -!- Irssi: OTR key generation for localhost started
01:53 -!- Irssi: OTR key generation for localhost started
01:53 -!- Irssi: OTR key generation for localhost is still in progress
01:53 -!- Irssi: OTR key generation for localhost is still in progress
01:53 -!- Irssi: OTR key generation for localhost completed

Double keygen, first one completes, second one crashes.

After the initial keygen is done separately, having the xmlconsole open doesn't hurt as much. It still processes messages that it shouldn't and probably sends garbage to the xmpp server too, but that's more or less what you'd expect from receiving random OTR tagged messages in a query window.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Feb 26, 2018

as for the build system, can we default to module build?

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Feb 26, 2018

@ailin-nemui could you check if my latest patch does what you are thinking of when it comes to building as module by default?

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Feb 26, 2018

I think i have fixed the various issues pointed out by @dequis now except for the crash in key generation.

Do you run on a system where key generation is slow? libotr uses /dev/random and not /dev/urandom as entropy source, which means it's crazy slow on Linux. On a Linux with haveged or on other Unix platforms it would be harder to trigger that crash. Let me see what I can do about it.

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Feb 26, 2018

Still need to fix the documentation a bit.

Add OTR support.
This patch adds support for the OTR protocol to irssi. This is an import
of the external irssi-otr project that we are now taking over
maintership for.

Major thanks to the original authors of Irssi-OTR: Uli Meis and David
Goulet. Thanks to the OTR community in #OTR on OFTC, thanks to everyone
who have helped testing the patches and submitted UI suggestions.

@ahf ahf force-pushed the ahf/otr branch from 7bca586 to 016b42b Feb 26, 2018

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Feb 27, 2018

Key generation is fast enough, this is a desktop so i got plenty of entropy sources (when do we get to convince the libotr devs to use urandom btw?)

I think that what you actually need is to start two of them simultaneously, which can probably be done by receiving two otr messages from two different contacts at the same time. Or do the xmlconsole thing.

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Feb 27, 2018

Ah, makes sense. Let me see if I can reproduce this later today.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 12, 2018

@ahf any updates here?

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Mar 14, 2018

We need to fix the README issues and the slightly odd crash issue that can happen in a very pathological case. Don't think there is any updates.

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Mar 14, 2018

For what it's worth I think we might as well merge this in and fix those two issues afterwards, but having more people than @dequis to review the code would be useful.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 15, 2018

I can't even load the otr module, it doesn't seem to implement our ABI check function. Could you please test that this thing loads?

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Mar 15, 2018

Fixed in 8d07f52

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 16, 2018

can you please fix this bug:

/msg yourself hi

results in

-!- Irssi: critical privmsg_get_query: assertion 'nick != NULL' failed
-!- Irssi: critical query_find: assertion 'nick != NULL' failed
[msg()] hi
otr: add KEY_GEN_STARTED state to avoid starting it twice
Fun race condition there, got this when testing with the jabber
xmlconsole open in bitlbee
@dequis

This comment has been minimized.

Copy link
Member

dequis commented Mar 29, 2018

Pushed b52cf10 fixing the xmlconsole double keygen thing from #854 (comment)

dequis added some commits Mar 29, 2018

otr: fix missing 'target' param in 'message private' signal
Fixes warnings like these on '/msg yourself'

-!- Irssi: critical privmsg_get_query: assertion 'nick != NULL' failed
-!- Irssi: critical query_find: assertion 'nick != NULL' failed
@dequis

This comment has been minimized.

Copy link
Member

dequis commented Mar 29, 2018

Pushed dd7dc7c fixing the /msg yourself warnings and 7d7c697 fixing the same bug in a different place.

It was the "message private" signal missing one param in the documentation that was fixed by lemon in 0fe81ca (2016). As far as I can see that parameter has always been there.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 29, 2018

nice

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Mar 29, 2018

The patches looks good to me!

ailin-nemui added some commits Mar 29, 2018

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 29, 2018

@irssi/developers can we merge it then?

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Mar 29, 2018

One last thing, can we rename module.c to otr-module.c? (or something else with otr in the name)

It was a bit misleading when reading backtraces in gdb.

@ahf

This comment has been minimized.

Copy link
Member

ahf commented Mar 29, 2018

I think that's fine to do.

dequis added some commits Mar 31, 2018

@ailin-nemui ailin-nemui merged commit 0c1db8f into master Mar 31, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dequis dequis referenced this pull request Apr 2, 2018

Closed

irssi build failure #1292

@micah micah referenced this pull request May 16, 2018

Open

make a new release #64

@ailin-nemui ailin-nemui deleted the ahf/otr branch Sep 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment