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

aws_vpn_connection resource's customer_gateway_configuration should be sensitive #15806

Conversation

b-dean
Copy link
Contributor

@b-dean b-dean commented Oct 23, 2020

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

The Problem

The computed customer_gateway_configuration on aws_vpn_connection contains the raw XML of the VPN connection. This includes all the pre-shared keys which are already sensitive on the resource. Seems like the xml should be too so you don't see it in the stdout when the resource is destroyed:

# aws_vpn_connection.example will be destroyed
   - resource "aws_vpn_connection" "example" {
       - arn                            = "arn:aws:ec2:us-east-1:11111111:vpn-connection/vpn-11111111" -> null
       - customer_gateway_configuration = <<~EOT
             <?xml version="1.0" encoding="UTF-8"?>
             <vpn_connection id="vpn-11111111">
               <customer_gateway_id>cgw-11111111</customer_gateway_id>
               <vpn_gateway_id>vgw-11111111</vpn_gateway_id>
               <vpn_connection_type>ipsec.1</vpn_connection_type>
               <vpn_connection_attributes>NoBGPVPNConnection</vpn_connection_attributes>
               <ipsec_tunnel>
                 <customer_gateway>
                   <tunnel_outside_address>
                     <ip_address>1.2.3.4</ip_address>
                   </tunnel_outside_address>
                   <tunnel_inside_address>
                     <ip_address>4.3.2.1</ip_address>
                     <network_mask>255.255.255.252</network_mask>
                     <network_cidr>30</network_cidr>
                   </tunnel_inside_address>
                 </customer_gateway>
                 <vpn_gateway>
                   <tunnel_outside_address>
                     <ip_address>9.8.7.6</ip_address>
                   </tunnel_outside_address>
                   <tunnel_inside_address>
                     <ip_address>6.7.8.9</ip_address>
                     <network_mask>255.255.255.252</network_mask>
                     <network_cidr>30</network_cidr>
                   </tunnel_inside_address>
                 </vpn_gateway>
                 <ike>
                   <authentication_protocol>sha1</authentication_protocol>
                   <encryption_protocol>aes-128-cbc</encryption_protocol>
                   <lifetime>28800</lifetime>
                   <perfect_forward_secrecy>group2</perfect_forward_secrecy>
                   <mode>main</mode>
                   <pre_shared_key>xxxxxxxYYYYYYYzzzzzz</pre_shared_key>
                 </ike>
                 <ipsec>
                   <protocol>esp</protocol>
                   <authentication_protocol>hmac-sha1-96</authentication_protocol>
                   <encryption_protocol>aes-128-cbc</encryption_protocol>
                   <lifetime>3600</lifetime>
                   <perfect_forward_secrecy>group2</perfect_forward_secrecy>
                   <mode>tunnel</mode>
                   <clear_df_bit>true</clear_df_bit>
                   <fragmentation_before_encryption>true</fragmentation_before_encryption>
                   <tcp_mss_adjustment>1379</tcp_mss_adjustment>
                   <dead_peer_detection>
                     <interval>10</interval>
                     <retries>3</retries>
                   </dead_peer_detection>
                 </ipsec>
               </ipsec_tunnel>
               <ipsec_tunnel>
                 <customer_gateway>
                   <tunnel_outside_address>
                     <ip_address>2.3.4.5</ip_address>
                   </tunnel_outside_address>
                   <tunnel_inside_address>
                     <ip_address>5.4.3.2</ip_address>
                     <network_mask>255.255.255.252</network_mask>
                     <network_cidr>30</network_cidr>
                   </tunnel_inside_address>
                 </customer_gateway>
                 <vpn_gateway>
                   <tunnel_outside_address>
                     <ip_address>4.5.6.7</ip_address>
                   </tunnel_outside_address>
                   <tunnel_inside_address>
                     <ip_address>7.6.5.4</ip_address>
                     <network_mask>255.255.255.252</network_mask>
                     <network_cidr>30</network_cidr>
                   </tunnel_inside_address>
                 </vpn_gateway>
                 <ike>
                   <authentication_protocol>sha1</authentication_protocol>
                   <encryption_protocol>aes-128-cbc</encryption_protocol>
                   <lifetime>28800</lifetime>
                   <perfect_forward_secrecy>group2</perfect_forward_secrecy>
                   <mode>main</mode>
                   <pre_shared_key>zzzzzzzzYYYYYYYYxxxxxx</pre_shared_key>
                 </ike>
                 <ipsec>
                   <protocol>esp</protocol>
                   <authentication_protocol>hmac-sha1-96</authentication_protocol>
                   <encryption_protocol>aes-128-cbc</encryption_protocol>
                   <lifetime>3600</lifetime>
                   <perfect_forward_secrecy>group2</perfect_forward_secrecy>
                   <mode>tunnel</mode>
                   <clear_df_bit>true</clear_df_bit>
                   <fragmentation_before_encryption>true</fragmentation_before_encryption>
                   <tcp_mss_adjustment>1379</tcp_mss_adjustment>
                   <dead_peer_detection>
                     <interval>10</interval>
                     <retries>3</retries>
                   </dead_peer_detection>
                 </ipsec>
               </ipsec_tunnel>
             </vpn_connection>
         EOT -> null
       - customer_gateway_id            = "cgw-1111111111111" -> null
       - id                             = "vpn-1111111111111" -> null
       - routes                         = [
           - {
               - destination_cidr_block = "1.1.1.0/24"
               - source                 = ""
               - state                  = "available"
             },
         ] -> null
       - static_routes_only             = true -> null
       - tags                           = {
           - "Name"        = "example"
         } -> null
       - tunnel1_address                = "1.2.3.4" -> null
       - tunnel1_bgp_holdtime           = 0 -> null
       - tunnel1_cgw_inside_address     = "4.3.2.1" -> null
       - tunnel1_preshared_key          = (sensitive value)
       - tunnel1_vgw_inside_address     = "9.8.7.6" -> null
       - tunnel2_address                = "6.7.8.9" -> null
       - tunnel2_bgp_holdtime           = 0 -> null
       - tunnel2_cgw_inside_address     = "4.5.6.7" -> null
       - tunnel2_preshared_key          = (sensitive value)
       - tunnel2_vgw_inside_address     = "7.6.5.4" -> null
       - type                           = "ipsec.1" -> null
       - vgw_telemetry                  = [
           - {
               - accepted_route_count = 1
               - last_status_change   = "2020-01-24T11:56:58Z"
               - outside_ip_address   = "1.2.3.4"
               - status               = "DOWN"
               - status_message       = ""
             },
           - {
               - accepted_route_count = 1
               - last_status_change   = "2020-10-09T20:17:13Z"
               - outside_ip_address   = "2.3.4.5"
               - status               = "DOWN"
               - status_message       = ""
             },
         ] -> null
       - vpn_gateway_id                 = "vgw-111111111111" -> null
     }

