Skip to content

GitLab

Unverified Commit 10a17e0a authored by Francesc Guasch's avatar Francesc Guasch Committed by GitHub
Browse files

fix(networking): properly deny restricted ports (#1147) 

issue #1134
parent 0cbe5265
Hide whitespace changes
Inline Side-by-side
lib/Ravada.pm
View file @ 10a17e0a
......@@ -3,7 +3,7 @@ package Ravada;
use warnings;
use strict;
our $VERSION = '0.5.0-beta2';
our $VERSION = '0.5.0-beta8';
use Carp qw(carp croak);
use Data::Dumper;
......@@ -1270,6 +1270,8 @@ sub _upgrade_tables {
$self->_upgrade_table('vms','mac','char(18)');
$self->_upgrade_table('volumes','name','char(200)');
$self->_upgrade_table('domain_ports', 'internal_ip','char(200)');
}
......
lib/Ravada/Domain.pm
View file @ 10a17e0a
......@@ -2180,6 +2180,8 @@ sub expose($self, @args) {
$id_port = delete $args{id_port};
$internal_port = delete $args{port};
$internal_port = delete $args{internal_port} if exists $args{internal_port};
delete $args{internal_ip};
confess "Error: Missing port" if !defined $internal_port && !$id_port;
confess "Error: internal port not a number '".($internal_port or '<UNDEF>')."'"
if defined $internal_port && $internal_port !~ /^\d+$/;
......@@ -2281,6 +2283,11 @@ sub _open_exposed_port($self, $internal_port, $restricted) {
confess "Error: I can't get the internal IP of ".$self->name
if !$internal_ip || $internal_ip !~ /^(\d+\.\d+)/;
$sth = $$CONNECTOR->dbh->prepare("UPDATE domain_ports set internal_ip=?"
." WHERE id_domain=? AND internal_port=?"
);
$sth->execute($internal_ip, $self->id, $internal_port);
if ( !$> ) {
$self->_vm->iptables_unique(
t => 'nat'
......@@ -2292,10 +2299,8 @@ sub _open_exposed_port($self, $internal_port, $restricted) {
,'to-destination' => "$internal_ip:$internal_port"
) if !$>;
if ($restricted) {
$self->_open_exposed_port_client($public_port);
}
$self->_open_iptables_state();
$self->_open_exposed_port_client($internal_port, $restricted);
}
}
......@@ -2312,29 +2317,34 @@ sub _open_iptables_state($self) {
);
}
sub _open_exposed_port_client($self, $public_port) {
my $remote_ip = $self->remote_ip;
return if !$remote_ip;
sub _open_exposed_port_client($self, $internal_port, $restricted) {
my $local_ip = $self->_vm->ip;
my $internal_ip = $self->ip;
my $remote_ip = '0.0.0.0/0';
$remote_ip = $self->remote_ip if $restricted;
return if !$remote_ip;
if ( $restricted ) {
$self->_vm->iptables_unique(
I => 'FORWARD'
,d => $internal_ip
,m => 'tcp'
,p => 'tcp'
,dport => $internal_port
,j => 'DROP'
);
}
$self->_vm->iptables_unique(
A => $IPTABLES_CHAIN
I => 'FORWARD'
,s => $remote_ip
,d => $local_ip
,d => $internal_ip
,m => 'tcp'
,p => 'tcp'
,dport => $public_port
,dport => $internal_port
,j => 'ACCEPT'
);
$self->_vm->iptables_unique(
A => $IPTABLES_CHAIN
,d => $local_ip
,m => 'tcp'
,p => 'tcp'
,dport => $public_port
,j => 'DROP'
);
}
sub open_exposed_ports($self) {
......@@ -2351,7 +2361,7 @@ sub open_exposed_ports($self) {
}
sub _close_exposed_port($self,$internal_port_req=undef) {
my $query = "SELECT public_port,internal_port "
my $query = "SELECT public_port,internal_port, internal_ip "
." FROM domain_ports"
." WHERE id_domain=? ";
$query .= " AND internal_port=?" if $internal_port_req;
......@@ -2365,9 +2375,11 @@ sub _close_exposed_port($self,$internal_port_req=undef) {
}
my %port;
while ( my ($public_port, $internal_port) = $sth->fetchrow() ) {
$port{$public_port} = $internal_port;
while ( my $row = $sth->fetchrow_hashref() ) {
lock_hash(%$row);
$port{$row->{public_port}} = $row;
}
lock_hash(%port);
my $iptables = $self->_vm->iptables_list();
......@@ -2382,17 +2394,24 @@ sub _close_exposed_port($self,$internal_port_req=undef) {
sub _close_exposed_port_client($self, $iptables, %port) {
my $ip = $self->_vm->ip."/32";
my %ip = map {
my $ip = '0.0.0.0/0';
$ip = $port{$_}->{internal_ip}."/32" if $port{$_}->{internal_ip};
$port{$_}->{internal_port} => $ip;
} keys %port;
for my $line (@{$iptables->{'filter'}}) {
my %args = @$line;
next if $args{A} ne 'RAVADA';
next if $args{A} ne 'FORWARD';
if (exists $args{j}
&& exists $args{d} && $args{d} eq $ip
&& exists $args{dport} && $port{$args{dport}}) {
&& exists $args{dport} && $ip{$args{dport}}
&& exists $args{d} && $args{d} eq $ip{$args{dport}}
) {
my @delete = (
D => 'RAVADA'
D => 'FORWARD'
, p => 'tcp', m => 'tcp'
, d => $ip
, d => $ip{$args{dport}}
, dport => $args{dport}
, j => $args{j}
);
......@@ -2412,7 +2431,7 @@ sub _close_exposed_port_nat($self, $iptables, %port) {
&& exists $args{dport}
&& exists $args{'to-destination'}
) {
my $internal_port = $port{$args{dport}} or next;
my $internal_port = $port{$args{dport}}->{internal_port} or next;
if ( $args{'to-destination'}=~/\:$internal_port$/ ) {
my %delete = %args;
delete $delete{A};
......
lib/Ravada/VM.pm
View file @ 10a17e0a
......@@ -1234,16 +1234,16 @@ sub remove_file( $self, $file ) {
return $self->run_command("/bin/rm", $file);
}
sub create_iptables_chain($self,$chain) {
sub create_iptables_chain($self, $chain, $jchain='INPUT') {
my ($out, $err) = $self->run_command("/sbin/iptables","-n","-L",$chain);
$self->run_command("/sbin/iptables", '-N' => $chain)
if $out !~ /^Chain $chain/;
($out, $err) = $self->run_command("/sbin/iptables","-n","-L",'INPUT');
return if grep(/^RAVADA /, split(/\n/,$out));
($out, $err) = $self->run_command("/sbin/iptables","-n","-L",$jchain);
return if grep(/^$chain /, split(/\n/,$out));
$self->run_command("/sbin/iptables", '-A','INPUT', '-j' => $chain);
$self->run_command("/sbin/iptables", '-I', $jchain, '-j' => $chain);
}
......
t/kvm/n10_nodes.t
View file @ 10a17e0a
......@@ -279,7 +279,7 @@ sub test_sync_base {
my $vm =rvd_back->search_vm($vm_name);
my $base = create_domain($vm_name);
$base->add_volume(name => 'vdb', swap => 1, size => 512*1024 );
$base->add_volume(swap => 1, size => 512*1024 );
my $clone = $base->clone(
name => new_domain_name
,user => user_admin
......
t/lib/Test/Ravada.pm
View file @ 10a17e0a
......@@ -901,6 +901,8 @@ sub search_iptable_remote {
my $chain = (delete $args{chain} or $CHAIN);
my $to_dest = delete $args{'to-destination'};
confess "Error: unknown args ".Dumper(\%args) if keys %args;
my $iptables = $node->iptables_list();
$remote_ip .= "/32" if defined $remote_ip && $remote_ip !~ m{/};
......@@ -913,6 +915,7 @@ sub search_iptable_remote {
my %args = @$line;
next if $args{A} ne $chain;
$count++;
$args{s} = "0.0.0.0/0" if !exists $args{s} && exists $args{dport};
if(
(!defined $jump || exists $args{j} && $args{j} eq $jump )
......
t/nodes/10_basic.t
View file @ 10a17e0a
......@@ -20,7 +20,7 @@ init();
sub test_reuse_vm($node) {
my $domain = create_domain($node->type);
$domain->add_volume(name => 'vdb', swap => 1, size => 512 * 1024);
$domain->add_volume(swap => 1, size => 512 * 1024);
$domain->prepare_base(user_admin);
$domain->set_base_vm(vm => $node, user => user_admin);
......
t/vm/92_ports.t
View file @ 10a17e0a
......@@ -42,13 +42,12 @@ sub test_no_dupe($vm) {
# No requests because no ports exposed
is(scalar @request,0) or exit;
delete_request('enforce_limits');
wait_request(debug => 1, background => 0);
wait_request(debug => 0, background => 0);
my $client_ip = $domain->remote_ip();
is($client_ip, $remote_ip);
my $public_port;
_wait_ip($vm->type, $domain);
my $internal_ip = $domain->ip;
my $internal_ip = _wait_ip($vm->type, $domain) or die "Error: no ip for ".$domain->name;
my $internal_net = $internal_ip;
$internal_net =~ s{(.*)\.\d+$}{$1.0/24};
......@@ -58,7 +57,7 @@ sub test_no_dupe($vm) {
, restricted => 1
);
delete_request('enforce_limits');
wait_request(background => 0, debug => 1);
wait_request(background => 0, debug => 0);
run3(['/sbin/iptables','-t','nat','-L','PREROUTING','-n'],\($in, $out, $err));
@out = split /\n/,$out;
......@@ -69,10 +68,10 @@ sub test_no_dupe($vm) {
is(grep(m{^ACCEPT.*$internal_net\s+state NEW},@out),1,"Expecting rule for $internal_net")
or die $out;
run3(['/sbin/iptables','-L','RAVADA','-n'],\($in, $out, $err));
run3(['/sbin/iptables','-L','FORWARD','-n'],\($in, $out, $err));
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*$remote_ip\s+$local_ip.*dpt:$public_port},@out),1) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$local_ip.*dpt:$public_port},@out),1) or die $out;
is(grep(m{^ACCEPT.*$remote_ip\s+$internal_ip.*dpt:$internal_port},@out),1) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$internal_ip.*dpt:$internal_port},@out),1) or die $out;
##########################################################################
#
......@@ -88,10 +87,10 @@ sub test_no_dupe($vm) {
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*$internal_net\s+state NEW},@out),1) or die $out;
run3(['/sbin/iptables','-L','RAVADA','-n'],\($in, $out, $err));
run3(['/sbin/iptables','-L','FORWARD','-n'],\($in, $out, $err));
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*$remote_ip\s+$local_ip.*dpt:$public_port},@out),1) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$local_ip.*dpt:$public_port},@out),1) or die $out;
is(grep(m{^ACCEPT.*$remote_ip\s+$internal_ip.*dpt:$internal_port},@out),1) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$internal_ip.*dpt:$internal_port},@out),1) or die $out;
test_hibernate($domain, $local_ip, $public_port, $internal_ip, $internal_port,$remote_ip);
test_start_after_hibernate($domain, $local_ip, $public_port, $internal_ip, $internal_port,$remote_ip);
......@@ -103,6 +102,7 @@ sub test_no_dupe($vm) {
sub test_hibernate($domain
,$local_ip, $public_port, $internal_ip, $internal_port, $remote_ip) {
$domain->hibernate(user_admin);
is($domain->is_hibernated,1);
my ($in,$out,$err);
run3(['/sbin/iptables','-t','nat','-L','PREROUTING','-n'],\($in, $out, $err));
......@@ -113,10 +113,10 @@ sub test_hibernate($domain
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*192.168.\d+\.0/24\sstate NEW},@out),0);
run3(['/sbin/iptables','-L','RAVADA','-n'],\($in, $out, $err));
run3(['/sbin/iptables','-L','FORWARD','-n'],\($in, $out, $err));
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*$remote_ip\s+$local_ip.*dpt:$public_port},@out),0) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$local_ip.*dpt:$public_port},@out),0) or die $out;
is(grep(m{^ACCEPT.*$remote_ip\s+$internal_ip.*dpt:$internal_port},@out),0) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$internal_ip.*dpt:$internal_port},@out),0) or die $out;
}
......@@ -129,7 +129,7 @@ sub test_start_after_hibernate($domain
$domain->start(user => user_admin, remote_ip => $remote_ip);
delete_request('enforce_limits');
wait_request(debug => 1, background => 0);
wait_request(debug => 0, background => 0);
my ($in,$out,$err);
run3(['/sbin/iptables','-t','nat','-L','PREROUTING','-n'],\($in, $out, $err));
......@@ -140,10 +140,10 @@ sub test_start_after_hibernate($domain
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*$internal_net\s+state NEW},@out),1) or die $out;
run3(['/sbin/iptables','-L','RAVADA','-n'],\($in, $out, $err));
run3(['/sbin/iptables','-L','FORWARD','-n'],\($in, $out, $err));
@out = split /\n/,$out;
is(grep(m{^ACCEPT.*$remote_ip\s+$local_ip.*dpt:$public_port},@out),1) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$local_ip.*dpt:$public_port},@out),1) or die $out;
is(grep(m{^ACCEPT.*$remote_ip\s+$internal_ip.*dpt:$internal_port},@out),1) or die $out;
is(grep(m{^DROP.*0.0.0.0.+$internal_ip.*dpt:$internal_port},@out),1) or die $out;
}
......@@ -238,7 +238,7 @@ sub test_one_port($vm) {
#
$domain->start(user => user_admin, remote_ip => $remote_ip);
delete_request('enforce_limits');
wait_request(debug => 1, background => 0);
wait_request(debug => 0, background => 0);
($n_rule)
= search_iptable_remote(local_ip => "$local_ip/32"
......@@ -501,7 +501,6 @@ sub _wait_ip {
my $vm_name = shift;
my $domain = shift or confess "Missing domain arg";
return if $domain->_vm->type !~ /kvm|qemu/i;
return $domain->ip if $domain->ip;
sleep 1;
......@@ -563,7 +562,7 @@ sub test_host_down {
$domain->start(user => user_admin, remote_ip => $remote_ip);
_wait_requests($domain);
wait_request(debug => 1, background => 0);
wait_request(debug => 0, background => 0);
my $domain_ip = $domain->ip;
ok($domain_ip,"[$vm_name] Expecting an IP for domain ".$domain->name.", got ".($domain_ip or '')) or return;
......@@ -680,6 +679,7 @@ sub test_restricted($vm, $restricted) {
$domain->start(user => user_admin, remote_ip => $remote_ip);
_wait_ip($vm->type, $domain);
my $internal_ip = $domain->ip;
my $internal_port = 22;
$domain->expose(port => $internal_port, restricted => $restricted);
......@@ -689,28 +689,32 @@ sub test_restricted($vm, $restricted) {
my $public_port = $list_ports[0]->{public_port};
is($list_ports[0]->{restricted}, $restricted);
my $remote_ip_check ='0.0.0.0/0';
$remote_ip_check = $remote_ip if $restricted;
my ($n_rule)
= search_iptable_remote(
local_ip => "$local_ip/32"
, remote_ip => $remote_ip
, local_port => $public_port
local_ip => "$internal_ip/32"
, chain => 'FORWARD'
, remote_ip => $remote_ip_check
, local_port => $internal_port
, node => $vm
, jump => 'ACCEPT'
);
my ($n_rule_drop)
= search_iptable_remote(
local_ip => "$local_ip/32"
, local_port => $public_port
local_ip => "$internal_ip/32"
, chain => 'FORWARD'
, local_port => $internal_port
, node => $vm
, jump => 'DROP'
);
ok($n_rule,"Expecting rule for $remote_ip_check -> $internal_ip:$internal_port")
or exit;
if ($restricted) {
ok($n_rule,"Expecting rule for $remote_ip -> $local_ip:$public_port") or exit;
ok($n_rule_drop,"Expecting drop rule for any -> $local_ip:$public_port") or exit;
ok($n_rule_drop,"Expecting drop rule for any -> $internal_ip:$internal_port") or exit;
} else {
ok(!$n_rule,"Expecting no rule for $remote_ip -> $local_ip:$public_port") or exit;
ok(!$n_rule_drop,"Expecting drop no rule for any -> $local_ip:$public_port") or exit;
ok(!$n_rule_drop,"Expecting drop no rule for any -> $internal_ip:$internal_port") or exit;
}
# check for FORWARD
......@@ -793,19 +797,30 @@ sub test_change_expose_3($vm) {
my $remote_ip = '10.0.0.4';
$domain->start(user => user_admin, remote_ip => $remote_ip);
_wait_ip($vm->type, $domain);
rvd_back->_process_requests_dont_fork(1);
my $internal_ip = _wait_ip($vm->type, $domain);
rvd_back->_process_requests_dont_fork();
_wait_requests($domain);
_check_port_rules($domain, $remote_ip);
is($domain->list_ports, 3);
for my $port ($domain->list_ports) {
my $restricted = ! $port->{restricted};
$domain->expose(id_port => $port->{id}, restricted => $restricted);
wait_request(background => 0, debug => 1);
_check_port_rules($domain, $remote_ip
,"Changed port $port->{internal_port} restricted=$restricted");
wait_request(background => 0, debug => 0);
my ($in, $out, $err);
run3(['/sbin/iptables','-L','FORWARD','-n'],\($in, $out, $err));
my @out = split /\n/,$out;
if ($restricted) {
my $port_re = qr{^ACCEPT.*$remote_ip\s+$internal_ip.*dpt:$port->{internal_port}};
is(grep(m{$port_re}
,@out),1)
or die "Expecting $port_re in $out";
is(grep(m{^DROP.*0.0.0.0.+$internal_ip.*dpt:$port->{internal_port}},@out),1)
or die $out;
} else {
is(grep(m{^ACCEPT.*0.0.0.0/0\s+$internal_ip.*dpt:$port->{internal_port}},@out),1)
or die $out;
}
}
$domain->remove(user_admin);
......@@ -817,7 +832,7 @@ sub _check_port_rules($domain, $remote_ip, $msg='') {
ok($n_rule_nat,"Expecting NAT rule ".Dumper($port)."\n$msg")
or confess;
if ($port->{restricted}) {
ok($n_rule);
ok($n_rule) or confess;
ok($n_rule_drop);
} else {
ok(!$n_rule);
......@@ -859,7 +874,7 @@ sub _search_rules($domain, $remote_ip, $internal_port, $public_port) {
sub _wait_requests($domain) {
_wait_ip($domain->_vm->type, $domain);
for (;;) {
rvd_back->_process_requests_dont_fork(1);
rvd_back->_process_requests_dont_fork();
last if !$domain->list_requests(1);
sleep 1;
}
......@@ -869,7 +884,7 @@ sub _wait_requests($domain) {
##############################################################
#clean();
clean();
init();
Test::Ravada::_clean_db();
......
templates/main/run_request.html.ep
View file @ 10a17e0a
......@@ -10,7 +10,7 @@
ng-controller="run_domain_req"
ng-init="id_request=<%= $request->id %>;auto_view=<%= $auto_view %>;timeout=<%= $timeout %>"
>
<div class="jumbotron">
<div class="jumbotron" ng-cloak="">
<h2><%=l 'Running' %> {{domain.name}}</h2>
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment