Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password changes #2446

Merged
merged 19 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions backend/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { SERVER_INSTANCE } from './instance-keys.js'
import path from 'path'
import chalk from 'chalk'
import './database.js'
import { registrationKey, register, getChallenge, getContractSalt, updateContractSalt } from './zkppSalt.js'
import { registrationKey, register, getChallenge, getContractSalt, updateContractSalt, redeemSaltUpdateToken } from './zkppSalt.js'
import Bottleneck from 'bottleneck'

const MEGABYTE = 1048576 // TODO: add settings for these
Expand Down Expand Up @@ -116,7 +116,24 @@ route.POST('/event', {
}
}
}
const saltUpdateToken = request.headers['shelter-salt-update-token']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implements the atomic update using a token.

let updateSalts
if (saltUpdateToken) {
// If we've got a salt update token (i.e., a password change), fetch
// the username associated to the contract to see if they match, and
// then validate the token
const name = request.headers['shelter-name']
const namedContractID = name && await sbp('backend/db/lookupName', name)
if (namedContractID !== deserializedHEAD.contractID) {
throw new Error('Mismatched contract ID and name')
}
updateSalts = await redeemSaltUpdateToken(name, saltUpdateToken)
}
await sbp('backend/server/handleEntry', deserializedHEAD, request.payload)
// If it's a salt update, do it now after handling the message. This way
// we make it less likely that someone will end up locked out from their
// identity contract.
await updateSalts?.(deserializedHEAD.hash)
Comment on lines +120 to +136
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude:

There's a potential race condition between validating the token and applying the update. If multiple requests come in with the same token, the validation could succeed for both but only one should be allowed to update. The token validation and update should be atomic.

Is he full of it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially it could happen, however, not really, because the token only applies to one contract and we can't write to the same contract simultaneously (or shouldn't be able to), so handleEntry will throw.

if (deserializedHEAD.isFirstMessage) {
// Store attribution information
if (credentials?.billableContractID) {
Expand Down Expand Up @@ -821,7 +838,7 @@ route.GET('/zkpp/{name}/contract_hash', {
return Boom.internal('internal error')
})

route.POST('/zkpp/updatePasswordHash/{name}', {
route.POST('/zkpp/{name}/updatePasswordHash', {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed for consistency

validate: {
payload: Joi.object({
r: Joi.string().required(),
Expand All @@ -841,7 +858,7 @@ route.POST('/zkpp/updatePasswordHash/{name}', {
}
} catch (e) {
e.ip = req.headers['x-real-ip'] || req.info.remoteAddress
console.error(e, 'Error at POST /zkpp/updatePasswordHash/{name}: ' + e.message)
console.error(e, 'Error at POST /zkpp/{name}/updatePasswordHash: ' + e.message)
}

return Boom.internal('internal error')
Expand Down
85 changes: 68 additions & 17 deletions backend/zkppSalt.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import sbp from '@sbp/sbp'
import { randomBytes, timingSafeEqual } from 'crypto'
import nacl from 'tweetnacl'
import { base64ToBase64url, base64urlToBase64, boxKeyPair, computeCAndHc, encryptContractSalt, hash, hashRawStringArray, hashStringArray, parseRegisterSalt, randomNonce } from '~/shared/zkpp.js'
import { base64ToBase64url, base64urlToBase64, boxKeyPair, computeCAndHc, decryptSaltUpdate, encryptContractSalt, encryptSaltUpdate, hash, hashRawStringArray, hashStringArray, parseRegisterSalt, randomNonce } from '~/shared/zkpp.js'
import { AUTHSALT, CONTRACTSALT, SALT_LENGTH_IN_OCTETS, SU } from '~/shared/zkppConstants.js'

// used to encrypt salts in database
let recordSecret: string
// corresponds to the key for the keyed Hash function in "Log in / session establishment"
let challengeSecret: string
// corresponds to a component of s in Step 3 of "Salt registration"
let registrationSecret: string
// used to encrypt a stateless token for atomic hash updates
let hashUpdateSecret: string

// Input keying material used to derive various secret keys used in this
// protocol: recordSecret, challengeSecret and registrationSecret.
Expand Down Expand Up @@ -60,10 +63,23 @@ export const initZkpp = async () => {
recordSecret = Buffer.from(hashStringArray('private/recordSecret', IKM)).toString('base64')
challengeSecret = Buffer.from(hashStringArray('private/challengeSecret', IKM)).toString('base64')
registrationSecret = Buffer.from(hashStringArray('private/registrationSecret', IKM)).toString('base64')
hashUpdateSecret = Buffer.from(hashStringArray('private/hashUpdateSecret', IKM)).toString('base64')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used for encrypting the update token.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these strings like 'private/hashUpdateSecret' something that should be put in zkppConstants.js too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, since they're only used here for initialisation purposes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They also have no particular significance with regard to the protocol.

}

const maxAge = 30

const computeZkppSaltRecordId = async (contractID: string) => {
const recordId = `_private_rid_${contractID}`
const record = await sbp('chelonia/db/get', recordId)

if (!record) {
return null
}

const recordBuf = Buffer.concat([Buffer.from(contractID), Buffer.from(record)])
return hash(recordBuf)
}

const getZkppSaltRecord = async (contractID: string) => {
const recordId = `_private_rid_${contractID}`
const record = await sbp('chelonia/db/get', recordId)
Expand All @@ -85,17 +101,23 @@ const getZkppSaltRecord = async (contractID: string) => {
try {
const recordObj = JSON.parse(recordString)

if (!Array.isArray(recordObj) || recordObj.length !== 3 || !recordObj.reduce((acc, cv) => acc && typeof cv === 'string', true)) {
if (
!Array.isArray(recordObj) ||
(recordObj.length !== 3 && recordObj.length !== 4) ||
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because before the record was a triplet, and now it can be a triplet or a 4-tuple (with the 4th entry being a CID, if there's been a password update)

recordObj.slice(0, 3).some((r) => !r || typeof r !== 'string') ||
(recordObj[3] !== null && typeof recordObj[3] !== 'string')
) {
console.error('Error validating encrypted JSON object ' + recordId)
return null
}

const [hashedPassword, authSalt, contractSalt] = recordObj
const [hashedPassword, authSalt, contractSalt, cid] = recordObj

return {
hashedPassword,
authSalt,
contractSalt
contractSalt,
cid
}
} catch {
console.error('Error parsing encrypted JSON object ' + recordId)
Expand All @@ -105,11 +127,11 @@ const getZkppSaltRecord = async (contractID: string) => {
return null
}

const setZkppSaltRecord = async (contractID: string, hashedPassword: string, authSalt: string, contractSalt: string) => {
const setZkppSaltRecord = async (contractID: string, hashedPassword: string, authSalt: string, contractSalt: string, cid: ?string) => {
const recordId = `_private_rid_${contractID}`
const encryptionKey = hashStringArray('REK', contractID, recordSecret).slice(0, nacl.secretbox.keyLength)
const nonce = nacl.randomBytes(nacl.secretbox.nonceLength)
const recordPlaintext = JSON.stringify([hashedPassword, authSalt, contractSalt])
const recordPlaintext = JSON.stringify([hashedPassword, authSalt, contractSalt, cid])
const recordCiphertext = nacl.secretbox(Buffer.from(recordPlaintext), nonce, encryptionKey)
const recordBuf = Buffer.concat([nonce, recordCiphertext])
const record = base64ToBase64url(recordBuf.toString('base64'))
Expand Down Expand Up @@ -242,7 +264,7 @@ export const getContractSalt = async (contract: string, r: string, s: string, si
return false
}

const { hashedPassword, contractSalt } = record
const { hashedPassword, contractSalt, cid } = record

const c = contractSaltVerifyC(hashedPassword, r, s, hc)

Expand All @@ -251,10 +273,10 @@ export const getContractSalt = async (contract: string, r: string, s: string, si
throw new Error('getContractSalt: Bad challenge')
}

return encryptContractSalt(c, contractSalt)
return encryptContractSalt(c, JSON.stringify([contractSalt, cid]))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now return the contractSalt and the CID.

}

export const updateContractSalt = async (contract: string, r: string, s: string, sig: string, hc: string, encryptedArgs: string): Promise<boolean> => {
export const updateContractSalt = async (contract: string, r: string, s: string, sig: string, hc: string, encryptedArgs: string): Promise<boolean | string> => {
if (!verifyChallenge(contract, r, s, sig)) {
console.warn('update: Error validating challenge: ' + JSON.stringify({ contract, r, s, sig }))
throw new Error('update: Bad challenge')
Expand All @@ -266,7 +288,7 @@ export const updateContractSalt = async (contract: string, r: string, s: string,
console.error('update: Error obtaining ZKPP salt record for contract ID ' + contract)
return false
}
const { hashedPassword } = record
const { hashedPassword, contractSalt: oldContractSalt } = record

const c = contractSaltVerifyC(hashedPassword, r, s, hc)

Expand All @@ -275,7 +297,7 @@ export const updateContractSalt = async (contract: string, r: string, s: string,
throw new Error('update: Bad challenge')
}

const encryptionKey = hashRawStringArray('SU', c).slice(0, nacl.secretbox.keyLength)
const encryptionKey = hashRawStringArray(SU, c).slice(0, nacl.secretbox.keyLength)
const encryptedArgsBuf = Buffer.from(base64urlToBase64(encryptedArgs), 'base64')
const nonce = encryptedArgsBuf.slice(0, nacl.secretbox.nonceLength)
const encryptedArgsCiphertext = encryptedArgsBuf.slice(nacl.secretbox.nonceLength)
Expand All @@ -288,21 +310,50 @@ export const updateContractSalt = async (contract: string, r: string, s: string,
}

try {
const argsObj = JSON.parse(Buffer.from(args).toString())
const hashedPassword = Buffer.from(args).toString()

if (!Array.isArray(argsObj) || argsObj.length !== 3 || !argsObj.reduce((acc, cv) => acc && typeof cv === 'string', true)) {
console.error(`update: Error validating the encrypted arguments for contract ID ${contract} (${JSON.stringify({ r, s, hc })})`)
const recordId = await computeZkppSaltRecordId(contract)
if (!recordId) {
console.error(`update: Error obtaining record ID for contract ID ${contract}`)
return false
}

const [hashedPassword, authSalt, contractSalt] = argsObj
const authSalt = Buffer.from(hashStringArray(AUTHSALT, c)).slice(0, SALT_LENGTH_IN_OCTETS).toString('base64')
const contractSalt = Buffer.from(hashStringArray(CONTRACTSALT, c)).slice(0, SALT_LENGTH_IN_OCTETS).toString('base64')

await setZkppSaltRecord(contract, hashedPassword, authSalt, contractSalt)
const token = encryptSaltUpdate(
hashUpdateSecret,
recordId,
JSON.stringify([Date.now(), hashedPassword, authSalt, contractSalt])
)

return true
return encryptContractSalt(c, JSON.stringify([oldContractSalt, token]))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning true, we return the token for requesting an update. We also return the oldContractSalt because it's helpful for avoiding an extra request, as we need it to decrypt the old (current) IEK.

} catch {
console.error(`update: Error parsing encrypted arguments for contract ID ${contract} (${JSON.stringify({ r, s, hc })})`)
}

return false
}

export const redeemSaltUpdateToken = async (contract: string, token: string): Promise<(cid: ?string) => Promise<void>> => {
const recordId = await computeZkppSaltRecordId(contract)
if (!recordId) {
throw new Error('Record ID not found')
}

const decryptedToken = decryptSaltUpdate(
hashUpdateSecret,
recordId,
token
)

const [timestamp, hashedPassword, authSalt, contractSalt] = JSON.parse(decryptedToken)

if (timestamp < (Date.now() - 180e3)) {
throw new Error('ZKPP token expired')
}

return (cid: ?string) => {
return setZkppSaltRecord(contract, hashedPassword, authSalt, contractSalt, cid)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two-step process to allow validating the token first before making the change.

}
}
15 changes: 9 additions & 6 deletions backend/zkppSalt.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import should from 'should'
import initDB from './database.js'
import 'should-sinon'

import { AUTHSALT, CONTRACTSALT, CS, SALT_LENGTH_IN_OCTETS, SU } from '~/shared/zkppConstants.js'
import { registrationKey, register, getChallenge, getContractSalt, updateContractSalt } from './zkppSalt.js'

const saltsAndEncryptedHashedPassword = (p: string, secretKey: Uint8Array, hash: string) => {
const nonce = nacl.randomBytes(nacl.secretbox.nonceLength)
const dhKey = nacl.hash(nacl.box.before(Buffer.from(p, 'base64url'), secretKey))
const authSalt = Buffer.from(nacl.hash(Buffer.concat([nacl.hash(Buffer.from('AUTHSALT')), dhKey]))).slice(0, 18).toString('base64')
const contractSalt = Buffer.from(nacl.hash(Buffer.concat([nacl.hash(Buffer.from('CONTRACTSALT')), dhKey]))).slice(0, 18).toString('base64')
const authSalt = Buffer.from(nacl.hash(Buffer.concat([nacl.hash(Buffer.from(AUTHSALT)), dhKey]))).slice(0, SALT_LENGTH_IN_OCTETS).toString('base64')
const contractSalt = Buffer.from(nacl.hash(Buffer.concat([nacl.hash(Buffer.from(CONTRACTSALT)), dhKey]))).slice(0, SALT_LENGTH_IN_OCTETS).toString('base64')
const encryptionKey = nacl.hash(Buffer.from(authSalt + contractSalt)).slice(0, nacl.secretbox.keyLength)
const encryptedHashedPassword = Buffer.concat([nonce, nacl.secretbox(Buffer.from(hash), nonce, encryptionKey)]).toString('base64url')

Expand Down Expand Up @@ -75,8 +76,10 @@ describe('ZKPP Salt functions', () => {

const saltBuf = Buffer.from(salt, 'base64url')
const nonce = saltBuf.slice(0, nacl.secretbox.nonceLength)
const encryptionKey = nacl.hash(Buffer.concat([Buffer.from('CS'), c])).slice(0, nacl.secretbox.keyLength)
const retrievedContractSalt = Buffer.from(nacl.secretbox.open(saltBuf.slice(nacl.secretbox.nonceLength), nonce, encryptionKey)).toString()
const encryptionKey = nacl.hash(Buffer.concat([Buffer.from(CS), c])).slice(0, nacl.secretbox.keyLength)
const [retrievedContractSalt] = JSON.parse(
Buffer.from(nacl.secretbox.open(saltBuf.slice(nacl.secretbox.nonceLength), nonce, encryptionKey)).toString()
)
should(retrievedContractSalt).equal(contractSalt, 'mismatched contractSalt')
})

Expand All @@ -103,14 +106,14 @@ describe('ZKPP Salt functions', () => {
const c = nacl.hash(Buffer.concat([nacl.hash(Buffer.from(hash)), nacl.hash(ħ)]))
const hc = nacl.hash(c)

const encryptionKey = nacl.hash(Buffer.concat([Buffer.from('SU'), c])).slice(0, nacl.secretbox.keyLength)
const encryptionKey = nacl.hash(Buffer.concat([Buffer.from(SU), c])).slice(0, nacl.secretbox.keyLength)
const nonce = nacl.randomBytes(nacl.secretbox.nonceLength)

const encryptedArgsCiphertext = nacl.secretbox(Buffer.from(JSON.stringify(['a', 'b', 'c'])), nonce, encryptionKey)

const encryptedArgs = Buffer.concat([nonce, encryptedArgsCiphertext]).toString('base64url')

const updateRes = await updateContractSalt(contract, r, challenge.s, challenge.sig, Buffer.from(hc).toString('base64url'), encryptedArgs)
should(updateRes).equal(true, 'updateContractSalt should be successful')
should(!!updateRes).equal(true, 'updateContractSalt should be successful')
})
})
Loading