-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[DNS mode] Cloudflare New API Tokens #2398
Comments
fixed. please try with the latest acme.sh --upgrade -b dev
|
I believe this needs to be based on Zone ID, rather than account ID. Running a general account ID query with a specific zone being allowed (with no other zones being allowed) gives you an error that the account cannot list zones since the default API call tries to list all the entire account's information when it only has access to a specific zone.
Running this with a specific Zone ID allows permission to list that specific zone, but even permissions across all zones will allow you to list that specific zone as well. Running the API call with Also, this error just throws a generic "invalid domain" even though the problem was permissioning on the token. |
I'm having the same issue and had to allow the API token access to all zones to get this to work. It's better than what we had before since you can still limit access to only Zone and DNS settings, but it would be more secure to limit access to only those zones for which acme.sh needs DNS editing capabilities. For instance, I manage multiple small businesses' domains and DNS through Cloudflare, and would not want an acme.sh instance in one domain to have editing capabilities on another. I like @Berzerker's idea, but how would this work with multiple domains, e.g. aa.com and bb.com? Perhaps the Zone ID variable should be comprised of a delimited list of all zones corresponding to the domains we're issuing certs for? |
Making multiple calls to add the txt records then running all of the checks after is probably the easiest way with keeping the granular permissioning the tokens allow. |
Are you suggesting multiple separate calls to acme.sh for each domain? Wouldn't that necessitate updating the environment variables before each call? How then would saved parameters and cron functionality be maintained? |
I’m saying the acme.sh script should make multiple calls based on multiple zone IDs and/or account IDs set. I’m agreeing with you. |
Got it! Thanks for clarifying. |
@Neilpang I don't think this should be closed. As per the last few comments, this isn't working 100% based on the functionality of the API Tokens. As suggested, this should be switched to a Zone ID vs Account ID API call, with multiple calls being made if there are multiple domains/zones in play. Having it be Account ID call-based doesn't support setting permissions for specific zones as the token would not have access to list all of the zones of the account. |
@Neilpang I'd be happy to submit a pull request for the CF ZoneID functionality if you have a specific preference as to how you'd like to see this implemented. Or would you rather we open a new issue and address the new functionality there? |
Thanks, show me the PR first. |
I have filed a bug report to CloudFlare that |
IMO the way it works is expected functionality with that kind of an API call. If you call a global This is just an integration issue on the acme.sh end. |
I'd argue otherwise; consider that /v4/zones lists all zones your account has access to, and not all zones registered on CloudFlare. Following that thought process, the actor calling the API can use /v4/zones to list the zones that specific actor can access; this theoretically should mean tokens calling /v4/zones only see the zones those specific tokens can access, just like account-wide api keys can view all zones belonging to that account. |
I'm confused by your first line of thought. This is a CloudFlare-specific API call, how would it call zones not registered on CloudFlare? Regardless, I wouldn't call it a bug. This certainly seems like intended functionality; you're probably better off requesting functionality change rather than reporting it as a bug. |
The first line states that the /v4/zones api returns zones you can access rather than all zones on Cloudflare; this means that the api already filters the zones on their network to the ones your account can access; similarly, if authenticated with a token, it should filter to the zones the token can access. The fact it requires a global list permission to list some entries in a scoped api call sounds like an oversight, moreso than an intended functionality. Especially that a few times the API has returned an unhandled server error in some specific configurations. |
This issue was automatically closed by my fix. I just reopened it, in case you have more ideas. Thanks |
The wiki needs updating badly for non-uber techies like me. Like how do I get the CF_Account_ID value? "xxxxxxxx" isn't wonderfully helpful ;) Am I right in thinking that I need to use the CF call here https://api.cloudflare.com/#user-user-details If not ...? Thanks |
You can also pipe it into Look for the
|
So what permissions does acme.sh exactly need to do com.cloudflare.api.account.zone.list? I gave it zone, zone, read but it still has the same issue |
hmm apparently acme.sh needs full access and cannot work with the limited access to a specific zone e.g. zone resources = include, all zones vs a specific zone |
It needs access to all zones, since it does a query that needs to list all zones, but this is still a ton better than using the global key which has full administrative access to your account. |
It's still sort of bad as it allows acme.sh to edit any dns setting for any domain vs editing specific domains that use acme.sh |
Of course it could be better, but I’ll take this for now until I or someone else has time to redo this. |
Surely if you specified the domain id in the initial issueance command or whatever, you could skip the all zone list that requires the higher perms? |
The current implementation offers nothing really over using the global api key :/ Since the domain id (called the "account id" in cloudflare terms) is defined along with the API key, there is no need for acme.sh to fetch lists of domains. |
I ran into this issue today, and found that if you assign the API token permission to "Account -> Account Settings: Read", this implicitly grants Allowing the token to read all account settings isn't ideal, but I think it's a better solution than giving it access to all zones on an account. |
@typoworx-de Permissions Everything worked flawlessly. What do you mean with this?
You should be using your API Token and your CF_Account_ID. The point of using API Tokens is not having to use the global API key... EDIT 2020-03-25: I started having issues again. Maybe I did something different the first time or I was trying to renew instead of issue a new certificate. (see my new post here) |
Agreed this is still an issue. CF I don't think will be doing anything about it despite several requests: https://community.cloudflare.com/t/bug-zone-detail-by-name-requires-zone-list-permission/128042 @ptts permissions above work for me and seem to be the least restrictive you can grant using the token API to get this to work. At the very least this should be updated in the docs to prevent people from constantly having to research this from scratch. Going to put in PR with doc link changes. |
Apparently I didn't need to PR a request to the docs, they were public edit. |
@bengalih It would be better if we have a dedicated wiki page to demonstrate the usage in detail. |
This patch allows you to put the Zone ID into the Account ID field, and use a CloudFlare API Key that is restricted to the zone and has no account level access, and it will work fine. @Neilpang would this be an acceptable patch for a pull request, or would it be preferred to add yet an additional field called Zone ID to be used in this scenario? |
@artooro Please give me more detailed explanation about your change. |
@Neilpang no problem. Myself and others would like to restrict API keys to the specific domain zone. My patch allows this to happen by the user putting a Zone ID into the Account ID field, and dns_cf.sh will try to use it and soft-fail to Account ID. If this is merged officially, I think doing it as a new field would be best, so I'll go ahead and do that and create a merge request for it. |
I was trying to use @artooro 's fix like this:
Yet I always get:
In the debug logs I can see that my provided My token permissions are:
When I change the token permissions to include Anything else I can try? |
Are you running 2.8.6? This hasn't been released yet, you have to clone the repo. |
Hi, what if I use 2 tokens for 2 domains ?
certs issued with these tokens:
Then I noticed that |
@laoyur |
Looks like this is working on version 2.8.6 with the fix from @artooro. Here is what I did to get everything working in case people are still having trouble.
Further expanded example here: https://gist.github.com/colinmcintosh/016088860d35f01658e545b5ba75ba41 |
I can confirm |
@laoyur awesome! Thanks for the tip! I just tried this and it worked without the Account ID. I'll update the gist and my snippet to reflect |
Where (if anywhere) does acme.sh keep track of these variables per-domain for the renewal cron? If I were to issue several star-certs on the same machine, how would acme.sh know which Zone ID and CF token to use for each renewal? |
I just created two certs for two domains using two separate API tokens. Then I deleted the @Neilpang could you confirm? One (related) suggestion: maybe chmod 600 the account.conf and domain.com.conf files given they contain auth data? |
No, the let'sencrypt server has cached the verification status for a few days. |
Continuing here from #3026 I thought that might be the case and i'm already using the --accountconf workaround. But that seems unnecessary given there is already a domain conf file in domain/domain.conf, so why not cache the token,zone,account IDs there instead? Seems like the natural thing to do given everything domain specific is stored there, rather than keeping a 2nd file that is domain specific. Will also avoid messing with custom and extra cronjobs. I'll try to do a PR for that when i get the chance. |
I've made a patch, because I had the same problem Don't know if any of this is usefull for a PR |
Has anybody submitted a PR for this? This should definitely be fixed. As it stands, you are unable to generate certs for domains in different Cloudflare accounts (you can only create tokens for domains in the same account). This should be saved in the How have you all handled the issue thus far? |
Many guides on setting up ACME certs with Cloudflare in pfSense show filling out all five authentication fields. This is not required for acme.sh to work correctly and potentially exposes Cloudflare credentials with broad access though the pfSense UI and configuration backups. There are several ways that acme.sh can authenticate to Cloudflare, from least to most permissive: 1. Token with Zone.DNS:Edit permission and Zone ID. This only works with certs that cover a single zone. 2. Token with Zone.Zone:Read and Zone.DNS:Edit permission and Account ID. This works with certs that cover multiple zones. acme.sh uses the Account ID to look up all Zone IDs. 3. Global API key and email address. This is not recommended since it provides complete access to the entire Cloudflare account. References: https://github.com/acmesh-official/acme.sh/wiki/dnsapi#dns_cf acmesh-official/acme.sh#2398 acmesh-official/acme.sh@c25947d
Hello,
Cloudflare just releasing new API Tokens that can specify each API key for it's usage (Access Permission), that more secure than using Global API key.
As stated on https://api.cloudflare.com/#getting-started-requests (section API Tokens) implementing new API is just dirt easy by sending header
"Authorization: Bearer <token>"
instead of Global API that using headerX-AUTH-EMAIL:
&X-AUTH-KEY:
It's quite possible for adding new variable on account.conf like
CF_API_Tokens=<tokens>
and make some logic on dns_cf.sh that can deal with both new API Tokens & Global API header payload.Regards.
The text was updated successfully, but these errors were encountered: