Bug#471563: After the first 'id' command is issues Cyrus IMAPD always returns an error

[Sergio Gelato]

* Dan White [2010-01-16 21:58:33 -0600]:
>> a1 id ("vendor" "Zimbra" "os" "Linux" "os-version" "12")
>> * ID ("name" "Cyrus IMAPD" "version" "v2.2.13-Debian-2.2.13-10 2006/11/13 16:17:53" "vendor" "Project Cyrus" "support-url" "http://asg.web.cmu.edu/cyrus" "os" "Linux" "os-version" "2.6.18-ovz-028stab051.1" "environment" "Built w/ Cyrus SASL 2.1.22; Running w/Cyrus SASL 2.1.22; Built w/Sleepycat Software: Berkeley DB 4.2.52: (December  3, 2003); Running w/Sleepycat Software: Berkeley DB 4.2.52: (December  3, 2003); Built w/OpenSSL 0.9.8c 05 Sep 2006; Running w/ OpenSSL 0.9.8c 05 Sep 2006; CMU Sieve 2.2; TCP Wrappers; NET-SNMP; mmap = shared; lock = fcntl; nonblock = fcntl; idle = poll")
>> a1 OK Completed
>> a2 id ("vendor" "zimbra")
>> a2 NO Only one Id allowed in non-authenticated state

I don't see NO listed as a valid response in RFC 2971 § 3.1, only OK and BAD.
So it would seem that this behavior is not RFC-compliant.

>> ...which makes it look like an upstream bug in Cyrus IMAP where any ID
>> command will result in that error to any subsequent ID command or, at least, where
>> that happens iff you don't authenticate correctly the first time.

The way Cyrus IMAP (I'm looking at 2.3.15, but I doubt that code has changed
much since then) keeps track of this is with a
   static int did_id
variable local to function cmd_id() (in imap/imapd.c). That variable would
have to be per-connection instead.

>> While the Zimbra client should probably cope with the failure of the id
>> command it is not reasonable, I think, that any user can cause ID
>> commands to fail globally for all other users.

I don't think it's reasonable for Cyrus IMAP to expect clients to cope with
a response that isn't allowed by the specification. If the server doesn't
want to waste time processing further ID commands, it should probably reply
with its own ID (or * ID NIL) and an OK status, and skip the parsing and
logging of the client data. (Replying with BAD would mean that the server
has parsed the client's ID and found it invalid; it's easier to just accept
the information and discard it unparsed.)

> I confirmed that this happens with 2.2.13-17, and also with an undebianized
> 2.3.16.

Yes, the bug is present in the upstream sources.

> The issue is that if subsequent connections come in to the same imapd
> process and no other users have authenticated against that imapd process,
> then subsequent ID commands will receive the 'NO Only one Id allowed in
> non-authenticated state' error, until either a new imapd process is fired
> up, or until an authentication happens.

Authentication only causes the did_id test to be skipped, it doesn't reset 
the did_id flag. The next client serviced by the same process will still
get a NO on the first ID command it issues in the preauth state.

Besides not ever replying NO to an ID command, the server ought to reset the
did_id flag on entry to cmdloop().

> The ID command is governed by RFC 2971, which states:
>
>   7. Security Considerations
>   ...
>   Since this command includes arbitrary data and does not require the
>   user to authenticate, server implementations are cautioned to guard
>   against an attacker sending arbitrary garbage data in order to fill
>   up the ID log.  In particular, if a server naively logs each ID
>   command to disk without inspecting it, an attacker can simply fire up
>   thousands of connections and send a few kilobytes of random data.
>   Servers have to guard against this.  Methods include truncating
>   abnormally large responses; collating responses by storing only a
>   single copy, then keeping a counter of the number of times that
>   response has been seen; keeping only particularly interesting parts
>   of responses; and only logging responses of users who actually log
>   in.
>
>
> This 'functionality' may be Cyrus's way of circumventing a denial of
> service attack by a string of unauthenticated users.

They've worried about authenticated users as well: there is a MAXIDLOG
limit on the number of ID responses that will be logged "from a given
client". The corresponding logged_id counter is also not reset on entry
to cmdloop() as it should. It seems that the author of cmd_id() expected
every connection to get a fresh process.