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

Update dns-over-https.md #186

Merged
merged 4 commits into from
Feb 25, 2020
Merged

Update dns-over-https.md #186

merged 4 commits into from
Feb 25, 2020

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Dec 3, 2019

Recent versions of cloudflared include a service install/uninstall command which saves some manual steps

I only tested this on Raspbian buster and not for a long time, but it does seem to work. Let me know what you think

  • Add uninstall instructions
  • Add update instructions
  • Revisit uninstall commands; maybe add sudo systemctl disable cloudflared and sudo systemctl daemon-reload

Preview: https://deploy-preview-186--pihole-docs.netlify.com/guides/dns-over-https/

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 3, 2019

BTW ideally we should list an official way to uninstall cloudflared but I had trouble with the service removal on my test system.

EDIT: Now that I think about it, maybe it'd be better if we had both ways listed. I'll update the patch tomorrow.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 3, 2019

Alright, I added the changes. I'd like a couple more confirmations in case I missed something, but this feels a lot simpler.

@PromoFaux
Copy link
Member

BTW ideally we should list an official way to uninstall cloudflared but I had trouble with the service removal on my test system.

May be worth adding in a note at the bottom that says to uninstall use the cloudflared service uninstall command, but with a note that it may not work as cleanly as expected.

Out of interest, what troubles did you face when trying to remove it? Personally I've gone the route of unbound, so I've not actually tried any of this out!

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 3, 2019

I couldn't get the service removed but maybe I missed something because I was tired when I tried it :P

Other than that, I'm not 100% sure about the potential security issues because in the original implementation the user has limited rights.

I can add a new paragraph about uninstalling cloudflared for both ways; for the manual way I've done quite a few times with 100% success.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 4, 2019

Alright, I added a few more info. Please someone else try this before merging.

Now install the service via `cloudflared`'s [service command](https://developers.cloudflare.com/argo-tunnel/reference/arguments/#service-command):

```sh
sudo cloudflared service install
Copy link
Member

Choose a reason for hiding this comment

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

I tried doing this but got an error about the cert.pem file. That said, it is likely I did something wrong. In any case, time for a reflash of my Pi. I will try again later (and on a droplet, this time...)

Copy link
Contributor Author

@XhmikosR XhmikosR Dec 4, 2019

Choose a reason for hiding this comment

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

I will also try it tomorrow on my Pi, but being I don't currently have it backed up, you know, I'm extra careful :)

Oh, BTW when using this command, there's an extra option being passed which I think we should probably have it in the manual way too. --no-autoupdate=true https://developers.cloudflare.com/argo-tunnel/reference/arguments/#no-autoupdate

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 4, 2019

@PromoFaux I had the path wrong for the automatic way. Now it works, I just tested it again.

@XhmikosR
Copy link
Contributor Author

I've been using the automatic install method for weeks now without any issues. As long as we make sure we don't mind cloudflared running as root, I think it's way simpler to use this way as the recommended way

@DL6ER
Copy link
Member

DL6ER commented Dec 20, 2019

Nothing should really run as root. Do we know if they do at least try to reduce privileges? With FTL, we do this. When started as root (which is not recommended!), we detect this "mistake", bind the privileged ports and then drop to an entirely unprivileged user for the process. Thereby, no harm can be caused by the process after the startup phase.

@XhmikosR
Copy link
Contributor Author

I'm not sure about it. Here are their docs https://developers.cloudflare.com/argo-tunnel/reference/service/

@DL6ER
Copy link
Member

DL6ER commented Dec 20, 2019

They probably don't care about such (actually not even that subtle) security measures. I have not found anything indicating a drop of privileges there.

@dschaper
Copy link
Member

Is this ready for merge?

@XhmikosR
Copy link
Contributor Author

I don't think so. See the above comments. We could still offer the official solution and highlight the fact that cloudflared will run as root.

@dschaper
Copy link
Member

Probably the only option. I don't think there's a canonical way to run without root priv. Might be able to do it with systemd service files but it's a problem for CF to solve.

@netlify
Copy link

netlify bot commented Feb 25, 2020

Deploy preview for pihole-docs ready!

Built with commit 3d0c1dc

https://deploy-preview-186--pihole-docs.netlify.com

XhmikosR and others added 2 commits February 25, 2020 19:32
Recent versions of cloudflared include a service install/uninstall command which saves some manual steps.

Also add uninstall and update steps.
@XhmikosR
Copy link
Contributor Author

@dschaper done. I also removed the auto update sentence because I'm not 100% sure if that's the case. We'll need more people to chime in and help with this page after this patch lands.

Also, we might need to close and ask #215 to be updated/created again

@dschaper
Copy link
Member

Builds seem to be stalled. There was a GitHub glitch a hour or so ago.

@dschaper dschaper merged commit 37c26e7 into pi-hole:master Feb 25, 2020
@XhmikosR XhmikosR deleted the patch-1 branch February 25, 2020 20:08
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.

4 participants