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

why need Azure Resource Group Permission? #2372

Closed
esgeer opened this issue Apr 28, 2023 · 15 comments
Closed

why need Azure Resource Group Permission? #2372

esgeer opened this issue Apr 28, 2023 · 15 comments

Comments

@esgeer
Copy link

esgeer commented Apr 28, 2023

Describe the bug
We are using the latest win-acme (v2.2.4.1500) version and try to renew a certificate for azure dns using a managed identy:

--validation azure --azureusemsi --azuresubscriptionid x --azureresourcegroupname x --azurehostedzone x

The managed identy does have DNS Zone Contributor rights on the Azure DNS Zone ... but not on the Resource Group. When we try to renew certificate we get:

Azure.RequestFailedException: The client 'XXXXXXXX-XXXX-XXXXX-XXXXX-XXXXXXXXXXXX' with object id 'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' does not have authorization to perform action 'Microsoft.Resources/subscriptions/resourcegroups/read' over scope '/subscriptions/XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/resourcegroups/XXXXXX' or the scope is invalid. If access was recently granted, please refresh your credentials. Status: 403 (Forbidden) ErrorCode: AuthorizationFailed
so it seems win-acme tries to scan the resource group for the specific dns zone, even when we give the name of the zone.
So we gave the identy reader rights on the resource group and then it works.

Expected behavior
win-acme should be able to handle certificates when only having the correct rights on the azure dns hosted zone without permissions on the resource group.

@WouterTinus
Copy link
Member

WouterTinus commented Apr 28, 2023

I'd happy to accept a PR if you can find a better way to do it. Frankly of all DNS API's that I've seen in the course of maintaining this project, Microsofts one has been by far the worst one to deal with. As far as I know, even when the name of the zone is known, I still have to get a ResourceGroupResource first to be able to call GetDnsZoneAsync (i.e. the entity used query/modify a specific zone).

I haven't found a way to materialize a ResourceGroupResource without querying it from the SubscriptionResource, which is probably why it wants that Microsoft.Resources/subscriptions/resourcegroups/read permission. In the previous-generation packages (this is the third time that there's a new "generation" of SDK for Azure) the resource group name could simply be provided as a string, but not anymore.

The SubscriptionResource (i.e. one level up from the resource group) has a function called GetDnsZonesAsync, but that is supposed to return a list of all zones in all resource groups, so I assume it requires even broader read access. (At least to be able to get all zones from a specific resource group you need broader access, which is why we introduced --azurehostedzone in the first place.)

@webprofusion-chrisc
Copy link
Contributor

Hi @WouterTinus I was looking at this same update for Certify The Web recently and while their docs all do use the resource method I found you could avoid extra permissions by going via the subscription.

https://github.com/webprofusion/certify/blob/development/src/Certify.Providers/DNS/Azure/DnsProviderAzure.cs#L201

Along the lines of:

_subscription = await _azureClient.GetDefaultSubscriptionAsync();
...
...
var zones = _subscription.GetDnsZonesAsync(1000);
var zone = <find matching zone in list>;
...
...
var currentRecords = zone.GetDnsTxtRecords();
var existing = currentRecords.FirstOrDefault(t => t.Data.Name == recordName);
...
...

@WouterTinus
Copy link
Member

Hi Christopher, that may be a solution for the regular case, but I have some users (like OP) saying that they don't want to grant permission to enumerate all zones, so instead they provide the name of a specific zone that they want used. It seems to me that you either need the zone enumeration permission or the resource group permission.

It would be nice if on enumeration Microsoft would
simply automatically filter out zones that we don't have access to, but at least until last year or so it didn't work like that.

At this point I'm happy that the plugin works, I'm not inclined to refactor again and deal with potential fallout from that to avoid granting some harmless permission. I'd sooner suggest moving to a decent DNS provider that at least support DNSSEC 😄

@webprofusion-chrisc
Copy link
Contributor

Cool, it's just that the DNS contributor role is the one that doesn't allow the resource query so users will have to update their IAM settings for their application user.

In CTW we take a zoneid for some providers, in the case of Azure that can be the zone name so you could do similar (let them specify the zone name and therefore skip the zone query), but yes Azure is one of the more complicated DNS providers and their API is a bit cumbersome!

@WouterTinus
Copy link
Member

