Add HTTP Basic Auth #245
Add HTTP Basic Auth #245
Conversation
|
Cool, thanks for taking the torch on this one! |
lib/http-server.js
Outdated
| before.push(function (req, res) { | ||
| var credentials = auth(req); | ||
|
|
||
| if (credentials && credentials.name === options.username && credentials.pass === options.password) { |
There was a problem hiding this comment.
It might be worth using something like secure-compare rather than a simple string comparison? (Secure compare is designed to avoid timing attacks.)
To my mind, I wouldn't be too stress about security (because if I was concerned about security, I probably wouldn't be using basic auth...) but it feels like a good idea to "do it right" (and demonstrate better security practices), and it's quite simple to use a simple secure comparison rather than a ===.
Feel free to ignore me :-) (I'm just some random on the internet :-p ) Just something that came to mind when I saw this :-)
There was a problem hiding this comment.
I think in my head I was thinking 'better not make it secure or people might actually use it to secure things!' but that is obviously unhelpful because a) most people won't know what a timing attack is and so won't benefit from knowing it's not secure, and b) well, it just is.
So: great idea, good reasoning. I'll implement it this week.
|
Implemented @bryce-gibson's idea to use secure-compare. Since it didn't seem too widely used, I looked into it in some depth and wrote some caveats in the commit message (there's a JSPerf that might make some of it clearer here: http://jsperf.com/secompare/7). I've also pinned to the exact version. Obvious disclaimer, I'm no security expert. [Edit: Rebased onto master also] |
|
Nice! |
|
Would love to see this merged if a maintainer has the time :) cc @indexzero |
|
+1 #270 |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
Nice ! EDIT : It might worth knowing that on linux, if you set a password with something like : "foo$bar", the retained password (in options.password) will be : foo, the $ is not taken into account. On windows no problem about it. |
|
Whoops — good catch, thanks @Alex-Werner! Fixed the typo, squashed in, and rebased on latest master for good measure |
|
Any relation to http-server-with-auth? |
|
Yeah, sorry about that. But read the disclaimer I put on, whenerver this will be merge I delete the npm repo. |
|
No objections from me @Alex-Werner! Rebased onto upstream master to fix merge conflicts. |
|
@Alex-Werner can you rebase onto upstream master? |
Using https://www.npmjs.com/package/basic-auth Inspired and built on the work of jondlm in http-party#51
Note that the order of arguments to secureCompare
will have an effect upon the running time — though
not in a way that leaks information about whether
the passwords match.
These:
```javascript
compare("short", "short");
compare("short", "loooo<...>ng");
```
Will run faster than these:
```javascript
compare("loooo<...>ng", "short");
compare("loooo<...>ng", "loooo<...>ng");
```
If someone could duplicate your exact set-up, they
could possibly discover how long your secret
password is by testing how long yours takes to run
compared to how long their identical server takes
to run given passwords of a known length.
Given that this isn't the US Govt (I hope), it's
not a huge concern, and I don't know enough to do
it better. However, given that `http-server`
typically runs on its own (without any other
software that would 'pad out' the timing), you
might want to consider and mitigate this vector of
attack.
|
Rebased this one in case that's what you meant @piecioshka |
|
@BigBlueHat can you merge this PR? |
|
Please merge this! |
|
Bumping thread. I'd want to see it merged |
Add HTTP Basic Auth http-party#245
Built on the work of @jondlm in #51. Using what I figured was the most popular basic auth library: https://www.npmjs.com/package/basic-auth
Can be configured with CLI options or env variables (separated out into its own commit in case you don't want that).
I've erred on the generous side when it comes to tests, as people will probably be pretty peeved if this stops working in some non-obvious way!