Refine secret key handling for better flexibility and security (#220)

* Refine secret key handling for better flexibility and security

* Improve secret derivation with cryptographic hashing

index.js

@@ -58,15 +58,12 @@ function fastifySecureSession (fastify, options, next) {
     }
 
     let key
-    if (sessionOptions.secret) {
+
+    if (sessionOptions.secret && !sessionOptions.key) {
       if (Buffer.byteLength(sessionOptions.secret) < 32) {
         return next(new Error('secret must be at least 32 bytes'))
       }
 
-      if (!defaultSecret) {
-        defaultSecret = sessionOptions.secret
-      }
-
       key = Buffer.allocUnsafe(sodium.crypto_secretbox_KEYBYTES)
 
       // static salt to be used for key derivation, not great for security,
@@ -87,6 +84,8 @@ function fastifySecureSession (fastify, options, next) {
         sodium.crypto_pwhash_OPSLIMIT_MODERATE,
         sodium.crypto_pwhash_MEMLIMIT_MODERATE,
         sodium.crypto_pwhash_ALG_DEFAULT)
+
+      defaultSecret = sessionOptions.secret
     }
 
     if (sessionOptions.key) {
@@ -108,6 +107,16 @@ function fastifySecureSession (fastify, options, next) {
       } else if (Array.isArray(key) && key.every(isBufferKeyLengthInvalid)) {
         return next(new Error(`key lengths must be ${sodium.crypto_secretbox_KEYBYTES} bytes`))
       }
+
+      const outputHash = Buffer.alloc(sodium.crypto_generichash_BYTES)
+
+      if (Array.isArray(key)) {
+        sodium.crypto_generichash(outputHash, key[0])
+      } else {
+        sodium.crypto_generichash(outputHash, key)
+      }
+
+      defaultSecret = outputHash.toString('hex')
     }
 
     if (!key) {

test/key-rotation.js

@@ -161,3 +161,169 @@ tap.test('does not support an empty key array', async t => {
 
   await t.rejects(() => fastify.after())
 })
+
+tap.test('signing works with only a string key array', function (t) {
+  const fastify = Fastify({ logger: false })
+
+  const key1 = Buffer.alloc(sodium.crypto_secretbox_KEYBYTES)
+  sodium.randombytes_buf(key1)
+
+  const key2 = Buffer.alloc(sodium.crypto_secretbox_KEYBYTES)
+  sodium.randombytes_buf(key2)
+
+  fastify.register(fastifySecureSession, {
+    key: [key1.toString('base64'), key2.toString('base64')]
+  })
+
+  fastify.post('/', (request, reply) => {
+    request.session.set('data', request.body)
+    reply.setCookie('my-session', JSON.stringify(request.body), {
+      httpOnly: true,
+      secure: true,
+      maxAge: 3600,
+      signed: true,
+      path: '/'
+    })
+    reply.send('session set')
+  })
+
+  fastify.get('/secure-session', (request, reply) => {
+    const data = request.session.get('data')
+    if (!data) {
+      reply.code(404).send()
+      return
+    }
+    reply.send(data)
+  })
+
+  fastify.get('/cookie-signed', (request, reply) => {
+    const data = request.unsignCookie(request.cookies['my-session'])
+    if (!data.valid) {
+      reply.code(404).send()
+      return
+    }
+    reply.send(data.value)
+  })
+
+  t.teardown(fastify.close.bind(fastify))
+  t.plan(7)
+
+  fastify.inject({
+    method: 'POST',
+    url: '/',
+    payload: {
+      some: 'data'
+    }
+  }, (error, response) => {
+    t.error(error)
+    t.equal(response.statusCode, 200)
+    t.ok(response.headers['set-cookie'])
+
+    const cookieHeader = response.headers['set-cookie'].join(';')
+
+    fastify.inject({
+      method: 'GET',
+      url: '/secure-session',
+      headers: {
+        cookie: cookieHeader
+      }
+    }, (error, response) => {
+      t.error(error)
+      t.same(JSON.parse(response.payload), { some: 'data' })
+
+      fastify.inject({
+        method: 'GET',
+        url: '/cookie-signed',
+        headers: {
+          cookie: cookieHeader
+        }
+      }, (error, response) => {
+        t.error(error)
+        t.same(JSON.parse(response.payload), { some: 'data' })
+      })
+    })
+  })
+})
+
+tap.test('signing works with only a buffer key array', function (t) {
+  const fastify = Fastify({ logger: false })
+
+  const key1 = Buffer.alloc(sodium.crypto_secretbox_KEYBYTES)
+  sodium.randombytes_buf(key1)
+
+  const key2 = Buffer.alloc(sodium.crypto_secretbox_KEYBYTES)
+  sodium.randombytes_buf(key2)
+
+  fastify.register(fastifySecureSession, {
+    key: [key1, key2]
+  })
+
+  fastify.post('/', (request, reply) => {
+    request.session.set('data', request.body)
+    reply.setCookie('my-session', JSON.stringify(request.body), {
+      httpOnly: true,
+      secure: true,
+      maxAge: 3600,
+      signed: true,
+      path: '/'
+    })
+    reply.send('session set')
+  })
+
+  fastify.get('/secure-session', (request, reply) => {
+    const data = request.session.get('data')
+    if (!data) {
+      reply.code(404).send()
+      return
+    }
+    reply.send(data)
+  })
+
+  fastify.get('/cookie-signed', (request, reply) => {
+    const data = request.unsignCookie(request.cookies['my-session'])
+    if (!data.valid) {
+      reply.code(404).send()
+      return
+    }
+    reply.send(data.value)
+  })
+
+  t.teardown(fastify.close.bind(fastify))
+  t.plan(7)
+
+  fastify.inject({
+    method: 'POST',
+    url: '/',
+    payload: {
+      some: 'data'
+    }
+  }, (error, response) => {
+    t.error(error)
+    t.equal(response.statusCode, 200)
+    t.ok(response.headers['set-cookie'])
+
+    const cookieHeader = response.headers['set-cookie'].join(';')
+
+    fastify.inject({
+      method: 'GET',
+      url: '/secure-session',
+      headers: {
+        cookie: cookieHeader
+      }
+    }, (error, response) => {
+      t.error(error)
+      t.same(JSON.parse(response.payload), { some: 'data' })
+
+      fastify.inject({
+        method: 'GET',
+        url: '/cookie-signed',
+        headers: {
+          cookie: cookieHeader
+        }
+      }, (error, response) => {
+        t.error(error)
+        t.same(JSON.parse(response.payload), { some: 'data' })
+      })
+    })
+  })
+})

test/key.js

@@ -135,3 +135,83 @@ tap.test('does not support key length greater than sodium "crypto_secretbox_KEYB
 
   await t.rejects(() => fastify.after())
 })
+
+tap.test('signing works with only a key', function (t) {
+  const fastify = Fastify({ logger: false })
+  const key = Buffer.alloc(sodium.crypto_secretbox_KEYBYTES)
+
+  sodium.randombytes_buf(key)
+
+  fastify.register(fastifySecureSession, {
+    key
+  })
+
+  fastify.post('/', (request, reply) => {
+    request.session.set('data', request.body)
+    reply.setCookie('my-session', JSON.stringify(request.body), {
+      httpOnly: true,
+      secure: true,
+      maxAge: 3600,
+      signed: true,
+      path: '/'
+    })
+    reply.send('session set')
+  })
+
+  fastify.get('/secure-session', (request, reply) => {
+    const data = request.session.get('data')
+    if (!data) {
+      reply.code(404).send()
+      return
+    }
+    reply.send(data)
+  })
+
+  fastify.get('/cookie-signed', (request, reply) => {
+    const data = request.unsignCookie(request.cookies['my-session'])
+    if (!data.valid) {
+      reply.code(404).send()
+      return
+    }
+    reply.send(data.value)
+  })
+
+  t.teardown(fastify.close.bind(fastify))
+  t.plan(7)
+
+  fastify.inject({
+    method: 'POST',
+    url: '/',
+    payload: {
+      some: 'data'
+    }
+  }, (error, response) => {
+    t.error(error)
+    t.equal(response.statusCode, 200)
+    t.ok(response.headers['set-cookie'])
+
+    const cookieHeader = response.headers['set-cookie'].join(';')
+
+    fastify.inject({
+      method: 'GET',
+      url: '/secure-session',
+      headers: {
+        cookie: cookieHeader
+      }
+    }, (error, response) => {
+      t.error(error)
+      t.same(JSON.parse(response.payload), { some: 'data' })
+
+      fastify.inject({
+        method: 'GET',
+        url: '/cookie-signed',
+        headers: {
+          cookie: cookieHeader
+        }
+      }, (error, response) => {
+        t.error(error)
+        t.same(JSON.parse(response.payload), { some: 'data' })
+      })
+    })
+  })
+})