diff options
author | intrigeri <intrigeri@boum.org> | 2019-12-31 15:57:19 +0000 |
---|---|---|
committer | intrigeri <intrigeri@boum.org> | 2020-03-08 15:58:25 +0000 |
commit | 83cfbe152b49232ac5cf97c516c718a7e78dc208 (patch) | |
tree | 7decdaea9f25c0b3cb9539f6850d510b6e4cdb44 | |
parent | 85a1ecef1e3fb6e2dd2739a1a3cfdca4fe0ba987 (diff) |
WIP: Make it possible to resume an IUK download from within Tails (refs: #15875)feature/15875-upgrader-resume-iuk-download
5 files changed, 238 insertions, 52 deletions
diff --git a/config/chroot_local-includes/usr/src/iuk/dist.ini b/config/chroot_local-includes/usr/src/iuk/dist.ini index 0c86fcc..5fb2224 100644 --- a/config/chroot_local-includes/usr/src/iuk/dist.ini +++ b/config/chroot_local-includes/usr/src/iuk/dist.ini @@ -45,6 +45,7 @@ Sys::Filesystem = 1.28 Test::BDD::Cucumber = 0.16 Test::EOL = 0.9 Test::Fatal = 0.010 +Test::LWP::UserAgent = 0.031 Test::Most = 0.22 Test::Perl::Critic = 1.02 Test::Spec = 0.40 diff --git a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature index 0aeb793..9c4722c 100644 --- a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature +++ b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature @@ -29,6 +29,19 @@ Feature: download and verify a target file | whatever1.file | / | abc | 3 | ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad | | whatever2.file | /sub/dir | 123 | 3 | a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3 | + Scenario: Successfully resuming an interrupted download + Given a HTTP server that supports Range requests and serves "<file>" in "<webdir>" with content "<content>" and hash "<sha256>" + When I download "<file>" (of expected size <size>) from "<webdir>", and check its hash is "<sha256>" + Then it should succeed + And I should be told "Resuming download after 1 bytes" + And I should see the downloaded file in the temporary directory + And the SHA-256 of the downloaded file should be "<sha256>" + And the downloaded file should be world-readable + Examples: + | file | webdir | content | size | sha256 | + | whatever1.file | / | abc | 3 | ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad | + | whatever2.file | /sub/dir | 123 | 3 | a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3 | + Scenario: Successful download and verification with redirect to another hostname over HTTPS Given a HTTP server that redirects to 127.0.0.2 over HTTPS And a HTTPS server on 127.0.0.2 that serves "<file>" in "<webdir>" with content "<content>" and hash "<sha256>" diff --git a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl index 147c8f5..2312a26 100644 --- a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl +++ b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl @@ -12,6 +12,8 @@ use Data::Dumper; use English qw{-no_match_vars}; use Fcntl ':mode'; use Function::Parameters; +use IPC::System::Simple qw{systemx}; +use LWP::UserAgent; use Test::More; use Test::BDD::Cucumber::StepFile; @@ -22,6 +24,7 @@ use Test::WebServer::RedirectToHTTPS; use Test::WebServer::Static; use Test::WebServer::Static::SSL; use Types::Path::Tiny qw{AbsPath}; +use Types::Standard qw{Int}; my $bindir = path(__FILE__)->parent->parent->parent->parent->child('bin')->absolute; @@ -77,21 +80,77 @@ fun prepare_webroot (AbsPath $webroot, $filename, $present, $webdir, $spec_type, } } -Given qr{^a HTTP server that(| does not) serve[s]? "([^"]+)" in "([^"]+)"(?: with (content|size) "?([^"]*)"?)?}, fun ($c) { - my $present = $c->matches->[0] ? 0 : 1; - my $filename = $c->matches->[1]; - my $webdir = $c->matches->[2]; - my $spec_type = $c->matches->[3]; - my $spec_value = $c->matches->[4]; +fun generate_nginx_conf ( + AbsPath :$pid_file, + AbsPath :$conf_file, + AbsPath :$webroot, + Int :$port + ) { + $conf_file->spew( + <<EOTEMPLATE +pid $pid_file; +error_log stderr; +events { } +http { + server { + listen $port; + root $webroot; + access_log /dev/null; + autoindex on; + } +} +EOTEMPLATE + ); +} + +Given qr{^a HTTP server that( supports Range requests and)?(| does not) serve[s]? "([^"]+)" in "([^"]+)"(?: with (content|size) "?([^"]*)"?)?}, fun ($c) { + my $range_req = $c->matches->[0]; + my $present = $c->matches->[1] ? 0 : 1; + my $filename = $c->matches->[2]; + my $webdir = $c->matches->[3]; + my $spec_type = $c->matches->[4]; + my $spec_value = $c->matches->[5]; my $webroot = path($c->{stash}->{scenario}->{tempdir}, 'webroot'); prepare_webroot($webroot, $filename, $present, $webdir, $spec_type, $spec_value); - my $port = $c->{stash}->{scenario}->{server}->{port}; - my $s = Test::WebServer::Static->new({webroot => $webroot}, $port); - is($s->port(), $port, "Constructor set port correctly"); - my $pid = $c->{stash}->{scenario}->{server}->{http_pid} = $s->background(); - like($pid, '/^-?\d+$/', 'PID is numeric'); + + if ($range_req) { + $c->{stash}->{scenario}->{test_resume} = 1; + my $nginx_unit = 'test-tails-iuk-nginx.service'; + # Clean up leftovers from a previous test suite run that + # failed badly enough that the After hook did not trigger. + `systemctl --user stop '$nginx_unit' 2>&1`; + my $nginx_conf_file = Path::Tiny->tempfile; + my $nginx_pid_file = Path::Tiny->tempfile; + generate_nginx_conf( + conf_file => $nginx_conf_file, + pid_file => $nginx_pid_file, + webroot => $webroot, + port => $port, + ); + push @{$c->{stash}->{scenario}->{systemd_transient_units}}, $nginx_unit; + systemx( + 'systemd-run', + '--user', + "--unit=${nginx_unit}", + '--service-type=forking', + '--quiet', + # XXX: enable this once we require Buster or newer for running + # this test suite + # '--collect', + '/usr/sbin/nginx', '-c', $nginx_conf_file, + ); + for (1..16) { + last if LWP::UserAgent->new->get("http://127.0.0.1:$port/")->is_success; + sleep $_; + } + } else { + my $s = Test::WebServer::Static->new({webroot => $webroot}, $port); + is($s->port(), $port, "Constructor set port correctly"); + my $pid = $c->{stash}->{scenario}->{server}->{http_pid} = $s->background(); + like($pid, '/^-?\d+$/', 'PID is numeric'); + } }; Given qr{^a HTTP server that redirects to ([^ ]+) over HTTPS$}, fun ($c) { @@ -160,13 +219,10 @@ When qr{^I download "([^"]+)" \(of expected size (\d+)\) from "([^"]+)", and che my $webdir = $c->matches->[2]; my $expected_hash = $c->matches->[3]; - my $uri = sprintf("%s:%d/%s/%s", + my $uri = sprintf("http://%s:%d%s", "127.0.0.1", $c->{stash}->{scenario}->{server}->{port}, - $webdir, - $filename); - $uri =~ s{//}{/}gxms; - $uri = "http://$uri"; + path($webdir, $filename)); my $output_filename = $c->{stash}->{scenario}->{output_filename} @@ -180,6 +236,15 @@ When qr{^I download "([^"]+)" \(of expected size (\d+)\) from "([^"]+)", and che ' --output_file "' . $output_filename . '"' . ' --size "' . $expected_size . '"' ; + + if ($c->{stash}->{scenario}->{test_resume}) { + my $dl_file = path($c->{stash}->{scenario}->{tempdir}, 'webroot', $webdir, $filename); + if ($dl_file->is_file) { + my ($first_byte) = unpack('H*', $dl_file->openr_raw->getc); + $cmdline .= " --first_byte $first_byte"; + } + } + $c->{stash}->{scenario}->{output} = `umask 077 && $cmdline 2>&1`; $c->{stash}->{scenario}->{exit_code} = ${^CHILD_ERROR_NATIVE}; }; @@ -245,4 +310,9 @@ After fun ($c) { if (defined $c->{stash}->{scenario}->{server}->{http_pid}) { kill $c->{stash}->{scenario}->{server}->{http_pid}; } + for (@{$c->{stash}->{scenario}->{systemd_transient_units}}) { + systemx('systemctl', '--user', 'stop', $_); + } + # XXX: remove once we use --collect for all our transient units + systemx('systemctl', '--user', 'reset-failed'); }; diff --git a/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm b/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm index a51a60d..fef6e1d 100644 --- a/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm +++ b/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm @@ -17,7 +17,6 @@ use Carp; use Carp::Assert; use Cwd; use Digest::SHA; -use File::Temp qw{tempfile}; use Function::Parameters; use HTTP::Request; use Path::Tiny; @@ -25,7 +24,7 @@ use String::Errf qw{errf}; use Tails::IUK::LWP::UserAgent::WithProgress; use Tails::IUK::Utils qw{space_available_in}; use Types::Path::Tiny qw{AbsPath}; -use Types::Standard qw{Enum Int Str}; +use Types::Standard qw{Enum InstanceOf Int Object Str}; use namespace::clean; @@ -65,11 +64,68 @@ option 'size' => ( format => 's', ); +option 'max_retries' => ( + is => 'ro', + isa => Int, + format => 's', + default => sub { + my $self = shift; + if ($ENV{HARNESS_ACTIVE}) { + $self->has_first_byte + ? 1 # in this case we are testing resuming a download + : 0 # in other cases, let's things speed up and abort early + } else { + 6 + } + }, +); + +has 'ua' => ( + is => 'lazy', + isa => InstanceOf['LWP::UserAgent'], +); + +# Test suite only! +# In unpack('H*', $data) format +option 'first_byte' => ( + is => 'ro', + isa => Str, + format => 's', + predicate => 1, +); + =head1 METHODS =cut +method _build_ua () { + my $ua; + if ($ENV{HARNESS_ACTIVE} && $self->has_first_byte) { + # When running under the test suite and --first_byte is + # passed, simulate a HTTP server that only sends the first + # byte of the requested file, for the first download attempt: + # this allows exercising this class' ability to resume an + # interrupted download. + require Test::LWP::UserAgent; + Test::LWP::UserAgent->import; + $ua = Test::LWP::UserAgent->new; + $self->patch_response_handler($ua); + } else { + $ua = Tails::IUK::LWP::UserAgent::WithProgress->new(ssl_opts => { + verify_hostname => 0, + SSL_verify_mode => 0, + }); + } + unless ($ENV{HARNESS_ACTIVE} or $ENV{DISABLE_PROXY}) { + $ua->proxy([qw(http https)] => 'socks://127.0.0.1:9062'); + } + $ua->protocols_allowed([qw(http https)]); + $ua->max_size($self->size); + + return $ua; +} + method fatal (@msg) { Tails::IUK::Utils::fatal(msg => \@msg); } @@ -89,21 +145,33 @@ method check_available_space () { )); } +method patch_response_handler (Object $ua) { + $ua->map_response( + qr{}, + sub { + # Disable this mapped response, so it does not trigger + # for subsequent download attempts, that will instead + # go directly to the test web server + $ua->unmap_all; + $ua->network_fallback(1); + # Return the mocked response + HTTP::Response->new( + 200, 'OK', + HTTP::Headers->new(), + pack('H*', $self->first_byte) + ); + } + ); +} + method run () { $self->check_available_space; - my $ua = Tails::IUK::LWP::UserAgent::WithProgress->new(ssl_opts => { - verify_hostname => 0, - SSL_verify_mode => 0, - }); - unless ($ENV{HARNESS_ACTIVE} or $ENV{DISABLE_PROXY}) { - $ua->proxy([qw(http https)] => 'socks://127.0.0.1:9062'); - } - $ua->protocols_allowed([qw(http https)]); my $req = HTTP::Request->new('GET', $self->uri); - my ($temp_fh, $temp_filename) = tempfile; - close $temp_fh; + my $temp_file = Path::Tiny->tempfile; + my $temp_fh = $temp_file->opena_raw; + $temp_fh->autoflush(1); sub clean_fatal { my $self = shift; @@ -112,38 +180,71 @@ method run () { $self->fatal(@_); } - $ua->max_size($self->size); - my $res = $ua->request($req, $temp_filename); + my $retries = 0; + my $sleep = 1; + my $success; + while ($retries <= $self->max_retries) { + if ($retries) { + sleep $sleep; + $sleep *= 2; + my $range_start = -s $temp_fh; + say STDERR "Resuming download after $range_start bytes (retry no. $retries)"; + $req->header(Range => "bytes=${range_start}-"); + } - defined $res or clean_fatal($self, $temp_filename, sprintf( - "Could not download '%s' to '%s': undefined result", - $self->uri, $temp_filename, - )); + $retries++; + my $res = $self->ua->request($req, sub { print $temp_fh shift; }); - for my $lwp_failure_header (qw{Client-Aborted X-Died}) { - my $header = $res->header($lwp_failure_header); - ! defined $header or clean_fatal($self, $temp_filename, sprintf( - "Could not download '%s' to '%s' (%s): %s", - $self->uri, $temp_filename, $lwp_failure_header, $header, - )); - } + unless (defined $res) { + warn(sprintf( + "Could not download '%s' to '%s': undefined result", + $self->uri, $temp_file, + )); + next; + } - $res->is_success or clean_fatal($self, $temp_filename, sprintf( - "Could not download '%s' to '%s', request failed:\n%s\n", - $self->uri, $temp_filename, $res->status_line, - )); + for my $lwp_failure_header (qw{Client-Aborted X-Died}) { + my $header = $res->header($lwp_failure_header); + if (defined $header) { + warn(sprintf( + "Could not download '%s' to '%s' (%s): %s", + $self->uri, $temp_file, $lwp_failure_header, $header, + )); + next; + } + } + + unless ($res->is_success) { + warn(sprintf( + "Could not download '%s' to '%s', request failed:\n%s\n", + $self->uri, $temp_file, $res->status_line, + )); + next; + } + + if (-s $temp_file != $self->size) { + warn(sprintf( + "The file '%s' was downloaded but its size (%d) should be %d", + $self->uri, -s $temp_file, $self->size, + )); + next; + } + + $temp_fh->close; + $success = 1; + last; + } - -s $temp_filename == $self->size or clean_fatal( - $self, $temp_filename, sprintf( - "The file '%s' was downloaded but its size (%d) should be %d", - $self->uri, -s $temp_filename, $self->size, + $success or clean_fatal($self, $temp_file, sprintf( + "Could not download '%s' to '%s'", + $self->uri, $temp_file, )); my $sha = Digest::SHA->new(256); - $sha->addfile($temp_filename); + $sha->addfile($temp_file->stringify); my $actual_hash = $sha->hexdigest; $actual_hash eq $self->hash_value or clean_fatal( - $self, $temp_filename, sprintf( + $self, $temp_file, sprintf( "The file '%s' was downloaded but its hash is not correct:\n" . " - expected: %s\n" . " - actual: %s", @@ -152,7 +253,7 @@ method run () { $actual_hash, )); - rename($temp_filename, $self->output_file); + $temp_file->move($self->output_file); # autodie is supposed to throw an exception on rename error, # but one can't be too careful. assert(-e $self->output_file); diff --git a/wiki/src/contribute/release_process/tails-iuk.mdwn b/wiki/src/contribute/release_process/tails-iuk.mdwn index c27ddd9..a1c8479 100644 --- a/wiki/src/contribute/release_process/tails-iuk.mdwn +++ b/wiki/src/contribute/release_process/tails-iuk.mdwn @@ -37,6 +37,7 @@ Install dependencies needed to generate IUKs, UDFs, and to run the test suite libdata-dumper-concise-perl \ libdatetime-perl \ libfile-copy-recursive-perl \ + nginx \ rsync \ squashfs-tools \ syslinux |