Fix ports expose (#1432)

fix(ports): properly search unused ports in iptables

issue #1429

lib/Ravada.pm

@@ -3975,7 +3975,7 @@ sub _refresh_active_domain($self, $domain, $active_domain) {
 sub _refresh_hibernated($self, $domain) {
     return unless $domain->is_hibernated();
 
-    $domain->_post_hibernated() if !$domain->_data('post_hibernated');
+    $domain->_post_hibernate() if !$domain->_data('post_hibernated');
 }
 
 sub _refresh_down_domains($self, $active_domain, $active_vm) {

lib/Ravada/Domain.pm

@@ -303,6 +303,8 @@ sub _vm_disconnect {
 
 sub _around_start($orig, $self, @arg) {
 
+    $self->_post_hibernate() if $self->is_hibernated && !$self->_data('post_hibernated');
+
     $self->_data( 'post_shutdown' => 0);
     $self->_data( 'post_hibernated' => 0);
     $self->_start_preconditions(@arg);
@@ -2793,6 +2795,13 @@ sub _set_public_port($self, $id_port, $internal_port, $name, $restricted) {
     }
 }
 
+sub _used_ports_iptables($self, $port) {
+    my $used_port = {};
+    $self->_vm->_list_used_ports_iptables($used_port);
+    return 0 if !$used_port->{$port};
+    return 1;
+}
+
 sub _open_exposed_port($self, $internal_port, $name, $restricted) {
     my $sth = $$CONNECTOR->dbh->prepare("SELECT id,public_port FROM domain_ports"
         ." WHERE id_domain=? AND internal_port=?"
@@ -2800,6 +2809,8 @@ sub _open_exposed_port($self, $internal_port, $name, $restricted) {
     $sth->execute($self->id, $internal_port);
     my ($id_port, $public_port) = $sth->fetchrow();
 
+    $public_port = undef if $public_port && $self->_used_ports_iptables($public_port);
+
     $public_port = $self->_set_public_port($id_port, $internal_port, $name, $restricted)
     if !$public_port;
 

lib/Ravada/VM.pm

@@ -1905,8 +1905,8 @@ sub _list_used_ports_iptables($self, $used_port) {
     my $iptables = $self->iptables_list();
     for my $rule ( @{$iptables->{nat}} ) {
         my %rule = @{$rule};
-        next if !exists $rule{A} || $rule{A} ne 'PREROUTING';
-        $used_port->{dport}++;
+        next if !exists $rule{A} || $rule{A} ne 'PREROUTING' || !$rule{dport};
+        $used_port->{$rule{dport}}++;
     }
 }
 

t/vm/92_ports.t

@@ -511,6 +511,113 @@ sub test_clone_exports($vm) {
     $base->remove(user_admin);
 }
 
+sub test_routing_hibernated($vm) {
+    my $base = create_domain($vm, user_admin,'debian stretch');
+    $base->expose(port => 22, name => "ssh");
+
+    my @base_ports0 = $base->list_ports();
+    my $public_port0 = $base_ports0[0]->{public_port};
+
+    my $remote_ip = '4.4.4.4';
+    $base->start(remote_ip => $remote_ip,  user => user_admin);
+
+    _wait_ip($vm, $base);
+    wait_request( debug => 0 );
+
+    my @base_ports1 = $base->list_ports();
+
+    my $public_port1 = $base_ports1[0]->{public_port};
+
+    is($public_port1, $public_port0) or exit;
+
+    hibernate_domain_internal($base);
+
+    $base->start(remote_ip => $remote_ip,  user => user_admin);
+
+    _wait_ip($vm, $base);
+    wait_request( debug => 0 );
+
+    my @base_ports2 = $base->list_ports();
+
+    my $public_port2 = $base_ports2[0]->{public_port};
+
+    is($public_port2, $public_port0) or exit;
+    is($public_port2, $public_port1) or exit;
+
+    $base->remove(user_admin);
+}
+
+sub test_routing_already_used($vm) {
+    my $base = create_domain($vm, user_admin,'debian stretch');
+    $base->expose(port => 22, name => "ssh");
+    my @base_ports0 = $base->list_ports();
+
+    my $public_port0 = $base_ports0[0]->{public_port};
+
+    $vm->iptables_unique(
+            t => 'nat'
+            ,A => 'PREROUTING'
+            ,p => 'tcp'
+            ,dport => $public_port0
+            ,j => 'DNAT'
+            ,'to-destination' => "1.2.3.4:1111"
+    );
+
+    my @iptables0 = _iptables_save($vm,'nat','PREROUTING');
+    my $remote_ip = '3.3.3.3';
+    $base->start(remote_ip => $remote_ip,  user => user_admin);
+
+    _wait_ip($vm, $base);
+    wait_request( debug => 0 );
+
+    my @base_ports1 = $base->list_ports();
+
+    my $public_port1 = $base_ports1[0]->{public_port};
+
+    isnt($public_port1, $public_port0) or exit;
+    my @iptables1 = _iptables_save($vm,'nat' ,'PREROUTING');
+    is(scalar(@iptables1),scalar(@iptables0)+1,"Expecting 1 chain more "
+    .Dumper(\@iptables0,\@iptables1)) or exit;
+
+    # open again the ports, nothing should change
+    for ( 1 .. 3 ) {
+        my $req = Ravada::Request->open_iptables(
+            uid => user_admin->id
+            ,id_domain => $base->id
+            ,remote_ip => $remote_ip
+        );
+        wait_request(debug => 0);
+        is($req->status,'done');
+        is($req->error, '');
+
+        my @base_ports2 = $base->list_ports();
+
+        my $public_port2 = $base_ports2[0]->{public_port};
+
+        isnt($public_port2, $public_port0) or exit;
+        is($public_port2, $public_port1) or exit;
+
+        my @iptables2 = _iptables_save($vm,'nat' ,'PREROUTING');
+        is(scalar(@iptables1),scalar(@iptables2)) or die Dumper(\@iptables1,\@iptables2);
+    }
+
+    $base->remove(user_admin);
+}
+
+sub _iptables_save($vm,$table=undef,$chain=undef) {
+    my @cmd = ("iptables-save");
+    push @cmd,("-t",$table) if $table;
+
+    my ($out,$err) = $vm->run_command(@cmd);
+
+    my @out;
+    for my $line (split /\n/,$out) {
+        next if $chain && $line !~ /^-A $chain/;
+        push @out,($line);
+    }
+    return @out;
+}
+
 sub test_clone_exports_add_ports($vm) {
 
     my $base = create_domain($vm, user_admin,'debian stretch');
@@ -966,6 +1073,9 @@ for my $vm_name ( 'KVM', 'Void' ) {
     skip $msg,10    if !$vm;
 
     diag("Testing $vm_name");
+    test_routing_hibernated($vm);
+    test_routing_already_used($vm);
+
     test_clone_exports_add_ports($vm);
 
     test_no_dupe($vm);