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

Provide optional SRI hash when displaying embed code #565

Closed
m90 opened this issue Feb 26, 2021 · 22 comments
Closed

Provide optional SRI hash when displaying embed code #565

m90 opened this issue Feb 26, 2021 · 22 comments

Comments

@m90
Copy link
Member

m90 commented Feb 26, 2021

Some operators might prefer to embed the Offen script with an integrity attribute to prevent eventual MITM scenarios.

The embed code box in the Auditorium could provide a toggle for displaying the snippet with such an SRI value added. The default behavior should still be using no hash (as this is easier to upgrade).

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 20, 2022

At what point do you think the integrity hash should be created for the JS file?

My first guess would be to generate it on application / server start how ever I'm not quite sure whether it is possible that the JS file (and therefore the corrosponding hash) can update during runtime?

@m90
Copy link
Member Author

m90 commented Jul 20, 2022

how ever I'm not quite sure whether it is possible that the JS file (and therefore the corrosponding hash) can update during runtime?

The only time when it could update would be at update time (which is why having the SRI hash should not be the default: it could break on update).

Computing the hash could probably happen in two ways:

  • at build time, the hash of the asset is computed and passed to the go build command, "baking" it into the binary
  • at run time, the server calculates the hash whenever the Auditorium view is rendered

I think both options are fine, and in case you are planning to implement this (?) I'd probably pick the option you are more familiar with. Let me know if I can provide any further help.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 20, 2022

Thanks for your thoughts!
I might do it. I'm currently evaluating my implementation approach.

My approach would be something like the following:

  1. On auditorium page instantiation create a hash clientside via JS (I hope that works)
  2. Add a slider button inside the auditorium to either chose between SRI version / None SRI version
    • Is it okay for you to add additional dependencies like this one?
    • In case: just run npm install ... inside the auditorium part? (This lead to some error for me)
    • Render selected script for embedding
  3. I did not look at content policies yet. Probably we do not need to change anything here.

I think that's it?

@m90
Copy link
Member Author

m90 commented Jul 20, 2022

Is it okay for you to add additional dependencies like this one

Tagging @hendr-ik who might have a picture of how such a feature could look like.

On auditorium page instantiation create a hash clientside via JS (I hope that works)

This should probably work if you do it on demand only.

In case: just run npm install ... inside the auditorium part? (This lead to some error for me)

We're stuck on npm 6 here because of the project layout and some npm issues
, so I would advise to use the docker-compose setup for development. After cloning

make setup
make bootstrap
make up

should give you a working development environment.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 20, 2022

The docker-compose setup worked like a charm.
The following error how ever occurs:

/private-projecs/offen/auditorium$ - sudo npm i react-toggle-slider
npm WARN deprecated [email protected]: Redundant dependency in your project.

> [email protected] install /home/ms/private-projecs/offen/auditorium/node_modules/puppeteer
> node install.js

ERROR: Failed to set up Chromium r970485! Set "PUPPETEER_SKIP_DOWNLOAD" env variable to skip download.
{ [Error: EACCES: permission denied, mkdir '/home/ms/private-projecs/offen/auditorium/node_modules/puppeteer/.local-chromium']
  errno: -13,
  code: 'EACCES',
  syscall: 'mkdir',
  path:
   '/home/ms/private-projecs/offen/auditorium/node_modules/puppeteer/.local-chromium' }

This might be a local probmen though.
Lets see what @hendr-ik says.

I think I will get this going somehow and hopefully come back with a PR.

@m90
Copy link
Member Author

m90 commented Jul 20, 2022

In the docker-compose setup, you need to run every command inside the matching container. I.e. your command would turn into

docker-compose run --rm auditorium npm i react-toggle-slider

The node_modules used are hidden in a Docker volume.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 24, 2022

Thanks! That worked.

I'm currently thinking about what would be the best way to retrieve the content of the script.js file to generate a hash from the auditorium at runtime?

Since I'm not really familiar with preact/react I'm wondering what the best way to do it.
A quick and dirty approach to generate a hash would be:

fetch(`"${window.location.origin}/script.js"`).then((r)=>{r.text().then((d)=>{ 
console.log(CryptoJS.SHA1(d).toString());
})});

My current understanding how ever is that preact does server side rendering (?) so it would be possible to spare that HTTP fetch call, right? Otherwise I would do the fetch and hash creation async on page load of embed-code.js.

Could you maybe share your thoughts on that?

@m90
Copy link
Member Author

m90 commented Jul 24, 2022

Unfortunately I forgot about one thing when I wrote my last comment: if you compute the hash off of a version that you fetch at runtime, the very calculation is vulnerable to MITM attacks, which is what the SRI hash is even supposed to prevent. I.e. if someone compromises the script.js asset somehow without you noticing, you will happily add the integrity value of the compromised asset. I'm sorry I didn't catch this earlier and I hope you didn't invest too much time in it yet.

Moving on, I can see two other ways to implement this:

Compute hash at build time

The build for auditorium assets could pull in the previously built script assets (plural as there are multiple locales available) and compute their hashes. These values are then injected into the build as environment variables and the code could just read them from process.env.SCRIPT_INTEGRITY_HASH or similar.

This is probably the "safest" way to do it.

Compute hash on the server

Alternatively, the hash could be computed by the Go application at runtime. This value is then injected into the rendered HTML template for the Auditorium which can then read the hash from a global variable or similar.


Do you think that makes sense? Let me know if I should clarify anything or if you think that I'm missing something.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 24, 2022

Makes sense. I should have come up with that by myself.
I'll probably experiment with computing the hash at build time in that case and get back to you.

It is probably the best way to create one environment variable for each locale?

Thanks for your input! Really helpful.

@m90
Copy link
Member Author

m90 commented Jul 24, 2022

It is probably the best way to create one environment variable for each locale?

The script asset is bundled once per locale, so in the code we could always be using the same key on 'process.env'. The relevant place in the code would be:

if (transform === 'envify' || (Array.isArray(transform) && transform[0] === 'envify')) {
return ['envify', { LOCALE: locale }]
}
where you could add the value.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 25, 2022

I see. I'll take a look at it. Thank you.

Will defining the envrionment variable inside the gulpfile be enough to make it available inside the embed-code.js file?

Adding the envrionment variable always result in being undefined in the embed-code.js.
How ever I was just quickly taking a look at it and did not fully grasp the gulpfile code.

@m90
Copy link
Member Author

m90 commented Jul 25, 2022

Will defining the envrionment variable inside the gulpfile be enough to make it available inside the embed-code.js file?

For the production build, yes. The gulpfile does not have to do anything with the dev environment though, in this case env vars are set in the docker-compose.yml:

offen/docker-compose.yml

Lines 46 to 48 in c7aed87

environment:
LOCALE: ${LOCALE:-en}
PORT: 9977

I would assume a placeholder value is ok for use in development here?

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 25, 2022

I agree with you. A placeholder seems to be fine how ever I think it is important to mention in the docs that under development circumstances the script integrity function does not work because it is only a placerholder value.

This question might seem a bit silly but how exactly do I trigger a production build? In the docs I only could find instructions related to the make things which all seem to serve development purposes. I was looking into the buid folder but I could not run the dockerfile in there.

After that I'll probably be able to make a pull request that you can review and give feedback to.

@m90
Copy link
Member Author

m90 commented Jul 26, 2022

If you don't pass a target to make it will tell you about what's available:

➜  offen git:(development) make
TARGETS:
  help                  Print this message
  setup                 Build the development containers and install app dependencies
  bootstrap             Set up and seed databases - **Important**: this deletes any existing data
  up                    Start the development server
  down                  Tear down the development containers
  extract-strings       Extract user facing strings for localization from source code
  test                  Run unit tests for all subapps
  integration           Run integration tests
  dev-build             Build the Docker images for local development
  update                Install and/or update dependencies for the subapp containers
  migrate               Apply pending migrations to the development database
  secret                Create an application secret
  build                 Build the application binary
  build-docker          Build the docker image
  setup-docs            Setup the development environment for working on the docs site
  docs                  Run the development environment for the docs site
  build-docs            Build the static assets for the docs site

