diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 1e61b99ac..2f4cd0704 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -51,6 +51,10 @@ + +

Set config before Main::main() call to avoid secrets being exposed in a stack trace.

+
+ diff --git a/lib/pgBackRest/Config/Config.pm b/lib/pgBackRest/Config/Config.pm index 2b4fdc1fe..56c8d24e2 100644 --- a/lib/pgBackRest/Config/Config.pm +++ b/lib/pgBackRest/Config/Config.pm @@ -83,7 +83,7 @@ sub configLoad my $bInitLogging = shift; my $strBackRestBin = shift; my $strCommandName = shift; - my $strConfigJson = shift; + my $rstrConfigJson = shift; # Clear option in case it was loaded before %oOption = (); @@ -100,14 +100,14 @@ sub configLoad eval { # Hacky fix for backslashes that need to be escaped - $strConfigJson =~ s/\\/\\\\/g; + $$rstrConfigJson =~ s/\\/\\\\/g; - $rhOption = (JSON::PP->new()->allow_nonref())->decode($strConfigJson); + $rhOption = (JSON::PP->new()->allow_nonref())->decode($$rstrConfigJson); return true; } or do { - confess &log(ASSERT, "$EVAL_ERROR" . (defined($strConfigJson) ? ":\n${strConfigJson}" : "")); + confess &log(ASSERT, "unable to parse config JSON"); }; # Load options into final option hash diff --git a/lib/pgBackRest/Main.pm b/lib/pgBackRest/Main.pm index c84afae35..d6f55d131 100644 --- a/lib/pgBackRest/Main.pm +++ b/lib/pgBackRest/Main.pm @@ -24,6 +24,16 @@ use pgBackRest::Config::Config; use pgBackRest::Protocol::Helper; use pgBackRest::Version; +#################################################################################################################################### +# Set config JSON separately to avoid exposing secrets in the stack trace +#################################################################################################################################### +my $strConfigJson; + +sub configSet +{ + $strConfigJson = shift; +} + #################################################################################################################################### # Main entry point for the library #################################################################################################################################### @@ -31,7 +41,6 @@ sub main { my $strBackRestBin = shift; my $strCommand = shift; - my $strConfigJson = shift; my @stryCommandArg = @_; ################################################################################################################################ @@ -40,9 +49,9 @@ sub main eval { ############################################################################################################################ - # Load command line parameters and config + # Load command line parameters and config -- pass config by reference to hide secrets more than for efficiency ############################################################################################################################ - configLoad(undef, $strBackRestBin, $strCommand, $strConfigJson); + configLoad(undef, $strBackRestBin, $strCommand, \$strConfigJson); # Set test options if (cfgOptionTest(CFGOPT_TEST) && cfgOption(CFGOPT_TEST)) diff --git a/src/perl/exec.c b/src/perl/exec.c index c227b0c37..c444a0b23 100644 --- a/src/perl/exec.c +++ b/src/perl/exec.c @@ -42,6 +42,7 @@ Constants used to build perl options ***********************************************************************************************************************************/ #define PGBACKREST_MODULE PGBACKREST_NAME "::Main" #define PGBACKREST_MAIN PGBACKREST_MODULE "::main" +#define PGBACKREST_CONFIG_SET PGBACKREST_MODULE "::configSet" /*********************************************************************************************************************************** Build list of parameters to use for perl main @@ -57,8 +58,7 @@ perlMain() // Construct Perl main call String *mainCall = strNewFmt( - PGBACKREST_MAIN "('%s','%s','%s'%s)", strPtr(cfgExe()), cfgCommandName(cfgCommand()), strPtr(perlOptionJson()), - strPtr(commandParam)); + PGBACKREST_MAIN "('%s','%s'%s)", strPtr(cfgExe()), cfgCommandName(cfgCommand()), strPtr(commandParam)); return mainCall; } @@ -103,6 +103,9 @@ perlExec() PL_exit_flags |= PERL_EXIT_DESTRUCT_END; perl_run(my_perl); + // Set config data -- this is done separately to avoid it being included in stack traces + eval_pv(strPtr(strNewFmt(PGBACKREST_CONFIG_SET "('%s')", strPtr(perlOptionJson()))), TRUE); + // Run perl main function eval_pv(strPtr(perlMain()), TRUE); } // {uncoverable - perlExec() does not return} diff --git a/test/lib/pgBackRestTest/Env/ConfigEnvTest.pm b/test/lib/pgBackRestTest/Env/ConfigEnvTest.pm index c71b08d41..f2a0c903d 100644 --- a/test/lib/pgBackRestTest/Env/ConfigEnvTest.pm +++ b/test/lib/pgBackRestTest/Env/ConfigEnvTest.pm @@ -115,7 +115,7 @@ sub configTestLoad my @stryArg = $self->commandTestWrite(cfgCommandName($iCommandId), $self->{&CONFIGENVTEST}); my $strConfigJson = cfgParseTest(backrestBin(), join('|', @stryArg)); $self->testResult( - sub {configLoad(false, backrestBin(), cfgCommandName($iCommandId), $strConfigJson)}, + sub {configLoad(false, backrestBin(), cfgCommandName($iCommandId), \$strConfigJson)}, true, 'config load: ' . join(" ", @stryArg)); } diff --git a/test/src/module/perl/execTest.c b/test/src/module/perl/execTest.c index 33950c79e..441b67026 100644 --- a/test/src/module/perl/execTest.c +++ b/test/src/module/perl/execTest.c @@ -19,7 +19,7 @@ testRun() cfgExeSet(strNew("/path/to/pgbackrest")); TEST_RESULT_STR( - strPtr(perlMain()), "pgBackRest::Main::main('/path/to/pgbackrest','info','{}')", "command with no options"); + strPtr(perlMain()), "pgBackRest::Main::main('/path/to/pgbackrest','info')", "command with no options"); // ------------------------------------------------------------------------------------------------------------------------- cfgOptionValidSet(cfgOptCompress, true); @@ -32,7 +32,7 @@ testRun() TEST_RESULT_STR( strPtr(perlMain()), - "pgBackRest::Main::main('/path/to/pgbackrest','info','{\"compress\":{\"source\":\"param\",\"value\":true}}','A','B')", + "pgBackRest::Main::main('/path/to/pgbackrest','info','A','B')", "command with one option and params"); }