Amphtml: Support Subresource Integrity

[alech]

I think it would be a benefit if SRI (http://www.w3.org/TR/SRI/) was supported, i.e. the example should have an integrity attribute for the script tag and the CDN-server should be CORS-enabled in order to make the verification of the hash possible.

[dvoytenko]

That looks like a good idea. @cramforce wdyt? Generally we serve everything via HTTPS, but still...

[jridgewell]

HTTPS doesn't guarantee the integrity of the linked file. The Github Engineering Blog explains why this is important now that everything is served via CDNs.

[mozfreddyb]

SRI spec editor here. Happy to help with whatever question you may have.

[cramforce]

I think for AMP's primary JS files this cannot be used as AMP changes over time and those changes cannot require changing documents.

I think the security benefits of not letting docs lock themselves into insecure versions of AMP outweigh the risk of insecure versions of AMP.

[fulldecent]

We are not concerned about insecure versions of amp; we are concerned with insecure versions of NOT amp. Integrity guarantees that something an author links to is the same thing a viewer gets.

[cramforce]

I understand, but against SRI is fundamentally incompatible with AMP. AMP, as it stands, cannot be used without trusting https://cdn.ampproject.org to deliver only official AMP versions.

I think what would make a lot of sense is to document how external parties can verify that the delivered sources are actually build from the open source repositiory.

[Gregable]

@cramforce, what do you think about allowing these for stylesheets, e.g.:

<link rel=stylesheet href='https://fonts.googleapis.com/css?family=Lato|Bad+Script' integrity="...">

[cramforce]

@Gregable we can definitely allow integrity on all non-JS resource elements. for amp-img and similar it would require a small JS change,

[Gregable]

Hm, I don't think this attribute exists except for <link> and <script>, at least not according to https://www.w3.org/TR/SRI/#elements

So, I don't think there is any reason to implement code for <amp-img>

[cramforce]

Definitely +1 on allowing it on link

[fulldecent]

To be clear: this is issue is concerned with AMP creating versioned and integrity-verified releases of AMP. Issues related to font CSS integrity are separate and I have created an issue https://github.com/ampproject/amphtml/issues/7060 to discuss that separately. Also that includes more specific action points to that issue.

[fulldecent]

Following is an action plan for correcting this instant issue of SRI for AMP. Note: this action plan require zero changes from the installed base of authors using AMP and it additionally fixes this issue.

• [ ] Make a release NOW using a real version number.
  
  
  
  • Discussion: the latest version of this project is 1484265443309.
  
  
  • Recommendation: release https://cdn.ampproject.org/v0.js as https://cdn.ampproject.org/v0.0.1-1484265443309.js
  
  
• [ ] Commit to permanently publishing versioned releases on CDN.
  
  
  
  • Discussion: this is the official policy of EVERY other CDN. One example: https://www.bootstrapcdn.com/ shows the current and all prior versions.
  
  
• [ ] Create redirects to accomplish the current AMP project's workflow.
  
  
  
  • Discussion: Currently you can reference https://cdn.ampproject.org/v0.js and get the latest version of AMP, with updates being applied automatically. WE CAN KEEP THIS RECOMMENDED USE CASE. Simply create a redirect (303) from https://cdn.ampproject.org/v0.js to https://cdn.ampproject.org/v0.0.1-1484265443309.js.
  
  
• [ ] Update documentation to announce support for SRI and continue recommending to NOT use SRI.
  
  
  
  • Discussion: This note can be added: "We recommend that you link to the latest version of AMP by using https://cdn.ampproject.org/v0.js However, for application that require subresource integrity validation, you can link to a specific version, which is guaranteed not to change. An example with SRI is:
    

Following is additional related work to bring the AMP project in line with current best practices:

• [ ] Officially adopt the unaltered semantic versioning for this project.
  
  
  
  • Discussion: because other developers will depend on this project and must build around it, then therefore this project has a public API. As other developers use this API, it is important for them to quickly understand if an updated version requires changing their usage of the API. Semantic versioning is a well-understood pattern to adhere to. See http://semver.org/.
  
  
  • [ ] Update release versioning documentation at https://github.com/ampproject/amphtml/blob/5264bd033a1e449d579f7691faa1eb14e5d087b3/spec/amp-versioning-policy.md.
  
  
  • [ ] Update release numbering at https://github.com/ampproject/amphtml/blob/master/build-system/release.sh.
  
  
• [ ] Release a version 1.0.0 or greater.
  
  
  
  • Discussion: semver.org answers the question How do I know when to release 1.0.0?. Google is currently caching and serving AMP pages from its homepage, yes this counts as production use, so yes we are already at 1.0+.
  
  

[cramforce]

AMP is an evergreen JavaScript library. We will not allow users to lock themselves into a particular release. Using an old version of AMP is bad for the same reason of using an old version of Chrome.

If link is covered in a separate issue we can close this.

[fulldecent]

@cramforce Could you please link to where this "evergreen JavaScript library" concept is documented as a best practice?

Please note that the work plan above does allow AMP to continue recommending that developers link to the always-latest version of the project, as is currently the case today.

In your comment you imply that using AMP may have unknown/undisclosed security or usability implications and that they are mitigated by having the AMP project decide when to release new AMP versions. This is even more reason for authors to carefully study and complicate the implications of adopting AMP for their project. It is only possible for downstream adopters to study AMP if they are able to review a specific, versioned release.

[cramforce]

https://github.com/ampproject/amphtml/releases has all releases documented.

I should at some point write a retrospective on the evergreen JS library concept.

[fulldecent]

Just to clarify for anyone that does not know about SRI, the main goal of SRI is that:

Compromise of a third-party service should not automatically mean compromise of every site which includes its scripts.

"Evergreen JS" is NOT compatible with SRI. Additionally, the AMP specification states that:

The AMP runtime is loaded via the mandatory <script src="https://cdn.ampproject.org/v0.js"></script> tag

Therefore, compromise of a third-party service would automatically mean compromise of every site using AMP.

[cramforce]

That is correct.

Pages can, of course, choose to serve their own version of AMP. They won't be considered valid AMP, but otherwise can take advantage of all its features.

[fulldecent]

That would not count as AMP and does not solve the current issue.

Also, another note: Gary Illyes, Webmaster Trends Analyst at Google was present at SEJ Summit Chicago and stated that:

"Currently, AMP is not a mobile ranking factor."

In other words, he failed to state that Google would not use AMP as a ranking signal.

---

In summary we have:

Google has made a statement that they may feature websites less prominently unless they reduce their security and allow their websites to be vulnerable to a compromise of Google servers.

I know we are all engineers here, but perhaps a Google lawyer should be interpreting the implications of that statement above.

[cramforce]

For Google specific questions, please refer to the Google Search & AMP forum. https://goo.gl/utQ1KZ This is not a very good venue for such questions as nobody who works on Google Search regularly hangs out here.

Closing this. We'll put in the change for link tags.