Improve handling of htpasswd file

[anonymous]

As explained in this pull request (​https://github.com/edgewall/trac/pull/7) the handling of htpasswd files for HTTP Basic Auth in tracd is not very robust. In particular, it cannot handle basic formatting that other tools (notably apache and nginx) can handle — and it fails in ways that could allow logins that are not intended to be valid. It does not handle comment lines, so that logins that are intended to be commented out are just logins with a # prefixed. Also, if more than just the login name and password are provided, the entire line is printed to the log, exposing passwords. This breaks compatibility with other tools like dokuwiki, which users more fields.

The fix is fairly simple: remove everything after a #, and ignore all but the first two components of any line that remains.

[Ryan J Ollos]

Replying to anonymous:

The fix is fairly simple: remove everything after a #, and ignore all but the first two components of any line that remains.

Is the htpasswd format formally documented anywhere? I assume this should apply to htdigest files as well.

We should consider just making passlib a Trac dependency and using ​passlib.apache.Htpasswd. If not immediately, then at least when we integrate some pieces of ​AccountManagerPlugin.

[anonymous]

TL;DR: Passlib is also broken. Apache, nginx, and dokuwiki each actively support additional colon-separated fields, and so should you.

---

Judging from ​this comment, it looks like passlib also goes by the output from the htpasswd program, rather than acceptable input to the httpd function that reads the files. In particular, ​the parsing function raises an exception if there are not exactly 2 fields that result from a split by colons. But this behavior, like trac's, is inconsistent with apache, nginx, and dokuwiki.

What little documentation there is for apache appears to be ​here. The actual code in apache that does the reading of each line is ​here. The key lines where the username and password are extracted are

w = ap_getword(r->pool, &rpw, ':');

and

file_password = ap_getword(r->pool, &rpw, ':');

Here, &rpw is a const char**. ​This reference says that inside ap_getword, the "char** is updated after each call so that it points to the place where the previous call left off." So that second call to get file_password is explicitly allowing the line to have another colon after the password itself — though if that colon is not found before the end of line, that is also acceptable. (Note that apache also skips blank lines or lines beginning with #, as seen just before those calls.)

Similarly, ​the nginx function looks for the end of the password by searching for a line ending (LF or CR) or a colon. (Again, nginx skips blank lines or lines beginning with #, as seen shortly before the line I linked.)

I won't bother looking through dokuwiki's source; instead I'll just point out that ​the documentation here explicitly gives the list of colon-separated fields it uses.

[anonymous]

I've noted this problem as a ​passlib issue.

[Ryan J Ollos]

I don't see a problem with ignoring everything including and after the second :. We'll need to add a test for the change, and also look into making a similar change for HtDigest.

[Ryan J Ollos]

Proposed changes in [d3e4955cf/rjollos.git].

[Ryan J Ollos]

Committed to 1.2-stable in r15922, merged to trunk in r15923.