Skip to content

Commit

Permalink
refactor: removed payload parsing from JWS.verify
Browse files Browse the repository at this point in the history
BREAKING CHANGE: JWS.verify returned payloads are now always buffers
BREAKING CHANGE: JWS.verify options `encoding` and `parse` were removed
  • Loading branch information
panva committed Sep 8, 2020
1 parent cadbd04 commit ba5c897
Show file tree
Hide file tree
Showing 40 changed files with 122 additions and 160 deletions.
23 changes: 10 additions & 13 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ JWS.verify(unsecuredJWS, anActualKey)
// code: 'ERR_JWK_KEY_SUPPORT'

JWS.verify(unsecuredJWS, None)
// foobar
// => verifies
```

---
Expand Down Expand Up @@ -1273,12 +1273,8 @@ Verifies the provided JWS in either serialization with a given `<JWK.Key>` or `<
algorithms available on the keys
- `complete`: `<boolean>` When true returns a complete object with the parsed headers and payload
instead of only the verified payload. **Default:** 'false'
- `parse`: `<boolean>` When true attempts to JSON.parse the payload and falls back to returning
the payload encoded using the specified encoding. When false returns the payload as a Buffer.
**Default:** 'true'
- `encoding`: `string` The encoding to use for the payload. **Default:** 'utf8'
- `crit`: `string[]` Array of Critical Header Parameter names to recognize. **Default:** '[]'
- Returns: `<string>` &vert; `<Object>`
- Returns: `<Buffer>`

<details>
<summary><em><strong>Example</strong></em> (Click to expand)</summary>
Expand Down Expand Up @@ -1312,35 +1308,36 @@ const general = { payload: 'eyJzdWIiOiJKb2huIERvZSJ9',
'R7e5ZUkgiZQLh8JagoCbwAY21e9A-Y0rhUGQkhihLOvIU8JG2AyZ9zROOUICaUucf8NQKc2dEaIKdRCXy-fDdQ' } ] }

JWS.verify(compact, key)
// { sub: 'John Doe' }
// <Buffer ...>

JWS.verify(flattened, key2)
// Thrown:
// JWSVerificationFailed: signature verification failed

JWS.verify(compact, key, { complete: true })
// { payload: { sub: 'John Doe' }, protected: { alg: 'HS256' }, key: OctKey {} }
// { payload: <Buffer ...>, protected: { alg: 'HS256' }, key: OctKey {} }

JWS.verify(flattened, key, { algorithms: ['PS256'] })
// JOSEAlgNotWhitelisted: alg not whitelisted

JWS.verify(general, key)
// { sub: 'John Doe' }
// <Buffer ...>
JWS.verify(general, key2)
// { sub: 'John Doe' }
// <Buffer ...>

