db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-14 10:13:05 +02:00
Code Issues Releases Activity

Improve error message when an S3 bucket name contains dots.

Browse Source 
The Perl lib we have been using for TLS allows dots in wildcards, but this is forbidden by RFC-2818.  The new TLS implementation in C forbids this pattern, just as PostgreSQL and curl do.

However, this does present a problem for users who have been using bucket names with dots in older versions of pgBackRest.  Since this limitation exists for security reasons there appears to be no option but to take a hard line and do our best to notify the user of the issue as clearly as possible.
This commit is contained in:
David Steele 2019-04-08 19:38:06 -04:00
parent 21c83eea59
commit 6099729e92
3 changed files with 73 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/xml/release.xml
View File

@ -14,6 +14,8 @@
<release-list>
<release date="XXXX-XX-XX" version="2.12dev" title="UNDER DEVELOPMENT">
<release-core-list>
<p><b>IMPORTANT NOTE</b>: The new <proper>TLS</proper>/<proper>SSL</proper> implementation forbids dots in <proper>S3</proper> bucket names per RFC-2818. This security fix is required for compliant hostname verification.</p>
<release-bug-list>
<release-item>
<release-item-contributor-list>
@ -48,6 +50,10 @@
<p>Increase <br-option>process-max</br-option> limit to <id>999</id>.</p>
</release-item>
<release-item>
<p>Improve error message when an <proper>S3</proper> bucket name contains dots.</p>
</release-item>
</release-improvement-list>
<release-development-list>

13
src/config/load.c
View File

@ -212,6 +212,19 @@ cfgLoadUpdateOption(void)
}
}
// Error if an S3 bucket name contains dots
if (cfgOptionTest(cfgOptRepoS3Bucket) && cfgOptionBool(cfgOptRepoS3VerifySsl) &&
strChr(cfgOptionStr(cfgOptRepoS3Bucket), '.') != -1)
{
THROW_FMT(
OptionInvalidValueError,
"'%s' is not valid for option '%s'"
"\nHINT: RFC-2818 forbids dots in wildcard matches"
"\nHINT: TLS/SSL verification cannot proceed with this bucket name"
"\nHINT: remove dots from the bucket name",
strPtr(cfgOptionStr(cfgOptRepoS3Bucket)), cfgOptionName(cfgOptRepoS3Bucket));
}
FUNCTION_LOG_RETURN_VOID();
}

54
test/src/module/config/loadTest.c
View File

@ -249,6 +249,60 @@ testRun(void)
"P00 WARN: option 'repo1-retention-diff' is not set for 'repo1-retention-archive-type=diff'\n"
" HINT: to retain differential backups indefinitely (without warning), set option 'repo1-retention-diff'"
" to the maximum.");
// -------------------------------------------------------------------------------------------------------------------------
setenv("PGBACKREST_REPO1_S3_KEY", "mykey", true);
setenv("PGBACKREST_REPO1_S3_KEY_SECRET", "mysecretkey", true);
// Invalid bucket name with verification enabled fails
argList = strLstNew();
strLstAdd(argList, strNew("pgbackrest"));
strLstAdd(argList, strNew("--stanza=db"));
strLstAdd(argList, strNew("--repo1-type=s3"));
strLstAdd(argList, strNew("--repo1-s3-bucket=bogus.bucket"));
strLstAdd(argList, strNew("--repo1-s3-region=region"));
strLstAdd(argList, strNew("--repo1-s3-endpoint=endpoint"));
strLstAdd(argList, strNew("--repo1-path=/repo"));
strLstAdd(argList, strNew("archive-get"));
TEST_ERROR(
harnessCfgLoad(strLstSize(argList), strLstPtr(argList)), OptionInvalidValueError,
"'bogus.bucket' is not valid for option 'repo1-s3-bucket'"
"\nHINT: RFC-2818 forbids dots in wildcard matches"
"\nHINT: TLS/SSL verification cannot proceed with this bucket name"
"\nHINT: remove dots from the bucket name");
// Invalid bucket name with verification disabled succeeds
argList = strLstNew();
strLstAdd(argList, strNew("pgbackrest"));
strLstAdd(argList, strNew("--stanza=db"));
strLstAdd(argList, strNew("--repo1-type=s3"));
strLstAdd(argList, strNew("--repo1-s3-bucket=bogus.bucket"));
strLstAdd(argList, strNew("--repo1-s3-region=region"));
strLstAdd(argList, strNew("--repo1-s3-endpoint=endpoint"));
strLstAdd(argList, strNew("--no-repo1-s3-verify-ssl"));
strLstAdd(argList, strNew("--repo1-path=/repo"));
strLstAdd(argList, strNew("archive-get"));
TEST_RESULT_VOID(harnessCfgLoad(strLstSize(argList), strLstPtr(argList)), "invalid bucket with no verification");
TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoS3Bucket)), "bogus.bucket", " check bucket value");
// Valid bucket name
argList = strLstNew();
strLstAdd(argList, strNew("pgbackrest"));
strLstAdd(argList, strNew("--stanza=db"));
strLstAdd(argList, strNew("--repo1-type=s3"));
strLstAdd(argList, strNew("--repo1-s3-bucket=cool-bucket"));
strLstAdd(argList, strNew("--repo1-s3-region=region"));
strLstAdd(argList, strNew("--repo1-s3-endpoint=endpoint"));
strLstAdd(argList, strNew("--repo1-path=/repo"));
strLstAdd(argList, strNew("archive-get"));
TEST_RESULT_VOID(harnessCfgLoad(strLstSize(argList), strLstPtr(argList)), "valid bucket name");
TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoS3Bucket)), "cool-bucket", " check bucket value");
unsetenv("PGBACKREST_REPO1_S3_KEY");
unsetenv("PGBACKREST_REPO1_S3_KEY_SECRET");
}
// *****************************************************************************************************************************