[Pkg-privacy-commits] [irssi-plugin-otr] 161/267: Refactor OTR key generation
Ximin Luo
infinity0 at moszumanska.debian.org
Sat Aug 22 12:26:28 UTC 2015
This is an automated email from the git hooks/post-receive script.
infinity0 pushed a commit to branch debian
in repository irssi-plugin-otr.
commit 205fd2bef97d3b06bd353bab7c19bd4ce930cd04
Author: David Goulet <dgoulet at ev0ke.net>
Date: Sat Dec 1 19:45:13 2012 -0500
Refactor OTR key generation
Use a thread for key generation. However, libperl used by irssi does not
seems to like very much emitting irssi signal inside a thread which
triggers segfaults. So, for this reason, we can not print anything
inside the thread. This is why the key gen. check is added at each event
of the modules so we can notify at some point in time the key
completion. This is quite a big limitation but for now it's the only way
I found to not trigger irssi bad behaviors in a forked process or inside
the thread.
Also, the genkey abort option is removed since when using the libgcrypt
in a multithread program, gcrypt mutexes need to be initialized then
used to lock down the key generation which basically stalls irssi until
completion. So, not having an abort command avoids the non trivial task
of handling thread cancellation and libgcrypt internal mutexes state so
not to stall irssi or hitting an assert() if no thread model is set for
the lib.
Hopefully, it would be nice to find a way to print text inside irssi
windows without the use of libperl or fixing the segfault when inside a
newly spawned thread.
Signed-off-by: David Goulet <dgoulet at ev0ke.net>
---
src/Makefile.am | 2 +-
src/cmd.c | 6 +-
src/key.c | 284 ++++++++++++++++++--------------------------------------
src/key.h | 20 +++-
src/module.c | 10 ++
src/otr-ops.c | 2 +-
src/otr.c | 2 -
src/otr.h | 1 -
8 files changed, 123 insertions(+), 204 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 083e496..c4b9302 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -21,7 +21,7 @@ otr_la_SOURCES = otr-formats.c otr-formats.h \
utils.h utils.c otr.h module.c irssi_otr.h
otr_la_LDFLAGS = -avoid-version -module
-otr_la_LDFLAGS += $(LIBOTR_LIBS) $(LIBGCRYPT_LIBS)
+otr_la_LDFLAGS += $(LIBOTR_LIBS) $(LIBGCRYPT_LIBS) -lpthread
install-exec-hook:
mkdir -p $(plugindir)
diff --git a/src/cmd.c b/src/cmd.c
index e02f983..1702b75 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -135,10 +135,8 @@ static void _cmd_genkey(struct otr_user_state *ustate, SERVER_REC *irssi,
utils_explode_args(data, &argv, &argc);
if (argc) {
- if (strncmp(argv[0], "abort", strlen("abort")) == 0) {
- key_generation_abort(ustate, FALSE);
- } else if (strchr(argv[0], '@')) {
- key_generation_run(ustate, argv[0]);
+ if (strchr(argv[0], '@')) {
+ key_gen_run(ustate, argv[0]);
} else {
IRSSI_INFO(NULL, NULL, "I need an account name. "
"Try something like /otr genkey mynick at irc.server.net");
diff --git a/src/key.c b/src/key.c
index 8b8d960..addf3b9 100644
--- a/src/key.c
+++ b/src/key.c
@@ -29,17 +29,15 @@
#include "key.h"
-static struct {
- keygen_status_t status;
- char *accountname;
- char *protocol;
- time_t started;
- GIOChannel *ch[2];
- guint cpid;
- guint cwid;
- pid_t pid;
- struct otr_user_state *ustate;
-} kg_st = { .status = KEYGEN_NO };
+/*
+ * Key generation data for the thread in charge of creating the key.
+ */
+static struct key_gen_data key_gen_state = {
+ .status = KEY_GEN_IDLE,
+ .gcry_error = GPG_ERR_NO_ERROR,
+};
+
+static pthread_t keygen_thread;
static char *file_path_build(const char *path)
{
@@ -59,103 +57,77 @@ static char *file_path_build(const char *path)
return filename;
}
-static void keygen_childwatch(GPid pid, gint status, gpointer data)
+/*
+ * Reset key generation state and status is IDLE.
+ */
+static void reset_key_gen_state(void)
{
- int ret;
- struct pollfd pfd = {
- .fd = g_io_channel_unix_get_fd(kg_st.ch[0]),
- .events = POLLIN
- };
-
- /* nothing to do if keygen_complete has already been called */
- if (data) {
- goto end;
+ /* Safety. */
+ if (key_gen_state.key_file_path) {
+ free(key_gen_state.key_file_path);
}
- kg_st.pid = 0;
-
- ret = poll(&pfd, 1, 0);
- if (ret == 1) {
- /* data is there, let's wait for keygen_complete to be called */
- return;
- } else if (ret == 0) {
- /* no data, report error and reset kg_st */
- if (WIFSIGNALED(status)) {
- char sigstr[16];
-
- ret = snprintf(sigstr, sizeof(sigstr),
-#ifndef HAVE_STRSIGNAL
- "%d", WTERMSIG(status)
-#else
- "%s", strsignal(WTERMSIG(status))
-#endif
- );
- IRSSI_INFO(NULL, NULL, "Key generation for %s, child was killed "
- "by signal %s", kg_st.accountname, sigstr);
- } else {
- IRSSI_INFO(NULL, NULL, "Key generation for %s, child terminated "
- "for unknown reason", kg_st.accountname);
- }
- } else if (ret < 0) {
- IRSSI_INFO(NULL, NULL, "Key generation for %s. Poll error %s",
- kg_st.accountname, strerror(errno));
+ /* Pointer dup when key_gen_run is called. */
+ if (key_gen_state.account_name) {
+ free(key_gen_state.account_name);
}
- key_generation_abort(kg_st.ustate, FALSE);
-
-end:
- return;
+ /* Nullify everything. */
+ memset(&key_gen_state, 0, sizeof(key_gen_state));
+ key_gen_state.status = KEY_GEN_IDLE;
+ key_gen_state.gcry_error = GPG_ERR_NO_ERROR;
}
-/*
- * Installed as g_io_watch and called when the key generation
- * process finishs.
- */
-static gboolean keygen_complete(GIOChannel *source, GIOCondition condition,
- gpointer data)
+static void *generate_key(void *data)
{
- int ret;
gcry_error_t err;
- const char *clconfdir = get_client_config_dir();
- char *filename = g_strconcat(clconfdir, OTR_KEYFILE, NULL);
- char *tmpfilename = g_strconcat(clconfdir, OTR_TMP_KEYFILE, NULL);
- ret = read(g_io_channel_unix_get_fd(kg_st.ch[0]), &err, sizeof(err));
- if (ret < 0) {
- IRSSI_INFO(NULL, NULL, "Unable to read on key gen IO channel.");
- goto error;
- }
+ assert(key_gen_state.newkey);
- g_source_remove(kg_st.cpid);
- g_io_channel_shutdown(kg_st.ch[0], FALSE, NULL);
- g_io_channel_shutdown(kg_st.ch[1], FALSE, NULL);
- g_io_channel_unref(kg_st.ch[0]);
- g_io_channel_unref(kg_st.ch[1]);
+ key_gen_state.status = KEY_GEN_RUNNING;
- if (err) {
- IRSSI_INFO(NULL, NULL, "Key generation failed for %s (err: %s)",
- kg_st.accountname, gcry_strerror(err));
- } else {
- /* reload keys */
- IRSSI_INFO(NULL, NULL, "Key generation for %s completed in %d seconds."
- " Reloading keys.", kg_st.accountname,
- time(NULL) - kg_st.started);
- rename(tmpfilename, filename);
- //otrl_privkey_forget_all(otr_state); <-- done by lib
- key_load(kg_st.ustate);
+ err = otrl_privkey_generate_calculate(key_gen_state.newkey);
+ if (err != GPG_ERR_NO_ERROR) {
+ key_gen_state.status = KEY_GEN_ERROR;
+ key_gen_state.gcry_error = err;
+ goto error;
}
- g_source_remove(kg_st.cwid);
- kg_st.cwid = g_child_watch_add(kg_st.pid, keygen_childwatch, (void*) 1);
+ key_gen_state.status = KEY_GEN_FINISHED;
- kg_st.status = KEYGEN_NO;
- g_free(kg_st.accountname);
+error:
+ return NULL;
+}
- g_free(filename);
- g_free(tmpfilename);
+void key_gen_check(void)
+{
+ gcry_error_t err;
-error:
- return FALSE;
+ switch (key_gen_state.status) {
+ case KEY_GEN_FINISHED:
+ err = otrl_privkey_generate_finish(key_gen_state.ustate->otr_state,
+ key_gen_state.newkey, key_gen_state.key_file_path);
+ if (err != GPG_ERR_NO_ERROR) {
+ IRSSI_MSG("Key generation finish state failed. Err: %s",
+ gcry_strerror(err));
+ } else {
+ IRSSI_MSG("Key generation for %9%s%n completed",
+ key_gen_state.account_name);
+ }
+ reset_key_gen_state();
+ break;
+ case KEY_GEN_ERROR:
+ IRSSI_MSG("Key generation for %9%s%n failed. Err: %s (%d)",
+ key_gen_state.account_name,
+ gcry_strerror(key_gen_state.gcry_error),
+ key_gen_state.gcry_error);
+ reset_key_gen_state();
+ break;
+ case KEY_GEN_RUNNING:
+ case KEY_GEN_IDLE:
+ /* Do nothing */
+ break;
+ };
}
/*
@@ -163,122 +135,50 @@ error:
* will rewrite the key file, we shouldn't change anything till it's done and
* we've reloaded the keys.
*/
-void key_generation_run(struct otr_user_state *ustate, const char *accname)
+void key_gen_run(struct otr_user_state *ustate, const char *account_name)
{
- gcry_error_t err;
int ret;
- int fds[2];
- char *filename = g_strconcat(get_client_config_dir(), OTR_TMP_KEYFILE, NULL);
- char *filenamedup = g_strdup(filename);
- char *dir = dirname(filenamedup);
-
- if (kg_st.status != KEYGEN_NO) {
- if (strncmp(accname, kg_st.accountname, strlen(accname)) != 0) {
- IRSSI_INFO(NULL, NULL, "Key generation for %s aborted. "
- "Key generation for %s still in progress", accname,
- kg_st.accountname);
- }
- g_free(filenamedup);
- goto end;
- }
+ gcry_error_t err;
- if (!g_file_test(dir, G_FILE_TEST_EXISTS)) {
- if (g_mkdir(dir, S_IRWXU)) {
- IRSSI_INFO(NULL, NULL, "Key generation for %s aborted. Failed "
- "creating directory %s (err: %s)",
- accname, dir, strerror(errno));
- g_free(dir);
- g_free(filenamedup);
- goto end;
- } else {
- IRSSI_INFO(NULL, NULL, "Key generation created directory %9%s%9",
- dir);
- }
+ if (key_gen_state.status != KEY_GEN_IDLE) {
+ IRSSI_INFO(NULL, NULL, "Key generation for %s is still in progress. ",
+ "Please wait until completion before creating a new key.",
+ key_gen_state.account_name);
+ goto error_status;
}
- g_free(filenamedup);
+ /* Make sure the pointer does not go away during the proess. */
+ key_gen_state.account_name = strdup(account_name);
+ key_gen_state.ustate = ustate;
- ret = pipe(fds);
- if (ret < 0) {
- IRSSI_INFO(NULL, NULL, "Key generation for %s. Error creating "
- "pipe (err: %s)", accname, strerror(errno));
- goto end;
+ /* Creating key file path. */
+ key_gen_state.key_file_path = file_path_build(OTR_KEYFILE);
+ if (!key_gen_state.key_file_path) {
+ IRSSI_INFO(NULL, NULL, "Key generation failed. ENOMEM");
+ goto error;
}
- kg_st.ch[0] = g_io_channel_unix_new(fds[0]);
- kg_st.ch[1] = g_io_channel_unix_new(fds[1]);
-
- kg_st.accountname = g_strdup(accname);
- kg_st.ustate = ustate;
- kg_st.protocol = OTR_PROTOCOL_ID;
- kg_st.started = time(NULL);
-
- if ((ret = fork())) {
- if (ret == -1) {
- IRSSI_INFO(NULL, NULL, "Key generation for %s. Fork error "
- "(err: %s)", accname, strerror(errno));
- goto end;
- }
+ IRSSI_MSG("Key generation started for %9%s%n", key_gen_state.account_name);
- kg_st.status = KEYGEN_RUNNING;
- kg_st.pid = ret;
-
- IRSSI_INFO(NULL, NULL, "Key generation for %s initiated. "
- "This might take several minutes or on some systems even an "
- "hour. If you wanna check that something is happening, see if "
- "there are two processes of your IRC client.", accname);
-
- kg_st.cpid = g_io_add_watch(kg_st.ch[0], G_IO_IN,
- (GIOFunc) keygen_complete, NULL);
- kg_st.cwid = g_child_watch_add(kg_st.pid, keygen_childwatch, NULL);
- kg_st.started = time(NULL);
- goto end;
+ err = otrl_privkey_generate_start(ustate->otr_state, account_name,
+ OTR_PROTOCOL_ID, &key_gen_state.newkey);
+ if (err != GPG_ERR_NO_ERROR || !key_gen_state.newkey) {
+ IRSSI_MSG("Key generation start failed. Err: %s", gcry_strerror(err));
+ goto error;
}
- /* child */
-
- err = otrl_privkey_generate(ustate->otr_state, filename, accname,
- OTR_PROTOCOL_ID);
- ret = write(fds[1], &err, sizeof(err));
- if (ret != sizeof(err) || ret < 0) {
- IRSSI_INFO(NULL, NULL, "Unable to write to pipe at key gen.");
+ ret = pthread_create(&keygen_thread, NULL, generate_key, NULL);
+ if (ret < 0) {
+ IRSSI_MSG("Key generation failed. Thread failure: %s",
+ strerror(errno));
+ goto error;
}
- g_free(filename);
-
- exit(EXIT_SUCCESS);
-
-end:
- g_free(filename);
return;
-}
-/*
- * Abort ongoing key generation.
- */
-void key_generation_abort(struct otr_user_state *ustate, int ignoreidle)
-{
- if (kg_st.status != KEYGEN_RUNNING) {
- if (!ignoreidle) {
- IRSSI_INFO(NULL, NULL, "No ongoing key generation to abort");
- }
- goto end;
- }
-
- IRSSI_INFO(NULL, NULL, "Key generation for %s aborted", kg_st.accountname);
-
- g_source_remove(kg_st.cpid);
- g_source_remove(kg_st.cwid);
- g_free(kg_st.accountname);
-
- if (kg_st.pid != 0) {
- kill(kg_st.pid, SIGTERM);
- g_child_watch_add(kg_st.pid, keygen_childwatch, (void *) 1);
- }
-
- kg_st.status = KEYGEN_NO;
-
-end:
+error:
+ reset_key_gen_state();
+error_status:
return;
}
diff --git a/src/key.h b/src/key.h
index 4facb51..ff3762c 100644
--- a/src/key.h
+++ b/src/key.h
@@ -21,10 +21,24 @@
#include "otr.h"
-typedef enum { KEYGEN_NO, KEYGEN_RUNNING } keygen_status_t;
+enum key_gen_status {
+ KEY_GEN_IDLE = 0,
+ KEY_GEN_RUNNING = 1,
+ KEY_GEN_FINISHED = 2,
+ KEY_GEN_ERROR = 3,
+};
-void key_generation_abort(struct otr_user_state *ustate, int ignoreidle);
-void key_generation_run(struct otr_user_state *ustate, const char *accname);
+struct key_gen_data {
+ struct otr_user_state *ustate;
+ char *account_name;
+ char *key_file_path;
+ enum key_gen_status status;
+ gcry_error_t gcry_error;
+ void *newkey;
+};
+
+void key_gen_check(void);
+void key_gen_run(struct otr_user_state *ustate, const char *account_name);
void key_load(struct otr_user_state *ustate);
void key_load_fingerprints(struct otr_user_state *ustate);
void key_write_fingerprints(struct otr_user_state *ustate);
diff --git a/src/module.c b/src/module.c
index cec16f7..52510e0 100644
--- a/src/module.c
+++ b/src/module.c
@@ -27,6 +27,7 @@
#include <stdio.h>
#include "cmd.h"
+#include "key.h"
#include "otr.h"
#include "otr-formats.h"
#include "utils.h"
@@ -57,6 +58,8 @@ static void sig_server_sendmsg(SERVER_REC *server, const char *target,
int ret;
char *otrmsg = NULL;
+ key_gen_check();
+
if (GPOINTER_TO_INT(target_type_p) != SEND_TARGET_NICK) {
goto end;
}
@@ -90,6 +93,8 @@ void sig_message_private(SERVER_REC *server, const char *msg,
int ret;
char *new_msg = NULL;
+ key_gen_check();
+
ret = otr_receive(server, msg, nick, &new_msg);
if (ret) {
signal_stop();
@@ -132,6 +137,8 @@ static void cmd_me(const char *data, IRC_SERVER_REC *server,
query = QUERY(item);
+ key_gen_check();
+
if (!query || !query->server) {
goto end;
}
@@ -184,6 +191,9 @@ static void cmd_otr(const char *data, void *server, WI_ITEM_REC *item)
query = QUERY(item);
+ /* Check key generation state. */
+ key_gen_check();
+
if (*data == '\0') {
IRSSI_INFO(NULL, NULL, "Alive!");
goto end;
diff --git a/src/otr-ops.c b/src/otr-ops.c
index 086955b..8bc9329 100644
--- a/src/otr-ops.c
+++ b/src/otr-ops.c
@@ -43,7 +43,7 @@ static OtrlPolicy ops_policy(void *opdata, ConnContext *context)
static void ops_create_privkey(void *opdata, const char *accountname,
const char *protocol)
{
- key_generation_run(user_state_global, accountname);
+ key_gen_run(user_state_global, accountname);
}
/*
diff --git a/src/otr.c b/src/otr.c
index 3d04e0a..d9b2e8e 100644
--- a/src/otr.c
+++ b/src/otr.c
@@ -266,8 +266,6 @@ error:
void otr_free_user(struct otr_user_state *ustate)
{
- key_generation_abort(ustate, TRUE);
-
if (ustate->otr_state) {
otrl_userstate_free(ustate->otr_state);
ustate->otr_state = NULL;
diff --git a/src/otr.h b/src/otr.h
index ded42dc..a0634ef 100644
--- a/src/otr.h
+++ b/src/otr.h
@@ -41,7 +41,6 @@
#define OTR_PROTOCOL_ID "IRC"
#define OTR_KEYFILE "/otr/otr.key"
-#define OTR_TMP_KEYFILE "/otr/otr.key.tmp"
#define OTR_FINGERPRINTS_FILE "/otr/otr.fp"
#define OTR_INSTAG_FILE "/otr/otr.instag"
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-privacy/packages/irssi-plugin-otr.git
More information about the Pkg-privacy-commits
mailing list