December 2008 - Dillo-dev - lists.mailman3.com

[patch]: Basic Authentication

by onepoint＠starurchin.org

I reviewed the final auth patch. No changes to suggest, just a few comments. > > The authentication dialog does not know which browser window > > opened the URL so it can't call a_Nav_reload(). Instead I > > implemented a_Nav_reload_url(). > > Removed a_Nav_reload_url() and set bw as a parameter. <facepalm/> Why didn't I think of that? :-) > > There's an interesting case when the server demands authentication > > for a realm that we have already authenticated ourselves to. It > > could be that we didn't send authentication because we didn't know > > the path was in the realm, or it could be that the authentication > > we sent was wrong. This requires the code to deduce what it did > > last time it saw this URL and rely on what it will do the next > > time. It works, but it feels fragile to me. I would prefer the > > URL to explicitly record authentication attempts so that the code > > doesn't have to second-guess itself. Thoughts? > > This will need debugging. Doesn't seem critical though. I've been testing it locally and it seems to work as expected. > > I implemented a full token-value list parser as per the RFC. > > I'd appreciate it in smaller functions and without gotos. OK, I'll investigate breaking it down. > > If the user opens several pages in a realm in a very short time > > then it is conceivable that multiple authentication dialogs for > > the same realm could pop up > > Just added a check. It doesn't solve the problem, but avoids the > storm and sends a warning to stdout. What happens if the user opens pages for different realms at almost the same time. Will the first authentication dialog suppress the other? If so then the user will have to reload the second page before they can log in. > * Does Auth_path_is_inside() need to be case sensitive? I think so. From RFC 2616 Hypertext Transfer Protocol -- HTTP/1.1: 3.2.3 URI Comparison When comparing two URIs to decide if they match or not, a client SHOULD use a case-sensitive octet-by-octet comparison of the entire URIs, with these exceptions: None of the exceptions apply to the path component (except for the condition that "" and "/" are the same path). Since "/foo" and "/FOO" are different paths I deduce that "/foo/bar" is not inside "/FOO". Regards, Jeremy Henty

Dec. 21, 2008

[patch]: Basic Authentication

by onepoint＠starurchin.org

On Sat, Dec 20, 2008 at 09:34:45AM -0300, Jorge Arellano Cid wrote: > ... I don't know of a single site with basic auth to test! :-) I set up a quick test: http://starurchin.org:8080/basic_auth user: dillo password: dillo You can also try my Linux From Scratch Trac account: http://wiki.linuxfromscratch.org/blfs/login user: jeremy_henty password: VglP96IxQ (I'll change the password once you have tested it.) Regards, Jeremy Henty

Dec. 21, 2008

patch: error pages from file dpi

by jcid＠dillo.org

On Fri, Dec 19, 2008 at 10:46:03PM +0000, corvid wrote: > What do you think of sending little HTTP error pages for > file not found and so on? > > I always find it a little jarring when nothing happens and > I have to look down at the status bar for the error. > Committed. -- Cheers Jorge.-

Dec. 21, 2008

Mercurial repository

by jcid＠dillo.org

Hi, The source code repository page was updated to Mercurial. Please send me the patches as advised there, and please add a one-liner entry in the ChangeLog file. That way it'kll be easier for me and proper credits will appear in both. -- Cheers Jorge.-

Dec. 21, 2008

[patch]: Basic Authentication

by corvid＠lavabit.com

auth.c:296: warning: 'match_length' may be used uninitialized in this function

Dec. 20, 2008

[patch]: Basic Authentication

by jcid＠dillo.org

