rmbolger / Posh-ACME

PowerShell module and ACME client to create certificates from Let's Encrypt (or other ACME CA)
https://poshac.me/docs/latest/
MIT License
766 stars 186 forks source link

Azure DNS plugin - AZUseIMDS on Azure Automation fails #346

Closed K-a-r-l closed 3 years ago

K-a-r-l commented 3 years ago

When using the AZUseIMDS = $true argument for the Azure DNS plugin in an Azure automation runbook, the New-PACertificate process fails with the below error

GET http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https:%2F%2Fmanagement.core.windows.net%2F with 0-byte payload
Unable to connect to the remote server

It seems that you can't use the method described in how-to-use-vm-token#get-a-token-using-azure-powershell to get a token - which calls the 169.254.169.254 endpoint - you have to use $env:IDENTITY_ENDPOINT (http://127.0.0.1:40056/oauth2/token) instead, as described here enable-managed-identity-for-automation#sample-get-request.

I've reverted back to using AZAccessToken and manually calling the above endpoint, but it did confuse me for a while.

Wondering if it's worth having the IMDS section of the code check for the existence of the $env:IDENTITY_ENDPOINT and use that instead of the 169 address if it's found, that way it should work in a VM and in Azure Automation?

rmbolger commented 3 years ago

Hey @K-a-r-l, thanks for the report. I'd be happy to try and implement something that conditionally uses the IDENTITY environment variables if they exist. But my Azure Automation chops are pretty lacking at the moment and you'd probably have to test it for me. If the changes were in a branch here on github, would you be able to pull down that version and use it to test with? Or would I need to publish something to PSGallery first?

K-a-r-l commented 3 years ago

Yep, I can do that. I can remove the gallery version and upload a modified zip of the module into Azure Automation with the changes, then test that it works.

rmbolger commented 3 years ago

Ok, my first attempt at a fix is now in the azure-imds branch. If it matters, it also includes the zone apex fix from #348. Give it a try and let me know how it goes. I put some extra verbose messages in there to call out the IDENTITY_ENDPOINT and IDENTITY_HEADER environment variables if they were found. That should make it easier to tell if the new code path was triggered.

K-a-r-l commented 3 years ago

I gave it a test and worked out the call to the IDENTITY_ENDPOINT didn't like the escaped resource string (returned a 500 error initially) so I made the below change and then it worked:

# Line 435, changed this
$body = @{ resource = [uri]::EscapeDataString("$($script:AZEnvironment.ManagementUrl)/") }

#To this:
$body = @{ resource = "$($script:AZEnvironment.ManagementUrl)/" }

I jumped on an Azure VM to see if a call to the 169.254.169.254 endpoint worked without the escaped resource and it did, so I think it's probably safe to make the change and both the VM and Automation runbook versions of IDMS should work IDMS

I also tested the zone apex fix #348 it works perfectly.

rmbolger commented 3 years ago

Ooh yes. Nice catch. I forgot that passing the querystring params as a hashtable body would take care of escaping things like that ManagementUrl. So yeah, it was probably double-escaping and screwing things up. Fix incoming.

rmbolger commented 3 years ago

Forgot to tag the issue in the commit message. But the fix is in the same branch now if you want to test it.

K-a-r-l commented 3 years ago

I tested the latest version of the azure-imds branch, and it's all working now.

Thanks very much Ryan! This is the first time I've logged an issue with an open source project, and it was a really pleasant experience :D

I'm looking forward to getting this setup at work now, to get certificates for all our non-prod websites (work won't allow Let's Encrypt for prod, but it's still going to save a bunch of time - and money - for the dev/test/uat certs).

rmbolger commented 3 years ago

You're very welcome! Always happy to help. I'll get the changes merged into the main branch and they should be included in the next release.