On Mon, 2006-10-09 at 10:03 +0100, Philip Hazel wrote:
fprintf(f, "VERSION\t%d\t%d\nCPID\t%d\n" "AUTH\t%d\t%s\tservice=smtp\trip=%s\tlip=%s\tresp=%s\n", VERSION_MAJOR, VERSION_MINOR, getpid(), cuid, ablock->public_name, sender_host_address, interface_address, data ? (char *) data : "");Can data parameter contain tab characters? If it can, you should prevent them from going to dovecot-auth.
Indeed. However, the only one of those fields that might contain tabs is "data", but it is supposed to be base-64 encoded, so it shouldn't. However, some evil person might send an illegal tab in there I suppose. Exim can trivially check for tabs or that the data is valid base-64, but shouldn't Dovecot also do that? The Dovecot home page says "Dovecot is an open source IMAP and POP3 server for Linux/UNIX-like systems, written with security primarily in mind." I would hope, therefore, that whatever junk was passed to it would be rigorously checked.
Since tab is the field separator in the protocol and the fprintf() above just places all of them into same string, it isn't really possible to differentiate between legitimate separator and user-given tab..
I guess I should add checks that the same field isn't given twice, but that doesn't prevent user from giving fields that just weren't given normally.
BTW. There are also two more fields that the Exim code doesn't currently support, but which might be useful for some people:
"secured": Set if SSL/TLS is used, or if remote IP == local IP
"valid-client-cert": Set if client certificate was received and it was verified to be trusted
I'll put in a test for tabs. I am disappointed that new software should be using tabs as separators, however. They are confusing and lead to no end of trouble in other places where they are used like this (Makefiles, Sendmail configs, for example).
It's an internal protocol, not supposed to be user-writable and it needs to be user-readable only up to the point that debugging is possible. :) I think tabs make it pretty easy to read for users and easy to write the parser code.
I personally think that all whitespace characters should be treated as equal. You can't distinguish tabs from spaces when they are displayed, and if you cut and paste text, tabs can get lost.
In that case the spaces would have to be escaped in some way and it'd be more difficult to read the debugging messages..
Well, another protocol that I recently wrote uses ';' as separator and escapes them using "\.". Still pretty human readable and writable, and simple to write parsers for. I guess that could have been a better choice for Dovecot auth protocol also, but it's now a bit too late to change it.