Hi Jeremy, Committed a modified version of the last patch. Here I'm adding some comments. On Thu, Dec 04, 2008 at 11:11:03AM +0000, Jeremy Henty wrote: > > OK, here it is. Please comment. Some observations: > > When the cache parses the HTTP header we just attach any > authentication headers to the cache entry. Nothing else happens until > we process the clients. Just before iterating over the clients we > check if all the data is in. If it is then we do any necessary > authentication. Just moved the test after the for loop. > Opening the authentication dialog runs the FLTK event loop, which > means that incoming data can call the cache again, triggering CCC > warnings. We work around it by deferring the dialog to a timeout so > that the cache finishes iterating over the clients before the dialog > appears. This is annoying but I don't see a better way at the moment. That's the way to do it. > The authentication dialog does not know which browser window opened > the URL so it can't call a_Nav_reload(). Instead I implemented > a_Nav_reload_url(). Removed a_Nav_reload_url() and set bw as a parameter. > There's an interesting case when the server demands authentication for > a realm that we have already authenticated ourselves to. It could be > that we didn't send authentication because we didn't know the path was > in the realm, or it could be that the authentication we sent was > wrong. This requires the code to deduce what it did last time it saw > this URL and rely on what it will do the next time. It works, but it > feels fragile to me. I would prefer the URL to explicitly record > authentication attempts so that the code doesn't have to second-guess > itself. Thoughts? This will need debugging. Doesn't seem critical though. > I implemented a full token-value list parser as per the RFC. Maybe > this is overkill for Basic authentication, but it will be necessary > for Digest authentication. Yes, it has gotos; I began by setting > flags but decided that gotos were actually cleaner. Maybe it should > be rewritten in state-machine style? I'd appreciate it in smaller functions and without gotos. > If the user opens several pages in a realm in a very short time then > it is conceivable that multiple authentication dialogs for the same > realm could pop up (just like the https DPI sometimes does with > certificate dialogs). I don't address this at the moment because I > think it won't be a problem in practice. If it needs fixing then we > will have to monitor the list of currently open dialogs to avoid > duplicates. Just added a check. It doesn't solve the problem, but avoids the storm and sends a warning to stdout. Please review the changes. It almost has no testing. Some changes I remember: - Modified a bit the code ignoring the last '/' of a path. - Reordered function so no massive forward declarations are necessary. - Used VOID2INT macro. - Added some comments (there're still plenty of uncommented functions) - Prefixed all functions in auth.c with "Auth" - Suffixed "_t" the typedefed data structures. - Removed a_Bw_each() - Removed compiler warnings. - some others... * Does Auth_path_is_inside() need to be case sensitive? Well, now it seems is time to fix the missing points and to real-world test it! -- Cheers Jorge.-

Dec. 20, 2008

[patch]: Basic Authentication

by newman.x＠gmail.com

On Sat, Dec 20, 2008 at 09:34:45AM -0300, Jorge Arellano Cid wrote: > There're several minor things I'd like to arrange, but > believe it or not, I don't know of a single site with basic auth > to test! :-) > > Would you mind sending me an example? Yeah, not much of them :). www.csfd.cz user: dillo pass: dillo Look for the link "Prihlasit" ["log-in"] (points to http://www.csfd.cz/login.php) The log-in seems to work, I can see there "Bu? zdr?v! :-)", which means "Howdy :)". But in Firefox there's also some redirection, perhaps via javascript and some cookie mangling, becausu if I go to www.csfd.cz, after log-in, again, it does not look like I am in (still the "Prihlasit" text there). Michal

Dec. 20, 2008

[patch]: Basic Authentication

by jcid＠dillo.org

Hi Jeremy, On Sat, Dec 06, 2008 at 02:25:57PM +0000, Jeremy Henty wrote: > > Next iteration: one bugfix. > > * In Auth_parse_auth_basic(), fix the test for the "realm" token. Today I gave a detailed review to the patch. There're several minor things I'd like to arrange, but believe it or not, I don't know of a single site with basic auth to test! :-) Would you mind sending me an example? TIA. -- Cheers Jorge.-

Dec. 20, 2008

patch: error pages from file dpi

by onepoint＠starurchin.org

On Fri, Dec 19, 2008 at 10:46:03PM +0000, corvid wrote: > What do you think of sending little HTTP error pages for file not > found and so on? I like it. It annoys me when Dillo displays blank pages in these situations and this is definitely better. Jeremy Henty

Dec. 20, 2008

patch: error pages from file dpi

by corvid＠lavabit.com

What do you think of sending little HTTP error pages for file not found and so on? I always find it a little jarring when nothing happens and I have to look down at the status bar for the error.

Dec. 19, 2008