From 37ebe331178e2b7d225a31f64463aef5448d4970 Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Fri, 8 Nov 2019 05:27:36 +0100 Subject: libinterimap: honor compress={Yes/No}. --- lib/Net/IMAP/InterIMAP.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Net/IMAP') diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index bb27009..3d5bdcf 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -61,7 +61,7 @@ my %OPTIONS = ( auth => qr/\A($RE_ATOM_CHAR+(?: $RE_ATOM_CHAR+)*)\z/, command => qr/\A(\P{Control}+)\z/, 'null-stderr' => qr/\A(YES|NO)\z/i, - compress => qr/\A($RE_ATOM_CHAR+(?: $RE_ATOM_CHAR+)*)\z/, + compress => qr/\A(YES|NO)\z/i, SSL_protocols => qr/\A(!?$RE_SSL_PROTO(?: !?$RE_SSL_PROTO)*)\z/, SSL_fingerprint => qr/\A((?:[A-Za-z0-9]+\$)?\p{AHex}+)\z/, SSL_cipherlist => qr/\A(\P{Control}+)\z/, -- cgit v1.2.3 From d08ee23dccf56af292a9616986e58cc39386e3fb Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Fri, 8 Nov 2019 06:53:19 +0100 Subject: Remove deprecated/buggy 'o' regexp modifier. --- interimap | 7 +++++-- lib/Net/IMAP/InterIMAP.pm | 11 +++++------ pullimap | 3 ++- 3 files changed, 12 insertions(+), 9 deletions(-) (limited to 'lib/Net/IMAP') diff --git a/interimap b/interimap index 87c3a64..f170ded 100755 --- a/interimap +++ b/interimap @@ -636,6 +636,10 @@ sub sync_mailbox_list() { state $sth_subscribe = $DBH->prepare(q{ UPDATE mailboxes SET subscribed = ? WHERE idx = ? }); + state $ignore_mailbox = do { + my $re = $CONF->{_}->{"ignore-mailbox"}; + defined $re ? qr/$re/ : undef + }; foreach my $name (qw/local remote/) { foreach my $mbx (keys %{$IMAP->{$name}->{mailboxes}}) { @@ -645,8 +649,7 @@ sub sync_mailbox_list() { # exclude ignored mailboxes (taken from the default config as it doesn't # make sense to ignore mailboxes from one side but not the other - next if !@ARGV and defined $CONF->{_}->{"ignore-mailbox"} - and $mbx =~ /$CONF->{_}->{"ignore-mailbox"}/o; + next if !@ARGV and defined $ignore_mailbox and $mbx =~ $ignore_mailbox; $mailboxes{$mbx} = 1; } } diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index 3d5bdcf..77c1b14 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -1408,10 +1408,10 @@ sub _ssl_error($$@) { # RFC 3986 appendix A my $RE_IPv4 = do { my $dec = qr/[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]/; - qr/$dec(?:\.$dec){3}/o }; + qr/$dec(?:\.$dec){3}/ }; my $RE_IPv6 = do { my $h16 = qr/[0-9A-Fa-f]{1,4}/; - my $ls32 = qr/$h16:$h16|$RE_IPv4/o; + my $ls32 = qr/$h16:$h16|$RE_IPv4/; qr/ (?: $h16 : ){6} $ls32 | :: (?: $h16 : ){5} $ls32 | (?: $h16 )? :: (?: $h16 : ){4} $ls32 @@ -1421,7 +1421,7 @@ my $RE_IPv6 = do { | (?: (?: $h16 : ){0,4} $h16 )? :: $ls32 | (?: (?: $h16 : ){0,5} $h16 )? :: $h16 | (?: (?: $h16 : ){0,6} $h16 )? :: - /xo }; + /x }; # Opens a TCP socket to the given $host and $port. @@ -1429,11 +1429,10 @@ sub _tcp_connect($$$) { my ($self, $host, $port) = @_; my %hints = (socktype => SOCK_STREAM, protocol => IPPROTO_TCP); - if ($host =~ qr/\A$RE_IPv4\z/o) { + if ($host =~ qr/\A$RE_IPv4\z/) { $hints{family} = AF_INET; $hints{flags} |= AI_NUMERICHOST; - } - elsif ($host =~ qr/\A\[($RE_IPv6)\]\z/o) { + } elsif ($host =~ qr/\A\[($RE_IPv6)\]\z/) { $host = $1; $hints{family} = AF_INET6; $hints{flags} |= AI_NUMERICHOST; diff --git a/pullimap b/pullimap index dcbe59b..f983fe1 100755 --- a/pullimap +++ b/pullimap @@ -286,6 +286,7 @@ my $ATTRS = "ENVELOPE INTERNALDATE"; $ATTRS .= " BODY.PEEK[]" unless $CONFIG{'no-delivery'}; my $RE_ATOM = qr/[\x21\x23-\x27\x2A\x2B\x2D\x2F-\x39\x3D\x3F\x41-\x5A\x5E-\x7E]+/; +my $DOT_STRING = qr/\A$RE_ATOM(?:\.$RE_ATOM)*\z/; sub pull_callback($$) { my ($uids, $mail) = @_; return unless exists $mail->{RFC822} or $CONFIG{'no-delivery'}; # not for us @@ -294,7 +295,7 @@ sub pull_callback($$) { my $e = $mail->{ENVELOPE}->[3]; my $sender = ''; if (defined $e and defined (my $l = $e->[0]->[2]) and defined (my $d = $e->[0]->[3])) { - if ($l =~ /\A$RE_ATOM(?:\.$RE_ATOM)*\z/o) { + if ($l =~ $DOT_STRING) { $sender = $l.'@'.$d; } elsif ($l =~ /\A[\x20-\x7E]*\z/) { # quote the local part if not Dot-string (RFC 5321) -- cgit v1.2.3 From a7c364bf90a4593cfbc7911b1b7536dc66b1c879 Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Sun, 10 Nov 2019 05:39:41 +0100 Subject: Test suite: add new tests for SSL/TLS. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSL connections are accepted on TCP port 10993. Also, fix STARTTLS directive, broken since fba1c36… --- Changelog | 1 + lib/Net/IMAP/InterIMAP.pm | 11 ++-- tests/list | 9 +++ tests/snippets/dovecot/dhparams.pem | 8 +++ tests/snippets/dovecot/dovecot.key | 5 ++ tests/snippets/dovecot/dovecot.pem | 11 ++++ tests/snippets/dovecot/imapd.conf | 4 ++ tests/snippets/dovecot/ssl.conf | 4 ++ tests/starttls-logindisabled/interimap.remote | 4 ++ tests/starttls-logindisabled/remote.conf | 5 ++ tests/starttls-logindisabled/t | 19 +++++++ tests/starttls/interimap.remote | 4 ++ tests/starttls/remote.conf | 2 + tests/starttls/t | 27 +++++++++ tests/sync-live-tls/interimap.remote | 1 + tests/sync-live-tls/local.conf | 1 + tests/sync-live-tls/remote.conf | 6 ++ tests/sync-live-tls/t | 1 + tests/tls-pin-fingerprint/interimap.remote | 1 + tests/tls-pin-fingerprint/remote.conf | 1 + tests/tls-pin-fingerprint/t | 33 +++++++++++ tests/tls-protocols/interimap.remote | 1 + tests/tls-protocols/remote.conf | 1 + tests/tls-protocols/t | 39 +++++++++++++ tests/tls-verify-peer/interimap.remote | 2 + tests/tls-verify-peer/remote.conf | 1 + tests/tls-verify-peer/t | 80 +++++++++++++++++++++++++++ tests/tls/interimap.remote | 3 + tests/tls/remote.conf | 2 + tests/tls/t | 14 +++++ 30 files changed, 296 insertions(+), 5 deletions(-) create mode 100644 tests/snippets/dovecot/dhparams.pem create mode 100644 tests/snippets/dovecot/dovecot.key create mode 100644 tests/snippets/dovecot/dovecot.pem create mode 100644 tests/snippets/dovecot/ssl.conf create mode 100644 tests/starttls-logindisabled/interimap.remote create mode 100644 tests/starttls-logindisabled/remote.conf create mode 100644 tests/starttls-logindisabled/t create mode 100644 tests/starttls/interimap.remote create mode 100644 tests/starttls/remote.conf create mode 100644 tests/starttls/t create mode 120000 tests/sync-live-tls/interimap.remote create mode 120000 tests/sync-live-tls/local.conf create mode 100644 tests/sync-live-tls/remote.conf create mode 120000 tests/sync-live-tls/t create mode 120000 tests/tls-pin-fingerprint/interimap.remote create mode 120000 tests/tls-pin-fingerprint/remote.conf create mode 100644 tests/tls-pin-fingerprint/t create mode 120000 tests/tls-protocols/interimap.remote create mode 120000 tests/tls-protocols/remote.conf create mode 100644 tests/tls-protocols/t create mode 100644 tests/tls-verify-peer/interimap.remote create mode 120000 tests/tls-verify-peer/remote.conf create mode 100644 tests/tls-verify-peer/t create mode 100644 tests/tls/interimap.remote create mode 100644 tests/tls/remote.conf create mode 100644 tests/tls/t (limited to 'lib/Net/IMAP') diff --git a/Changelog b/Changelog index 5b43c6f..c08de80 100644 --- a/Changelog +++ b/Changelog @@ -85,6 +85,7 @@ interimap (0.5) upstream; - libinterimap: don't panic() when inflate() reports the end of the compression stream is reached. - libinterimap: the 'compress' boolean wasn't honored. + - libinterimap: fix STARTTLS directive, broken since 0.2. -- Guilhem Moulin Fri, 10 May 2019 00:58:14 +0200 diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index 77c1b14..b4d8bec 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -215,7 +215,7 @@ sub in_set($$) { return 1 if $x == $1; } elsif ($r eq '*' or $r eq '*:*') { - warn "Assuming $x belongs to set $set! (Dunno what \"*\" means.)"; + warn "Assuming $x belongs to set $set! (Dunno what \"*\" means.)"; return 1; } elsif ($r =~ /\A([0-9]+):\*\z/ or $r =~ /\A\*:([0-9]+)\z/) { @@ -398,7 +398,8 @@ sub new($%) { if ($self->{type} eq 'imap' and $self->{STARTTLS}) { # RFC 2595 section 5.1 $self->fail("Server did not advertise STARTTLS capability.") unless grep {$_ eq 'STARTTLS'} @caps; - $self->_start_ssl($self->{S}) if $self->{type} eq 'imaps'; + $self->_send('STARTTLS'); + $self->_start_ssl($self->{S}); # refresh the previous CAPABILITY list since the previous one could have been spoofed delete $self->{_CAPABILITIES}; @@ -1610,7 +1611,7 @@ sub _ssl_verify($$$) { my $pkey = Net::SSLeay::X509_get_X509_PUBKEY($cert); unless (defined $pkey and Net::SSLeay::EVP_Digest($pkey, $type) eq $digest) { - $self->warn("Fingerprint doesn't match! MiTM in action?"); + $self->warn("Fingerprint doesn't match! MiTM in action?"); $ok = 0; } } @@ -2355,7 +2356,7 @@ sub _resp($$;&$$) { # /!\ No bookkeeping since there is no internal cache mapping sequence numbers to UIDs if ($self->_enabled('QRESYNC')) { $self->panic("$1 <= $cache->{EXISTS}") if $1 <= $cache->{EXISTS}; # sanity check - $self->fail("RFC 7162 violation! Got an EXPUNGE response with QRESYNC enabled."); + $self->fail("RFC 7162 violation! Got an EXPUNGE response with QRESYNC enabled."); } # the new message was expunged before it was synced $self->{_NEW} = 0 if $self->{_NEW} == 1 and $cache->{EXISTS} == $1; @@ -2406,7 +2407,7 @@ sub _resp($$;&$$) { /\A \((\\?$RE_ATOM_CHAR+ [0-9]+(?: \\?$RE_ATOM_CHAR+ [0-9]+)*)?\)\z/ or $self->panic($_); my %status = split / /, $1; $mailbox = 'INBOX' if uc $mailbox eq 'INBOX'; # INBOX is case-insensitive - $self->panic("RFC 5465 violation! Missing HIGHESTMODSEQ data item in STATUS response") + $self->panic("RFC 5465 violation! Missing HIGHESTMODSEQ data item in STATUS response") if $self->_enabled('QRESYNC') and !defined $status{HIGHESTMODSEQ} and defined $cmd and ($cmd eq 'NOTIFY' or $cmd eq 'slurp'); $self->_update_cache_for($mailbox, %status); diff --git a/tests/list b/tests/list index 86034ef..bc8d144 100644 --- a/tests/list +++ b/tests/list @@ -38,7 +38,16 @@ repair --repair auth-logindisabled LOGINDISABLED auth-noplaintext abort when STARTTLS is not offered +. SSL/TLS + starttls-logindisabled LOGINDISABLED STARTTLS + starttls STARTTLS + tls SSL/TLS handshake + ... tls-verify-peer + tls-pin-fingerprint pubkey fingerprint pinning + tls-protocols force TLS protocol versions + . Live synchronization (60s) sync-live local/remote simulation sync-live-crippled local/remote simulation (crippled remote) + sync-live-tls local/remote simulation (TLS remote) sync-live-multi local/remote1+remote2+remote3 simulation (3 local namespaces) diff --git a/tests/snippets/dovecot/dhparams.pem b/tests/snippets/dovecot/dhparams.pem new file mode 100644 index 0000000..7734d2a --- /dev/null +++ b/tests/snippets/dovecot/dhparams.pem @@ -0,0 +1,8 @@ +-----BEGIN DH PARAMETERS----- +MIIBCAKCAQEA0J1dU8erRgIk4bMCBMLezjx32pcQpXrdNgl04dxZVxnJ5Ik2gGhA +uQRbbZhAlHNHtFtp9s4TdQ3Ddrv9SuWXYul8U5BWbcxs4nOtwFU8912SfiuVr/kc +4ok2zQ1hdMODtaqWS2ZKBmwcuk4QM6e7fMEAkuZX+Dtf2u8bG5G9B7OL5LggYtrP +cFVNQDtfhs64D+sUKJLWkgeg5NH6nbf+0Gs5a8v3/urHKvoxdVScGmKzF+LsFsBm +ycQjYeVtA9gLr41mo80rrFysUQqZtNkbdkaXOIA2r9JGTYex1l/XaediR8J94ck9 +dwAe2ubRqWcPjmoLJYQIPKiCbvXuJAd0wwIBAg== +-----END DH PARAMETERS----- diff --git a/tests/snippets/dovecot/dovecot.key b/tests/snippets/dovecot/dovecot.key new file mode 100644 index 0000000..95c9846 --- /dev/null +++ b/tests/snippets/dovecot/dovecot.key @@ -0,0 +1,5 @@ +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIGkqkKq69zVeF17S3y2U2HkQWh8z9M/xeblCztkKIfzJoAoGCCqGSM49 +AwEHoUQDQgAE1LLppulKw8KjINrDhOjEd0NTax5iDCds+vpA2PwsvvtGoprNAjQM +zX+40u30N3CE0r591txqohSBQ/X+nvG2ug== +-----END EC PRIVATE KEY----- diff --git a/tests/snippets/dovecot/dovecot.pem b/tests/snippets/dovecot/dovecot.pem new file mode 100644 index 0000000..7e53d90 --- /dev/null +++ b/tests/snippets/dovecot/dovecot.pem @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBkzCCATmgAwIBAgIUQ+3hBMsPJcl59xDDujDDfexurOswCgYIKoZIzj0EAwIw +HzEdMBsGA1UEAwwUSW50ZXJJTUFQIHRlc3Qgc3VpdGUwHhcNMTkxMTEwMTM1NDAw +WhcNMjkxMTA3MTM1NDAwWjAfMR0wGwYDVQQDDBRJbnRlcklNQVAgdGVzdCBzdWl0 +ZTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABNSy6abpSsPCoyDaw4ToxHdDU2se +YgwnbPr6QNj8LL77RqKazQI0DM1/uNLt9DdwhNK+fdbcaqIUgUP1/p7xtrqjUzBR +MB0GA1UdDgQWBBRlh8nSwyX+VlhwuhV7RKYwvKLyDzAfBgNVHSMEGDAWgBRlh8nS +wyX+VlhwuhV7RKYwvKLyDzAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49BAMCA0gA +MEUCIQDK8xPPHTbYW5JnZ1Siy8ChZ6GOu2sRwQu7OgtGYGZRSQIgFKn1oAhnq2Oi +aIPqxjvBPMsK/sjrdI/rNsr2XgaulU4= +-----END CERTIFICATE----- diff --git a/tests/snippets/dovecot/imapd.conf b/tests/snippets/dovecot/imapd.conf index c9926ce..18c60f8 100644 --- a/tests/snippets/dovecot/imapd.conf +++ b/tests/snippets/dovecot/imapd.conf @@ -9,4 +9,8 @@ service imap-login { inet_listener imap { port = 10143 } + inet_listener imaps { + port = 10993 + ssl = yes + } } diff --git a/tests/snippets/dovecot/ssl.conf b/tests/snippets/dovecot/ssl.conf new file mode 100644 index 0000000..240f24b --- /dev/null +++ b/tests/snippets/dovecot/ssl.conf @@ -0,0 +1,4 @@ +ssl = required +ssl_cert = "$TMPDIR/capability" + +sed -ri 's/^remote: S: \* OK \[CAPABILITY (.*)\]$/\1/' "$TMPDIR/capability" +tr " " "\\n" <"$TMPDIR/capability" >"$TMPDIR/capabilities" +grep -Fx "IMAP4rev1" <"$TMPDIR/capabilities" || error +grep -Fx "LOGINDISABLED" <"$TMPDIR/capabilities" || error + +# make sure we upgraded the connection and check the capability again +grep -Fx "STARTTLS" <"$TMPDIR/capabilities" || error +grep -Fx "remote: C: 000000 STARTTLS" <"$STDERR" || error +grep -Fx "remote: C: 000001 CAPABILITY" <"$STDERR" || error + +# can't go further as the capability string still has the manually +# enforced 'LOGINDISABLED' + +# vim: set filetype=sh : diff --git a/tests/starttls/interimap.remote b/tests/starttls/interimap.remote new file mode 100644 index 0000000..5d7571d --- /dev/null +++ b/tests/starttls/interimap.remote @@ -0,0 +1,4 @@ +type = imap +host = 127.0.0.1 +port = 10143 +SSL_verify = no diff --git a/tests/starttls/remote.conf b/tests/starttls/remote.conf new file mode 100644 index 0000000..3d07ea9 --- /dev/null +++ b/tests/starttls/remote.conf @@ -0,0 +1,2 @@ +!include conf.d/imapd.conf +!include conf.d/ssl.conf diff --git a/tests/starttls/t b/tests/starttls/t new file mode 100644 index 0000000..99a39c2 --- /dev/null +++ b/tests/starttls/t @@ -0,0 +1,27 @@ +for ((i = 0; i < 32; i++)); do + u="$(shuf -n1 -e "local" "remote")" + sample_message | deliver -u "$u" +done + +interimap --debug || error + +# double check the presence of 'STARTTLS' in the preauth capability list +grep -oE -m1 '^remote: S: \* OK \[CAPABILITY IMAP4rev1( [^]]*)? AUTH=[^]]*\]' <"$STDERR" >"$TMPDIR/capability" + +sed -ri 's/^remote: S: \* OK \[CAPABILITY (.*)\]$/\1/' "$TMPDIR/capability" +tr " " "\\n" <"$TMPDIR/capability" >"$TMPDIR/capabilities" +grep -Fx "IMAP4rev1" <"$TMPDIR/capabilities" || error +grep -Fx "STARTTLS" <"$TMPDIR/capabilities" || error + +# make sure we upgraded the connection and check the capability again +grep -Fx "remote: C: 000000 STARTTLS" <"$STDERR" || error +grep -Fx "remote: C: 000001 CAPABILITY" <"$STDERR" || error + +grep -Fx "remote: Disabling SSL protocols: SSLv3, TLSv1, TLSv1.1" <"$STDERR" || error +grep -Fx "remote: Peer certificate fingerprint: sha256\$35944e3bd3300d3ac310bb497a32cc1eef6931482a587ddbc95343740cdf1323" <"$STDERR" || error +grep "^remote: SSL protocol: TLSv1\.[23] " <"$STDERR" || error +grep "^remote: SSL cipher: " <"$STDERR" || error + +check_mailbox_status "INBOX" + +# vim: set filetype=sh : diff --git a/tests/sync-live-tls/interimap.remote b/tests/sync-live-tls/interimap.remote new file mode 120000 index 0000000..daf3741 --- /dev/null +++ b/tests/sync-live-tls/interimap.remote @@ -0,0 +1 @@ +../tls/interimap.remote \ No newline at end of file diff --git a/tests/sync-live-tls/local.conf b/tests/sync-live-tls/local.conf new file mode 120000 index 0000000..ad27dd1 --- /dev/null +++ b/tests/sync-live-tls/local.conf @@ -0,0 +1 @@ +../sync-live/local.conf \ No newline at end of file diff --git a/tests/sync-live-tls/remote.conf b/tests/sync-live-tls/remote.conf new file mode 100644 index 0000000..b2af2d2 --- /dev/null +++ b/tests/sync-live-tls/remote.conf @@ -0,0 +1,6 @@ +namespace inbox { + separator = ^ +} + +!include conf.d/imapd.conf +!include conf.d/ssl.conf diff --git a/tests/sync-live-tls/t b/tests/sync-live-tls/t new file mode 120000 index 0000000..189360e --- /dev/null +++ b/tests/sync-live-tls/t @@ -0,0 +1 @@ +../sync-live/t \ No newline at end of file diff --git a/tests/tls-pin-fingerprint/interimap.remote b/tests/tls-pin-fingerprint/interimap.remote new file mode 120000 index 0000000..daf3741 --- /dev/null +++ b/tests/tls-pin-fingerprint/interimap.remote @@ -0,0 +1 @@ +../tls/interimap.remote \ No newline at end of file diff --git a/tests/tls-pin-fingerprint/remote.conf b/tests/tls-pin-fingerprint/remote.conf new file mode 120000 index 0000000..6029749 --- /dev/null +++ b/tests/tls-pin-fingerprint/remote.conf @@ -0,0 +1 @@ +../tls/remote.conf \ No newline at end of file diff --git a/tests/tls-pin-fingerprint/t b/tests/tls-pin-fingerprint/t new file mode 100644 index 0000000..1b84390 --- /dev/null +++ b/tests/tls-pin-fingerprint/t @@ -0,0 +1,33 @@ +# backup config +install -m0600 "$XDG_CONFIG_HOME/interimap/config" "$XDG_CONFIG_HOME/interimap/config~" +with_remote_config() { + install -m0600 "$XDG_CONFIG_HOME/interimap/config~" "$XDG_CONFIG_HOME/interimap/config" + cat >>"$XDG_CONFIG_HOME/interimap/config" +} + +# pinned valid fingerprint +with_remote_config <<-EOF + SSL_fingerprint = sha256\$e8fc8d03ffe75e03897136a2f1c5647bf8c36be7136a6883a732a8c4961c1614 +EOF + +for ((i = 0; i < 32; i++)); do + u="$(shuf -n1 -e "local" "remote")" + sample_message | deliver -u "$u" +done +interimap_init +check_mailbox_status "INBOX" + + +# and now an invalid one +with_remote_config <<-EOF + SSL_fingerprint = sha256\$deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef +EOF +! interimap --debug || error + +grep -Fx "remote: ERROR: Can't initiate TLS/SSL handshake" <"$STDERR" || error +grep -Fx "remote: WARNING: Fingerprint doesn't match! MiTM in action?" <"$STDERR" || error +grep -Fx "remote: ERROR: Can't initiate TLS/SSL handshake" <"$STDERR" || error +# make sure we didn't send any credentials +! grep -E "^remote: C: .* (AUTHENTICATE|LOGIN) " <"$STDERR" || error + +# vim: set filetype=sh : diff --git a/tests/tls-protocols/interimap.remote b/tests/tls-protocols/interimap.remote new file mode 120000 index 0000000..daf3741 --- /dev/null +++ b/tests/tls-protocols/interimap.remote @@ -0,0 +1 @@ +../tls/interimap.remote \ No newline at end of file diff --git a/tests/tls-protocols/remote.conf b/tests/tls-protocols/remote.conf new file mode 120000 index 0000000..6029749 --- /dev/null +++ b/tests/tls-protocols/remote.conf @@ -0,0 +1 @@ +../tls/remote.conf \ No newline at end of file diff --git a/tests/tls-protocols/t b/tests/tls-protocols/t new file mode 100644 index 0000000..f34a95b --- /dev/null +++ b/tests/tls-protocols/t @@ -0,0 +1,39 @@ +# backup config +install -m0600 "$XDG_CONFIG_HOME/interimap/config" "$XDG_CONFIG_HOME/interimap/config~" +with_remote_tls_protocols() { + install -m0600 "$XDG_CONFIG_HOME/interimap/config~" "$XDG_CONFIG_HOME/interimap/config" + printf "SSL_protocols = %s\\n" "$*" >>"$XDG_CONFIG_HOME/interimap/config" +} + +# default +interimap --debug || error +grep -Fx "remote: Disabling SSL protocols: SSLv3, TLSv1, TLSv1.1" <"$STDERR" || error +grep -E "^remote: SSL protocol: TLSv1\.[23] " <"$STDERR" || error + +# also disable TLSv1.2 +with_remote_tls_protocols "!SSLv2" "!SSLv3" "!TLSv1" "!TLSv1.1" "!TLSv1.2" +interimap --debug || error +grep -Fx "remote: Disabling SSL protocols: SSLv3, TLSv1, TLSv1.1, TLSv1.2" <"$STDERR" || error +grep -E "^remote: SSL protocol: TLSv1\.3 " <"$STDERR" || error + +# force TLSv1.2 +with_remote_tls_protocols "TLSv1.2" +interimap --debug || error +grep -Fx "remote: Disabling SSL protocols: SSLv3, TLSv1, TLSv1.1, TLSv1.3" <"$STDERR" || error +grep -E "^remote: SSL protocol: TLSv1\.2 " <"$STDERR" || error + +# force TLSv1 to TLSv1.2 +with_remote_tls_protocols "TLSv1" "TLSv1.1" "TLSv1.2" +interimap --debug || error +grep -Fx "remote: Disabling SSL protocols: SSLv3, TLSv1.3" <"$STDERR" || error +grep -E "^remote: SSL protocol: TLSv(1\.[12])? " <"$STDERR" || error + +# force SSLv2 and SSLv3, fails as it's disabled server side +with_remote_tls_protocols "SSLv2" "SSLv3" +! interimap --debug || error +grep -Fx "remote: Disabling SSL protocols: TLSv1, TLSv1.1, TLSv1.2, TLSv1.3" <"$STDERR" || error +grep -Fx "remote: ERROR: Can't initiate TLS/SSL handshake" <"$STDERR" || error +# make sure we didn't send any credentials +! grep -E "^remote: C: .* (AUTHENTICATE|LOGIN) " <"$STDERR" || error + +# vim: set filetype=sh : diff --git a/tests/tls-verify-peer/interimap.remote b/tests/tls-verify-peer/interimap.remote new file mode 100644 index 0000000..b02fcd0 --- /dev/null +++ b/tests/tls-verify-peer/interimap.remote @@ -0,0 +1,2 @@ +host = ::1 +port = 10993 diff --git a/tests/tls-verify-peer/remote.conf b/tests/tls-verify-peer/remote.conf new file mode 120000 index 0000000..6029749 --- /dev/null +++ b/tests/tls-verify-peer/remote.conf @@ -0,0 +1 @@ +../tls/remote.conf \ No newline at end of file diff --git a/tests/tls-verify-peer/t b/tests/tls-verify-peer/t new file mode 100644 index 0000000..d84328a --- /dev/null +++ b/tests/tls-verify-peer/t @@ -0,0 +1,80 @@ +CERT=~/.dovecot/conf.d/dovecot.pem + +unverified_peer() { + ! interimap --debug || error + + grep -Fx "remote: ERROR: Can't initiate TLS/SSL handshake" <"$STDERR" || error + sed -nr "s/remote: \[[0-9]+\] (preverify=[0-9]+)$/\1/p" <"$STDERR" >"$TMPDIR/preverify" + [ -s "$TMPDIR/preverify" ] || error + ! grep -Fvx "preverify=0" <"$TMPDIR/preverify" || error + + # make sure we didn't send any credentials + ! grep -E "^remote: C: .* (AUTHENTICATE|LOGIN) " <"$STDERR" || error +} +verified_peer() { + local i u + for ((i = 0; i < 32; i++)); do + u="$(shuf -n1 -e "local" "remote")" + sample_message | deliver -u "$u" + done + interimap --debug || error + + sed -nr "s/remote: \[[0-9]+\] (preverify=[0-9]+)$/\1/p" <"$STDERR" >"$TMPDIR/preverify" + [ -s "$TMPDIR/preverify" ] || error + ! grep -Fvx "preverify=1" <"$TMPDIR/preverify" || error + + grep "^remote: SSL protocol: TLSv1\.[23] " <"$STDERR" || error + grep "^remote: SSL cipher: " <"$STDERR" || error + + check_mailbox_status "INBOX" +} + +# backup config +install -m0600 "$XDG_CONFIG_HOME/interimap/config" "$XDG_CONFIG_HOME/interimap/config~" +with_remote_config() { + install -m0600 "$XDG_CONFIG_HOME/interimap/config~" "$XDG_CONFIG_HOME/interimap/config" + cat >>"$XDG_CONFIG_HOME/interimap/config" +} + +step_start "peer verification enabled by default" +unverified_peer +step_done + +step_start "peer verification result honored when pinned pubkey matches" +pkey_sha256="$(openssl x509 -pubkey <"$CERT" | openssl pkey -pubin -outform DER \ + | openssl dgst -sha256 | sed -rn "/^.*=\\s*/ {s///p;q}")" +with_remote_config <<-EOF + SSL_fingerprint = sha256\$$pkey_sha256 +EOF +unverified_peer +! grep -Fx "remote: WARNING: Fingerprint doesn't match! MiTM in action?" <"$STDERR" || error +step_done + + +step_start "SSL_CAfile" +if [ -f "/etc/ssl/certs/ca-certificates.crt" ]; then + # the self-signed cert should not be in there + with_remote_config <<<"SSL_CAfile = /etc/ssl/certs/ca-certificates.crt" + unverified_peer +fi +with_remote_config <<<"SSL_CAfile = $CERT" +verified_peer +step_done + + +step_start "SSL_CApath" +if [ -d "/etc/ssl/certs" ]; then + # the self-signed cert should not be in there + with_remote_config <<<"SSL_CApath = /etc/ssl/certs" + unverified_peer +fi + +capath=$(mktemp --tmpdir="$TMPDIR" --directory capath.XXXXXX) +cp -t"$capath" "$CERT" +c_rehash "$capath" + +with_remote_config <<<"SSL_CApath = $capath" +verified_peer +step_done + +# vim: set filetype=sh : diff --git a/tests/tls/interimap.remote b/tests/tls/interimap.remote new file mode 100644 index 0000000..2c0e37e --- /dev/null +++ b/tests/tls/interimap.remote @@ -0,0 +1,3 @@ +host = ::1 +port = 10993 +SSL_verify = no diff --git a/tests/tls/remote.conf b/tests/tls/remote.conf new file mode 100644 index 0000000..3d07ea9 --- /dev/null +++ b/tests/tls/remote.conf @@ -0,0 +1,2 @@ +!include conf.d/imapd.conf +!include conf.d/ssl.conf diff --git a/tests/tls/t b/tests/tls/t new file mode 100644 index 0000000..dd6d955 --- /dev/null +++ b/tests/tls/t @@ -0,0 +1,14 @@ +for ((i = 0; i < 32; i++)); do + u="$(shuf -n1 -e "local" "remote")" + sample_message | deliver -u "$u" +done + +interimap --debug || error +grep -Fx "remote: Disabling SSL protocols: SSLv3, TLSv1, TLSv1.1" <"$STDERR" || error +grep -Fx "remote: Peer certificate fingerprint: sha256\$35944e3bd3300d3ac310bb497a32cc1eef6931482a587ddbc95343740cdf1323" <"$STDERR" || error +grep "^remote: SSL protocol: TLSv1\.[23] " <"$STDERR" || error +grep "^remote: SSL cipher: " <"$STDERR" || error + +check_mailbox_status "INBOX" + +# vim: set filetype=sh : -- cgit v1.2.3 From 23046d58204e636880ff4412e52799e0c06065b4 Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Mon, 11 Nov 2019 00:20:26 +0100 Subject: Bump minimum Perl for Net::IMAP::InterIMAP to v5.20. We're using s///r which was introduced in 5.14, and hash slices which were introduced in 5.20. --- lib/Net/IMAP/InterIMAP.pm | 2 +- pullimap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/Net/IMAP') diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index b4d8bec..c25df27 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -17,7 +17,7 @@ #---------------------------------------------------------------------- package Net::IMAP::InterIMAP v0.0.5; -use v5.10.0; +use v5.20.0; use warnings; use strict; diff --git a/pullimap b/pullimap index f983fe1..3d1a0ec 100755 --- a/pullimap +++ b/pullimap @@ -18,10 +18,10 @@ # along with this program. If not, see . #---------------------------------------------------------------------- +use v5.20.2; use strict; use warnings; -use v5.20.2; our $VERSION = '0.4'; my $NAME = 'pullimap'; -- cgit v1.2.3 From 3aa5593af18bd4925235d1820fd0fe7c646843aa Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Tue, 12 Nov 2019 01:39:29 +0100 Subject: Net::IMAP::InterIMAP::push_flag_updates() bugfixes. The UNCHANGEDSINCE test from the CONDSTORE extension was incorrectly placed after the flag list in UID STORE commands. In practice this meant the server didn't add the MODIFIED code when needed. The server won't send an untagged FETCH command (and won't increase the message's MODSEQ) if no change was made to the flag list. A panic() was incorrectly triggered in that case. When the flag list was set (by another client) to a superset of the UID STORE command currently processed, the extra flags were not synchronized. Cf. RFC 7162 sec. 3.1.3 ex. 10. --- Changelog | 7 ++++++ lib/Net/IMAP/InterIMAP.pm | 58 ++++++++++++++++++++--------------------------- tests/condstore/t | 50 ++++++++++++++++++++++++++++++++++++++++ tests/list | 1 + 4 files changed, 82 insertions(+), 34 deletions(-) create mode 100644 tests/condstore/t (limited to 'lib/Net/IMAP') diff --git a/Changelog b/Changelog index f90c28c..763a7a1 100644 --- a/Changelog +++ b/Changelog @@ -87,6 +87,13 @@ interimap (0.5) upstream; compression stream is reached. - libinterimap: the 'compress' boolean wasn't honored. - libinterimap: fix STARTTLS directive, broken since 0.2. + - libinterimap: push_flag_updates(): the UNCHANGEDSINCE test from + the CONDSTORE extension was incorrectly placed after the flag list in + UID STORE commands. + - libinterimap: push_flag_updates(): ignore UIDs for which no untagged + FETCH response was received. + - libinterimap: push_flag_updates(): don't ignores received updates (by + another client) to a superset of the desigred flag list. -- Guilhem Moulin Fri, 10 May 2019 00:58:14 +0200 diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index c25df27..a838dd0 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -1197,16 +1197,15 @@ sub pull_updates($;$) { my $mailbox = $self->{_SELECTED} // $self->panic(); my $pcache = $self->{_PCACHE}->{$mailbox}; - my %modified; $self->_send("UID FETCH 1:".($pcache->{UIDNEXT}-1)." (MODSEQ FLAGS)") if $full and ($pcache->{UIDNEXT} // 1) > 1; - my @missing; + my %modified; while (%{$self->{_MODIFIED}}) { + my @missing; while (my ($uid,$v) = each %{$self->{_MODIFIED}}) { - # don't filter on the fly (during FETCH responses) because - # FLAG updates can arrive while processing pull_new_messages - # for instance + # don't filter on the fly (during FETCH responses) because FLAG updates + # can arrive while processing pull_new_messages() for instance if (defined $v->[1] and $v->[0] > 0) { # setting the MODSEQ to 0 forces a FETCH next unless $uid < ($pcache->{UIDNEXT} // 1) # out of bounds and ($full or $v->[0] > ($pcache->{HIGHESTMODSEQ} // 0)); # already seen @@ -1216,8 +1215,9 @@ sub pull_updates($;$) { } } $self->{_MODIFIED} = {}; + # non-empty @missing indicates a discouraged (but allowed) CONDSTORE server behavior, + # cf. RFC 7162 sec. 3.1.3 ex. 8 and the comment in push_flag_updates() below $self->_send("UID FETCH ".compact_set(@missing)." (MODSEQ FLAGS)") if @missing; - @missing = (); } # do that afterwards since the UID FETCH command above can produce VANISHED responses @@ -1307,22 +1307,18 @@ sub push_flag_updates($$@) { my $mailbox = $self->{_SELECTED} // $self->panic(); my $modseq = $self->{_PCACHE}->{$mailbox}->{HIGHESTMODSEQ} // $self->panic(); - my $command = "UID STORE ".compact_set(@set)." FLAGS.SILENT ($flags) (UNCHANGEDSINCE $modseq)"; - - my %listed; - $self->_send($command, sub($){ $listed{shift->{UID}}++; }); + my $command = "UID STORE ".compact_set(@set)." (UNCHANGEDSINCE $modseq) FLAGS.SILENT ($flags)"; my %failed; + $self->_send($command); if ($IMAP_text =~ /\A\Q$IMAP_cond\E \[MODIFIED ([0-9,:]+)\] $RE_TEXT_CHAR+\z/) { foreach (split /,/, $1) { if (/\A([0-9]+)\z/) { $failed{$1} = 1; - } - elsif (/\A([0-9]+):([0-9]+)\z/) { + } elsif (/\A([0-9]+):([0-9]+)\z/) { my ($min, $max) = $1 < $2 ? ($1,$2) : ($2,$1); $failed{$_} = 1 foreach ($min .. $max); - } - else { + } else { $self->panic($_); } } @@ -1330,34 +1326,28 @@ sub push_flag_updates($$@) { my @ok; foreach my $uid (@set) { + my $modified = $self->{_MODIFIED}; if ($failed{$uid}) { - # $uid was listed in the MODIFIED response code - $self->{_MODIFIED}->{$uid} //= [ 0, undef ]; # will be downloaded again in pull_updates - delete $self->{_MODIFIED}->{$uid} if - # got a FLAG update for $uid; ignore it if it's $flags - defined $self->{_MODIFIED}->{$uid}->[1] and - $self->{_MODIFIED}->{$uid}->[1] eq $flags; - } - else { - # $uid wasn't listed in the MODIFIED response code - next unless defined $self->{_MODIFIED}->{$uid}; # already stored - $self->panic() unless defined $listed{$uid} and $listed{$uid} > 0; # sanity check - if ($listed{$uid} == 1) { - # ignore succesful update - delete $self->{_MODIFIED}->{$uid}; - } - elsif ($self->{_MODIFIED}->{$uid}->[1] and $self->{_MODIFIED}->{$uid}->[1] eq $flags) { - # got multiple FETCH responses for $uid, the last one with $flags - delete $self->{_MODIFIED}->{$uid}; + # $uid was listed in the MODIFIED response code from RFC 7162; will FETCH + # again in pull_updates(); per RFC 7162 sec. 3.1.3 $modified->{$uid} might not + # be defined ("nice" servers send an untagged FETCH response, cf. example 10, + # but they might omit it - allowed but discouraged CONDSTORE server behavior - + # cf. example 8) + $modified->{$uid} //= [ 0, undef ]; + } elsif (defined (my $m = $modified->{$uid})) { + # received an untagged FETCH response, remove from the list of pending changes + # if the flag list was up to date (either implicitely or explicitely) + if (!defined $m->[1] or $m->[1] eq $flags) { + delete $modified->{$uid}; + push @ok, $uid; } - push @ok, $uid; } } unless ($self->{quiet}) { $self->log("Updated flags ($flags) for UID ".compact_set(@ok)) if @ok; $self->log("Couldn't update flags ($flags) for UID ".compact_set(keys %failed).', '. - "trying again later") if %failed; + "will try again later") if %failed; } return keys %failed; } diff --git a/tests/condstore/t b/tests/condstore/t new file mode 100644 index 0000000..d4da50f --- /dev/null +++ b/tests/condstore/t @@ -0,0 +1,50 @@ +TIMEOUT=60 +N=4096 + +# test CONDSTORE/QRESYNC (behavior) in UID STORE commands, in particular +# the UNCHANGEDSINCE test: populate, keep assiging keywords at random, +# and make sure interimap is able to reconciliate the changes + +# populate (with dummy messages to speed things up) only one server +# before initializing interimap, so UIDs concide with sequence numbers +# and are identical on both servers +for ((i = 0; i < N; i++)); do + deliver -u "local" <<< . +done + +interimap_init + +# assign a set of 16 tags; not more because in order to maximize the +# likelyhood of conflicts we want UID STORE commands to use large sets +declare -a FLAGS=(0 1 2 3 4 5 6 7 8 9 a b c d e f) + +# start a long-lived interimap process +interimap --watch=1 & PID=$! +trap "ptree_abort $PID" EXIT INT TERM + +timer=$(( $(date +%s) + TIMEOUT )) +while [ $(date +%s) -le $timer ]; do + a="$(shuf -n1 -e "add" "remove" "replace")" + u="$(shuf -n1 -e "local" "remote")" + f="$(shuf -n1 -e "${FLAGS[@]}")" + seqs="$(shuf -n$((N/8)) -i1-$N)" # trigger changes on 1/8 of all messages + doveadm -u "$u" flags "$a" "$f" mailbox "INBOX" "${seqs//$'\n'/,}" + sleep "0.0$(shuf -n1 -i10-99)" # 10 to 99ms +done +sleep 2 + +ptree_abort $PID +trap - EXIT INT TERM + +# make sure the list of uids for a given tag match +flagged_uids() { + local u="$1" f="$2" + doveadm -u "$u" search mailbox "INBOX" keyword "$f" | cut -d" " -f2 | sort -n +} +for f in "${FLAGS[@]}"; do + diff --label="local/$f" --label="remote/$f" -u -- \ + <(flagged_uids "local" "$f") <(flagged_uids "remote" "$f") || + error "UID list differs for keyword '$f'" +done + +# vim: set filetype=sh : diff --git a/tests/list b/tests/list index 9ce0c06..8bb4478 100644 --- a/tests/list +++ b/tests/list @@ -39,6 +39,7 @@ repair --repair auth-noplaintext abort when STARTTLS is not offered compress COMPRESS=DEFLATE +condstore CONDSTORE . SSL/TLS starttls-logindisabled LOGINDISABLED STARTTLS -- cgit v1.2.3 From ccf90182d04c064bd9327c5e7067ed4b9dc32f41 Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Tue, 12 Nov 2019 05:04:45 +0100 Subject: Net::IMAP::InterIMAP: Don't export $IMAP_text and $IMAP_cond. --- lib/Net/IMAP/InterIMAP.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Net/IMAP') diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index a838dd0..e595060 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -36,7 +36,7 @@ BEGIN { Net::SSLeay::SSLeay_add_ssl_algorithms(); Net::SSLeay::randomize(); - our @EXPORT_OK = qw/xdg_basedir read_config compact_set $IMAP_text $IMAP_cond + our @EXPORT_OK = qw/xdg_basedir read_config compact_set slurp is_dirty has_new_mails/; } -- cgit v1.2.3 From 0a2558aabfefd6800fe74c24e5aff2b0d47cc5e2 Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Mon, 11 Nov 2019 00:39:09 +0100 Subject: Avoid sending large UID EXPUNGE|FETCH|STORE and APPEND commands. UID EXPUNGE|FETCH|STORE commands are now split into multiple (sequential) commands when their set representation exceeds 4096 bytes in size. Without splitting logic set representations could grow arbitrarily large, and exceed the server's maximum command size. This adds roundtrips which could be eliminated by pipelining, but it's unlikely to make any difference in typical synchronization work. While set representations seem to remain small in practice, they might grow significantly if many non-contiguous UIDs were flagged and/or expunged, and later synchronized at once. Furthermore, for MULTIAPPEND-capable servers, the number of messages is limited to 128 per APPEND command (also subject to a combined literal size of 1MiB like before). These numbers are currently not configurable. They're intentionally lower than Dovecot's default maximum command size (64k) in order to avoid a deadlock situation after sending 8k-long commands under COMPRESS=DEFLATE: https://dovecot.org/pipermail/dovecot/2019-November/117522.html . --- Changelog | 11 +++++++ interimap | 4 +-- lib/Net/IMAP/InterIMAP.pm | 64 ++++++++++++++++++++++++++------------- tests/list | 1 + tests/snippets/dovecot/imapd.conf | 3 ++ tests/split-set/interimap.remote | 1 + tests/split-set/remote.conf | 1 + tests/split-set/t | 43 ++++++++++++++++++++++++++ 8 files changed, 105 insertions(+), 23 deletions(-) create mode 120000 tests/split-set/interimap.remote create mode 120000 tests/split-set/remote.conf create mode 100644 tests/split-set/t (limited to 'lib/Net/IMAP') diff --git a/Changelog b/Changelog index 763a7a1..f0c9e50 100644 --- a/Changelog +++ b/Changelog @@ -94,6 +94,17 @@ interimap (0.5) upstream; FETCH response was received. - libinterimap: push_flag_updates(): don't ignores received updates (by another client) to a superset of the desigred flag list. + - libinterimap: avoid sending large UID EXPUNGE|FETCH|STORE commands as + they might exceed the server's max acceptable command size; these + commands are now split into multiple (sequential) commands when their + set representation exceeds 4096 bytes in size. Performance could be + improved by pipelining but given the scope of this software + (synchronization) it's unlikely to make any difference in practice. + This is a also a workaround for a bug in Dovecot 2.3.4: + https://dovecot.org/pipermail/dovecot/2019-November/117522.html + - interimap: for the reason explained above, limit number of messages + to 128 per APPEND command (only on servers advertizing MULTIAPPEND, + for other servers the number remains 1). -- Guilhem Moulin Fri, 10 May 2019 00:58:14 +0200 diff --git a/interimap b/interimap index 9a1df0b..386492e 100755 --- a/interimap +++ b/interimap @@ -1166,8 +1166,8 @@ sub callback_new_message($$$$;$$$) { } else { # use MULTIAPPEND (RFC 3502) - # proceed by 1MiB batches to save roundtrips without blowing up the memory - if (@$buff and $$bufflen + $length > 1048576) { + # proceed by batches of 128/1MiB to save roundtrips without blowing up the memory + if ($#$buff >= 127 or (@$buff and $$bufflen + $length > 1048576)) { @UIDs = callback_new_message_flush($idx, $mailbox, $name, @$buff); @$buff = (); $$bufflen = 0; diff --git a/lib/Net/IMAP/InterIMAP.pm b/lib/Net/IMAP/InterIMAP.pm index e595060..02ae65f 100644 --- a/lib/Net/IMAP/InterIMAP.pm +++ b/lib/Net/IMAP/InterIMAP.pm @@ -203,6 +203,21 @@ sub compact_list(@) { return $set; } +# with_set($set, $cmd) +# Split long commands over multiple subsets to avoid exceeding the server limit +sub with_set($&) { + my ($set, $cmd) = @_; + my $max_length = 4096; + for (my $length = length($set); $length > $max_length;) { + my $l = rindex($set, ',', $max_length); + die unless $l > 0; # sanity check + $cmd->(substr($set, 0, $l)); + $set = substr($set, ++$l); + $length -= $l; + } + return $cmd->($set); +} + # in_set($x, $set) # Return true if the UID or sequence number $x belongs to the set $set. @@ -841,9 +856,10 @@ sub remove_message($@) { $self->fail("Server did not advertise UIDPLUS (RFC 4315) capability.") unless $self->_capable('UIDPLUS'); - my $set = compact_set(@set); - $self->_send("UID STORE $set +FLAGS.SILENT (\\Deleted)"); - $self->_send("UID EXPUNGE $set"); # RFC 4315 UIDPLUS + with_set(compact_set(@set), sub($) { + $self->_send("UID STORE $_[0] +FLAGS.SILENT (\\Deleted)"); + $self->_send("UID EXPUNGE $_[0]"); # RFC 4315 UIDPLUS + }); my %vanished = map {$_ => 1} @{$self->{_VANISHED}}; @@ -960,7 +976,9 @@ sub append($$@) { # optional $callback. sub fetch($$$;&) { my ($self, $set, $flags, $callback) = @_; - $self->_send("UID FETCH $set $flags", $callback); + return with_set($set, sub($) { + $self->_send("UID FETCH $_[0] $flags", $callback); + }); } @@ -1217,7 +1235,9 @@ sub pull_updates($;$) { $self->{_MODIFIED} = {}; # non-empty @missing indicates a discouraged (but allowed) CONDSTORE server behavior, # cf. RFC 7162 sec. 3.1.3 ex. 8 and the comment in push_flag_updates() below - $self->_send("UID FETCH ".compact_set(@missing)." (MODSEQ FLAGS)") if @missing; + with_set(compact_set(@missing), sub($) { + $self->_send("UID FETCH $_[0] (MODSEQ FLAGS)") + }) if @missing; } # do that afterwards since the UID FETCH command above can produce VANISHED responses @@ -1279,7 +1299,7 @@ sub pull_new_messages($$&@) { $range .= "$since:4294967295"; $UIDNEXT = $cache->{UIDNEXT} // $self->panic(); # sanity check - $self->_send("UID FETCH $range ($attrs)", sub($) { + $self->fetch($range, "($attrs)", sub($) { my $mail = shift; $UIDNEXT = $mail->{UID} + 1 if $UIDNEXT <= $mail->{UID}; $callback->($mail) if defined $callback; @@ -1307,22 +1327,23 @@ sub push_flag_updates($$@) { my $mailbox = $self->{_SELECTED} // $self->panic(); my $modseq = $self->{_PCACHE}->{$mailbox}->{HIGHESTMODSEQ} // $self->panic(); - my $command = "UID STORE ".compact_set(@set)." (UNCHANGEDSINCE $modseq) FLAGS.SILENT ($flags)"; my %failed; - $self->_send($command); - if ($IMAP_text =~ /\A\Q$IMAP_cond\E \[MODIFIED ([0-9,:]+)\] $RE_TEXT_CHAR+\z/) { - foreach (split /,/, $1) { - if (/\A([0-9]+)\z/) { - $failed{$1} = 1; - } elsif (/\A([0-9]+):([0-9]+)\z/) { - my ($min, $max) = $1 < $2 ? ($1,$2) : ($2,$1); - $failed{$_} = 1 foreach ($min .. $max); - } else { - $self->panic($_); + with_set(compact_set(@set), sub($) { + $self->_send("UID STORE $_[0] (UNCHANGEDSINCE $modseq) FLAGS.SILENT ($flags)"); + if ($IMAP_text =~ /\A\Q$IMAP_cond\E \[MODIFIED ([0-9,:]+)\] $RE_TEXT_CHAR+\z/) { + foreach (split /,/, $1) { + if (/\A([0-9]+)\z/) { + $failed{$1} = 1; + } elsif (/\A([0-9]+):([0-9]+)\z/) { + my ($min, $max) = $1 < $2 ? ($1,$2) : ($2,$1); + $failed{$_} = 1 foreach ($min .. $max); + } else { + $self->panic($_); + } } } - } + }); my @ok; foreach my $uid (@set) { @@ -1360,8 +1381,9 @@ sub push_flag_updates($$@) { sub silent_store($$$@) { my $self = shift; my $set = shift; - my $mod = shift; - $self->_send("UID STORE $set ${mod}FLAGS.SILENT (".join(' ', @_).")"); + my $subcmd = shift . "FLAGS.SILENT"; + my $flags = join(' ', @_); + with_set($set, sub($) { $self->_send("UID STORE $_[0] $subcmd ($flags)") }); } @@ -1374,7 +1396,7 @@ sub expunge($$) { $self->fail("Server did not advertise UIDPLUS (RFC 4315) capability.") unless $self->_capable('UIDPLUS'); - $self->_send("UID EXPUNGE $set"); + with_set($set, sub($) { $self->_send("UID EXPUNGE $_[0]") }); } diff --git a/tests/list b/tests/list index 8bb4478..a18cb29 100644 --- a/tests/list +++ b/tests/list @@ -40,6 +40,7 @@ repair --repair compress COMPRESS=DEFLATE condstore CONDSTORE +split-set Split large sets to avoid extra-long command lines . SSL/TLS starttls-logindisabled LOGINDISABLED STARTTLS diff --git a/tests/snippets/dovecot/imapd.conf b/tests/snippets/dovecot/imapd.conf index 18c60f8..2b26451 100644 --- a/tests/snippets/dovecot/imapd.conf +++ b/tests/snippets/dovecot/imapd.conf @@ -14,3 +14,6 @@ service imap-login { ssl = yes } } + +# we should avoid sending command lines that are too long +imap_max_line_length = 8192 diff --git a/tests/split-set/interimap.remote b/tests/split-set/interimap.remote new file mode 120000 index 0000000..a4ea3f3 --- /dev/null +++ b/tests/split-set/interimap.remote @@ -0,0 +1 @@ +../auth-sasl-plain/interimap.remote \ No newline at end of file diff --git a/tests/split-set/remote.conf b/tests/split-set/remote.conf new file mode 120000 index 0000000..dbbb908 --- /dev/null +++ b/tests/split-set/remote.conf @@ -0,0 +1 @@ +../auth-sasl-plain/remote.conf \ No newline at end of file diff --git a/tests/split-set/t b/tests/split-set/t new file mode 100644 index 0000000..5e8ea52 --- /dev/null +++ b/tests/split-set/t @@ -0,0 +1,43 @@ +N=2048 + +# XXX with COMPRESS=DEFLATE dovecot-imapd 2.3.4 hangs when the command +# line exceeds 'imap_max_line_length' (or 8192, whichever is smaller) +# bytes, instead of returning a tagged BAD response. +# https://dovecot.org/pipermail/dovecot/2019-November/117522.html + +# set UIDNEXT to 10^9 so all uids are 10 chars long, otherwise we'd need +# to add many more messages to obtain large sets +doveadm -u "local" mailbox update --min-next-uid 1000000000 "INBOX" +doveadm -u "remote" mailbox update --min-next-uid 1000000000 "INBOX" + +for ((i = 0; i < N; i++)); do + u="$(shuf -n1 -e "local" "remote")" + sample_message | deliver -u "$u" +done + +interimap_init +check_mailbox_status "INBOX" + +# mark every other message as \Seen on the local server +for ((i = 0; i < N; i+=2)); do + doveadm -u "local" flags add "\\Seen" mailbox "INBOX" $((N-i)) +done + +# send the changes to the remote; this results into an UID STORE set +# representation of size 11*N/2-1, which exceeds $imap_max_line_length +interimap +check_mailbox_status "INBOX" + +# now expunge every other message on the remote server; this results +# into large UID STORE and UID EXPUNGE set representation +for ((i = 0; i < N; i+=2)); do + doveadm -u "local" expunge mailbox "INBOX" $((N-i)) + # add some more messages + u="$(shuf -n1 -e "local" "remote")" + sample_message | deliver -u "$u" +done + +interimap || error +check_mailbox_status "INBOX" + +# vim: set filetype=sh : -- cgit v1.2.3