From 6f7d416bb80bda2c48243628527831c0966fe8bb Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 8 Nov 2014 13:50:59 +0100 Subject: [PATCH 1/4] fix acceptance of trusted SSL certs we should make no assumptions about the layout of OpenSSL's certificate store, in particular when they are wrong. so copy the interesting part instead of "deep-linking" into it. this code is uglier than it should be, but OpenSSL's extensive use of macros to manage data types would force us to include the ssl headers into our headers otherwise, which would be even uglier. REFMAIL: <545442CC.9020400@nodivisions.com> --- src/socket.c | 8 ++++---- src/socket.h | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/socket.c b/src/socket.c index f47e375..fc06f8b 100644 --- a/src/socket.c +++ b/src/socket.c @@ -178,11 +178,11 @@ ssl_verify_callback( int ok, X509_STORE_CTX *ctx ) if (!conn->force_trusted) { X509 *cert = sk_X509_value( ctx->chain, 0 ); - STACK_OF(X509_OBJECT) *trusted = ctx->ctx->objs; - unsigned i; + STACK_OF(X509_OBJECT) *trusted = (STACK_OF(X509_OBJECT) *)conn->conf->trusted_certs; + int i; conn->force_trusted = -1; - for (i = 0; i < conn->conf->num_trusted; i++) { + for (i = 0; i < sk_X509_OBJECT_num( trusted ); i++) { if (!X509_cmp( cert, sk_X509_OBJECT_value( trusted, i )->data.x509 )) { conn->force_trusted = 1; break; @@ -227,7 +227,7 @@ init_ssl_ctx( const server_conf_t *conf ) conf->cert_file, ERR_error_string( ERR_get_error(), 0 ) ); return 0; } - mconf->num_trusted = sk_X509_OBJECT_num( SSL_CTX_get_cert_store( mconf->SSLContext )->objs ); + mconf->trusted_certs = (_STACK *)sk_X509_OBJECT_dup( SSL_CTX_get_cert_store( mconf->SSLContext )->objs ); if (!SSL_CTX_set_default_verify_paths( mconf->SSLContext )) warn( "Warning: Unable to load default certificate files: %s\n", ERR_error_string( ERR_get_error(), 0 ) ); diff --git a/src/socket.h b/src/socket.h index 1545b39..193330e 100644 --- a/src/socket.h +++ b/src/socket.h @@ -27,6 +27,7 @@ typedef struct ssl_st SSL; typedef struct ssl_ctx_st SSL_CTX; +typedef struct stack_st _STACK; typedef struct server_conf { char *tunnel; @@ -39,7 +40,7 @@ typedef struct server_conf { /* these are actually variables and are leaked at the end */ char ssl_ctx_valid; - unsigned num_trusted; + _STACK *trusted_certs; SSL_CTX *SSLContext; #endif } server_conf_t; From 9eba3d8cd9f73ddd12f450f17632c431fc94dfbf Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 2 Jan 2015 11:29:51 +0100 Subject: [PATCH 2/4] don't leave 2nd store in limbo if opening 1st store fails synchronously we can't leave the store FRESH, as otherwise the error handling code will assume it is still being opened and will return to the main loop. depending on the config this would cause an immediate termination or an indefinite wait. --- src/main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main.c b/src/main.c index 3efd179..770de0d 100644 --- a/src/main.c +++ b/src/main.c @@ -593,12 +593,16 @@ sync_chans( main_vars_t *mvars, int ent ) labels[M] = "M: ", labels[S] = "S: "; else labels[M] = labels[S] = ""; - for (t = 0; t < 2; t++) { + for (t = 0; ; t++) { info( "Opening %s %s...\n", str_ms[t], mvars->chan->stores[t]->name ); mvars->drv[t] = mvars->chan->stores[t]->driver; mvars->drv[t]->open_store( mvars->chan->stores[t], labels[t], store_opened, AUX ); - if (mvars->skip) + if (t) break; + if (mvars->skip) { + mvars->state[1] = ST_CLOSED; + break; + } } mvars->cben = 1; opened: From 958af473a08c386a86e88277d5da9669ead4a6cd Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 2 Jan 2015 12:38:48 +0100 Subject: [PATCH 3/4] fix conditional for early failure in cancel_done() --- src/sync.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sync.c b/src/sync.c index fd1ba4f..99f3895 100644 --- a/src/sync.c +++ b/src/sync.c @@ -497,11 +497,12 @@ cancel_done( void *aux ) svars->state[t] |= ST_CANCELED; if (svars->state[1-t] & ST_CANCELED) { - if (svars->lfd) { + if (svars->lfd >= 0) { Fclose( svars->nfp, 0 ); Fclose( svars->jfp, 0 ); sync_bail( svars ); } else { + /* Early failure during box selection. */ sync_bail2( svars ); } } From 2fa75cf159d18c5705a877690b62f9e5de160c81 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 11 Jan 2015 14:29:19 +0100 Subject: [PATCH 4/4] fix UID assignment with some non-UIDPLUS servers the seznam.cz IMAP server seems very eager to send UIDNEXT responses despite not supporting UIDPLUS. this doesn't appear to be a particularly sensible combination, but it's valid nonetheless. however, that means that we need to save the UIDNEXT value before we start storing messages, lest imap_find_new_msgs() will simply overlook them. we do that outside the driver, in an already present field - this actually makes the main path more consistent with the journal recovery path. analysis by Tomas Tintera . REFMAIL: 20141220215032.GA10115@kyvadlo.trosos.seznam.cz --- src/driver.h | 2 +- src/drv_imap.c | 21 ++++++++++++++------- src/drv_maildir.c | 2 +- src/sync.c | 5 +++-- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/driver.h b/src/driver.h index 31ee1bf..4557aa6 100644 --- a/src/driver.h +++ b/src/driver.h @@ -192,7 +192,7 @@ struct driver { /* Index the messages which have newly appeared in the mailbox, including their * temporary UID headers. This is needed if store_msg() does not guarantee returning * a UID; otherwise the driver needs to implement only the OPEN_FIND flag. */ - void (*find_new_msgs)( store_t *ctx, + void (*find_new_msgs)( store_t *ctx, int newuid, void (*cb)( int sts, void *aux ), void *aux ); /* Add/remove the named flags to/from the given message. The message may be either diff --git a/src/drv_imap.c b/src/drv_imap.c index e6d93ee..f58736d 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -153,6 +153,11 @@ struct imap_cmd_out_uid { int out_uid; }; +struct imap_cmd_find_new { + struct imap_cmd_simple gen; + int uid; +}; + struct imap_cmd_refcounted_state { void (*callback)( int sts, void *aux ); void *callback_aux; @@ -2158,28 +2163,30 @@ imap_store_msg_p2( imap_store_t *ctx ATTR_UNUSED, struct imap_cmd *cmd, int resp static void imap_find_new_msgs_p2( imap_store_t *, struct imap_cmd *, int ); static void -imap_find_new_msgs( store_t *gctx, +imap_find_new_msgs( store_t *gctx, int newuid, void (*cb)( int sts, void *aux ), void *aux ) { imap_store_t *ctx = (imap_store_t *)gctx; - struct imap_cmd_simple *cmd; + struct imap_cmd_find_new *cmd; - INIT_IMAP_CMD(imap_cmd_simple, cmd, cb, aux) - imap_exec( (imap_store_t *)ctx, &cmd->gen, imap_find_new_msgs_p2, "CHECK" ); + INIT_IMAP_CMD_X(imap_cmd_find_new, cmd, cb, aux) + cmd->uid = newuid; + imap_exec( (imap_store_t *)ctx, &cmd->gen.gen, imap_find_new_msgs_p2, "CHECK" ); } static void imap_find_new_msgs_p2( imap_store_t *ctx, struct imap_cmd *gcmd, int response ) { - struct imap_cmd_simple *cmdp = (struct imap_cmd_simple *)gcmd, *cmd; + struct imap_cmd_find_new *cmdp = (struct imap_cmd_find_new *)gcmd; + struct imap_cmd_simple *cmd; if (response != RESP_OK) { imap_done_simple_box( ctx, gcmd, response ); return; } - INIT_IMAP_CMD(imap_cmd_simple, cmd, cmdp->callback, cmdp->callback_aux) + INIT_IMAP_CMD(imap_cmd_simple, cmd, cmdp->gen.callback, cmdp->gen.callback_aux) imap_exec( (imap_store_t *)ctx, &cmd->gen, imap_done_simple_box, - "UID FETCH %d:1000000000 (UID BODY.PEEK[HEADER.FIELDS (X-TUID)])", ctx->gen.uidnext ); + "UID FETCH %d:1000000000 (UID BODY.PEEK[HEADER.FIELDS (X-TUID)])", cmdp->uid ); } /******************* imap_list *******************/ diff --git a/src/drv_maildir.c b/src/drv_maildir.c index f936938..f151b5b 100644 --- a/src/drv_maildir.c +++ b/src/drv_maildir.c @@ -1292,7 +1292,7 @@ maildir_store_msg( store_t *gctx, msg_data_t *data, int to_trash, } static void -maildir_find_new_msgs( store_t *gctx ATTR_UNUSED, +maildir_find_new_msgs( store_t *gctx ATTR_UNUSED, int newuid ATTR_UNUSED, void (*cb)( int sts, void *aux ) ATTR_UNUSED, void *aux ATTR_UNUSED ) { assert( !"maildir_find_new_msgs is not supposed to be called" ); diff --git a/src/sync.c b/src/sync.c index 99f3895..443da1b 100644 --- a/src/sync.c +++ b/src/sync.c @@ -1506,7 +1506,8 @@ box_loaded( int sts, void *aux ) if (UseFSync) fdatasync( fileno( svars->jfp ) ); for (t = 0; t < 2; t++) { - Fprintf( svars->jfp, "%c %d\n", "{}"[t], svars->ctx[t]->uidnext ); + svars->newuid[t] = svars->ctx[t]->uidnext; + Fprintf( svars->jfp, "%c %d\n", "{}"[t], svars->newuid[t] ); for (tmsg = svars->ctx[1-t]->msgs; tmsg; tmsg = tmsg->next) { if ((srec = tmsg->srec) && srec->tuid[0]) { svars->new_total[t]++; @@ -1604,7 +1605,7 @@ msgs_copied( sync_vars_t *svars, int t ) if (svars->state[t] & ST_FIND_NEW) { debug( "finding just copied messages on %s\n", str_ms[t] ); - svars->drv[t]->find_new_msgs( svars->ctx[t], msgs_found_new, AUX ); + svars->drv[t]->find_new_msgs( svars->ctx[t], svars->newuid[t], msgs_found_new, AUX ); } else { msgs_new_done( svars, t ); }