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

ref(relay): Remove PK and rely on INTERNAL_IPS #572

Merged
merged 3 commits into from Jul 10, 2020

Conversation

BYK
Copy link
Collaborator

@BYK BYK commented Jul 9, 2020

This patch adds INTERNAL_IPS definition to sentry.conf.py by sniffing the network from eth0 and relies on this for trusted Relays instead of the ALLOWLISTED PKs. This removes the necessity of syncing Relay PKs to sentry.conf.py.

This PR needs getsentry/sentry#19798 to work.

This patch adds INTERNAL_IPS definition to sentry.conf.py by sniffing the network from eth0 and relies on this for trusted Relays instead of the ALLOWLISTED PKs. This removes the necessity of generating and syncing Relay PKs.
@BYK BYK requested a review from jan-auer July 9, 2020 21:54
BYK added a commit to getsentry/sentry that referenced this pull request Jul 9, 2020
`settings.INTERNAL_IPS` is meant to be a list of IP networks, not individual IPs, hence the `is_internal_relay` check was not working (see getsentry/self-hosted#572). This PR fixes the issue by borrowing some IP checking code from https://github.com/getsentry/sentry/blob/95767d455b8004ec4b4c5026d84b64b6348e6d37/src/sentry/auth/superuser.py#L64
@BYK BYK requested a review from untitaker July 9, 2020 21:58
Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

in that case we might as well hardcode credentials.json as it serves no purpose

@BYK
Copy link
Collaborator Author

BYK commented Jul 9, 2020

in that case we might as well hardcode credentials.json as it serves no purpose

Good idea. Will consider this.

@BYK BYK marked this pull request as ready for review July 10, 2020 18:20
Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

🍪 (provided your copy pasta works, I haven't run it)

BYK added a commit to getsentry/sentry that referenced this pull request Jul 10, 2020
`settings.INTERNAL_IPS` is meant to be a list of IP networks, not individual IPs, hence the `is_internal_relay` check was not working (see getsentry/self-hosted#572). This PR fixes the issue by replacing the manual and incorrect check with `sentry.auth.system.is_internal_ip`. This also means we are now checking against `INTERNAL_SYSTEM_IPS` instead of `INTERNAL_IPS` which is more accurate.
@BYK BYK merged commit 73213bc into master Jul 10, 2020
@BYK BYK deleted the byk/ref/relay_pk_internal_net branch July 10, 2020 20:53
BYK added a commit that referenced this pull request Jul 13, 2020
`INTERNAL_IPS` is used to check whether to allow superuser access or not. Limiting this to the Docker internal network makes it impossible for anyone to reach admin pages with on-premise setup.

This is a follow up to #572 and it fixes #577.
BYK added a commit that referenced this pull request Jul 13, 2020
`INTERNAL_IPS` is used to check whether to allow superuser access or not. Limiting this to the Docker internal network makes it impossible for anyone to reach admin pages with on-premise setup.

This is a follow up to #572 and it fixes #577.
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants