Skip to content
Permalink
Browse files

Enforce requiring repo-cipher-pass at config parse time.

This was not enforced at parse time because repo1-cipher-type could be passed on the command-line even in cases where encryption was not needed by the subprocess.

Filter repo-cipher-type so it is never passed on the command line.  If the subprocess does not have access to the passphrase then knowing the encryption type is useless anyway.
  • Loading branch information...
dwsteele committed Jun 5, 2019
1 parent d7bd0c5 commit 6ff3325c7744a8f6f0e1e965fa673040a2f40a71
@@ -1454,7 +1454,7 @@ my %hConfigDefine =
&CFGDEF_PREFIX => CFGDEF_PREFIX_REPO,
&CFGDEF_INDEX_TOTAL => CFGDEF_INDEX_REPO,
&CFGDEF_SECURE => true,
&CFGDEF_REQUIRED => false,
&CFGDEF_REQUIRED => true,
&CFGDEF_DEPEND =>
{
&CFGDEF_DEPEND_OPTION => CFGOPT_REPO_CIPHER_TYPE,
@@ -356,7 +356,7 @@ sub cfgCommandWrite
my $bSecure = cfgDefOptionSecure($iOptionId);

# Skip option if it is secure and should not be output in logs or the command line
next if ($bSecure && !$bDisplayOnly);
next if (($bSecure || $iOptionId == CFGOPT_REPO_CIPHER_TYPE) && !$bDisplayOnly);

# Process any option id overrides first
if (defined($oOptionOverride->{$iOptionId}))
@@ -2629,7 +2629,7 @@ static ConfigDefineOptionData configDefineOptionData[] = CFGDEFDATA_OPTION_LIST
CFGDEFDATA_OPTION
(
CFGDEFDATA_OPTION_NAME("repo-cipher-pass")
CFGDEFDATA_OPTION_REQUIRED(false)
CFGDEFDATA_OPTION_REQUIRED(true)
CFGDEFDATA_OPTION_SECTION(cfgDefSectionGlobal)
CFGDEFDATA_OPTION_TYPE(cfgDefOptTypeString)
CFGDEFDATA_OPTION_INTERNAL(false)
@@ -32,9 +32,14 @@ cfgExecParam(ConfigCommand commandId, const KeyValue *optionReplace)
{
ConfigDefineOption optionDefId = cfgOptionDefIdFromId(optionId);

// Skip the option if it is not valid for the specified command or if is secure
if (!cfgDefOptionValid(commandDefId, optionDefId) || cfgDefOptionSecure(optionDefId))
// Skip the option if it is not valid for the specified command or if is secure. Also skip repo1-cipher-type because
// there's no point of passing it if the other process doesn't have access to repo1-cipher-pass. There is probably a
// better way to do this...
if (!cfgDefOptionValid(commandDefId, optionDefId) || cfgDefOptionSecure(optionDefId) ||
optionDefId == cfgDefOptRepoCipherType)
{
continue;
}

// First check for a replacement
const Variant *key = VARSTRZ(cfgOptionName(optionId));
@@ -7670,7 +7670,7 @@ static const EmbeddedModule embeddedModule[] =
"my $strOption = cfgOptionName($iOptionId);\n"
"my $bSecure = cfgDefOptionSecure($iOptionId);\n"
"\n\n"
"next if ($bSecure && !$bDisplayOnly);\n"
"next if (($bSecure || $iOptionId == CFGOPT_REPO_CIPHER_TYPE) && !$bDisplayOnly);\n"
"\n\n"
"if (defined($oOptionOverride->{$iOptionId}))\n"
"{\n"
@@ -125,11 +125,9 @@ sub run
{
my $strStanzaEncrypt = 'test-encrypt';
$self->optionTestSet(CFGOPT_REPO_CIPHER_TYPE, CFGOPTVAL_REPO_CIPHER_TYPE_AES_256_CBC);
$self->configTestLoad(CFGCMD_ARCHIVE_PUSH);

# Encryption passphrase required when encryption type not 'none' (default)
$self->testException(sub {storageRepo({strStanza => $strStanzaEncrypt})}, ERROR_ASSERT, 'option ' .
cfgOptionName(CFGOPT_REPO_CIPHER_PASS) . ' is required');
$self->testException(
sub {$self->configTestLoad(CFGCMD_ARCHIVE_PUSH)}, ERROR_OPTION_REQUIRED,
'archive-push command requires option: repo1-cipher-pass');

# Set the encryption passphrase and confirm passphrase and type have been set in the storage object
$self->optionTestSet(CFGOPT_REPO_CIPHER_PASS, 'x');
@@ -24,19 +24,18 @@ testRun(void)
strLstAddZ(argList, "--reset-neutral-umask");
strLstAddZ(argList, "--repo-cipher-type=aes-256-cbc");
strLstAddZ(argList, "archive-get");
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));

// Set repo1-cipher-pass to make sure it is not passed on the command line
cfgOptionValidSet(cfgOptRepoCipherPass, true);
cfgOptionSet(cfgOptRepoCipherPass, cfgSourceConfig, varNewStrZ("1234"));
setenv("PGBACKREST_REPO1_CIPHER_PASS", "1234", true);
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
unsetenv("PGBACKREST_REPO1_CIPHER_PASS");

TEST_RESULT_STR(
strPtr(strLstJoin(cfgExecParam(cfgCmdLocal, NULL), "|")),
strPtr(
strNewFmt(
"--no-config|--log-subprocess|--pg1-path=\"%s/db path\"|--repo1-cipher-type=aes-256-cbc|--repo1-path=%s/repo"
"|--stanza=test1|local",
testPath(), testPath())),
"--no-config|--log-subprocess|--pg1-path=\"%s/db path\"|--repo1-path=%s/repo|--stanza=test1|local", testPath(),
testPath())),
"exec archive-get -> local");

// -------------------------------------------------------------------------------------------------------------------------

0 comments on commit 6ff3325

Please sign in to comment.
You can’t perform that action at this time.