Skip to content
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

graphql: check the integrity of the cdn files #19742

Merged
merged 2 commits into from
Jun 24, 2019

Conversation

kurkomisi
Copy link
Contributor

@kurkomisi kurkomisi commented Jun 20, 2019

Supersedes #19727

The graphql explorer relies on external cdn files, and verifying their integrity guarantees that the fetched resources have been delivered without unexpected manipulation.

For the mechanism of the subresource integrity check see https://www.w3.org/TR/SRI/.
https://www.srihash.org/ is used to calculate the correct hashes.
Thanks @holiman for the pinpoints.

@kurkomisi kurkomisi force-pushed the graphql-asset-integrity-check branch from 81e4466 to 3a4f61c Compare June 20, 2019 12:51
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (haven't tested the PR though, but the code looks ok)

@karalabe
Copy link
Member

This PR seems a bit complicated now. If we're not embedding the network resources, then I think we should omit the whole gobindata dance and just leave the index content as a string in there as it was originally. Simpler to edit.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@karalabe karalabe merged commit 92a90d7 into ethereum:master Jun 24, 2019
@karalabe karalabe added this to the 1.9.0 milestone Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants