Add IO::Socket::SSL, i.e.
Harden protocol handshake to handle race conditions.
diff --git a/lib/pgBackRest/Common/Http/Client.pm b/lib/pgBackRest/Common/Http/Client.pm index e6e8eda8c..75054bb83 100644 --- a/lib/pgBackRest/Common/Http/Client.pm +++ b/lib/pgBackRest/Common/Http/Client.pm @@ -56,6 +56,8 @@ sub new $iProtocolTimeout, $lBufferMax, $bVerifySsl, + $strCaPath, + $strCaFile, ) = logDebugParam ( @@ -69,15 +71,31 @@ sub new {name => 'iProtocolTimeout', optional => true, default => 30, trace => true}, {name => 'lBufferMax', optional => true, default => 32768, trace => true}, {name => 'bVerifySsl', optional => true, default => true, trace => true}, + {name => 'strCaPath', optional => true, trace => true}, + {name => 'strCaFile', optional => true, trace => true}, ); # Connect to the server - my $oSocket = IO::Socket::SSL->new( - PeerHost => $strHost, PeerPort => 'https', SSL_verify_mode => $bVerifySsl ? SSL_VERIFY_PEER : SSL_VERIFY_NONE); + my $oSocket; + eval + { + $oSocket = IO::Socket::SSL->new( + PeerHost => $strHost, PeerPort => 'https', SSL_verify_mode => $bVerifySsl ? SSL_VERIFY_PEER : SSL_VERIFY_NONE, + SSL_ca_path => $strCaPath, SSL_ca_file => $strCaFile); + + return 1; + } + or do + { + logErrorResult(ERROR_HOST_CONNECT, $EVAL_ERROR); + }; + + # Check for errors if (!defined($oSocket)) { - logErrorResult(ERROR_PROTOCOL, "unable to connect $!", $SSL_ERROR); + logErrorResult( + ERROR_HOST_CONNECT, coalesce(length($!) == 0 ? undef : $!, $SSL_ERROR), length($!) > 0 ? $SSL_ERROR : undef); } # Create the buffered IO object diff --git a/lib/pgBackRest/Config/Config.pm b/lib/pgBackRest/Config/Config.pm index df78d5599..8c30e7358 100644 --- a/lib/pgBackRest/Config/Config.pm +++ b/lib/pgBackRest/Config/Config.pm @@ -307,6 +307,10 @@ use constant OPTION_REPO_S3_KEY_SECRET => 'repo-s3- push @EXPORT, qw(OPTION_REPO_S3_KEY_SECRET); use constant OPTION_REPO_S3_BUCKET => 'repo-s3-bucket'; push @EXPORT, qw(OPTION_REPO_S3_BUCKET); +use constant OPTION_REPO_S3_CA_FILE => 'repo-s3-ca-file'; + push @EXPORT, qw(OPTION_REPO_S3_CA_FILE); +use constant OPTION_REPO_S3_CA_PATH => 'repo-s3-ca-path'; + push @EXPORT, qw(OPTION_REPO_S3_CA_PATH); use constant OPTION_REPO_S3_ENDPOINT => 'repo-s3-endpoint'; push @EXPORT, qw(OPTION_REPO_S3_ENDPOINT); use constant OPTION_REPO_S3_HOST => 'repo-s3-host'; @@ -1302,6 +1306,9 @@ my %oOptionRule = &OPTION_RULE_COMMAND => OPTION_REPO_TYPE, }, + &OPTION_REPO_S3_CA_FILE => &OPTION_REPO_S3_HOST, + &OPTION_REPO_S3_CA_PATH => &OPTION_REPO_S3_HOST, + &OPTION_REPO_S3_KEY => { &OPTION_RULE_SECTION => CONFIG_SECTION_GLOBAL, diff --git a/lib/pgBackRest/Config/ConfigHelpData.pm b/lib/pgBackRest/Config/ConfigHelpData.pm index 294f1dd38..2782d9589 100644 --- a/lib/pgBackRest/Config/ConfigHelpData.pm +++ b/lib/pgBackRest/Config/ConfigHelpData.pm @@ -622,6 +622,28 @@ my $oConfigHelpData = "specify a prefix, such as /repo, so logs and other AWS generated content can also be stored in the bucket." }, + # REPO-S3-CA-FILE Option Help + #--------------------------------------------------------------------------------------------------------------------------- + 'repo-s3-ca-file' => + { + section => 'general', + summary => + "S3 SSL CA File.", + description => + "Use a CA file other than the system default." + }, + + # REPO-S3-CA-PATH Option Help + #--------------------------------------------------------------------------------------------------------------------------- + 'repo-s3-ca-path' => + { + section => 'general', + summary => + "S3 SSL CA Path.", + description => + "Use a CA path other than the system default." + }, + # REPO-S3-ENDPOINT Option Help #--------------------------------------------------------------------------------------------------------------------------- 'repo-s3-endpoint' => @@ -916,6 +938,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -965,6 +989,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1043,6 +1069,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1127,6 +1155,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1165,6 +1195,8 @@ my $oConfigHelpData = 'log-timestamp' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1242,6 +1274,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1313,6 +1347,8 @@ my $oConfigHelpData = 'recovery-option' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1462,6 +1498,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1512,6 +1550,8 @@ my $oConfigHelpData = 'protocol-timeout' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1553,6 +1593,8 @@ my $oConfigHelpData = 'log-timestamp' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', @@ -1610,6 +1652,8 @@ my $oConfigHelpData = 'log-timestamp' => 'section', 'repo-path' => 'section', 'repo-s3-bucket' => 'section', + 'repo-s3-ca-file' => 'section', + 'repo-s3-ca-path' => 'section', 'repo-s3-endpoint' => 'section', 'repo-s3-host' => 'section', 'repo-s3-key' => 'section', diff --git a/lib/pgBackRest/Protocol/Storage/Helper.pm b/lib/pgBackRest/Protocol/Storage/Helper.pm index ccfc088b8..2a7d1e2fe 100644 --- a/lib/pgBackRest/Protocol/Storage/Helper.pm +++ b/lib/pgBackRest/Protocol/Storage/Helper.pm @@ -191,8 +191,9 @@ sub storageRepo $oDriver = new pgBackRest::Storage::S3::Driver( optionGet(OPTION_REPO_S3_BUCKET), optionGet(OPTION_REPO_S3_ENDPOINT), optionGet(OPTION_REPO_S3_REGION), optionGet(OPTION_REPO_S3_KEY), optionGet(OPTION_REPO_S3_KEY_SECRET), - {strHost => optionGet(OPTION_REPO_S3_HOST, false), bVerifySsl => optionGet(OPTION_REPO_S3_VERIFY_SSL, false)}, - lBufferMax => optionGet(OPTION_BUFFER_SIZE)); + {strHost => optionGet(OPTION_REPO_S3_HOST, false), bVerifySsl => optionGet(OPTION_REPO_S3_VERIFY_SSL, false), + strCaPath => optionGet(OPTION_REPO_S3_CA_PATH, false), + strCaFile => optionGet(OPTION_REPO_S3_CA_FILE, false), lBufferMax => optionGet(OPTION_BUFFER_SIZE)}); } elsif (optionTest(OPTION_REPO_TYPE, REPO_TYPE_CIFS)) { diff --git a/lib/pgBackRest/Storage/S3/Request.pm b/lib/pgBackRest/Storage/S3/Request.pm index dee954f11..7a6c77ec7 100644 --- a/lib/pgBackRest/Storage/S3/Request.pm +++ b/lib/pgBackRest/Storage/S3/Request.pm @@ -67,6 +67,8 @@ sub new $self->{strSecretAccessKey}, $self->{strHost}, $self->{bVerifySsl}, + $self->{strCaPath}, + $self->{strCaFile}, $self->{lBufferMax}, ) = logDebugParam @@ -79,6 +81,8 @@ sub new {name => 'strSecretAccessKey', trace => true}, {name => 'strHost', optional => true, trace => true}, {name => 'bVerifySsl', optional => true, default => true, trace => true}, + {name => 'strCaPath', optional => true, trace => true}, + {name => 'strCaFile', optional => true, trace => true}, {name => 'lBufferMax', optional => true, default => COMMON_IO_BUFFER_MAX, trace => true}, ); @@ -140,7 +144,8 @@ sub request my $oHttpClient = new pgBackRest::Common::Http::Client( $self->{strHost}, $strVerb, {strUri => $strUri, hQuery => $hQuery, hRequestHeader => $hHeader, rstrRequestBody => $rstrBody, - bVerifySsl => $self->{bVerifySsl}, lBufferMax => $self->{lBufferMax}}); + bVerifySsl => $self->{bVerifySsl}, strCaPath => $self->{strCaPath}, strCaFile => $self->{strCaFile}, + lBufferMax => $self->{lBufferMax}}); # Check response code my $iReponseCode = $oHttpClient->responseCode(); diff --git a/test/expect/help-help-001.log b/test/expect/help-help-001.log index b289ada53..f169fec1e 100644 --- a/test/expect/help-help-001.log +++ b/test/expect/help-help-001.log @@ -79,6 +79,8 @@ compress=n [default=3] --repo-path repository path where WAL segments and backups stored [default=/var/lib/pgbackrest] --repo-s3-bucket s3 repository bucket + --repo-s3-ca-file s3 SSL CA File + --repo-s3-ca-path s3 SSL CA Path --repo-s3-endpoint s3 repository endpoint --repo-s3-host s3 repository host --repo-s3-key s3 repository access key diff --git a/test/lib/pgBackRestTest/Common/DefineTest.pm b/test/lib/pgBackRestTest/Common/DefineTest.pm index 92f602231..79f2a3e2b 100644 --- a/test/lib/pgBackRestTest/Common/DefineTest.pm +++ b/test/lib/pgBackRestTest/Common/DefineTest.pm @@ -206,6 +206,10 @@ my $oTestDef = 'Storage/S3/Auth' => TESTDEF_COVERAGE_FULL, }, }, + { + &TESTDEF_NAME => 's3-cert', + &TESTDEF_TOTAL => 1, + }, { &TESTDEF_NAME => 's3', &TESTDEF_TOTAL => 7, diff --git a/test/lib/pgBackRestTest/Env/S3EnvTest.pm b/test/lib/pgBackRestTest/Env/S3EnvTest.pm index 430d74b94..a2d893fb4 100644 --- a/test/lib/pgBackRestTest/Env/S3EnvTest.pm +++ b/test/lib/pgBackRestTest/Env/S3EnvTest.pm @@ -18,6 +18,7 @@ use pgBackRest::Storage::S3::Driver; use pgBackRestTest::Common::ExecuteTest; use pgBackRestTest::Common::RunTest; +use pgBackRestTest::Common::VmTest; #################################################################################################################################### # initS3 @@ -38,6 +39,9 @@ sub initS3 $self->{strS3Command} = 'export PYTHONWARNINGS="ignore" && aws s3 --no-verify-ssl'; + # Make sure the cert is visible + executeTest('sudo chmod o+r,o+x /root /root/scalitys3 && sudo chmod o+r /root/scalitys3/ca.crt'); + executeTest("echo '127.0.0.1 ${strBucket}.${strEndPoint} ${strEndPoint}' | sudo tee -a /etc/hosts"); executeTest('sudo sed -i "s/logLevel\"\: \"info\"/logLevel\"\: \"trace\"/" /root/scalitys3/config.json'); executeTest("sudo npm start --prefix /root/scalitys3 > ${strS3ServerLogFile} 2>&1 &"); @@ -50,7 +54,9 @@ sub initS3 # Initialize the driver return new pgBackRest::Storage::S3::Driver( - $strBucket, $strEndPoint, $strRegion, $strAccessKeyId, $strSecretAccessKey, {bVerifySsl => false, lBufferMax => 1048576}); + $strBucket, $strEndPoint, $strRegion, $strAccessKeyId, $strSecretAccessKey, + {strCaFile => $self->vm() eq VM_CO7 ? '/root/scalitys3/ca.crt' : undef, + bVerifySsl => $self->vm() eq VM_U16 ? false : undef, lBufferMax => 1048576}); } 1; diff --git a/test/lib/pgBackRestTest/Module/Storage/StorageS3CertTest.pm b/test/lib/pgBackRestTest/Module/Storage/StorageS3CertTest.pm new file mode 100644 index 000000000..e854af4e6 --- /dev/null +++ b/test/lib/pgBackRestTest/Module/Storage/StorageS3CertTest.pm @@ -0,0 +1,103 @@ +#################################################################################################################################### +# S3 SSL Certificate Tests +# +# Verify that SSL certificate validation works on live S3 servers. +#################################################################################################################################### +package pgBackRestTest::Module::Storage::StorageS3CertTest; +use parent 'pgBackRestTest::Env::ConfigEnvTest'; + +#################################################################################################################################### +# Perl includes +#################################################################################################################################### +use strict; +use warnings FATAL => qw(all); +use Carp qw(confess); +use English '-no_match_vars'; + +use Storable qw(dclone); + +use pgBackRest::Common::Exception; +use pgBackRest::Common::Log; +use pgBackRest::Common::Wait; +use pgBackRest::Config::Config; +use pgBackRest::Protocol::Storage::Helper; + +use pgBackRestTest::Common::RunTest; +use pgBackRestTest::Common::VmTest; + +#################################################################################################################################### +# run +#################################################################################################################################### +sub run +{ + my $self = shift; + + # Use long random string so bucket lookups will fail and expose access errors + my $strBucket = 'bnBfyKpXR8ZqQY5RXszxemRgvtmjXd4tf5HkFYhTpT9BndUCYMDy5NCCyRz'; + my $strEndpoint = 's3-us-west-2.amazonaws.com'; + my $strRegion = 'us-west-2'; + + # Options + my $oOptionGlobal = {}; + $self->optionSetTest($oOptionGlobal, OPTION_REPO_TYPE, REPO_TYPE_S3); + $self->optionSetTest($oOptionGlobal, OPTION_REPO_S3_KEY, BOGUS); + $self->optionSetTest($oOptionGlobal, OPTION_REPO_S3_KEY_SECRET, BOGUS); + $self->optionSetTest($oOptionGlobal, OPTION_REPO_S3_BUCKET, $strBucket); + $self->optionSetTest($oOptionGlobal, OPTION_REPO_S3_ENDPOINT, $strEndpoint); + $self->optionSetTest($oOptionGlobal, OPTION_REPO_S3_REGION, $strRegion); + + $self->optionSetTest($oOptionGlobal, OPTION_STANZA, $self->stanza()); + + ################################################################################################################################ + if ($self->begin('validation')) + { + #--------------------------------------------------------------------------------------------------------------------------- + if ($self->vm() eq VM_CO7) + { + # Tests fails on co7 because by default certs cannot be located. This logic may need to be changed in the future if + # this bug gets fixed by Red Hat. + $self->testResult(sub {$self->configLoadExpect(dclone($oOptionGlobal), CMD_ARCHIVE_PUSH)}, '', 'config load'); + + $self->testException( + sub {storageRepo({strStanza => 'test1'})->list('/')}, ERROR_HOST_CONNECT, + 'IO::Socket::IP configuration failed SSL connect attempt failed.*certificate verify failed', + 'cert verify fails on ' . VM_CO7); + + # It should work when verification is disabled + my $oOptionLocal = dclone($oOptionGlobal); + $self->optionBoolSetTest($oOptionLocal, OPTION_REPO_S3_VERIFY_SSL, false); + $self->testResult(sub {$self->configLoadExpect($oOptionLocal, CMD_ARCHIVE_PUSH)}, '', 'config load'); + + $self->testException( + sub {storageRepo({strStanza => 'test2'})->list('/')}, ERROR_PROTOCOL, 'S3 request error \[403\] Forbidden.*', + 'connection succeeds with verification disabled, (expected) error on invalid access key'); + } + + #--------------------------------------------------------------------------------------------------------------------------- + my $oOptionLocal = dclone($oOptionGlobal); + + # CO7 doesn't locate certs automatically so specify the path + if ($self->vm() eq VM_CO7) + { + $self->optionSetTest($oOptionLocal, OPTION_REPO_S3_CA_FILE, '/etc/pki/tls/certs/ca-bundle.crt'); + } + + $self->testResult(sub {$self->configLoadExpect($oOptionLocal, CMD_ARCHIVE_PUSH)}, '', 'config load'); + + $self->testException( + sub {storageRepo({strStanza => 'test3'})->list('/')}, ERROR_PROTOCOL, 'S3 request error \[403\] Forbidden.*', + 'connection succeeds, (expected) error on invalid access key'); + + #--------------------------------------------------------------------------------------------------------------------------- + $oOptionLocal = dclone($oOptionGlobal); + $self->optionSetTest($oOptionLocal, OPTION_REPO_S3_CA_PATH, '/bogus'); + $self->testResult(sub {$self->configLoadExpect($oOptionLocal, CMD_ARCHIVE_PUSH)}, '', 'config load'); + + $self->testException( + sub {storageRepo({strStanza => 'test4'})->list('/')}, ERROR_HOST_CONNECT, + $self->vm() eq VM_CO6 ? 'IO::Socket::INET configuration failed' : 'SSL_ca_path /bogus does not exist', + 'invalid ca path'); + } +} + +1;