jquery / codeorigin.jquery.com

jQuery CDN
https://releases.jquery.com
Other
57 stars 601 forks source link

Add support for SRI #20

Closed jonathanKingston closed 8 years ago

jonathanKingston commented 9 years ago

As a CDN provider it will soon become good practice to provide integrities. See: https://github.com/jsdelivr/jsdelivr/issues/6029 https://github.com/cdnjs/cdnjs/issues/4599

For all static assets that never change can integrities be added?

I'm here to help if anything is needed.

scottgonzalez commented 9 years ago

Doesn't seem like it'd be too hard to add. Are there any browsers that support SRI yet? I wouldn't want to add this until we can properly test it.

acorncom commented 9 years ago

Looks like it's currently in the nightlies for both Firefox and Chrome, but hasn't hit stable yet ...

jonathanKingston commented 9 years ago

Chrome unstable has it in.

The readme of my add-on might be useful here: https://www.npmjs.com/package/ember-cli-sri

mozfreddyb commented 9 years ago

Bootstrap CDN already shows integrity attributes in its HTML examples, see http://www.bootstrapcdn.com/ and click on the arrows.

Any chances you may want to show these too in your website?

cc @aulvi, whith whom I spoke before about adding Access-Control-Allow-Origin headers to code.jquery.com.

kborchers commented 8 years ago

Can we circle back to this now? Chrome stable now supports this and Firefox will support it in 43 (December 2015).

jdorfman commented 8 years ago

@kborchers @scottgonzalez getting this out would be a huge win for Web Application security. Please let me know if I can be of help in anyway.

scottgonzalez commented 8 years ago

Sure, you can send a PR. Everything is built in the Gruntfile. Thanks!

jdorfman commented 8 years ago

@scottgonzalez sweet I will see what I can do.

scottgonzalez commented 8 years ago

Let me know if you have any questions about how everything is built.

jdorfman commented 8 years ago

@scottgonzalez will do. Going to consult with @jonathanKingston regarding UX. Do you have any preferences in terms of how the code snippets (with the SRI hash) should look? Here is how we do it on BootstrapCDN:

bootstrapcdn_by_maxcdn

scottgonzalez commented 8 years ago

I don't have any preference, but I expect this to require a major redesign.

jdorfman commented 8 years ago

@jmervine and I are going to come up with something simple and elegant that will work with the current design. First step will be configuring the Gruntfile.

jmervine commented 8 years ago

@scottgonzalez @jdorfman Generating shouldn't be an issue -- it will require a host that supports running openssl (see: bootstrap-cdn:scripts/integrity.js implementation).

@jdorfman Any direction on updating the UI would be fantastic, otherwise, I'll just drop it in there somewhere and we can discuss making it pretty as a follow up.

jonathanKingston commented 8 years ago

Sorry for the delay here, what https://www.jsdelivr.com/ do is quite nice and slightly less invasive explaining technology that people might not understand. Requiring developers to enable could actually entice more thought that fancy badges or lengthy explanations do. I think easy copy facility would be worth having as the output is longer and potentially not completely visible.

The approach I have taken is adding in multiple SRI hashes into the integrity attribute which I feel somewhat explains the fail over principle of the algo used even though totally not required (However my use case installs it into the application anyway). So feel free to use one here if it simplifies the output.

I think a press release informing the user that SRI is good and that the CDN is going out of their way to protect the users of websites should reduce the number of complaints if any.

Other than that, I think keeping it simple actually works in selling this (developers mostly are inquisitive of things they have not seen before)

jdorfman commented 8 years ago

@jonathanKingston I agree with everything you said. Hopefully we will have something ready to present soon.

jdorfman commented 8 years ago

@scottgonzalez @jonathanKingston please give me your feedback before we go forward. It is heavily inspired from jsDelivr, which I checked with @jimaek first and got his blessing.

jquery_cdn-sri

scottgonzalez commented 8 years ago

I think there are a few things we can do to improve the usability of that form:

jdorfman commented 8 years ago

@scottgonzalez I will take this awesome feedback and get back asap. Thank you.

jdorfman commented 8 years ago

@scottgonzalez how about this?

popup

scottgonzalez commented 8 years ago

That looks better. Maybe we can just drop the https option and always include the protocol. We also need a tooltip on the SRI option.

jdorfman commented 8 years ago

@scottgonzalez couldn't agree more. With that feedback could I skip the next mockup and start the code?

scottgonzalez commented 8 years ago

I wonder what this would look like if we dropped all of the options and just provided the full set of data to the user. This would result in four copyable blocks:

I could even see us only providing the script tag with SRI since it's more secure and doesn't cause any issues unless the user manually changes the URL to point to a different source. This would probably result in a similarly sized dialog, but removes the cognitive overhead of deciding if any options should change. This does add cognitive overhead for deciding which block to copy, but I feel like users already know whether they want just the URL or a full script tag when they come here.

What do you think?

jdorfman commented 8 years ago

@scottgonzalez Good call. I will get that mocked up.

jdorfman commented 8 years ago

@scottgonzalez after thinking about it, I think the current mockup will cause less confusion. I will still get the other mockup done so we can get some other expert opinions from the jQuery community. I am in no way saying your feedback is wrong or bad I just want a second opinion.

