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.

Sign up for GitHub

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

🐛fix CSP frame-ancestors for dynamic-sidecar services #2977

Merged
merged 5 commits into from
Apr 12, 2022
Merged

🐛fix CSP frame-ancestors for dynamic-sidecar services #2977

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a189add
fix CSP frame-ancestors
Apr 8, 2022
591deac
Merge branch 'master' into fix-csp-policy
GitHK Apr 8, 2022
5b86915
Merge branch 'master' into fix-csp-policy
GitHK Apr 11, 2022
d6a76b9
Merge branch 'master' into fix-csp-policy
GitHK Apr 11, 2022
51cb117
Merge branch 'master' into fix-csp-policy
GitHK Apr 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_service_specs/proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def get_dynamic_proxy_spec(
"swarm_stack_name": dynamic_sidecar_settings.SWARM_STACK_NAME,
"traefik.docker.network": swarm_network_name,
"traefik.enable": "true",
f"traefik.http.middlewares.{scheduler_data.proxy_service_name}-security-headers.headers.customresponseheaders.Content-Security-Policy": f"frame-ancestors {scheduler_data.request_dns}",
f"traefik.http.middlewares.{scheduler_data.proxy_service_name}-security-headers.headers.customresponseheaders.Content-Security-Policy": f"frame-ancestors {scheduler_data.request_dns} {scheduler_data.node_uuid}.services.{scheduler_data.request_dns}",
Copy link
Member

@pcrespov pcrespov Apr 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Don't you need commas between these two values?

I see other entries of traefik that requires that ... for instance

osparc-simcore/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_service_specs/proxy.py

Line 57 in dd94393

f"traefik.http.middlewares.{scheduler_data.proxy_service_name}-security-headers.headers.accessControlAllowOriginList": ",".join(

Copy link
Member

Choose a reason for hiding this comment

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

Q: scheduler_data.node_uuid}.services.{scheduler_data.request_dns} is the entrypoint of a dynamic services, right? I see that the project id is not used for that. Does this assume that nodes have universal identifiers? i.e. two projects cannot include a node with the same NodeID as dynamic service ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is exactly the entrypoint and we are saying to CSP that it can start iframes, to allow for iframe in iframes.

Yes, that is the same assumption as the legacy services!

Copy link
Member

Choose a reason for hiding this comment

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

@GitHK @sanderegg OK, just for the record:
it would be good to identify dynamic services with something more than the node UUID (e.g. project_uuid/node_uuid)

We would like to guarantee the node-uuid uniquenes only within the scope of its project. That would help us a lot on:

  • simplifies duplication of projects
  • simplifies caching in metamodling iterations
  • ...

See #2133 for more details

f"traefik.http.middlewares.{scheduler_data.proxy_service_name}-security-headers.headers.accesscontrolallowmethods": "GET,OPTIONS,PUT,POST,DELETE,PATCH,HEAD",
f"traefik.http.middlewares.{scheduler_data.proxy_service_name}-security-headers.headers.accessControlAllowOriginList": ",".join(
[
Expand Down