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

support creating routes for multiple subnets #15

Merged
merged 1 commit into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/slack-notifications.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ cfn-vpn modify [name] --slack-webhook-url "https://hooks.slack.com/services/T000
- `ROUTE_LIMIT_EXCEEDED`: no new routes can be added to the route table due to AWS route table limit
- `AUTH_RULE_LIMIT_EXCEEDED`: no new authorization rules can be added to the rule list due to AWS auth rule limit
- `RESOLVE_FAILED`: failed to resolve the provided dns entry
- `RATE_LIMIT_EXCEEDED`: concurrent modifications of the route table is being rated limited
- `SUBNET_NOT_ASSOCIATED`: no subnets are associated with the Client VPN
- `QUOTA_INCREASE_REQUEST`: automatic quota increase made

Expand Down
7 changes: 3 additions & 4 deletions lib/cfnvpn/actions/embedded.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ def download_certificates

def download_config
vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
@endpoint_id = vpn.get_endpoint_id()
CfnVpn::Log.logger.debug "downloading client config for #{@endpoint_id}"
@config = vpn.get_config(@endpoint_id)
CfnVpn::Log.logger.debug "downloading client config for #{vpn.endpoint_id}"
@config = vpn.get_config()
string = (0...8).map { (65 + rand(26)).chr.downcase }.join
@config.sub!(@endpoint_id, "#{string}.#{@endpoint_id}")
@config.sub!(vpn.endpoint_id, "#{string}.#{vpn.endpoint_id}")
end

def add_routes
Expand Down
3 changes: 1 addition & 2 deletions lib/cfnvpn/actions/init.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ def deploy_vpn

def finish
vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
@endpoint_id = vpn.get_endpoint_id()
CfnVpn::Log.logger.info "Client VPN #{@endpoint_id} created. Run `cfn-vpn config #{@name}` to setup the client config"
CfnVpn::Log.logger.info "Client VPN #{vpn.endpoint_id} created. Run `cfn-vpn config #{@name}` to setup the client config"
end

end
Expand Down
3 changes: 1 addition & 2 deletions lib/cfnvpn/actions/modify.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ def deploy_vpn

def finish
vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
@endpoint_id = vpn.get_endpoint_id()
CfnVpn::Log.logger.info "Client VPN #{@endpoint_id} modified."
CfnVpn::Log.logger.info "Client VPN #{vpn.endpoint_id} modified."
end

end
Expand Down
5 changes: 2 additions & 3 deletions lib/cfnvpn/actions/revoke.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ def revoke_certificate

def apply_rekocation_list
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming this should be revocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol v and k and not even close on the keyboard, i'll resolve this in a separate PR

vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
endpoint_id = vpn.get_endpoint_id()
vpn.put_revoke_list(endpoint_id,"#{@cert_dir}/crl.pem")
CfnVpn::Log.logger.info("revoked client #{@options['client_cn']} from #{endpoint_id}")
vpn.put_revoke_list("#{@cert_dir}/crl.pem")
CfnVpn::Log.logger.info("revoked client #{@options['client_cn']} from #{vpn.endpoint_id}")
end

end
Expand Down
36 changes: 20 additions & 16 deletions lib/cfnvpn/actions/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Routes < Thor::Group

class_option :cidr, desc: 'cidr range'
class_option :dns, desc: 'dns record to auto lookup ip'
class_option :subnet, desc: 'the target vpc subnet to route through, if none is supplied the default subnet is used'
class_option :subnets, type: :array, desc: 'target vpc subnets to route through, if none is supplied the default subnets are used'
class_option :desc, desc: 'description of the route'

class_option :groups, type: :array, desc: 'override all authorised groups on thr route'
Expand Down Expand Up @@ -83,23 +83,23 @@ def set_route
CfnVpn::Log.logger.warn "description for this route cannot be updated in place. To alter delete the route and add with the new description"
end

if @options[:subnet]
CfnVpn::Log.logger.warn "the target subnet for this route cannot be updated in place. To alter delete the route and add with the new target subnet"
if @options[:subnets]
CfnVpn::Log.logger.warn "the target subnets for this route cannot be updated in place. To alter delete the route and add with the new target subnet"
end
elsif !@route && @options[:cidr]
CfnVpn::Log.logger.info "adding new route for #{@options[:cidr]}"
@config[:routes] << {
cidr: @options[:cidr],
desc: @options.fetch(:desc, ""),
subnet: @options.fetch(:subnet, @config[:subnet_ids].first),
subnets: @options.fetch(:subnets, @config[:subnet_ids]),
groups: @options.fetch(:groups, []) + @options.fetch(:add_groups, [])
}
elsif !@route && @options[:dns]
CfnVpn::Log.logger.info "adding new route lookup for dns record #{@options[:dns]}"
@config[:routes] << {
dns: @options[:dns],
desc: @options.fetch(:desc, ""),
subnet: @options.fetch(:subnet, @config[:subnet_ids].first),
subnets: @options.fetch(:subnets, @config[:subnet_ids]),
groups: @options.fetch(:groups, []) + @options.fetch(:add_groups, [])
}
else
Expand Down Expand Up @@ -163,27 +163,31 @@ def deploy_vpn
end
end

def get_routes
@vpn = CfnVpn::ClientVpn.new(@name, @options['region'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to exist? And if so what does it do? I can see @vpn gets used below in other code, not sure of the connection to this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a Thor Actions implementation include Thor::Actions https://github.com/rails/thor/wiki/Actions

Thor doco is not great but it's basically executing each of these methods in the class from top to bottom. The method name is poorly worded it should be def setup. What it's doing is setting an instance of the ClientVpn class which can then be consumed further down the line.

end

def cleanup_dns_routes
@vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
unless @dns_route_cleanup.nil?
routes = @vpn.get_routes()
CfnVpn::Log.logger.info("Cleaning up expired routes for #{@dns_route_cleanup}")
expired_routes = routes.select {|route| route.description.include?(@dns_route_cleanup) }
expired_routes = @vpn.get_routes(@dns_route_cleanup)
expired_routes.each do |route|
CfnVpn::Log.logger.info("Removing expired route #{route.destination_cidr} for target subnet #{route.target_subnet}")
@vpn.delete_route(route.destination_cidr, route.target_subnet)
@vpn.revoke_auth(route.destination_cidr)
end
end
end

def get_routes
@endpoint = @vpn.get_endpoint_id()
@routes = @vpn.get_routes()
expired_rules = @vpn.get_auth_rules(@dns_route_cleanup)
expired_rules.each do |rule|
CfnVpn::Log.logger.info("Removing expired auth rule for route #{route.destination_cidr}")
@vpn.revoke_auth(rule.destination_cidr)
end
end
end

def display_routes
rows = @routes.collect do |s|
groups = @vpn.get_groups_for_route(@endpoint, s.destination_cidr)
routes = @vpn.get_routes()
rows = routes.collect do |s|
groups = @vpn.get_groups_for_route(s.destination_cidr)
[ s.destination_cidr, s.description, s.status.code, s.target_subnet, s.type, s.origin, (!groups.join("").empty? ? groups.join(' ') : 'AllowAll') ]
end
table = Terminal::Table.new(
Expand Down
9 changes: 4 additions & 5 deletions lib/cfnvpn/actions/sessions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,19 @@ def set_directory
@build_dir = "#{CfnVpn.cfnvpn_path}/#{@name}"
end

def get_endpoint
def setup
Copy link
Contributor

Choose a reason for hiding this comment

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

same thing here, is this a thor thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see above

@vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
@endpoint_id = @vpn.get_endpoint_id()
end

def kill_session
if !@options['kill'].nil?
sessions = @vpn.get_sessions(@endpoint_id)
sessions = @vpn.get_sessions()
session = sessions.select { |s| s if s.connection_id == @options['kill'] }.first
if session.any? && session.status.code == "active"
terminate = yes? "Terminate connection #{@options['kill']} for #{session.common_name}?", :yellow
if terminate
CfnVpn::Log.logger.info "Terminating connection #{@options['kill']} for #{session.common_name}"
@vpn.kill_session(@endpoint_id,@options['kill'])
@vpn.kill_session(@options['kill'])
end
else
CfnVpn::Log.logger.error "Connection id #{@options['kill']} doesn't exist or is not active"
Expand All @@ -51,7 +50,7 @@ def kill_session
end

def display_sessions
sessions = @vpn.get_sessions(@endpoint_id)
sessions = @vpn.get_sessions()
rows = sessions.collect do |s|
[ s.common_name, s.connection_established_time, s.status.code, s.client_ip, s.connection_id, s.ingress_bytes, s.egress_bytes ]
end
Expand Down
7 changes: 3 additions & 4 deletions lib/cfnvpn/actions/share.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ def set_loglevel

def copy_config_to_s3
vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
@endpoint_id = vpn.get_endpoint_id()
CfnVpn::Log.logger.debug "downloading client config for #{@endpoint_id}"
@config = vpn.get_config(@endpoint_id)
CfnVpn::Log.logger.debug "downloading client config for #{vpn.endpoint_id}"
@config = vpn.get_config()
string = (0...8).map { (65 + rand(26)).chr.downcase }.join
@config.sub!(@endpoint_id, "#{string}.#{@endpoint_id}")
@config.sub!(vpn.endpoint_id, "#{string}.#{vpn.endpoint_id}")
end

def add_routes
Expand Down
8 changes: 2 additions & 6 deletions lib/cfnvpn/actions/subnets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,9 @@ def disassociate
end
end

def get_endpoint
@vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
@endpoint_id = @vpn.get_endpoint_id()
end

def associations
associations = @vpn.get_associations(@endpoint_id)
vpn = CfnVpn::ClientVpn.new(@name,@options['region'])
associations = vpn.get_associations()
table = Terminal::Table.new(
:headings => ['ID', 'Subnet', 'Status', 'CIDR', 'AZ', 'Groups'],
:rows => associations.map {|ass| ass.values})
Expand Down
60 changes: 38 additions & 22 deletions lib/cfnvpn/clientvpn.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
module CfnVpn
class ClientVpn

attr_reader :endpoint_id

def initialize(name,region)
@client = Aws::EC2::Client.new(region: region)
Expand All @@ -31,56 +32,71 @@ def get_dns_servers()
return get_endpoint().dns_servers
end

def get_config(endpoint_id)
def get_config()
resp = @client.export_client_vpn_client_configuration({
client_vpn_endpoint_id: endpoint_id
client_vpn_endpoint_id: @endpoint_id
})
return resp.client_configuration
end

def get_rekove_list(endpoint_id)
def get_rekove_list()
Copy link
Contributor

Choose a reason for hiding this comment

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

revoke

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix this name in a separate PR. This repo needs a going over with my new found spell checker

resp = @client.export_client_vpn_client_certificate_revocation_list({
client_vpn_endpoint_id: endpoint_id
client_vpn_endpoint_id: @endpoint_id
})
return resp.certificate_revocation_list
end

def put_revoke_list(endpoint_id,revoke_list)
def put_revoke_list(revoke_list)
list = File.read(revoke_list)
@client.import_client_vpn_client_certificate_revocation_list({
client_vpn_endpoint_id: endpoint_id,
client_vpn_endpoint_id: @endpoint_id,
certificate_revocation_list: list
})
end

def get_sessions(endpoint_id)
def get_sessions()
params = {
client_vpn_endpoint_id: endpoint_id,
client_vpn_endpoint_id: @endpoint_id,
max_results: 20
}
resp = @client.describe_client_vpn_connections(params)
return resp.connections
end

def kill_session(endpoint_id, connection_id)
def kill_session(connection_id)
@client.terminate_client_vpn_connections({
client_vpn_endpoint_id: endpoint_id,
client_vpn_endpoint_id: @endpoint_id,
connection_id: connection_id
})
end

def get_routes()
endpoint_id = get_endpoint_id()
resp = @client.describe_client_vpn_routes({
client_vpn_endpoint_id: endpoint_id,
max_results: 20
})
return resp.routes
def get_routes(dns_route=nil)
routes = []
@client.describe_client_vpn_routes({client_vpn_endpoint_id: @endpoint_id}).each do |resp|
if dns_route
routes.concat resp.routes.select {|route| route.description.include?(dns_route) }
else
routes.concat resp.routes
end
end
return routes
end

def get_groups_for_route(endpoint, cidr)
def get_auth_rules(dns_route=nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

is dns route the correct variable here and in the ensuing loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dns_route variable is used here because the value is the dns endpoint that is used to generate the routes. This function searches for authorisation rules with the value of dns_route in the description.

rules = []
@client.describe_client_vpn_authorization_rules({client_vpn_endpoint_id: @endpoint_id}) do |resp|
if dns_route
rules.concat resp.authorization_rules.select {|rule| rule.description.include?(dns_route) }
else
rules.concat resp.routes
end
end
return rules
end

def get_groups_for_route(cidr)
auth_resp = @client.describe_client_vpn_authorization_rules({
client_vpn_endpoint_id: endpoint,
client_vpn_endpoint_id: @endpoint_id,
filters: [
{
name: 'destination-cidr',
Expand All @@ -91,18 +107,18 @@ def get_groups_for_route(endpoint, cidr)
return auth_resp.authorization_rules.map {|rule| rule.group_id }
end

def get_associations(endpoint)
def get_associations()
associations = []
resp = @client.describe_client_vpn_target_networks({
client_vpn_endpoint_id: endpoint
client_vpn_endpoint_id: @endpoint_id
})

resp.client_vpn_target_networks.each do |net|
subnet_resp = @client.describe_subnets({
subnet_ids: [net.target_network_id]
})
subnet = subnet_resp.subnets.first
groups = get_groups_for_route(endpoint, subnet.cidr_block)
groups = get_groups_for_route(subnet.cidr_block)

associations.push({
association_id: net.association_id,
Expand Down
Loading