Hey folks :) attached is a patch that implements http digest authentication. Some thoughs: * add-md5.patch adds md5.{c,h}, a BSD licensed md5 implementation by L. Peter Deutsch from [0] * the digest logic resides in digest.c * I adapted a lot of challenge parsing code from Jeremys basic auth to better suit my needs, sorry for the big patch, but this reduces code duplication (even mailman thinks the patch is too big >,<) * rfc2617 is a bit fuzzy about the quoted strings in the challenge. strings are enclosed in double quotes, but it is not specified (or I overlooked it..) how double quotes within this string should be escaped. I intuitively escaped them with a \, and apache2 plays along nicely. So does my code. Funny thing is, neither one of the mayor browsers (I tested ff, epiphany, opera, konqueror, ie6) can cope with realms containing quotes... Am I missing something here? * http digest can provide integrity protection for the request body by computing a hash over the request body, but I have no pointer to the request body, only the request URL. We might need to pass the whole request object (is there such a thing?) to the auth code instead of just the url. The code is there, just the pointer is missing. The code in auth.c will select the method without integrity protection for now, hoping that the server will accept both auth and auth-int. * there is a memory leak (well, at least one that I am aware of) in a_Auth_get_auth_str (grep 'this leaks memory' auth.c). This function is called for each request dillo makes to a host using http auth. For basic authentication the challenge response is always the same, so a reference to that string is stored in the realm and it is freed with its destruction. For digest authentication a different response is needed for each request (the request uri and a nonce_counter needs to be updated), so a new string has to be generated for each request. This should be easily fixable, but I didn't looked into that yet (need to touch the callers code). That's it :) feedback is welcome! Justus 0: http://sourceforge.net/project/showfiles.php?group_id=42360
Justus wrote:
* rfc2617 is a bit fuzzy about the quoted strings in the challenge. strings are enclosed in double quotes, but it is not specified (or I overlooked it..) how double quotes within this string should be escaped. I intuitively escaped them with a \, and apache2 plays along nicely. So does my code. Funny thing is, neither one of the mayor browsers (I tested ff, epiphany, opera, konqueror, ie6) can cope with realms containing quotes... Am I missing something here?
I think that's right. Section 1.1 says that it relies on HTTP/1.1 for grammar stuff, and if I go there, I find: quoted-string = ( <"> *(qdtext | quoted-pair ) <"> ) qdtext = <any TEXT except <">> quoted-pair = "\" CHAR
* http digest can provide integrity protection for the request body by computing a hash over the request body, but I have no pointer to the request body, only the request URL. We might need to pass the whole request object (is there such a thing?) to the auth code instead of just the url. The code is there, just the pointer is missing. The code in auth.c will select the method without integrity protection for now, hoping that the server will accept both auth and auth-int.
It sounds like entity-body is what you get _after_ dealing with the transfer coding (chunked) but _before_ dealing with the content coding (gzip). How annoying. It sounds like it would be necessary to hack some special field into the cache entry to be used when we see a WWW-Authenticate header, at least if it was gzipped...
Justus wrote:
corvid wrote:
* http digest can provide integrity protection for the request body by computing a hash over the request body, but I have no pointer to the request body, only the request URL. We might need to pass the whole request object (is there such a thing?) to the auth code instead of just the url. The code is there, just the pointer is missing. The code in auth.c will select the method without integrity protection for now, hoping that the server will accept both auth and auth-int.
It sounds like entity-body is what you get _after_ dealing with the transfer coding (chunked) but _before_ dealing with the content coding (gzip). How annoying. It sounds like it would be necessary to hack some special field into the cache entry to be used when we see a WWW-Authenticate header, at least if it was gzipped... Any hints on that one?
The ever-growing CacheEntry_t would need an EntityBody field, and in a_Cache_process_dbuf, it would go something like if (entry->TransferDecoder) { dstr1 = a_Decode_process(entry->TransferDecoder, str, len); str = dstr1->str; len = dstr1->len; } if (entry->Auth) Dstr_append_l(entry->EntityBody, str, len); if (entry->ContentDecoder) { dstr2 = a_Decode_process(entry->ContentDecoder, str, len); str = dstr2->str; len = dstr2->len; } And then your code will need to call something like a_Cache_get_entity_body(url).
Oh, also...somewhere in there I noticed that an int was being set from a test for equality, with the result assumed to be 0 or 1. I might be wrong here, but I think C only specifies that you get 0 or nonzero. Auth_unquote_value contained that assumption which I fixed. Was that the
corvid wrote: part in question?
Probably.
Any more comments on my patch?
Just that I asked the search engine for "digest authentication 'entity-body'" and it gave me a bunch of people wondering what's supposed to happen when the entity-body is empty. So there's a weird test case for you.
On Wed, Apr 08, 2009 at 03:42:46AM +0000, corvid wrote:
Justus wrote:
corvid wrote:
* http digest can provide integrity protection for the request body by computing a hash over the request body, but I have no pointer to the request body, only the request URL. We might need to pass the whole request object (is there such a thing?) to the auth code instead of just the url. The code is there, just the pointer is missing. The code in auth.c will select the method without integrity protection for now, hoping that the server will accept both auth and auth-int.
It sounds like entity-body is what you get _after_ dealing with the transfer coding (chunked) but _before_ dealing with the content coding (gzip). How annoying. It sounds like it would be necessary to hack some special field into the cache entry to be used when we see a WWW-Authenticate header, at least if it was gzipped... Any hints on that one?
The ever-growing CacheEntry_t would need an EntityBody field, and in a_Cache_process_dbuf, it would go something like if (entry->TransferDecoder) { dstr1 = a_Decode_process(entry->TransferDecoder, str, len); str = dstr1->str; len = dstr1->len; } if (entry->Auth) Dstr_append_l(entry->EntityBody, str, len);
if (entry->ContentDecoder) { dstr2 = a_Decode_process(entry->ContentDecoder, str, len); str = dstr2->str; len = dstr2->len; }
And then your code will need to call something like a_Cache_get_entity_body(url).
Maybe I am the one being completly wrong here, but we're talking about the request body, right? I wouldn't expect to find that that one in the cache... I think that we should abandon auth-int, as noone else supports it (apache doesn't [0] and neither does mozilla [1]. libneon once supported it, but the code in question was removed in 2005 [2]). I am gonna update my code to reflect that fact. Any more thoughts? Justus 0: http://httpd.apache.org/docs/2.2/mod/mod_auth_digest.html#authdigestqop 1: https://bugzilla.mozilla.org/show_bug.cgi?id=168942 2: $ grep -A 3 r462 neon27-0.28.2/ChangeLog r462 | joe | 2005-01-27 22:04:44 +0000 (Thu, 27 Jan 2005) | 7 lines * src/ne_auth.c: Drop qop=auth-int support, sice it is universally unimplemented by servers and comes with too much baggage. (struct
Justus wrote:
On Wed, Apr 08, 2009 at 03:42:46AM +0000, corvid wrote:
Justus wrote:
corvid wrote:
* http digest can provide integrity protection for the request body by computing a hash over the request body, but I have no pointer to the request body, only the request URL. We might need to pass the whole request object (is there such a thing?) to the auth code instead of just the url. The code is there, just the pointer is missing. The code in auth.c will select the method without integrity protection for now, hoping that the server will accept both auth and auth-int.
It sounds like entity-body is what you get _after_ dealing with the transfer coding (chunked) but _before_ dealing with the content coding (gzip). How annoying. It sounds like it would be necessary to hack some special field into the cache entry to be used when we see a WWW-Authenticate header, at least if it was gzipped... Any hints on that one?
The ever-growing CacheEntry_t would need an EntityBody field, and in a_Cache_process_dbuf, it would go something like if (entry->TransferDecoder) { dstr1 = a_Decode_process(entry->TransferDecoder, str, len); str = dstr1->str; len = dstr1->len; } if (entry->Auth) Dstr_append_l(entry->EntityBody, str, len);
if (entry->ContentDecoder) { dstr2 = a_Decode_process(entry->ContentDecoder, str, len); str = dstr2->str; len = dstr2->len; }
And then your code will need to call something like a_Cache_get_entity_body(url).
Maybe I am the one being completly wrong here, but we're talking about the request body, right? I wouldn't expect to find that that one in the cache...
Oh, oh the request... (I don't really know the mechanics of auth at all yet, having felt content to have Jeremy and Justus as the auth experts :) Does URL_DATA(url)->str have what you might expect for POST?
I think that we should abandon auth-int, as noone else supports it (apache doesn't [0] and neither does mozilla [1]. libneon once supported it, but the code in question was removed in 2005 [2]).
I am gonna update my code to reflect that fact. Any more thoughts?
Justus
0: http://httpd.apache.org/docs/2.2/mod/mod_auth_digest.html#authdigestqop 1: https://bugzilla.mozilla.org/show_bug.cgi?id=168942 2: $ grep -A 3 r462 neon27-0.28.2/ChangeLog r462 | joe | 2005-01-27 22:04:44 +0000 (Thu, 27 Jan 2005) | 7 lines
* src/ne_auth.c: Drop qop=auth-int support, sice it is universally unimplemented by servers and comes with too much baggage. (struct
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 corvid wrote:
Justus wrote:
On Wed, Apr 08, 2009 at 03:42:46AM +0000, corvid wrote:
Justus wrote:
corvid wrote:
* http digest can provide integrity protection for the request body by computing a hash over the request body, but I have no pointer to the request body, only the request URL. We might need to pass the whole request object (is there such a thing?) to the auth code instead of just the url. The code is there, just the pointer is missing. The code in auth.c will select the method without integrity protection for now, hoping that the server will accept both auth and auth-int. *snip* Does URL_DATA(url)->str have what you might expect for POST? I think you are right.
I think that we should abandon auth-int, as noone else supports it (apache doesn't [0] and neither does mozilla [1]. libneon once supported it, but the code in question was removed in 2005 [2]).
I am gonna update my code to reflect that fact. Any more thoughts? I commented out the code in question (in case anyone wants to implement it) and left a comment explaining the situation.
If corvid is right and url->data->str contains all the information we need this should be trivial to implement. I also located one server side implementation of http digest auth with auth-int support written in PHP [0]. I am going to set it up next week and give it a try. I published my http digest feature branch at [1] for your (and my) convenience. If you have access to a site that uses http digest authentication please give it a try. I'd also appreciate any comments regarding the inclusion of the bsd licensed md5 implementation. Is that an acceptable practice? Justus 0: http://www.xiven.com/sourcecode/digestauthentication.php 1: http://teythoon.cryptobitch.de/devel/dillo/http-digest -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkndqvgACgkQoPmwNWhsaZYNpQCgtClBdvKwhcIR2gw61K2kUWpj jEIAoJNVBmQtfEKt3WUSNSxqsKYMb++I =rmhj -----END PGP SIGNATURE-----
On Thu, Apr 09, 2009 at 09:59:53AM +0200, Justus Winter wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
corvid wrote:
Justus wrote:
On Wed, Apr 08, 2009 at 03:42:46AM +0000, corvid wrote:
Justus wrote:
corvid wrote:
> * http digest can provide integrity protection for the request body > by computing a hash over the request body, but I have no pointer to > the request body, only the request URL. We might need to pass the > whole request object (is there such a thing?) to the auth code instead > of just the url. The code is there, just the pointer is missing. The > code in auth.c will select the method without integrity protection for > now, hoping that the server will accept both auth and auth-int. *snip* Does URL_DATA(url)->str have what you might expect for POST? I think you are right.
I think that we should abandon auth-int, as noone else supports it (apache doesn't [0] and neither does mozilla [1]. libneon once supported it, but the code in question was removed in 2005 [2]).
I am gonna update my code to reflect that fact. Any more thoughts? I commented out the code in question (in case anyone wants to implement it) and left a comment explaining the situation.
If corvid is right and url->data->str contains all the information we need this should be trivial to implement. I also located one server side implementation of http digest auth with auth-int support written in PHP [0]. I am going to set it up next week and give it a try.
I published my http digest feature branch at [1] for your (and my) convenience. If you have access to a site that uses http digest authentication please give it a try. I'd also appreciate any comments regarding the inclusion of the bsd licensed md5 implementation. Is that an acceptable practice?
I've just one minor comment for now: It would be nice if the password dialog would show the authentication method used. Or can one already see what is going to happen? Cheers, Johannes
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hofmann Johannes wrote:
I've just one minor comment for now: It would be nice if the password dialog would show the authentication method used. Or can one already see what is going to happen? You are absolutely right, and this is in fact on my wishlist as well (grep 'inform the user' src/auth.c). I am going to fix that next week.
Justus -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkndv+oACgkQoPmwNWhsaZbJtACgoi7ELkYRm22j+iJE0RNfSvda TMsAnikrxyb4oo4iArMf1kYgoTePpR5c =f6d9 -----END PGP SIGNATURE-----
On Thu, Apr 09, 2009 at 11:29:14AM +0200, Justus Winter wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hofmann Johannes wrote:
I've just one minor comment for now: It would be nice if the password dialog would show the authentication method used. Or can one already see what is going to happen? You are absolutely right, and this is in fact on my wishlist as well (grep 'inform the user' src/auth.c). I am going to fix that next week.
Ah nice! Johannes
participants (3)
-
4winter＠informatik.uni-hamburg.de -
corvid＠lavabit.com -
Johannes.Hofmann＠gmx.de