diff --git a/README.md b/README.md index c61ee59..cc11772 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,7 @@ return `true` or `false` to indicate that the credentials were approved or not: app.use(basicAuth( { authorizer: myAuthorizer } )) function myAuthorizer(username, password) { + // Take care to use a timing attack safe compare function in real code return username.startsWith('A') && password.startsWith('secret') } ``` @@ -79,6 +80,31 @@ This will authorize all requests with credentials where the username begins with `'A'` and the password begins with `'secret'`. In an actual application you would likely look up some data instead ;-) +### Timing attacks + +When using a custom validation callback, even when comparing hashes, make sure +to **avoid standard string compare (`==`)**. Use a "constant time compare +function". This protects you against [timing +attacks](https://en.wikipedia.org/wiki/Timing_attack). + +For example, using +[safe-compare](https://www.npmjs.com/package/safe-compare): + +```js +const compare = require('safe-compare'); +app.use(basicAuth( { authorizer: myAuthorizer } )) + +function myAuthorizer(username, password) { + // Use non-short-circuiting evaluation to ensure constant time comparison + return compare("admin", username) & compare("admin-password", password); +} +``` + +To avoid leaking information about the username, a +non-[short-circuiting](https://en.wikipedia.org/wiki/Short-circuit_evaluation) +and operator is used; `&`. This guarantees both username and password are always +compared. + ### Custom Async Authorization Note that the `authorizer` function above is expected to be synchronous. This is diff --git a/index.js b/index.js index ee08756..9a39ee1 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,6 @@ const auth = require('basic-auth') const assert = require('assert') +const compare = require('safe-compare'); function ensureFunction(option, defaultValue) { if(option == undefined) @@ -24,7 +25,7 @@ function buildMiddleware(options) { function staticUsersAuthorizer(username, password) { for(var i in users) - if(username == i && password == users[i]) + if(compare(username, i) & compare(password, users[i])) return true return false diff --git a/package-lock.json b/package-lock.json index 1ae814c..e55d8e8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -149,6 +149,25 @@ "integrity": "sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw==", "dev": true }, + "buffer-alloc": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/buffer-alloc/-/buffer-alloc-1.2.0.tgz", + "integrity": "sha512-CFsHQgjtW1UChdXgbyJGtnm+O/uLQeZdtbDo8mfUgYXCHSM1wgrVxXm6bSyrUuErEb+4sYVGCzASBRot7zyrow==", + "requires": { + "buffer-alloc-unsafe": "^1.1.0", + "buffer-fill": "^1.0.0" + } + }, + "buffer-alloc-unsafe": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/buffer-alloc-unsafe/-/buffer-alloc-unsafe-1.1.0.tgz", + "integrity": "sha512-TEM2iMIEQdJ2yjPJoSIsldnleVaAk1oW3DBVUykyOLsEsFmEc9kn+SFFPz+gl54KQNxlDnAwCXosOS9Okx2xAg==" + }, + "buffer-fill": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/buffer-fill/-/buffer-fill-1.0.0.tgz", + "integrity": "sha1-+PeLdniYiO858gXNY39o5wISKyw=" + }, "bytes": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.0.0.tgz", @@ -657,6 +676,14 @@ "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==" }, + "safe-compare": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/safe-compare/-/safe-compare-1.1.3.tgz", + "integrity": "sha512-iAKjxcWhyoNa97H0P8KRHxSt05WQtbUac7FoI3DVCdYi9m0KRaFCagIj5pplVSkGVDrzCANepmdMB/rrBkNhNA==", + "requires": { + "buffer-alloc": "^1.2.0" + } + }, "safer-buffer": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", diff --git a/package.json b/package.json index 38114c3..2dba4ed 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,8 @@ }, "homepage": "/~https://github.com/LionC/express-basic-auth#readme", "dependencies": { - "basic-auth": "^2.0.1" + "basic-auth": "^2.0.1", + "safe-compare": "^1.1.3" }, "devDependencies": { "@types/express": "^4.16.0",