diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 9006d56a3..9785589fb 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -163,6 +163,16 @@ + + + + +

Fixed authentication issue in S3 retry.

+ + + + + diff --git a/lib/pgBackRest/Storage/S3/Auth.pm b/lib/pgBackRest/Storage/S3/Auth.pm index ef7bb69be..bc78b7e47 100644 --- a/lib/pgBackRest/Storage/S3/Auth.pm +++ b/lib/pgBackRest/Storage/S3/Auth.pm @@ -237,6 +237,9 @@ sub s3AuthorizationHeader {name => 'strPayloadHash', trace => true}, ); + # Delete the authorization header if it already exists. This could happen on a retry. + delete($hHeader->{&S3_HEADER_AUTHORIZATION}); + # Add s3 required headers $hHeader->{&S3_HEADER_HOST} = $strHost; $hHeader->{&S3_HEADER_CONTENT_SHA256} = $strPayloadHash; @@ -244,18 +247,21 @@ sub s3AuthorizationHeader # Create authorization string my ($strCanonicalRequest, $strSignedHeaders) = s3CanonicalRequest($strVerb, $strUri, $strQuery, $hHeader, $strPayloadHash); + my $strStringToSign = s3StringToSign($strDateTime, $strRegion, sha256_hex($strCanonicalRequest)); $hHeader->{&S3_HEADER_AUTHORIZATION} = AWS4_HMAC_SHA256 . " Credential=${strAccessKeyId}/" . substr($strDateTime, 0, 8) . "/${strRegion}/" . S3 . qw(/) . - AWS4_REQUEST . ",SignedHeaders=${strSignedHeaders},Signature=" . hmac_sha256_hex(s3StringToSign( - $strDateTime, $strRegion, sha256_hex($strCanonicalRequest)), + AWS4_REQUEST . ",SignedHeaders=${strSignedHeaders},Signature=" . hmac_sha256_hex($strStringToSign, s3SigningKey(substr($strDateTime, 0, 8), $strRegion, $strSecretAccessKey)); # Return from function and log return values if any return logDebugReturn ( $strOperation, - {name => 'hHeader', value => $hHeader, trace => true} + {name => 'hHeader', value => $hHeader, trace => true}, + {name => 'strCanonicalRequest', value => $strCanonicalRequest, trace => true}, + {name => 'strSignedHeaders', value => $strSignedHeaders, trace => true}, + {name => 'strStringToSign', value => $strStringToSign, trace => true}, ); } diff --git a/lib/pgBackRest/Storage/S3/Request.pm b/lib/pgBackRest/Storage/S3/Request.pm index 3b46f5128..a8cab9df3 100644 --- a/lib/pgBackRest/Storage/S3/Request.pm +++ b/lib/pgBackRest/Storage/S3/Request.pm @@ -47,6 +47,7 @@ use constant S3_RESPONSE_TYPE_XML => 'xml'; push @EXPORT, qw(S3_RESPONSE_TYPE_XML); use constant S3_RESPONSE_CODE_SUCCESS => 200; +use constant S3_RESPONSE_CODE_ERROR_AUTH => 403; use constant S3_RESPONSE_CODE_ERROR_NOT_FOUND => 404; use constant S3_RESPONSE_CODE_ERROR_INTERNAL => 500; @@ -140,7 +141,7 @@ sub request my $oResponse; # Allow retries on S3 internal failures - my $bRetry = false; + my $bRetry; my $iRetryTotal = 0; do @@ -148,16 +149,13 @@ sub request # Assume that a retry will not be attempted which is true in most cases $bRetry = false; - # Get datetime to be used for auth requests - my $strDateTime = s3DateTime(); - # Set content length and hash $hHeader->{&S3_HEADER_CONTENT_SHA256} = defined($rstrBody) ? sha256_hex($$rstrBody) : PAYLOAD_DEFAULT_HASH; $hHeader->{&S3_HEADER_CONTENT_LENGTH} = defined($rstrBody) ? length($$rstrBody) : 0; # Generate authorization header - $hHeader = s3AuthorizationHeader( - $self->{strRegion}, "$self->{strBucket}.$self->{strEndPoint}", $strVerb, $strUri, httpQuery($hQuery), $strDateTime, + ($hHeader, my $strCanonicalRequest, my $strSignedHeaders, my $strStringToSign) = s3AuthorizationHeader( + $self->{strRegion}, "$self->{strBucket}.$self->{strEndPoint}", $strVerb, $strUri, httpQuery($hQuery), s3DateTime(), $hHeader, $self->{strAccessKeyId}, $self->{strSecretAccessKey}, $hHeader->{&S3_HEADER_CONTENT_SHA256}); # Send the request @@ -168,9 +166,9 @@ sub request strCaFile => $self->{strCaFile}, lBufferMax => $self->{lBufferMax}}); # Check response code - my $iReponseCode = $oHttpClient->responseCode(); + my $iResponseCode = $oHttpClient->responseCode(); - if ($iReponseCode == S3_RESPONSE_CODE_SUCCESS) + if ($iResponseCode == S3_RESPONSE_CODE_SUCCESS) { # Save the response headers locally $self->{hResponseHeader} = $oHttpClient->responseHeader(); @@ -198,7 +196,7 @@ sub request else { # If file was not found - if ($iReponseCode == S3_RESPONSE_CODE_ERROR_NOT_FOUND) + if ($iResponseCode == S3_RESPONSE_CODE_ERROR_NOT_FOUND) { # If missing files should not be ignored then error if (!$bIgnoreMissing) @@ -208,14 +206,21 @@ sub request $bRetry = false; } - # Else a more serrious error + # Else a more serious error else { # Retry for S3 internal errors - if ($iReponseCode == S3_RESPONSE_CODE_ERROR_INTERNAL) + if ($iResponseCode == S3_RESPONSE_CODE_ERROR_INTERNAL) { + # Increment retry total and check if retry should be attempted $iRetryTotal++; $bRetry = $iRetryTotal <= S3_RETRY_MAX; + + # Sleep after first retry just in case data needs to stabilize + if ($iRetryTotal > 1) + { + sleep(5); + } } # If no retry then throw the error @@ -224,9 +229,14 @@ sub request my $rstrResponseBody = $oHttpClient->responseBody(); confess &log(ERROR, - "S3 request error [$iReponseCode] " . $oHttpClient->responseMessage() . + 'S3 request error' . ($iRetryTotal > 0 ? " after retries" : '') . + " [$iResponseCode] " . $oHttpClient->responseMessage() . "\n*** request header ***\n" . $oHttpClient->requestHeaderText() . - "\n*** reponse header ***\n" . $oHttpClient->responseHeaderText() . + ($iResponseCode == S3_RESPONSE_CODE_ERROR_AUTH ? + "\n*** canonical request ***\n" . $strCanonicalRequest . + "\n*** signed headers ***\n" . $strSignedHeaders . + "\n*** string to sign ***\n" . $strStringToSign : '') . + "\n*** response header ***\n" . $oHttpClient->responseHeaderText() . (defined($$rstrResponseBody) ? "\n*** response body ***\n${$rstrResponseBody}" : ''), ERROR_PROTOCOL); } diff --git a/lib/pgBackRest/Version.pm b/lib/pgBackRest/Version.pm index 6644af8d8..2169beb5e 100644 --- a/lib/pgBackRest/Version.pm +++ b/lib/pgBackRest/Version.pm @@ -35,7 +35,7 @@ use constant BACKREST_BIN => abs_path( # Defines the current version of the BackRest executable. The version number is used to track features but does not affect what # repositories or manifests can be read - that's the job of the format number. #----------------------------------------------------------------------------------------------------------------------------------- -use constant BACKREST_VERSION => '1.21'; +use constant BACKREST_VERSION => '1.22dev'; push @EXPORT, qw(BACKREST_VERSION); # Format Format Number diff --git a/libc/lib/pgBackRest/LibC.pm b/libc/lib/pgBackRest/LibC.pm index c5058e819..7b7c77a4c 100644 --- a/libc/lib/pgBackRest/LibC.pm +++ b/libc/lib/pgBackRest/LibC.pm @@ -11,7 +11,7 @@ use AutoLoader; our @ISA = qw(Exporter); # Library version (add .999 during development) -our $VERSION = '1.21'; +our $VERSION = '1.22.999'; sub libCVersion {return $VERSION}; diff --git a/test/lib/pgBackRestTest/Module/Storage/StorageS3AuthTest.pm b/test/lib/pgBackRestTest/Module/Storage/StorageS3AuthTest.pm index 0e0e07d42..eb3810f53 100644 --- a/test/lib/pgBackRestTest/Module/Storage/StorageS3AuthTest.pm +++ b/test/lib/pgBackRestTest/Module/Storage/StorageS3AuthTest.pm @@ -19,6 +19,8 @@ use pgBackRest::Common::Log; use pgBackRest::Common::Wait; use pgBackRest::Storage::S3::Auth; +use pgBackRestTest::Common::RunTest; + #################################################################################################################################### # run #################################################################################################################################### @@ -95,15 +97,29 @@ sub run $self->testResult( sub {s3AuthorizationHeader( 'us-east-1', 'bucket.s3.amazonaws.com', 'GET', qw(/), 'list-type=2', '20170606T121212Z', - {'host' => 'bucket.s3.amazonaws.com', 'x-amz-date' => '20170606T121212Z'}, + {'authorization' => BOGUS, 'host' => 'bucket.s3.amazonaws.com', 'x-amz-date' => '20170606T121212Z'}, 'AKIAIOSFODNN7EXAMPLE', 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855')}, - '{authorization => AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20170606/us-east-1/s3/aws4_request,' . + '({authorization => AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20170606/us-east-1/s3/aws4_request,' . 'SignedHeaders=host;x-amz-content-sha256;x-amz-date,' . 'Signature=cb03bf1d575c1f8904dabf0e573990375340ab293ef7ad18d049fc1338fd89b3,' . ' host => bucket.s3.amazonaws.com,' . ' x-amz-content-sha256 => e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855,' . - ' x-amz-date => 20170606T121212Z}', + ' x-amz-date => 20170606T121212Z}, ' . + "GET\n" . + "/\n" . + "list-type=2\n" . + "host:bucket.s3.amazonaws.com\n" . + "x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\n" . + "x-amz-date:20170606T121212Z\n" . + "\n" . + "host;x-amz-content-sha256;x-amz-date\n" . + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, " . + "host;x-amz-content-sha256;x-amz-date, " . + "AWS4-HMAC-SHA256\n" . + "20170606T121212Z\n" . + "20170606/us-east-1/s3/aws4_request\n" . + "4f2d4ee971f579e60ba6b3895e87434e17b1260f04392f02b512c1e8bada72dd)", 'authorization header request'); } } diff --git a/test/lib/pgBackRestTest/Module/Storage/StorageS3RequestTest.pm b/test/lib/pgBackRestTest/Module/Storage/StorageS3RequestTest.pm index 1a6042748..7c69778b8 100644 --- a/test/lib/pgBackRestTest/Module/Storage/StorageS3RequestTest.pm +++ b/test/lib/pgBackRestTest/Module/Storage/StorageS3RequestTest.pm @@ -171,7 +171,7 @@ sub run #--------------------------------------------------------------------------------------------------------------------------- $self->testResult(sub {$oS3Request->request(HTTP_VERB_GET)}, undef, 'successful request after retries'); $self->testException( - sub {$oS3Request->request(HTTP_VERB_GET)}, ERROR_PROTOCOL, 'S3 request error \[500\] GenericMessage.*'); + sub {$oS3Request->request(HTTP_VERB_GET)}, ERROR_PROTOCOL, 'S3 request error after retries \[500\] GenericMessage.*'); } }