JWS.verify(general, key, { complete: true })
// { payload: { sub: 'John Doe' },
// { payload: <Buffer ...>,
// protected: { alg: 'HS256' },
// header: { foo: 'bar' },
// key: : OctKey {} } <- key
JWS.verify(general, key2, { complete: true })
// { payload: { sub: 'John Doe' },
// { payload: <Buffer ...>,
// protected: { alg: 'HS512' },
// header: { foo: 'baz' },
// key: : OctKey {} } <- key2
const keystore = new JWKS.KeyStore(key)
JWS.verify(general, keystore, { complete: true })
// { payload: { sub: 'John Doe' },
// { payload: <Buffer ...>,
// protected: { alg: 'HS256' },
// header: { foo: 'bar' },
// key: : OctKey {} } <- key that matched in the keystore
Expand Down
16 changes: 7 additions & 9 deletions lib/jws/verify.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const SINGLE_RECIPIENT = new Set(['compact', 'flattened', 'preparsed'])
/*
* @public
*/
const jwsVerify = (skipDisjointCheck, serialization, jws, key, { crit = [], complete = false, algorithms, parse = true, encoding = 'utf8' } = {}) => {
const jwsVerify = (skipDisjointCheck, serialization, jws, key, { crit = [], complete = false, algorithms } = {}) => {
key = getKey(key, true)

if (algorithms !== undefined && (!Array.isArray(algorithms) || algorithms.some(s => typeof s !== 'string' || !s))) {
Expand Down Expand Up @@ -113,7 +113,7 @@ const jwsVerify = (skipDisjointCheck, serialization, jws, key, { crit = [], comp
const errs = []
for (const key of keys) {
try {
return jwsVerify(true, serialization, jws, key, { crit, complete, encoding, parse, algorithms: algorithms ? [...algorithms] : undefined })
return jwsVerify(true, serialization, jws, key, { crit, complete, algorithms: algorithms ? [...algorithms] : undefined })
} catch (err) {
errs.push(err)
continue
Expand Down Expand Up @@ -159,12 +159,10 @@ const jwsVerify = (skipDisjointCheck, serialization, jws, key, { crit = [], comp
throw new errors.JWSVerificationFailed()
}

if (!combinedHeader.crit || !combinedHeader.crit.includes('b64') || combinedHeader.b64) {
if (parse) {
payload = decoded ? decoded.payload : base64url.JSON.decode.try(payload, encoding)
} else {
payload = base64url.decodeToBuffer(payload)
}
if (combinedHeader.b64 === false) {
payload = Buffer.from(payload)
} else {
payload = base64url.decodeToBuffer(payload)
}

if (complete) {
Expand All @@ -182,7 +180,7 @@ const jwsVerify = (skipDisjointCheck, serialization, jws, key, { crit = [], comp
const errs = []
for (const recipient of signatures) {
try {
return jwsVerify(false, 'flattened', { ...root, ...recipient }, key, { crit, complete, encoding, parse, algorithms: algorithms ? [...algorithms] : undefined })
return jwsVerify(false, 'flattened', { ...root, ...recipient }, key, { crit, complete, algorithms: algorithms ? [...algorithms] : undefined })
} catch (err) {
errs.push(err)
continue
Expand Down
12 changes: 6 additions & 6 deletions test/cookbook/4_1.rsa_v15_signature.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,28 @@ test(`${recipe.title} - general sign`, t => {
})

test(`${recipe.title} - compact verify (using key)`, t => {
t.is(JWS.verify(recipe.output.compact, key), payload)
t.deepEqual(JWS.verify(recipe.output.compact, key), payload)
})

test(`${recipe.title} - flattened verify (using key)`, t => {
t.is(JWS.verify(recipe.output.json_flat, key), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, key), payload)
})

test(`${recipe.title} - general verify (using key)`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})

;[keystoreMatchOne, keystoreMatchMore].forEach((keystore, i) => {
test(`${recipe.title} - compact verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.compact, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.compact, keystore), payload)
})

test(`${recipe.title} - flattened verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json_flat, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, keystore), payload)
})

test(`${recipe.title} - general verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})
})

Expand Down
18 changes: 9 additions & 9 deletions test/cookbook/4_2.rsa-pss_signature.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,44 +17,44 @@ const keystoreMatchNone = new KeyStore(generateSync(key.kty), generateSync(key.k
test(`${recipe.title} - compact sign (random)`, t => {
const res = JWS.sign(payload, key, header)
verifiers.compact(t, res, recipe.output.compact)
t.is(JWS.verify(res, key), payload)
t.deepEqual(JWS.verify(res, key), payload)
})

test(`${recipe.title} - flattened sign (random)`, t => {
const res = JWS.sign.flattened(payload, key, header)
verifiers.flattened(t, res, recipe.output.json_flat)
t.is(JWS.verify(res, key), payload)
t.deepEqual(JWS.verify(res, key), payload)
})

test(`${recipe.title} - general sign (random)`, t => {
const res = JWS.sign.general(payload, key, header)
verifiers.general(t, res, recipe.output.json)
t.is(JWS.verify(res, key), payload)
t.deepEqual(JWS.verify(res, key), payload)
})

test(`${recipe.title} - compact verify`, t => {
t.is(JWS.verify(recipe.output.compact, key), payload)
t.deepEqual(JWS.verify(recipe.output.compact, key), payload)
})

test(`${recipe.title} - flattened verify`, t => {
t.is(JWS.verify(recipe.output.json_flat, key), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, key), payload)
})

test(`${recipe.title} - general verify`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})

;[keystoreMatchOne, keystoreMatchMore].forEach((keystore, i) => {
test(`${recipe.title} - compact verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.compact, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.compact, keystore), payload)
})

test(`${recipe.title} - flattened verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json_flat, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, keystore), payload)
})

test(`${recipe.title} - general verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})
})

Expand Down
18 changes: 9 additions & 9 deletions test/cookbook/4_3.ecdsa_signature.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,44 +17,44 @@ const keystoreMatchNone = new KeyStore(generateSync(key.kty), generateSync(key.k
test(`${recipe.title} - compact sign (random)`, t => {
const res = JWS.sign(payload, key, header)
verifiers.compact(t, res, recipe.output.compact)
t.is(JWS.verify(res, key), payload)
t.deepEqual(JWS.verify(res, key), payload)
})

test(`${recipe.title} - flattened sign (random)`, t => {
const res = JWS.sign.flattened(payload, key, header)
verifiers.flattened(t, res, recipe.output.json_flat)
t.is(JWS.verify(res, key), payload)
t.deepEqual(JWS.verify(res, key), payload)
})

test(`${recipe.title} - general sign (random)`, t => {
const res = JWS.sign.general(payload, key, header)
verifiers.general(t, res, recipe.output.json)
t.is(JWS.verify(res, key), payload)
t.deepEqual(JWS.verify(res, key), payload)
})

test(`${recipe.title} - compact verify`, t => {
t.is(JWS.verify(recipe.output.compact, key), payload)
t.deepEqual(JWS.verify(recipe.output.compact, key), payload)
})

test(`${recipe.title} - flattened verify`, t => {
t.is(JWS.verify(recipe.output.json_flat, key), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, key), payload)
})

test(`${recipe.title} - general verify`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})

;[keystoreMatchOne, keystoreMatchMore].forEach((keystore, i) => {
test(`${recipe.title} - compact verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.compact, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.compact, keystore), payload)
})

test(`${recipe.title} - flattened verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json_flat, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, keystore), payload)
})

test(`${recipe.title} - general verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})
})

Expand Down
12 changes: 6 additions & 6 deletions test/cookbook/4_4.hmac-sha2_integrity_protection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,28 @@ test(`${recipe.title} - general sign`, t => {
})

test(`${recipe.title} - compact verify`, t => {
t.is(JWS.verify(recipe.output.compact, key), payload)
t.deepEqual(JWS.verify(recipe.output.compact, key), payload)
})

test(`${recipe.title} - flattened verify`, t => {
t.is(JWS.verify(recipe.output.json_flat, key), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, key), payload)
})

test(`${recipe.title} - general verify`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})

;[keystoreMatchOne, keystoreMatchMore].forEach((keystore, i) => {
test(`${recipe.title} - compact verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.compact, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.compact, keystore), payload)
})

test(`${recipe.title} - flattened verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json_flat, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, keystore), payload)
})

test(`${recipe.title} - general verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})
})

Expand Down
8 changes: 4 additions & 4 deletions test/cookbook/4_6.protecting_specific_header_fields.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ test(`${recipe.title} - general sign`, t => {
})

test(`${recipe.title} - flattened verify`, t => {
t.is(JWS.verify(recipe.output.json_flat, key), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, key), payload)
})

test(`${recipe.title} - general verify`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})

;[keystoreMatchOne, keystoreMatchMore].forEach((keystore, i) => {
test(`${recipe.title} - flattened verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json_flat, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, keystore), payload)
})

test(`${recipe.title} - general verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})
})

Expand Down
8 changes: 4 additions & 4 deletions test/cookbook/4_7.protecting_content_only.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ test(`${recipe.title} - general sign`, t => {
})

test(`${recipe.title} - flattened verify`, t => {
t.is(JWS.verify(recipe.output.json_flat, key), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, key), payload)
})

test(`${recipe.title} - general verify`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})

;[keystoreMatchOne, keystoreMatchMore].forEach((keystore, i) => {
test(`${recipe.title} - flattened verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json_flat, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json_flat, keystore), payload)
})

test(`${recipe.title} - general verify (using keystore ${i + 1}/2)`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})
})

Expand Down
6 changes: 3 additions & 3 deletions test/cookbook/4_8.multiple_signatures.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ test(`${recipe.title} - general sign`, t => {
t.deepEqual(result.signatures[2], recipe.output.json.signatures[2])

keys.forEach((key) => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})
})

keys.forEach((key, i) => {
test(`${recipe.title} - general verify - key ${i + 1}`, t => {
t.is(JWS.verify(recipe.output.json, key), payload)
t.deepEqual(JWS.verify(recipe.output.json, key), payload)
})
})

test(`${recipe.title} - general verify - keystore`, t => {
t.is(JWS.verify(recipe.output.json, keystore), payload)
t.deepEqual(JWS.verify(recipe.output.json, keystore), payload)
})

test(`${recipe.title} - general verify (failing)`, t => {
Expand Down
2 changes: 1 addition & 1 deletion test/cookbook/recipes/4_1.rsa_v15_signature.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
title: 'RSA v1.5 Signature',
input: {
payload: 'It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.',
payload: Buffer.from('It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.'),
key: {
kty: 'RSA',
kid: 'bilbo.baggins@hobbiton.example',
Expand Down
2 changes: 1 addition & 1 deletion test/cookbook/recipes/4_2.rsa-pss_signature.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
title: 'RSA-PSS Signature',
input: {
payload: 'It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.',
payload: Buffer.from('It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.'),
key: {
kty: 'RSA',
kid: 'bilbo.baggins@hobbiton.example',
Expand Down
2 changes: 1 addition & 1 deletion test/cookbook/recipes/4_3.ecdsa_signature.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
title: 'ECDSA Signature',
input: {
payload: 'It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.',
payload: Buffer.from('It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.'),
key: {
kty: 'EC',
kid: 'bilbo.baggins@hobbiton.example',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
title: 'HMAC-SHA2 Integrity Protection',
input: {
payload: 'It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.',
payload: Buffer.from('It’s a dangerous business, Frodo, going out your door. You step onto the road, and if you don\'t keep your feet, there’s no knowing where you might be swept off to.'),
key: {
kty: 'oct',
kid: '018c0ae5-4d9b-471b-bfd6-eef314bc7037',
Expand Down

0 comments on commit ba5c897

Please sign in to comment.