[Dovecot] [PATCH] SCRAM-SHA-1 authentication

[Timo Sirainen]

On Fri, 2011-09-16 at 02:47 +0200, Florian Zeitz wrote:

> over the last days I have implemented SCRAM-SHA-1 in Dovecot's 2.1
> branch. It does not do SCRAM-SHA-1-PLUS, but should be extendable enough
> to introduce it later.

Looks pretty good. Below are a few things I noticed. I could fix these
myself next week also, or you can do them during weekend if you want
to. :)

> I also note that there are a lot of fields in the scram_auth_request
> struct. I think they are all there for a reason, however feel free to
> prove me wrong.

The username wouldn't necessarily have to be there. Also its name was
confusing me for a while since I thought you were setting
auth_request->user directly.

> + snonce[i] = (snonce[i] % ('~' - '!')) + '!';
> + if (snonce[i] == ',')
> + snonce[i] = '.';

Here '~' is actually never used. But a nice solution would be to simply
replace ',' with '~' so '.' isn't more likely to occur than others.

> + fields = t_strsplit((const char*)data, ",");

Not safe. data isn't guaranteed to be NUL-terminated. One simple
solution would be: t_strsplit(t_strndup(data, size), ",")

And others:

 - Could be nicer if client->proof was stored base64-decoded, so its
validity could be checked and also later there wouldn't be need to
base64-encode signature when testing it.

 - There's no log message is authentication fails due to wrong password?

 - Doesn't verify_credentials() need to check the credentials in any way
that it contains expected (sized) data? Anything is allowed?