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.
