1
\$\begingroup\$

This working code inserts a nonce attribute and value for each style and script tag inside an HTML file, for each GET request. It is using Express. nonce1 and dirViews are already defined. limiter is for rate limiting and is a node module found on npm.

//on get homepage
app.get('/', limiter.middleware(), function(req, res, next) {
    //send an edited index.html containing nonces
    fs.readFile(dirViews+'/index.html', 'utf8', function(err, html){
        let newHTML = html.replace(/<script/g, '<script nonce="'+nonce1+'"').replace(/<style/g, '<style nonce="'+nonce1+'"');
        res.send(newHTML);
    });
});

I am unsure if this is the correct way to do this (set nonce attributes on each GET request), and whether my code follows best practices.

\$\endgroup\$
  • \$\begingroup\$ please can you make a suggestion for improving my question, since it was downvoted? I have already read /help/how-to-ask etc. \$\endgroup\$ – George Nov 12 '17 at 20:21
  • \$\begingroup\$ Does your code actually work? As in, it accomplishes the task you mean it to? \$\endgroup\$ – Phrancis Nov 13 '17 at 0:03
  • \$\begingroup\$ yes it does.... \$\endgroup\$ – George Nov 13 '17 at 11:27
3
\$\begingroup\$

I am unsure if this is the correct way to do this (set nonces on each get)

Yes, it is

Remember that nonces must be regenerated for every page request and they must be unguessable.

In terms of best practises, couple of things I'd recommend:

  • Move your nonce manipulation into it's own middleware
  • Cache your page content, don't read it from disk every time

The end result would look something like:

csp.js

const path = require('path');
const fs = require('fs');

const cache = new Map();

const getHtml = path => new Promise((resolve, reject) =>
  fs.readFile(path, 'utf-8', (err, contents) => {
    if (err) return reject(err);

    cache.set(path, contents);
    return resolve(contents);
  }));

const csp = page => async (req, res, next) => {
  const pagePath = path.join(dirViews, page);
  let html = cache.get(pagePath);
  if (!html) {
    html = await getHtml(pagePath);
  }
  html = html.replace(/<script/g, `<script nonce="${nonce1}"`)
    .replace(/<style/g, `<style nonce="${nonce1}"`);
  res.send(newHTML);
}

export default csp;

app.js

app.get('/', limiter.middleware(), csp('index.html'))
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Hopefully the response by James is sufficient for your concern about best practices. I did notice some redundancy in that snippet of Javascript.

Specifically, the line that handles adding the nonce attribute to the script and style tags could be simplified:

let newHTML = html.replace(/<script/g, '<script nonce="'+nonce1+'"').replace(/<style/g, '<style nonce="'+nonce1+'"');

Since it already uses regular expressions (e.g. /<script/g), a capturing group could be added with the two cases for tag names: (i.e. script or style): (script|style) combined with an OR operator (i.e. |), and refer to that value using a back-reference (i.e. $1 for the first captured group) when adding the nonce attribute (in the replacement string parameter).

See a demonstration of this in the snippet below (should be portable to NodeJS).

const html = document.head.innerHTML;
const nonce1 = 'blablabla12125125';
let newHTML = html.replace(/<(script|style)/g, '<$1 nonce="' + nonce1 + '"');
console.log(newHTML);

That way there is only one call to String.replace(). While it might not save much processing time or power, it can at least make the line shorter and hopefully easier to read. In addition, if there was a need to add the attribute to additional tags, that tag name could easily be added to the expression in the capturing group.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Use of capture group definitely better :) \$\endgroup\$ – James Feb 17 '18 at 8:37
  • \$\begingroup\$ It is not using best practices, use stream instead of readFile into memory \$\endgroup\$ – George Feb 17 '18 at 9:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.