Implement basic auth htpasswd file refresh

[mfuterko]

Implemented basic auth htpasswd file refresh to avoid fabio restart on the htpasswd file change similarly to the cert store refresh.

[lverba]

Nice feature, good to have it!

[pschultz]

Looks good to me.

Is there any harm in enabling refresh by default, say, every 30 seconds or so?

In my experience mtime isn't as reliable as you might expect, especially when network filesystems are involved. How about checking the hash of the file contents instead? A simple ioutil.ReadFile + sha1.Sum should do.

I also suggest to reload the file on SIGHUP. That helps configuration management software that expects a change to the file to work immediately. That would require an update to the log message in exit/listen.go.

[pschultz]

Another thing: currently, if the file is deleted we don't do anything (besides logging a warning). Should we disable basic auth instead (for instance by reloading an empty temp file)?

[mfuterko]

Hi. Thanks for the review and comments.

I disabled refresh by default only for backward compatibility as there was no such functionality before.

As regards to SHA checksum, I think it's a bit overkill to check it every so often so even if there's some delay on NFS etc the file will be re-read whey mtime eventually changes.

Provided Fabio was created to work in zero configuration scenarios via Consul etc adding SIGHUP handler to re-read configuration would be somewhat contradictory on my opinion?

From the security point of stand removing the htpasswd file should rather fail all login attempts but definitely should not disable auth completely.

[aaronhurt]

Thanks for the work and review everyone. @mfuterko would you mind also updating the relevant documentation on the auth feature?

[aaronhurt]

Thank you @mfuterko!

[mfuterko]

@leprechau, no problem, happy to be helpful to the project!

[pschultz]

As regards to SHA checksum, I think it's a bit overkill to check it every so often so even if there's some delay on NFS etc the file will be re-read whey mtime eventually changes.

There are filesystems such as davfs that may not support mtime at all (maybe depending on the server). But I admit that this is an edge-case that is not likely to come up in practice and we can change it if and when it becomes a problem for someone.

From the security point of stand removing the htpasswd file should rather fail all login attempts but definitely should not disable auth completely.

That is what I meant, my wording was a bit misleading. In the current implementation all logins continue to work even after removing the file, because the error is logged but otherwise ignored:

sh1 $ echo foo:{PLAIN}bar > htpasswd
sh1 $ fabio \
    -proxy.addr=:9999 \
    -proxy.auth='name=basic;type=basic;file=htpasswd;refresh=1s' \
    -registry.static.routes='service-a /a http://127.0.0.1:8080/ opts "auth=basic"'

sh2 $ curl -u foo:bar -I http://localhost:9999/a
HTTP/1.1 502 Bad Gateway
Date: Tue, 05 Mar 2019 11:31:54 GMT

sh2 $ mv htpasswd{,.off}
# Wait for reload and warning messages

sh2 $ curl -u foo:bar -I http://localhost:9999/a
HTTP/1.1 502 Bad Gateway
Date: Tue, 05 Mar 2019 11:31:56 GMT

# foo:bar still works; expected 401 instead.

[pschultz]

tg123/go-htpasswd#3 has been merged, so clearing all credentials on error should be as simple as

secrets.ReloadFromReader(&bytes.Buffer{}, bad)

@mfuterko, do you want to submit a follow-up or do you want me to take care of it?

[mfuterko]

@pschultz, thanks for the comments - I'll submit the patch in next day or so.

[mfuterko]

Hi @pschultz, please take a look at #610. Thanks.