New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix or remove CSP detect #1461

Closed
patrickkettner opened this Issue Nov 27, 2014 · 13 comments

Comments

Projects
None yet
2 participants
@patrickkettner
Member

patrickkettner commented Nov 27, 2014

Our CSP detect in 3.0 is 100% broken, since the javascript interface was removed almost a year ago while a better API was being worked on.

Looking over the spec, I can't really find any foothold to detect it, so I reached out to @mikewest @dveditz, and @abarth on twitter

thanks to @tylershambora for pointing this out in #1451

@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Nov 27, 2014

My gut is that we could inject a <meta> tag version of CSP into a local iframe or something and see if we get an error for some inline js. Dunno if it'd be worth it though, there's no fallback.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Nov 27, 2014

yeah, iframe was my thought too. I'm checking to see if I can fix any of the other things brought up in #1451 before diving deep on a fix for this

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Nov 28, 2014

This is my first pass at it

define(['Modernizr', 'addTest'], function(Modernizr, addTest, createElement) {

  Modernizr.addAsyncTest(function() {

    if (!('srcdoc' in iframe)) {
      return addTest('contentsecuritypolicy', false);
    }

    var iframe = createElement('iframe');

    iframe.srcdoc = "<html><head><meta http-equiv='Content-Security-Policy' content='script-src \'self\'></head><body><style>body{color:rgb(255, 0, 0)!important}</style><script>document.body.style.color='rgb(0 ,0, 255)'<\/script></body></html>";
    document.body.appendChild(iframe);

    function getResult(timeout, cb) {
      setTimeout(function() {
        try {
          result = getComputedStyle(iframe.contentDocument.body).color;
          if (result.indexOf('255') === -1) {
            throw Error
          };
          cb(result === 'rgb(255, 0, 0)');
        } catch (e) {
          getResult(timeout, cb);
        }
      }, timeout);
    }

    getResult(5, function(result) {
      addTest('contentsecuritypolicy', result);
    });
  });
});

thoughts @SlexAxton ?

@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Nov 28, 2014

Seems like you could just inline a global variable and then check for it.

Then test for iframe.contentWindow.modcsptest === 5 onload.

It'd be nifty to data uri the iframe contents into the src, but I think IE
will eat it.

On Thu, Nov 27, 2014, 6:31 PM patrick kettner notifications@github.com
wrote:

This is my first pass at it

define(['Modernizr', 'addTest'], function(Modernizr, addTest, createElement) {

Modernizr.addAsyncTest(function() {

if (!('srcdoc' in iframe)) {
  return addTest('contentsecuritypolicy', false);
}

var iframe = createElement('iframe');

iframe.srcdoc = "<html><head><meta http-equiv='Content-Security-Policy' content='script-src \'self\'></head><body><style>body{color:rgb(255, 0, 0)!important}</style><script>document.body.style.color='rgb(0 ,0, 255)'<\/script>hi</body></html>";
document.body.appendChild(iframe);

function getResult(timeout, cb) {
  setTimeout(function() {
    try {
      result = getComputedStyle(iframe.contentDocument.body).color;
      if (result.indexOf('255') === -1) {
        throw Error
      };
      cb(result === 'rgb(255, 0, 0)');
    } catch (e) {
      getResult(timeout, cb);
    }
  }, timeout);
}

getResult(5, function(result) {
  addTest('contentsecuritypolicy', result);
});

});
});

thoughts @SlexAxton https://github.com/SlexAxton ?


Reply to this email directly or view it on GitHub
#1461 (comment)
.

@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Nov 28, 2014

Sigh. Inbox doesn't like html much...

@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Nov 28, 2014

<script>window.modcsptest=5;</script> as the inline test

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Nov 28, 2014

I tried that, however since we are adding an iframe to the page, there is a delay before the page goes up. as a result, it could be undefined because of CSP, or because of the race condition. Using a style with a known fallback gives us the ability to know wether or not one of the two options has actually been hit.

@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Nov 28, 2014

Even if we wait for onload?

On Thu, Nov 27, 2014, 6:42 PM patrick kettner notifications@github.com
wrote:

I tried that, however since we are adding an iframe to the page, there is
a delay before the page goes up. as a result, it could be undefined because
of CSP, or because of the race condition. Using a style with a known
fallback gives us the ability to know wether or not one of the two options
has actually been hit.


Reply to this email directly or view it on GitHub
#1461 (comment)
.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Nov 28, 2014

duh.

define(['Modernizr', 'addTest'], function(Modernizr, addTest, createElement, getBody) {

  Modernizr.addAsyncTest(function() {

   var iframe = createElement('iframe');
   var body;

    if (!('srcdoc' in iframe)) {
      return addTest('contentsecuritypolicy', false);
    } 

    body = getBody(); 

  iframe.onload = function() {
    var supported = !iframe.contentWindow.csp
    body.removeChild(iframe)
    addTest('contentsecuritypolicy', supported);
  };

  iframe.srcdoc = "<html><head><meta http-equiv='Content-Security-Policy' content='script-src \'self\'></head><body><script>window.csp = true</script></body></html>";

  body.appendChild(iframe);
}
@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Nov 28, 2014

Gutcheck it on a few browsers and shiiiiip.

On Thu, Nov 27, 2014, 6:56 PM patrick kettner notifications@github.com
wrote:

duh.

define(['Modernizr', 'addTest'], function(Modernizr, addTest, createElement) {

Modernizr.addAsyncTest(

function() {

var iframe = document.createElement('iframe');

if (!('srcdoc' in iframe)) {
  return addTest('contentsecuritypolicy', false);
}

iframe.onload = function() {
var supported = !iframe.contentWindow.csp
document.body.removeChild(iframe)
addTest('contentsecuritypolicy', supported);
};

iframe.srcdoc = "<meta http-equiv='Content-Security-Policy' content='script-src 'self'><script>window.csp = true</script>";

document.body.appendChild(iframe);
}
``


Reply to this email directly or view it on GitHub
#1461 (comment)
.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Nov 28, 2014

yep yep. got some pie to eat first.

biggest concerns are relying on srcdoc, and false negatives on browsers that support it via headers, but its better than what we have now I suppose...

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Dec 1, 2014

So firefox doesn't support this, and IE only supports it via a header. This feels like too many false negatives to ship. I am leaning towards pulling the detect until the js api lands later on

@SlexAxton

This comment has been minimized.

Member

SlexAxton commented Dec 1, 2014

Good catch. Agreed w/ ff info.

On Mon, Dec 1, 2014, 2:40 AM patrick kettner notifications@github.com
wrote:

So firefox doesn't support this
https://bugzilla.mozilla.org/show_bug.cgi?id=663570, and IE only
supports it via a header. This feels like too many false negatives to ship.
I am leaning towards pulling the detect until the js api lands later on


Reply to this email directly or view it on GitHub
#1461 (comment)
.

patrickkettner added a commit to patrickkettner/Modernizr that referenced this issue Feb 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment