azurerm_front_door - Add minimum_tls_version property #5539
Conversation
There was a problem hiding this comment.
@sean-nixon thanks so much for this PR and adding the minimum TLS version attribute. This LGTM! 🚀 I have also added your TODO task to my tracking issue #4627 to update the TLS post 2.0 release.
|
@sean-nixon I noticed that if you set the |
There was a problem hiding this comment.
@sean-nixon this is almost ready to go, looking at this closer and comparing it to the current portal experience don't you think it would make more sense if this was a computed attribute and always send 1.0, since the portal does not allow you to select the version of the TLS to use? Then once 2.0 is released we can change it to always send TLS 1.2? Thanks again for your contribution. 🚀
|
@WodansSon Thanks for catching that issue with updates. I agree it might make sense to make the value computed given the Azure Portal behavior of completely hiding this. However, I would prefer not to always send 1.0. TLS 1.0 is no longer PCI compliant and a lot of organizations have policies to always disabling TLS 1.0, so it's not really an option for me personally. What would you think of instead using the minimum TLS version config if it exists? If it doesn't exist already, we can then safely default to TLS 1.2 which would match the behavior in the Azure Portal. Alternatively, I'm seeing if I can update the code to properly update the min TLS version. I think it may be possible (currently testing this), but the downside is it's essentially a complete redeployment of custom HTTPS certificates so it takes a long time. My test is still running after almost 2 hours, so I'm not sure if it will eventually error out. |
|
@sean-nixon yep, that totally makes sense... If the value is already set in azure keep sending whatever value that was previously set, be it 1.0 or 1.2, and if it is not yet defined always send 1.2. I agree that It is kind of painful that it can take up to 8 hours to completely revoke the cert once it has been deployed, but on the bright side with the new version of the API it now takes only about 10 minutes to deploy a Front Door managed cert, where in the old API it could take up to 20 hours. 😄 |
|
Have you tested this and TLS 1.2 works with front door? I implemented this locally but when I scanned it with ssllabs it still said it was using 1.0 but possibly I was doing something wrong |
|
(Even if it doesn’t work much better to fix creation of new endpoints, unfortunately when I was testing in the upgrade I must’ve just been updating them) |
|
@timja I created two frontends, one with 1.2 and one with 1.0 and ran Qualys checks on both of them. It does seem like 1.0 is technically supported even if you select 1.2, but only with clients that don't support SNI. There's a marked difference between the two reports: TLS 1.2 Frontend: TLS 1.0 Frontend: |
|
Hmm okay I didn't notice this when I was testing, otherwise I would have submitted it earlier, bit unexpected though |
This updates the behavior to better fit with the Portal experience. Existing frontends will continue to use TLS 1.0 while new frontends will use TLS 1.2 as the minimum
|
@WodansSon I took a stab at making the attribute computed. I also removed the |
That attribute is now calculated so it should not be set by the user
|
@sean-nixon Agreed. Hope you don't mind, but I added a test to validate the functionality and pushed it to your branch. If the test passes I am going to merge this code and get it in for the v1.43 release of the provider. Thanks for your help, this is great! 🚀 |
|
@WodansSon I definitely don't mind. I hadn't added one because of the external dependencies. What are the typical practices around acceptance tests that have external dependencies that need to be set up before they can pass? For example, in this case a custom domain and CNAME record for validation would need to be created and the frontend name updated before the tests could pass. |
|
@sean-nixon those are more difficult, and the need to continually test it is made on a case by case basis. However, here we can work around that by using the FrontDoor managed Custom HTTPS cert provided by Microsoft, so as to not add any external dependencies. A truly custom HTTPS cert would be very difficult to code, so this way we can at least verify that the settings get set. 😃 |
|
Oh, I see what you did now. I misread your configuration. Does this actually succeed for you locally? I didn't think the default |
|
Yep, it works just fine for me... they may have changed something in the new API that allows this. |
|
|
|
This has been released in version 1.43.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 1.43.0"
}
# ... other configuration ... |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #5538
The latest version of the Front Door API requires a
minimumTlsVersionproperty set in the request to enable custom domain HTTPS. This PR adds support for this property with a default of "1.0" to preserved backwards compatibility. Since September 2019, Azure is making TLS 1.2 the default minimum. Once version 2.0 is released, I think the default should be updated to 1.2 to align with Azure's defaults. I've included a TODO comment and note in the upgrade guide to mark this.