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.

Sign up for GitHub

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

Convert Buffer and node crypto to web apis #11

Merged
merged 1 commit into from
Sep 8, 2024
Merged

Convert Buffer and node crypto to web apis #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 33 additions & 38 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/**
* This was copy/paste/modified/tested from https://npm.im/notp (MIT)
*/
import * as crypto from 'node:crypto'

import base32Encode from 'base32-encode'
import base32Decode from 'base32-decode'

Expand All @@ -11,7 +9,7 @@ import base32Decode from 'base32-decode'
// That said, if you're acting the role of both client and server and your TOTP
// is longer lived, you can definitely use a more secure algorithm like SHA256.
// Learn more: https://www.rfc-editor.org/rfc/rfc4226#page-25 (B.1. SHA-1 Status)
const DEFAULT_ALGORITHM = 'SHA1'
const DEFAULT_ALGORITHM = 'SHA-1'
Copy link
Member

Choose a reason for hiding this comment

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

Can you help me understand why it was important to change these algorithm names? OTP clients break when you include the hyphen:

image

cc @dev-xo have you seen issues with this change?

Copy link
Member

Choose a reason for hiding this comment

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

For now I'm just planning on doing this:

const otpUri = getTOTPAuthUri({
	...verification,
	// OTP clients break with the `-` in the algorithm name.
	algorithm: verification.algorithm.replaceAll('-', ''),
	accountName: user.email,
	issuer,
})

But the other thing is this change necessitates a data migration if people saved the algorithm in the database and that kinda stinks. Would rather revert this change if it's not actually important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

web crypto apis use this spelling

Copy link

@dev-xo dev-xo Oct 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No idea @kentcdodds (also no issues found for us).

I think the same changes were applied for remix-auth-totp if I'm not wrong, although the Strategy does not store the algorithm in the database, so probably those refactors were a bit safer for us.

Feel free to take the changes/actions you think @epic-web/totp may require Kent, as those probably do not affect us, and if does, we can adapt the Strategy to those.

A question I have for @bitofbreeze: Do you know if SHA1 and SHA-1 (with hyphen) are the same, or if the algorithm is recognized in both formats?

Copy link
Contributor Author

@bitofbreeze bitofbreeze Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Here are the possible values https://developer.mozilla.org/en-US/docs/Web/API/HmacImportParams#instance_properties

const DEFAULT_CHAR_SET = '0123456789'
const DEFAULT_DIGITS = 6
const DEFAULT_WINDOW = 1
Expand All @@ -30,9 +28,9 @@ const DEFAULT_PERIOD = 30
* @param {string} [options.algorithm='SHA1'] - The algorithm to use for the
* HOTP. Defaults to 'SHA1'.
* @param {string} [options.charSet='0123456789'] - The character set to use, defaults to the numbers 0-9.
* @returns {string} The generated HOTP.
* @returns {Promise<string>} The generated HOTP.
*/
function generateHOTP(
async function generateHOTP(
secret,
{
counter = 0,
Expand All @@ -41,11 +39,16 @@ function generateHOTP(
charSet = DEFAULT_CHAR_SET,
} = {}
) {
const byteCounter = Buffer.from(intToBytes(counter))
const secretBuffer = Buffer.from(secret)
const hmac = crypto.createHmac(algorithm, secretBuffer)
const digest = hmac.update(byteCounter).digest('hex')
const hashBytes = hexToBytes(digest)
const byteCounter = intToBytes(counter)
const key = await crypto.subtle.importKey(
'raw',
secret,
{ name: 'HMAC', hash: algorithm },
false,
['sign']
)
const signature = await crypto.subtle.sign('HMAC', key, byteCounter)
const hashBytes = new Uint8Array(signature)
const offset = hashBytes[19] & 0xf
let hotpVal =
((hashBytes[offset] & 0x7f) << 24) |
Expand All @@ -67,7 +70,7 @@ function generateHOTP(
* configuration options.
*
* @param {string} otp - The OTP to verify.
* @param {Buffer} secret - The secret used to generate the HOTP.
* @param {ArrayBuffer} secret - The secret used to generate the HOTP.
* @param {Object} options - The configuration options for the HOTP.
* @param {number} [options.counter=0] - The counter value to use for the HOTP.
* Defaults to 0.
Expand All @@ -78,11 +81,11 @@ function generateHOTP(
* @param {string} [options.charSet='0123456789'] - The character set to use, defaults to the numbers 0-9.
* @param {number} [options.window=1] - The number of counter values to check
* before and after the current counter value. Defaults to 1.
* @returns {{delta: number}|null} An object with the `delta` property
* @returns {Promise<{delta: number}|null>} An object with the `delta` property
* indicating the number of counter values between the current counter value and
* the verified counter value, or `null` if the OTP could not be verified.
*/
function verifyHOTP(
async function verifyHOTP(
otp,
secret,
{
Expand All @@ -95,7 +98,7 @@ function verifyHOTP(
) {
for (let i = counter - window; i <= counter + window; ++i) {
if (
generateHOTP(secret, { counter: i, digits, algorithm, charSet }) === otp
await generateHOTP(secret, { counter: i, digits, algorithm, charSet }) === otp
) {
return { delta: i - counter }
}
Expand All @@ -117,18 +120,18 @@ function verifyHOTP(
* @param {string} [options.charSet='0123456789'] - The character set to use, defaults to the numbers 0-9.
* @param {string} [options.secret] The secret to use for the TOTP. It should be
* base32 encoded (you can use https://npm.im/thirty-two). Defaults to a random
* secret: base32Encode(crypto.randomBytes(10), 'RFC4648').
* @returns {{otp: string, secret: string, period: number, digits: number, algorithm: string, charSet: string}}
* secret: base32Encode(crypto.getRandomValues(new Uint8Array(10)), 'RFC4648').
* @returns {Promise<{otp: string, secret: string, period: number, digits: number, algorithm: string, charSet: string}>}
* The OTP, secret, and config options used to generate the OTP.
*/
export function generateTOTP({
export async function generateTOTP({
period = DEFAULT_PERIOD,
digits = DEFAULT_DIGITS,
algorithm = DEFAULT_ALGORITHM,
secret = base32Encode(crypto.randomBytes(10), 'RFC4648'),
secret = base32Encode(crypto.getRandomValues(new Uint8Array(10)), 'RFC4648'),
charSet = DEFAULT_CHAR_SET,
} = {}) {
const otp = generateHOTP(base32Decode(secret, 'RFC4648'), {
const otp = await generateHOTP(base32Decode(secret, 'RFC4648'), {
counter: getCounter(period),
digits,
algorithm,
Expand Down Expand Up @@ -191,11 +194,11 @@ export function getTOTPAuthUri({
* @param {number} [options.window] The number of OTPs to check before and after
* the current OTP. Defaults to 1.
*
* @returns {{delta: number}|null} an object with "delta" which is the delta
* @returns {Promise<{delta: number}|null>} an object with "delta" which is the delta
* between the current OTP and the OTP that was verified, or null if the OTP is
* invalid.
*/
export function verifyTOTP({
export async function verifyTOTP({
otp,
secret,
period,
Expand All @@ -212,7 +215,7 @@ export function verifyTOTP({
return null
}

return verifyHOTP(otp, Buffer.from(decodedSecret), {
return verifyHOTP(otp, new Uint8Array(decodedSecret), {
counter: getCounter(period),
digits,
window,
Expand All @@ -225,23 +228,15 @@ export function verifyTOTP({
* Converts a number to a byte array.
*
* @param {number} num The number to convert to a byte array.
* @returns {number[]} The byte array representation of the number.
* @returns {Uint8Array} The byte array representation of the number.
*/
function intToBytes(num) {
const buffer = Buffer.alloc(8)
// eslint-disable-next-line no-undef
buffer.writeBigInt64BE(BigInt(num))
return [...buffer]
}

/**
* Converts a hexadecimal string to a byte array.
*
* @param {string} hex The hexadecimal string to convert to a byte array.
* @returns {number[]} The byte array representation of the hexadecimal string.
*/
function hexToBytes(hex) {
return [...Buffer.from(hex, 'hex')]
const arr = new Uint8Array(8)
for (let i = 7; i >= 0; i--) {
arr[i] = num & 0xff
num = num >> 8
}
return arr
}

/**
Expand All @@ -255,4 +250,4 @@ function getCounter(period = DEFAULT_PERIOD) {
const now = new Date().getTime()
const counter = Math.floor(now / 1000 / period)
return counter
}
}
69 changes: 35 additions & 34 deletions index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@ import { test } from 'node:test'
import base32Encode from 'base32-encode'
import { generateTOTP, getTOTPAuthUri, verifyTOTP } from './index.js'

test('OTP can be generated and verified', () => {
const { secret, otp, algorithm, period, digits } = generateTOTP()
assert.strictEqual(algorithm, 'SHA1')
test('OTP can be generated and verified', async () => {
const { secret, otp, algorithm, period, digits } = await generateTOTP()

assert.strictEqual(algorithm, 'SHA-1')
assert.strictEqual(period, 30)
assert.strictEqual(digits, 6)
const result = verifyTOTP({ otp, secret })
const result = await verifyTOTP({ otp, secret })
assert.deepStrictEqual(result, { delta: 0 })
})

test('options can be customized', () => {
test('options can be customized', async () => {
const options = {
algorithm: 'SHA256',
algorithm: 'SHA-256',
period: 60,
digits: 8,
secret: base32Encode(
Expand All @@ -23,86 +24,86 @@ test('options can be customized', () => {
).toString(),
charSet: 'abcdef',
}
const { otp, ...config } = generateTOTP(options)
const { otp, ...config } = await generateTOTP(options)
assert.deepStrictEqual(config, options)
const result = verifyTOTP({ otp, ...config })
const result = await verifyTOTP({ otp, ...config })
assert.deepStrictEqual(result, { delta: 0 })
})

test('Verify TOTP within the specified time window', () => {
const { otp, secret } = generateTOTP()
const result = verifyTOTP({ otp, secret, window: 0 })
test('Verify TOTP within the specified time window', async () => {
const { otp, secret } = await generateTOTP()
const result = await verifyTOTP({ otp, secret, window: 0 })
assert.notStrictEqual(result, null)
})

test('Fail to verify an invalid TOTP', () => {
test('Fail to verify an invalid TOTP', async () => {
const secret = Math.random().toString()
const tooShortNumber = Math.random().toString().slice(2, 7)
const result = verifyTOTP({ otp: tooShortNumber, secret })
const result = await verifyTOTP({ otp: tooShortNumber, secret })
assert.strictEqual(result, null)
})

test('Fail to verify TOTP outside the specified time window', async () => {
const { otp, secret: key } = generateTOTP({ period: 0.0001 })
const { otp, secret: key } = await generateTOTP({ period: 0.0001 })
await new Promise((resolve) => setTimeout(resolve, 1))
const result = verifyTOTP({ otp, secret: key })
const result = await verifyTOTP({ otp, secret: key })
assert.strictEqual(result, null)
})

test('Clock drift is handled by window', async () => {
// super small period
const { otp, secret: key, period } = generateTOTP({ period: 0.0001 })
const { otp, secret: key, period } = await generateTOTP({ period: 0.0001 })
// waiting a tiny bit
await new Promise((resolve) => setTimeout(resolve, 1))
// super big window (to accomodate slow machines running this test)
const result = verifyTOTP({ otp, secret: key, window: 200, period })
const result = await verifyTOTP({ otp, secret: key, window: 200, period })
// should still validate
assert.notDeepStrictEqual(result, null)
})

test('Setting a different period config for generating and verifying will fail', () => {
test('Setting a different period config for generating and verifying will fail', async () => {
const desiredPeriod = 60
const { otp, secret, period } = generateTOTP({
const { otp, secret, period } = await generateTOTP({
period: desiredPeriod,
})
assert.strictEqual(period, desiredPeriod)
const result = verifyTOTP({ otp, secret, period: period + 1 })
const result = await verifyTOTP({ otp, secret, period: period + 1 })
assert.strictEqual(result, null)
})

test('Setting a different algo config for generating and verifying will fail', () => {
const desiredAlgo = 'SHA512'
const { otp, secret, algorithm } = generateTOTP({
test('Setting a different algo config for generating and verifying will fail', async () => {
const desiredAlgo = 'SHA-512'
const { otp, secret, algorithm } = await generateTOTP({
algorithm: desiredAlgo,
})
assert.strictEqual(algorithm, desiredAlgo)
const result = verifyTOTP({ otp, secret, algorithm: 'SHA1' })
const result = await verifyTOTP({ otp, secret, algorithm: 'SHA-1' })
assert.strictEqual(result, null)
})

test('Generating and verifying also works with the algorithm name alias', () => {
const desiredAlgo = 'SHA1'
const { otp, secret, algorithm } = generateTOTP({
test('Generating and verifying also works with the algorithm name alias', async () => {
const desiredAlgo = 'SHA-1'
const { otp, secret, algorithm } = await generateTOTP({
algorithm: desiredAlgo,
})
assert.strictEqual(algorithm, desiredAlgo)

const result = verifyTOTP({ otp, secret, algorithm: 'sha1' })
const result = await verifyTOTP({ otp, secret, algorithm: 'sha-1' })
assert.notStrictEqual(result, null)
})

test('Charset defaults to numbers', () => {
const { otp } = generateTOTP()
test('Charset defaults to numbers', async () => {
const { otp } = await generateTOTP()
assert.match(otp, /^[0-9]+$/)
})

test('Charset can be customized', () => {
const { otp } = generateTOTP({ charSet: 'abcdef' })
test('Charset can be customized', async () => {
const { otp } = await generateTOTP({ charSet: 'abcdef' })
assert.match(otp, /^[abcdef]+$/)
})

test('OTP Auth URI can be generated', () => {
const { otp: _otp, secret, ...totpConfig } = generateTOTP()
test('OTP Auth URI can be generated', async () => {
const { otp: _otp, secret, ...totpConfig } = await generateTOTP()
const issuer = Math.random().toString(16).slice(2)
const accountName = Math.random().toString(16).slice(2)
const uri = getTOTPAuthUri({
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"base32-encode": "^2.0.0"
},
"engines": {
"node": ">=18"
"node": ">=20"
},
"prettier": {
"semi": false,
Expand Down
Loading