hwdsl2 / setup-ipsec-vpn

Scripts to build your own IPsec VPN server, with IPsec/L2TP, Cisco IPsec and IKEv2
Other
25.26k stars 6.32k forks source link

Cloudformation Template Improvements #1463

Closed scottpedia closed 1 year ago

scottpedia commented 1 year ago

AWS added means to create KeyPair resources directly with Cloudformation, so I replaced the legacy lambda-backed custom resource with an AWS::KeyPair resource. That way the key pair created is automatically deleted when the stack is deleted. Users retrieve the private key by using the AWS CLI. Corresponding documentations are updated too.

hwdsl2 commented 1 year ago

@scottpedia Thank you for contributing! I looked at your PR and there are two issues:

  1. The name of the created S3 bucket now contains the stack name, instead of 20 random characters. This could have much less entropy and may be less secure.
  2. Retrieving the private key now requires the user to set up AWS CLI tools on their local machine, and obtain the necessary API key credentials by visiting the AWS IAM console. Then they can use the command to retrieve the private key. I feel that this may be more steps for the user, and not all users know how to do it.

The original method does not have these two issues. Maybe we can keep using the original method? What do you think?

scottpedia commented 1 year ago

@hwdsl2 Hi

I think you are right. The existing solution has many problems. But I have an idea.

There is something that I can do to solve both issues. It is to create an extra lambda-backed custom resource that does the following things in the script:

  1. To generate a random combination of characters for the naming of the S3 bucket.
  2. To retrieve the key material and display it in the Outputs.

Comparatively speaking this solution has one single advantage over our traditional method, that is the created key pair can be automatically deleted as the stack is deleted.

It was just hours ago when I found out the embedded script has been creating two key pairs each time it is run instead of one. I did fix that yes but there is no way over the traditional method that we can make key pairs delete themselves when the stack is deleted. (due to an actual bug in the design of the template structure)

scottpedia commented 1 year ago

I think I will start working on it tomorrow if we are to proceed. Please let me know what you think. @hwdsl2

hwdsl2 commented 1 year ago

@scottpedia Thanks for providing your thoughts. You can give it a try but let's keep using the original method (with the duplicate key pair fixed) if this is complex to implement.

scottpedia commented 1 year ago

@hwdsl2 Hi I got it working. Now it does not have the problem of creating duplicate keys, and it automatically cleans up the created key pair during stack deletion. The S3 bucket's name is now also derived from the generated random combination of 20 characters. It also gives user a choice on how to retrieve the private key(copy&paste, or AWS CLI).