diff options
-rw-r--r-- | server/helpers/peertube-crypto.ts | 6 | ||||
-rw-r--r-- | server/initializers/constants.ts | 4 | ||||
-rw-r--r-- | server/middlewares/activitypub.ts | 11 | ||||
-rw-r--r-- | server/tests/api/activitypub/security.ts | 21 |
4 files changed, 39 insertions, 3 deletions
diff --git a/server/helpers/peertube-crypto.ts b/server/helpers/peertube-crypto.ts index 1655cd7b5..994f725d8 100644 --- a/server/helpers/peertube-crypto.ts +++ b/server/helpers/peertube-crypto.ts | |||
@@ -50,7 +50,11 @@ function isHTTPSignatureVerified (httpSignatureParsed: any, actor: MActor): bool | |||
50 | } | 50 | } |
51 | 51 | ||
52 | function parseHTTPSignature (req: Request, clockSkew?: number) { | 52 | function parseHTTPSignature (req: Request, clockSkew?: number) { |
53 | return httpSignature.parse(req, { clockSkew }) | 53 | const headers = req.method === 'POST' |
54 | ? HTTP_SIGNATURE.REQUIRED_HEADERS.POST | ||
55 | : HTTP_SIGNATURE.REQUIRED_HEADERS.ALL | ||
56 | |||
57 | return httpSignature.parse(req, { clockSkew, headers }) | ||
54 | } | 58 | } |
55 | 59 | ||
56 | // JSONLD | 60 | // JSONLD |
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 501e06396..679503731 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts | |||
@@ -513,6 +513,10 @@ const HTTP_SIGNATURE = { | |||
513 | HEADER_NAME: 'signature', | 513 | HEADER_NAME: 'signature', |
514 | ALGORITHM: 'rsa-sha256', | 514 | ALGORITHM: 'rsa-sha256', |
515 | HEADERS_TO_SIGN: [ '(request-target)', 'host', 'date', 'digest' ], | 515 | HEADERS_TO_SIGN: [ '(request-target)', 'host', 'date', 'digest' ], |
516 | REQUIRED_HEADERS: { | ||
517 | ALL: [ '(request-target)', 'host', 'date' ], | ||
518 | POST: [ '(request-target)', 'host', 'date', 'digest' ] | ||
519 | }, | ||
516 | CLOCK_SKEW_SECONDS: 1800 | 520 | CLOCK_SKEW_SECONDS: 1800 |
517 | } | 521 | } |
518 | 522 | ||
diff --git a/server/middlewares/activitypub.ts b/server/middlewares/activitypub.ts index 580606a68..d00594059 100644 --- a/server/middlewares/activitypub.ts +++ b/server/middlewares/activitypub.ts | |||
@@ -63,7 +63,16 @@ async function checkHttpSignature (req: Request, res: Response) { | |||
63 | const sig = req.headers[HTTP_SIGNATURE.HEADER_NAME] as string | 63 | const sig = req.headers[HTTP_SIGNATURE.HEADER_NAME] as string |
64 | if (sig && sig.startsWith('Signature ') === true) req.headers[HTTP_SIGNATURE.HEADER_NAME] = sig.replace(/^Signature /, '') | 64 | if (sig && sig.startsWith('Signature ') === true) req.headers[HTTP_SIGNATURE.HEADER_NAME] = sig.replace(/^Signature /, '') |
65 | 65 | ||
66 | const parsed = parseHTTPSignature(req, HTTP_SIGNATURE.CLOCK_SKEW_SECONDS) | 66 | let parsed: any |
67 | |||
68 | try { | ||
69 | parsed = parseHTTPSignature(req, HTTP_SIGNATURE.CLOCK_SKEW_SECONDS) | ||
70 | } catch (err) { | ||
71 | logger.warn('Invalid signature because of exception in signature parser', { reqBody: req.body, err }) | ||
72 | |||
73 | res.status(403).json({ error: err.message }) | ||
74 | return false | ||
75 | } | ||
67 | 76 | ||
68 | const keyId = parsed.keyId | 77 | const keyId = parsed.keyId |
69 | if (!keyId) { | 78 | if (!keyId) { |
diff --git a/server/tests/api/activitypub/security.ts b/server/tests/api/activitypub/security.ts index ac4bc7c6a..e6002b661 100644 --- a/server/tests/api/activitypub/security.ts +++ b/server/tests/api/activitypub/security.ts | |||
@@ -99,13 +99,32 @@ describe('Test ActivityPub security', function () { | |||
99 | expect(response.statusCode).to.equal(403) | 99 | expect(response.statusCode).to.equal(403) |
100 | }) | 100 | }) |
101 | 101 | ||
102 | it('Should succeed with a valid HTTP signature', async function () { | 102 | it('Should reject requests without appropriate signed headers', async function () { |
103 | await setKeysOfServer(servers[0], servers[1], keys.publicKey, keys.privateKey) | 103 | await setKeysOfServer(servers[0], servers[1], keys.publicKey, keys.privateKey) |
104 | await setKeysOfServer(servers[1], servers[1], keys.publicKey, keys.privateKey) | 104 | await setKeysOfServer(servers[1], servers[1], keys.publicKey, keys.privateKey) |
105 | 105 | ||
106 | const body = activityPubContextify(getAnnounceWithoutContext(servers[1])) | 106 | const body = activityPubContextify(getAnnounceWithoutContext(servers[1])) |
107 | const headers = buildGlobalHeaders(body) | 107 | const headers = buildGlobalHeaders(body) |
108 | 108 | ||
109 | const signatureOptions = baseHttpSignature() | ||
110 | const badHeadersMatrix = [ | ||
111 | [ '(request-target)', 'date', 'digest' ], | ||
112 | [ 'host', 'date', 'digest' ], | ||
113 | [ '(request-target)', 'host', 'digest' ] | ||
114 | ] | ||
115 | |||
116 | for (const badHeaders of badHeadersMatrix) { | ||
117 | signatureOptions.headers = badHeaders | ||
118 | |||
119 | const { response } = await makePOSTAPRequest(url, body, signatureOptions, headers) | ||
120 | expect(response.statusCode).to.equal(403) | ||
121 | } | ||
122 | }) | ||
123 | |||
124 | it('Should succeed with a valid HTTP signature', async function () { | ||
125 | const body = activityPubContextify(getAnnounceWithoutContext(servers[1])) | ||
126 | const headers = buildGlobalHeaders(body) | ||
127 | |||
109 | const { response } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers) | 128 | const { response } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers) |
110 | 129 | ||
111 | expect(response.statusCode).to.equal(204) | 130 | expect(response.statusCode).to.equal(204) |