scottgonzalez commented 8 years ago

No problem. I honestly don't know which will be better.

jdorfman commented 8 years ago

@scottgonzalez Choose one. I personally like the first image because it has less choices, making a better UX. I think ultimately we should have the jQuery community decide. Edit: There will be a tooltip for Enable SRI, just forgot to add it.

popup2

popup3

scottgonzalez commented 8 years ago

Oy, I imagined that somehow looking less intimidating without the checkboxes. Definitely the first.

fmarier commented 8 years ago

Pardon me for asking a stupid question, but why not make it extremely simple and simply show the full script tag with SRI?

People who just want the URL can trivially copy just that part of the text and those who copy the whole thing get the best script tag they can.

jonathanKingston commented 8 years ago

@fmarier :+1:

Make the src option first in the tag and just use that?

<script src="https://code.jquery.com/jquery-2.1.4.min.js" integrity="sha256-..." crossorigin="anonymous" ></script>
jdorfman commented 8 years ago

@fmarier please ignore my last deleted comment. I agree with you and @jonathanKingston. Keep it simple. @scottgonzalez your thoughts?

scottgonzalez commented 8 years ago

I'm good with that.

We could potentially remove the "Copy Below" heading and just put two buttons in that area: "Copy full script element" and "Copy just the URL". I don't have strong feelings on that though.

jdorfman commented 8 years ago

@scottgonzalez @jonathanKingston @fmarier

Final?

jquery_cdn-sri-2

scottgonzalez commented 8 years ago

Looks good, but I still think this needs information about SRI.

jonathanKingston commented 8 years ago

@jdorfman this is super picky. This looks like you are not using a text field to render this, if so would it be possible to wrap each attribute in a non-wrapping tag to prevent a line ending in a hyphen which may increase copying errors (media query could be used to disable on a phone etc)?

<style>
.copy-thing span {
  word-wrap: normal;
  white-space: nowrap;
}
</style>
<div class="copy-thing">
  &lt;script
  <span>src="https://code.jquery.com/jquery-2.1.4.min.js"</span>
  <span>integrity="sha512-vyuJDNBgAzdNxzVRs+bQX0Re2dkaVnnklTNi5SKIboJB8mu1OvDzKEbhRVaHScEY1FXY6rLX9q+cY1ObdLe1bw=="</span>
  <span>crossorigin="anonymous"</span>
  &gt;&lt;/script&gt;
</div>

Looks good, but I still think this needs information about SRI.

Tooltip on the tags above?

jdorfman commented 8 years ago

@scottgonzalez Good call, let me come up with something regarding the SRI explanation.

@jonathanKingston Thank you, I will use that in the next mock up. =)

jdorfman commented 8 years ago

@scottgonzalez @jonathanKingston Please note that this is my mediocre Photoshop "skills", but before I send it to the designer I want to see if we are on the right track:

jquery_cdn-sri3

Leave your feedback and I will give it to him and hopefully start coding. Thanks!

jonathanKingston commented 8 years ago

@jdorfman I meant more:

<style>
.copy-thing span {
  word-wrap: normal;
  white-space: nowrap;
}
.copy-thing span.tooltip {
  border-bottom: 2px solid #eee;
}
</style>
<div class="copy-thing">
  &lt;script
  <span>src="https://code.jquery.com/jquery-2.1.4.min.js"</span>
  <span class="tooltip">integrity="sha512-vyuJDNBgAzdNxzVRs+bQX0Re2dkaVnnklTNi5SKIboJB8mu1OvDzKEbhRVaHScEY1FXY6rLX9q+cY1ObdLe1bw=="</span>
  <span>crossorigin="anonymous"</span>
  &gt;&lt;/script&gt;
</div>

This also gives the advantage of being able to explain the crossorigin attr.

jdorfman commented 8 years ago

@jonathanKingston sorry forgot to add that. I will send that feedback to the designer and report back.

scottgonzalez commented 8 years ago

Now that we're down to a single tooltip, maybe we should drop the tooltip idea and just put the explanation right below the script element. We should include a link to a more detailed explanation as well. Do you know of a good article we can link to?

jdorfman commented 8 years ago

@scottgonzalez sounds good. For the article how about srihash.org? e.g.:

Read more at srihash.org

scottgonzalez commented 8 years ago

PERFECT!

jdorfman commented 8 years ago

Latest draft. I am not happy with the code block's font size, but not sure if there is anything that can be done. I will leave that up to the experts.

jquery_cdn-sri4

That is not the final SRI explanation, just an example of what we discussed about above.

jdorfman commented 8 years ago

@jonathanKingston @scottgonzalez what do you think?

fmarier commented 8 years ago

/me loves the simplicity and lack of unnecessary options :)

jdorfman commented 8 years ago

@fmarier I agree, thanks for leading us there =)

scottgonzalez commented 8 years ago

I think you're good to go ahead and start the implementation.

@jquery/content any further input?

jdorfman commented 8 years ago

@scottgonzalez great! @jmervine and I will have something delivered soon.

kswedberg commented 8 years ago

LGTM!

jdorfman commented 8 years ago

@kswedberg =)

AurelioDeRosa commented 8 years ago

LGTM too.