diff options
author | Chocobozzz <me@florianbigard.com> | 2021-03-10 11:17:20 +0100 |
---|---|---|
committer | Chocobozzz <me@florianbigard.com> | 2021-03-24 18:18:40 +0100 |
commit | e7053b1d9d7f77d0375155b38d3e845f2163ecd8 (patch) | |
tree | b52375913c9b0352f5c765c40122cbbbc54bad13 | |
parent | 266131e0ca2f2622bbb15299212f00b1efa36867 (diff) | |
download | PeerTube-e7053b1d9d7f77d0375155b38d3e845f2163ecd8.tar.gz PeerTube-e7053b1d9d7f77d0375155b38d3e845f2163ecd8.tar.zst PeerTube-e7053b1d9d7f77d0375155b38d3e845f2163ecd8.zip |
Fix AP security tests
-rw-r--r-- | server/helpers/requests.ts | 6 | ||||
-rw-r--r-- | server/lib/job-queue/handlers/utils/activitypub-http-utils.ts | 6 | ||||
-rw-r--r-- | server/middlewares/validators/activitypub/signature.ts | 2 | ||||
-rw-r--r-- | server/tests/api/activitypub/security.ts | 35 | ||||
-rw-r--r-- | 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) { | |||
170 | 170 | ||
171 | let headers = options.headers || {} | 171 | let headers = options.headers || {} |
172 | 172 | ||
173 | headers = { ...headers, date: new Date().toUTCString() } | 173 | if (!headers.date) { |
174 | headers = { ...headers, date: new Date().toUTCString() } | ||
175 | } | ||
174 | 176 | ||
175 | if (activityPub) { | 177 | if (activityPub && !headers.accept) { |
176 | headers = { ...headers, accept: ACTIVITY_PUB.ACCEPT_HEADER } | 178 | headers = { ...headers, accept: ACTIVITY_PUB.ACCEPT_HEADER } |
177 | } | 179 | } |
178 | 180 | ||
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<any>) { | |||
46 | 46 | ||
47 | function buildGlobalHeaders (body: any) { | 47 | function buildGlobalHeaders (body: any) { |
48 | return { | 48 | return { |
49 | 'Digest': buildDigest(body), | 49 | 'digest': buildDigest(body), |
50 | 'Content-Type': 'application/activity+json', | 50 | 'content-type': 'application/activity+json', |
51 | 'Accept': ACTIVITY_PUB.ACCEPT_HEADER | 51 | 'accept': ACTIVITY_PUB.ACCEPT_HEADER |
52 | } | 52 | } |
53 | } | 53 | } |
54 | 54 | ||
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 = [ | |||
23 | .custom(isSignatureValueValid).withMessage('Should have a valid signature value'), | 23 | .custom(isSignatureValueValid).withMessage('Should have a valid signature value'), |
24 | 24 | ||
25 | (req: express.Request, res: express.Response, next: express.NextFunction) => { | 25 | (req: express.Request, res: express.Response, next: express.NextFunction) => { |
26 | logger.debug('Checking activitypub signature parameter', { parameters: { signature: req.body.signature } }) | 26 | logger.debug('Checking Linked Data Signature parameter', { parameters: { signature: req.body.signature } }) |
27 | 27 | ||
28 | if (areValidationErrors(req, res)) return | 28 | if (areValidationErrors(req, res)) return |
29 | 29 | ||
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 { | |||
8 | cleanupTests, | 8 | cleanupTests, |
9 | closeAllSequelize, | 9 | closeAllSequelize, |
10 | flushAndRunMultipleServers, | 10 | flushAndRunMultipleServers, |
11 | killallServers, | ||
12 | reRunServer, | ||
11 | ServerInfo, | 13 | ServerInfo, |
12 | setActorField, | 14 | setActorField, |
13 | wait | 15 | wait |
@@ -20,21 +22,32 @@ import { buildGlobalHeaders } from '../../../lib/job-queue/handlers/utils/activi | |||
20 | const expect = chai.expect | 22 | const expect = chai.expect |
21 | 23 | ||
22 | function setKeysOfServer (onServer: ServerInfo, ofServer: ServerInfo, publicKey: string, privateKey: string) { | 24 | function setKeysOfServer (onServer: ServerInfo, ofServer: ServerInfo, publicKey: string, privateKey: string) { |
25 | const url = 'http://localhost:' + ofServer.port + '/accounts/peertube' | ||
26 | |||
27 | return Promise.all([ | ||
28 | setActorField(onServer.internalServerNumber, url, 'publicKey', publicKey), | ||
29 | setActorField(onServer.internalServerNumber, url, 'privateKey', privateKey) | ||
30 | ]) | ||
31 | } | ||
32 | |||
33 | function setUpdatedAtOfServer (onServer: ServerInfo, ofServer: ServerInfo, updatedAt: string) { | ||
34 | const url = 'http://localhost:' + ofServer.port + '/accounts/peertube' | ||
35 | |||
23 | return Promise.all([ | 36 | return Promise.all([ |
24 | setActorField(onServer.internalServerNumber, 'http://localhost:' + ofServer.port + '/accounts/peertube', 'publicKey', publicKey), | 37 | setActorField(onServer.internalServerNumber, url, 'createdAt', updatedAt), |
25 | setActorField(onServer.internalServerNumber, 'http://localhost:' + ofServer.port + '/accounts/peertube', 'privateKey', privateKey) | 38 | setActorField(onServer.internalServerNumber, url, 'updatedAt', updatedAt) |
26 | ]) | 39 | ]) |
27 | } | 40 | } |
28 | 41 | ||
29 | function getAnnounceWithoutContext (server2: ServerInfo) { | 42 | function getAnnounceWithoutContext (server: ServerInfo) { |
30 | const json = require('./json/peertube/announce-without-context.json') | 43 | const json = require('./json/peertube/announce-without-context.json') |
31 | const result: typeof json = {} | 44 | const result: typeof json = {} |
32 | 45 | ||
33 | for (const key of Object.keys(json)) { | 46 | for (const key of Object.keys(json)) { |
34 | if (Array.isArray(json[key])) { | 47 | if (Array.isArray(json[key])) { |
35 | result[key] = json[key].map(v => v.replace(':9002', `:${server2.port}`)) | 48 | result[key] = json[key].map(v => v.replace(':9002', `:${server.port}`)) |
36 | } else { | 49 | } else { |
37 | result[key] = json[key].replace(':9002', `:${server2.port}`) | 50 | result[key] = json[key].replace(':9002', `:${server.port}`) |
38 | } | 51 | } |
39 | } | 52 | } |
40 | 53 | ||
@@ -64,7 +77,8 @@ describe('Test ActivityPub security', function () { | |||
64 | 77 | ||
65 | url = servers[0].url + '/inbox' | 78 | url = servers[0].url + '/inbox' |
66 | 79 | ||
67 | await setKeysOfServer(servers[0], servers[1], keys.publicKey, keys.privateKey) | 80 | await setKeysOfServer(servers[0], servers[1], keys.publicKey, null) |
81 | await setKeysOfServer(servers[1], servers[1], keys.publicKey, keys.privateKey) | ||
68 | 82 | ||
69 | const to = { url: 'http://localhost:' + servers[0].port + '/accounts/peertube' } | 83 | const to = { url: 'http://localhost:' + servers[0].port + '/accounts/peertube' } |
70 | const by = { url: 'http://localhost:' + servers[1].port + '/accounts/peertube', privateKey: keys.privateKey } | 84 | const by = { url: 'http://localhost:' + servers[1].port + '/accounts/peertube', privateKey: keys.privateKey } |
@@ -152,12 +166,14 @@ describe('Test ActivityPub security', function () { | |||
152 | it('Should refresh the actor keys', async function () { | 166 | it('Should refresh the actor keys', async function () { |
153 | this.timeout(20000) | 167 | this.timeout(20000) |
154 | 168 | ||
155 | // Wait refresh invalidation | ||
156 | await wait(10000) | ||
157 | |||
158 | // Update keys of server 2 to invalid keys | 169 | // Update keys of server 2 to invalid keys |
159 | // Server 1 should refresh the actor and fail | 170 | // Server 1 should refresh the actor and fail |
160 | await setKeysOfServer(servers[1], servers[1], invalidKeys.publicKey, invalidKeys.privateKey) | 171 | await setKeysOfServer(servers[1], servers[1], invalidKeys.publicKey, invalidKeys.privateKey) |
172 | await setUpdatedAtOfServer(servers[0], servers[1], '2015-07-17 22:00:00+00') | ||
173 | |||
174 | // Invalid peertube actor cache | ||
175 | killallServers([ servers[1] ]) | ||
176 | await reRunServer(servers[1]) | ||
161 | 177 | ||
162 | const body = activityPubContextify(getAnnounceWithoutContext(servers[1])) | 178 | const body = activityPubContextify(getAnnounceWithoutContext(servers[1])) |
163 | const headers = buildGlobalHeaders(body) | 179 | const headers = buildGlobalHeaders(body) |
@@ -166,6 +182,7 @@ describe('Test ActivityPub security', function () { | |||
166 | await makePOSTAPRequest(url, body, baseHttpSignature(), headers) | 182 | await makePOSTAPRequest(url, body, baseHttpSignature(), headers) |
167 | expect(true, 'Did not throw').to.be.false | 183 | expect(true, 'Did not throw').to.be.false |
168 | } catch (err) { | 184 | } catch (err) { |
185 | console.error(err) | ||
169 | expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403) | 186 | expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403) |
170 | } | 187 | } |
171 | }) | 188 | }) |
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: | |||
17 | async function makeFollowRequest (to: { url: string }, by: { url: string, privateKey }) { | 17 | async function makeFollowRequest (to: { url: string }, by: { url: string, privateKey }) { |
18 | const follow = { | 18 | const follow = { |
19 | type: 'Follow', | 19 | type: 'Follow', |
20 | id: by.url + '/toto', | 20 | id: by.url + '/' + new Date().getTime(), |
21 | actor: by.url, | 21 | actor: by.url, |
22 | object: to.url | 22 | object: to.url |
23 | } | 23 | } |
@@ -33,7 +33,7 @@ async function makeFollowRequest (to: { url: string }, by: { url: string, privat | |||
33 | } | 33 | } |
34 | const headers = buildGlobalHeaders(body) | 34 | const headers = buildGlobalHeaders(body) |
35 | 35 | ||
36 | return makePOSTAPRequest(to.url, body, httpSignature, headers) | 36 | return makePOSTAPRequest(to.url + '/inbox', body, httpSignature, headers) |
37 | } | 37 | } |
38 | 38 | ||
39 | export { | 39 | export { |