From e7053b1d9d7f77d0375155b38d3e845f2163ecd8 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 10 Mar 2021 11:17:20 +0100 Subject: [PATCH] Fix AP security tests --- server/helpers/requests.ts | 6 ++-- .../handlers/utils/activitypub-http-utils.ts | 6 ++-- .../validators/activitypub/signature.ts | 2 +- server/tests/api/activitypub/security.ts | 35 ++++++++++++++----- shared/extra-utils/requests/activitypub.ts | 4 +-- 5 files changed, 36 insertions(+), 17 deletions(-) diff --git a/server/helpers/requests.ts b/server/helpers/requests.ts index 5eb69486d..fd2a56f30 100644 --- a/server/helpers/requests.ts +++ b/server/helpers/requests.ts @@ -170,9 +170,11 @@ function buildGotOptions (options: PeerTubeRequestOptions) { let headers = options.headers || {} - headers = { ...headers, date: new Date().toUTCString() } + if (!headers.date) { + headers = { ...headers, date: new Date().toUTCString() } + } - if (activityPub) { + if (activityPub && !headers.accept) { headers = { ...headers, accept: ACTIVITY_PUB.ACCEPT_HEADER } } diff --git a/server/lib/job-queue/handlers/utils/activitypub-http-utils.ts b/server/lib/job-queue/handlers/utils/activitypub-http-utils.ts index 4116a9c0e..e8a91450d 100644 --- a/server/lib/job-queue/handlers/utils/activitypub-http-utils.ts +++ b/server/lib/job-queue/handlers/utils/activitypub-http-utils.ts @@ -46,9 +46,9 @@ async function buildSignedRequestOptions (payload: Payload) { function buildGlobalHeaders (body: any) { return { - 'Digest': buildDigest(body), - 'Content-Type': 'application/activity+json', - 'Accept': ACTIVITY_PUB.ACCEPT_HEADER + 'digest': buildDigest(body), + 'content-type': 'application/activity+json', + 'accept': ACTIVITY_PUB.ACCEPT_HEADER } } diff --git a/server/middlewares/validators/activitypub/signature.ts b/server/middlewares/validators/activitypub/signature.ts index 02b191480..7c4e49463 100644 --- a/server/middlewares/validators/activitypub/signature.ts +++ b/server/middlewares/validators/activitypub/signature.ts @@ -23,7 +23,7 @@ const signatureValidator = [ .custom(isSignatureValueValid).withMessage('Should have a valid signature value'), (req: express.Request, res: express.Response, next: express.NextFunction) => { - logger.debug('Checking activitypub signature parameter', { parameters: { signature: req.body.signature } }) + logger.debug('Checking Linked Data Signature parameter', { parameters: { signature: req.body.signature } }) if (areValidationErrors(req, res)) return diff --git a/server/tests/api/activitypub/security.ts b/server/tests/api/activitypub/security.ts index 9745052a3..364b53e0f 100644 --- a/server/tests/api/activitypub/security.ts +++ b/server/tests/api/activitypub/security.ts @@ -8,6 +8,8 @@ import { cleanupTests, closeAllSequelize, flushAndRunMultipleServers, + killallServers, + reRunServer, ServerInfo, setActorField, wait @@ -20,21 +22,32 @@ import { buildGlobalHeaders } from '../../../lib/job-queue/handlers/utils/activi const expect = chai.expect function setKeysOfServer (onServer: ServerInfo, ofServer: ServerInfo, publicKey: string, privateKey: string) { + const url = 'http://localhost:' + ofServer.port + '/accounts/peertube' + + return Promise.all([ + setActorField(onServer.internalServerNumber, url, 'publicKey', publicKey), + setActorField(onServer.internalServerNumber, url, 'privateKey', privateKey) + ]) +} + +function setUpdatedAtOfServer (onServer: ServerInfo, ofServer: ServerInfo, updatedAt: string) { + const url = 'http://localhost:' + ofServer.port + '/accounts/peertube' + return Promise.all([ - setActorField(onServer.internalServerNumber, 'http://localhost:' + ofServer.port + '/accounts/peertube', 'publicKey', publicKey), - setActorField(onServer.internalServerNumber, 'http://localhost:' + ofServer.port + '/accounts/peertube', 'privateKey', privateKey) + setActorField(onServer.internalServerNumber, url, 'createdAt', updatedAt), + setActorField(onServer.internalServerNumber, url, 'updatedAt', updatedAt) ]) } -function getAnnounceWithoutContext (server2: ServerInfo) { +function getAnnounceWithoutContext (server: ServerInfo) { const json = require('./json/peertube/announce-without-context.json') const result: typeof json = {} for (const key of Object.keys(json)) { if (Array.isArray(json[key])) { - result[key] = json[key].map(v => v.replace(':9002', `:${server2.port}`)) + result[key] = json[key].map(v => v.replace(':9002', `:${server.port}`)) } else { - result[key] = json[key].replace(':9002', `:${server2.port}`) + result[key] = json[key].replace(':9002', `:${server.port}`) } } @@ -64,7 +77,8 @@ describe('Test ActivityPub security', function () { url = servers[0].url + '/inbox' - await setKeysOfServer(servers[0], servers[1], keys.publicKey, keys.privateKey) + await setKeysOfServer(servers[0], servers[1], keys.publicKey, null) + await setKeysOfServer(servers[1], servers[1], keys.publicKey, keys.privateKey) const to = { url: 'http://localhost:' + servers[0].port + '/accounts/peertube' } const by = { url: 'http://localhost:' + servers[1].port + '/accounts/peertube', privateKey: keys.privateKey } @@ -152,12 +166,14 @@ describe('Test ActivityPub security', function () { it('Should refresh the actor keys', async function () { this.timeout(20000) - // Wait refresh invalidation - await wait(10000) - // Update keys of server 2 to invalid keys // Server 1 should refresh the actor and fail await setKeysOfServer(servers[1], servers[1], invalidKeys.publicKey, invalidKeys.privateKey) + await setUpdatedAtOfServer(servers[0], servers[1], '2015-07-17 22:00:00+00') + + // Invalid peertube actor cache + killallServers([ servers[1] ]) + await reRunServer(servers[1]) const body = activityPubContextify(getAnnounceWithoutContext(servers[1])) const headers = buildGlobalHeaders(body) @@ -166,6 +182,7 @@ describe('Test ActivityPub security', function () { await makePOSTAPRequest(url, body, baseHttpSignature(), headers) expect(true, 'Did not throw').to.be.false } catch (err) { + console.error(err) expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403) } }) diff --git a/shared/extra-utils/requests/activitypub.ts b/shared/extra-utils/requests/activitypub.ts index 2a7f20289..ecd8ce823 100644 --- a/shared/extra-utils/requests/activitypub.ts +++ b/shared/extra-utils/requests/activitypub.ts @@ -17,7 +17,7 @@ function makePOSTAPRequest (url: string, body: any, httpSignature: any, headers: async function makeFollowRequest (to: { url: string }, by: { url: string, privateKey }) { const follow = { type: 'Follow', - id: by.url + '/toto', + id: by.url + '/' + new Date().getTime(), actor: by.url, object: to.url } @@ -33,7 +33,7 @@ async function makeFollowRequest (to: { url: string }, by: { url: string, privat } const headers = buildGlobalHeaders(body) - return makePOSTAPRequest(to.url, body, httpSignature, headers) + return makePOSTAPRequest(to.url + '/inbox', body, httpSignature, headers) } export { -- 2.41.0