express-basic-auth - npm Package Compare versions

Comparing version 1.1.6 to 1.1.7

express-basic-auth.d.ts

@@ -18,2 +18,16 @@ /// <reference types="express" />
     /**
+     * Time safe string comparison function to protect against timing attacks.
+     * 
+     * It is important to provide the arguments in the correct order, as the runtime
+     * depends only on the `userInput` argument. Switching the order would expose the `secret`
+     * to timing attacks.
+     * 
+     * @param userInput The user input to be compared
+     * @param secret The secret value the user input should be compared with
+     * 
+     * @returns true if `userInput` matches `secret`, false if not
+     */
+    export function safeCompare(userInput: string, secret: string): boolean
+
+    /**
      * The configuration you pass to the middleware can take three forms, either:
@@ -20,0 +34,0 @@ *  - A map of static users ({ bob: 'pa$$w0rd', ... }) ;

index.js

 const auth = require('basic-auth')
 const assert = require('assert')
+const timingSafeEqual = require('crypto').timingSafeEqual
 
+// Credits for the actual algorithm go to github/@Bruce17
+// Thanks to github/@hraban for making me implement this
+function safeCompare(userInput, secret) {
+    const userInputLength = Buffer.byteLength(userInput)
+    const secretLength = Buffer.byteLength(secret)
+
+    const userInputBuffer = Buffer.alloc(userInputLength, 0, 'utf8')
+    userInputBuffer.write(userInput)
+    const secretBuffer = Buffer.alloc(userInputLength, 0, 'utf8')
+    secretBuffer.write(secret)
+
+    return !!(timingSafeEqual(userInputBuffer, secretBuffer) & userInputLength === secretLength)
+}
+
 function ensureFunction(option, defaultValue) {
@@ -27,3 +42,3 @@ if(option == undefined)
         for(var i in users)
-            if(username == i && password == users[i])
+            if(safeCompare(username, i) & safeCompare(password, users[i]))
                 return true
@@ -83,2 +98,3 @@
 
+buildMiddleware.safeCompare = safeCompare
 module.exports = buildMiddleware

package.json

 {
   "name": "express-basic-auth",
-  "version": "1.1.6",
+  "version": "1.1.7",
   "description": "Plug & play basic auth middleware for express",
@@ -5,0 +5,0 @@ "main": "index.js",

README.md

@@ -68,4 +68,11 @@ # express-basic-auth
 however you want. It will be called with a username and password and is expected to
-return `true` or `false` to indicate that the credentials were approved or not:
+return `true` or `false` to indicate that the credentials were approved or not.
 
+When using your own `authorizer`, make sure **not to use standard string comparison (`==` / `===`)**
+when comparing user input with secret credentials, as that would make you vulnerable against
+[timing attacks](https://en.wikipedia.org/wiki/Timing_attack). Use the provided `safeCompare`
+function instead - always provide the user input as its first argument. Also make sure to use bitwise
+logic operators (`|` and `&`) instead of the standard ones (`||` and `&&`) for the same reason, as
+the standard ones use shortcuts.
+
 ```js
@@ -75,9 +82,13 @@ app.use(basicAuth( { authorizer: myAuthorizer } ))
 function myAuthorizer(username, password) {
-    return username.startsWith('A') && password.startsWith('secret')
+    const userMatches = basicAuth.safeCompare(username, 'customuser')
+    const passwordMatches = basicAuth.safeCompare(password, 'custompassword')
+
+    return userMatches & passwordMatches
 }
 ```
 
-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 ;-)
+This will authorize all requests with the credentials 'customuser:custompassword'.
+In an actual application you would likely look up some data instead ;-) You can do whatever you
+want in custom authorizers, just return `true` or `false` in the end and stay aware of timing
+attacks.
 
@@ -100,3 +111,3 @@ ### Custom Async Authorization
 function myAsyncAuthorizer(username, password, cb) {
-    if (username.startsWith('A') && password.startsWith('secret'))
+    if (username.startsWith('A') & password.startsWith('secret'))
         return cb(null, true)
@@ -103,0 +114,0 @@ else

test.js

 const should = require('should')
-const basicAuth = require('./index.js')
 const express = require('express')
-const supertest = require('supertest');
+const supertest = require('supertest')
 
+const basicAuth = require('./index.js')
+
 var app = express()
@@ -21,2 +22,7 @@
 
+//Uses a custom (synchronous) authorizer function
+var customCompareAuth = basicAuth({
+    authorizer: myComparingAuthorizer
+})
+
 //Same, but sends a basic auth challenge header when authorization fails
@@ -72,2 +78,6 @@ var challengeAuth = basicAuth({
 
+app.get('/custom-compare', customCompareAuth, function(req, res) {
+    res.status(200).send('You passed')
+})
+
 app.get('/challenge', challengeAuth, function(req, res) {
@@ -114,2 +124,6 @@ res.status(200).send('You passed')
 
+function myComparingAuthorizer(username, password) {
+    return basicAuth.safeCompare(username, 'Testeroni') & basicAuth.safeCompare(password, 'testsecret')
+}
+
 function getUnauthorizedResponse(req) {
@@ -120,2 +134,18 @@ return req.auth ? ('Credentials ' + req.auth.user + ':' + req.auth.password + ' rejected') : 'No credentials provided'
 describe('express-basic-auth', function() {
+    describe('safe compare', function() {
+        const safeCompare = basicAuth.safeCompare
+
+        it('should return false on different inputs', function() {
+            (!!safeCompare('asdf', 'rftghe')).should.be.false()
+        })
+
+        it('should return false on prefix inputs', function() {
+            (!!safeCompare('some', 'something')).should.be.false()
+        })
+
+        it('should return false on different inputs', function() {
+            (!!safeCompare('anothersecret', 'anothersecret')).should.be.true()
+        })
+    })
+
     describe('static users', function() {
@@ -137,2 +167,9 @@ const endpoint = '/static'
 
+        it('should reject on shorter prefix', function(done) {
+            supertest(app)
+                .get(endpoint)
+                .auth('Admin', 'secret')
+                .expect(401, done)
+        })
+
         it('should reject without challenge', function(done) {
@@ -179,2 +216,27 @@ supertest(app)
         })
+
+        describe('with safe compare', function() {
+            const endpoint = '/custom-compare'
+            
+            it('should reject wrong credentials', function(done) {
+                supertest(app)
+                    .get(endpoint)
+                    .auth('bla', 'blub')
+                    .expect(401, done)
+            })
+
+            it('should reject prefix credentials', function(done) {
+                supertest(app)
+                    .get(endpoint)
+                    .auth('Test', 'test')
+                    .expect(401, done)
+            })
+
+            it('should accept fitting credentials', function(done) {
+                supertest(app)
+                    .get(endpoint)
+                    .auth('Testeroni', 'testsecret')
+                    .expect(200, 'You passed', done)
+            })
+        })
     })
@@ -181,0 +243,0 @@

No alert changes

Improved metrics

Total package byte prevSize

31332

increased by14.66%

Lines of code

574

increased by15.03%

Number of lines in readme file

217

increased by5.34%

No dependency changes