Skip to content
Open
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
6 changes: 6 additions & 0 deletions src/server/server.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,12 @@ const conf = convict({
format: String,
env: 'OFFERWALL_SECRET',
default: ''
},
onramperUrlSigningSecret: {
doc: 'Onramper URL signing secret',
format: String,
env: 'ONRAMPER_URL_SIGNING_SECRET',
default: ''
}
})

Expand Down
40 changes: 38 additions & 2 deletions src/server/verification/__tests__/verificationAPI.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import request from 'supertest'
import MockAdapter from 'axios-mock-adapter'
import crypto from 'crypto'

import { assign, omit } from 'lodash'
import moment from 'moment'
Expand All @@ -25,7 +26,7 @@ GoodIDUtils.ageGenderCheck = jest.fn().mockResolvedValue({ age: { min: 15 } })

describe('verificationAPI', () => {
let server
const { skipEmailVerification, zoomProductionMode, defaultWhitelistChainId } = Config
const { skipEmailVerification, zoomProductionMode, defaultWhitelistChainId, onramperUrlSigningSecret } = Config
const userIdentifier = '0x7ac080f6607405705aed79675789701a48c76f55'

beforeAll(async () => {
Expand All @@ -45,7 +46,7 @@ describe('verificationAPI', () => {

afterAll(async () => {
// restore original config
Object.assign(Config, { skipEmailVerification, zoomProductionMode })
Object.assign(Config, { skipEmailVerification, zoomProductionMode, onramperUrlSigningSecret })
await storage.model.deleteMany({ fullName: new RegExp('test_user_sendemail', 'i') })

await new Promise(res =>
Expand Down Expand Up @@ -529,4 +530,39 @@ describe('verificationAPI', () => {
await testDisposalState(true)
})
})

describe('onramper signing', () => {
beforeEach(() => {
Config.onramperUrlSigningSecret = onramperUrlSigningSecret || 'test-onramper-secret'
})

test('POST /verify/onramper/sign signs already-normalized signContent as-is', async () => {
const signContent =
'networkWallets=bitcoin:1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2,ethereum:1BvBMSEYstWetqTFn5wrwrhGFryetusJaNVN2&walletAddressTags=btc:1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2,eth:1BvBMSEYstWetqTFn5wrwrhGFryetusJaNVN2&wallets=btc:1Lbcfr7sAHTD9CgdQo3HTMTkV8LK4ZnX71,eth:1Lbcfr7sAUTEFCgdQo3HTMTkV8LK4ZnX71'
const payload = { signContent }
const signature = crypto.createHmac('sha256', Config.onramperUrlSigningSecret).update(signContent).digest('hex')

await request(server).post('/verify/onramper/sign').send(payload).expect(200, {
ok: 1,
signContent,
signature
})
})

test('POST /verify/onramper/sign returns 400 for missing signContent', async () => {
await request(server).post('/verify/onramper/sign').send({}).expect(400, {
ok: -1,
error: 'missing signContent'
})
})

Comment on lines +552 to +558
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Add a test case for non-string signContent to cover the type check branch.

The case where signContent is present but not a string (typeof signContent !== 'string') isn’t currently tested. Please add a test (e.g. send({ signContent: { foo: 'bar' } }) or a number/array) and assert it returns the same 400 response, so the type guard branch is covered and protected against regressions.

Suggested change
test('POST /verify/onramper/sign returns 400 for missing signContent', async () => {
await request(server).post('/verify/onramper/sign').send({}).expect(400, {
ok: -1,
error: 'missing signContent'
})
})
test('POST /verify/onramper/sign returns 400 for missing signContent', async () => {
await request(server).post('/verify/onramper/sign').send({}).expect(400, {
ok: -1,
error: 'missing signContent'
})
})
test('POST /verify/onramper/sign returns 400 for non-string signContent', async () => {
await request(server).post('/verify/onramper/sign').send({
signContent: { foo: 'bar' }
}).expect(400, {
ok: -1,
error: 'missing signContent'
})
})

test('POST /verify/onramper/sign returns 500 when secret is not configured', async () => {
Config.onramperUrlSigningSecret = ''

await request(server).post('/verify/onramper/sign').send({ signContent: 'wallets=btc:abc' }).expect(500, {
ok: -1,
error: 'Onramper signing secret is not configured'
})
})
})
})
23 changes: 23 additions & 0 deletions src/server/verification/verificationAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,29 @@ const setup = (app: Router, verifier: VerificationAPI, storage: StorageAPI) => {
})
)

app.post(
'/verify/onramper/sign',
requestRateLimiter(20, 1),
passport.authenticate(['jwt', 'anonymous'], { session: false }),
Copy link
Contributor

Choose a reason for hiding this comment

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

🚨 issue (security): Allowing the anonymous strategy here turns this into a public signing oracle, which effectively exposes the HMAC capability to anyone.

With passport.authenticate(['jwt', 'anonymous']), this endpoint will sign arbitrary signContent with the shared HMAC secret for unauthenticated callers, effectively exposing the signing capability and undermining the purpose of the secret.

Require a strictly authenticated strategy (e.g. jwt only) and/or add authorization so only trusted callers can access this endpoint; otherwise any external party can generate valid Onramper signatures as if they held the secret.

Copy link
Contributor

Choose a reason for hiding this comment

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

@L03TJ3
We should implement CORS for this endpoint

wrapAsync(async (req, res) => {
const { log, body = {} } = req
const { signContent } = body

if (!signContent || typeof signContent !== 'string') {
return res.status(400).json({ ok: -1, error: 'missing signContent' })
}

if (!conf.onramperUrlSigningSecret) {
log.error('onramper sign request failed, missing ONRAMPER_URL_SIGNING_SECRET')
return res.status(500).json({ ok: -1, error: 'Onramper signing secret is not configured' })
}

const signature = crypto.createHmac('sha256', conf.onramperUrlSigningSecret).update(signContent).digest('hex')

return res.json({ ok: 1, signContent, signature })
})
)

/**
* @api {post} /verify/email Send verification email endpoint
* @apiName Send Email
Expand Down
Loading