so make build is what you want. Be warned though that this downloads the 1GB+ xgo Docker image so your first build will be extremely slow. Subsequent builds will also be rather slow as the build is optimized for reliability rather than speed. You can make it a tad faster if you pass SKIP_LOCALES="1" which means it will build a version that contains English strings only.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 26, 2022

Alright. make build works for me, thank you. Unfortunately running the produced binary fails for me because I have no database set up.

WARN[0000] SMTP for transactional email is not configured right now, mail delivery will be unreliable
WARN[0000] Refer to the documentation to find out how to configure SMTP
FATA[0000] Unable to establish database connection       error="error opening database: unable to open database file: no such file or directory"

I'll set everything up and get back to you after production testing.
If you want me to I can already create a PR and you can have look and give some feedback?

@m90
Copy link
Member Author

m90 commented Jul 26, 2022

Unfortunately running the produced binary fails for me because I have no database set up.

I usually use the demo feature to test a binary that I built locally as it requires zero setup:

./bin/offen-linux-amd64 demo -users 1

should give you a local instance using your build (I doubt you need fake usage data for your needs so you can pass a low number to `-users').

If you want me to I can already create a PR and you can have look and give some feedback?

Whatever works best for you.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 26, 2022

Ah. demo did the trick. Thank you.

One thing I stumbled across:

It is probably the best way to create one environment variable for each locale?

The script asset is bundled once per locale, so in the code we could always be using the same key on 'process.env'. The relevant place in the code would be:

if (transform === 'envify' || (Array.isArray(transform) && transform[0] === 'envify')) {
return ['envify', { LOCALE: locale }]
}

where you could add the value.

You are referring to the gulpfile inside the auditorium how ever the script file which later on is embedded by the user is under /script/index.js, right?

If so do we need to create the environment variable in the gulpfile of the script? Or do we set the environment variable in the gulpfile of the auditorium and are going to reference the index.js of the script folder?

@m90
Copy link
Member Author

m90 commented Jul 26, 2022

If so do we need to create the environment variable in the gulpfile of the script? Or do we set the environment variable in the gulpfile of the auditorium and are going to reference the index.js of the script folder?

That's a bit of a mindbender, yes. As for setting the env var this definitely needs to happen in the auditorium gulpfile. As for computing the value there are two options:

  • The auditorium gulpfile pulls in the built script asset and computes the integrity value before bundling
  • The script gulpfile produces an additional artefact containg the hashes for each language

Both options are fine I guess, the first one might be a little easier to get done as it's only a single place to change.

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 26, 2022

I see - sorry to bother you again but how would such a pull of a script file work in theory?

I have to admit I'm not quite sure whether this is the relevant piece to look at but the Dockerfile.build creates the auditorium before the script part.

Inside the auditorium gulpfile.js I can not find the script folder either because my path is completly wrong or it is not there yet.

@m90
Copy link
Member Author

m90 commented Jul 26, 2022

In the Dockerfile.builds auditorium stage you can copy in the artifacts produced by the script stage (copy them to some arbitrary location) and then reference them in the auditorium/gulpfile.js. Docker will figure out that the auditorium stage now depends on the script stage and wait for it to build.

@hendr-ik
Copy link
Member

Is it okay for you to add additional dependencies like this one

Tagging @hendr-ik who might have a picture of how such a feature could look like.

On auditorium page instantiation create a hash clientside via JS (I hope that works)

This should probably work if you do it on demand only.

In case: just run npm install ... inside the auditorium part? (This lead to some error for me)

We're stuck on npm 6 here because of the project layout and some npm issues , so I would advise to use the docker-compose setup for development. After cloning

make setup
make bootstrap
make up

should give you a working development environment.

Hey there and thanks @raLaaaa for your contribution 👏 👏 👏
I would suggest the following implementation as its based on a previously used UI element.

Figma_qnaxS0xkc2

@raLaaaa
Copy link
Contributor

raLaaaa commented Jul 28, 2022

Hey, thank you for your feedback @hendr-ik .
I think I like your suggestion more than my current implementation because it is a little bit more straight forward and more clear.

I'll still post it here as basis for discussion:

image

I also created a work in progress merge where you can give some feedback on the implementation 👍

#730

@m90 m90 closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants