krakenjs / lusca

Application security for express apps.
Other
1.78k stars 123 forks source link

Change csp api to handle more directives #79

Closed jasisk closed 8 years ago

jasisk commented 8 years ago

This change is long overdue and has had a couple attempts by others (sorry, folks!).

Implemented a more flexible, yet backwards-compatible csp api. Csp policies are more fluid than our current api supports with things like argument-less directives (e.g., block-all-mixed-content).

New api supports string policies for maximum flexibility, array policies (just fancy string concatenation), and the old object policy api.

Big thanks to @turboMaCk and @giladgo for their PRs ages ago. I've thrown commits from you two into this PR to ensure they're at least captured in the repo.

Closes #69, #71, #72.

edit does introduce a behavior change but only the case of a failed policy. Previously would either result in a context-less throw or fail silently. Now, an unhanded policy type throws explicitly.

turboMaCk commented 8 years ago

100% agree that this is better solution! Nice work :+1:

giladgo commented 8 years ago

Nice work :+1: tbh, I'm a bit confused, does this code allow writing things like

"policy": {
  "default-src": ["'self'", "https://*.google-analytics.com", "https://*.bla.com"]
}

? If yes, can we add a test case for this?

jasisk commented 8 years ago

If yes, can we add a test case for this?

It does not. The equivalent would be:

{
  "policy": "default-src 'self' https://*.google-analytics.com https://*.bla.com"
}

// or, as is already supported:
{
  "policy": {
    "default-src": "'self' https://*.google-analytics.com https://*.bla.com"
  }
}

... or, if one of many directives:

{
  "policy": [
    "default-src 'self' https://*.google-analytics.com https://*.bla.com",
    "image-src *"
  ]
}

// or
{
  "policy": [
    {
      "default-src":  "'self' https://*.google-analytics.com https://*.bla.com"
    },
    "image-src *"
  ]
}