Skip to content

Commit

Permalink
feat(auth): validate content-digest header (#777)
Browse files Browse the repository at this point in the history
  • Loading branch information
njlie authored Nov 29, 2022
1 parent 6dfc5dc commit a5c36d7
Show file tree
Hide file tree
Showing 9 changed files with 204 additions and 79 deletions.
1 change: 1 addition & 0 deletions packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@koa/router": "^12.0.0",
"ajv": "^8.11.0",
"axios": "^0.27.2",
"httpbis-digest-headers": "github:interledger/httpbis-digest-headers",
"jose": "^4.9.0",
"knex": "^0.95",
"koa": "^2.13.4",
Expand Down
70 changes: 54 additions & 16 deletions packages/auth/src/signature/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { faker } from '@faker-js/faker'
import { importJWK } from 'jose'
import { v4 } from 'uuid'
import { Knex } from 'knex'
import { createContentDigestHeader } from 'httpbis-digest-headers'

import { createTestApp, TestContainer } from '../tests/app'
import { truncateTables } from '../tests/tableManager'
Expand Down Expand Up @@ -58,6 +59,8 @@ describe('Signature Service', (): void => {
).resolves.toBe(true)
})

const testRequestBody = { foo: 'bar' }

test.each`
title | withAuthorization | withRequestBody
${''} | ${true} | ${true}
Expand All @@ -71,14 +74,17 @@ describe('Signature Service', (): void => {
const headers = {
'Content-Type': 'application/json'
}
let expectedChallenge = `"@method": GET\n"@target-uri": /test\n"content-type": application/json\n`
let expectedChallenge = `"@method": GET\n"@target-uri": http://example.com/test\n"content-type": application/json\n`
const contentDigest = createContentDigestHeader(
JSON.stringify(testRequestBody),
['sha-512']
)

if (withRequestBody) {
sigInputHeader += ' "content-digest" "content-length"'
headers['Content-Digest'] = 'sha-256=:test-hash:'
headers['Content-Digest'] = contentDigest
headers['Content-Length'] = '1234'
expectedChallenge +=
'"content-digest": sha-256=:test-hash:\n"content-length": 1234\n'
expectedChallenge += `"content-digest": ${contentDigest}\n"content-length": 1234\n`
}

if (withAuthorization) {
Expand All @@ -98,13 +104,13 @@ describe('Signature Service', (): void => {
{
headers,
method: 'GET',
url: '/test'
url: 'example.com/test'
},
{},
deps
)

ctx.request.body = withRequestBody ? { foo: 'bar' } : {}
ctx.request.body = withRequestBody ? testRequestBody : {}

const challenge = sigInputToChallenge(sigInputHeader, ctx)

Expand All @@ -126,7 +132,10 @@ describe('Signature Service', (): void => {
{
headers: {
'Content-Type': 'application/json',
'Content-Digest': 'sha-256=:test-hash:',
'Content-Digest': createContentDigestHeader(
JSON.stringify(testRequestBody),
['sha-512']
),
'Content-Length': '1234',
'Signature-Input': sigInputHeader,
Authorization: 'GNAP test-access-token'
Expand All @@ -138,7 +147,7 @@ describe('Signature Service', (): void => {
deps
)

ctx.request.body = { foo: 'bar' }
ctx.request.body = testRequestBody
ctx.method = 'GET'
ctx.request.url = '/test'

Expand Down Expand Up @@ -234,7 +243,7 @@ describe('Signature Service', (): void => {
headers: {
Accept: 'application/json'
},
url: '/',
url: 'http://example.com/',
method: 'POST'
},
{},
Expand Down Expand Up @@ -268,7 +277,7 @@ describe('Signature Service', (): void => {
Accept: 'application/json',
Authorization: `GNAP ${grant.continueToken}`
},
url: '/continue',
url: 'http://example.com/continue',
method: 'POST'
},
{ id: grant.continueId },
Expand Down Expand Up @@ -298,7 +307,7 @@ describe('Signature Service', (): void => {
headers: {
Accept: 'application/json'
},
url: tokenManagementUrl,
url: 'http://example.com' + tokenManagementUrl,
method: 'DELETE'
},
{ id: managementId },
Expand Down Expand Up @@ -329,7 +338,7 @@ describe('Signature Service', (): void => {
headers: {
Accept: 'application/json'
},
url: tokenManagementUrl,
url: 'http://example.com' + tokenManagementUrl,
method
},
{ id: managementId },
Expand All @@ -341,10 +350,11 @@ describe('Signature Service', (): void => {
proof: 'httpsig',
resource_server: 'test'
}
await tokenHttpsigMiddleware(ctx, next)
expect(ctx.response.status).toEqual(400)
expect(ctx.response.body.error).toEqual('invalid_request')
expect(ctx.response.body.message).toEqual('invalid signature headers')
await expect(tokenHttpsigMiddleware(ctx, next)).rejects.toMatchObject({
status: 400,
error: 'invalid_request',
message: 'invalid signature headers'
})
})

test('middleware fails if signature is invalid', async (): Promise<void> => {
Expand Down Expand Up @@ -404,5 +414,33 @@ describe('Signature Service', (): void => {
message: 'invalid client'
})
})

test('middleware fails if content-digest is invalid', async (): Promise<void> => {
const ctx = await createContextWithSigHeaders(
{
headers: {
Accept: 'application/json'
},
url: '/',
method: 'POST'
},
{},
{
client: CLIENT
},
privateKey,
testClientKey.kid,
deps
)

ctx.request.body = { test: 'this is wrong' }

await expect(
grantInitiationHttpsigMiddleware(ctx, next)
).rejects.toMatchObject({
status: 400,
message: 'invalid signature headers'
})
})
})
})
39 changes: 17 additions & 22 deletions packages/auth/src/signature/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import * as crypto from 'crypto'
import { importJWK } from 'jose'
import { JWK } from 'open-payments'
import { verifyContentDigest } from 'httpbis-digest-headers'

import { AppContext } from '../app'
import { Grant } from '../grant/model'
Expand Down Expand Up @@ -105,12 +106,21 @@ function validateSigInputComponents(
if (component !== component.toLowerCase()) return false
}

const isValidContentDigest =
!sigInputComponents.includes('content-digest') ||
(!!ctx.headers['content-digest'] &&
ctx.request.body &&
Object.keys(ctx.request.body).length > 0 &&
sigInputComponents.includes('content-digest') &&
verifyContentDigest(
JSON.stringify(ctx.request.body),
ctx.headers['content-digest'] as string
))

return !(
!isValidContentDigest ||
!sigInputComponents.includes('@method') ||
!sigInputComponents.includes('@target-uri') ||
(ctx.request.body &&
Object.keys(ctx.request.body).length > 0 &&
!sigInputComponents.includes('content-digest')) ||
(ctx.headers['authorization'] &&
!sigInputComponents.includes('authorization'))
)
Expand All @@ -134,7 +144,7 @@ export function sigInputToChallenge(
if (component === '@method') {
signatureBase += `"@method": ${ctx.request.method}\n`
} else if (component === '@target-uri') {
signatureBase += `"@target-uri": ${ctx.request.url}\n`
signatureBase += `"@target-uri": ${ctx.request.href}\n`
} else {
signatureBase += `"${component}": ${ctx.headers[component]}\n`
}
Expand Down Expand Up @@ -178,12 +188,7 @@ export async function grantContinueHttpsigMiddleware(
next: () => Promise<any>
): Promise<void> {
if (!validateHttpSigHeaders(ctx)) {
ctx.status = 400
ctx.body = {
error: 'invalid_request',
message: 'invalid signature headers'
}
return
ctx.throw(400, 'invalid signature headers', { error: 'invalid_request' })
}

const continueToken = ctx.headers['authorization'].replace(
Expand Down Expand Up @@ -226,12 +231,7 @@ export async function grantInitiationHttpsigMiddleware(
next: () => Promise<any>
): Promise<void> {
if (!validateHttpSigHeaders(ctx)) {
ctx.status = 400
ctx.body = {
error: 'invalid_request',
message: 'invalid signature headers'
}
return
ctx.throw(400, 'invalid signature headers', { error: 'invalid_request' })
}

const { body } = ctx.request
Expand All @@ -251,12 +251,7 @@ export async function tokenHttpsigMiddleware(
next: () => Promise<any>
): Promise<void> {
if (!validateHttpSigHeaders(ctx)) {
ctx.status = 400
ctx.body = {
error: 'invalid_request',
message: 'invalid signature headers'
}
return
ctx.throw(400, 'invalid signature headers', { error: 'invalid_request' })
}

const accessTokenService = await ctx.container.use('accessTokenService')
Expand Down
6 changes: 2 additions & 4 deletions packages/auth/src/tests/signature.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import crypto from 'crypto'
import { v4 } from 'uuid'
import { createContentDigestHeader } from 'httpbis-digest-headers'
import { importJWK, exportJWK } from 'jose'
import { JWKWithRequired } from '../client/service'

Expand Down Expand Up @@ -78,10 +79,7 @@ export async function generateSigHeaders({
let challenge = `"@method": ${method}\n"@target-uri": ${url}\n`
let contentDigest
if (body) {
const hash = crypto.createHash('sha256')
hash.update(Buffer.from(JSON.stringify(body)))
const bodyDigest = hash.digest()
contentDigest = `sha-256:${bodyDigest.toString('base64')}:`
contentDigest = createContentDigestHeader(JSON.stringify(body), ['sha-512'])
challenge += `"content-digest": ${contentDigest}\n`
}

Expand Down
1 change: 1 addition & 0 deletions packages/open-payments/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"dependencies": {
"axios": "^1.1.2",
"http-message-signatures": "^0.1.2",
"httpbis-digest-headers": "github:interledger/httpbis-digest-headers",
"openapi": "workspace:../openapi",
"pino": "^8.4.2"
}
Expand Down
31 changes: 30 additions & 1 deletion packages/open-payments/src/client/requests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,35 @@ describe('requests', (): void => {
id: 'id'
}

nock(baseUrl).post('/grant', body).reply(status, body)
// https://github.com/nock/nock/issues/2200#issuecomment-1280957462
jest
.useFakeTimers({
doNotFake: [
'nextTick',
'setImmediate',
'clearImmediate',
'setInterval',
'clearInterval',
'setTimeout',
'clearTimeout'
]
})
.setSystemTime(new Date())

const scope = nock(baseUrl)
.matchHeader('Signature', /sig1=:([a-zA-Z0-9+/]){86}==:/)
.matchHeader(
'Signature-Input',
`sig1=("@method" "@target-uri" "content-digest" "content-length" "content-type");created=${Math.floor(
Date.now() / 1000
)};keyid="${keyId}";alg="ed25519"`
)
.matchHeader('Content-Digest', /sha-512=:([a-zA-Z0-9+/]){86}==:/)
.matchHeader('Content-Length', 11)
.matchHeader('Content-Type', 'application/json')
.post('/grant', body)
// TODO: verify signature
.reply(status, body)

await post(
{ axiosInstance, logger },
Expand All @@ -185,6 +213,7 @@ describe('requests', (): void => {
},
responseValidators.successfulValidator
)
scope.done()

expect(axiosInstance.post).toHaveBeenCalledWith(`${baseUrl}/grant`, body)
})
Expand Down
13 changes: 12 additions & 1 deletion packages/open-payments/src/client/requests.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import axios, { AxiosInstance } from 'axios'
import { KeyLike } from 'crypto'
import { createContentDigestHeader } from 'httpbis-digest-headers'
import { ResponseValidator } from 'openapi'
import { BaseDeps } from '.'
import { createSignatureHeaders } from './signatures'
Expand Down Expand Up @@ -128,6 +129,14 @@ export const createAxiosInstance = (args: {
if (args.privateKey && args.keyId) {
axiosInstance.interceptors.request.use(
async (config) => {
if (config.data) {
const data = JSON.stringify(config.data)
config.headers['Content-Digest'] = createContentDigestHeader(data, [
'sha-512'
])
config.headers['Content-Length'] = Buffer.from(data, 'utf-8').length
config.headers['Content-Type'] = 'application/json'
}
const sigHeaders = await createSignatureHeaders({
request: {
method: config.method.toUpperCase(),
Expand All @@ -144,7 +153,9 @@ export const createAxiosInstance = (args: {
},
null,
{
runWhen: (config) => !!config.headers['Authorization']
runWhen: (config) =>
config.method.toLowerCase() === 'post' ||
!!config.headers['Authorization']
}
)
}
Expand Down
4 changes: 1 addition & 3 deletions packages/open-payments/src/client/signatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ export const createSignatureHeaders = async ({
components.push('authorization')
}
if (request.body) {
// TODO: 'content-digest'
// https://github.com/interledger/rafiki/issues/655
components.push('content-length', 'content-type')
components.push('content-digest', 'content-length', 'content-type')
}
const { headers } = await httpsig.sign(request, {
components,
Expand Down
Loading

0 comments on commit a5c36d7

Please sign in to comment.