Created attachment 244172 [details] Proposed patch to www/caddy I don't think it's an appropriate default to encourage users to run a webserver as root:wheel. This patch changes the default caddy_user and _group to www, and adds appropriate pkg-message entries to walk the user through configuring mac_portacl(4) to enable it to bind to ports r80 and 443. This does break existing installs that have accepted the prior default user/group configuration without setting the user and group explicitly in rc.conf. The upgrade message should be sufficient to walk the user through a migration, as well as offering sufficient advice as to how to restore the previous behaviour. I note at least one port (dns/dnscrypt-proxy2) automates the use of mac_portacl in its rc script, including loading the module and adding appropriate rules. While it would be possible to copy this approach, it does appear slightly fragile in that it will never remove rulesets it adds, so changes in the configuration could leave stale rules - this seems unwise in a security context. Possibly there should be more infrastructure around this to simplify both user and port-managed mac_portacl rules, but this is another conversation.
Thanks for submitting this! I agree that caddy is better off running as an unprivileged user, but I'm not sure that it should be the default. There's a major POLA problem with breaking nearly 100% of current caddy installs, and I'm pretty hesitant about the port being unrunnable by default. I'm willing to be convinced otherwise, but at this time I'm leaning towards leaving it as root by default and including your clear instructions in the pkg-message along with a message strongly urging users to follow it. Also: do your instructions need to be modified to run caddy in a jail? What are your thoughts on this?
Breaking existing setups is definitely unfortunate. It could be argued that running a webserver as root is a sufficiently bad idea to justify it, but it's easy for me to say that when it's not going to break *my* webserver! Perhaps it could be softened somewhat by delaying that change and having a pkg-message warning to give people a chance to switch to mac_portacl, explicitly set caddy_user/group, or Unrunnable-by-default is also unfortunate. Perhaps it makes sense to default to it binding to high ports and leave getting it on privileged ports to the user by whichever means they favour - mac_portacl, proxy, pf rdr, etc. This might even be nicer for people looking to replace an existing server, as it isn't going to immediately conflict with it, and it offers a chance to do other configuration and testing before it goes live. mac_portacl is system-global, so setting it to allow uid 80 to bind to port 80 will also allow a jailed uid 80 to bind to that port within the jail. Incidentally I've made a start on an rc script for mac_portacl to simplify its use - I'd like to see it eventually make its way into base: https://github.com/Freaky/portacl This reduces the instructions to: sysrc portacl_users="www" portacl_user_www_tcp="http https" service enable portacl service start portacl
I agree with everything you just said. Running as root IS a bad idea, but breaking existing installs is a bad plan too. I always tend towards giving people the tools and the knowledge and letting them choose whether to shoot themselves in the foot or not. I think we should: - Include USERS/GROUPS in the Makefile so the www user is created - Continue defaulting to root for now but plan to change to www at some point in the future - Include clear instructions in pkg-message and rc.d/caddy for how to run as www - Include strongly-worded encouragement to run as www - I absolutely LOVE the rc(8) portacl script idea! It absolutely should live in base. In the meantime, what about making a port for it and having caddy depend upon it?
(In reply to Adam Weinberger from comment #3) That all sounds great to me. I've knocked off whatever rough edges on the rc script I can find and submitted it as security/portacl-rc. There's also a PR on the go to get it into base which I'll push forward on when it's had a bit of exposure.
Per https://news.ycombinator.com/item?id=37477798 I think this would also be a worthwhile change to the default Caddyfile provided by the port: admin listen unix//var/run/caddy/admin.sock Instead of having the admin interface listen on localhost:2019 so any local user can make arbitrary changes to the server configuration (and where it's vulnerable to request smuggling), bind it to a domain socket protected by file permissions that restrict it to root/www. This will also be entirely transparent as far as the rc script is concerned - all that needs changing is ${caddy_flags} adding to the stop command.
(In reply to Thomas Hurst from comment #5) See also: https://github.com/caddyserver/caddy/issues/5317 A CADDY_ADMIN env var aimed at package maintainers, so it could also be specified in an rc.conf variable.
Created attachment 244812 [details] Proposed patch to www/caddy, redux This sets the admin interface to /var/run/caddy/caddy.sock, adds the caddy_admin rc variable, and updates install and upgrade messages to inform users of this change and encourage them off running the server as root.
(In reply to Thomas Hurst from comment #7) This is great! I'm a little swamped right now but I'll get to this over the coming days.
Might be desirable to have a bit more blurb about running in a jail, clarifying the portacl stuff needs to be on the host. Also a sharp edge with vnet jails: net.inet.ip.portrange.reservedhigh must be set to 0 within the jail. Looking at all the places where this could be mentioned now, I think maybe a link to a guide could be more appropriate than exhaustive instructions on install, upgrade, and in the rc script should someone happen to look there.
https://wiki.freebsd.org/ exists for just that purpose, and it'd allow you to keep modifying it over time until it feels right.
This is committed. Thank you so much for all your work on this, and for your patience! I really got swamped and it took far longer to get this deployed than I’d wanted. I forgot to tag the PR (but I did tag you as the author) in the commit. (As an aside, please watch for trailing spaces in your patches.) Seriously, really good work on this PR.
(In reply to Adam Weinberger from comment #11) Thanks! I'd hoped to get to it sooner, but I've made a start on a Wiki article with more details on configuration, including running it in a jail: https://wiki.freebsd.org/ThomasHurst/Caddy