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

Subresource integrity #106

Open
ruricolist opened this Issue Oct 5, 2015 · 17 comments

Comments

Projects
None yet
7 participants
@ruricolist

ruricolist commented Oct 5, 2015

It would be nice if you could specify a value for the integrity attribute of the link, to enforce subresource integrity.

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Oct 6, 2015

Member

Nice idea. +1

Member

scottjehl commented Oct 6, 2015

Nice idea. +1

@scottjehl scottjehl self-assigned this Oct 6, 2015

@zachleat

This comment has been minimized.

Show comment
Hide comment
@zachleat

zachleat Oct 6, 2015

Member

Ooh yes I like this.

Member

zachleat commented Oct 6, 2015

Ooh yes I like this.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Jan 30, 2017

I've opened #204 the other day without noticing this issue. I guess it's the same issue after all.

XhmikosR commented Jan 30, 2017

I've opened #204 the other day without noticing this issue. I guess it's the same issue after all.

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Feb 13, 2017

Member

As mentioned in #204, I'm not sure how this would work if the attribute is only allowed with link[rel=stylesheet]. We could polyfill it but that wouldn't help native support.

Member

scottjehl commented Feb 13, 2017

As mentioned in #204, I'm not sure how this would work if the attribute is only allowed with link[rel=stylesheet]. We could polyfill it but that wouldn't help native support.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Feb 14, 2017

@sideshowbarker : @scottjehl is right. If we use rel="stylesheet preload" this would not work in Chrome, would it?

XhmikosR commented Feb 14, 2017

@sideshowbarker : @scottjehl is right. If we use rel="stylesheet preload" this would not work in Chrome, would it?

@sideshowbarker

This comment has been minimized.

Show comment
Hide comment
@sideshowbarker

sideshowbarker Feb 14, 2017

If we use rel="stylesheet preload" this would not work in Chrome, would it?

Why would it not work? Neither the Preload spec nor the HTML spec prohibits it.

Or am I misunderstanding something?

What I do know is that the HTML spec defines the value of the rel attribute as a set of space-separated tokens and the HTML spec requires it to be parsed that way and for UAs to process it based on what the value contains, not on an exact match against the entire value.

sideshowbarker commented Feb 14, 2017

If we use rel="stylesheet preload" this would not work in Chrome, would it?

Why would it not work? Neither the Preload spec nor the HTML spec prohibits it.

Or am I misunderstanding something?

What I do know is that the HTML spec defines the value of the rel attribute as a set of space-separated tokens and the HTML spec requires it to be parsed that way and for UAs to process it based on what the value contains, not on an exact match against the entire value.

@sideshowbarker

This comment has been minimized.

Show comment
Hide comment
@sideshowbarker

sideshowbarker Feb 14, 2017

As mentioned in #204, I'm not sure how this would work if the attribute is only allowed with link[rel=stylesheet].

Per https://html.spec.whatwg.org/multipage/semantics.html#attr-link-integrity it’s allowed if the rel value contains stylesheet (not just is stylesheet). Since the rel attribute can contain a set of values, it can contain both stylesheet and preload and still conform to that requirement.

sideshowbarker commented Feb 14, 2017

As mentioned in #204, I'm not sure how this would work if the attribute is only allowed with link[rel=stylesheet].

Per https://html.spec.whatwg.org/multipage/semantics.html#attr-link-integrity it’s allowed if the rel value contains stylesheet (not just is stylesheet). Since the rel attribute can contain a set of values, it can contain both stylesheet and preload and still conform to that requirement.

@sideshowbarker

This comment has been minimized.

Show comment
Hide comment
@sideshowbarker

sideshowbarker Feb 14, 2017

My previous comments were mostly just about the context document-conformance requirements. I guess w3c/webappsec-subresource-integrity#26 is more relevant as far as the UA requirements.

sideshowbarker commented Feb 14, 2017

My previous comments were mostly just about the context document-conformance requirements. I guess w3c/webappsec-subresource-integrity#26 is more relevant as far as the UA requirements.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Feb 14, 2017

@scottjehl: in theory if we changed the check in preload to link.rel.indexOf("preload") !== -1 should work.

Can you verify?

XhmikosR commented Feb 14, 2017

@scottjehl: in theory if we changed the check in preload to link.rel.indexOf("preload") !== -1 should work.

