Force signed headers in http signatures

Thanks Roger

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
 }
 
 function parseHTTPSignature (req: Request, clockSkew?: number) {
-  return httpSignature.parse(req, { clockSkew })
+  const headers = req.method === 'POST'
+    ? HTTP_SIGNATURE.REQUIRED_HEADERS.POST
+    : HTTP_SIGNATURE.REQUIRED_HEADERS.ALL
+
+  return httpSignature.parse(req, { clockSkew, headers })
 }
 
 // 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 = {
   HEADER_NAME: 'signature',
   ALGORITHM: 'rsa-sha256',
   HEADERS_TO_SIGN: [ '(request-target)', 'host', 'date', 'digest' ],
+  REQUIRED_HEADERS: {
+    ALL: [ '(request-target)', 'host', 'date' ],
+    POST: [ '(request-target)', 'host', 'date', 'digest' ]
+  },
   CLOCK_SKEW_SECONDS: 1800
 }
 
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) {
   const sig = req.headers[HTTP_SIGNATURE.HEADER_NAME] as string
   if (sig && sig.startsWith('Signature ') === true) req.headers[HTTP_SIGNATURE.HEADER_NAME] = sig.replace(/^Signature /, '')
 
-  const parsed = parseHTTPSignature(req, HTTP_SIGNATURE.CLOCK_SKEW_SECONDS)
+  let parsed: any
+
+  try {
+    parsed = parseHTTPSignature(req, HTTP_SIGNATURE.CLOCK_SKEW_SECONDS)
+  } catch (err) {
+    logger.warn('Invalid signature because of exception in signature parser', { reqBody: req.body, err })
+
+    res.status(403).json({ error: err.message })
+    return false
+  }
 
   const keyId = parsed.keyId
   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 () {
       expect(response.statusCode).to.equal(403)
     })
 
-    it('Should succeed with a valid HTTP signature', async function () {
+    it('Should reject requests without appropriate signed headers', async function () {
       await setKeysOfServer(servers[0], servers[1], keys.publicKey, keys.privateKey)
       await setKeysOfServer(servers[1], servers[1], keys.publicKey, keys.privateKey)
 
       const body = activityPubContextify(getAnnounceWithoutContext(servers[1]))
       const headers = buildGlobalHeaders(body)
 
+      const signatureOptions = baseHttpSignature()
+      const badHeadersMatrix = [
+        [ '(request-target)', 'date', 'digest' ],
+        [ 'host', 'date', 'digest' ],
+        [ '(request-target)', 'host', 'digest' ]
+      ]
+
+      for (const badHeaders of badHeadersMatrix) {
+        signatureOptions.headers = badHeaders
+
+        const { response } = await makePOSTAPRequest(url, body, signatureOptions, headers)
+        expect(response.statusCode).to.equal(403)
+      }
+    })
+
+    it('Should succeed with a valid HTTP signature', async function () {
+      const body = activityPubContextify(getAnnounceWithoutContext(servers[1]))
+      const headers = buildGlobalHeaders(body)
+
       const { response } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
 
       expect(response.statusCode).to.equal(204)