dovecot - Sep 2011 - [PATCH] SCRAM-SHA-1 authentication

Hy,

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.

There are some checks for the message format which (assuming the client
acts correclty) are not strictly necessary during parsing. This is
partially in the hope that it might aid client implementers, partially
because it (IMHO) improves readability when checking against the RFC.

Also errors found in this way could be sent to the client, this is
however strictly OPTIONAL in the RFC, for now they are just logged.

Some of the variable names are rather long. This is in order to have
them match the terms introduced in the RFC, again I expect it to help
readability (maybe my recent Objective-C programming showing though).

I do feel somewhat insecure about my usage of some lib functions.
Hopefully no API has been abused too much.

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.

Attached is a hg export.
It also includes a hmac-sha1 implementation, an adaption off of the
hmac-md5 implementation already in Dovecot. I guess those should
eventually be merged into a hash-independent hmac implementation, but I
figured this would have to do for now.

The implementation has been tested against GNU SASL and does appear to
work fine. (The command line was `gsasl -m SCRAM-SHA-1 -a user -p pass
--imap host` for those curious)

Regards
Florian "Florob" Zeitz
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: scram-sha-1.exp
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20110916/c268709f/attachment-0004.pl>

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?

On 11/23/2011 9:57 PM, Timo Sirainen wrote:
> On Sun, 2011-09-18 at 03:44 +0200, Florian Zeitz wrote:
>> Am 16.09.2011 14:40, schrieb Timo Sirainen:
>>> On Fri, 2011-09-16 at 02:47 +0200, Florian Zeitz wrote:
>>>
>>> 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 decided to do it myself, hope this fixes all issues.
> Committed finally to http://hg.dovecot.org/dovecot-2.1 with some
> cleanups.
Great, the Pigeonhole ManageSieve implementation is now suddenly fully 
RFC compliant, without any effort on my part! :)

Regards,

Stephan.