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

Make registration endpoint URI configurable #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make registration endpoint URI configurable #109

wants to merge 1 commit into from

Conversation

stblassitude
Copy link
Contributor

This allows hiding the registration while leaving it enabled and
exposed to the wider network.

@coveralls
Copy link

coveralls commented Aug 21, 2018

Coverage Status

Coverage remained the same at 93.429% when pulling 94f0bfd on stblassitude:regauth into d66ccff on joohoi:master.

This allows hiding the registration while leaving it enabled and
exposed to the wider network.
@jfchevrette
Copy link

I was going to propose a PR for this.

@stblassitude are you able to update the PR to fix the struct field name?

types.go:57:2: struct field RegistrationUri should be RegistrationURI

@Yannik
Copy link
Contributor

Yannik commented Sep 27, 2018

This is security by obscurity. I disagree with merging this PR as it provides no actual security and instead adds bloat.

@joohoi
Copy link
Owner

joohoi commented Sep 27, 2018

I see the reasoning behind the feature, but I have to agree with @Yannik about this approach. As an alternative, I would like to propose adding a optional predefined key that would be passed along the POST request headers to /register endpoint, much alike the actual credentials that are later used for /update endpoint. Only in this case it would be static, and could well be defined as clear text in the actual config.cfg.

@Ajedi32
Copy link
Contributor

Ajedi32 commented Oct 1, 2018

I disagree that this is security by obscurity. A sufficiently long random token in a URL path is comparably secure to a bearer token in a request header. [1][2]

That said, from an API design perspective I agree that it's better to handle authorization via request headers rather than URL paths.

@ghost
Copy link

ghost commented Dec 10, 2019

Hello,

I would like to reopen discussion and consideration of this issue. While the point about this being 'security through obscurity' has some validity, I do not want script kiddies being able to run scanning utilities against my DNS software.

Security works best in layers. Why can I not have both a configurable endpoint and a custom auth token?

Thanks,

Copy link

@saudiqbal saudiqbal left a comment

Choose a reason for hiding this comment

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

I would like to see this added to acme-dns too.

@saudiqbal
Copy link

Any update on the issue?

@TomyLobo
Copy link

TomyLobo commented Sep 3, 2024

This PR is lacking test cases and updates to the manual and it needs to be rebased to resolve conflicts.

@maddes-b
Copy link

maddes-b commented Sep 3, 2024

Side note: added support for custom URL paths in my client update 0.10.0, see https://github.com/maddes-b/acme-dns-client-2/

@maddes-b
Copy link

maddes-b commented Sep 11, 2024

Right now it is really easy to find out and abuse individual acme-dns setups:

  • check the challenge domain of a known domain, e.g. for www.maddes.net it would be _acme-challenge.www.maddes.net
  • if it is a CNAME like aaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee.something.example.net, then...
    • first part is an indicator for a possible acme-dns server.
    • responsible name server would be the acme-dns DNS service.
    • API service would be https://something.example.net.
    • use https://something.example.net/register with curl/wget to verify (extra: do scan for HTTPS port and test these).

Possible ways to avoid unwanted registrations and backtracing:

1. Custom URL paths
Custom URL paths like https://something.example.net/abcdefghij/<register|update> avoid generic probing for acme-dns API.
The custom URL path is a shared secret. Avoids unwanted registrations.
You may call this "Security by Obscurity" but hiding is a valid tactic to avoid attack surfaces.

2. Secret in Header/Basic Auth
Secrets for /register (and maybe also /update) could be just a shared single token or done via a basic user administration (user/password).
Avoids unwanted registrations.

My knowledge, situation, thoughts and conclusion:
I know that...
A) custom URL paths could also be achieved by reverse proxing with nginx or similar
B) whitelisting IPs and VPNs can limit access

Both add other tools and more complexity/pitfalls to the setup, plus this may not be possible in all use cases.

In my use case several friends are using my acme-dns instance to get certificates for their home machines.
Most of them are not technically experienced and VPNs would overburden my users (or overtax my support).
Nearly all have dynamic IP addresses, therefore Whitelisting IPs is not applicable.
So my acme-dns API is and stays publicly exposed.

Adding Basic Auth (user/password) for /register would add too much hassle to acme-dns: functionality to add a user via CLI/shell or admin API (would need another user/pw setup).

The shared secret for the header and the shared custom URL path are equally a) secure, b) encrypted via SSL and c) depending on the admin's willing to use a random long secret in his setup.
Adding tips to the README should help here, like "Write two random words into a text file, save the text file, calculate a SHA256 checksum for that text file (or MD5, SHA1, SHA512, etc.), use the checksum as your secret/custom URL path"

If none of these ideas will be added to the code, then the README should have a reverse proxying paragraph with some notes, links to information and a typical example for nginx:

  • Scenario A: Apache running on 80/443, nginx reverse proxy running on 1111 and acme-dns running on localhost:2222
  • Scenario B: nginx as web server and reverse proxy running on 80/443 and acme-dns running on localhost:2222
  • Scenario C: nginx possibilities: basic auth, rate limiting

acme-dns may be the first contact with reverse proxying and therefore some starting points will be helpful and reduce support questions.

I would be willing to write that paragraph, but I need guidance how to set it up correctly: Maybe @m00nwtchr (#345) @lachesis (#345) @webprofusion-chrisc (#263) @TomyLobo (#263).
(What about certificate handling for that reverse proxing? Must nginx use the cert that acme-dns retrieved for itself or is there a certificate pass-through possible?)
I would also update my acme-dns-client-2 with basic auth support too.

@m00nwtchr
Copy link

m00nwtchr commented Sep 12, 2024

@maddes-b

acme-dns may be the first contact with reverse proxying and therefore some starting points will be helpful and reduce support questions.

I still don't understand why people even reverse-proxy the acme-dns HTTP API in the first place. Only the DNS server needs to be exposed to the internet for it to work.

Unless you're trying to use 1 acme-dns instance for multiple servers? But it's a lot easier to just set up separate acme-dns instances for that, IMO (and if you really want to use only 1, a VPN is always the better option and isn't that hard to set up with e.g. https://github.com/DigitallyRefined/docker-wireguard-tunnel )

@maddes-b
Copy link

maddes-b commented Sep 12, 2024

The first draft of that paragraph is complete: https://github.com/maddes-b/acme-dns/tree/feature/reverse-proxy#https-api
Information should be complete just re-reading for typos, missing information or links.
Any comments?

@joohoi: Would you accept it this way? Change structure or something else?

@saudiqbal
Copy link

I dont care about acme-dns anymore, I moved to BIND dns server with a PHP API where I can simply add and remove domains in a single PHP file.

https://saudiqbal.github.io/Linux/LetsEncrypt-PHP-API-BIND-DNS-ACME-DNS-01-server-setup.html

maddes-b added a commit to maddes-b/acme-dns that referenced this pull request Sep 21, 2024
Also replace non-breakable spaces with normal ones
Closes joohoi#109
Closes joohoi#345
Closes joohoi#295
@TomyLobo
Copy link

TomyLobo commented Oct 11, 2024

@saudiqbal Why not just firewall the API port?

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.

10 participants