Re: Segfault when using doveadm batch -A : kick

2018-12-07 Thread Apollon Oikonomopoulos
On 21:33 Fri 07 Dec , Apollon Oikonomopoulos wrote:
> Hi,
> 
> Apparently the "kick" doveadm_cmd_ver2 struct lacks a .mail_cmd member 
> pointing to an appropriate allocation function, causing a NULL pointer 
> dereference when used via `doveadm batch`.
> 
> (gdb) bt
> #0  0x in ?? ()
> #1  0x55585882 in doveadm_mail_cmd_init 
> (cmd=cmd@entry=0x7fffe680, set=0x555f2440) at doveadm-mail.c:596
> #2  0x55586f68 in cmd_batch_add (argv=, 
> argc=, batchctx=0x55606538) at doveadm-mail-batch.c:78
> #3  cmd_batch_preinit () at doveadm-mail-batch.c:126
> #4  0x555854ce in doveadm_mail_cmd_exec () at doveadm-mail.c:632
> #5  0x55585e66 in doveadm_mail_cmd (argv=, argc=4, 
> cmd=0x55602a00) at doveadm-mail.c:748
> #6  doveadm_mail_try_run () at doveadm-mail.c:821
> #7  0x55575e7f in main () at doveadm.c:404
> #8  0x774acb17 in __libc_start_main (main=0x55575990 , 
> argc=5, argv=0x7fffea18, init=, fini=, 
> rtld_fini=, stack_end=0x7fffea08) at 
> ../csu/libc-start.c:310
> #9  0x55575fca in _start () at doveadm-mail.c:1127
> 
> (gdb) p *cmd
> $5 = {alloc = 0x0, name = 0x555bdd0c "kick", usage_args = 0x555be738 
> "[-a ] [|]"}
> 

Forgot to add, this seems to affect both, 2.2 and 2.3 series.


Segfault when using doveadm batch -A : kick

2018-12-07 Thread Apollon Oikonomopoulos
Hi,

Apparently the "kick" doveadm_cmd_ver2 struct lacks a .mail_cmd member 
pointing to an appropriate allocation function, causing a NULL pointer 
dereference when used via `doveadm batch`.

(gdb) bt
#0  0x in ?? ()
#1  0x55585882 in doveadm_mail_cmd_init (cmd=cmd@entry=0x7fffe680, 
set=0x555f2440) at doveadm-mail.c:596
#2  0x55586f68 in cmd_batch_add (argv=, argc=, batchctx=0x55606538) at doveadm-mail-batch.c:78
#3  cmd_batch_preinit () at doveadm-mail-batch.c:126
#4  0x555854ce in doveadm_mail_cmd_exec () at doveadm-mail.c:632
#5  0x55585e66 in doveadm_mail_cmd (argv=, argc=4, 
cmd=0x55602a00) at doveadm-mail.c:748
#6  doveadm_mail_try_run () at doveadm-mail.c:821
#7  0x55575e7f in main () at doveadm.c:404
#8  0x774acb17 in __libc_start_main (main=0x55575990 , 
argc=5, argv=0x7fffea18, init=, fini=, 
rtld_fini=, stack_end=0x7fffea08) at 
../csu/libc-start.c:310
#9  0x55575fca in _start () at doveadm-mail.c:1127

(gdb) p *cmd
$5 = {alloc = 0x0, name = 0x555bdd0c "kick", usage_args = 0x555be738 
"[-a ] [|]"}

(This is Debian bug #915411[1])

Regards,
Apollon

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=915411


Re: murmurhash3 test failures on big-endian systems

2018-03-29 Thread Apollon Oikonomopoulos
On 15:55 Wed 28 Mar , Josef 'Jeff' Sipek wrote:
> Ok, there are two commits:
> 
> 35497604d80090a02619024aeec069b32568e4b4 and 
> 5522b8b3d3ed1a99c3b63bb120216af0bd427403
> 
> Together, they should be identical to the patch I sent the other day plus
> your fixup.  Let me know if you have any problems.

Indeed, there are no differences, but I updated the patch in the package 
to include the commit IDs nevertheless.

Also, dovecot now seems to build fine on all big-endian 
architectures[1].

Thanks again,
Apollon

[1] https://buildd.debian.org/status/package.php?p=dovecot&suite=experimental


Re: murmurhash3 test failures on big-endian systems

2018-03-27 Thread Apollon Oikonomopoulos
On 13:05 Tue 27 Mar , Apollon Oikonomopoulos wrote:
> On 11:31 Tue 27 Mar , Apollon Oikonomopoulos wrote:
> It turns out there's a missing byte-inversion when loading the blocks 
> which should be addressed in getblock{32,64}. Murmurhash treats each 
> block as an integer expecting little-endian storage. Applying this 
> additional change fixes the build on s390x (and does not break it on 
> x864_64):
> 
> --- b/src/lib/murmurhash3.c
> +++ b/src/lib/murmurhash3.c
> @@ -23,7 +23,7 @@
> 
>  static inline uint32_t getblock32(const uint32_t *p, int i)
>  {
> -  return p[i];
> +  return cpu32_to_le(p[i]);

… or perhaps le32_to_cpu, although it should be the same in the end. 

>  }
> 
>  
> //-
> @@ -105,7 +105,7 @@
> 
>  static inline uint64_t getblock64(const uint64_t *p, int i)
>  {
> -  return p[i];
> +  return cpu64_to_le(p[i]);
>  }
> 
> Regards,
> Apollon


Re: murmurhash3 test failures on big-endian systems

2018-03-27 Thread Apollon Oikonomopoulos
On 11:31 Tue 27 Mar , Apollon Oikonomopoulos wrote:
> Hi,
> 
> On 12:55 Mon 26 Mar , Josef 'Jeff' Sipek wrote:
> > On Mon, Mar 26, 2018 at 15:57:01 +0300, Apollon Oikonomopoulos wrote:
> > ...
> > > I'd be happy to test the patch, thanks!
> > 
> > Ok, try the attached patch.  (It is a first pass at the issue, so it may not
> > be the final diff that'll end up getting committed.  It'd be good to know if
> > it actually fixes the issue for you - sadly, I don't have a big endian
> > system to play with.)
> 
> Thanks for the quick response!
> 
> Unfortunately still fails, although with fewer assertion errors than 
> before:
> 
> test-murmurhash3.c:34: Assert(#8) failed: memcmp(result, vectors[i].result, 
> sizeof(result)) == 0
> test-murmurhash3.c:34: Assert(#11) failed: memcmp(result, vectors[i].result, 
> sizeof(result)) == 0
> test-murmurhash3.c:34: Assert(#12) failed: memcmp(result, vectors[i].result, 
> sizeof(result)) == 0
> murmurhash3 (murmurhash3_32) . : 
> FAILED
> test-murmurhash3.c:34: Assert(#12) failed: memcmp(result, vectors[i].result, 
> sizeof(result)) == 0
> murmurhash3 (murmurhash3_128)  : 
> FAILED

It turns out there's a missing byte-inversion when loading the blocks 
which should be addressed in getblock{32,64}. Murmurhash treats each 
block as an integer expecting little-endian storage. Applying this 
additional change fixes the build on s390x (and does not break it on 
x864_64):

--- b/src/lib/murmurhash3.c
+++ b/src/lib/murmurhash3.c
@@ -23,7 +23,7 @@

 static inline uint32_t getblock32(const uint32_t *p, int i)
 {
-  return p[i];
+  return cpu32_to_le(p[i]);
 }

 //-
@@ -105,7 +105,7 @@

 static inline uint64_t getblock64(const uint64_t *p, int i)
 {
-  return p[i];
+  return cpu64_to_le(p[i]);
 }

Regards,
Apollon


Re: murmurhash3 test failures on big-endian systems

2018-03-27 Thread Apollon Oikonomopoulos
Hi,

On 12:55 Mon 26 Mar , Josef 'Jeff' Sipek wrote:
> On Mon, Mar 26, 2018 at 15:57:01 +0300, Apollon Oikonomopoulos wrote:
> ...
> > I'd be happy to test the patch, thanks!
> 
> Ok, try the attached patch.  (It is a first pass at the issue, so it may not
> be the final diff that'll end up getting committed.  It'd be good to know if
> it actually fixes the issue for you - sadly, I don't have a big endian
> system to play with.)

Thanks for the quick response!

Unfortunately still fails, although with fewer assertion errors than 
before:

test-murmurhash3.c:34: Assert(#8) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
test-murmurhash3.c:34: Assert(#11) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
test-murmurhash3.c:34: Assert(#12) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
murmurhash3 (murmurhash3_32) . : FAILED
test-murmurhash3.c:34: Assert(#12) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
murmurhash3 (murmurhash3_128)  : FAILED

Regards,
Apollon


Re: murmurhash3 test failures on big-endian systems

2018-03-26 Thread Apollon Oikonomopoulos
Hi Aki,

On 15:55 Mon 26 Mar , Aki Tuomi wrote:
> On 26.03.2018 15:49, Apollon Oikonomopoulos wrote:
> > Hi,
> >
> > The dovecot 2.3.0.1 Debian package currently fails to build on all 
> > big-endian architectures[1], due to murmurhash3 tests failing. The 
> > relevant output from e.g. s390x is:
> >
> >  test-murmurhash3.c:22: Assert(#8) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#11) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#12) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  murmurhash3 (murmurhash3_32) . : 
> > FAILED
> >  test-murmurhash3.c:22: Assert(#1) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#2) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#3) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#4) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#5) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#6) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#7) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#8) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#9) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#10) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#11) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#12) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  test-murmurhash3.c:22: Assert(#13) failed: memcmp(result, 
> > vectors[i].result, sizeof(result)) == 0
> >  murmurhash3 (murmurhash3_128)  : 
> > FAILED
> >
> > Looks like the murmurhash3 implementation in Dovecot is currently broken on
> > big-endian systems.
> >
> > Regards,
> > Apollon
> >
> > [1] 
> > https://buildd.debian.org/status/package.php?p=dovecot&suite=experimental
> Hi!
> 
> Thanks for reporting this, we'll look at it. It's not going to get fixed
> on 2.3.1 though, but we can provide a patch for that.

I'd be happy to test the patch, thanks!

Apollon


murmurhash3 test failures on big-endian systems

2018-03-26 Thread Apollon Oikonomopoulos
Hi,

The dovecot 2.3.0.1 Debian package currently fails to build on all 
big-endian architectures[1], due to murmurhash3 tests failing. The 
relevant output from e.g. s390x is:

 test-murmurhash3.c:22: Assert(#8) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#11) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#12) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 murmurhash3 (murmurhash3_32) . : FAILED
 test-murmurhash3.c:22: Assert(#1) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#2) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#3) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#4) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#5) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#6) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#7) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#8) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#9) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#10) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#11) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#12) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 test-murmurhash3.c:22: Assert(#13) failed: memcmp(result, vectors[i].result, 
sizeof(result)) == 0
 murmurhash3 (murmurhash3_128)  : FAILED

Looks like the murmurhash3 implementation in Dovecot is currently broken on
big-endian systems.

Regards,
Apollon

[1] https://buildd.debian.org/status/package.php?p=dovecot&suite=experimental


Re: Bug#876364: dovecot-sieve: Just discovered imap_sieve/sieve_imapsieve is not set up to work with virtual mailboxes.

2017-09-21 Thread Apollon Oikonomopoulos
Control: tags -1 + moreinfo upstream

[Forwarding this to the Dovecot mailing list, just in case someone can help]

Hi,

Thanks for the report! See my comments inline.

On 11:56 Thu 21 Sep , Thurgood Angelou wrote:
> Package: dovecot-core
> Version: 1:2.2.32-2
>
> I've just discovered a bug where the sieve plugin (especially IMAP) 
> will not work with a virtual mailbox. I recently found this out when 
> testing a virtual mailbox setup and I use imap_sieve for SPAM 
> filtering so to lose that would be painful.
> 
> This is what is reported in dovecot's debug log...
> 
> Sep 21 10:53:01 imap(perso...@othersider92.com): Panic: file 
> mail-index-map.c: line 549 (mail_index_map_lookup_seq_range): assertion 
> failed: (first_uid > 0)
> Sep 21 10:53:01 imap(perso...@othersider92.com): Error: Raw backtrace: 
> /usr/lib/dovecot/libdovecot.so.0(+0x9f0a2) [0x7f4226fd50a2] -> 
> /usr/lib/dovecot/libdovecot.so.0(+0x9f19a) [0x7f4226fd519a] -> 
> /usr/lib/dovecot/libdovecot.so.0(i_fatal+0) [0x7f4226f65cf8] -> 
> /usr/lib/dovecot/libdovecot-storage.so.0(mail_index_map_lookup_seq_range+0x120)
>  [0x7f422733dae0] -> /usr/lib/dovecot/libdovecot-storage.so.0(+0xe8d1d) 
> [0x7f422734cd1d] -> 
> /usr/lib/dovecot/libdovecot-storage.so.0(mail_index_lookup_seq+0xf) 
> [0x7f4227350e9f] -> /usr/lib/dovecot/modules/lib20_virtual_plugin.so(+0x82aa) 
> [0x7f422678a2aa] -> 
> /usr/lib/dovecot/modules/lib95_imap_sieve_plugin.so(+0x708c) [0x7f422657d08c] 
> -> 
> /usr/lib/dovecot/libdovecot-storage.so.0(mailbox_transaction_commit_get_changes+0x52)
>  [0x7f42272adca2] -> dovecot/imap(+0x10084) [0x556502adb084] -> 
> dovecot/imap(command_exec+0x5c) [0x556502ae7d9c] -> dovecot/imap(+0x1b322) 
> [0x556502ae6322] -> dovecot/imap(+0x1b3bc) [0x556502ae63bc] -> 
> dovecot/imap(client_handle_input
>  +0x18d) [0x556502ae677d] -> dovecot/imap(client_input+0xac) [0x556502ae6ccc] 
> -> /usr/lib/dovecot/libdovecot.so.0(io_loop_call_io+0x52) [0x7f4226fea742] -> 
> /usr/lib/dovecot/libdovecot.so.0(io_loop_handler_run_internal+0x12e) 
> [0x7f4226febd8e] -> 
> /usr/lib/dovecot/libdovecot.so.0(io_loop_handler_run+0x36) [0x7f4226fea7d6] 
> -> /usr/lib/dovecot/libdovecot.so.0(io_loop_run+0x38) [0x7f4226fea988] -> 
> /usr/lib/dovecot/libdovecot.so.0(master_service_run+0x13) [0x7f4226f70353] -> 
> dovecot/imap(main+0x329) [0x556502ad9139] -> 
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1) [0x7f4226bb92e1] -> 
> dovecot/imap(_start+0x2a) [0x556502ad92ca]

After mixing in the debug symbols, this backtrace becomes:

/usr/lib/dovecot/libdovecot.so.0(+0x9f0a2) [0x7f4226fd50a2]
default_fatal_finish at ./src/lib/failures.c:195
/usr/lib/dovecot/libdovecot.so.0(+0x9f19a) [0x7f4226fd519a]
?? at ./src/lib/failures.c:670
/usr/lib/dovecot/libdovecot.so.0(i_fatal+0) [0x7f4226f65cf8]
i_fatal at ./src/lib/failures.c:280
/usr/lib/dovecot/libdovecot-storage.so.0(mail_index_map_lookup_seq_range+0x120) 
[0x7f422733dae0]
mail_index_map_lookup_seq_range at ./src/lib-index/mail-index-map.c:550
/usr/lib/dovecot/libdovecot-storage.so.0(+0xe8d1d) [0x7f422734cd1d]
tview_lookup_seq_range at ./src/lib-index/mail-index-transaction-view.c:178
/usr/lib/dovecot/libdovecot-storage.so.0(mail_index_lookup_seq+0xf) 
[0x7f4227350e9f]
mail_index_lookup_seq at ./src/lib-index/mail-index-view.c:522
/usr/lib/dovecot/modules/lib20_virtual_plugin.so(+0x82aa) [0x7f422678a2aa]
virtual_mail_set_uid at ./src/plugins/virtual/virtual-mail.c:213
  
/usr/lib/dovecot/modules/lib95_imap_sieve_plugin.so(+0x708c) [0x7f422657d08c]
imap_sieve_mailbox_transaction_run at 
./pigeonhole/src/plugins/imapsieve/imap-sieve-storage.c:752 (inlined by: 
imap_sieve_mailbox_transaction_commit at 
./pigeonhole/src/plugins/imapsieve/imap-sieve-storage.c:807)
/usr/lib/dovecot/libdovecot-storage.so.0(mailbox_transaction_commit_get_changes+0x52)
 [0x7f42272adca2]
mailbox_transaction_commit_get_changes at 
./src/lib-storage/mail-storage.c:2083
dovecot/imap(+0x10084) [0x556502adb084]
cmd_copy_full at ./src/imap/cmd-copy.c:146
dovecot/imap(command_exec+0x5c) [0x556502ae7d9c]
command_exec at ./src/imap/imap-commands.c:200
dovecot/imap(+0x1b322) [0x556502ae6322]
client_command_input at ./src/imap/imap-client.c:1088
dovecot/imap(+0x1b3bc) [0x556502ae63bc]
client_command_input at ./src/imap/imap-client.c:1150
dovecot/imap(client_handle_input+0x18d) [0x556502ae677d]
client_handle_input at ./src/imap/imap-client.c:1203
dovecot/imap(client_input+0xac) [0x556502ae6ccc]
client_input at ./src/imap/imap-client.c:1249
/usr/lib/dovecot/libdovecot.so.0(io_loop_call_io+0x52) [0x7f4226fea742]
io_loop_call_io at ./src/lib/ioloop.c:600
/usr/lib/dovecot/libdovecot.so.0(io_loop_handler_run_internal+0x12e) 
[0x7f4226febd8e]
io_loop_handler_run_internal at ./src/lib/ioloop-epoll.c:223
/usr/lib/dovecot/libdovecot.so.0(io_loop_handler_run+0x36) [0x7f4226fea7d6]
io_loop_handler_run at ./src/lib/ioloop.c:648
/usr/lib/dovecot/libdove

[RFC master-2.2 1/1] Support setting min/max SSL protocol version

2017-09-13 Thread Apollon Oikonomopoulos
OpenSSL 1.1 exposes a new API for setting the minimum and maximum
supported SSL protocol version, using SSL_CTX_set_min_proto_version and
SSL_CTX_set_max_proto_version respectively. The main difference with the
old SSL_CTX_set_options API is that the new API can either restrict or
relax the library defaults; the old API could only be used to
selectively disable protocols (but not enable what might have been
disabled by default).

The new API allows distributions and vendors to ship OpenSSL versions
with stricter run-time defaults (e.g. TLSv1.2-only), while still
allowing applications to enable older protocols (e.g. TLSv1) when
dealing with legacy clients.

To support the new API, we add two new config file options,
ssl_min_proto_version and ssl_max_proto_version. These settings are only
effective when built against OpenSSL 1.1. Also, dovecot will issue a
warning if the old-style ssl_options config file option is encountered
while running on OpenSSL 1.1 (although it will not ignore the option at
this point).

Signed-off-by: Apollon Oikonomopoulos 
---
 doc/example-config/conf.d/10-ssl.conf  |  4 
 src/config/config-parser.c | 25 +
 src/lib-master/master-service-ssl-settings.c   |  4 
 src/lib-master/master-service-ssl-settings.h   |  2 ++
 src/lib-master/master-service-ssl.c|  2 ++
 src/lib-ssl-iostream/iostream-openssl-common.c | 12 +++
 src/lib-ssl-iostream/iostream-openssl.h|  1 +
 src/lib-ssl-iostream/iostream-ssl.h|  2 ++
 src/login-common/ssl-proxy-openssl.c   | 30 ++
 9 files changed, 82 insertions(+)

diff --git a/doc/example-config/conf.d/10-ssl.conf 
b/doc/example-config/conf.d/10-ssl.conf
index cf651c252..aceae233a 100644
--- a/doc/example-config/conf.d/10-ssl.conf
+++ b/doc/example-config/conf.d/10-ssl.conf
@@ -47,6 +47,10 @@ ssl_key = 
 #include 
 #include 
+#include 
 #ifdef HAVE_GLOB_H
 #  include 
 #endif
@@ -419,6 +420,11 @@ config_all_parsers_check(struct config_parser_context *ctx,
struct master_service_settings_output output;
unsigned int i, count;
const char *ssl_set, *global_ssl_set;
+#if OPENSSL_VERSION_NUMBER >= 0x1010
+   const char *ssl_protocols;
+#else
+   const char *ssl_min_proto_version, *ssl_max_proto_version;
+#endif
pool_t tmp_pool;
bool ssl_warned = FALSE;
int ret = 0;
@@ -454,6 +460,25 @@ config_all_parsers_check(struct config_parser_context *ctx,
ssl_warned = TRUE;
}
 
+#if OPENSSL_VERSION_NUMBER >= 0x1010
+   ssl_protocols = get_str_setting(parsers[i], "ssl_protocols", 
"");
+   if (*ssl_protocols != '\0')
+   i_warning("ssl_protocols is deprecated and will be "
+ "ignored in future versions when running "
+ "with OpenSSL 1.1. Please use "
+ "ssl_min_proto_version and "
+ "ssl_max_proto_version instead.");
+#else
+   ssl_min_proto_version = get_str_setting(parsers[i],
+   "ssl_min_proto_version", "");
+   ssl_max_proto_version = get_str_setting(parsers[i],
+   "ssl_max_proto_version", "");
+   if ((*ssl_min_proto_version != '\0') ||
+   (*ssl_max_proto_version != '\0'))
+   i_warning("ssl_*_proto_version ignored, "
+ "not supported by OpenSSL");
+#endif
+
ret = config_filter_parser_check(ctx, tmp_parsers, error_r);
config_filter_parsers_free(tmp_parsers);
p_clear(tmp_pool);
diff --git a/src/lib-master/master-service-ssl-settings.c 
b/src/lib-master/master-service-ssl-settings.c
index 2487c8369..484022618 100644
--- a/src/lib-master/master-service-ssl-settings.c
+++ b/src/lib-master/master-service-ssl-settings.c
@@ -24,6 +24,8 @@ static const struct setting_define 
master_service_ssl_setting_defines[] = {
DEF(SET_STR, ssl_key_password),
DEF(SET_STR, ssl_cipher_list),
DEF(SET_STR, ssl_protocols),
+   DEF(SET_STR, ssl_min_proto_version),
+   DEF(SET_STR, ssl_max_proto_version),
DEF(SET_STR, ssl_cert_username_field),
DEF(SET_STR, ssl_crypto_device),
DEF(SET_BOOL, ssl_verify_client_cert),
@@ -53,6 +55,8 @@ static const struct master_service_ssl_settings 
master_service_ssl_default_setti
 #else
.ssl_protocols = "!SSLv3",
 #endif
+   .ssl_min_proto_version = "",
+   .ssl_max_proto_version = "",
.ssl_cert_username_field = "commonName",
.ssl_crypto_device = "",

[RFC master-2.2 0/1] Support OpenSSL 1.1 API for setting allowed TLS versions

2017-09-13 Thread Apollon Oikonomopoulos
Hi,

I came up with the following patch while trying to figure out a good solution
for the situation described in Debian bug #871987[1]. In short, OpenSSL in
Debian unstable has disabled TLSv1.0 and TLSv1.1 *by default*. That means that
unless an application requests otherwise, only TLSv1.2 is supported. In the
world of e-mail this is seemingly an issue, as there are still way too many old
clients out there supporting only TLSv1 or TLSv1.1.

Now, traditionally OpenSSL 0.9.8/1.0 used SSL_CTX_set_options() to allow
*disabling* specific protocols, without offering a way to enable previously
disabled protocols. OpenSSL 1.1 introduced a dedicated API[2] to set allowed
protocol versions, taking a linear version approach: the application may
request a minimum and a maximum allowed version (inclusive), allowing all
versions inbetween as well.

Dovecot's existing ssl_protocols option is probably not ideal to use with this
new "linear" model. Instead, I introduced two new options,
ssl_min_proto_version and ssl_max_proto_version, that map directly to OpenSSL
1.1 concepts.

I have tested the patch with both OpenSSL 1.0 and OpenSSL 1.1. With OpenSSL 1.1
it works as expected; with OpenSSL 1.0 it doesn't seem to break anything. Other
than that, this is a first version; I'm sure there are still things to improve,
so comments are welcome :)

Regards,
Apollon

[1] https://bugs.debian.org/871987
[2] https://www.openssl.org/docs/man1.1.0/ssl/SSL_CTX_set_min_proto_version.html

Apollon Oikonomopoulos (1):
  Support setting min/max SSL protocol version

 doc/example-config/conf.d/10-ssl.conf  |  4 
 src/config/config-parser.c | 25 +
 src/lib-master/master-service-ssl-settings.c   |  4 
 src/lib-master/master-service-ssl-settings.h   |  2 ++
 src/lib-master/master-service-ssl.c|  2 ++
 src/lib-ssl-iostream/iostream-openssl-common.c | 12 +++
 src/lib-ssl-iostream/iostream-openssl.h|  1 +
 src/lib-ssl-iostream/iostream-ssl.h|  2 ++
 src/login-common/ssl-proxy-openssl.c   | 30 ++
 9 files changed, 82 insertions(+)

-- 
2.14.1


Re: [PATCH] Manually cleanup OpenSSL from dovecot_openssl_common_global_unref()

2016-11-21 Thread Apollon Oikonomopoulos
Hi,

On 22:59 Sun 20 Nov , Reuben Farrelly wrote:
> Hi,
> 
> This patch:
> 
> 
> ... which was committed as c164f8afe58c8d83ef2a48aae629c72408dfea01 in
> master-2.2, terminally breaks the build with LibreSSL.  Obviously this
> wasn't tested or considered ;)

Yes, unfortunately LibreSSL fakes OpenSSL version numbers and reports a 
version much bigger than 1.1 while actually providing the API of 1.0.x.  
I will submit an additional patch to fix the guard condition there.

Regards,
Apollon


[PATCH] ssl: fix reference to SSLv2 and disable SSLv3

2016-11-15 Thread Apollon Oikonomopoulos
This is driven by the fact that OpenSSL 1.1 does not know about SSLv2 at
all and dovecot's defaults simply make OpenSSL error out with "Unknown
protocol 'SSLv2'"[1]. So we change the defaults to refer to SSLv2 iff OpenSSL
seems to know something about it.

While at it, it's also a good idea to disable SSLv3 by default as well.

[1] https://bugs.debian.org/844347

Signed-off-by: Apollon Oikonomopoulos 
---
 doc/example-config/conf.d/10-ssl.conf| 2 +-
 src/lib-master/master-service-ssl-settings.c | 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/doc/example-config/conf.d/10-ssl.conf 
b/doc/example-config/conf.d/10-ssl.conf
index 31b750c..2cd445b 100644
--- a/doc/example-config/conf.d/10-ssl.conf
+++ b/doc/example-config/conf.d/10-ssl.conf
@@ -46,7 +46,7 @@ ssl_key = 

[PATCH] Manually cleanup OpenSSL from dovecot_openssl_common_global_unref()

2016-11-13 Thread Apollon Oikonomopoulos
OpenSSL 1.1 features a cleanup function that is automatically run on shutdown
using atexit(3). This function frees all OpenSSL-allocated resources.

In dovecot, OpenSSL is loaded indirectly using dlopen(3) against the relevant
dovecot crypto module and is finally unloaded using dlclose(3). Until
OpenSSL 1.0.1c this worked fine, however OpenSSL 1.0.1c makes sure[1] that the
library stays loaded after the initial dlclose() so that the atexit(3)
handlers can run on shutdown. This, together with the fact that dovecot
uses custom allocation functions for OpenSSL and has already partially
free()'d some of OpenSSL's resources in module_free(), leads to a
segfault at process shutdown[2].

We fix this by explicitly calling OPENSSL_cleanup() during module unload. This
is safe to do, as long as we will never want to subsequently re-initialize
OpenSSL.

[1] 
https://github.com/openssl/openssl/commit/4af9f7fe79ff82b90c16969b7e5871435056377b
[2] 
https://buildd.debian.org/status/fetch.php?pkg=dovecot&arch=amd64&ver=1:2.2.26.0-2&stamp=1478873022

Signed-off-by: Apollon Oikonomopoulos 
---
 src/lib-ssl-iostream/dovecot-openssl-common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/lib-ssl-iostream/dovecot-openssl-common.c 
b/src/lib-ssl-iostream/dovecot-openssl-common.c
index 51ea3ad..2bf6307 100644
--- a/src/lib-ssl-iostream/dovecot-openssl-common.c
+++ b/src/lib-ssl-iostream/dovecot-openssl-common.c
@@ -101,6 +101,9 @@ bool dovecot_openssl_common_global_unref(void)
ERR_remove_thread_state(NULL);
 #endif
ERR_free_strings();
+#if OPENSSL_VERSION_NUMBER >= 0x1010L
+   OPENSSL_cleanup();
+#endif
return FALSE;
 }
 
-- 
2.10.1


Sieve script replication glitches

2015-12-09 Thread Apollon Oikonomopoulos
Hi,

I'm running a replicated setup on Debian with Dovecot 2.2.18 and 
Pigeonhole 0.4.8 and run into some glitches with sieve script syncing.  
Replication is implemented via DSync/doveadm over SSL.

While e-mail replication works as expected, sieve script replication is 
not always successful. In general I have observed the following 
behaviour re. sieve script replication:

 - Sometimes sieve scripts are replicated but have an mtime of 0 (Jan 1 
   1970) on the destination.

 - Subsequent updates to these "bad" scripts are not replicated at all.

 - Sometimes after creating *another* sieve script (which may or may not 
   get the correct timestamp), the "bad" script's timestamp/contents are 
   corrected on the receiving side.

The following is a minimal real-world example of what happens (including
transaction log entries):

1. On the master create a new sieve script:
cat test.sieve | doveadm sieve put -u u...@example.com test1

   Sieve storage on the master:
-rw--- 1 vmail vmail 2001 Dec  9 15:53 test1.sieve

   Transaction log on the master:
record: offset=44756, type=attribute-update (ext), size=72, modseq=514
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test1 : 
timestamp=2015-12-09 15:53:28 value_len=0
record: offset=44828, type=modseq-update (ext), size=20
 - uid=0 modseq=723

   Sieve storage on the slave:
-rw--- 1 vmail vmail 2001 Jan  1  1970 test1.sieve

   Transaction log on the slave:
record: offset=55356, type=attribute-update (ext), size=72, modseq=597
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test1 : 
timestamp=2015-12-09 15:53:29 value_len=0
record: offset=55428, type=attribute-update (sync), size=72, modseq=598
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test1 : 
timestamp=2015-12-09 15:53:29 value_len=0

2. Create a new script on the master:
cat test.sieve | doveadm sieve put -u u...@example.com test2

   Sieve storage on the master:
-rw--- 1 vmail vmail 2001 Dec  9 15:53 test1.sieve
-rw--- 1 vmail vmail 2001 Dec  9 15:57 test2.sieve

   Transaction log on the master:
record: offset=44848, type=attribute-update (ext), size=72, modseq=515
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test2 : 
timestamp=2015-12-09 15:57:14 value_len=0
record: offset=44920, type=modseq-update (ext), size=20
 - uid=0 modseq=725
record: offset=44940, type=modseq-update (ext), size=20
 - uid=0 modseq=728
   
   Sieve storage on the slave:
-rw--- 1 vmail vmail 2001 Dec  9 15:53 test1.sieve
-rw--- 1 vmail vmail 2001 Dec  9 15:57 test2.sieve

   Transaction log on the slave:
record: offset=55500, type=attribute-update (ext), size=72, modseq=599
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test2 : 
timestamp=2015-12-09 15:57:14 value_len=0
record: offset=55572, type=attribute-update (sync), size=72, modseq=600
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test2 : 
timestamp=2015-12-09 15:57:14 value_len=0
record: offset=55644, type=attribute-update (ext), size=72, modseq=601
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test1 : 
timestamp=2015-12-09 15:57:14 value_len=0
record: offset=55716, type=attribute-update (ext), size=72, modseq=602
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test2 : 
timestamp=2015-12-09 15:57:14 value_len=0
record: offset=55788, type=attribute-update (sync), size=132, modseq=603
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test1 : 
timestamp=2015-12-09 15:53:28 value_len=0
 - add: private/vendor/vendor.dovecot/pvt/server/sieve/files/test2 : 
timestamp=2015-12-09 15:57:14 value_len=0

Apparently something gets messed up on the receiving side and strange 
things appear in the transaction log (e.g. test1 with a timestamp of 
15:57:14 which it never had). The fact that it happens sometimes (around 
30-40% of the time) likely indicates a race condition somewhere.  Also 
I'm not at all sure sieve transactions should appear twice on the 
receiving side (ext and then sync); I see other entries (e.g.  
ext-intro) as a single "(ext) (sync)" record.

Any ideas?

Thanks,
Apollon

doveconf -n output follows:

# 2.2.18: /etc/dovecot/dovecot.conf
# Pigeonhole version 0.4.8 (0c4ae064f307+)
# OS: Linux 3.16.0-4-amd64 x86_64 Debian 8.2 
auth_mechanisms = plain login
default_vsz_limit = 320 M
mail_location = mdbox:~/mdbox:ALT=/srv/mail/alt/%d/%n
mail_plugins = " acl notify replication acl"
managesieve_notify_capability = mailto
managesieve_sieve_capability = fileinto reject envelope encoded-character 
vacation subaddress comparator-i;ascii-numeric relational regex imap4flags copy 
include variables body enotify environment mailbox date index ihave duplicate
namespace {
  location = 
maildir:/home/vmail/example.com/staging/Maildir:INDEX=/home/vmail/%d/%n/stag...@example.com
  prefix = Staging.
  subscriptions