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

[0.6] client/webserver: keep IPv6 out of csp header #2287

Merged
merged 1 commit into from Apr 4, 2023

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Apr 3, 2023

Minimal fix for release-v0.6.

Pertaining to #2283, this just filters out IPv6 addresses from injection into the csp part of the http response header.

Even though all IPv4 except 127.0.0.1 does not match (we must rely on 'self' working correctly for these), they do not make the CSP entry invalid, so we are continuing to insert those so we do not create a change of behavior.

On master we may actually remove this content-src injection since the browser bug was fixed about a year ago. On master, we should also update the backend log line that prints out the URL so that it doesn't include the wildcard address since that's no good for a browser address.

All this PR does is to avoid adding IPv6 addresses, since those actually make the CSP invalid. Examples:

  • "[::1]:5759"
  • "[::]:5759"
  • "[2607:f8b0:4000:81a::200e]:5759"
  • "0.0.0.0:5759" -- this is the culprit in the Docker issue because on a machine with an IPv6 stack, the socket resolution generally converts the wildcard in to the equivalent IPv6 wildcard of [::]

Copy link
Contributor

@ukane-philemon ukane-philemon left a comment

Choose a reason for hiding this comment

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

This works, at least for newer versions of the Safari browser.

@ukane-philemon
Copy link
Contributor

On master we may actually remove this content-src injection since the browser bug was fixed about a year ago.

We could add a notice somewhere, maybe in the release notes that this workaround would be removed in subsequent dcrdex releases.

@chappjc
Copy link
Member Author

chappjc commented Apr 4, 2023

We could add a notice somewhere, maybe in the release notes that this workaround would be removed in subsequent dcrdex releases.

Will do.

@chappjc chappjc added this to the 0.6 milestone Apr 4, 2023
@chappjc chappjc merged commit c3301de into decred:release-v0.6 Apr 4, 2023
5 checks passed
@chappjc chappjc deleted the csp-no-ipv6 branch April 4, 2023 16:03
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.

None yet

2 participants