Skip to content

Commit 1dd94e9

Browse files
elliefmelliefm
committed
imapd: drop connection if bad tag on first command
1 parent d50ebec commit 1dd94e9

File tree

2 files changed

+132
-6
lines changed

2 files changed

+132
-6
lines changed

‎cassandane/Cassandane/Cyrus/Alpaca.pm

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
package Cassandane::Cyrus::Alpaca;
4141
use strict;
4242
use warnings;
43+
use Cwd qw(abs_path);
4344
use Data::Dumper;
4445

4546
use lib '.';
@@ -250,11 +251,129 @@ FIN
250251
# cyrus should have dropped the connection before we sent all the lines
251252
$self->assert_num_lt(scalar @request, scalar @response);
252253

253-
# should have gotten as many untagged BAD responses as cyrus's limit
254-
$self->assert_num_equals(10, scalar grep { m/^\* BAD/ } @response);
254+
# cyrus should have dropped the connection at the POST in first line
255+
$self->assert_num_equals(0, scalar grep { m/^\* BAD/ } @response);
255256

256257
# snarky last response back from the server
257258
$self->assert_matches(qr{This is an IMAP server}, $response[-1]);
258259
}
259260

261+
sub test_http_post_drop_connection1
262+
{
263+
my ($self) = @_;
264+
265+
# get a pristine connection
266+
$self->{store}->disconnect();
267+
my $talk = $self->{store}->get_client(NoLogin => 1);
268+
269+
# consume initial capabilities response
270+
my ($ok, $capability) = $talk->_parse_response('', { IdleResponse => 1 });
271+
$self->assert_str_equals('ok', $ok);
272+
$self->assert_str_equals('capability', $capability);
273+
274+
# mimic a HTTP client sending "POST / HTTP/1.1\r\n" command,
275+
# expect "* BYE This is an IMAP server" and disconnect
276+
# throws "IMAPTalk: Connection was unexpectedly closed by host"
277+
eval {
278+
imap_cmd_with_tag($talk, 'POST',
279+
'/', 0, '/',
280+
'HTTP/1.1');
281+
};
282+
# XXX can't see the real exception text cause cass obscures it,
283+
# XXX but there should have at least been some exception
284+
$self->assert_not_null($@);
285+
$self->assert_str_equals('This is an IMAP server',
286+
$talk->get_response_code('bye'));
287+
288+
# should be back to unconnected state
289+
$self->assert_num_equals(0, $talk->state());
290+
}
291+
292+
sub test_http_post_drop_connection2
293+
{
294+
my ($self) = @_;
295+
296+
# get a pristine connection
297+
$self->{store}->disconnect();
298+
my $talk = $self->{store}->get_client(NoLogin => 1);
299+
300+
# consume initial capabilities response
301+
my ($ok, $capability) = $talk->_parse_response('', { IdleResponse => 1 });
302+
$self->assert_str_equals('ok', $ok);
303+
$self->assert_str_equals('capability', $capability);
304+
305+
# as above, but mimic a HTTP connection trying sneak a POST request
306+
# past by requesting a resource whose name is a valid IMAP command
307+
# which accepts an argument, e.g. "POST authenticate HTTP/1.1\r\n"
308+
eval {
309+
imap_cmd_with_tag($talk, 'POST',
310+
'authenticate', 0, 'authenticate',
311+
'HTTP/1.1');
312+
};
313+
# XXX can't see the real exception text cause cass obscures it,
314+
# XXX but there should have at least been some exception
315+
$self->assert_not_null($@);
316+
$self->assert_str_equals('This is an IMAP server',
317+
$talk->get_response_code('bye'));
318+
319+
# should be back to unconnected state
320+
$self->assert_num_equals(0, $talk->state());
321+
}
322+
323+
sub test_http_post_drop_connection3
324+
:TLS :needs_dependency_openssl
325+
{
326+
my ($self) = @_;
327+
328+
# get a pristine connection
329+
$self->{store}->disconnect();
330+
my $talk = $self->{store}->get_client(NoLogin => 1);
331+
332+
# first command after STARTTLS should be treated same as first command
333+
$talk->_imap_cmd('starttls', 0, 'starttls');
334+
$self->assert_str_equals('ok', $talk->get_last_completion_response());
335+
my $ca_file = abs_path("data/certs/cacert.pem");
336+
IO::Socket::SSL->start_SSL($talk->{Socket},
337+
SSL_ca_file => $ca_file,
338+
SSL_verifycn_scheme => 'none',
339+
);
340+
$self->assert_str_equals('IO::Socket::SSL', ref $talk->{Socket});
341+
342+
# mimic a HTTP client sending "POST / HTTP/1.1\r\n" command,
343+
# expect "* BYE This is an IMAP server" and disconnect
344+
# throws "IMAPTalk: Connection was unexpectedly closed by host"
345+
eval {
346+
imap_cmd_with_tag($talk, 'POST',
347+
'/', 0, '/',
348+
'HTTP/1.1');
349+
};
350+
# XXX can't see the real exception text cause cass obscures it,
351+
# XXX but there should have at least been some exception
352+
$self->assert_not_null($@);
353+
$self->assert_str_equals('This is an IMAP server',
354+
$talk->get_response_code('bye'));
355+
356+
# should be back to unconnected state
357+
$self->assert_num_equals(0, $talk->state());
358+
}
359+
360+
sub test_imap_http_methods_ok
361+
{
362+
my ($self) = @_;
363+
364+
# get a normal, already logged in IMAP connection
365+
my $talk = $self->{store}->get_client();
366+
367+
my @http_methods = qw(
368+
ACL BIND LOCK MKCALENDAR MKCOL PATCH POST
369+
PROPFIND PROPPATCH PUT REPORT SEARCH UNBIND
370+
);
371+
372+
# HTTP method names are not forbidden tags during a normal IMAP session
373+
foreach my $meth (@http_methods) {
374+
imap_cmd_with_tag($talk, $meth, 'noop', 0, 'noop');
375+
$self->assert_str_equals('ok', $talk->get_last_completion_response());
376+
}
377+
}
378+
260379
1;

‎imap/imapd.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,10 +1601,17 @@ static void cmdloop(void)
16011601
goto done;
16021602
}
16031603
if (c != ' ' || !imparse_istag(tag.s, command_count)) {
1604-
syntax_errors ++;
1605-
prot_printf(imapd_out, "* BAD Invalid tag\r\n");
1606-
eatline(imapd_in, c);
1607-
continue;
1604+
if (command_count) {
1605+
syntax_errors ++;
1606+
prot_printf(imapd_out, "* BAD Invalid tag\r\n");
1607+
eatline(imapd_in, c);
1608+
continue;
1609+
}
1610+
else {
1611+
/* bad tag on very first command? probably not speaking IMAP */
1612+
prot_printf(imapd_out, "* BYE This is an IMAP server\r\n");
1613+
goto done;
1614+
}
16081615
}
16091616

16101617
/* Parse command name */

0 commit comments

Comments
 (0)