Can you verify?

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Feb 14, 2017

Member

I'm not sure that specifying both values would behave as one would want... would it block or fetch async? I've not seen this spec'd or tested.

Member

scottjehl commented Feb 14, 2017

I'm not sure that specifying both values would behave as one would want... would it block or fetch async? I've not seen this spec'd or tested.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Feb 14, 2017

Please wait a few minutes so that @sideshowbarker explains the situation as discussed on IRC.

XhmikosR commented Feb 14, 2017

Please wait a few minutes so that @sideshowbarker explains the situation as discussed on IRC.

@sideshowbarker

This comment has been minimized.

Show comment
Hide comment
@sideshowbarker

sideshowbarker Feb 14, 2017

I'm not sure that specifying both values would behave as one would want... would it block or fetch async? I've not seen this spec'd or tested.

Yeah, see related discussion at http://logs.glob.uno/?c=freenode%23whatwg&s=14%20Feb%202017&e=14%20Feb%202017#c1020544 among @yoavweiss and @zcorpan and @annevk

sideshowbarker commented Feb 14, 2017

I'm not sure that specifying both values would behave as one would want... would it block or fetch async? I've not seen this spec'd or tested.

Yeah, see related discussion at http://logs.glob.uno/?c=freenode%23whatwg&s=14%20Feb%202017&e=14%20Feb%202017#c1020544 among @yoavweiss and @zcorpan and @annevk

@scottjehl

This comment has been minimized.

Show comment
Hide comment
@scottjehl

scottjehl Feb 16, 2017

Member

So @sideshowbarker it seems that folks want to allow SRI to work with preload alone, but it currently does not, and specifying rel="stylesheet preload" will currently block rendering. Is that correct? If so, should we wait on this one? We can polyfill it but we'd want native support to work as expected first...

Member

scottjehl commented Feb 16, 2017

So @sideshowbarker it seems that folks want to allow SRI to work with preload alone, but it currently does not, and specifying rel="stylesheet preload" will currently block rendering. Is that correct? If so, should we wait on this one? We can polyfill it but we'd want native support to work as expected first...

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Feb 17, 2017

Waiting for this to be figured out in browser implementation first seems very reasonable to me. Meanwhile you can use rel="preload" data-integrity="..." and add the integrity attribute at the same time as adding the stylesheet relation.

zcorpan commented Feb 17, 2017

Waiting for this to be figured out in browser implementation first seems very reasonable to me. Meanwhile you can use rel="preload" data-integrity="..." and add the integrity attribute at the same time as adding the stylesheet relation.

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Feb 17, 2017

(I don't know if there was an issue in reusing a sans integrity preloaded resource as an integrity stylesheet... @yoavweiss?)

zcorpan commented Feb 17, 2017

(I don't know if there was an issue in reusing a sans integrity preloaded resource as an integrity stylesheet... @yoavweiss?)

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Feb 17, 2017

Waiting for this to be figured out in browser implementation first seems very reasonable to me. Meanwhile you can use rel="preload" data-integrity="..." and add the integrity attribute at the same time as adding the stylesheet relation.

That is a cleaver workaround! Have you tried it in action?

XhmikosR commented Feb 17, 2017

Waiting for this to be figured out in browser implementation first seems very reasonable to me. Meanwhile you can use rel="preload" data-integrity="..." and add the integrity attribute at the same time as adding the stylesheet relation.

That is a cleaver workaround! Have you tried it in action?

@yoavweiss

This comment has been minimized.

Show comment
Hide comment
@yoavweiss

yoavweiss Feb 17, 2017

(I don't know if there was an issue in reusing a sans integrity preloaded resource as an integrity stylesheet... @yoavweiss?)

I wouldn't expect that to work, at least not with current code. I think handling of integrity and preloaded resources (link preload or otherwise) needs some work. See https://bugs.chromium.org/p/chromium/issues/detail?id=677022

yoavweiss commented Feb 17, 2017

(I don't know if there was an issue in reusing a sans integrity preloaded resource as an integrity stylesheet... @yoavweiss?)

I wouldn't expect that to work, at least not with current code. I think handling of integrity and preloaded resources (link preload or otherwise) needs some work. See https://bugs.chromium.org/p/chromium/issues/detail?id=677022

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