Allow secrets to be passed via environment variables.

When environment variables were added in d0b9f986 they were classified as cfgSourceParam, but one of the restrictions on this type is that they can't pass secrets because they might be exposed in the process list.

The solution is to reclassify environment variables as cfgSourceConfig.  This allows them to handle secrets because they will not pass values to subprocesses as parameters.  Instead, each subprocess is expected to check the environment directly during configuration parsing.

In passing, move the error about secrets being passed on the command-line up to command-line parsing and make the error more generic with respect to the configuration file now that multiple configuration files are allowed.

src/config/config.h

@@ -72,7 +72,7 @@ typedef enum
 {
     cfgSourceDefault,                                               // Default value
     cfgSourceParam,                                                 // Passed as command-line parameter
-    cfgSourceConfig,                                                // From configuration file
+    cfgSourceConfig,                                                // From configuration file or environment variable
 } ConfigSource;
 
 /***********************************************************************************************************************************

src/config/parse.c

@@ -460,6 +460,17 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
                     // Make sure the option id is valid
                     ASSERT_DEBUG(optionId < CFG_OPTION_TOTAL);
 
+                    // Error if this option is secure and cannot be passed on the command line
+                    if (cfgDefOptionSecure(cfgOptionDefIdFromId(optionId)))
+                    {
+                        THROW_FMT(
+                            OptionInvalidError,
+                            "option '%s' is not allowed on the command-line\n"
+                            "HINT: this option could expose secrets in the process list.\n"
+                            "HINT: specify the option in a configuration file or an environment variable instead.",
+                            cfgOptionName(optionId));
+                    }
+
                     // If the the option has not been found yet then set it
                     if (!parseOptionList[optionId].found)
                     {
@@ -596,7 +607,7 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
                         continue;
 
                     parseOptionList[optionId].found = true;
-                    parseOptionList[optionId].source = cfgSourceParam;
+                    parseOptionList[optionId].source = cfgSourceConfig;
 
                     // Convert boolean to string
                     if (cfgDefOptionType(optionDefId) == cfgDefOptTypeBoolean)
@@ -733,7 +744,7 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
                             continue;
                         }
 
-                        // Continue if this option has already been found in another section
+                        // Continue if this option has already been found in another section or command-line/environment
                         if (parseOptionList[optionId].found)
                             continue;
 
@@ -792,17 +803,6 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
                         cfgCommandName(cfgCommand()));
                 }
 
-                // Error if this option is secure and cannot be passed on the command line
-                if (parseOption->found && parseOption->source == cfgSourceParam && cfgDefOptionSecure(optionDefId))
-                {
-                    THROW_FMT(
-                        OptionInvalidError,
-                        "option '%s' is not allowed on the command-line\n"
-                        "HINT: this option could expose secrets in the process list.\n"
-                        "HINT: specify the option in '%s' instead.",
-                        cfgOptionName(optionId), cfgDefOptionDefault(commandDefId, cfgDefOptConfig));
-                }
-
                 // Error if this option does not allow multiple arguments
                 if (parseOption->valueList != NULL && strLstSize(parseOption->valueList) > 1 &&
                     !(cfgDefOptionType(cfgOptionDefIdFromId(optionId)) == cfgDefOptTypeHash ||

test/src/module/config/parseTest.c

@@ -739,7 +739,7 @@ testRun(void)
             configParse(strLstSize(argList), strLstPtr(argList), false), OptionInvalidError,
             "option 'repo1-s3-key' is not allowed on the command-line\n"
             "HINT: this option could expose secrets in the process list.\n"
-            "HINT: specify the option in '/etc/pgbackrest/pgbackrest.conf' instead.");
+            "HINT: specify the option in a configuration file or an environment variable instead.");
 
         // -------------------------------------------------------------------------------------------------------------------------
         argList = strLstNew();
@@ -747,12 +747,11 @@ testRun(void)
         strLstAdd(argList, strNew("--pg1-path=/path/to/db"));
         strLstAdd(argList, strNew("--no-config"));
         strLstAdd(argList, strNew("--stanza=db"));
-        setenv("PGBACKREST_REPO1_S3_HOST", "xxx", true);
+        strLstAdd(argList, strNew("--repo1-s3-host=xxx"));
         strLstAdd(argList, strNew(TEST_COMMAND_BACKUP));
         TEST_ERROR(
             configParse(strLstSize(argList), strLstPtr(argList), false), OptionInvalidError,
             "option 'repo1-s3-host' not valid without option 'repo1-type' = 's3'");
-        unsetenv("PGBACKREST_REPO1_S3_HOST");
 
         // -------------------------------------------------------------------------------------------------------------------------
         argList = strLstNew();
@@ -1032,7 +1031,13 @@ testRun(void)
         strLstAdd(argList, strNew("--pg1-path=/path/to/db"));
         strLstAdd(argList, strNew("--no-online"));
         strLstAdd(argList, strNew("--no-config"));
+        strLstAdd(argList, strNew("--repo1-type=s3"));
+        strLstAdd(argList, strNew("--repo1-s3-bucket=test"));
+        strLstAdd(argList, strNew("--repo1-s3-endpoint=test"));
+        strLstAdd(argList, strNew("--repo1-s3-region=test"));
         strLstAdd(argList, strNew(TEST_COMMAND_BACKUP));
+        setenv("PGBACKREST_REPO1_S3_KEY", "xxx", true);
+        setenv("PGBACKREST_REPO1_S3_KEY_SECRET", "xxx", true);
         TEST_RESULT_VOID(configParse(strLstSize(argList), strLstPtr(argList), false), TEST_COMMAND_BACKUP " command");
         TEST_RESULT_INT(cfgCommand(), cfgCmdBackup, "    command is " TEST_COMMAND_BACKUP);
 
@@ -1046,6 +1051,8 @@ testRun(void)
         TEST_RESULT_INT(cfgOptionSource(cfgOptStanza), cfgSourceParam, "    stanza is source param");
         TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgPath)), "/path/to/db", "    pg1-path is set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptPgPath), cfgSourceParam, "    pg1-path is source param");
+        TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoS3KeySecret)), "xxx", "    repo1-s3-secret is set");
+        TEST_RESULT_INT(cfgOptionSource(cfgOptRepoS3KeySecret), cfgSourceConfig, "    repo1-s3-secret is source env");
         TEST_RESULT_BOOL(cfgOptionBool(cfgOptOnline), false, "    online is not set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptOnline), cfgSourceParam, "    online is source default");
         TEST_RESULT_BOOL(cfgOptionBool(cfgOptCompress), true, "    compress is set");
@@ -1053,6 +1060,9 @@ testRun(void)
         TEST_RESULT_INT(cfgOptionInt(cfgOptBufferSize), 4194304, "    buffer-size is set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptBufferSize), cfgSourceDefault, "    buffer-size is source default");
 
+        unsetenv("PGBACKREST_REPO1_S3_KEY");
+        unsetenv("PGBACKREST_REPO1_S3_KEY_SECRET");
+
         // -------------------------------------------------------------------------------------------------------------------------
         argList = strLstNew();
         strLstAdd(argList, strNew(TEST_BACKREST_EXE));
@@ -1117,11 +1127,11 @@ testRun(void)
         TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgPath)), "/path/to/db", "    pg1-path is set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptPgPath), cfgSourceConfig, "    pg1-path is source config");
         TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgSocketPath)), "/path/to/socket", "    pg1-socket-path is set");
-        TEST_RESULT_INT(cfgOptionSource(cfgOptPgSocketPath), cfgSourceParam, "    pg1-socket-path is source param");
+        TEST_RESULT_INT(cfgOptionSource(cfgOptPgSocketPath), cfgSourceConfig, "    pg1-socket-path is config param");
         TEST_RESULT_BOOL(cfgOptionBool(cfgOptOnline), false, "    online not is set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptOnline), cfgSourceParam, "    online is source param");
         TEST_RESULT_BOOL(cfgOptionBool(cfgOptStartFast), false, "    start-fast not is set");
-        TEST_RESULT_INT(cfgOptionSource(cfgOptStartFast), cfgSourceParam, "    start-fast is source param");
+        TEST_RESULT_INT(cfgOptionSource(cfgOptStartFast), cfgSourceConfig, "    start-fast is config param");
         TEST_RESULT_BOOL(cfgOptionBool(cfgOptCompress), false, "    compress not is set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptCompress), cfgSourceConfig, "    compress is source config");
         TEST_RESULT_BOOL(cfgOptionTest(cfgOptArchiveCheck), false, "    archive-check is not set");