So are you saying that when getting the list from the subscription it doesn't require specific enumeration access? It would just return the zone(s) that one has contributer access to?

@webprofusion-chrisc
Copy link
Contributor

The DNS contributor role allows listing the zones but I don't know what it does for zones you can't update, however the old API certainly works ok with that role without complaints.

@WouterTinus
Copy link
Member

Hi @sveng-r - I've kicked off a build using the technique suggested by @webprofusion-chrisc, would you mind to test if this works with minimal permissions as you expected it to?

https://ci.appveyor.com/project/WouterTinus/win-acme-s8t9q/builds/46993810/artifacts

@esgeer
Copy link
Author

esgeer commented May 10, 2023

Sorry for coming back so late. We removed the permissions for the managed identy on the resource group, leaving only "DNS Zone Contributor" role on the specific DNS Zone and let my customer test.

He said he got some errors, but in the end the certificate renewal was sucessfull. So this fix looks fine for me so far.. thx alot.

He also gave me the log file in case it is of any help, all sensitiv data should be masked with XXXXXXXXXX

Winacme.log

@webprofusion-chrisc
Copy link
Contributor

webprofusion-chrisc commented May 10, 2023

If you already know the subscription ID and set that when setting up the arm client then GetDefaultSubscriptionAsync() just returns that one.

_subscriptionResource = _options.SubscriptionId == null

Maybe GetSubscriptions() isn't working if you just have DNS Zone Contributor.

So I'm thinking you could just have subscriptionResource = await Client.GetDefaultSubscriptionAsync() - that what I use in Certify The Web and I'm pretty sure I tested that on an account with multiple subscriptions.

https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/resourcemanager/Azure.ResourceManager/samples/Sample2_ManagingResourceGroups.md

@WouterTinus
Copy link
Member

So this fix looks fine for me so far.. thx alot.

Hate to dissapoint you, but the only reason that worked was because the authorization result was still cached (at Let's Encrypt). That build will more than likely fail for the next renewal.

Chris' suggestions look good but I'll need to find some time to look into them.

@WouterTinus
Copy link
Member

This build might do the trick, we don't look at the resource group at all and only request for the default subscription: https://ci.appveyor.com/project/WouterTinus/win-acme-s8t9q/builds/47008072/artifacts

@esgeer
Copy link
Author

esgeer commented May 12, 2023

my customer tried again with Option "T: Run the renewal (force, no cache)"

it worked and he got:

[DBUG] [XXXXXXXXXX.net] Attempting to create DNS record under _acme-challenge.XXXXXXXXXX.net...
[EROR] [XXXXXXXXXX.net] Error preparing for challenge answer
System.Exception: Unable to find subscription XXXXXXXXXX
   at PKISharp.WACS.Plugins.ValidationPlugins.Dns.Azure.Subscription()
   at PKISharp.WACS.Plugins.ValidationPlugins.Dns.Azure.GetHostedZone(String recordName)
   at PKISharp.WACS.Plugins.ValidationPlugins.Dns.Azure.CreateRecord(DnsValidationRecord record)
   at PKISharp.WACS.Plugins.ValidationPlugins.DnsValidation1.PrepareChallenge(ValidationContext context, Dns01ChallengeValidationDetails challenge)    at PKISharp.WACS.Plugins.ValidationPlugins.Validation1.PrepareChallenge(ValidationContext context)
   at PKISharp.WACS.RenewalValidator.Prepare(ValidationContext context, RunLevel runLevel)
[VERB] Starting commit stage
[VERB] Commit was succesfull

thx for your time&effort
fyi: i am afk for about 10 days if there is any further tests or info you need

@WouterTinus
Copy link
Member

That was an stupid typo. I've fixed it and also managed to access my own Azure environment for testing, it seems that this build works better: https://ci.appveyor.com/project/WouterTinus/win-acme-s8t9q/builds/47033743/artifacts

@WouterTinus WouterTinus added this to the 2.2.5 milestone May 24, 2023
@WouterTinus
Copy link
Member

Fix included in 2.2.5

@WouterTinus WouterTinus modified the milestones: 2.2.5, 2.2.6 Jul 5, 2023
@WouterTinus
Copy link
Member

It looks like this was not properly merged for the 2.2.5 release, so it will only be included in 2.2.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants