From b04e136ee0fdda5b2a04a0b3d7dbc8b657dff7cb Mon Sep 17 00:00:00 2001
From: Cogent Apps <cogentapps@fastmail.com>
Date: Fri, 17 Mar 2023 18:57:32 +0000
Subject: [PATCH] more secure password hashing

---
 server/package.json           |  2 ++
 server/src/database/index.ts  |  4 ++--
 server/src/database/sqlite.ts |  6 ++---
 server/src/passport.ts        | 42 ++++++++++++++++++-----------------
 4 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/server/package.json b/server/package.json
index 1d11842..5da2fde 100644
--- a/server/package.json
+++ b/server/package.json
@@ -10,6 +10,7 @@
   "license": "MIT",
   "dependencies": {
     "@aws-sdk/client-s3": "^3.282.0",
+    "@types/bcrypt": "^5.0.0",
     "@types/compression": "^1.7.2",
     "@types/connect-sqlite3": "^0.9.2",
     "@types/debug": "^4.1.7",
@@ -23,6 +24,7 @@
     "@types/pg": "^8.6.6",
     "@types/sqlite3": "^3.1.8",
     "axios": "^1.3.4",
+    "bcrypt": "^5.1.0",
     "compression": "^1.7.4",
     "connect-sqlite3": "^0.9.13",
     "debug": "^4.3.4",
diff --git a/server/src/database/index.ts b/server/src/database/index.ts
index 761f3cf..1ccd41e 100644
--- a/server/src/database/index.ts
+++ b/server/src/database/index.ts
@@ -1,11 +1,11 @@
 export default abstract class Database {
     public async initialize() {}
-    public abstract createUser(email: string, passwordHash: Buffer, salt: Buffer): Promise<void>;
+    public abstract createUser(email: string, passwordHash: Buffer): Promise<void>;
     public abstract getUser(email: string): Promise<{
         id: string;
         email: string;
         passwordHash: Buffer;
-        salt: Buffer;
+        salt: Buffer | null;
     }>;
     public abstract getChats(userID: string): Promise<any[]>;
     public abstract getMessages(userID: string): Promise<any[]>;
diff --git a/server/src/database/sqlite.ts b/server/src/database/sqlite.ts
index 46770dd..11b02ab 100644
--- a/server/src/database/sqlite.ts
+++ b/server/src/database/sqlite.ts
@@ -61,14 +61,14 @@ export class SQLiteAdapter extends Database {
         });
     }
 
-    public createUser(email: string, passwordHash: Buffer, salt: Buffer): Promise<void> {
+    public createUser(email: string, passwordHash: Buffer): Promise<void> {
         return new Promise((resolve, reject) => {
             if (!validateEmailAddress(email)) {
                 reject(new Error('invalid email address'));
                 return;
             }
 
-            db.run(`INSERT INTO authentication (id, email, password_hash, salt) VALUES (?, ?, ?, ?)`, [email, email, passwordHash, salt], (err) => {
+            db.run(`INSERT INTO authentication (id, email, password_hash) VALUES (?, ?, ?)`, [email, email, passwordHash], (err) => {
                 if (err) {
                     reject(err);
                     console.log(`[database:sqlite] failed to create user ${email}`);
@@ -90,7 +90,7 @@ export class SQLiteAdapter extends Database {
                     resolve({
                         ...row,
                         passwordHash: Buffer.from(row.password_hash),
-                        salt: Buffer.from(row.salt),
+                        salt: row.salt ? Buffer.from(row.salt) : null,
                     });
                     console.log(`[database:sqlite] retrieved user ${email}`);
                 }
diff --git a/server/src/passport.ts b/server/src/passport.ts
index 83dec3e..e690417 100644
--- a/server/src/passport.ts
+++ b/server/src/passport.ts
@@ -1,3 +1,4 @@
+import bcrypt from 'bcrypt';
 import crypto from 'crypto';
 import passport from 'passport';
 import session from 'express-session';
@@ -18,17 +19,20 @@ export function configurePassport(context: ChatServer) {
             return cb(null, false, { message: 'Incorrect username or password.' });
         }
 
-        crypto.pbkdf2(password, user.salt, 310000, 32, 'sha256', (err, hashedPassword) => {
-            if (err) {
-                return cb(err);
-            }
+        try {
+            console.log(user.salt ? 'Using pbkdf2' : 'Using bcrypt');
+            const isPasswordCorrect = user.salt
+                ? crypto.timingSafeEqual(user.passwordHash, crypto.pbkdf2Sync(password, user.salt, 310000, 32, 'sha256'))
+                : await bcrypt.compare(password, user.passwordHash.toString());
 
-            if (!crypto.timingSafeEqual(user.passwordHash, hashedPassword)) {
+            if (!isPasswordCorrect) {
                 return cb(null, false, { message: 'Incorrect username or password.' });
             }
 
             return cb(null, user);
-        });
+        } catch (e) {
+            cb(e);
+        }
     }));
 
     passport.serializeUser((user: any, cb: any) => {
@@ -58,21 +62,19 @@ export function configurePassport(context: ChatServer) {
 
     context.app.post('/chatapi/register', async (req, res, next) => {
         const { username, password } = req.body;
-        const salt = crypto.randomBytes(32);
-        crypto.pbkdf2(password, salt, 310000, 32, 'sha256', async (err, hashedPassword) => {
-            if (err) {
-                return next(err);
-            }
-            try {
-                await context.database.createUser(username, hashedPassword, salt);
 
-                passport.authenticate('local')(req, res, () => {
-                    res.redirect('/');
-                });
-            } catch (err) {
-                res.redirect('/?error=register');
-            }
-        });
+        const hashedPassword = await bcrypt.hash(password, 12);
+
+        try {
+            await context.database.createUser(username, Buffer.from(hashedPassword));
+
+            passport.authenticate('local')(req, res, () => {
+                res.redirect('/');
+            });
+        } catch (err) {
+            console.error(err);
+            res.redirect('/?error=register');
+        }
     });
 
     context.app.all('/chatapi/logout', (req, res, next) => {