Skip to content

Commit 29a5ff5

Browse files
cannikinjtoar
andcommitted
Update crypto library, CryptoJS CVE & deprecation (#9350)
So CryptoJS just dropped a bomb: everything they do by default is not as strong as it could be. Oh and by the way, the entire library is now deprecated. :( GHSA-xwcq-pm8m-c4vf Unfortunately we can't just upgrade to the latest release 4.2.0 because the hashing algorithm has changed, and a user would no longer be able to login: the default hash generated by CryptoJS 4.2.0 won't match the hash generated by CryptoJS 4.1.0. Note that this is only an issue if someone got the contents of your database and wanted to figure out user passwords (but it still cost [$45,000 per password](https://eprint.iacr.org/2020/014.pdf) apparently?). In the wake of this CVE we're going to convert dbAuth to use the built-in `node:crypto` library instead, with more sensible default configuration. There are two areas where we use the crypto libs: 1. Hashing the user's password to store in the DB and compare on login 2. Encrypting/decrypting the session data in a cookie We're going to do this in a non-breaking way by supporting *both* the original CryptoJS-derived values, and the new `node:crypto` ones. The alternative would be to require every user to change their password, which seems like a non-starter. 1. On signup, store the hashedPassword using the new `node:crypto` algorithm 2. On login, compare the user's hashedPassword using the `node:crypto` algorithm: * If a match is found, user is logged in * If a match fails, fall back to the original CryptoJS algorithm (but using the `node:crypto` implementation) * If a match is found, update the `hashedPassword` in the database to the new algorithm, user is logged in * If a match is still not found, the user entered the wrong password. Likewise for cookies and login: 1. When encrypting the user's session, always use the new `node:crypto` algorithm 2. When decrypting the user's session, first try with `node:crypto` * If decrypting works, user is logged in * If decrypting fails, try the older CryptoJS algorithm ([I haven't figured how](brix/crypto-js#468) to use `node:crypto` to decrypt something that was encrypted with CryptoJS yet, so we'll need to keep the dependency on CryptoJS around for now) * If decrypting works, re-encrypt the cookie using the new `node:crypto` algorithm, user is logged in * If decrypting still fails, the session is invalid (someone tampered with the cookie) so log them out We could announce in the Release Notes that if a platform wants the absolute safest route, they should change their `SESSION_SECRET` *and* have users change their password if, for example, they suspect that their database may have been compromised before our release. The next most secure thing would be to just change `SESSION_SECRET` which would log everyone out, and on next login their password will get re-hashed with the new algorithm. But the default for most people will probably be to just go about business as usual, and as time goes by more and more users' passwords will be re-hashed on login. Related to #9337 #9338 #9339 #9340 --------- Co-authored-by: Dominic Saadi <[email protected]>
1 parent 50e3822 commit 29a5ff5

File tree

18 files changed

+581
-316
lines changed

18 files changed

+581
-316
lines changed

packages/auth-providers/dbAuth/api/package.json

-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
"@redwoodjs/project-config": "6.3.2",
2727
"base64url": "3.0.1",
2828
"core-js": "3.32.2",
29-
"crypto-js": "4.1.1",
3029
"md5": "2.3.0",
3130
"uuid": "9.0.0"
3231
},
@@ -35,7 +34,6 @@
3534
"@babel/core": "^7.22.20",
3635
"@redwoodjs/api": "6.3.2",
3736
"@simplewebauthn/server": "7.3.1",
38-
"@types/crypto-js": "4.1.1",
3937
"@types/md5": "2.3.2",
4038
"@types/uuid": "9.0.2",
4139
"jest": "29.7.0",

packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts

+69-29
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type {
1313
} from '@simplewebauthn/typescript-types'
1414
import type { APIGatewayProxyEvent, Context as LambdaContext } from 'aws-lambda'
1515
import base64url from 'base64url'
16-
import CryptoJS from 'crypto-js'
1716
import md5 from 'md5'
1817
import { v4 as uuidv4 } from 'uuid'
1918

@@ -24,11 +23,15 @@ import * as DbAuthError from './errors'
2423
import {
2524
cookieName,
2625
decryptSession,
26+
encryptSession,
2727
extractCookie,
2828
getSession,
2929
hashPassword,
30+
legacyHashPassword,
31+
isLegacySession,
3032
hashToken,
3133
webAuthnSession,
34+
extractHashingOptions,
3235
} from './shared'
3336

3437
type SetCookieHeader = { 'set-cookie': string }
@@ -569,11 +572,15 @@ export class DbAuthHandler<
569572
async getToken() {
570573
try {
571574
const user = await this._getCurrentUser()
575+
let headers = {}
572576

573-
// need to return *something* for our existing Authorization header stuff
574-
// to work, so return the user's ID in case we can use it for something
575-
// in the future
576-
return [user[this.options.authFields.id]]
577+
// if the session was encrypted with the old algorithm, re-encrypt it
578+
// with the new one
579+
if (isLegacySession(this.cookie)) {
580+
headers = this._loginResponse(user)[1]
581+
}
582+
583+
return [user[this.options.authFields.id], headers]
577584
} catch (e: any) {
578585
if (e instanceof DbAuthError.NotLoggedInError) {
579586
return this._logoutResponse()
@@ -636,12 +643,16 @@ export class DbAuthHandler<
636643
}
637644

638645
let user = await this._findUserByToken(resetToken as string)
639-
const [hashedPassword] = hashPassword(password, user.salt)
646+
const [hashedPassword] = hashPassword(password, {
647+
salt: user.salt,
648+
})
649+
const [legacyHashedPassword] = legacyHashPassword(password, user.salt)
640650

641651
if (
642-
!(this.options.resetPassword as ResetPasswordFlowOptions)
652+
(!(this.options.resetPassword as ResetPasswordFlowOptions)
643653
.allowReusedPassword &&
644-
user.hashedPassword === hashedPassword
654+
user.hashedPassword === hashedPassword) ||
655+
user.hashedPassword === legacyHashedPassword
645656
) {
646657
throw new DbAuthError.ReusedPasswordError(
647658
(
@@ -1154,21 +1165,16 @@ export class DbAuthHandler<
11541165
return meta
11551166
}
11561167

1157-
// encrypts a string with the SESSION_SECRET
1158-
_encrypt(data: string) {
1159-
return CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET as string)
1160-
}
1161-
11621168
// returns the set-cookie header to be returned in the request (effectively
11631169
// creates the session)
11641170
_createSessionHeader<TIdType = any>(
11651171
data: DbAuthSession<TIdType>,
11661172
csrfToken: string
11671173
): SetCookieHeader {
11681174
const session = JSON.stringify(data) + ';' + csrfToken
1169-
const encrypted = this._encrypt(session)
1175+
const encrypted = encryptSession(session)
11701176
const cookie = [
1171-
`${cookieName(this.options.cookie?.name)}=${encrypted.toString()}`,
1177+
`${cookieName(this.options.cookie?.name)}=${encrypted}`,
11721178
...this._cookieAttributes({ expires: this.sessionExpiresDate }),
11731179
].join(';')
11741180

@@ -1279,19 +1285,56 @@ export class DbAuthHandler<
12791285
)
12801286
}
12811287

1282-
// is password correct?
1283-
const [hashedPassword, _salt] = hashPassword(
1284-
password,
1285-
user[this.options.authFields.salt]
1288+
await this._verifyPassword(user, password)
1289+
return user
1290+
}
1291+
1292+
// extracts scrypt strength options from hashed password (if present) and
1293+
// compares the hashed plain text password just submitted using those options
1294+
// with the one in the database. Falls back to the legacy CryptoJS algorihtm
1295+
// if no options are present.
1296+
async _verifyPassword(user: Record<string, unknown>, password: string) {
1297+
const options = extractHashingOptions(
1298+
user[this.options.authFields.hashedPassword] as string
12861299
)
1287-
if (hashedPassword === user[this.options.authFields.hashedPassword]) {
1288-
return user
1300+
1301+
if (Object.keys(options).length) {
1302+
// hashed using the node:crypto algorithm
1303+
const [hashedPassword] = hashPassword(password, {
1304+
salt: user[this.options.authFields.salt] as string,
1305+
options,
1306+
})
1307+
1308+
if (hashedPassword === user[this.options.authFields.hashedPassword]) {
1309+
return user
1310+
}
12891311
} else {
1290-
throw new DbAuthError.IncorrectPasswordError(
1291-
username,
1292-
(this.options.login as LoginFlowOptions)?.errors?.incorrectPassword
1312+
// fallback to old CryptoJS hashing
1313+
const [legacyHashedPassword] = legacyHashPassword(
1314+
password,
1315+
user[this.options.authFields.salt] as string
12931316
)
1317+
1318+
if (
1319+
legacyHashedPassword === user[this.options.authFields.hashedPassword]
1320+
) {
1321+
const [newHashedPassword] = hashPassword(password, {
1322+
salt: user[this.options.authFields.salt] as string,
1323+
})
1324+
1325+
// update user's hash to the new algorithm
1326+
await this.dbAccessor.update({
1327+
where: { id: user.id },
1328+
data: { [this.options.authFields.hashedPassword]: newHashedPassword },
1329+
})
1330+
return user
1331+
}
12941332
}
1333+
1334+
throw new DbAuthError.IncorrectPasswordError(
1335+
user[this.options.authFields.username] as string,
1336+
(this.options.login as LoginFlowOptions)?.errors?.incorrectPassword
1337+
)
12951338
}
12961339

12971340
// gets the user from the database and returns only its ID
@@ -1351,8 +1394,7 @@ export class DbAuthHandler<
13511394
)
13521395
}
13531396

1354-
// if we get here everything is good, call the app's signup handler and let
1355-
// them worry about scrubbing data and saving to the DB
1397+
// if we get here everything is good, call the app's signup handler
13561398
const [hashedPassword, salt] = hashPassword(password)
13571399
const newUser = await (this.options.signup as SignupFlowOptions).handler({
13581400
username,
@@ -1406,9 +1448,7 @@ export class DbAuthHandler<
14061448
] {
14071449
const sessionData = { id: user[this.options.authFields.id] }
14081450

1409-
// TODO: this needs to go into graphql somewhere so that each request makes
1410-
// a new CSRF token and sets it in both the encrypted session and the
1411-
// csrf-token header
1451+
// TODO: this needs to go into graphql somewhere so that each request makes a new CSRF token and sets it in both the encrypted session and the csrf-token header
14121452
const csrfToken = DbAuthHandler.CSRF_TOKEN
14131453

14141454
return [

packages/auth-providers/dbAuth/api/src/__tests__/DbAuthHandler.test.js

+78-16
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1+
import crypto from 'node:crypto'
12
import path from 'node:path'
23

3-
import CryptoJS from 'crypto-js'
4-
54
import { DbAuthHandler } from '../DbAuthHandler'
65
import * as dbAuthError from '../errors'
76
import { hashToken } from '../shared'
@@ -81,10 +80,10 @@ const db = new DbMock(['user', 'userCredential'])
8180

8281
const UUID_REGEX =
8382
/\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b/
84-
const SET_SESSION_REGEX = /^session=[a-zA-Z0-9+=/]+;/
83+
const SET_SESSION_REGEX = /^session=[a-zA-Z0-9+=/]|[a-zA-Z0-9+=/]+;/
8584
const UTC_DATE_REGEX = /\w{3}, \d{2} \w{3} \d{4} [\d:]{8} GMT/
8685
const LOGOUT_COOKIE = 'session=;Expires=Thu, 01 Jan 1970 00:00:00 GMT'
87-
86+
const SESSION_SECRET = '540d03ebb00b441f8f7442cbc39958ad'
8887
const FIXTURE_PATH = path.resolve(
8988
__dirname,
9089
'../../../../../../__fixtures__/example-todo-main'
@@ -102,9 +101,10 @@ const createDbUser = async (attributes = {}) => {
102101
return await db.user.create({
103102
data: {
104103
104+
// default hashedPassword is from `node:crypto`
105105
hashedPassword:
106-
'0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba',
107-
salt: '2ef27f4073c603ba8b7807c6de6d6a89',
106+
'230847bea5154b6c7d281d09593ad1be26fa03a93c04a73bcc2b608c073a8213|16384|8|1',
107+
salt: 'ba8b7807c6de6d6a892ef27f4073c603',
108108
...attributes,
109109
},
110110
})
@@ -119,7 +119,16 @@ const expectLoggedInResponse = (response) => {
119119
}
120120

121121
const encryptToCookie = (data) => {
122-
return `session=${CryptoJS.AES.encrypt(data, process.env.SESSION_SECRET)}`
122+
const iv = crypto.randomBytes(16)
123+
const cipher = crypto.createCipheriv(
124+
'aes-256-cbc',
125+
SESSION_SECRET.substring(0, 32),
126+
iv
127+
)
128+
let encryptedSession = cipher.update(data, 'utf-8', 'base64')
129+
encryptedSession += cipher.final('base64')
130+
131+
return `session=${encryptedSession}|${iv.toString('base64')}`
123132
}
124133

125134
let event, context, options
@@ -129,7 +138,7 @@ describe('dbAuth', () => {
129138
// hide deprecation warnings during test
130139
jest.spyOn(console, 'warn').mockImplementation(() => {})
131140
// encryption key so results are consistent regardless of settings in .env
132-
process.env.SESSION_SECRET = 'nREjs1HPS7cFia6tQHK70EWGtfhOgbqJQKsHQz3S'
141+
process.env.SESSION_SECRET = SESSION_SECRET
133142
delete process.env.DBAUTH_COOKIE_DOMAIN
134143

135144
event = {
@@ -581,7 +590,7 @@ describe('dbAuth', () => {
581590
event = {
582591
headers: {
583592
cookie:
584-
'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx',
593+
'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w==',
585594
},
586595
}
587596
const dbAuth = new DbAuthHandler(event, context, options)
@@ -614,7 +623,7 @@ describe('dbAuth', () => {
614623
event.body = JSON.stringify({ method: 'logout' })
615624
event.httpMethod = 'GET'
616625
event.headers.cookie =
617-
'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx'
626+
'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w=='
618627
const dbAuth = new DbAuthHandler(event, context, options)
619628
const response = await dbAuth.invoke()
620629

@@ -625,7 +634,7 @@ describe('dbAuth', () => {
625634
event.body = JSON.stringify({ method: 'foobar' })
626635
event.httpMethod = 'POST'
627636
event.headers.cookie =
628-
'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx'
637+
'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w=='
629638
const dbAuth = new DbAuthHandler(event, context, options)
630639
const response = await dbAuth.invoke()
631640

@@ -636,7 +645,7 @@ describe('dbAuth', () => {
636645
event.body = JSON.stringify({ method: 'logout' })
637646
event.httpMethod = 'POST'
638647
event.headers.cookie =
639-
'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx'
648+
'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w=='
640649
const dbAuth = new DbAuthHandler(event, context, options)
641650
dbAuth.logout = jest.fn(() => {
642651
throw Error('Logout error')
@@ -674,7 +683,7 @@ describe('dbAuth', () => {
674683
event.body = JSON.stringify({ method: 'logout' })
675684
event.httpMethod = 'POST'
676685
event.headers.cookie =
677-
'session=U2FsdGVkX1/zRHVlEQhffsOufy7VLRAR6R4gb818vxblQQJFZI6W/T8uzxNUbQMx'
686+
'session=ko6iXKV11DSjb6kFJ4iwcf1FEqa5wPpbL1sdtKiV51Y=|cQaYkOPG/r3ILxWiFiz90w=='
678687
const dbAuth = new DbAuthHandler(event, context, options)
679688
dbAuth.logout = jest.fn(() => ['body', { foo: 'bar' }])
680689
const response = await dbAuth.invoke()
@@ -1595,6 +1604,27 @@ describe('dbAuth', () => {
15951604

15961605
expect(response[0]).toEqual('{"error":"User not found"}')
15971606
})
1607+
1608+
it('re-encrypts the session cookie if using the legacy algorithm', async () => {
1609+
await createDbUser({ id: 7 })
1610+
event = {
1611+
headers: {
1612+
// legacy session with { id: 7 } for userID
1613+
cookie: 'session=U2FsdGVkX1+s7seQJnVgGgInxuXm13l8VvzA3Mg2fYg=',
1614+
},
1615+
}
1616+
process.env.SESSION_SECRET =
1617+
'QKxN2vFSHAf94XYynK8LUALfDuDSdFowG6evfkFX8uszh4YZqhTiqEdshrhWbwbw'
1618+
1619+
const dbAuth = new DbAuthHandler(event, context, options)
1620+
const [userId, headers] = await dbAuth.getToken()
1621+
1622+
expect(userId).toEqual(7)
1623+
expect(headers['set-cookie']).toMatch(SET_SESSION_REGEX)
1624+
1625+
// set session back to default
1626+
process.env.SESSION_SECRET = SESSION_SECRET
1627+
})
15981628
})
15991629

16001630
describe('When a developer has set GraphiQL headers to mock a session cookie', () => {
@@ -2168,11 +2198,11 @@ describe('dbAuth', () => {
21682198
`Expires=${dbAuth.sessionExpiresDate}`
21692199
)
21702200
// can't really match on the session value since it will change on every render,
2171-
// due to CSRF token generation but we can check that it contains a only the
2172-
// characters that would be returned by the hash function
2201+
// due to CSRF token generation but we can check that it contains only the
2202+
// characters that would be returned by the encrypt function
21732203
expect(headers['set-cookie']).toMatch(SET_SESSION_REGEX)
21742204
// and we can check that it's a certain number of characters
2175-
expect(headers['set-cookie'].split(';')[0].length).toEqual(72)
2205+
expect(headers['set-cookie'].split(';')[0].length).toEqual(77)
21762206
})
21772207
})
21782208

@@ -2335,6 +2365,38 @@ describe('dbAuth', () => {
23352365

23362366
expect(user.id).toEqual(dbUser.id)
23372367
})
2368+
2369+
it('returns the user if password is hashed with legacy algorithm', async () => {
2370+
const dbUser = await createDbUser({
2371+
// CryptoJS hashed password
2372+
hashedPassword:
2373+
'0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba',
2374+
salt: '2ef27f4073c603ba8b7807c6de6d6a89',
2375+
})
2376+
const dbAuth = new DbAuthHandler(event, context, options)
2377+
const user = await dbAuth._verifyUser(dbUser.email, 'password')
2378+
2379+
expect(user.id).toEqual(dbUser.id)
2380+
})
2381+
2382+
it('updates the user hashPassword to the new algorithm', async () => {
2383+
const dbUser = await createDbUser({
2384+
// CryptoJS hashed password
2385+
hashedPassword:
2386+
'0c2b24e20ee76a887eac1415cc2c175ff961e7a0f057cead74789c43399dd5ba',
2387+
salt: '2ef27f4073c603ba8b7807c6de6d6a89',
2388+
})
2389+
const dbAuth = new DbAuthHandler(event, context, options)
2390+
await dbAuth._verifyUser(dbUser.email, 'password')
2391+
const user = await db.user.findFirst({ where: { id: dbUser.id } })
2392+
2393+
// password now hashed by node:crypto
2394+
expect(user.hashedPassword).toEqual(
2395+
'f20d69d478fa1afc85057384e21bd457a76b23b23e2a94f5bd982976f700a552|16384|8|1'
2396+
)
2397+
// salt should remain the same
2398+
expect(user.salt).toEqual('2ef27f4073c603ba8b7807c6de6d6a89')
2399+
})
23382400
})
23392401

23402402
describe('_getCurrentUser()', () => {

0 commit comments

Comments
 (0)