(note that the IPs and resources have been changed in that destroy plan example, but you get the idea).

Notice how tunnel1_preshared_key and tunnel2_preshared_key are (sensitive) but the raw XML which also contains those keys is not.

Release note, etc

Release note for CHANGELOG:

`aws_vpn_connection` resource attribute `customer_gateway_configuration` is sensitive

Output from acceptance testing:

This doesn't really affect any acceptance tests as far as I can tell. I could not find any tests in this provider that were checking that sensitive attributes were not displayed. hashicorp/terraform has tests for that.

Relates #20433.
Relates #22776.
Supersedes #18495.

@b-dean b-dean requested a review from a team October 23, 2020 13:41
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. labels Oct 23, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 23, 2020
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 568f58e to 0c2c88e Compare January 5, 2021 22:09
@b-dean b-dean requested a review from a team as a code owner January 5, 2021 22:09
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch 2 times, most recently from 7d6a280 to 376a54d Compare January 15, 2021 20:59
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 376a54d to 0b171d9 Compare January 22, 2021 17:01
Base automatically changed from master to main January 23, 2021 00:59
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 0b171d9 to 43c5fe0 Compare January 25, 2021 21:17
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch 2 times, most recently from 844a272 to 63e2e26 Compare February 5, 2021 14:16
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch 2 times, most recently from e1c5839 to 784414e Compare February 19, 2021 16:58
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 784414e to 85f7291 Compare February 23, 2021 16:33
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 85f7291 to 6857af9 Compare March 5, 2021 18:57
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch 2 times, most recently from 62af0e3 to da70d2e Compare March 23, 2021 13:00
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch 2 times, most recently from cc4f828 to bfb287f Compare March 26, 2021 13:58
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from bfb287f to 70a1a5d Compare April 2, 2021 13:57
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 70a1a5d to 18de301 Compare April 19, 2021 18:57
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 18de301 to 80a6335 Compare May 21, 2021 16:38
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 80a6335 to 28f6a1e Compare June 14, 2021 15:33
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 28f6a1e to b7f9d51 Compare July 27, 2021 19:08
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from b7f9d51 to d49605b Compare August 23, 2021 16:23
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from d49605b to c03cc03 Compare September 9, 2021 13:04
@breathingdust breathingdust added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 16, 2021
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from c03cc03 to 4a436b0 Compare September 24, 2021 14:54
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 4a436b0 to c7801c6 Compare October 5, 2021 19:12
@zhelding
Copy link
Contributor

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from c7801c6 to 69e317d Compare October 20, 2021 14:15
@b-dean
Copy link
Contributor Author

b-dean commented Oct 20, 2021

@zhelding, rebased and fixed for the latest changes on main

@ewbankkit ewbankkit added this to the v4.0.0 milestone Oct 26, 2021
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 69e317d to 5cdcafe Compare November 12, 2021 19:18
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from 5cdcafe to ba3c761 Compare January 3, 2022 14:42
@b-dean b-dean force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch 2 times, most recently from 349dd0a to a0944ef Compare January 21, 2022 14:58
b-dean and others added 2 commits January 24, 2022 07:59
…e sensitive because the xml contains tunnel psk

Signed-off-by: Ben Dean <ben.dean@ontariosystems.com>
@ewbankkit ewbankkit force-pushed the b-aws_vpn_connection.customer_gateway_configuration-sensitive branch from a0944ef to ac6fec9 Compare January 24, 2022 13:02
@ewbankkit ewbankkit changed the base branch from main to release/4.x January 24, 2022 13:32
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccEC2VPNConnection_basic\|TestAccEC2VPNConnection_tunnelOptions' PKG=ec2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 20  -run=TestAccEC2VPNConnection_basic\|TestAccEC2VPNConnection_tunnelOptions -timeout 180m
=== RUN   TestAccEC2VPNConnection_basic
=== PAUSE TestAccEC2VPNConnection_basic
=== RUN   TestAccEC2VPNConnection_tunnelOptions
=== PAUSE TestAccEC2VPNConnection_tunnelOptions
=== RUN   TestAccEC2VPNConnection_tunnelOptionsLesser
=== PAUSE TestAccEC2VPNConnection_tunnelOptionsLesser
=== CONT  TestAccEC2VPNConnection_basic
=== CONT  TestAccEC2VPNConnection_tunnelOptionsLesser
=== CONT  TestAccEC2VPNConnection_tunnelOptions
--- PASS: TestAccEC2VPNConnection_tunnelOptions (281.52s)
--- PASS: TestAccEC2VPNConnection_basic (470.06s)
--- PASS: TestAccEC2VPNConnection_tunnelOptionsLesser (1537.85s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/ec2        1543.790s

Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccEC2VPNConnection_basic\|TestAccEC2VPNConnection_tunnelOptions' PKG=ec2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 20  -run=TestAccEC2VPNConnection_basic\|TestAccEC2VPNConnection_tunnelOptions -timeout 180m
=== RUN   TestAccEC2VPNConnection_basic
=== PAUSE TestAccEC2VPNConnection_basic
=== RUN   TestAccEC2VPNConnection_tunnelOptions
=== PAUSE TestAccEC2VPNConnection_tunnelOptions
=== RUN   TestAccEC2VPNConnection_tunnelOptionsLesser
=== PAUSE TestAccEC2VPNConnection_tunnelOptionsLesser
=== CONT  TestAccEC2VPNConnection_basic
=== CONT  TestAccEC2VPNConnection_tunnelOptionsLesser
=== CONT  TestAccEC2VPNConnection_tunnelOptions
--- PASS: TestAccEC2VPNConnection_tunnelOptions (281.52s)
--- PASS: TestAccEC2VPNConnection_basic (470.06s)
--- PASS: TestAccEC2VPNConnection_tunnelOptionsLesser (1537.85s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/ec2        1543.790s

@ewbankkit ewbankkit merged commit e7f6a69 into hashicorp:release/4.x Jan 24, 2022
@github-actions
Copy link

This functionality has been released in v4.0.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants